[PATCH] D118426: [InstCombine] Remove side effect of replaced constrained intrinsics

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1057
+        CI.replaceAllUsesWith(V);
+      return ConstrainedIntr;
+    }
----------------
You can't use RAUW in InstCombine. This should be `return replaceInstUsesWith(II, V)`.

...actually, as we already called SimplifyCall above, this situation can only happen if you have a constrained intrinsic with use_empty(). I'd suggest to check for use_empty() + simplify succeeds and directly erase the call in that case. I think that will be more obvious that adding a readnone attribute to get it erased in a separate iteration of InstCombine.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3376
+    // existed, SimplifyInstruction would return null value for this
+    // constrained intrinsic.
+    if (Res != &CI)
----------------
Can you please add a comment to InstructionSimplify that documents this additional requirement? That is, that constrained intrinsics are only allowed to be folded if they do not side effect (which is different from the usual InstSimplify rules).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118426