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

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:52
+  void UpdateSSA(DominatorTree *DT) const;
+  DenseMap<CallInst *, CallBrInst *> Map;
 
----------------
this is dangerous to keep in the FunctionPass since they can be reused

I'd create it per-invocation in `run()`


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:171
+
+void CallBrPrepare::UpdateSSA(DominatorTree *DT) const {
+  SSAUpdater SSAUpdate;
----------------
make this a reference since it's required


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:181
+    for (Use &U : CBR->uses()) {
+      PrintDebugDomInfo(DT, U, LandingPad, /*IsDefaultDest*/ false);
+      PrintDebugDomInfo(DT, U, DefaultDest, /*IsDefaultDest*/ true);
----------------
I'd just `ifndef NDEBUG` around these two lines and remove the empty `PrintDebugDomInfo` implementation


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:187-189
+        SSAUpdate.Initialize(U->getType(), U->getName());
+        SSAUpdate.AddAvailableValue(LandingPad, Intrinsic);
+        SSAUpdate.RewriteUseAfterInsertions(U);
----------------
maybe I'm misunderstanding what is going on, but can this be `U.set(Intrinsic)`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139970