[PATCH] D49837: [SelectionDAG] Handle unary SelectPatternFlavor for ABS case in SelectionDAGBuilder::visitSelect.

[Ivan Kulagin via Phabricator via llvm-commits]

ikulagin added inline comments.


================
Comment at: lib/Analysis/ValueTracking.cpp:4856
       // (-X >s 0) ? -X : X or (-X >s -1) ? -X : X --> ABS(X)
-      if (Pred == ICmpInst::ICMP_SGT && match(CmpRHS, ZeroOrAllOnes))
+      if ((Pred == ICmpInst::ICMP_SGT || Pred == ICmpInst::ICMP_SGE) && match(CmpRHS, ZeroOrAllOnes))
         return {SPF_ABS, SPNB_NA, false};
----------------
spatel wrote:
> This is saying that:
> X >= -1 ? X : -X 
> ...is an abs(). But that's not correct:
> https://rise4fun.com/Alive/eVzs
> 
> This diff should be split into its own patch and include correctness tests. Preferably, the tests can go in:
> llvm/unittests/Analysis/ValueTrackingTest.cpp
Yes, for that pattern we need something like follow:
```
// (X >= 0) ? X : -X or (X >= 1) ? X : -x --> ABS(X)
if (Pred == ICmpInst::ICMP_SGE && match(CmpRHS, ZeroOrOne))
  return {SPF_ABS, SPNB_NA, false};
```


Repository:
  rL LLVM

https://reviews.llvm.org/D49837