jdoerfert added inline comments.

================
Comment at: clang/test/Frontend/embed-object.ll:6
+; CHECK: @[[OBJECT:.+]] = private constant [120 x i8] 
c"\10\FF\10\AD{{.*}}\00", section ".llvm.offloading", align 8
+; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* 
getelementptr inbounds ([120 x i8], [120 x i8]* @[[OBJECT]], i32 0, i32 0)], 
section "llvm.metadata"
 
----------------
Can we salvage the test intention and embed two objects?


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:311
+
+      assert(Binary.getVersion() == 1 && "Incompatible device image version");
+
----------------
This should be a proper conditional and error check.


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:316
+      StringRef Suffix =
+          getImageKindName(static_cast<ImageKind>(Binary.getImageKind()));
 
----------------
Why is the cast at the call site not in the callee? Are these casts even 
necessary at all?


================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:469
+      Offset += Binary.getSize();
+    }
     ToBeDeleted.push_back(&GV);
----------------
Isn't this 99% the same as above, from `offset = 0` to the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122683

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to