Page MenuHomePhabricator

[InstCombine] Move negation handling into freelyNegateValue()
ClosedPublic

Authored by nikic on Jan 23 2020, 12:38 PM.

Details

Summary

Followup to D72978. This moves existing negation handling in InstCombine into freelyNegateValue(), which make it more composable. In particular, root negations of div/zext/sext/ashr/lshr/sub can now always be performed through a shl/trunc as well.

The next step is to add multiply support to fix https://bugs.llvm.org/show_bug.cgi?id=44234.

Diff Detail

Event Timeline

nikic created this revision.Jan 23 2020, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2020, 12:38 PM
nikic updated this revision to Diff 240284.Jan 24 2020, 1:48 PM
nikic edited the summary of this revision. (Show Details)

Also move the ashr/lshr and trunc patterns into freelyNegateValue() and keep existing handling regarding one-use checks.

spatel added inline comments.Jan 27 2020, 9:01 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
867

Seems like this would be more efficient as a "switch(V->getOpcode())"?

Then, we could combine cases like ashr/lshr by using Builder.CreateBinOp(V->getOpcode()...). Similarly combine zext/sext with Builder.CreateCast().

nikic updated this revision to Diff 240629.Mon, Jan 27, 10:13 AM

Use switch instead of match chain.

nikic marked an inline comment as done.Mon, Jan 27, 10:15 AM
nikic added inline comments.
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
867

I've switched this to use switch, but ended up not combining the cases. The problem is that we need to swap the signedness of the opcodes (zext <-> sext, lshr <-> ashr), which is just awkward enough that keeping the cases separate looks more readable. (Especially as there is no direct API for creating an "lshr or ashr" with exact flag.)

spatel accepted this revision.Mon, Jan 27, 10:30 AM
This revision is now accepted and ready to land.Mon, Jan 27, 10:30 AM
This revision was automatically updated to reflect the committed changes.