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

[Hans Wennborg]

On Fri, Sep 21, 2012 at 8:20 PM, Sean Silva <silvas at purdue.edu> wrote:
> LGTM.

Thanks!

> 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?

I did a Clang build with this last week; the transformation fired 295
times and reduced the binary size by 8328 bytes. That's not much of an
overall reduction, but at least it's a decent reduction per switch
where it fires.

Chromium showed similar numbers: the transformation fired 140 times
and binary size reduction was 4424 bytes.

In both cases, 99% of the bitmaps consisted of i1 values; the rest
consisted of i8 values.

Does anyone else have comments? OK to commit?

Thanks,
Hans

> 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.