[llvm] [SimplifyCFG] Fix hoisting problem in SimplifyCFG (PR #78615)

via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 5 08:46:50 PST 2024


================
@@ -1742,15 +1767,6 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
       continue;
     }
 
-    SmallVector<Instruction *, 8> OtherInsts;
-    if (SameLevelHoist) { 
----------------
amehsan wrote:

@DianQK 

I have discussed this patch with Rouzbeh offline (before implementation and also regarding the recent discussion).  About your suggestion of separating steps 1,2 and 3 above this is how I think about it.

1. separating step 3 makes sense. It makes the code cleaner and also it is easy to implement.
2. separating step 1 and 2 from each other sounds a little problematic for the following reasons: (a) I do not agree that separating this two steps makes the code easier to read. If it does, the improvement is marginal. I think current implementation is more elegant. (b) the separation could result in subtle issues. It is not clear when we want to go from 1 to 2 and different decisions could potentially cause different problems: either compile time overhead, or corner cases that are difficult to handle.


https://github.com/llvm/llvm-project/pull/78615


More information about the llvm-commits mailing list