[PATCH] D60506: [CGP] Make ICMP_EQ use CR result of ICMP_S(L|G)T dominators

[Yi-Hong Lyu via Phabricator via llvm-commits]

Yi-Hong.Lyu marked an inline comment as done.
Yi-Hong.Lyu added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1420
+  ICmpInst::Predicate Pred = Cmp->getPredicate();
+  if (Pred != ICmpInst::ICMP_EQ)
+    return false;
----------------
nemanjai wrote:
> Yi-Hong.Lyu wrote:
> > nemanjai wrote:
> > > Why do we restrict this to `EQ` rather than also supporting `NE`? Is this just the canonical form and `NE` never shows up?
> > The pattern we observed that could be optimized is something like:
> > ```
> > Block A:
> > icmp slt i32 %1, 100
> > ...
> > Block B (with A being the single predecessor):
> > icmp eq i32 %1, 100 ; Equality is exactly opposite to SGT.
> > ...
> > ```
> > `icmp ne i32 %1, 100` is not supposed to be there, otherwise it would be a redundant LLVM IR.
> I don't follow why this would be redundant IR.
> ```
> BlockA:
> %cmp1 = icmp slt i32 %1, 100
> br i1 %cmp1, BlockC, BlockB
> 
> BlockB: ; %1 is signed-greater-than-or-equal-to 100
> %cmp2 = icmp ne i32 %1, 100
> br i1 %cmp2, BlockD, BlockE
> ...
> BlockC: ; %1 is signed-less-than 100
> ...
> BlockD: ; %1 is signed-greater-than 100
> ...
> BlockE: ; %1 is equal to 100
> ```
> 
> Of course, if the EQ case is the canonical form, that's a different statement, but I don't think NE is automatically redundant.
After some experiments, I think `EQ` is canonical form and `NE` never shows up. I change your pseudo LLVM IR a little bit to make it work:
```
; ModuleID = '<stdin>'
source_filename = "<stdin>"
target datalayout = "e-m:e-i64:64-n32:64"
target triple = "powerpc64le-unknown-linux-gnu"

define i64 @icmp_ne(i64 %a, i64 %b) {
BlockA:
  %shl = shl i64 %a, %b
  %cmp1 = icmp slt i64 %shl, 1
  br i1 %cmp1, label %BlockC, label %BlockB

BlockB: ; %1 is signed-greater-than-or-equal-to 1
  %cmp2 = icmp ne i64 %shl, 1
  br i1 %cmp2, label %BlockD, label %BlockE

BlockC: ; %1 is signed-less-than 1
  ret i64 %a

BlockD: ; %1 is signed-greater-than 1
  ret i64 %b

BlockE: ; %1 is equal to 1
  %mul = mul nsw i64 %a, %b
  ret i64 %mul
}
```
However, the `NE` would be  transformed into `EQ` as:
```
$ opt -instcombine -S < icmp_ne.ll -o -
; ModuleID = '<stdin>'
source_filename = "<stdin>"
target datalayout = "e-m:e-i64:64-n32:64"
target triple = "powerpc64le-unknown-linux-gnu"

define i64 @icmp_ne(i64 %a, i64 %b) {
BlockA:
  %shl = shl i64 %a, %b
  %cmp1 = icmp slt i64 %shl, 1
  br i1 %cmp1, label %BlockC, label %BlockB

BlockB:                                           ; preds = %BlockA
  %cmp2 = icmp eq i64 %shl, 1
  br i1 %cmp2, label %BlockE, label %BlockD

BlockC:                                           ; preds = %BlockA
  ret i64 %a

BlockD:                                           ; preds = %BlockB
  ret i64 %b

BlockE:                                           ; preds = %BlockB
  %mul = mul nsw i64 %a, %b
  ret i64 %mul
}
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60506/new/

https://reviews.llvm.org/D60506