[PATCH] D31080: [DAG] Extract switch lowering as a spearate object NFC

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added a comment.

> The current implementation does try to split the analysis and lowering, but it doesn't succeed completely.
> 
> For example, findJumpTables() calls buildJumpTable() which creates a new MachineBasicBlock, creates a JumpTableHeader that it adds to the MachineFunction, and so on. In other words, it performs >some lowering.
> 
> Your patch works around this by providing virtual methods for buildJumpTable(): one that doesn't actually build a jump table, and the other that does, which means it has to depend on SelectionDAGBuilder.

Yes, I agree that the analysis shouldn't have to know about SelectionDAGBuilder.

> I'm saying it would be nice if the analysis and lowering could be separated further, so that the analysis does not have to know about SelectionDAGBuilder at all, and SelectionDAGBuilder would do the lowering entirely based on the results of the analysis.

If the main concern is the visibility of SelectionDAGBuilder in the base class for the analysis (SwitchLoweringCaseClusterBuilder in my current patch), don't you think further refactoring to remove SelectionDAGBuilder in SwitchLoweringCaseClusterBuilder by moving SwitchLoweringCaseClusterBuilderForDAG into SelectionDAGBuilder  could be reasonable design?


https://reviews.llvm.org/D31080