[PATCH] D67799: [InstCombine] Fold a shifty implementation of clamp0.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Please change `clamp0` everywhere to `clamp negative to zero`, it wasn't obvious to what `clamp0` means until reading all of the patch.
This looks ok otherwise. Please wait for @spatel to comment.

In D67799#1677606 <https://reviews.llvm.org/D67799#1677606>, @huihuiz wrote:

> For X86, AArch64 and ARM target, backend produce better ASM with this transformation. Please refer to below examples:


I'd agree. @spatel ?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1927-1928
 
+  // and(ashr(sub(0, V), ScalarSizeInBits -1), V) --> V s< 0 ? 0 : V, where sub
+  // hasNoSignedWrap.
+  {
----------------
maybe just
```
// and(ashr(subNSW(0, V), ScalarSizeInBits -1), V) --> V s< 0 ? 0 : V
```
?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1937-1939
+      Value *NewICmpInst =
+          Builder.CreateICmpSLT(V, ConstantInt::getNullValue(Ty));
+      return SelectInst::Create(NewICmpInst, ConstantInt::getNullValue(Ty), V);
----------------
Let's emit what we get in the tests?
```
      Value *NewICmpInst =
          Builder.CreateICmpSGT(V, ConstantInt::getNullValue(Ty));
      return SelectInst::Create(NewICmpInst, V, ConstantInt::getNullValue(Ty));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67799