<div class="gmail_quote">On Fri, Jan 27, 2012 at 1:44 AM, James Molloy <span dir="ltr"><<a href="mailto:james.molloy@arm.com">james.molloy@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi,<br>
<br>
The attached patch changes the way CC1 options are handled, unifying the<br>
generated enums for both driver and CC1. This:<br>
  * Will remove duplicated CC1/Driver options, removing around 140<br>
duplicated options.<br>
  * Stops the need to add new compiler options in two places.<br>
  * Paves the way for the driver to construct a CompilerInvocation instead<br>
of manually translating options and adding defaults. When this is done it<br>
will reduce the code in Tools.cpp drastically.<br></blockquote><div><br></div><div>Really cool! So, I assume one of the next big things on this path is to lift the current serialization / deserialization / parsing logic inside of the Frontend into the Driver as well, making the Frontend simply accept option structs and interfaces? That's been a long-standing goal of mine. Essentially, the options shouldn't escape the driver.</div>
<div><br></div><div>The other big thing I think to simplify Tools.cpp is to come up with common patterns for the flags that go through a round of translation when going from the 'clang' CLI to the CC1 CLI, and to specify these patterns in the TableGen spec for the flags. That where you see this going? I just want to get a good shared picture of how this will look now, so that we don't have too many false starts. =]</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">There are only two minor functionality changes (apart from that most flags<br>
that were not correctly forwarded by the driver now get forwarded):<br></blockquote><div><br></div><div>It would be awesome to get some more test coverage for these...</div><div><br></div><div>Actually, it would be awesome to have a comprehensive (even if initially mechanically generated) test for all of the pure forwarding behavior, and lack of pure forwarding. That way each time a flag is added we know to go add it to one of the test cases. Is that do-able?</div>
<div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This is the first step in having only one place that arguments are parsed<br>
for Clang.<br>
<br>
The patch is large mainly because of all the flags that needed to be<br>
de-duplicated (and I also copied over the HelpText from CC1Options.td in<br>
these cases because Options.td was sorely lacking in help text).</blockquote><div><br></div><div>Yea, the help text is a real sore spot for the actual options we expose.</div><div><br></div><div>I have one request to change the way you've structured the result. I'd like to pull all of the pure-forwarded CC1 options down into a single file. What I'm imagining conceptually is the following. We have essentially two CLIs (and hopefully eventually a third):</div>
<div><br></div><div>1) CC1's CLI -- Every functional switch has to exist at least here.</div><div>2) GCC-compat CLI</div><div>3) (eventually) cl.exe compat CLI</div><div><br></div><div>For simplicity and sanity, I think its healthy to make CC1 be as much a subset of #2 (or in theory #3, but that ship sailed long ago) as possible just to reduce the size of the interface. The result is that CC1 has all of the GCC-compat options that make enough sense to use directly, and al of the Clang-specific options in a GCC-style formulation. The GCC-compat layer adds on top of CC1 all the GCC flags that have too confusing of names to deal with internally or don't map directly onto CC1 flag, as well as any logic necessary to translate into CC1. #2 will also suppress any CC1 flags from #1 that if exposed would break some compat aspect (I don't know what thes ewould be, but in theory...). Finally, the #3 layer will be essentially the same as #2, but with essentially everything from CC1 suppressed, and a complete wrapper with translation logic provided.</div>
<div><br></div><div>If this is the direction you're thinking as well, I'd like to move the CC1 bits down into the CC1Options.td file, and then (likely after your patch) rename Options.td to something better (GCCOptions.td doesn't seem quite right, but not sure what is). The goal being to start separating in the tables the CC1 interface from the compat wrappers.</div>
<div><br></div><div>Thoughts?</div><div><br></div><div>I've not looked at some of the details of the patch, I'll try to get to that tomorrow.. The only thing that looked even a little odd was the -O handling -- might just want comments or something there.</div>
</div>