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

[Chris Lattner]

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.

-Chris