This is an archive of the discontinued LLVM Phabricator instance.

[InstSimplify] SimplifyICmpInst - icmp eq/ne %X, undef -> undef
ClosedPublic

Authored by RKSimon on Mar 19 2019, 5:03 AM.

Details

Summary

As discussed on PR41125 and D59363, we have a mismatch between icmp eq/ne cases with an undef operand:

When the other operand is constant we fold to undef (handled in ConstantFoldCompareInstruction)
When the other operand is non-constant we fold to a bool constant based on isTrueWhenEqual (handled in SimplifyICmpInst).

Neither it really wrong, but this patch changes the logic in SimplifyICmpInst to consistently fold to undef.

The NewGVN test change is annoying (as with most heavily reduced tests) but AFAICT I have kept the purpose of the test based on rL291968.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Mar 19 2019, 5:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 19 2019, 5:03 AM
spatel accepted this revision.Mar 19 2019, 6:29 AM

LGTM

lib/Analysis/InstructionSimplify.cpp
3053

If both operands are undef, we should not get here - that should always get constant folded above this.
So I'd remove that line of the code comment. You could add an assert to be extra safe...although that general sequence (handle constant folding first) happens in all or almost all functions within InstSimplify.

This revision is now accepted and ready to land.Mar 19 2019, 6:29 AM
This revision was automatically updated to reflect the committed changes.