This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add test cases to prepare for overring TargetLowering::hasAndNot. NFC
ClosedPublic

Authored by craig.topper on Nov 15 2021, 12:50 PM.

Details

Summary

These test files are copied directly from AArch64. Some of the cases
may benefit from ANDN with the Zbb extension. Somes cases already
improve use ANDN.

selectcc-to-shiftand.ll also contains tests that test select->and
conversion even when a ANDN isn't needed. I think this improves our
coverage of these optimizations.

Diff Detail

Event Timeline

craig.topper created this revision.Nov 15 2021, 12:50 PM
craig.topper requested review of this revision.Nov 15 2021, 12:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2021, 12:50 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
jrtc27 added inline comments.Nov 15 2021, 1:09 PM
llvm/test/CodeGen/RISCV/selectcc-to-shiftand.ll
4

I think we normally use a 2 space indent for this, not four?

167

Maybe not useful for a RISC-V test? Though the fix for this is what introduced the TLI hook and was otherwise target-independent.

170

Maybe wants a more informative name as a result? sub_clamp_zero or something similarly descriptive

llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll
12

This one was filed against X86 but looks to have really been target-independent? Not sure if the bug link is worthwhile or not.

169

I don't understand what this is referring to

233

From this point on there aren't blank lines around functions (and headers like this), it's all crammed together

craig.topper added inline comments.Nov 15 2021, 1:28 PM
llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll
169

I think it was trying to say that we should transform each of the in* functions the equivalent of the corresponding out* test.

lebedev.ri added inline comments.Nov 15 2021, 1:31 PM
llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll
169

Yep. The out is the unmerged form of masked-merge, the in is merged form.

Address review comments

jrtc27 added inline comments.Nov 15 2021, 5:30 PM
llvm/test/CodeGen/RISCV/selectcc-to-shiftand.ll
30

Wrap all these and put them right up against the test?

77

Seems we fail to do this for everything except RV32 pos_sel_special_constant, though the RV64 version of that is probably just as performant.

llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll
169

Right, I see now

233

I think you still want a blank line after each of these block headers (including the TODO), since they apply to multiple tests and not just the one they're (currently) touching

952

Add some TODOs. Add more blank lines.

jrtc27 accepted this revision.Nov 15 2021, 5:46 PM

Nothing left that stands out to me for nitpicking

This revision is now accepted and ready to land.Nov 15 2021, 5:46 PM