[PATCH] [SimplifyCFG] threshold for folding branches with common destination

Jingyue Wu jingyue at google.com
Mon Sep 29 16:43:08 PDT 2014


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2034
@@ +2033,3 @@
+      continue;
+    if (!I->hasOneUse() || !isSafeToSpeculativelyExecute(I, DL))
+      return false;
----------------
hfinkel wrote:
> I don't think that the hasOneUse check here really does what you want once we allow for more than once instruction. We used to check that the single bonus instruction had one user and this user specifically was the Cond. Now you'd like to allow for some variable number of single-use instructions (regardless of a relationship to Cond). This could change behavior even when allowing only a single instruction.
> 
> I think that what you really want to do is to walk up the operand graph from Cond, accumulating instructions until you reach your limit, keeping the hasOneUse check and the check that the use is Cond for the first instruction.
Hi Hal, 

Thanks for the careful review! However, I don't think the modified code changes the behavior when allowing only a single bonus instruction. Note that in Line 2037 I check whether the only user is in the same BB (and appears after the potential bonus instruction otherwise def doesn't dominate use). When there is only one potential bonus instruction, it is either used by Cond or BI (DbgInfoIntrinsic only uses MDNode but not Instruction). Being used by BI is impossible, because BI only uses Cond as its first operand and other operands are all BB labels. Therefore, this bonus instruction must be used by Cond. Does this make sense? I agree it is at least worth a comment. 

I like your suggestion of early exiting once we reach the limit. I'll change that part. 

Thanks,
Jingyue

http://reviews.llvm.org/D5529






More information about the llvm-commits mailing list