[llvm-commits] [llvm] r163302 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/switch_create.ll test/Transforms/SimplifyCFG/switch_to_lookup_table.ll

[Hans Wennborg]

On Thu, Sep 13, 2012 at 6:13 PM, Hans Wennborg <hans at chromium.org> wrote:
> On Wed, Sep 12, 2012 at 8:26 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> I think this is a reasonable approach. Can you verify that it handles all the cases? That is, disabling the simplifycfg optimization generates the same code as simplifycfg + converting table back to switches?
>
> I've polished the patch a bit and attached the new version. Yes, this
> handles all the cases. The code is not going to be identical to if we
> disable simplifycfg, e.g. if we had branch frequency metadata on the
> original switch, that will be gone. It might also change which is
> default case of the switch.
>
> My question now is how to hook this up and test it. I guess we should
> add a flag in TargetLoweringInfo to enable/disable it? I looked at
> SupportJumpTables, but that's only used (i.e. set to false) by PPC for
> JIT compilation. Which targets do we want to enable this for?
>
> Once we know which targets to enable this for, I guess I should test
> it by running llc for that target, and check that we don't emit the
> lookup table?
>
> Duncan: Doing the more general approach scares me a little bit. I
> would prefer to use the more "narrow" approach of looking at the GEPs,
> I believe it should cover all the lookup tables generated by my
> original transform.

I've attached a new patch that turns the lookup tables back into
switches for targets that don't support jump tables. That was the best
hook in TargetLowering that I could find. It also has a test.

OK to commit?

Comments for hooking it up better and/or better testing are welcome.

Thanks,
Hans


>> On Sep 11, 2012, at 10:01 AM, Hans Wennborg <hans at chromium.org> wrote:
>>> On Mon, Sep 10, 2012 at 7:19 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>>>> The *right* longer term solution is to expose more target info to LLVM IR
>>>> passes. It's going to be more and more important especially with recent
>>>> interest in vectorization. I am fairly certain we'll start exposing some
>>>> target "instruction cost" (to start) in 2013.
>>>>
>>>> That said, I think we will need a short term compromise. Hans, can you
>>>> experiment with moving this to codegenprep? I'd like to see what are the
>>>> interaction with other IR optimizations.
>>>
>>> I've been looking into moving this to codegenprep today. One of the
>>> issues that came up is that if we move out of SimplifyCFG, we don't
>>> get the benefit from some of the clean-ups it does, e.g. removing
>>> unreachable blocks, merging blocks, etc. It seems that the codegen
>>> passes manage to clean this up pretty well, but the output on the IR
>>> level is not as good.
>>>
>>> Another thing is that I couldn't find a way to run codegenprep via opt
>>> so that TargetLowerInfo is available. I guess this means I wouldn't be
>>> able to test my transformation via opt, but should use llc instead? Or
>>> I guess I could turn the logic around and do my transformation
>>> *unless* there is TLI available which says we shouldn't do it.
>>>
>>> Talking to Duncan on IRC, he thinks we should not move this to
>>> CodeGenPrepare, but instead look into converting the table look-ups
>>> back into switches for those targets that need it, and that this would
>>> benefit other loads from constants as well. We could do this quite
>>> easily in CodeGenPrepare, I think. It would look something like the
>>> attached (work in progress) patch.
>>>
>>> Please let me know what you think.
>>>
>>> Thanks,
>>> Hans
>>> <load_to_switch.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: load_to_switch3.patch
Type: application/octet-stream
Size: 9469 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120918/184e4056/attachment.obj>