This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Add a constant folding combine.
ClosedPublic

Authored by aemerson on Jul 25 2021, 3:10 PM.

Details

Summary

These don't always get folded because when the instructions are created the constants are obscured by artifacts, or other reasons.

Use it AArch64 post-legal combiner too.

Diff Detail

Event Timeline

aemerson created this revision.Jul 25 2021, 3:10 PM
aemerson requested review of this revision.Jul 25 2021, 3:10 PM
paquette added inline comments.Jul 26 2021, 9:23 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4267

This looks like it could use replaceInstWithConstant; does it need APInt? (Or should that helper be changed to use APInt?)

aemerson updated this revision to Diff 361731.Jul 26 2021, 11:12 AM

Add a variant of replaceInstWithConstant taking APInt. It still seems convenient to have an int64_t version since that one builds the constant with the type of the DstOp, while we'd have to construct a correctly sized APInt in each call site if we replaced it.

aemerson updated this revision to Diff 361732.Jul 26 2021, 11:13 AM

Forgot to remove old apply method.

This revision is now accepted and ready to land.Jul 26 2021, 11:56 AM
This revision was landed with ongoing or failed builds.Jul 26 2021, 2:54 PM
This revision was automatically updated to reflect the committed changes.
foad added a comment.Jul 28 2021, 6:47 AM

These don't always get folded because when the instructions are created the constants are obscured by artifacts, or other reasons.

For the AMDGPU fshl/fshr tests, the reason is that the foldable instructions are created during legalization, and the AMDGPU legalizer doesn't use CSEMIRBuilder yet.

foad added a comment.Jul 29 2021, 5:19 AM

These don't always get folded because when the instructions are created the constants are obscured by artifacts, or other reasons.

For the AMDGPU fshl/fshr tests, the reason is that the foldable instructions are created during legalization, and the AMDGPU legalizer doesn't use CSEMIRBuilder yet.

Scratch that. The reason is that the constants are obscured by artifacts like G_ZEXT, which we don't constant fold (I previously tried that in D89392).