This is an archive of the discontinued LLVM Phabricator instance.

Fix ARM peephole optimizeCompare to avoid optimizing unsigned cmp to 0.
ClosedPublic

Authored by jvoung on Jan 29 2015, 3:17 PM.

Details

Summary

Previously it only avoided optimizing signed comparisons to 0.
Sometimes the DAGCombiner will optimize the unsigned comparisons
to 0 before it gets to the peephole pass, but sometimes it doesn't.

Fix for PR22373.

Diff Detail

Repository
rL LLVM

Event Timeline

jvoung updated this revision to Diff 18998.Jan 29 2015, 3:17 PM
jvoung retitled this revision from to Fix ARM peephole optimizeCompare to avoid optimizing unsigned cmp to 0..
jvoung updated this object.
jvoung edited the test plan for this revision. (Show Details)
jvoung added reviewers: jfb, manmanren.
jvoung added a subscriber: Unknown Object (MLST).
jfb edited edge metadata.Jan 31 2015, 1:21 PM

lgtm otherwise.

lib/Target/ARM/ARMBaseInstrInfo.cpp
2570 ↗(On Diff #18998)

I think it would be better to rewire the code as:

switch (CC) {
case ARMCC::EQ: // Z
case ARMCC::NE: // Z
case ARMCC::MI: // N
case ARMCC::PL: // N
case ARMCC::AL: // none
  // CPSR can be used multiple times, we should continue.
  break;
case ARMCC::CS: // C
case ARMCC::CC: // C
case ARMCC::VS: // V
case ARMCC::VC: // V
case ARMCC::HI: // C Z
case ARMCC::LS: // C Z
case ARMCC::GE: // N V
case ARMCC::LT: // N V
case ARMCC::GT: // Z N V
case ARMCC::LE: // Z N V
  // The instruction uses the V bit or C bit which is not safe.
  return false;
}
jvoung updated this revision to Diff 19106.Jan 31 2015, 7:22 PM
jvoung edited edge metadata.

Apply JF's review suggestion -- annotate the switch cases w/ the bits used.

jfb accepted this revision.Jan 31 2015, 9:23 PM
jfb edited edge metadata.
This revision is now accepted and ready to land.Jan 31 2015, 9:23 PM
This revision was automatically updated to reflect the committed changes.