[PATCH] D101141: [SimplifyCFG] Preserve metadata when unconditionalizing branches.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:151
       // Replace the conditional branch with an unconditional one.
       Builder.CreateBr(Dest1);
       Value *Cond = BI->getCondition();
----------------
jeroen.dobbelaere wrote:
> Shouldn't we also take care of the metadata here ?
Yes, this patch is about the fixing PR50060 and I don't have a test case for this one.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:166
+      SmallVector<std::pair<unsigned, MDNode *>, 4> BranchMetadata;
+      BI->getAllMetadata(BranchMetadata);
+
----------------
jeroen.dobbelaere wrote:
> Can't we just copy the metadata from the `BI` directly ? Or will removing predecessors modify the attached metadata ?
> 
Removing code bears the risk that instructions are invalidated and should not be accessed anymore. I am not sure enough about removePredecessor since it causes an orphaned block. However, NewBr could be created before removePredecessor.
However, `BI->eraseFromParent()` is also only called afterwards, so it should be fine.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:175
+
+      // Transfer the metadata to the new branch instruction.
+      for (const auto &MDPair : BranchMetadata) {
----------------
lebedev.ri wrote:
> What about prof md?
Edge weights are not in the scope of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101141