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

[weiwei chen via llvm-commits]

================
@@ -72,12 +72,21 @@ 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;
+        if (AllocInstPtrTy->getAddressSpace() != ADDRESS_SPACE_LOCAL) {
----------------
weiweichen wrote:

> If the choice was between alloca in generic AS and alloca in AS(5), then indeed the choice would be to cast or skip. However, considering that alloca's AS can be explicitly specified to be in any AS, it would be prudent to have an assertion, so we catch unexpected input early, at the point where we care about it.
> 
> If we don't have an assertion, and we get an alloca in AS 333, we'll happily insert an ASC which will cause lowering problems further down in the pipeline and whoever will have to debug that will have to back-track to here before moving on to the real root cause -- nonsensical input IR. I'd much rather break things here, as soon as we're aware that something is wrong.

Definitely agree that assert should be there if the AS is not local! However, the change in my PR does not remove that assert if the AS is say `333` because the `if` added to avoid cast will not be true and the `cast` will still happen and assert, right? Do you mean that an explicit assert should be added in `NVPTXLowerAlloca.cpp` instead of rely on the constructor of `AddrSpaceCastInst` to assert? If so, I think that's a good point too, and we can provide more meaningful error message as well. 

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