[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 3 inline comments 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:
> 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.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1425
+  const APInt *C;
+  if (!match(Y, m_APInt(C)))
+    return false;
----------------
nemanjai wrote:
> Why does this need to be restricted to constant comparisons? Could we not equivalently handle comparisons between two virtual registers?
Well. I think we might be able to handle comparisons between two virtual registers equivalently. However I cannot generate test cases for it. I have tried:
```
long long test1(long long a, long long b, long long c) {
  if (a << b > c)
    return b;
  if (a << b < c)
    return a;
  return a * b;
}
```
Here is the generated LLVM IR:
```
; Function Attrs: norecurse nounwind readnone
define dso_local i64 @_Z5test1xxx(i64 %a, i64 %b, i64 %c) local_unnamed_addr #0 {
entry:
  %shl = shl i64 %a, %b
  %cmp = icmp sgt i64 %shl, %c
  br i1 %cmp, label %return, label %if.end

if.end:                                           ; preds = %entry
  %cmp2 = icmp slt i64 %shl, %c
  %mul = select i1 %cmp2, i64 1, i64 %b
  %spec.select = mul nsw i64 %mul, %a
  ret i64 %spec.select

return:                                           ; preds = %entry
  ret i64 %b
}
```
Apparently, it is not the test case we want. Do you have any test cases in mind?


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1433
+      continue;
+    return false;
+  }
----------------
nemanjai wrote:
> We could probably handle logical operations just as easily, couldn't we?
For other logical operations, we need to introduce a `not` instruction. It would add more LLVM IR and that might produce more instructions, which is we would like to avoid.


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

https://reviews.llvm.org/D60506