[PATCH] D149725: [InstCombine] Don't break the min/max idiom by mutating compares.

Amara Emerson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 23:37:46 PDT 2023


aemerson created this revision.
aemerson added a reviewer: dmgreen.
aemerson added a project: LLVM.
Herald added a subscriber: hiraditya.
Herald added a project: All.
aemerson requested review of this revision.

This is motivated by the troubles in D143624 <https://reviews.llvm.org/D143624> where a change in inlining
order resulted in instcombine seeing more code and then breaking the
smin idiom. The problem is that code like this:

  %shr = ashr i32 %mul, 15
  %cmp4.i = icmp sgt i32 %shr, 32767
  %switch.i = icmp ult i1 %cmp4.i, true
  %spec.select.i = select i1 %switch.i, i32 %shr, i32 32767
   

has the %cmp4.i being modified so that the idiom doesn't match. This patch
extends some of the attempts to detect a potential idiom by looking for the
"not" idiom of the %switch.i icmp in between the first cmp and the select.

This however isn't quite enough. After we prevent the first breakage, another
combine mutates the constant operand and condition of %cmp4.i, and unfortunately
that again prevents the idiom-preserving bail out code from working. To
get around this we re-use some of the canonicalization code that undoes that
transform to again prevent breakage.

I don't feel good about any of this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149725

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
  llvm/test/Transforms/InstCombine/prevent-minmax-pattern-break.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149725.518982.patch
Type: text/x-patch
Size: 8850 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230503/a5e8e7ee/attachment.bin>


More information about the llvm-commits mailing list