svenvh requested changes to this revision.
svenvh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Driver/Driver.cpp:3728
 
+  // Linking separate translation units for SPIR-V is not supported yet.
+  // It can be done either by LLVM IR linking before conversion of the final
----------------
FIXME? (assuming this is something we want to address eventually)


================
Comment at: clang/test/Driver/spirv-toolchain.cl:10
+// SPV64-SAME: "-o" [[BC:".*bc"]]
+// SPV64: {{".*llvm-spirv.*"}} [[BC]] "-o" {{".*o"}}
+
----------------
Any reason to not just check for `llvm-spirv{{.*}}`, for consistency with the 
clang check above?


================
Comment at: clang/test/Misc/warning-flags.c:21
 
-CHECK: Warnings without flags (67):
+CHECK: Warnings without flags (68):
 
----------------
The comment above says: "The list of warnings below should NEVER grow.", and 
the current patch violates that.  You'll need to add a warning group to the new 
warning.


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

https://reviews.llvm.org/D112410

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

Reply via email to