This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use branchless form for selects with 0 in either arm
ClosedPublic

Authored by reames on Oct 10 2022, 9:18 AM.

Details

Summary

Continuing the theme of adding branchless lowerings for simple selects, this time handle the 0 arm case. This is very common for various umin idioms, etc..

Diff Detail

Event Timeline

reames created this revision.Oct 10 2022, 9:18 AM
reames requested review of this revision.Oct 10 2022, 9:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2022, 9:18 AM
craig.topper added inline comments.Oct 10 2022, 9:55 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9505

Too much copy/paste. :) This should be isNullConstant

reames added inline comments.Oct 10 2022, 11:17 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9505

Eek, how did I miss that? I swear I looked at the test diffs too!

reames updated this revision to Diff 466572.Oct 10 2022, 11:43 AM

Address bug caught by @craig.topper. Due to copy paste mistake, second case was dead code. With fix, impact is even broader, including making many floating point conversion idioms branchless.

Is the branchless form better though? Branch+move can be fused but these forms can't and have more instructions to execute.

Is the branchless form better though? Branch+move can be fused but these forms can't and have more instructions to execute.

We need an mtune flag for CPUs that can fuse these like sifive-7-series. But we also need to implement a fusion guarantee too so that code motion doesn't sinks things into the basic block and break fusion. I assume rocket doesn't fuse these?

craig.topper added inline comments.Oct 12 2022, 11:06 AM
llvm/test/CodeGen/RISCV/double-convert.ll
97

This is interesting. The seqz isn't necessary. I'll take a look at this.

craig.topper accepted this revision.Oct 12 2022, 12:15 PM

LGTM. I think this is a good starting point. I have a followup I'm going to try to fix the issue I noticed, but that shouldn't block this.

This revision is now accepted and ready to land.Oct 12 2022, 12:15 PM
This revision was landed with ongoing or failed builds.Oct 12 2022, 1:52 PM
This revision was automatically updated to reflect the committed changes.