Re: [AArch64] PR target/20666, fix wrong encoding of new introduced BFC pseudo

[Jiong Wang <jiong dot wang at foss dot arm dot com>]

On 07/11/16 12:26, Jan Beulich wrote:
> > > > On 07.11.16 at 13:22, <jiong.wang@foss.arm.com> wrote:
> > On 07/11/16 11:55, Jan Beulich wrote:
> > > > > > On 10.10.16 at 18:40, <jiong.wang@foss.arm.com> wrote:
> > > > On 24/11/15 11:17, Matthew Wahab wrote:
> > > > > opcodes/
> > > > > 2015-11-24  Matthew Wahab  <matthew.wahab@arm.com>
> > > > >
> > > > >       * aarch64-asm-2.c: Regenerate.
> > > > >       * aarch64-asm.c (convert_bfc_to_bfm): New.
> > > > There looks like a copy & paste error in this function, and caused PR
> > > > target/20666.
> > > >
> > > > +convert_bfc_to_bfm (aarch64_inst *inst)
> > > > +{
> > > > +  int64_t lsb, width;
> > > > +
> > > > +  /* Insert XZR.  */
> > > > +  copy_operand_info (inst, 3, 2);
> > > > +  copy_operand_info (inst, 2, 1);
> > > > +  copy_operand_info (inst, 2, 0);
> > > >                                ^~~~~~~ should be "0".
> > > Why 0? That makes this particular copy_operand_info() invocation
> > > a no-op afaict. Aiui you mean 1 here instead.
> >     I mean 0 there.  as  "BFC Xd, lsb, width" is the simplified form of
> > "BFI Xd, XZR, lsb, width".
> >
> >     Operands 1 is forced to XZR by the following code:
> >         inst->operands[1].reg.regno = 0x1f;
> No, that only changes the register number, not the actual operand
> type (which is still an immediate). The  copy_operand_info() with 1
> as the second argument would clone operand 0 (which already is a
> register) and then update the register number. Only that will
> guarantee a full fledged register operand (in case any other entity
> cares to look at it).
Hmm.. The types update might be fine, as there will be a 
aarch64_replace_opcode call for
all such opcode conversion, but the qualifier is not handled so there 
might be hiding issues.

Do you mind sending a patch for this?

Thanks.

Regards,
Jiong