Page MenuHomePhabricator

[X86] Improve optimizeCompareInstr for signed comparisons after BMI/TBM instructions
ClosedPublic

Authored by craig.topper on Jan 15 2021, 7:47 PM.

Details

Summary

We previously couldn't optimize out a TEST if the branch/setcc/cmov
used the overflow flag. This patches allows the TEST to be removed
if the flag producing instruction is known to clear the OF flag.
Thats what the TEST instruction would have done so that should be
equivalent.

Need to add test cases. I'll try to get back to this if I have bandwidth.

Fixes PR48768.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 15 2021, 7:47 PM
craig.topper requested review of this revision.Jan 15 2021, 7:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 15 2021, 7:47 PM

@craig.topper Are you still hoping to deal with this one or should one of us commandeer it?

@craig.topper Are you still hoping to deal with this one or should one of us commandeer it?

I'll pick this back up.

llvm/lib/Target/X86/X86InstrInfo.cpp
4058

Surprisingly LZCNT and TZCNT don't define the value of the O flag so this is wrong. POPCNT does clear it.

Restrict to just the BLS* and TBM instructions with tests. We already had tests for these instructions that were easy to replicate with new condition code

Do we have specific test coverage for the PR48768 case?

craig.topper retitled this revision from [X86] WIP Improve optimizeCompareInstr for signed comparisons after logical operations. to [X86]Improve optimizeCompareInstr for signed comparisons after BMI/TBM instructions.Wed, Mar 31, 9:20 AM
craig.topper retitled this revision from [X86]Improve optimizeCompareInstr for signed comparisons after BMI/TBM instructions to [X86] Improve optimizeCompareInstr for signed comparisons after BMI/TBM instructions.
craig.topper added inline comments.
llvm/test/CodeGen/X86/bmi.ll
542 ↗(On Diff #334357)

This should be equivalent to the PR48768, but with cmovcc instead of setcc.

RKSimon accepted this revision.Wed, Mar 31, 9:24 AM

LGTM cheers

llvm/test/CodeGen/X86/bmi.ll
542 ↗(On Diff #334357)

SGTM - please can you just add a comment mentioning PR48768 ?

This revision is now accepted and ready to land.Wed, Mar 31, 9:24 AM
This revision was landed with ongoing or failed builds.Wed, Mar 31, 9:46 AM
This revision was automatically updated to reflect the committed changes.