[PATCH] D144518: Preserve the address space for llvm.used and llvm.compiler.used global variables in GlobalOpt pass.

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added a comment.

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



================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2150
+  if (const auto *VAT = dyn_cast<ArrayType>(UsedArrayType))
+    if (const auto *VEPT = dyn_cast<PointerType>(VAT->getArrayElementType()))
+      ElemAddrSpace = VEPT->getAddressSpace();
----------------
Would the IR have passed the verifier if these checks don't hold? If not we could use `cast` instead.


================
Comment at: llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll:5
+
+; ModuleID = 'global-opt-addrspace.ll'
+source_filename = "device_global_internal_failure.cpp"
----------------
Not needed


================
Comment at: llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll:18
+
+; 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"
+
----------------
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?


================
Comment at: llvm/test/Transforms/GlobalOpt/global-opt-addrspace.ll:43
+
+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" }
----------------
(almost?) all of these lines should not be required for the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144518