[PATCH] D146928: [NaryReassociate] Transform expression (X << C1) + C2 to (X + C3) << C1,

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Scalar/NaryReassociate.h:152
 
+  // Tries to match X, C1 and C2 by using LHS and RHS.
+  bool matchConstLShiftOp(BinaryOperator *I, Value *LHS, Value *RHS, Value *&X,
----------------
```
// Tries to match (X << C1) + C2...
```
?


================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:510
+    auto *C3Val = ConstantInt::get(I->getType(), C2->lshr(*C1));
+    auto *Add = BinaryOperator::CreateAdd(X, C3Val, "add.nary", I);
+    auto *C1Val = ConstantInt::get(I->getType(), *C1);
----------------
If the initial operations had `nuw/nsw` flags, will they now be lost? Can we preserve them somehow?


================
Comment at: llvm/test/Transforms/NaryReassociate/NVPTX/const-shl.ll:1
+; RUN: opt < %s -passes=instcombine,nary-reassociate -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s --check-prefix=INSTCOMB
----------------
Could you please precommit the test to show what your patch is changing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146928