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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1420
+  ICmpInst::Predicate Pred = Cmp->getPredicate();
+  if (Pred != ICmpInst::ICMP_EQ)
+    return false;
----------------
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.


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

https://reviews.llvm.org/D60506