Option round-tripping q.

After the recent refactoring to Clang option handling that introduced
"round-tripping" we had a couple of issues come up with options that
we have defined downstream. In some cases I believe these showed up
as test failures and got fixed by adding code to GenerateCodeGenArgs;
that's fine. And right now I'm doing the marshalling thing so we can
remove some local patches to CompilerInvocation (yay!).

But we also had other options that silently weren't propagated, and
we discovered this only by chance. I would rather have had them show
up as test failures. This likely points to an inadequacy in our test
coverage, but it brings me to a question:

When round-tripping is enabled, is the re-generated command line
textually compared to the original command line in some way? If it
is, I'd have expected it to catch these cases, because some option
on the original CC1 command line wouldn't be in the re-generated
command. But, that didn't happen.

For options that Clang itself does something with, presumably tests
for how the option affects Clang's own behavior would catch issues
(I'm pretty sure that's how we found the cases we did, although I
wasn't the one to do that work). But in the case of a Clang option
that just gets passed down to LLVM (through LLVM's TargetOptions, say)
it's not clear there's a good way to test that actually happens.

Thanks,
--paulr

Hi Paul,

When round-tripping, we don’t textually compare the generated command-line with the original. That’s because the original is not guaranteed to be canonical: it may contain aliased options and the options may be in arbitrary order.

What we could have done is compare the CompilerInvocation created from the original command-line with CompilerInvocation created from the generated command-line. However, for that we’d need to define operator== and remember to update it when adding new CompilerInvocation fields. There was an attempt to generate this automatically: https://reviews.llvm.org/D86290 (Move all fields of ‘-cc1’ option related classes into def file databases), but got reverted.

Currently, we compare CompilerInvocation (B) created from the generated command-line with CompilerInvocation (C) created from command-line generated from (B): https://github.com/llvm/llvm-project/blob/92f9852/clang/lib/Frontend/CompilerInvocation.cpp#L618. That ensures the generation is deterministic, but detecting forgotten options is still up to the test suite.

AFAIK LLVM’s TargetOptions are constructed from parts of Clang’s CompilerInvocation and get end-to-end tested in CodeGen. Am I missing something here?

Cheers,
Jan

Hi Jan,

Thanks for the explanation! I wouldn't expect a full-string
comparison of the command lines, but I hadn't considered aliases
which would make even item-by-item comparison problematic.

AFAIK LLVM's TargetOptions are constructed from parts of Clang's
CompilerInvocation and get end-to-end tested in CodeGen. Am I
missing something here?

Hmmm... I do see (for example) -ffunction-sections tested that way;
although IMO this constitutes end-to-end testing which doesn't
properly belong in the Clang lit suite. But at the moment we don't
have anywhere else to put it. I'll make sure our options have tests
in CodeGen, for lack of anything better.

Thanks,
--paulr