[PATCH] D109008: [AMDGPU][NFC] Refactor AMDGPUCallingConv.td

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCallingConv.td:230
+
+def CSR_AMDGPU_AllVGPRs : RegMask<
+  (sequence "VGPR%u", 0, 255)
----------------
scott.linder wrote:
> arsenm wrote:
> > Probably shouldn't have the CSR prefix if it's just all registers
> I had originally used a different prefix, and considered no prefix, but I wasn't sure if it would be confusing enough in e.g. the MIR to have some masks with the prefix and some without? Is there any risk of colliding with something else in the MIR syntax?
I looked in the MIRParser and it doesn't seem like stripping the prefix from these should cause any ambiguity:

```
    case MIToken::Identifier:
      if (const auto *RegMask = PFS.Target.getRegMask(Token.stringValue())) {
        Dest = MachineOperand::CreateRegMask(RegMask);
        lex();
        break;
      } else if (Token.stringValue() == "CustomRegMask") {
        return parseCustomRegisterMaskOperand(Dest);
      } else
        return parseTypedImmediateOperand(Dest);

...

  bool MIParser::parseTypedImmediateOperand(MachineOperand &Dest) {
    assert(Token.is(MIToken::Identifier));
    StringRef TypeStr = Token.range();
    if (TypeStr.front() != 'i' && TypeStr.front() != 's' &&
        TypeStr.front() != 'p')
      return error(
          "a typed immediate operand should start with one of 'i', 's', or 'p'");
```

Even if we collided with the prefixes used for immediates the parser prioritizes regmasks.

I'll remove the prefix from these and post an updated patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109008/new/

https://reviews.llvm.org/D109008