[SPIR-V] Add proposal for global & local variable address spaces

[Keenuts]

This commit adds a proposal on how to implement local and global variables in the SPIR-V backend given HLSL put them in the same address space while SPIR-V requires them to be in 2 distinct ones.

[s-perron]

I think this makes sense. Just a few minor comments.

[Keenuts]

@llvm-beanz is that proposal OK for you?

[Keenuts]

@llvm-beanz ping

[llvm-beanz]

Ugh... I worry a lot about the approach to make everything private.

When we add references to HLSL we will absolutely require address space annotation on the references. That's going to become a foundational part of HLSL, and we'll need to provide a convenient way to normalize address spaces for the cases where users don't want to overload functions or variables for every possible address space. Something like:

template <typename ParamTy, typename ArgTy> struct CastingValue {
  CastingValue(ArgTy &Arg) : A(Arg), P(Arg) {}
  ~CastingValue() { A = P; }
  operator ParamTy &() { return P; }
  ArgTy &A;
  ParamTy P;
};

template <typename ParamTy, typename ArgTy>
CastingValue<ParamTy, ArgTy> make_inout(ArgTy &Arg) {
  return CastingValue<ParamTy, ArgTy>(Arg);
}

void fn(int &V) {}

float call() {
  float F = 1.5;
  fn(make_inout<int>(F));
  return F;
}

The more I'm seeing these arrays of troubling options, the more I think we just need to bite the bullet and add the address space annotations now differentiating between Private and Function.

If "Bad solution 2" isn't an option (and I'm not really sure that noinline or library shares should be blockers), I think we likely need to reconsider just biting the bullet on "Bad solution 1".

Because HLSL doesn't have references, the inout and out parameter semantics will normalize the address spaces of variables passed across function boundaries, you can't take an address of an object, so it may mostly be fine. The biggest area I worry about is potential type inference problems around template instantiations, but we can deal with that if it is a problem.

[llvm-beanz]

PR llvm/llvm-project#127675 triggered something in my brain. If we go forward with making global and local variables have different address spaces, we'll need to have some strategy (probably just alwaysinline and addrspacecast) for enabling calling member functions on global and local variable declarations seamlessly.

[Keenuts]

PR llvm/llvm-project#127675 triggered something in my brain. If we go forward with making global and local variables have different address spaces, we'll need to have some strategy (probably just alwaysinline and addrspacecast) for enabling calling member functions on global and local variable declarations seamlessly.

Yes, that's one of the issues I had when implementing this. Because we don't have yet defined how overloads are working depending on this, we have a problem:

• do we want to allow function overloading depending on this address space?
• if yes, how to handle source back-compat? I think the easiest solution would be to allow overloading, default on selecting the default address space version of the method, unless another is defined.

But this yields a new issue: what about const correctness? Is the address-space overload going first, before the const version?

Ex:

struct S {
void foo() {  }
void foo() const { }
void foo() addrspace(private) const { }
};

static S s;

[...]
s.foo(); // Shall we call `private const foo`, which is possible, or `default foo` because `s` is not const?

[Keenuts]

My initial approach was to do consider the following:

struct MyStruct {
void foo() { }
};

static MyStruct private_global;

void main() {
    MyStruct function_local;
    private_global.foo();
    function_local.foo();
    // typeof(private_global) != typeof(function_local);
}

• The struct is initially emitted with its member as-is.
• each variable having a different address space, their type are now different (even if both use MyStruct, but the additional AS makes them different)
• this means I duplicate the MyStruct type & methods, and create a new type for AS private.
• when emitting codegen for private foo, each pointer is marked as addrspace(private)
• when emitting codegen for local foo, each pointer is kept in the default AS.

This require duplicating the function in the IR: one version for the private AS, one for the default AS.
It requires some IR legalization and come codegen work, but I feel it would be the most natural way to use this.

This however requires references to have a fixed address space, making this illegal in HLSL:

MyStruct& get_var(bool condition, MyStruct& function_local) {
    return condition ? private_global : function_local;
}

I would be fine with this, as this is not legal in SPIR-V, but I do agree there is something fishy here. We'd want an addressspace attribute on the return type, or something visible.

[Keenuts]

Thanks for the reviews!
As discussed in yesterday's meeting, changed the proposal to have 2 address spaces exposed at the HLSL level: hlsl_private and default.

(I could create an hlsl_function, but doesn't seem necessary).

[s-perron]

I'm in favour of this solution, if it is acceptable to @llvm-beanz and his team.

I have a suggestion to reorganize. Don't mention rejected solutions while describing the problem. Leave it for the alternative. I believe that one of the alternative solutions has more problems than you let on.

[Keenuts]

Thanks, removed the solution bits from the problem statement to leave them to alternative solutions.

I believe that one of the alternative solutions has more problems than you let on.

If that's the force-inline solution, the word masking the complexity is the "propagating": it's not a simple problem, and can require a big code rewrite/duplication yes.

[llvm-beanz]

I don't have a whole lot of time this week to really dig deeply into this. I think for now the path of least resistance is going to be to ignore the fact that references may someday exist in HLSL. References are going to require a whole host of addresses space annotations as well as casting and conversion helpers.

On the topic of overloading based on address space, I'm inclined to align with modern C++ rather than define a new syntax (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p0847r7.html), but certainly in the short term we're going to need to do some crazy hacks nonsense to preserve enough source compatibility so as to not be too much of a burden on adoption.

[llvm-beanz]

You mention this a bit above, but just to highlight: even with giving global and local variables their correct SPIRV address spaces, we're going to need to do some degree of this.

The member function problem is a source compatibility issue that I don't think we can just break. DXC even has some awful hacks to allow member functions to resolve on objects in cbuffers, despite the lack of proper constness on the implicit object (although that might be a small enough break that we can get away with not doing those hacks in Clang).

[s-perron]

I think the point is that even if everything was inlined, the problem would not be solved because of export functions and the need to duplicate code. There might even be some instruction whose result address space cannot be determined by the types of the input.

[Keenuts]

I don't have a whole lot of time this week to really dig deeply into this. I think for now the path of least resistance is going to be to ignore the fact that references may someday exist in HLSL. References are going to require a whole host of addresses space annotations as well as casting and conversion helpers.

On the topic of overloading based on address space, I'm inclined to align with modern C++ rather than define a new syntax (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p0847r7.html), but certainly in the short term we're going to need to do some crazy hacks nonsense to preserve enough source compatibility so as to not be too much of a burden on adoption.

Agree, there are many behaviors linked to the addition of address spaces (resources, input & this) which will require some later iteration.
Thanks for the review! Reformulated one of the alternatives as suggested by Steven, rebased and merged.