[PATCH] D123313: [OpenMP] Make clang argument handling for the new driver more generic

Tue Apr 19 10:20:42 PDT 2022

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