This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Add tests for combining (urem/srem (mul/shl X, Y), (mul/shl X, Z)); NFC
ClosedPublic

Authored by goldstein.w.n on Jan 31 2023, 1:57 PM.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Jan 31 2023, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2023, 1:57 PM
goldstein.w.n requested review of this revision.Jan 31 2023, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2023, 1:57 PM
MattDevereau added inline comments.Feb 7 2023, 3:30 AM
llvm/test/Transforms/InstCombine/urem-mul.ll
32 ↗(On Diff #495187)

This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like urem_XY_XZ_constY_is_gte_constZ?

58 ↗(On Diff #495187)

This could be called something like urem_XY_XZ_constY_is_lt_constZ to reflect why we return BO0

149 ↗(On Diff #495187)

%X is unused

goldstein.w.n marked 3 inline comments as done.Feb 7 2023, 11:40 PM
goldstein.w.n added inline comments.
llvm/test/Transforms/InstCombine/urem-mul.ll
32 ↗(On Diff #495187)

This test could be better named, since C is actually %X. The important thing here which lets us simplify this to 0 is that the constant LHS multiplicand >= the constant RHS multiplicand of the rem. Maybe something like urem_XY_XZ_constY_is_gte_constZ?

32 ↗(On Diff #495187)

Okay, renamed all functions to reflect the conditions rather than the result.

goldstein.w.n marked an inline comment as done.

Rename test functions

MattDevereau accepted this revision.Feb 8 2023, 2:24 AM
This revision is now accepted and ready to land.Feb 8 2023, 2:24 AM

Rename test file

Add vscale tests

Add tests for missing cases

This revision was landed with ongoing or failed builds.Mar 16 2023, 11:02 AM
This revision was automatically updated to reflect the committed changes.