[PATCH] D145890: [InstCombine] Generate better code for std::bit_floor from libstdc++

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:3312
+                               m_OneUse(m_Sub(m_APInt(C), m_Value(CTLZ)))))) ||
+      !(C->ugt(0) && C->ule(BitWidth)) ||
+      !match(CTLZ, m_OneUse(m_Intrinsic<Intrinsic::ctlz>(
----------------
Doesn't need to be added now, but please add TODO for `C==0` case if we have a mask i.e:
https://alive2.llvm.org/ce/z/epLwX_

In general, you could either match `C - CTZL` with `C > 0 && C <= BitWidth` or match `(C - CTLZ) % BitWidth` for any `C` (then set bit `(C->getZExtValue() - 1) % BitWidth`.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:3318
+  APInt ShiftedBit(BitWidth, 1);
+  ShiftedBit = ShiftedBit << (C->getZExtValue() - 1);
+
----------------
```
  APInt ShiftedBit = APInt::getOneBitSet(BitWidth, C->getZExtValue() - 1);
```


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:3325
+  return cast<Instruction>(
+      Builder.CreateLShr(ConstantInt::get(NType, ShiftedBit), NewCTLZ));
+}
----------------
This can be exact:
https://alive2.llvm.org/ce/z/Enurag

I think:

```
BinaryOperator *BO = BinaryOperator::CreateLShr(ConstantInt::get(NType, ShiftedBit), NewCTLZ);
  BO->setIsExact();
  return BO;
``` 

Should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145890