[llvm-commits] [Patch] Use bitmaps for switch lookup tables that fit in registers

[Sean Silva]

LGTM.

How often is the bitmap case triggering when building, e.g. LLVM/Clang
or another large project? Also, are you seeing any binary size
improvements?

--Sean Silva

On Fri, Sep 21, 2012 at 6:22 AM, Hans Wennborg <hans at chromium.org> wrote:
> Thank you for the comments! Replies inline and new versions of the
> patches attached.
>
>  - Hans
>
> On Fri, Sep 21, 2012 at 2:37 AM, Sean Silva <silvas at purdue.edu> wrote:
>>>> I know this is unrelated to this patch, which is just a bit of
>>>> refactoring, but is it really necessary to single out the 1-case
>>>> switch like this? Will a 1-case lookup table not get optimized away
>>>> somewhere else? It just seems weird to single it out like this:
>>>> wouldn't it be equally reasonable to single out when there are 2 cases
>>>> (convert to `select`)?
>>>
>>> It's not a 1-case switch, it's an n-case switch where all cases result
>>> in a single value.
>>
>> Ok, I see what was confusing me. A function called BuildLookup, which
>> is in a class called SwitchLookupTable, which is in a code path which
>> satisfies a predicate ShouldBuildLookupTable(), ends up not building a
>> lookup table.
>
> I agree that the names might not be optimal here. I'm happy to take
> suggestions for better ones.
>
> However, I still think of the class as an abstraction of a lookup
> table. It's just that for the special case when the table has the same
> value in each element, it stores that in an efficient way (i.e. just
> stores the value), and retrieving the value with BuildLookup becomes
> trivial.
>
>> Please factor this logic into two clear parts. One which "analyzes"
>> the switch and determines the best way to handle it, and another which
>> actually handles transforming the code.
>
> By creating the SwitchLookupTable class, I'm already dividing the
> logic into two clear parts: the SwitchToLookupTable function analyzes
> the switch and extracts the resulting values, and SwitchLookupTable
> builds a lookup table and instructions to retrieve values from it. I
> think it makes sense that the logic for how to build the table resides
> in the function that builds it.
>
>> Also, for each different
>> "kind" of switch that the analysis determines, please document (above
>> the enum constant for the kind would be a good place):
>> * A human description of that kind of switch.
>> * An example of that kind of switch.
>> * A human description of what it gets transformed into.
>
> I've expanded the descriptions a bit. Hopefully they're clearer now.