[PATCH] Change MachineOperand OpKind to a bitfield

[Pete Cooper]

Thanks Rafael.  r235528.

Also filed a clang bug (https://llvm.org/bugs/show_bug.cgi?id=23320 <https://llvm.org/bugs/show_bug.cgi?id=23320>) and a very similar llvm optimizer bug (https://llvm.org/bugs/show_bug.cgi?id=23321 <https://llvm.org/bugs/show_bug.cgi?id=23321>).

Cheers
Pete
> On Apr 22, 2015, at 8:46 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> 
> Thanks.
> 
> We probably shouldn't block improving clang on the potential for optimizing the existing code.
> 
> How about committing you patch and opening bugs on llvm and clang?
> 
> On Apr 20, 2015 8:36 PM, "Pete Cooper" <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> 
> > On Apr 20, 2015, at 5:25 PM, Matthias Braun <mbraun at apple.com <mailto:mbraun at apple.com>> wrote:
> >
> >
> >> On Apr 20, 2015, at 4:10 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> >>
> >> Hi Rafael
> >>
> >> MachineOperand::OpKind is an unsigned char, followed by 24-bits of bitfields.  clang is able to make all of that take 32-bits of space, but it generates pretty nasty code to do so.
> >>
> >> OpKind itself is loaded and stored as an i8 and so gets very nice IR.  However, loading an entry from one of the bitfields results in a load of an i24, then masking operations.  Given that i24 isn’t legal, the legalizer goes to great lengths to make this work, typically splitting it in to i8 and i16 operations.
> > I don't know the actual clang code, but isn't the canonical codegen for something like the following "unsigned SubReg_TargetFlags : 12;" to load an "unsigned" value and shift/mask that? Why does it use i24?
> Attached code to demonstrate this.  Here’s a reduced version of MachineOperand compiled with and without the change.
> 
> To answer your question, clang has decided that the 24 bits total of bitfields should be represented as a [3 x i8].  So to read one of the bitfields, it casts the [3 x i8]* to i24* then does a load.  I guess it just wants to always load and store the whole bitfield when operating on any one piece of it.
> 
> I’m not sure if this is a requirement of C++ or not.  Personally, i’d like clang to use powers of 2 which are a multiple of 8 (i.e., 8, 16, 32, 64) for bitfield operations in the IR, but i don’t know if it can or not.
> 
> Pete
> 
> 
> 
> >
> > - Matthias
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150422/b5f66c65/attachment.html>