This is an archive of the discontinued LLVM Phabricator instance.

[SDAG] Legalize all CondCodes by inverting them and/or swapping operands
ClosedPublic

Authored by kparzysz on Feb 1 2018, 4:41 AM.

Details

Summary

When expanding a CondCode, try all combinations of inverting the condition code and swapping the operands.

Diff Detail

Repository
rL LLVM

Event Timeline

kparzysz created this revision.Feb 1 2018, 4:41 AM

I believe this either needs [NFC] in subject, or tests.

There is no test because nobody uses that yet. The additional situations that this handles used to abort compilation. I have a patch for Hexagon that uses this, but I want to keep it separate from the target-independent part.

efriedma added inline comments.Feb 5 2018, 11:59 AM
lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
1637 ↗(On Diff #132361)

Could you put the code to invert the condition code here, instead of inside the switch, so it doesn't get mixed up with the floating-point-specific stuff?

kparzysz updated this revision to Diff 132883.Feb 5 2018, 12:51 PM

Place the added piece of swapping/inverting code together with the pre-existing part.

kparzysz marked an inline comment as done.Feb 5 2018, 12:51 PM
efriedma accepted this revision.Feb 5 2018, 1:26 PM

LGTM

This revision is now accepted and ready to land.Feb 5 2018, 1:26 PM
This revision was automatically updated to reflect the committed changes.
dschuff added a subscriber: dschuff.Feb 5 2018, 5:46 PM

So, it turns out that this actually does change functionality on targets (such as WebAssembly) that use the expansion for unordered comparisons because it tries inverting the condition before falling all the way through to line 1680. This actually results in better code for wasm; see https://reviews.llvm.org/rL324305

In the first version of the patch, the added code was executing last. In the committed version it takes precedence over the floating point code, so you're right, it wasn't really an NFC. I'm glad the result is a better code.