This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Combine G_UMULH x, (1 << c)) -> x >> (bitwidth - c)
ClosedPublic

Authored by aemerson on Oct 3 2021, 11:08 PM.

Details

Summary

In order to not generate an unnecessary G_CTLZ, I extended the constant folder
in the CSEMIRBuilder to handle G_CTLZ. I also added some extra handing of
vector constants too. It seems we don't have any support for doing constant
folding of vector constants, so the tests show some other useless G_SUB
instructions too.

Diff Detail

Event Timeline

aemerson created this revision.Oct 3 2021, 11:08 PM
aemerson requested review of this revision.Oct 3 2021, 11:08 PM
aemerson updated this revision to Diff 376806.Oct 3 2021, 11:17 PM

Remove unused header includes.

foad added a comment.Oct 4 2021, 1:16 AM

G_UMULH x, (1 << c)) -> x >> (bitwidth - c)

This doesn't work when c is 0 because you get an out of range shift. In general you need something like x >> 1 >> (bitwidth - 1 - c).

aemerson updated this revision to Diff 376977.Oct 4 2021, 12:05 PM

Don't fold if constant RHS is a one value.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 7 2021, 11:51 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Oops sorry about that, I didn't realize this was already on my branch when I committed 72ce310bf0de so this went in by mistake.

Since it's already gone through a round of review I'll leave it in and update with post-review comments or revert if requested.

foad added inline comments.Oct 8 2021, 12:08 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4598

This will no longer handle the "mix of non-NPQ/NPQ paths" that D110890 can generate.

aemerson added inline comments.Oct 8 2021, 11:30 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
4598

Thanks. I've fixed this by using matchUnaryPredicate() instead in 17b89f9daad56f80c29b46bed0b6ce94093dcfc9