[llvm-commits] [llvm] r162960 - in /llvm/trunk: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp test/CodeGen/X86/vec_select.ll

[Duncan Sands]

Hi Pete,

On 31/08/12 01:58, Pete Cooper wrote:
> Author: pete
> Date: Thu Aug 30 18:58:52 2012
> New Revision: 162960
>
> URL: http://llvm.org/viewvc/llvm-project?rev=162960&view=rev
> Log:
> Take account of boolean vector contents when promoting a build vector from i1 to some other type.  rdar://problem/12210060

unfortunately this is wrong, it should be in the "select" code.  A general
principle of type legalization is that extra bits are undefined, hold rubbish.
Thus when turning an i1 value into an i8 value, or a <4 x i1> value into
a <4 x i8> value (or whatever) then the upper 7 bits of each i8 value are
undefined.  Feeding such promoted values directly into some operations is
fine, for other operations it is not.  For example consider a sum of i1 values:
x = add i1 y, z.  If Y and Z are the corresponding i8 values (bit 0 of Y
equals y, other bits might be anything, etc), then Y+Z has the same bit 0 as
x, i.e. any rubbish in the upper bits of Y and Z has no effect on bit 0 of
the sum, so Y+Z can be used as the promoted value for x.  For other operations
directly feeding the promoted values in is not OK, for example consider x =
udiv y, z.  You can't use udiv Y, Z as the promoted value for x.  That's
because value of the upper bits in Y and Z does impact bit 0 of udiv Y, Z.
That's why when the legalization code visits a udiv, it first forms Y' and Z'
by clearing the upper 7 bits of Y and Z to 0, and then forms X = udiv Y', Z'
(which has the correct bit 0 so can be used as the promoted value for x).

Select and setcc nodes are an example of operations of the second kind: you
can't just take their i1 operand c and replace it with any value C with the
same bit 0.  You first need to form C' from C by zeroing off the top 7 bits,
or sign extending bit 0 (depending on the machine), and then use C' for the
new operand.  Notice how the existing uses of getBooleanContents (which says
which method to use) are all in legalization of (v)select or (v)setcc nodes.

In short you shouldn't be trying to fix up bits in outputs (build_vector), you
should be fixing them up on inputs (select/setcc).

So can you please correct the patch.

Best wishes, Duncan.

>
> Added:
>      llvm/trunk/test/CodeGen/X86/vec_select.ll
> Modified:
>      llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp?rev=162960&r1=162959&r2=162960&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp Thu Aug 30 18:58:52 2012
> @@ -2913,8 +2913,24 @@
>
>     SmallVector<SDValue, 8> Ops;
>     Ops.reserve(NumElems);
> +  unsigned ExtendOp = ISD::ANY_EXTEND;
> +  // Extending boolean constants needs to consider the
> +  // value boolean vector constants take on this target and extend
> +  // with sign or zeros appropriately.
> +  if (OutVT.getVectorElementType() == MVT::i1) {
> +    switch (TLI.getBooleanContents(true)) {
> +      case TargetLowering::UndefinedBooleanContent:
> +        break;
> +      case TargetLowering::ZeroOrOneBooleanContent:
> +      ExtendOp = ISD::ZERO_EXTEND;
> +        break;
> +      case TargetLowering::ZeroOrNegativeOneBooleanContent:
> +        ExtendOp = ISD::SIGN_EXTEND;
> +        break;
> +    }
> +  }
>     for (unsigned i = 0; i != NumElems; ++i) {
> -    SDValue Op = DAG.getNode(ISD::ANY_EXTEND, dl, NOutVTElem, N->getOperand(i));
> +    SDValue Op = DAG.getNode(ExtendOp, dl, NOutVTElem, N->getOperand(i));
>       Ops.push_back(Op);
>     }
>
>
> Added: llvm/trunk/test/CodeGen/X86/vec_select.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/vec_select.ll?rev=162960&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/vec_select.ll (added)
> +++ llvm/trunk/test/CodeGen/X86/vec_select.ll Thu Aug 30 18:58:52 2012
> @@ -0,0 +1,16 @@
> +; RUN: llc < %s -march=x86 | FileCheck %s
> +
> +; When legalizing the v4i1 constant, we need to consider the boolean contents
> +; For x86 a boolean vector constant is all ones so the constants in memory
> +; will be ~0U not 1.
> +
> +; CHECK: .long	4294967295
> +; CHECK: .long	4294967295
> +; CHECK: .long	0
> +; CHECK: .long	0
> +
> +; CHECK: test
> +define <4 x i8> @test(<4 x i8> %a, <4 x i8> %b) {
> +  %sel = select <4 x i1> <i1 true, i1 true, i1 false, i1 false>, <4 x i8> %a, <4 x i8> %b
> +	ret <4 x i8> %sel
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>