[PATCH] D63401: SROA: Simplify addrspacecasted allocas with volatile accesses

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

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?



================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:3033
     MaybeAlign SrcAlign = OtherAlign;
-    Value *DstPtr = &NewAI;
     MaybeAlign DstAlign = SliceAlign;
----------------
Can't we use getPtrToNewAI here and keep the rest of the code structure?


================
Comment at: llvm/test/CodeGen/AMDGPU/flat-address-space.ll:132
   %fptr = addrspacecast i32 addrspace(5)* %scratch to i32*
+  store volatile i32* %fptr, i32* addrspace(3)* undef
   %ld = load volatile i32, i32* %fptr, align 1
----------------
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.)


================
Comment at: llvm/test/Transforms/SROA/addrspacecast.ll:177
+; 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]]
----------------
Why does this load not get promoted (to undef)?


================
Comment at: llvm/test/Transforms/SROA/addrspacecast.ll:182
+  %A = alloca %struct
+  %B = addrspacecast %struct* %A to i65 addrspace(1)*
+  %gepA = getelementptr %struct, %struct* %A, i32 0, i32 0, i16 20
----------------
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.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63401/new/

https://reviews.llvm.org/D63401