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

Hans Wennborg hans at chromium.org
Fri Sep 21 03:22:18 PDT 2012


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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 001_switch_lookup_table2.patch
Type: application/octet-stream
Size: 10023 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120921/015447aa/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 002_switch_bitmap3.patch
Type: application/octet-stream
Size: 15568 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120921/015447aa/attachment-0001.obj>


More information about the llvm-commits mailing list