Preserve the address space for llvm.used and llvm.compiler.used global variables in GlobalOpt pass.

[nawalcopty]

Summary

The llvm.used (or llvm.compiler.used) global variable is an array that contains a list of pointers to global variables and functions.

The GlobalOpt (Global Variable Optimizer) pass is not preserving the address space for llvm.used and llvm.compiler.used global variables.This patch updates the setUsedInitializer() function in GlobalOpt.cpp, so the address space is preserved.

[arichardson]

Thanks for fixing a hardcoded AS 0! Code change looks correct to me but the test case can be reduced further.

llvm/lib/Transforms/IPO/GlobalOpt.cpp

2123

Would the IR have passed the verifier if these checks don't hold? If not we could use cast instead.

llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll

5

Not needed

18

This looks exactly the same as before so the test would also pass without globalopt. Could you add another entry so that the sorting logic in the modified function changes the output?

43

(almost?) all of these lines should not be required for the test.

[arsenm]

llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll

8

Don't need triple

21

Don't need all the metadata or comdat or linkage

23

use named values

[nawalcopty]

Made changes as indicated by reviewers.

[nawalcopty]

Fixed up the diff to include all changes (including the ones in the previous diff). Thanks.

[arsenm]

llvm/lib/Transforms/IPO/GlobalOpt.cpp

2122–2123

Don't need the if to check a cast<>

[nawalcopty]

Removed the if when using cast<>. Thanks.

[nawalcopty]

Could someone please review this patch? I think this patch is ready for approval. Thanks!

[aeubanks]

lg with nit

llvm/lib/Transforms/IPO/GlobalOpt.cpp

2124

just inline this below instead of having a ElemAddrSpace variable

[nawalcopty]

Removed unnecessary ElemAddrSpace variable, as @aeubanks suggested.

[aeubanks]

do you need me to land this for you? what name/email should I use?

[nawalcopty]

Hi @aeubanks. Yes, it would be great if you would land this for me. Please use the following name/email: "Nawal Copty <[e-mail]>"

Thank you!

llvm/lib/Transforms/IPO/GlobalOpt.cpp

if (Init.empty()) {
     V.eraseFromParent();
     return;
   }
 
   // Get address space of pointers in the array of pointers.
   unsigned ElemAddrSpace = 0;
   const Type *UsedArrayType = V.getValueType();
 
-  if (const auto *VAT = dyn_cast<ArrayType>(UsedArrayType))
-    if (const auto *VEPT = dyn_cast<PointerType>(VAT->getArrayElementType()))
+  if (const auto *VAT = cast<ArrayType>(UsedArrayType))
+    if (const auto *VEPT = cast<PointerType>(VAT->getArrayElementType()))
       ElemAddrSpace = VEPT->getAddressSpace();
 
   // Type of pointer to the array of pointers.
   PointerType *Int8PtrTy = Type::getInt8PtrTy(V.getContext(), ElemAddrSpace);
 
   SmallVector<Constant *, 8> UsedArray;
   for (GlobalValue *GV : Init) {
     Constant *Cast
       = ConstantExpr::getPointerBitCastOrAddrSpaceCast(GV, Int8PtrTy);
     UsedArray.push_back(Cast);
   }
+
   // Sort to get deterministic order.
   array_pod_sort(UsedArray.begin(), UsedArray.end(), compareNames);
   ArrayType *ATy = ArrayType::get(Int8PtrTy, UsedArray.size());
 
   Module *M = V.getParent();
   V.removeFromParent();
   GlobalVariable *NV =
       new GlobalVariable(*M, ATy, false, GlobalValue::AppendingLinkage,

llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll

 ; RUN: opt -S -passes=globalopt < %s | FileCheck %s
 ;; Check that Global Opt preserves address space of llvm.used and
 ;; llvm.compiler.used variables.
 
-; ModuleID = 'global-opt-addrspace.ll'
-source_filename = "device_global_internal_failure.cpp"
-target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
-target triple = "spir64-unknown-unknown"
-
 %struct.FakeDeviceGlobal = type { ptr addrspace(4) }
 %class.anon = type { i8 }
 
-$_ZTSZ4mainEUlvE_ = comdat any
+@_ZM2C = internal addrspace(1) global %struct.FakeDeviceGlobal zeroinitializer, align 8
+@_ZL1C = internal addrspace(1) global %struct.FakeDeviceGlobal zeroinitializer, align 8
 
-@_ZL1C = internal addrspace(1) global %struct.FakeDeviceGlobal zeroinitializer, align 8 #0
[e-mail] = appending global [1 x ptr addrspace(4)] [ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4))], section "llvm.metadata"
[e-mail] = appending global [2 x ptr addrspace(4)] [ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZM2C to ptr addrspace(4)), ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4))]
 
-; CHECK: @llvm.compiler.used = appending global [1 x ptr addrspace(4)] [ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4))], section "llvm.metadata"
+; CHECK: @llvm.compiler.used = appending global [2 x ptr addrspace(4)] [ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4)), ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZM2C to ptr addrspace(4))]
 
-; Function Attrs: convergent mustprogress noinline norecurse optnone
-define weak_odr dso_local spir_kernel void @_ZTSZ4mainEUlvE_() #1 comdat !kernel_arg_buffer_location !5 {
+define weak_odr dso_local void @foo() {
 entry:
-  %0 = alloca %class.anon, align 1
-  %1 = addrspacecast ptr %0 to ptr addrspace(4)
-  call spir_func void @_ZZ4mainENKUlvE_clEv(ptr addrspace(4) noundef align 1 dereferenceable_or_null(1) %1) #3
+  %A = alloca %class.anon, align 1
+  %A.addrspacecast = addrspacecast ptr %A to ptr addrspace(4)
+  call void @bar(ptr addrspace(4) noundef align 1 dereferenceable_or_null(1) %A.addrspacecast)
   ret void
 }
 
-; Function Attrs: convergent mustprogress noinline norecurse nounwind optnone
-define internal spir_func void @_ZZ4mainENKUlvE_clEv(ptr addrspace(4) noundef align 1 dereferenceable_or_null(1) %this) #2 align 2 {
+define internal void @bar(ptr addrspace(4) noundef align 1 dereferenceable_or_null(1) %this) align 2 {
 entry:
   %this.addr = alloca ptr addrspace(4), align 8
   %this.addr.ascast = addrspacecast ptr %this.addr to ptr addrspace(4)
   store ptr addrspace(4) %this, ptr addrspace(4) %this.addr.ascast, align 8
-  %this1 = load ptr addrspace(4), ptr addrspace(4) %this.addr.ascast, align 8
-  %0 = load ptr addrspace(4), ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4)), align 8
-  store i32 42, ptr addrspace(4) %0, align 4
+  %v1 = load ptr addrspace(4), ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZM2C to ptr addrspace(4)), align 8
+  store i32 42, ptr addrspace(4) %v1, align 4
+  %v2 = load ptr addrspace(4), ptr addrspace(4) addrspacecast (ptr addrspace(1) @_ZL1C to ptr addrspace(4)), align 8
+  store i32 42, ptr addrspace(4) %v2, align 4
   ret void
 }
 
-declare dso_local spir_func i32 @_Z18__spirv_ocl_printfPU3AS2Kcz(ptr addrspace(2), ...)
-
-attributes #0 = { "sycl-unique-id"="c5b4ae0b52852573____ZL1C" }
-attributes #1 = { convergent mustprogress noinline norecurse optnone "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "sycl-module-id"="device_global_internal_failure.cpp" "uniform-work-group-size"="true" }
-attributes #2 = { convergent mustprogress noinline norecurse nounwind optnone "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-attributes #3 = { convergent }
-
-!llvm.module.flags = !{!0, !1}
-!opencl.spir.version = !{!2}
-!spirv.Source = !{!3}
-!llvm.ident = !{!4}
-
-!0 = !{i32 1, !"wchar_size", i32 4}
-!1 = !{i32 7, !"frame-pointer", i32 2}
-!2 = !{i32 1, i32 2}
-!3 = !{i32 4, i32 100000}
-!4 = !{!"clang version 15.0.0 (https://github.com/intel/llvm.git 537e51b08b74438ed64eb7de6e254922e8ddeaa0)"}
-!5 = !{}