This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][WebAssembly][TargetLowering] Allow expandCTLZ/expandCTTZ to rely on CTPOP expansion for vectors.
ClosedPublic

Authored by craig.topper on Oct 15 2021, 6:40 PM.

Details

Summary

Our fallback expansion for CTLZ/CTTZ relies on CTPOP. If CTPOP
isn't legal or custom for a vector type we would scalarize the
CTLZ/CTTZ. This is different than CTPOP itself which would use a
vector expansion.

This patch teaches expandCTLZ/CTTZ to rely on the vector CTPOP
expansion instead of scalarizing. To do this I had to add additional
checks to make sure the operations used by CTPOP expansions are all
supported. Some of the operations were already needed for the CTLZ/CTTZ
expansion.

This is a huge improvement to the RISCV which doesn't have a scalar
ctlz or cttz in the base ISA.

For WebAssembly, I've added Custom lowering to keep the scalarizing behavior.
I've also extended the scalarizing to CTPOP.

Diff Detail

Event Timeline

craig.topper created this revision.Oct 15 2021, 6:40 PM
craig.topper requested review of this revision.Oct 15 2021, 6:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 15 2021, 6:40 PM

Hmm yeah the new lowerings for WebAssembly don't look great. Would there be a way to disable this change for Wasm?

Hmm yeah the new lowerings for WebAssembly don't look great. Would there be a way to disable this change for Wasm?

Would you want ctpop to go to scalarization as well?

Hmm yeah the new lowerings for WebAssembly don't look great. Would there be a way to disable this change for Wasm?

Would you want ctpop to go to scalarization as well?

Yeah actually that would probably be fine if it makes things simpler.

Hmm yeah the new lowerings for WebAssembly don't look great. Would there be a way to disable this change for Wasm?

Would you want ctpop to go to scalarization as well?

Yeah actually that would probably be fine if it makes things simpler.

I'm not sure if it is easier or not. I just thought it might be better for overall consistency.

Yes, I agree.

Add Custom lowering to WebAssembly to keep the unrolled code. Also unroll CTPOP.

craig.topper edited the summary of this revision. (Show Details)Oct 19 2021, 4:11 PM

Thanks, the WebAssembly portion LGTM. The TargetLowering.cpp change LGTM as well, modulo that one comment.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
7078

Would it make sense to extract this part into a separate variable called CanLowerCTPOP or similar? Then if the ctpop lowering ever changes it will be easier to know what to update here.

Move CTPOP checks into a helper function to share.

frasercrmck accepted this revision.Oct 20 2021, 1:56 AM

Hard to argue with the RISC-V changes. I think it's looking good with the helper method.

I think there's some clang-format warnings which can be fixed up but otherwise LGTM.

This revision is now accepted and ready to land.Oct 20 2021, 1:56 AM
This revision was landed with ongoing or failed builds.Oct 20 2021, 7:47 AM
This revision was automatically updated to reflect the committed changes.
llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp