[llvm-commits] [pr12251][patch] Use i1* for boolean loads and stores

[Duncan Sands]

Hi Rafael,

> In the testcase of pr12251 we use a i8* for loading a boolean value:
>
>    %3 = load i8* %2
>    %4 = trunc i8 %3 to i
>
> The problem then is that LLVM has no way of knowing that %3 can only
> be 0 or 1. A C file using uint8_t would have compiled to the same IL.
>
> It would be nice to have a general way of telling LLVM that a value is
> known to be in a range, but for the specific cases of booleans we can
> do better by just using a more restrictive type. This patch changes
> clang to use i1 pointers for doing the loads and stores. This changes
> the generated code from
>
>      movb    (%rdi), %al
>      andb    $1, %al
>      ret
>
> to just
>
>      movb    (%rdi), %al
>      ret
>
> I think the idea can be extended for systems that use 32 bit booleans.
> We would just have to be careful with endianess. I got the idea from
> http://llvm.org/bugs/show_bug.cgi?id=8730#c3.
>
> Is this patch OK?

the language ref currently says that it is undefined what gets stored in the
other 7 bits when you write an i1.  That was just to give freedom to be
efficient, but should be changed to say "target dependent" or something like
that if you want to go this way.

There are also some real problems.  First off, writing an i1 will only ever
write a byte, but doesn't ppc or some platform like that use 32 bit booleans?

That aside, the other problem is that the upper 7 bits are currently always
set to zero.  That's no good for platforms that want all 8 bits to be all
zero or all one, so the logic would need to change for them (see below).
Various places (the DAG combiner?) know about the top 7 bits being zero, so
they would need to be tweaked too.

OK, so where does the "setting the top bits to zero" happen?  When a store
of i1 hits the type legalizer, the promoted value (usually i8) is stored
instead.  The upper bits of the promoted value are undefined , so you may
wonder why I say the store will zero them.  The reason is that the store
is promoted to a trunc store, not an ordinary store, a trunc store of i8 to
i1.  As this i1 memory type isn't an operand or return type of the store (it
is hidden away in the store node somewhere) it goes flying through the type
legalizer without anything happening to it, and hits LegalizeDAG.

In LegalizeDAG it hits this code (starting at line 1225):

       if (StWidth != StVT.getStoreSizeInBits()) {
         // Promote to a byte-sized store with upper bits zero if not
         // storing an integral number of bytes.  For example, promote
         // TRUNCSTORE:i1 X -> TRUNCSTORE:i8 (and X, 1)
         EVT NVT = EVT::getIntegerVT(*DAG.getContext(),
                                     StVT.getStoreSizeInBits());
         Tmp3 = DAG.getZeroExtendInReg(Tmp3, dl, StVT);
         SDValue Result =
           DAG.getTruncStore(Tmp1, dl, Tmp3, Tmp2, ST->getPointerInfo(),
                             NVT, isVolatile, isNonTemporal, Alignment);
         ReplaceNode(SDValue(Node, 0), Result);

This is probably where it should be changed to look at getBooleanContents and
either use zero bits or same-bits depending on what that says.

In the case of a load the real logic also occurs in LegalizeDAG and needs to be
adjusted too.

Ciao, Duncan.