[PATCH] D150675: [AMDGPU] Rewrite device ctor / dtor handling to use .init / .fini sections

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:73
+        return new GlobalVariable(
+            M, ArrayType::get(IRB.getPtrTy(1), 0),
+            /*isConstant=*/true, GlobalValue::ExternalLinkage,
----------------
Use the enum or query from datalayout 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:78
+            /*InsertBefore=*/nullptr, GlobalVariable::NotThreadLocal,
+            /*AddressSpace=*/1);
+      });
----------------
Use the enum or query from datalayout 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:82
+      IsCtor ? "__init_array_end" : "__fini_array_end",
+      ArrayType::get(IRB.getPtrTy(1), 0), [&]() {
+        return new GlobalVariable(
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:84
+        return new GlobalVariable(
+            M, ArrayType::get(IRB.getPtrTy(1), 0),
+            /*isConstant=*/true, GlobalValue::ExternalLinkage,
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:89
+            /*InsertBefore=*/nullptr, GlobalVariable::NotThreadLocal,
+            /*AddressSpace=*/1);
+      });
----------------
Ditto


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:99
+  auto *CallBackPHI = IRB.CreatePHI(IRB.getPtrTy(1), 2, "ptr");
+  auto *CallBack = IRB.CreateLoad(PointerType::getUnqual(CallBackTy),
+                                  CallBackPHI, "callback");
----------------
I don't like getUnqual, it's a bad name that implies they're 0="unqualified" which is not really how things work. Query the function's address space 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:103
+  auto *NewCallBack =
+      IRB.CreateInBoundsGEP(IRB.getPtrTy(1), CallBackPHI,
+                            ConstantInt::get(IRB.getInt64Ty(), 1), "next");
----------------
Should just save the ptr type to a variable and reuse throughout 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150675