This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Add support for constant vector folding of binops in CSEMIRBuilder.
ClosedPublic

Authored by aemerson on Oct 11 2021, 12:14 AM.

Details

Summary

It's odd that we haven't seen the need to do this earlier.

Diff Detail

Event Timeline

aemerson created this revision.Oct 11 2021, 12:14 AM
aemerson requested review of this revision.Oct 11 2021, 12:14 AM
foad added a comment.Oct 11 2021, 1:58 AM

No objections from me, but I do wonder if there's a way to apply this more consistently to all the constant folds, not just the binops.

foad added inline comments.Oct 11 2021, 2:00 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/combine-urem-pow-2.mir
13–17

Urgh, it's hard to see the real diff with all this "-NEXT" churn :(

No objections from me, but I do wonder if there's a way to apply this more consistently to all the constant folds, not just the binops.

I did plan to extend this to unary ops too, should be fairly simple to do.

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-urem-pow-2.mir
13–17

Yeah sorry about that. I’ll update these tests and then update the patch before I commit, so the review will show the real changes.

aemerson updated this revision to Diff 378812.Oct 11 2021, 3:16 PM

Hoist end iterator eval in loop. Rebase on test check regeneration.

foad added a comment.Oct 12 2021, 1:36 AM

Rebase on test check regeneration.

Thanks! AMDGPU test changes look good.

llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-getelementptr.ll
195–196

BUILD_VECTOR2 and BUILD_VECTOR3 are dead now. I guess we don't do anything to clean up dead code at the end of IRTranslator?

aemerson added inline comments.Oct 12 2021, 10:00 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-getelementptr.ll
195–196

Correct, usually the prelegalizer combiner will handle this immediately after.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 12 2021, 11:31 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
arsenm added inline comments.Jan 11 2022, 7:18 AM
llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp
196–197

This is broken because it's ignoring the DstOp. If it was requesting a specific register, you end up with an undefined vreg