[PATCH] D60137: Describe stack-id as an enum

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D60137#1451655 <https://reviews.llvm.org/D60137#1451655>, @thegameg wrote:

> This is definitely nicer, thanks for working on it.
>
> I have two concerns about this though:
>
> - we're going from a target-specific "namespace" to a target-independent one, which means in this case that no other target can use the stack id 1
> - we're exposing target-specific names and enums in the target-independent code


Thanks! Yes, you're right that exposing the target-specific names/enums in target-independent code is less desirable, although note that this happens in some other places in LLVM as well; think for example of the enum in CallingConv. I made the trade-off to use the enum for several reasons:

- By using an enum way we don't get target-specific MIR parsing of common properties.
- If we want to make it target-independent, we'll need to manually parse it as a string and we lose out on out-of-the box YAML parsing, printing, verification and diagnostics of having an enum value.
- If the value needs to be a string, it will have ugly quotes around it :)
- At the moment there seems to be only very limited use of stack-id, in trunk only the AMDGPU target as far as I know. Unless we care deeply about allowing to programatically generate a range of stack-ids at compile-time (with their meaning only known at compile-time), I don't think it is much of a limitation to have a single namespace.


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

https://reviews.llvm.org/D60137