Page MenuHomePhabricator

[InstCombine] Fold lshr/ashr(or(neg(x),x),bw-1) --> zext/sext(icmp_ne(x,0)) (PR50816)
ClosedPublic

Authored by RKSimon on Jul 10 2021, 1:54 PM.

Details

Summary

Handle the missing fold reported in PR50816, which is a variant of the existing ashr(sub_nsw(X,Y),bw-1) --> sext(icmp_sgt(X,Y)) fold.

We also handle the lshr(or(neg(x),x),bw-1) --> zext(icmp_ne(x,0)) equivalent - https://alive2.llvm.org/ce/z/SnZmSj

We still allow multi uses of the neg(x) - as this is likely to let us further simplify other uses of the neg - but not multi uses of the or() which would increase instruction count.

Diff Detail

Event Timeline

RKSimon created this revision.Jul 10 2021, 1:54 PM
RKSimon requested review of this revision.Jul 10 2021, 1:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2021, 1:54 PM
spatel added inline comments.Jul 11 2021, 4:36 AM
llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
1331

Might as well handle the lshr -> zext variant at the same time?
https://alive2.llvm.org/ce/z/SnZmSj

llvm/test/Transforms/InstCombine/sub-ashr-or-to-icmp-select.ll
115

This isn't testing the path you intended. We always commute the or operands before we reach the matcher in this patch. Search for "thwart complexity" in the instcombine test directory for examples of how to work around that.

(Best to pre-commit the tests, so we can verify the patterns are as expected.)

lebedev.ri added inline comments.Jul 11 2021, 5:03 AM
llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
1333
RKSimon updated this revision to Diff 358244.Jul 13 2021, 6:01 AM
RKSimon retitled this revision from [InstCombine] Fold ashr(or(neg(x),x),bw-1) --> sext(icmp_ne(x,0)) (PR50816) to [InstCombine] Fold lshr/ashr(or(neg(x),x),bw-1) --> zext/sext(icmp_ne(x,0)) (PR50816).
RKSimon edited the summary of this revision. (Show Details)

Use m_Deferred and add lshr(or(neg(x),x),bw-1) --> zext(icmp_ne(x,0)) handling

lebedev.ri accepted this revision.Jul 13 2021, 6:10 AM

LG

llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
1144–1145

Could use Builder.CreateIsNotNull()

1333–1334

Same

This revision is now accepted and ready to land.Jul 13 2021, 6:10 AM
This revision was landed with ongoing or failed builds.Jul 13 2021, 6:48 AM
This revision was automatically updated to reflect the committed changes.