Page MenuHomePhabricator

[TargetLowering] Avoid infinite iteration on setcc fold
Needs ReviewPublic

Authored by greened on Feb 28 2020, 1:55 PM.

Details

Summary

When folding (X & Y) == Y to (~X & Y) == 0, we should only do the fold if the computed zero value is not Y. Otherwise, the resulting compare will match the same pattern ad infinitum.

Diff Detail

Event Timeline

greened created this revision.Feb 28 2020, 1:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2020, 1:55 PM
greened added a comment.EditedFeb 28 2020, 2:00 PM

This appeared using a quite old version of LLVM with an out-of-tree target and I'm having trouble getting a valid testcase out that runs with the latest LLVM/bugpoint. The code in question is exacly the same in the old and current LLVM so the bug still exists currently but I haven't yet been able to generate a testcase. Thought I would at least put this up for review. I'm not entirely confident current master could get into this situation but it seems wise to guard against it.

RKSimon added inline comments.Mar 1 2020, 1:21 PM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
2967

Is this happening with vector cases, in which case would this work instead:

ConstantSDNode *YConst = isConstOrConstSplat(Y) ?

greened marked an inline comment as done.Mar 2 2020, 6:45 AM
greened added inline comments.
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
2967

That would work for the case I saw. But it seems prudent to guard against future constructs which might appear. Anything that results in Y == Zero is going to cause an infinite loop.

RKSimon added inline comments.Mar 2 2020, 8:03 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
2967

Under what circumstances will the isConstOrConstSplat + isNullValue() fail but the comparison against Zero work?

RKSimon added inline comments.Apr 15 2020, 10:26 AM
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
2967

@greened - I'm not sure if you're still looking at this, but by inspection this should work. Any luck on creating a test case?

ConstantSDNode *YConst =
    isConstOrConstSplat(Y, /*AllowUndefs*/ true, /*AllowTruncation*/ true);
if (YConst && YConst->isNullValue())
  return SDValue();
assert(Y != Zero && "Unknown zero value");