[llvm-commits] [Patch] Use TargetTransformInfo to control switch-to-lookup table transform

[Eli Friedman]

On Mon, Oct 29, 2012 at 9:31 AM, Hans Wennborg <hans at chromium.org> wrote:
> On Mon, Oct 29, 2012 at 3:29 PM, Duncan Sands <baldrick at free.fr> wrote:
>>> When the switch-to-lookup tables transform landed in SimplifyCFG, it
>>> was pointed out [1] that this could be inappropriate for some targets.
>>> Since there was no way at the time for the pass to know anything about
>>> the target, an awkward reverse-transform was added in CodeGenPrepare
>>> that turned lookup tables back into switches for some targets.
>>>
>>> The attached patch uses the new TargetTransformInfo to determine if a
>>> switch should be transformed, and removes
>>> CodeGenPrepare::ConvertLoadToSwitch. Please take a look!
>>
>>
>> personally I think it is completely wrong to have codegen concepts such as
>> ISD::BR_JT and ISD::BRIND leaking into the IR level:
>>
>>> +bool ScalarTargetTransformImpl::shouldBuildLookupTables() const {
>>> +  return TLI->supportJumpTables() &&
>>> +      (TLI->isOperationLegalOrCustom(ISD::BR_JT, MVT::Other) ||
>>> +       TLI->isOperationLegalOrCustom(ISD::BRIND, MVT::Other));
>
> But as Hal pointed out below, those codegen concepts don't leak into
> the IR level. The only thing that goes into the IR level is a "yes" or
> "no" on whether the IR level should do this switch transformation or
> not. Is that what you're opposing, or are you opposing the way
> ScalarTargetTransformImpl::shouldBuildLookupTables is implemented?

Well, there is a rather obvious issue with the way it is implemented:
whether a target supports jump tables is mostly unrelated to whether
it supports lookup tables.

-Eli