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

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In https://reviews.llvm.org/D31080#705381, @junbuml wrote:

> Thanks Hans and Chandler for the reviews.  My original intention for this was to have a single logic for calculating case clusters so that we don't need to maintain several different versions depending on places it's used.
>
> From that perspective, I extract the logic for case clustering abstractized in SwitchLoweringCaseClusterBuilder with two virtual methods (buildJumpTable and buildBitTests). Basic  implementation of them in SwitchLoweringCaseClusterBuilder don't build anything related with DAGBuilder, which will be used in the TTI hook, but those in SwitchLoweringCaseClusterBuilderForDAG do extra stuffs for DAGbuilder as it will be invoked in SelectionDAGBuilder. However,  I agree that SwitchLoweringCaseClusterBuilder should not need to know about SDAGBuilder.
>
> I think having a single abstraction for calculating case clusters might be a good for maintenance and accuracy as long as the calculation cost is reasonable.  So, would it make sense to define more virtual methods to cut out expensive calculations for non-DAG builder case ?


Rather than splitting those out as virtual calls, I think the nicest design would be if the case clustering logic could work as an analysis: you feed it an array of cases, from which it computes some kind of result which indicates what cases go in jump tables, which are bit tests, etc. The SDAGBuilder would then consume that result to actually build the jump tables etc.


https://reviews.llvm.org/D31080