[llvm-commits] [llvm] r62790 - /llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

[Dan Gohman]

On Jan 22, 2009, at 3:21 PM, Chris Lattner wrote:

> On Jan 22, 2009, at 2:05 PM, Bob Wilson wrote:
>> URL: http://llvm.org/viewvc/llvm-project?rev=62790&view=rev
>> Log:
>> Fix a minor bug in DAGCombiner's folding of SELECT.  Folding "select
>> C, 0, 1"
>> to "C ^ 1" is only valid when C is known to be either 0 or 1.  Most
>> of the
>> similar foldings in this function only handle "i1" types, but this
>> one appears
>> intentionally written to handle larger integer types.  If C has an
>> integer
>> type larger than "i1", this needs to check if the high bits of a
>> boolean
>> are known to be zero.  I also changed the comment to describe this
>> folding as
>> "C ^ 1" instead of "~C", since that is what the code does and since
>> the latter
>> would only be valid for "i1" types.  The good news is that most LLVM
>> targets
>> use TargetLowering::ZeroOrOneBooleanContent so this change will not
>> disable
>> the optimization; the bad news is that I've been unable to come up
>> with a
>> testcase to demonstrate the problem.
>
> Hi Bob,
>
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Thu Jan 22
>> 16:05:48 2009
>> @@ -2746,8 +2746,11 @@
>>  // fold select C, 1, X -> C | X
>>  if (VT == MVT::i1 && N1C && N1C->getAPIntValue() == 1)
>>    return DAG.getNode(ISD::OR, VT, N0, N2);
>> +  // fold select C, 0, 1 -> C ^ 1
>> +  if (VT.isInteger() &&
>> +      (VT0 == MVT::i1 ||
>> +       (VT0.isInteger() &&
>> +        TLI.getBooleanContents() ==
>> TargetLowering::ZeroOrOneBooleanContent)) &&
>
> Instead of checking ZeroOrOneBooleanContent, it seems safer to me to
> do this only if MaskedValueIsZero tells you that the high bits are
> zero.  ZeroOrOneBooleanContent really only applies to memory (iirc)
> not to intermediate register values.


It's the other way around; BooleanContent indicates what kind
of values SETCC returns, and what kind of values SELECT can
expect.

Using MaskedValueIsZero shouldn't be necessary here, though it
would make this optimization slightly more aggressive on
UndefinedBooleanContent targets.

Dan