[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

Wed Apr 26 14:09:09 PDT 2023

dblaikie added inline comments.

================
Comment at: clang/lib/Driver/Driver.cpp:3884
+          nullptr, getOpts().getOption(options::OPT_dumpdir),
+          Args.MakeArgString(Args.getLastArgValue(options::OPT_o, "a") + "-"));
+      Arg->claim();
----------------
would be nice to have this "a" derive from wherever we hardcode "a.out" as the default output rather than independently hardcoded here?

& what does GCC do when the `-o` value has a `.` in it? (if you use `-o a.out` do you get the same `a-x.dwo` behavior, or do you get `a.out-x.dwo`?)

================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1261
-    Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
-                             options::OPT_fdebug_compilation_dir_EQ);
-    SmallString<128> T(A ? A->getValue() : "");
----------------
I was going to suggest we shouldn't remove this functionality without checking who implemented/whether they rely on it (despite it being untested)

Looks like it went in in some of the earliest patches from Google for Split DWARF ( 248357f62418831c7be9d43d96860b52aae1ef56 ).

We do use `-fdebug-compilation-dir` at Google, but I think we just set it to `.` anyway, so for that use case it'd be a no-op anyway, I think? Might want to test this patch internally just to be sure, though.

Though perhaps we never hit this path, because we'll always be building with `-o` and `-c` which should hit the `if` above?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193