[PATCH] D88285: [NARY-REASSOCIATE] Simplify traversal logic by post deleting dead instructions

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:244
+
+      const SCEV *OrigSCEV = SE->getSCEV(OrigI);
+      if (Instruction *NewI = tryReassociate(OrigI)) {
----------------
This is unused outside if, please move def closer to use.


================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:277
+          SeenExprs[OrigSCEV].push_back(WeakTrackingVH(NewI));
+      } else {
+        SeenExprs[OrigSCEV].push_back(WeakTrackingVH(OrigI));
----------------
nit: `{ }` not needed.


================
Comment at: llvm/test/Transforms/NaryReassociate/pr24301.ll:5
 
 define i32 @foo(i32 %tmp4) {
 ; CHECK-LABEL: @foo(
----------------
Not clear what has changed with your patch. Could you please auto-update the test, commit it as NFC and rebase on top of it to show the difference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88285