| Message ID | 20181023120454.28553-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | riscv decodetree followup | expand |
On 10/23/18 2:04 PM, Richard Henderson wrote: > ??? Needs simultaneous corresponding changes to all > translators using decodetree. > --- > scripts/decodetree.py | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > Was the only intend of the insn argument to be used for manual decoding in a trans_ function? Cheers, Bastian
On 10/23/18 2:04 PM, Bastian Koppelmann wrote: > On 10/23/18 2:04 PM, Richard Henderson wrote: >> ??? Needs simultaneous corresponding changes to all >> translators using decodetree. >> --- >> scripts/decodetree.py | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> > > Was the only intend of the insn argument to be used for manual decoding in a > trans_ function? I didn't quite know what it might be used for, and thought I was planning ahead. Now I see that it's probably counter-productive. In particular, think of how you were calling the trans_foo functions yourself, passing along the uint16_t input to the uint32_t output. Which would have worked not at all had you actually been using it for decode within the insn32 code base. r~
On 23/10/18 15:08, Richard Henderson wrote: > On 10/23/18 2:04 PM, Bastian Koppelmann wrote: >> On 10/23/18 2:04 PM, Richard Henderson wrote: >>> ??? Needs simultaneous corresponding changes to all >>> translators using decodetree. >>> --- >>> scripts/decodetree.py | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >> >> Was the only intend of the insn argument to be used for manual decoding in a >> trans_ function? > > I didn't quite know what it might be used for, and thought I was planning > ahead. Now I see that it's probably counter-productive. I agree. > > In particular, think of how you were calling the trans_foo functions yourself, > passing along the uint16_t input to the uint32_t output. Which would have > worked not at all had you actually been using it for decode within the insn32 > code base. > > > r~ >
On 10/23/18 3:08 PM, Richard Henderson wrote: > On 10/23/18 2:04 PM, Bastian Koppelmann wrote: >> On 10/23/18 2:04 PM, Richard Henderson wrote: >>> ??? Needs simultaneous corresponding changes to all >>> translators using decodetree. >>> --- >>> scripts/decodetree.py | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >> Was the only intend of the insn argument to be used for manual decoding in a >> trans_ function? > I didn't quite know what it might be used for, and thought I was planning > ahead. Now I see that it's probably counter-productive. Richard, can you pick up the first two patches together with the insn arg removal for openrisc and ARM, such that we don't break bisecting? I then squash the rest of the patches into my patch-set. Cheers, Bastian
On 10/23/18 2:25 PM, Bastian Koppelmann wrote: > Richard, can you pick up the first two patches together with the insn arg > removal for openrisc and ARM, such that we don't break bisecting? I then squash > the rest of the patches into my patch-set. I've now pushed these two to a new branch off master: https://github.com/rth7680/qemu.git decodetree r~
diff --git a/scripts/decodetree.py b/scripts/decodetree.py index a9b10452ef..c0bb447095 100755 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -468,8 +468,7 @@ class Pattern(General): output('typedef ', self.base.base.struct_name(), ' arg_', self.name, ';\n') output(translate_scope, 'bool ', translate_prefix, '_', self.name, - '(DisasContext *ctx, arg_', self.name, - ' *a, ', insntype, ' insn);\n') + '(DisasContext *ctx, arg_', self.name, ' *a);\n') def output_code(self, i, extracted, outerbits, outermask): global translate_prefix @@ -481,7 +480,7 @@ class Pattern(General): for n, f in self.fields.items(): output(ind, 'u.f_', arg, '.', n, ' = ', f.str_extract(), ';\n') output(ind, 'return ', translate_prefix, '_', self.name, - '(ctx, &u.f_', arg, ', insn);\n') + '(ctx, &u.f_', arg, ');\n') # end Pattern