[PATCH] D44704: [GlobalISel][X86][ARM] Relaxing type constraints on G_SHL and friends

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin added a comment.
Herald added a subscriber: chrib.

In https://reviews.llvm.org/D44704#1049848, @dsanders wrote:

> The GenericOpcodes.td change makes sense to me but I'm surprised that only AMGGPU's LegalizerInfo is changing and only for SHL. I'd have expected most (probably all) GlobalISel targets to need to change all three shifts.
>
> Also, could you add a test case?


AMDGPU only legalizes `G_SHL`, non of the others. And w/o the `setAction({G_SHL, 1, S32}, Legal);` already existing tests fail (`CodeGen/AMDGPU/GlobalISel/legalize-shl.mir` and `CodeGen/AMDGPU/GlobalISel/regbankselect-shl.mir`), so no need to add another one. As for the selector, this is entire AMDGPU's selector:

  case TargetOpcode::G_ADD:
    return selectG_ADD(I);
  case TargetOpcode::G_CONSTANT:
    return selectG_CONSTANT(I);
  case TargetOpcode::G_GEP:
    return selectG_GEP(I);
  case TargetOpcode::G_LOAD:
    return selectG_LOAD(I);
  case TargetOpcode::G_STORE:
    return selectG_STORE(I);

Every other target legalizes shifts by using the new-style legalization action builders, and those have a couple of rather dangerous properties:

1. If actions are built for multiple opcodes simultaneously, they are actually directly defined for a representative only, making the rest aliases, which is fine, but nothing checks that all the aliases agree in the number of type indices.
2. while actions are built for the representative, nothing checks that they are properly defined for every type index, implicitly making any type legal for any type index not mentioned with no action required.

This is why none of the other already existing tests fail, though there are plenty that test legalization of shifts.


Repository:
  rL LLVM

https://reviews.llvm.org/D44704