[llvm-commits] [PATCH] Fix decoding of shift-immediate #32

Richard Barton Richard.Barton at arm.com
Fri Mar 16 11:12:07 PDT 2012


ping.

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Richard Barton
> Sent: 14 March 2012 16:04
> To: Jim Grosbach
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Fix decoding of shift-immediate #32
>
> Hi Jim
>
> Thanks for the review and the clarification on the MCInst contents, I will
> bear this rule-of-thumb in mind in future.
>
> I have reversed my patch to correct the AsmParser behaviour to store #0 in the
> MCInst rather than #32, and have removed the special case handling in the
> CodeEmitter for #32.
>
> Please review.
>
> Thanks
> Rich
>
> > -----Original Message-----
> > From: Jim Grosbach [mailto:grosbach at apple.com]
> > Sent: 13 March 2012 23:18
> > To: Richard Barton
> > Cc: llvm-commits at cs.uiuc.edu
> > Subject: Re: [llvm-commits] [PATCH] Fix decoding of shift-immediate #32
> >
> > Hi Richard,
> >
> > MC tries to represent instruction operands with the values that will be
> > encoded into the instruction, not the semantic meaning. The arm backend isnt
> > yet completely consistent about it, but that's the general idea. In this
> case,
> > that means we should use an mcoperand value of zero, as that's what the
> > encoded value is.
> >
> > Anything creating an operand value of 32 for this is in error. It's getting
> > away with it because the encoder will just use the low 5 bits, but it's
> still
> > undesirable to rely on that.
> >
> > Jim
> >
> > On Mar 13, 2012, at 11:16 AM, Richard Barton <richard.barton at arm.com> wrote:
> >
> > > With the patch attached this time! Apologies.
> > >
> > >> Hello reviewers,
> > >>
> > >> An MCInst stores a register-shifted-immediate as 2 operands, a register
> and
> > an
> > >> immediate operand describing the shift kind and the shift value. This
> > >> immediate operand value is encoded as 3 bits for the shift kind, which is
> > an
> > >> enum type ShiftOpc, and 5 bits for the shift value itself. The assembler
> > >> creates an MCInst immediate operand with the value #32 for ... lsr #32,
> > whilst
> > >> the decoder creates an MCInst immediate operand with the value 0 for the
> > >> equivalent encoding. On the face of it, neither is more right than the
> > other,
> > >> and crucially the two paths followed by llvm-mc (assemble, encode) and
> > >> (decode, disassemble) match up in their expectations.
> > >>
> > >> Although it probably does not matter either way, intuitively I think it
> > makes
> > >> more sense for the decoder to write an MCInst operand describing the
> shift
> > as
> > >> [32][lsr] rather than [0][lsr] as the latter is not actually legal.
> > >>
> > >> The attached patch handles this case for the decoder. It also reverts
> part
> > of
> > >> r137322, which added special case handling for the old decoder behaviour
> in
> > >> the instruction printer.
> > >>
> > >> Please review.
> > >>
> > >> Thanks,
> > >>
> > >> Richard Barton
> > > <decode_lsr_asr_32.patch>
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium.  Thank you.

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.





More information about the llvm-commits mailing list