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

[Eli Friedman]

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)>;

-Eli