[PATCH] D69861: [JumpThreading] Factor out code to clone a basic block (NFC)

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:1976-1978
+/// Clone basic block BB except its terminator just for predecessor PredBB.
+/// Return the newly created basic block and the map from the variables in BB to
+/// the variables in the newly created basic block.
----------------
It is a little weird for the interface to leave the insertion of BB terminator outside. Can we add SuccBB as another param, and include some processing after the call of CloneBasicBlock into this function?


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:1991-1996
+  // Set the block frequency of NewBB.
+  if (HasProfileData) {
+    auto NewBBFreq =
+        BFI->getBlockFreq(PredBB) * BPI->getEdgeProbability(PredBB, BB);
+    BFI->setBlockFreq(NewBB, NewBBFreq.getFrequency());
+  }
----------------
The Block frequency update seems not precise enough. I feel it is more precise to use BB-->SuccBB branch probablity to compute the block frequency of NewBB. That can be a separated change from this refactoring, but it suggests we may want to include SuccBB as a param. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69861