yaxunl added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6949-6952
+    CmdArgs.push_back(Args.MakeArgString(
+        "-fembed-offload-object=" + File + "," +
+        Action::GetOffloadKindName(OffloadAction->getOffloadingDeviceKind()) +
+        "," + TC->getTripleString() + "," + Arch));
----------------
jhuber6 wrote:
> yaxunl wrote:
> > Linux path may contain ',' which cannot be passed by this option.
> > 
> > We've seen similar issues with -inputs= option of clang-offload-bundler. To 
> > solve that issue, we had to add support of multiple -input= options.
> Ugh, you could probably split this starting from the end since we assume it 
> will always end with at least three commas, but that's not a satisfying 
> solution. I've been pondering creating a new tool to do this, but haven't 
> quite decided how that should look. I'd imagine it would basically behave 
> like how we invoke `fatbinary`.
> 
> This issue isn't related to this patch, so I'll probably just parse it in 
> reverse upstream, thanks for bringing this up.
I agree the issue is orthogonal to this patch.

Parsing backward is an option. Another option is to put file name at the end of 
the option. Maybe the latter is less confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123313

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

Reply via email to