llvm dev - Jul 2021 - SimplifyCFG's switch to lookup table with illegal types.

This logic was originally added in
https://reviews.llvm.org/rG65df808f6254617b9eee931d00e95d900610b660 .  I’m not
sure what the code is trying to do.

In general, the number of instructions it takes to load a value into registers
from memory should be comparable to the number of instructions it takes to
synthesize it in registers.  It shouldn’t really matter if the type is legal.  I
can think of two reasons the type would matter:


  1.  Controlling the size of the lookup table; creating a table with types that
are very large, or have a lot of padding, could be inefficient.
  2.  Lowering a PHI node here could make it harder for other passes, like
instcombine, to slice it up later.  But we’re restricted the switch-to-lookup
table combine so it only runs late, so probably less of a concern now than it
used to be.

-Eli

From: Craig Topper <craig.topper at gmail.com>
Sent: Friday, July 30, 2021 8:56 AM
To: llvm-dev <llvm-dev at lists.llvm.org>; Eli Friedman <efriedma at
quicinc.com>; Alex Bradbury <asb at lowrisc.org>
Subject: [EXT] SimplifyCFG's switch to lookup table with illegal types.

ShouldBuildLookupTable contains a check to make sure the type of the Phi is
legal for the target. This currently prevents i32 lookup tables from being
formed on RISCV64 which only has i64 as a legal type. Obviously i32 is going to
be a more widespread type than i64 for C code so I would like to improve this.
But i8/i16 are also disabled on RISCV, ARM, AArch64, and probably other targets
which can do an i8/i16 load.

What is the right check here? Can we make sure it fits in the largest native
integer type from data layout instead? Or should we add another TTI hook to ask
the target?

Thanks,
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210730/c1545c38/attachment-0001.html>

What about types like i24 that likely need 2 or more loads with shifts? I
imagine synthesizing an i24 constant in a register might be simpler than
that?

Are you suggesting to remove the check altogether?

~Craig


On Fri, Jul 30, 2021 at 11:59 AM Eli Friedman <efriedma at quicinc.com>
wrote:
> This logic was originally added in
> https://reviews.llvm.org/rG65df808f6254617b9eee931d00e95d900610b660 .
> I’m not sure what the code is trying to do.
>
>
>
> In general, the number of instructions it takes to load a value into
> registers from memory should be comparable to the number of instructions it
> takes to synthesize it in registers.  It shouldn’t really matter if the
> type is legal.  I can think of two reasons the type would matter:
>
>
>
>    1. Controlling the size of the lookup table; creating a table with
>    types that are very large, or have a lot of padding, could be
inefficient.
>    2. Lowering a PHI node here could make it harder for other passes,
>    like instcombine, to slice it up later.  But we’re restricted the
>    switch-to-lookup table combine so it only runs late, so probably less of
a
>    concern now than it used to be.
>
>
>
> -Eli
>
>
>
> *From:* Craig Topper <craig.topper at gmail.com>
> *Sent:* Friday, July 30, 2021 8:56 AM
> *To:* llvm-dev <llvm-dev at lists.llvm.org>; Eli Friedman <
> efriedma at quicinc.com>; Alex Bradbury <asb at lowrisc.org>
> *Subject:* [EXT] SimplifyCFG's switch to lookup table with illegal
types.
>
>
>
> ShouldBuildLookupTable contains a check to make sure the type of the Phi
> is legal for the target. This currently prevents i32 lookup tables from
> being formed on RISCV64 which only has i64 as a legal type. Obviously i32
> is going to be a more widespread type than i64 for C code so I would like
> to improve this. But i8/i16 are also disabled on RISCV, ARM, AArch64, and
> probably other targets which can do an i8/i16 load.
>
>
>
> What is the right check here? Can we make sure it fits in the largest
> native integer type from data layout instead? Or should we add another TTI
> hook to ask the target?
>
>
>
> Thanks,
>
> ~Craig
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210730/5b8eb0b2/attachment.html>