[llvm-commits] [llvm] r140463 - /llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp

[Duncan Sands]

Hi Nadav,

> [Vector-Select] Address one of the problems in 10902.
>
> When generating the trunc-store of i1's, we need to use the vector type and not
> the scalar type.
>
> This patch fixes the assertion in CodeGen/Generic/bool-vector.ll when
> running with -promote-elements.

I think this is wrong.  Maybe wrong is too harsh :)  It was decided that vectors
would be stored bit-packed.  This doesn't make any difference for vectors of i8,
i32 etc.  But it means that <4 x i1> will be stored the same as an i4, namely
in 4 consecutive bits of one byte.  Your patch will store <4 x i1> using 4
bytes.  I'm not totally convinced that bit-packing is the right thing to do.
But even if we were to go with your method there are other problems: things
like the store size etc need to be consistent with what is actually done.  The
store size for <4 x i1> is (IIRC) 1 byte, but you are overwriting 4 bytes, so
alias analysis (which uses the store size) is going to return wrong results.
Also, the store size must never be larger than the type alloc size, so if you
change the store size you will need to touch the alloc size too.  However the
alloc size needs to agree with sizeof in C, so you will have to fix clang as
well (since it can produce these kinds of types IIRC).  There are a bunch of
places that assume that the store size is the size rounded up to the nearest
byte, so these will have to be fixed too.  And so on.  Getting this right,
having all parts of LLVM work correctly together, is a large project in
itself.  I doubt you want to go there.  I think you should revert this patch.
I'd rather have codegen fail with an assertion rather than silently produce
wrong code.  In fact it would be great if you added even more assertions!

Ciao, Duncan.

>
>
> Modified:
>      llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp?rev=140463&r1=140462&r2=140463&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp Sat Sep 24 13:32:19 2011
> @@ -1180,6 +1180,10 @@
>         // bytes.  For example, promote EXTLOAD:i20 ->  EXTLOAD:i24.
>         unsigned NewWidth = SrcVT.getStoreSizeInBits();
>         EVT NVT = EVT::getIntegerVT(*DAG.getContext(), NewWidth);
> +      if (SrcVT.isVector()) {
> +        NVT = EVT::getVectorVT(*DAG.getContext(), NVT,
> +                               SrcVT.getVectorNumElements());
> +      }
>         SDValue Ch;
>
>         // The extra bits are guaranteed to be zero, since we stored them that
> @@ -1521,7 +1525,12 @@
>           // TRUNCSTORE:i1 X ->  TRUNCSTORE:i8 (and X, 1)
>           EVT NVT = EVT::getIntegerVT(*DAG.getContext(),
>                                       StVT.getStoreSizeInBits());
> -        Tmp3 = DAG.getZeroExtendInReg(Tmp3, dl, StVT);
> +        if (StVT.isVector()) {
> +          NVT = EVT::getVectorVT(*DAG.getContext(), NVT,
> +                                 StVT.getVectorNumElements());
> +        }
> +
> +        Tmp3 = DAG.getZeroExtendInReg(Tmp3, dl, StVT.getScalarType());
>           Result = DAG.getTruncStore(Tmp1, dl, Tmp3, Tmp2, ST->getPointerInfo(),
>                                      NVT, isVolatile, isNonTemporal, Alignment);
>         } else if (StWidth&  (StWidth - 1)) {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits