[llvm] [NVPTX] Check Before inserting AddrSpaceCastInst in NVPTXLoweringAlloca (PR #106127)

Artem Belevich via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 4 11:02:12 PDT 2024


================
@@ -72,12 +72,26 @@ bool NVPTXLowerAlloca::runOnFunction(Function &F) {
         Changed = true;
         auto ETy = allocaInst->getAllocatedType();
         auto LocalAddrTy = PointerType::get(ETy, ADDRESS_SPACE_LOCAL);
-        auto NewASCToLocal = new AddrSpaceCastInst(allocaInst, LocalAddrTy, "");
-        auto GenericAddrTy = PointerType::get(ETy, ADDRESS_SPACE_GENERIC);
-        auto NewASCToGeneric =
-            new AddrSpaceCastInst(NewASCToLocal, GenericAddrTy, "");
-        NewASCToLocal->insertAfter(allocaInst);
-        NewASCToGeneric->insertAfter(NewASCToLocal);
+        PointerType *AllocInstPtrTy =
+            cast<PointerType>(allocaInst->getType()->getScalarType());
+        Instruction *NewASCToGeneric = allocaInst;
+        unsigned AllocAddrSpace = AllocInstPtrTy->getAddressSpace();
+        assert((AllocAddrSpace == ADDRESS_SPACE_GENERIC ||
+                AllocAddrSpace == ADDRESS_SPACE_LOCAL) &&
+               "AllocaInst can only be in Generic or Local address space for "
+               "NVPTX.");
+        if (AllocAddrSpace != ADDRESS_SPACE_LOCAL) {
+          // Only insert a new AddrSpaceCastInst if
+          // allocaInst is not already in ADDRESS_SPACE_LOCAL.
+          auto NewASCToLocal =
+              new AddrSpaceCastInst(allocaInst, LocalAddrTy, "");
+          auto GenericAddrTy = PointerType::get(ETy, ADDRESS_SPACE_GENERIC);
----------------
Artem-B wrote:

> -infer-address-spaces won't work properly if the extra ASCast to LOCAL is not inserted if alloca is already in Generic 

That didn't parse. Are you saying that inferring does not work for the plain `alloca()` -- it is already in generic AS. In other words it's broken right now?

Or that your patch skipped that "asc(asc(alloca, to AS(5)), to AS(0)" that the current code adds and that infer address spaces relies on? In that case, yes, we'll still need to keep those ASCs.


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


More information about the llvm-commits mailing list