SROA: Simplify addrspacecasted allocas with volatile accesses

[arsenm]

Summary

If the alloca is accessed through an addrspacecasted pointer, allow
the normal changes on the alloca. Cast back to the original use
address space instead of the new alloca's natural address space.

[sanjoy]

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

[reames]

In D63401#1546293, @sanjoy wrote:

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

[arsenm]

In D63401#1547107, @reames wrote:

In D63401#1546293, @sanjoy wrote:

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

For an equivalent bitcast, the type of the underlying alloca is changed into a nicer type and possibly split into multiple allocas (with volatile accesses). I conservatively added checks in r363462 to avoid using the alloca's natural address space for the new operations. I figure it's safer to not change the address space of volatile accesses, but I don't particularly care about this property. InferAddressSpaces avoids changing volatiles, but that was also my idea.

[reames]

In D63401#1547122, @arsenm wrote:

In D63401#1547107, @reames wrote:

In D63401#1546293, @sanjoy wrote:

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

For an equivalent bitcast, the type of the underlying alloca is changed into a nicer type and possibly split into multiple allocas (with volatile accesses). I conservatively added checks in r363462 to avoid using the alloca's natural address space for the new operations. I figure it's safer to not change the address space of volatile accesses, but I don't particularly care about this property. InferAddressSpaces avoids changing volatiles, but that was also my idea.

So, just to say this back to you, this would be treating an addrspacecast more like a bitcast when the source is known to be an alloca?

Honestly, this feels more than a bit suspect to me, but I can't argue against it on a principled basis. Semantics for addrspacecast are so ill specified that it's hard to say what is and isn't legal in terms of transforms. Just to be clear, the critical detail here to me is "how do we handle addrspacecast", not "how to do handle volatile". From what I can tell, the existing code for non-volatiles would already have surprising semantics. I don't see any need to have *different* semantics for volatile vs non-volatile accesses to an addrspacecast derived pointer. The asc either had semantics, or it didn't. Whether an access is later volatile shouldn't change that.

So, if anything, I'd either lean towards a more-aggressive transform which changed the AS for the volatile access, or not splitting through an addrspacecast at all. The intermediate semantics feel odd.

[arsenm]

In D63401#1547136, @reames wrote:

In D63401#1547122, @arsenm wrote:

In D63401#1547107, @reames wrote:

In D63401#1546293, @sanjoy wrote:

I don't have opinions on this, but Philip might have comments around whether this is okay for GC pointers.

No opinion GC wise, but mostly because I don't understand the intent of this patch. What is the desired outcome here?

For an equivalent bitcast, the type of the underlying alloca is changed into a nicer type and possibly split into multiple allocas (with volatile accesses). I conservatively added checks in r363462 to avoid using the alloca's natural address space for the new operations. I figure it's safer to not change the address space of volatile accesses, but I don't particularly care about this property. InferAddressSpaces avoids changing volatiles, but that was also my idea.

So, just to say this back to you, this would be treating an addrspacecast more like a bitcast when the source is known to be an alloca?

Honestly, this feels more than a bit suspect to me, but I can't argue against it on a principled basis. Semantics for addrspacecast are so ill specified that it's hard to say what is and isn't legal in terms of transforms. Just to be clear, the critical detail here to me is "how do we handle addrspacecast", not "how to do handle volatile". From what I can tell, the existing code for non-volatiles would already have surprising semantics. I don't see any need to have *different* semantics for volatile vs non-volatile accesses to an addrspacecast derived pointer. The asc either had semantics, or it didn't. Whether an access is later volatile shouldn't change that.

So, if anything, I'd either lean towards a more-aggressive transform which changed the AS for the volatile access, or not splitting through an addrspacecast at all. The intermediate semantics feel odd.

In the original patch for addrspacecast support in SROA, the only questions that came up were surrounding overflow behavior which is currently not specified in the LangRef. As for interchanging accesses in different address spaces, I think that is clearly allowed. The original and result pointer still need to access the same memory, so changing the address space or eliminating the access should be OK. I think volatile is the question here because different address spaces may have different caching behavior or something along those lines

[arsenm]

Rebase

[arsenm]

Rebase and ping

[nlopes]

It seems this patch contradicts the proposed LangRef patch. The LangRef patch says that you can't assume that a volatile operation yields the same result in different spaces.
And I think that's a fair condition. Otherwise we need to document that constraint for hardware vendors and change LangRef. Pick one of the patches basically :)

[arsenm]

In D63401#3894885, @nlopes wrote:

It seems this patch contradicts the proposed LangRef patch. The LangRef patch says that you can't assume that a volatile operation yields the same result in different spaces.
And I think that's a fair condition. Otherwise we need to document that constraint for hardware vendors and change LangRef. Pick one of the patches basically :)

This isn't contradictory. This is leaving the address space untouched, and doing the other simplifications SROA does (like replace byte arrays with int). A non-volatile access in this case would have been rewritten to point to the original alloca address space

[nlopes]

In D63401#3894996, @arsenm wrote:

In D63401#3894885, @nlopes wrote:

It seems this patch contradicts the proposed LangRef patch. The LangRef patch says that you can't assume that a volatile operation yields the same result in different spaces.
And I think that's a fair condition. Otherwise we need to document that constraint for hardware vendors and change LangRef. Pick one of the patches basically :)

This isn't contradictory. This is leaving the address space untouched, and doing the other simplifications SROA does (like replace byte arrays with int). A non-volatile access in this case would have been rewritten to point to the original alloca address space

Ah, my bad, I misread the summary.
The patch keeps the AS of the original instructions.

So the concept looks correct to me. But I don't know enough of the SROA's code to know if all transformations that are done are ok. The test coverage doesn't look very extensive.

[nikic]

Looks reasonable. I assume the case where this would be actually useful if an alloca gets split into parts that have volatile and non-volatile accesses, and the non-volatile accesses then get promoted?

llvm/lib/Transforms/Scalar/SROA.cpp

3033

Can't we use getPtrToNewAI here and keep the rest of the code structure?

llvm/test/CodeGen/AMDGPU/flat-address-space.ll

132

Can we use a proper pointer here to avoid making the store UB? (A null pointer is probably fine, given the non-zero address space.)

llvm/test/Transforms/SROA/addrspacecast.ll

177

Why does this load not get promoted (to undef)?

182

Test needs to be converted to opaque pointers. (We should probably add a check to guard against mixed opaque + typed pointer tests, where the typed pointers get promoted.)

[arsenm]

In D63401#3901422, @nikic wrote:

Looks reasonable. I assume the case where this would be actually useful if an alloca gets split into parts that have volatile and non-volatile accesses, and the non-volatile accesses then get promoted?

Yes, but I was mostly noticing the missing cleanup up of the aggregate types

llvm/lib/Transforms/Scalar/SROA.cpp

3033

I'm not sure what you're asking here, the new code does use it below

llvm/test/Transforms/SROA/addrspacecast.ll

177

I don't know, but I see the same thing without the patch and without address spaces or volatiles involved

[arsenm]

Address comments

[arsenm]

ping

[arichardson]

This looks good to me but I feel I'm not familiar enough with SROA to approve the patch.

[lebedev.ri]

Is there sufficient test coverage for this?
Did you check and ensure that this works as intended
on the addressspace-heavy code and does not miscompile it?

[arsenm]

In D63401#3960116, @lebedev.ri wrote:

Is there sufficient test coverage for this?
Did you check and ensure that this works as intended
on the addressspace-heavy code and does not miscompile it?

Every AMDGPU alloca has a non-0 address space, and this passed regular testing at some point (which includes some specific I-want-to-break-the-stack volatile tests). Volatile stack objects aren't super common for GPU code since that kind of defeats the point

[lebedev.ri]

LGTM, thank you.

[arsenm]

27387896cfe898e66f411ff26b8e34698a8f92fc

llvm/lib/Transforms/Scalar/SROA.cpp

private:
 
   void visitLoadInst(LoadInst &LI) {
     assert((!LI.isSimple() || LI.getType()->isSingleValueType()) &&
            "All simple FCA loads should have been pre-split");
 
     if (!IsOffsetKnown)
       return PI.setAborted(&LI);
 
-    if (LI.isVolatile() &&
-        LI.getPointerAddressSpace() != DL.getAllocaAddrSpace())
-      return PI.setAborted(&LI);
-
     if (isa<ScalableVectorType>(LI.getType()))
       return PI.setAborted(&LI);
 
     uint64_t Size = DL.getTypeStoreSize(LI.getType()).getFixedSize();
     return handleLoadOrStore(LI.getType(), LI, Offset, Size, LI.isVolatile());
   }
 
   void visitStoreInst(StoreInst &SI) {
     Value *ValOp = SI.getValueOperand();
     if (ValOp == *U)
       return PI.setEscapedAndAborted(&SI);
     if (!IsOffsetKnown)
       return PI.setAborted(&SI);
 
-    if (SI.isVolatile() &&
-        SI.getPointerAddressSpace() != DL.getAllocaAddrSpace())
-      return PI.setAborted(&SI);
-
     if (isa<ScalableVectorType>(ValOp->getType()))
       return PI.setAborted(&SI);
 
     uint64_t Size = DL.getTypeStoreSize(ValOp->getType()).getFixedSize();
 
     // If this memory access can be shown to *statically* extend outside the
     // bounds of the allocation, it's behavior is undefined, so simply
     // ignore it. Note that this is more strict than the generic clamping
void visitMemSetInst(MemSetInst &II) {
     if ((Length && Length->getValue() == 0) ||
         (IsOffsetKnown && Offset.uge(AllocSize)))
       // Zero-length mem transfer intrinsics can be ignored entirely.
       return markAsDead(II);
 
     if (!IsOffsetKnown)
       return PI.setAborted(&II);
 
-    // Don't replace this with a store with a different address space.  TODO:
-    // Use a store with the casted new alloca?
-    if (II.isVolatile() && II.getDestAddressSpace() != DL.getAllocaAddrSpace())
-      return PI.setAborted(&II);
-
     insertUse(II, Offset, Length ? Length->getLimitedValue()
                                  : AllocSize - Offset.getLimitedValue(),
               (bool)Length);
   }
 
   void visitMemTransferInst(MemTransferInst &II) {
     ConstantInt *Length = dyn_cast<ConstantInt>(II.getLength());
     if (Length && Length->getValue() == 0)
       // Zero-length mem transfer intrinsics can be ignored entirely.
       return markAsDead(II);
 
     // Because we can visit these intrinsics twice, also check to see if the
     // first time marked this instruction as dead. If so, skip it.
     if (VisitedDeadInsts.count(&II))
       return;
 
     if (!IsOffsetKnown)
       return PI.setAborted(&II);
 
-    // Don't replace this with a load/store with a different address space.
-    // TODO: Use a store with the casted new alloca?
-    if (II.isVolatile() &&
-        (II.getDestAddressSpace() != DL.getAllocaAddrSpace() ||
-         II.getSourceAddressSpace() != DL.getAllocaAddrSpace()))
-      return PI.setAborted(&II);
-
     // This side of the transfer is completely out-of-bounds, and so we can
     // nuke the entire transfer. However, we also need to nuke the other side
     // if already added to our partitions.
     // FIXME: Yet another place we really should bypass this when
     // instrumenting for ASan.
     if (Offset.uge(AllocSize)) {
       SmallDenseMap<Instruction *, unsigned>::iterator MTPI =
           MemTransferSliceMap.find(&II);
class llvm::sroa::AllocaSliceRewriter
   // Track post-rewrite users which are PHI nodes and Selects.
   SmallSetVector<PHINode *, 8> &PHIUsers;
   SmallSetVector<SelectInst *, 8> &SelectUsers;
 
   // Utility IR builder, whose name prefix is setup for each visited use, and
   // the insertion point is set to point to the user.
   IRBuilderTy IRB;
 
+  // Return the new alloca, addrspacecasted if required to avoid changing the
+  // addrspace of a volatile access.
+  Value *getPtrToNewAI(unsigned AddrSpace, bool IsVolatile) {
+    if (!IsVolatile || AddrSpace == NewAI.getType()->getPointerAddressSpace())
+      return &NewAI;
+
+    Type *AccessTy = NewAI.getAllocatedType()->getPointerTo(AddrSpace);
+    return IRB.CreateAddrSpaceCast(&NewAI, AccessTy);
+  }
+
 public:
   AllocaSliceRewriter(const DataLayout &DL, AllocaSlices &AS, SROAPass &Pass,
                       AllocaInst &OldAI, AllocaInst &NewAI,
                       uint64_t NewAllocaBeginOffset,
                       uint64_t NewAllocaEndOffset, bool IsIntegerPromotable,
                       VectorType *PromotableVecTy,
                       SmallSetVector<PHINode *, 8> &PHIUsers,
                       SmallSetVector<SelectInst *, 8> &SelectUsers)
if (VecTy) {
       V = rewriteVectorizedLoadInst(LI);
     } else if (IntTy && LI.getType()->isIntegerTy()) {
       V = rewriteIntegerLoad(LI);
     } else if (NewBeginOffset == NewAllocaBeginOffset &&
                NewEndOffset == NewAllocaEndOffset &&
                (canConvertValue(DL, NewAllocaTy, TargetTy) ||
                 (IsLoadPastEnd && NewAllocaTy->isIntegerTy() &&
                  TargetTy->isIntegerTy()))) {
-      LoadInst *NewLI = IRB.CreateAlignedLoad(NewAI.getAllocatedType(), &NewAI,
+      Value *NewPtr =
+          getPtrToNewAI(LI.getPointerAddressSpace(), LI.isVolatile());
+      LoadInst *NewLI = IRB.CreateAlignedLoad(NewAI.getAllocatedType(), NewPtr,
                                               NewAI.getAlign(), LI.isVolatile(),
                                               LI.getName());
       if (AATags)
         NewLI->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
       if (LI.isVolatile())
         NewLI->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
       if (NewLI->isAtomic())
         NewLI->setAlignment(LI.getAlign());
if (NewBeginOffset == NewAllocaBeginOffset &&
           if (VITy->getBitWidth() > AITy->getBitWidth()) {
             if (DL.isBigEndian())
               V = IRB.CreateLShr(V, VITy->getBitWidth() - AITy->getBitWidth(),
                                  "endian_shift");
             V = IRB.CreateTrunc(V, AITy, "load.trunc");
           }
 
       V = convertValue(DL, IRB, V, NewAllocaTy);
+      Value *NewPtr =
+          getPtrToNewAI(SI.getPointerAddressSpace(), SI.isVolatile());
+
       NewSI =
-          IRB.CreateAlignedStore(V, &NewAI, NewAI.getAlign(), SI.isVolatile());
+          IRB.CreateAlignedStore(V, NewPtr, NewAI.getAlign(), SI.isVolatile());
     } else {
       unsigned AS = SI.getPointerAddressSpace();
       Value *NewPtr = getNewAllocaSlicePtr(IRB, V->getType()->getPointerTo(AS));
       NewSI =
           IRB.CreateAlignedStore(V, NewPtr, getSliceAlign(), SI.isVolatile());
     }
     NewSI->copyMetadata(SI, {LLVMContext::MD_mem_parallel_loop_access,
                              LLVMContext::MD_access_group});
if (VecTy) {
                           DL.getTypeSizeInBits(ScalarTy).getFixedSize() / 8);
       if (VectorType *AllocaVecTy = dyn_cast<VectorType>(AllocaTy))
         V = getVectorSplat(
             V, cast<FixedVectorType>(AllocaVecTy)->getNumElements());
 
       V = convertValue(DL, IRB, V, AllocaTy);
     }
 
+    Value *NewPtr = getPtrToNewAI(II.getDestAddressSpace(), II.isVolatile());
     StoreInst *New =
-        IRB.CreateAlignedStore(V, &NewAI, NewAI.getAlign(), II.isVolatile());
+        IRB.CreateAlignedStore(V, NewPtr, NewAI.getAlign(), II.isVolatile());
     New->copyMetadata(II, {LLVMContext::MD_mem_parallel_loop_access,
                            LLVMContext::MD_access_group});
     if (AATags)
       New->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
     LLVM_DEBUG(dbgs() << "          to: " << *New << "\n");
     return !II.isVolatile();
   }
 
if (VecTy && !IsWholeAlloca) {
         OtherTy = FixedVectorType::get(VecTy->getElementType(), NumElements);
     } else if (IntTy && !IsWholeAlloca) {
       OtherTy = SubIntTy;
     } else {
       OtherTy = NewAllocaTy;
     }
     OtherPtrTy = OtherTy->getPointerTo(OtherAS);
 
-    Value *SrcPtr = getAdjustedPtr(IRB, DL, OtherPtr, OtherOffset, OtherPtrTy,
+    Value *AdjPtr = getAdjustedPtr(IRB, DL, OtherPtr, OtherOffset, OtherPtrTy,
                                    OtherPtr->getName() + ".");
     MaybeAlign SrcAlign = OtherAlign;
-    Value *DstPtr = &NewAI;
     MaybeAlign DstAlign = SliceAlign;
-    if (!IsDest) {
-      std::swap(SrcPtr, DstPtr);
+    if (!IsDest)
       std::swap(SrcAlign, DstAlign);
+
+    Value *SrcPtr;
+    Value *DstPtr;
+
+    if (IsDest) {
+      DstPtr = getPtrToNewAI(II.getDestAddressSpace(), II.isVolatile());
+      SrcPtr = AdjPtr;
+    } else {
+      DstPtr = AdjPtr;
+      SrcPtr = getPtrToNewAI(II.getSourceAddressSpace(), II.isVolatile());
     }
 
     Value *Src;
     if (VecTy && !IsWholeAlloca && !IsDest) {
       Src = IRB.CreateAlignedLoad(NewAI.getAllocatedType(), &NewAI,
                                   NewAI.getAlign(), "load");
       Src = extractVector(IRB, Src, BeginIndex, EndIndex, "vec");
     } else if (IntTy && !IsWholeAlloca && !IsDest) {

 ///
 /// We also record the alloca instructions deleted here so that they aren't
 /// subsequently handed to mem2reg to promote.
 bool SROAPass::deleteDeadInstructions(
     SmallPtrSetImpl<AllocaInst *> &DeletedAllocas) {
   bool Changed = false;
   while (!DeadInsts.empty()) {
     Instruction *I = dyn_cast_or_null<Instruction>(DeadInsts.pop_back_val());
-    if (!I) continue; 
+    if (!I)
+      continue;
     LLVM_DEBUG(dbgs() << "Deleting dead instruction: " << *I << "\n");
 
     // If the instruction is an alloca, find the possible dbg.declare connected
     // to it, and remove it too. We must do this before calling RAUW or we will
     // not be able to find it.
     if (AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
       DeletedAllocas.insert(AI);
       for (DbgVariableIntrinsic *OldDII : FindDbgAddrUses(AI))

llvm/test/CodeGen/AMDGPU/flat-address-space.ll

 ; CHECK-LABEL: flat_scratch_unaligned_load:
 ; CHECK: flat_load_ubyte
 ; CHECK: flat_load_ubyte
 ; CHECK: flat_load_ubyte
 ; CHECK: flat_load_ubyte
 define amdgpu_kernel void @flat_scratch_unaligned_load() {
   %scratch = alloca i32, addrspace(5)
   %fptr = addrspacecast i32 addrspace(5)* %scratch to i32*
+  store volatile i32* %fptr, i32* addrspace(3)* null
   %ld = load volatile i32, i32* %fptr, align 1
   ret void
 }
 
 ; CHECK-LABEL: flat_scratch_unaligned_store:
 ; CHECK: flat_store_byte
 ; CHECK: flat_store_byte
 ; CHECK: flat_store_byte
 ; CHECK: flat_store_byte
 define amdgpu_kernel void @flat_scratch_unaligned_store() {
   %scratch = alloca i32, addrspace(5)
   %fptr = addrspacecast i32 addrspace(5)* %scratch to i32*
+  store volatile i32* %fptr, i32* addrspace(3)* null
   store volatile i32 0, i32* %fptr, align 1
   ret void
 }
 
 ; CHECK-LABEL: flat_scratch_multidword_load:
 ; CIVI-HSA: flat_load_dword v
 ; CIVI-HSA: flat_load_dword v
 ; GFX9:  flat_load_dwordx2

llvm/test/Transforms/SROA/addrspacecast.ll

entry:
   %val = load i64, ptr %a
   ret i64 %val
 }
 
 ; Don't change the address space of a volatile operation
 define i64 @alloca_addrspacecast_bitcast_volatile_store(i64 %X) {
 ; CHECK-LABEL: @alloca_addrspacecast_bitcast_volatile_store(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca [8 x i8], align 1
-; CHECK-NEXT:    [[A_CAST:%.*]] = addrspacecast ptr [[A]] to ptr addrspace(1)
-; CHECK-NEXT:    store volatile i64 [[X:%.*]], ptr addrspace(1) [[A_CAST]], align 4
-; CHECK-NEXT:    [[Z:%.*]] = load i64, ptr addrspace(1) [[A_CAST]], align 4
-; CHECK-NEXT:    ret i64 [[Z]]
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca i64, align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = addrspacecast ptr [[A_SROA_0]] to ptr addrspace(1)
+; CHECK-NEXT:    store volatile i64 [[X:%.*]], ptr addrspace(1) [[TMP0]], align 8
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_0_Z:%.*]] = load i64, ptr [[A_SROA_0]], align 8
+; CHECK-NEXT:    ret i64 [[A_SROA_0_0_A_SROA_0_0_Z]]
 ;
 entry:
   %A = alloca [8 x i8]
   %A.cast = addrspacecast ptr %A to ptr addrspace(1)
   store volatile i64 %X, ptr addrspace(1) %A.cast
   %Z = load i64, ptr addrspace(1) %A.cast
   ret i64 %Z
 }
 
+%struct = type { [256 x i8], i32 }
+
+define i65 @volatile_store_addrspacecast_slice(i65 %X, i16 %idx) {
+; CHECK-LABEL: @volatile_store_addrspacecast_slice(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca [9 x i8], align 4
+; CHECK-NEXT:    [[A_SROA_1:%.*]] = alloca [9 x i8], align 8
+; CHECK-NEXT:    [[A_SROA_1_0_GEPB_SROA_CAST:%.*]] = addrspacecast ptr [[A_SROA_1]] to ptr addrspace(1)
+; CHECK-NEXT:    store volatile i65 [[X:%.*]], ptr addrspace(1) [[A_SROA_1_0_GEPB_SROA_CAST]], align 8
+; CHECK-NEXT:    br label [[L2:%.*]]
+; CHECK:       L2:
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_20_Z:%.*]] = load i65, ptr [[A_SROA_0]], align 4
+; CHECK-NEXT:    ret i65 [[A_SROA_0_0_A_SROA_0_20_Z]]
+;
+entry:
+  %A = alloca %struct
+  %B = addrspacecast ptr %A to ptr addrspace(1)
+  %gepA = getelementptr %struct, ptr %A, i32 0, i32 0, i16 20
+  %gepB = getelementptr i65, ptr addrspace(1) %B, i16 6
+  store volatile i65 %X, ptr addrspace(1) %gepB, align 1
+  br label %L2
+
+L2:
+  %Z = load i65, ptr %gepA, align 1
+  ret i65 %Z
+}
+
 ; Don't change the address space of a volatile operation
 define i64 @alloca_addrspacecast_bitcast_volatile_load(i64 %X) {
 ; CHECK-LABEL: @alloca_addrspacecast_bitcast_volatile_load(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca [8 x i8], align 1
-; CHECK-NEXT:    [[A_CAST:%.*]] = addrspacecast ptr [[A]] to ptr addrspace(1)
-; CHECK-NEXT:    store i64 [[X:%.*]], ptr addrspace(1) [[A_CAST]], align 4
-; CHECK-NEXT:    [[Z:%.*]] = load volatile i64, ptr addrspace(1) [[A_CAST]], align 4
-; CHECK-NEXT:    ret i64 [[Z]]
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca i64, align 8
+; CHECK-NEXT:    store i64 [[X:%.*]], ptr [[A_SROA_0]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = addrspacecast ptr [[A_SROA_0]] to ptr addrspace(1)
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_0_Z:%.*]] = load volatile i64, ptr addrspace(1) [[TMP0]], align 8
+; CHECK-NEXT:    ret i64 [[A_SROA_0_0_A_SROA_0_0_Z]]
 ;
 entry:
   %A = alloca [8 x i8]
   %A.cast = addrspacecast ptr %A to ptr addrspace(1)
   store i64 %X, ptr addrspace(1) %A.cast
   %Z = load volatile i64, ptr addrspace(1) %A.cast
   ret i64 %Z
 }
 
 declare void @llvm.memset.p1.i32(ptr addrspace(1) nocapture, i8, i32, i1) nounwind
 
+define i65 @volatile_load_addrspacecast_slice(i65 %X, i16 %idx) {
+; CHECK-LABEL: @volatile_load_addrspacecast_slice(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca [9 x i8], align 4
+; CHECK-NEXT:    [[A_SROA_1:%.*]] = alloca [9 x i8], align 8
+; CHECK-NEXT:    [[A_SROA_1_0_GEPB_SROA_CAST:%.*]] = addrspacecast ptr [[A_SROA_1]] to ptr addrspace(1)
+; CHECK-NEXT:    store i65 [[X:%.*]], ptr addrspace(1) [[A_SROA_1_0_GEPB_SROA_CAST]], align 8
+; CHECK-NEXT:    br label [[L2:%.*]]
+; CHECK:       L2:
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_20_Z:%.*]] = load volatile i65, ptr [[A_SROA_0]], align 4
+; CHECK-NEXT:    ret i65 [[A_SROA_0_0_A_SROA_0_20_Z]]
+;
+entry:
+  %A = alloca %struct
+  %B = addrspacecast ptr %A to ptr addrspace(1)
+  %gepA = getelementptr %struct, ptr %A, i32 0, i32 0, i16 20
+  %gepB = getelementptr i65, ptr addrspace(1) %B, i16 6
+  store i65 %X, ptr addrspace(1) %gepB, align 1
+  br label %L2
+
+L2:
+  %Z = load volatile i65, ptr %gepA, align 1
+  ret i65 %Z
+}
+
 ; Don't change the address space of a volatile operation
 define i32 @volatile_memset() {
 ; CHECK-LABEL: @volatile_memset(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT:    [[ASC:%.*]] = addrspacecast ptr [[A]] to ptr addrspace(1)
-; CHECK-NEXT:    call void @llvm.memset.p1.i32(ptr addrspace(1) [[ASC]], i8 42, i32 4, i1 true)
-; CHECK-NEXT:    [[VAL:%.*]] = load i32, ptr [[A]], align 4
-; CHECK-NEXT:    ret i32 [[VAL]]
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[TMP0:%.*]] = addrspacecast ptr [[A_SROA_0]] to ptr addrspace(1)
+; CHECK-NEXT:    store volatile i32 707406378, ptr addrspace(1) [[TMP0]], align 4
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_0_VAL:%.*]] = load i32, ptr [[A_SROA_0]], align 4
+; CHECK-NEXT:    ret i32 [[A_SROA_0_0_A_SROA_0_0_VAL]]
 ;
 entry:
   %a = alloca [4 x i8]
   %asc = addrspacecast ptr %a to ptr addrspace(1)
   call void @llvm.memset.p1.i32(ptr addrspace(1) %asc, i8 42, i32 4, i1 true)
   %val = load i32, ptr %a
   ret i32 %val
 }
 
 ; Don't change the address space of a volatile operation
 define void @volatile_memcpy(ptr %src, ptr %dst) {
 ; CHECK-LABEL: @volatile_memcpy(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca [4 x i8], align 1
-; CHECK-NEXT:    [[ASC:%.*]] = addrspacecast ptr [[A]] to ptr addrspace(1)
-; CHECK-NEXT:    call void @llvm.memcpy.p1.p0.i32(ptr addrspace(1) [[ASC]], ptr [[SRC:%.*]], i32 4, i1 true), !tbaa [[TBAA0:![0-9]+]]
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p1.i32(ptr [[DST:%.*]], ptr addrspace(1) [[ASC]], i32 4, i1 true), !tbaa [[TBAA3:![0-9]+]]
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[A_SROA_0_0_COPYLOAD:%.*]] = load volatile i32, ptr [[SRC:%.*]], align 1, !tbaa [[TBAA0:![0-9]+]]
+; CHECK-NEXT:    [[TMP0:%.*]] = addrspacecast ptr [[A_SROA_0]] to ptr addrspace(1)
+; CHECK-NEXT:    store volatile i32 [[A_SROA_0_0_COPYLOAD]], ptr addrspace(1) [[TMP0]], align 4, !tbaa [[TBAA0]]
+; CHECK-NEXT:    [[TMP1:%.*]] = addrspacecast ptr [[A_SROA_0]] to ptr addrspace(1)
+; CHECK-NEXT:    [[A_SROA_0_0_A_SROA_0_0_COPYLOAD1:%.*]] = load volatile i32, ptr addrspace(1) [[TMP1]], align 4, !tbaa [[TBAA3:![0-9]+]]
+; CHECK-NEXT:    store volatile i32 [[A_SROA_0_0_A_SROA_0_0_COPYLOAD1]], ptr [[DST:%.*]], align 1, !tbaa [[TBAA3]]
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %a = alloca [4 x i8]
   %asc = addrspacecast ptr %a to ptr addrspace(1)
   call void @llvm.memcpy.p1.p0.i32(ptr addrspace(1) %asc, ptr %src, i32 4, i1 true), !tbaa !0
   call void @llvm.memcpy.p0.p1.i32(ptr %dst, ptr addrspace(1) %asc, i32 4, i1 true), !tbaa !3
   ret void

llvm/test/Transforms/SROA/basictest.ll

entry:
   call void @llvm.memcpy.p0.p0.i32(ptr align 8 %gep, ptr align 8 %a, i32 16, i1 true)
   ret void
 }
 
 define void @PR14105_as1(ptr addrspace(1) %ptr) {
 ; Make sure this the right address space pointer is used for type check.
 ; CHECK-LABEL: @PR14105_as1(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[A:%.*]] = alloca { [16 x i8] }, align 8
+; CHECK-NEXT:    [[A_SROA_0:%.*]] = alloca [16 x i8], align 8
 ; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds { [16 x i8] }, ptr addrspace(1) [[PTR:%.*]], i64 -1
-; CHECK-NEXT:    call void @llvm.memcpy.p1.p0.i32(ptr addrspace(1) align 8 [[GEP]], ptr align 8 [[A]], i32 16, i1 true)
+; CHECK-NEXT:    call void @llvm.memcpy.p1.p0.i32(ptr addrspace(1) align 8 [[GEP]], ptr align 8 [[A_SROA_0]], i32 16, i1 true)
 ; CHECK-NEXT:    ret void
 ;
 
 entry:
   %a = alloca { [16 x i8] }, align 8
   %gep = getelementptr inbounds { [16 x i8] }, ptr addrspace(1) %ptr, i64 -1
   call void @llvm.memcpy.p1.p0.i32(ptr addrspace(1) align 8 %gep, ptr align 8 %a, i32 16, i1 true)
   ret void

 
 %struct.STest = type { %struct.SPos, %struct.SPos }
 %struct.SPos = type { float, float }
 
 define void @PR25873(ptr %outData) {
 ; CHECK-LABEL: @PR25873(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    store i32 1123418112, ptr [[OUTDATA:%.*]], align 4
-; CHECK-NEXT:    [[DOTSROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[OUTDATA:%.*]], i64 4
-; CHECK-NEXT:    store i32 1139015680, ptr [[DOTSROA_IDX]], align 4
-; CHECK-NEXT:    [[TMPDATA_SROA_6_0__SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[OUTDATA:%.*]], i64 8
+; CHECK-NEXT:    [[OUTDATA_SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[OUTDATA]], i64 4
+; CHECK-NEXT:    store i32 1139015680, ptr [[OUTDATA_SROA_IDX]], align 4
+; CHECK-NEXT:    [[TMPDATA_SROA_6_0_OUTDATA_SROA_IDX:%.*]] = getelementptr inbounds i8, ptr [[OUTDATA]], i64 8
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_4_0_INSERT_EXT:%.*]] = zext i32 1139015680 to i64
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_4_0_INSERT_SHIFT:%.*]] = shl i64 [[TMPDATA_SROA_6_SROA_4_0_INSERT_EXT]], 32
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_4_0_INSERT_MASK:%.*]] = and i64 undef, 4294967295
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_4_0_INSERT_INSERT:%.*]] = or i64 [[TMPDATA_SROA_6_SROA_4_0_INSERT_MASK]], [[TMPDATA_SROA_6_SROA_4_0_INSERT_SHIFT]]
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_0_0_INSERT_EXT:%.*]] = zext i32 1123418112 to i64
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_0_0_INSERT_MASK:%.*]] = and i64 [[TMPDATA_SROA_6_SROA_4_0_INSERT_INSERT]], -4294967296
 ; CHECK-NEXT:    [[TMPDATA_SROA_6_SROA_0_0_INSERT_INSERT:%.*]] = or i64 [[TMPDATA_SROA_6_SROA_0_0_INSERT_MASK]], [[TMPDATA_SROA_6_SROA_0_0_INSERT_EXT]]
-; CHECK-NEXT:    store i64 [[TMPDATA_SROA_6_SROA_0_0_INSERT_INSERT]], ptr [[TMPDATA_SROA_6_0__SROA_IDX]], align 4
+; CHECK-NEXT:    store i64 [[TMPDATA_SROA_6_SROA_0_0_INSERT_INSERT]], ptr [[TMPDATA_SROA_6_0_OUTDATA_SROA_IDX]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %tmpData = alloca %struct.STest, align 8
   call void @llvm.lifetime.start.p0(i64 16, ptr %tmpData)
   store float 1.230000e+02, ptr %tmpData, align 8
   %y = getelementptr inbounds %struct.STest, ptr %tmpData, i64 0, i32 0, i32 1
   store float 4.560000e+02, ptr %y, align 4