[PATCH] D139970: [llvm][CallBrPrepare] use SSAUpdater to use intrinsic value

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:182
+
+    for (Use &U : CBR->uses()) {
+      if (!Visited.insert(&U).second)
----------------
efriedma wrote:
> You can't iterate this way if you're going to modify the uses, I think?  Once you rewrite a use, it refers to a different use-list, so the iterator's increment won't do the right thing.
You're right, I'll add a test for that. I have one locally that's failing due to this. Will add it.


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:204
+      SSAUpdate.AddAvailableValue(LandingPad, Intrinsic);
+      SSAUpdate.RewriteUse(U);
+    }
----------------
nickdesaulniers wrote:
> efriedma wrote:
> > This usage of SSAUpdater seems a little strange.  It would be more efficient to construct one SSAUpdater per callbr, instead of one SSAUpdater per use.  You can construct the SSAUpdater, AddAvailableValue all the intrinsic call for that callbr, then rewrite all the relevant uses using that SSAUpdater.
> You're right, I'll add a test for that. I have one locally that's failing due to this. Will add it.
sorry, wrong comment on wrong thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139970