Page MenuHomePhabricator

[InstCombine] Add two optimizations for mul-and-icmp patterns
Needs ReviewPublic

Authored by fwolff on Fri, Nov 19, 12:40 PM.

Details

Summary

Fixes PR#40493, and in principle also PR#51551, but the known bits analysis isn't quite smart enough to handle the assume in the example there.

The entire logic for this patch has been discussed in PR#40493, it just seems that nobody has gotten around to implementing it yet.

https://alive2.llvm.org/ce/z/Bp6ZoV
https://alive2.llvm.org/ce/z/htSh_Q

Diff Detail

Event Timeline

fwolff created this revision.Fri, Nov 19, 12:40 PM
fwolff requested review of this revision.Fri, Nov 19, 12:40 PM
lebedev.ri added a subscriber: lebedev.ri.

alive2 proofs?

llvm/test/Transforms/InstCombine/pr40493.ll
2

Precommit tests please

alive2 proofs?

@test1: https://alive2.llvm.org/ce/z/BN74k2
@test2: https://alive2.llvm.org/ce/z/3PRnmg
@test2_2: https://alive2.llvm.org/ce/z/FmrCSL

The other tests don't do this transformation.

llvm/test/Transforms/InstCombine/pr40493.ll
2

Do you mean that I should submit this file separately first in a different revision?

alive2 proofs?

@test1: https://alive2.llvm.org/ce/z/BN74k2
@test2: https://alive2.llvm.org/ce/z/3PRnmg
@test2_2: https://alive2.llvm.org/ce/z/FmrCSL

The other tests don't do this transformation.

This only shows that those tests aren't miscompiled, i'm interested in the generalized proof for the actual preconditions.

RKSimon added inline comments.Fri, Nov 19, 2:23 PM
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
1920

(style) Don't use auto - APInt should be enough

1934–1935

Don't use auto

This only shows that those tests aren't miscompiled

I know; I thought that's what you wanted because the transformations themselves have already been checked in PR#40493, although the links now appear to be defunct.

Anyway, here are the proofs:
https://alive2.llvm.org/ce/z/Bp6ZoV
https://alive2.llvm.org/ce/z/htSh_Q

fwolff updated this revision to Diff 388632.Fri, Nov 19, 2:38 PM
fwolff marked 2 inline comments as done.

vector tests?

lebedev.ri edited the summary of this revision. (Show Details)Fri, Nov 26, 2:38 AM

Thank you for looking into this.

vector tests?

Please add vector tests (with splat constants (all constant elements being the same),
splat with undef (there are 2 constants, so 3 variants - same undef lane in either of the constants, and in all of them)),
non-splat, commit these baseline tests into the llvm repo, and rebase this patch - the diff here should not show the tests as being added, it should show the check lines as being changed.