AlexVlx wrote: > I'm fine with how you're handling the address spaces for now. > > I'd like to talk about the rule you're implementing, though. It looks like > it's supposed to be: > > * return values always use the alloca AS > * arguments always use the default AS > * whether something is indirect because it's non-POD or simply too big to fit > in registers doesn't make a difference > > That's a surprising rule; in fact, it's the exact opposite of the rule I > would expect. > > Indirect arguments are always true temporaries. The caller has total control > over where to allocate and initialize the temporary, and it has very little > reason to not always use the stack. So there's no reason for the ABI to not > specify that the argument pointer is passed in the alloca AS. > > Return values, in contrast, can be used to directly initialize all sorts of > different memory, not just objects on the stack. So the ABI should probably > be to pass as generic a pointer as the target supports. Moreover, while > passing a restricted pointer in C is okay because we can always use pass a > temporary and then relocate the object after the call, the same is not true > for types that are non-trivial to copy in C++ — we are not generally allowed > to introduce extra moves of such objects. So even if you normally want to > return values in the alloca AS as an optimization, you do need to make an > exception for non-trivial C++ objects. > > Unrelated note: I think most of the targets can just hardcode that they use > AS 0 for alloca and default AS; no need to query for that all over the place.
I think that calling it a "rule" gives it far too much credit, since it's just an artifact of wanting to avoid tinkering with current behaviour:) Previously `getIndirect` (and the convenience variant that forwards to it) would not have yielded an address space for the returned value, so this is not used at the moment anywhere / no extant caller would care about it being there / using it. Thus, args using the default AS is just the equivalent to hardcoding 0, now that the interface takes an AS. I am opposed to hardcoding 0 in general, and there were objections in this very review to the notion, hence using default. TL;DR, for arguments this is just maintaining current implicit behaviour whilst adopting the new interfaces, it's not trying to introduce anything new, and should really be NFC. In what regards the return value, whilst I appreciate the theoretical possibility, I will note that with how Clang works today we will never practically get a non `alloca`d return value, as far as I can tell. Moving away from that to the more general case you describe would require more work (in Clang itself), probably actually encoding the rule you describe, leveraging the interfaces we're adding with this change. IMHO, as currently written, we're just reflecting what Clang does for indirect returns, and adding interfaces that can be used to improve this in the future. Overall, if this has ended up doing too much, I'm happy to shrink it to merely addressing the immediate concern for AMDGPU + `sret`, and we can fork out the wider conversation into a separate PR. I would like to at least address the AMDGPU part of the problem as the current state of affairs creates some challenges. https://github.com/llvm/llvm-project/pull/114062 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
