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

[Peter Cooper]

Hi Duncan

Thanks for the detailed explanation!   I've reverted this in r163064.

I can now see what you mean about handling this in the select.  I need to move the code i have here to the legalization of vselect where it wasn't extending the boolean condition correctly if turning a vselect in to and, or, xor.

Thanks,
Pete
On Sep 1, 2012, at 9:35 AM, Duncan Sands <baldrick at free.fr> wrote:

> 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
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits