[Openmp-commits] [PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Matt Arsenault via Phabricator via Openmp-commits]

arsenm added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17067
+
+    Value *Iscov5 = CGF.Builder.CreateICmpSGE(
+        ABIVersion,
----------------
Capitalization is weird, IsCOV5?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17082-17083
+    Value *DispatchPtr = EmitAMDGPUDispatchPtr(CGF);
+    auto *DispatchGEP =
+        CGF.Builder.CreateGEP(CGF.Int8Ty, DispatchPtr, DispatchOffset);
+
----------------
CreateConstInBoundsGEP1_64


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:17100
+    }
+    auto *GEP = CGF.Builder.CreateGEP(CGF.Int8Ty, ArgPtr, Offset);
+    LD = CGF.Builder.CreateLoad(
----------------
CreateConstInBoundsGEP1_64


================
Comment at: clang/lib/CodeGen/Targets/AMDGPU.cpp:364
+    CodeGen::CodeGenModule &CGM) const {
+  auto AddGlobal = [&](StringRef Name,
+                       clang::TargetOptions::CodeObjectVersionKind Value,
----------------
Single use lamdba, just make this the function body


================
Comment at: clang/lib/CodeGen/Targets/AMDGPU.cpp:381
+    GV->setVisibility(llvm::GlobalValue::VisibilityTypes::HiddenVisibility);
+    GV->setAlignment(CGM.getDataLayout().getABITypeAlign(Type));
+  };
----------------
No real point setting the alignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730