[PATCH] D52002: Switch optimization for known maximum switch values

[Ayonam Ray via Phabricator via llvm-commits]

ayonam added a comment.

In https://reviews.llvm.org/D52002#1236729, @hans wrote:

> In https://reviews.llvm.org/D52002#1236722, @ayonam wrote:
>
> > I will move the storing of the MaxSwitchValue to the metadata of the instruction.  I think that is the right place to store these things.
>
>
> If we expand the switch at the IR Level, the pass that does the transformation could just depend on whatever Analysis pass is necessary to determine the value range, and we wouldn't have to store it anywhere.
>
> > However, I beg to differ on the second point.  If we do the changes in the IR by adding the requisite case statements, it will definitely generate the switch table that we expect to generate.  However, when we mark the default branch as unreachable, the lowering code will redirect the unreachable default to the most popular switch block.  And that would mean that the conditional branch for the default case would still be generated.
>
> Right, we would have to address the FIXME first as discussed below.
>
> > Also, when you point to the FIXME in the visitSwitch() do you mean that we can completely do away with an unreachable default case?  I feel theoretically we should be able to.  If there is an unreachable default, then we should be able to jump to the end of the switch block.  But even that would mean a conditional jump to the end of the switch block that would handle the default case.
>
> No, the idea would be that if the default is unreachable, we can just omit the range check from the jump table completely.
>
> SwitchToLookupTable in the SimplifyCFG pass already does the equivalent transformation: for switches used to select between constant values, it generates lookup tables, and if the default is unreachable it will omit the range check for the table completely.
>
> > I feel that the only way we can handle this is during lowering.  That decision about whether to completely skip the branch for the default case is only possible at that level.  Even if we do the creation of the expanded jump table by adding new case statements to the IR, this one thing probably has to be done at the time of lowering only.  Would you have other suggestions to handle this aspect?
>
> My suggestion is that the decision about whether to skip the branch for the default case should be driven by what the IR looks like: if the default basic block is unreachable, the branch can be skipped. I think this would be a clean design.


Thanks Hans for the detailed explanation.  I was about to go ahead and implement your suggestion but I noticed one problem here.

If the default basic block is unreachable and we completely omit the conditional branch in all such cases, then if the switch value at runtime turns out to be not one of the cases given in the switch block, the execution would reach the vectored branch to the jump table which would fail to reach anywhere.  So we need to omit the conditional branch only in such cases of the unreachable default block where the switch value is known to have a specific range of values.  That means that we still need to carry that information till the lowering phase as my current implementation is doing right now.  How we do that is the question.


Repository:
  rL LLVM

https://reviews.llvm.org/D52002