[clang] [clang][CodeGen] `sret` args should always point to the `alloca` AS, so use that (PR #114062)

[John McCall via cfe-commits]

================
@@ -225,7 +225,9 @@ ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty, bool Variadic,
     // Records with non-trivial destructors/copy-constructors should not be
     // passed by value.
     if (auto RAA = getRecordArgABI(Ty, getCXXABI()))
-      return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory);
+      return getNaturalAlignIndirect(
+          Ty, getContext().getTargetAddressSpace(LangAS::Default),
+          RAA == CGCXXABI::RAA_DirectInMemory);
----------------
rjmccall wrote:

Okay, let's step back for a second. Compiler implementation is a naturally *highly* combinatoric problem; everything we do here is prone to an explosion of complexity wherever features and/or target variations interact. It is very important that, as we work on the compiler, we always have a philosophical mindset, trying to figure out the right way to think about the problem and organize our solution. Otherwise, we find ourselves drowning in "special cases" that never quite seem to fix the bugs.

I am assuming here that this is one patch in a series that, cumulatively, will implement a sensible plan for handling address spaces for indirect argument and return values. This is a part of the compiler that clearly got overlooked a bit, so we shouldn't be surprised to find inconsistencies and unexpected behavior. Some of those will be bugs; others will actually be intended, and we should be looking at those for that philosophical insight about the right way to model what we're doing.

My experience is that patches that refactor bits of the API and therefore necessitate touching a lot of code are the ideal time to be looking at each of those places and trying to figure out what the code is supposed to be doing. You don't have to actually make semantic changes as you go, but you need to at least have these conversations where you acknowledge "okay, this is doing X right now, but it should really be using the general rule for Y" and then leave a FIXME behind saying that, one which you intend to fix within the next few patches.

But that is what I am doing in this code review: I'm not trying to derail your PR, I'm trying to set your whole series of patches up for success so that we end up with a coherent design and implementation.

https://github.com/llvm/llvm-project/pull/114062