Le 27 janvier 2012 11:46, James Molloy <[email protected]> a écrit :
> Hi Chandler, > > Thanks for the comments. > > > 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. > > The way I see it, the options should be serialized and deserialized by a > CompilerInvocation object. Whether that lives in libFrontend.a or > libDriver.a is really moot from my POV, but yes it can be moved wholesale > into the Driver. > > > 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. =] > > So the current patch deals with the trivial "round trip" of parsing a flag > in the driver and emitting that *exact same flag* for the frontend. Other > flags that are different in the 'clang' CLI to the CC1 CLI, can be dealt > with via Aliases I'd have thought? I'm not massively worried about them. > > What I am worried about is keeping the default options all in one place, > along with the [de]serialization. I'm also currently concerned about the > lack of orthogonality between -fFOO and -fno-FOO options - having the > negation specified separately seems to be heading for disaster where > there's no inverse for an option. I'd like to have the OptParser deal with > negation itself, if possible. > > Yes this "negation" has always bugged me, it's incredibly error-prone and seems like something tablegen should be able to handle automatically. I am not so sure the current option parser could easily be tweaked to deal with this though, but I may be pessimitisc. -- Matthieu > > 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? > > So you're suggesting a test suite that would test whether each option is > actually being used/acted on? I'm not sure - with the current patch we > essentially remove most forwarding. Only a CompilerInvocation will parse > most flags intended for cc1, so the driver need not worry about them or > forward them. What I'm suggesting is changing Clang::constructJob() to be > essentially: > > CompilerInvocation CI; > CompilerInvocation::CreateFromArgs(Args, CI); > // Modify CI here based on stuff only the driver knows - host/target > triple, isystem, -I etc. > CI.Serialize(CmdArgs); > > So, no forwarding and no defaults for the majority of options. > > > 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): > > It'd take a bit of work to go through and find all options the driver > isn't using itself - can I please do that as a followup patch? > > > 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. > > Yeah, although I admit that my priority is more "making Tools.cpp and > ToolChains.cpp manageable and not ridden with copypasta" and sorting out > the cross-compilation story via a target database. So the argument > translation is low on my radar (but still there). > > I agree with you though, totally. > > > 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. > > Yeah the reason that changed was that CC1 had special options for Os/Oz, > and the driver handles them all via Joined<"-O">. > > Cheers, > > James > > From: Chandler Carruth [mailto:[email protected]] > Sent: 27 January 2012 10:20 > To: James Molloy > Cc: [email protected]; Joerg Sonnenberger > Subject: Re: [PATCH] Driver: Unify CC1Options.td and Options.td > > On Fri, Jan 27, 2012 at 1:44 AM, James Molloy <[email protected]> > wrote: > Hi, > > The attached patch changes the way CC1 options are handled, unifying the > generated enums for both driver and CC1. This: > * Will remove duplicated CC1/Driver options, removing around 140 > duplicated options. > * Stops the need to add new compiler options in two places. > * Paves the way for the driver to construct a CompilerInvocation instead > of manually translating options and adding defaults. When this is done it > will reduce the code in Tools.cpp drastically. > > 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. > > 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. =] > > There are only two minor functionality changes (apart from that most flags > that were not correctly forwarded by the driver now get forwarded): > > It would be awesome to get some more test coverage for these... > > 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? > > > This is the first step in having only one place that arguments are parsed > for Clang. > > The patch is large mainly because of all the flags that needed to be > de-duplicated (and I also copied over the HelpText from CC1Options.td in > these cases because Options.td was sorely lacking in help text). > > Yea, the help text is a real sore spot for the actual options we expose. > > 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): > > 1) CC1's CLI -- Every functional switch has to exist at least here. > 2) GCC-compat CLI > 3) (eventually) cl.exe compat CLI > > 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. > > 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. > > Thoughts? > > 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. > > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
