Page MenuHomePhabricator

[DAGCombiner][X86] Fold sra (sub AddC, (shl X, N1C)), N1C --> sext (sub AddC1',(trunc X to (width - N1C)))
ClosedPublic

Authored by craig.topper on Jun 28 2022, 4:22 PM.

Details

Summary

We already handled this case for add with a constant RHS. A
similar pattern can occur for sub with a constant left hand side.

Test cases use add and a mul representing (neg (shl X, C)) because
that's what I saw in the wild. The mul will be decomposed and then
the new transform can kick in.

Tests have not been committed, but this patch shows the changes.

Diff Detail

Unit TestsFailed

TimeTest
60,160 msx64 debian > BOLT.runtime/X86::exceptions-pic.test
Script: -- : 'RUN: at line 5'; /usr/bin/clang --driver-mode=g++ -fpic -Wl,-q /var/lib/buildkite-agent/builds/llvm-project/bolt/test/runtime/X86/Inputs/exception4.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/tools/bolt/test/runtime/X86/Output/exceptions-pic.test.tmp.pic

Event Timeline

craig.topper created this revision.Jun 28 2022, 4:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 4:22 PM
craig.topper requested review of this revision.Jun 28 2022, 4:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 4:22 PM
craig.topper added a comment.EditedJun 28 2022, 4:45 PM

There's still a more general form of this that can show up in address arithmetic that we don't catch https://godbolt.org/z/WKfYvbvxe

craig.topper retitled this revision from [DAGCombiner] Fold sra (sub AddC, (shl X, N1C)), N1C --> sext (sub AddC1',(trunc X to (width - N1C))) to [DAGCombiner][X86] Fold sra (sub AddC, (shl X, N1C)), N1C --> sext (sub AddC1',(trunc X to (width - N1C))).Jun 30 2022, 12:29 PM
RKSimon added inline comments.Jun 30 2022, 12:43 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9294

Move N1C earlier? Move the expensive hasOneUse as late as possible

9300

Does this have to be a splat? I think technically only N1C has to be and AddC can be non-uniform except it requires the ShiftC calculation to be refactored for general constant folding. But I'm not certain how often isTruncateFree is going to return true for vector types....

Move N1C check.

craig.topper added inline comments.Jun 30 2022, 5:32 PM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9300

I think you're right, but I'd rather not do that in this patch.

RKSimon accepted this revision.Jul 9 2022, 4:57 AM

LGTM

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
9300

OK - please can you add a TODO comment?

This revision is now accepted and ready to land.Jul 9 2022, 4:57 AM
This revision was landed with ongoing or failed builds.Jul 9 2022, 11:54 AM
This revision was automatically updated to reflect the committed changes.