This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Add mul with negated power of 2 constant to canEvaluateShifted.
ClosedPublic

Authored by craig.topper on Jul 19 2022, 10:27 AM.

Details

Summary

If we are right shifting a multiply by a negated power of 2 where
the power of 2 is the same as the shift amount, we can replace with
a negate followed by an And.

New tests have not been committed yet but the patch shows the diffs.
Let me know if you want any changes or additional tests.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 19 2022, 10:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2022, 10:27 AM
craig.topper requested review of this revision.Jul 19 2022, 10:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2022, 10:27 AM
spatel added inline comments.Jul 20 2022, 6:11 AM
llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
691

Can we assert !isLeftShift and/or this is a mul with negated-pow2-constant?

692

Is there a difference between using IC.Builder vs. manually inserting?

llvm/test/Transforms/InstCombine/shift.ll
1844

Do we have a negative test with an extra use of the mul?

craig.topper added inline comments.Jul 20 2022, 9:22 AM
llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
692

Inserting with IC.Builder would insert at the outer LShr. Since we might have gone through a binop that would be the wrong location. foldShiftedShift handles this by calling moveBefore after using IC.Builder. But that still takes the debug location from the outer LShr which seemed wrong. InsertNewInstWith is used by the evaluateInDifferentType code in InstCombineCasts.cpp

-Add assert for !isLeftShift. Checking the constant would require calling match with m_APInt that we don't otherwise need.
-Add negative test with additional use of mul.

spatel accepted this revision.Jul 20 2022, 10:38 AM

LGTM

This revision is now accepted and ready to land.Jul 20 2022, 10:38 AM