[PATCH] D30529: [GlobalISel] Enable specifying how to legalize non-power-of-2 size types.

Kristof Beyls via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 04:38:26 PDT 2017


kristof.beyls added a comment.

In https://reviews.llvm.org/D30529#698648, @rengolin wrote:

> Hi Kristof,
>
> I haven't seen the patch at all, but what about situations where 64-bit is done with lib calls? For example, `ldivmod` takes 64-bit arguments and you wouldn't want to narrow them to 32-bits.
>
> If this patch is intended to just simplify the legal vs. others, it shouldn't have a narrow-all that spans to +inf. Makes sense?
>
> cheers,
> --renato


Hi Renato,

This patch should allow to specify everything you want to do for each individual bitsize, if that's what you want.
I'm not exactly sure what exact actions you're thinking of are needed for the different sizes of div or mod, but for example, you could specify:

  setAction({G_REM, LLT:scalar(1)},
            {{1, WidenScalar},  // bit sizes [ 1, 31[
             {32, Lower},       // bit sizes [32, 33[
             {33, NarrowScalar}, // bit sizes [33, 63[
             {64, Libcall}, // bit sizes [64, 65[
             {65, Unsupported} // bit sizes [65, +inf[
            });

I'm assuming the above example wouldn't be what you want to do in detail, but it just shows that for different bit sizes, you can specify to do different things to legalize.

Since the design should allow to specify what to do for all bit sizes, there should be a mapping from the set of natural numbers (all bit sizes) to what action to take.
In this patch, I chose for that mapping to be represented using a simple vector, with the boundary bit sizes where the action changes represented as an element in the array.

As this can get a bit verbose, I've added a few helper/syntactic sugar functions that help to specify typical specifications concisely. I created these functions based on the specifications that already exist in the existing backends that support GlobalISel.
The most-used example in this patch is `UnsupportedButFor`. An example of how it's used from the AArch64 backend is:

  for (unsigned BinOp : {G_SREM, G_UREM})
    setAction({BinOp, Scalar}, UnsupportedButFor({1,8,16,32,64}, Lower));

which without this helper function, would be written as something like:

  for (unsigned BinOp : {G_SREM, G_UREM})
    setAction({BinOp, Scalar}, 
          {{1, Lower},  // bit sizes [ 1, 1[
           {2, Unsupported},       // bit sizes [2, 8[
           {8, Lower},  // bit sizes [ 8, 8[
           {9, Unsupported},       // bit sizes [9, 16[
           {16, Lower},  // bit sizes [ 16, 17[
           {17, Unsupported},       // bit sizes [17, 32[
           {32, Lower},  // bit sizes [ 32, 33[
           {33, Unsupported},       // bit sizes [33, 64[
           {64, Lower},  // bit sizes [ 64, 65[
           {65, Unsupported} // bit sizes [65, +inf[
          }

You can find more examples in the changes in this patch in the {Target}LegalarizerInfo.cpp files.
Of course, the whole idea of this patch is to be able to easily specify what to do to legalize ranges of bit sizes, e.g. using "NarrowScalar" or "WidenScalar" on a wide range of bitsizes.
Without this patch, currently every single bitsize has to be explicitly enumerated, which is far from ideal.
There are a few examples of how this is done in the first version of the patch on this review. In the second version, I decided to make the patch NFC, which as a consequence means there aren't many (no?) examples of "NarrowScalar" or "WidenScalar" over a range of bit sizes, as that wasn't easily specified before.
One simple example could be:

  for (unsigned BinOp : {G_ADD, G_SUB, G_MUL, G_AND, G_OR, G_XOR, G_SHL}) {
      // These operations naturally get the right answer when used on
      // GPR32, even if the actual type is narrower.
      setAction({BinOp, s1},
        {{1, WidenScalar},
         {32, Legal},
         {33, WidenScalar},
         {64, Legal},
         {65, NarrowScalar}
        });

which specifies the intended action for all bit sizes. Before this patch, you'd have to specify (have a call to setAction) for every single bit size you'd want to support. Apart from wasting a lot of memory in tables, you'd also need to decide what the largest bit size would be you'd like to support, as you wouldn't be able to specify "all bit sizes larger than this should be legalized using this action".

I hope the above makes sense?

Thanks,

Kristof


https://reviews.llvm.org/D30529





More information about the llvm-commits mailing list