[PATCH] D52296: [Clang] - Add -fdwarf-fission=split,single option.

[George Rimar via Phabricator via cfe-commits]

grimar added inline comments.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:5889
   const llvm::Triple &T = getToolChain().getTriple();
-  if (Args.hasArg(options::OPT_gsplit_dwarf) &&
+  if ((getDebugFissionKind(D, Args) == DwarfFissionKind::Split) &&
       (T.isOSLinux() || T.isOSFuchsia())) {
----------------
grimar wrote:
> grimar wrote:
> > probinson wrote:
> > > grimar wrote:
> > > > dblaikie wrote:
> > > > > Could having more than one call to getDebugFissionKind lead to the error message (for -fdwarf-fission=garbage) being printed multiple times? Or is this call on a distinct/mutually exclusive codepath to the other?
> > > > I think it should not be possible. Currently, there are 2 inclusions of the `getDebugFissionKind `. One is used for construction clang job,
> > > > and one is here, it is used to construct an assembler tool job. I do not see a way how it can be printed multiple times.
> > > > 
> > > > For example, the following invocation will print it only once:
> > > > 
> > > > ```
> > > > clang main.c -o out.s -S -fdwarf-fission=foo
> > > > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > > > ```
> > > The example you give here will not create an assembler job. Try
> > > `clang main.c -fdwarf-fission=foo -fno-integrated-as` which I think will exercise the path David is asking about.
> > Oh, I did not know.
> > 
> > Seems the important option to test is `-save-temps`. Seems I have the same result with `-fno-integrated-as`/`-fintegrated-as`,
> > but with/without `-save-temps` I got:
> > 
> > ```
> > clang main.cpp -fdwarf-fission=foo -o 1.o
> > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > 
> > clang main.cpp -fdwarf-fission=foo -o 1.o -save-temps
> > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
> > ```
> > 
> > Will fix, thanks!
> > 
> > (The behavior seems weird to me. I would not expect tool will be creating any jobs should happen when the command line is invalid from the start. I am not very familiar with the clang code/logic though to say it wrong).
> "I would not expect tool will be creating any jobs should happen" -> "I would not expect tool will be creating any jobs when ..."
And returning to this my example, indeed it should not have to create assembler job.
My mistake, sorry, just my head was overloaded with tons of new things.

The correct example should be:

```
clang library.s -fdwarf-fission=foo -o 1.o
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
```

So in this case clang creates the assembler job and prints the message only once.

If we add some *.cpp then we will see multiple messages (one for each job):

```
clang library.s main.cpp -fdwarf-fission=foo -o 1.o
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
```

But this is consistent with what clang already do. For example `-masm=` will trigger the same behavior:

```
~/LLVM/build_lldb/bin/clang library.s main.cpp -fdwarf-fission=foo -o 1.o -masm=foo
clang-8: error: unsupported argument 'foo' to option 'masm='
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission='
clang-8: error: unsupported argument 'foo' to option 'masm='
clang-8: error: unsupported argument 'foo' to option 'fdwarf-fission=
```

i.e. for options that are used both in main clang job and "secondary" jobs, like `as` that is possible already.


https://reviews.llvm.org/D52296