[PATCH] D22696: [CodeGenPrep] Skip merging empty case blocks

Jun Bum Lim via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 5 12:12:59 PDT 2016


junbuml added a comment.

> Does it also enable more machine code sinking? If not, perhaps the heuristics to count copy insertion phis can be improved?

The initial purpose of this patch was to handle the cases which cannot be handled in MachineSink by letting ISel insert COPY in better place.  By avoiding creating critical edge which is non-splittable in MachineSink, I think it could also indirectly increase the change of machine sinking. Since more number of copy insertion PHIs means more chances to sink in later pass, integrating the number of PHIs in the heuristics seems to be reasonable to me. However,  I think the minimum frequency check should come first because we don't want to leave extra branch in a likely executed block regardless of the number of PHIs.  To integrate  NumCopyInsertionPHIs and minimum frequency requirement, I thought about the heuristics something like :

CurFreqRatio = Freq(PredBB) / Freq(BB);
if (CurFreqRatio  > MinThresholdToSkipMerge && (NumCopyInsertionPHIs * CurFreqRatio) > MinNumPhiInDestToSkipMerge * MinThresholdToSkipMerge) {

  // skip merging current empty case

}

Please let me know your thought.

> How do you come up with 1000? It seems like specially tuned for that benchmark where the switch top is in a loop while the switch body is not -- the patch basically will never kick in for any switch in normal form without profile.

In case of the benchmark I was targeting, it has a huge switch in a loop and the switch have many empty cases only used as incoming blocks in PHIs. So, merging those empty cases cause many COPYs in the header of switch and MachineSink cannot sink them because it cannot split the critical edges across the jump table. Since the switch is huge, the ratio of FreqOfSwitchHeader to FreqOfEmptyCase was pretty large, more than 3000. In this patch I wanted to hit only such clear cases where the frequency of case is certainly lower than the switch header because the cost of extra branch added by skipping merging empty cases is sometime non-trivial especially when the case is likely to be taken as we observed in the povray case.


https://reviews.llvm.org/D22696





More information about the llvm-commits mailing list