This is an archive of the discontinued LLVM Phabricator instance.

[DAGCombiner] Fold (mul (sra X, BW-1), Y) -> (neg (and (sra X, BW-1), Y))
Needs ReviewPublic

Authored by craig.topper on Sep 6 2022, 11:07 PM.

Details

Summary

(sra X, BW-1) is either 0 or -1. So the multiply is a conditional
negate of Y.

This pattern shows up when type legalizing wide multiplies involving
a sign extended value.

Fixes PR57549.

Diff Detail

Event Timeline

craig.topper created this revision.Sep 6 2022, 11:07 PM
craig.topper requested review of this revision.Sep 6 2022, 11:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 6 2022, 11:07 PM
RKSimon added inline comments.Sep 7 2022, 12:20 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3963

Would this work more generally if we tested for NumSignBits == BitWidth instead?

craig.topper added inline comments.Sep 7 2022, 8:35 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3963

It’s starts picking up vectors of constants if we do that and it wasn’t clear that the test changes were improvements.

I can add a TODO?

spatel added inline comments.Sep 7 2022, 9:06 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3960

Bail if optimizing for minsize?

RKSimon added inline comments.Sep 7 2022, 9:30 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
3963

Yes a TODO would be fine

@craig.topper reverse-ping - are you still looking at this patch?

Add TODO and minsize check.
Still need to add minsize test.

Add minsize tests to X86 extmul128.ll

This revision is now accepted and ready to land.Oct 10 2022, 11:58 AM
This revision was landed with ongoing or failed builds.Oct 11 2022, 4:21 PM
This revision was automatically updated to reflect the committed changes.
craig.topper reopened this revision.Oct 11 2022, 4:31 PM
This revision is now accepted and ready to land.Oct 11 2022, 4:31 PM
craig.topper planned changes to this revision.Oct 11 2022, 4:31 PM
This revision was not accepted when it landed; it landed in state Changes Planned.Oct 22 2022, 9:58 PM
This revision was automatically updated to reflect the committed changes.
craig.topper reopened this revision.Oct 22 2022, 10:52 PM
This revision is now accepted and ready to land.Oct 22 2022, 10:52 PM
craig.topper requested review of this revision.Oct 22 2022, 10:52 PM
craig.topper added subscribers: foad, arsenm.
craig.topper added inline comments.
llvm/test/CodeGen/AMDGPU/mad_64_32.ll
192

@arsenm @foad on some build bots this v_and_b32_e32 and the one after it seem to be in a different order. Any idea what could be going on?

Failing example https://lab.llvm.org/buildbot/#/builders/58/builds/27599/steps/6/logs/FAIL__LLVM__mad_64_32_ll

arsenm added inline comments.Oct 22 2022, 11:09 PM
llvm/test/CodeGen/AMDGPU/mad_64_32.ll
192

No idea. I'd guess nondeterminism somewhere else