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

Jun Bum Lim via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 23 14:33:55 PDT 2016


junbuml added a comment.

> I've thought about this only for a few minutes, and I'd definitely like to know if @danielcdh's patch also fixes this, but what is the benefit of creating critical edges here in the first place (i.e. when the case block is likely to be executed)? Shouldn't we decide later whether to speculate instructions (including copies that come from PHIs) later at the MI level (based on instruction costs and profiling data) as an independent set of decisions?


By skipping merging an empty case block, we could add an extra branch, so I do this only when the frequency of empty case block is significantly low using BFI. Based on the original purpose of CGP to better prepare IR for local ISel, doing this in CGP doesn't seem to be a really bad idea for me. However, handling this independently in MI level could be more reasonable.

     

> I tried dehao's patch on the original test case -- unfortunately it does not work. The reason is that MachineSink pass fails to split the critical edge because the indirect branch (from switch lowering) in the source BB prevents the edge from being split because it is not analyzable. See MachineBasicBlock::canSplitCriticalEdge


Thank you very much David for the test. Yes, I also confirmed that @danielcdh's patch didn't fix the case I was targeting. Let me take a closer look at the MachineSink pass to see if I can come up with a feasible solution there.


https://reviews.llvm.org/D22696





More information about the llvm-commits mailing list