[llvm-commits] [llvm] r133022 - in /llvm/trunk: lib/CodeGen/SelectionDAG/DAGCombiner.cpp test/CodeGen/X86/shl_undef.ll

[Dan Gohman]

On Jun 15, 2011, at 9:35 AM, Eli Friedman wrote:

> On Wed, Jun 15, 2011 at 9:21 AM, Dan Gohman <gohman at apple.com> wrote:
>> On Jun 15, 2011, at 9:03 AM, Bob Wilson wrote:
>> 
>>> It seems to me like "shl undef, 1" should always produce a zero lsb.  SimplifyDemandedBits is certainly assuming that.
>> 
>> I agree with this.
>> 
>>>  The testcase has "shl x, 1" where only the lsb is used, so SimplifyDemandedBits replaces x with undef:
>>> 
>>> ...if (DemandedMask == 0) {
>>>    // Not demanding any bits from Op.
>>>    if (Op.getOpcode() != ISD::UNDEF)
>>>      return TLO.CombineTo(Op, TLO.DAG.getUNDEF(Op.getValueType()));
>>> 
>>> x86 isel matches "shl undef, 1" to "add undef, undef" but then two different registers are used for the two undef operands, and the result is that the lsb is _not_ zero.
>> 
>> However, checking for UNDEF here is not sufficient.  In "shl x, 1",  x could
>> have an undef value without being an immediate UNDEF.  If you convert
>> this to "add x, x" and subsequent optimizations replace x with UNDEF,
>> it would be similarly miscompiled.
>> 
>> It seems to me that transforming "shl x, 1" to "add x, x" is *never* safe,
>> unless you can prove that x is never undef at runtime.  And that's
>> usually not possible.
> 
> How do you suggest rewriting the following pattern so it is safe?
> 
> def : Pat<(shl GR32:$src1, (i8 1)), (ADD32rr GR32:$src1, GR32:$src1)>;


Introducing a new pseudo-instruction for this that has only one input would be
safe, although it would still be vulnerable to peephole passes running after it
gets expanded.

Dan