> -----Original Message----- > From: Joseph Myers [mailto:jos...@codesourcery.com] > Sent: Friday, December 07, 2012 2:04 AM > To: Terry Guo > Cc: gcc-patches@gcc.gnu.org > Subject: RE: [RFC] New feature to reuse one multilib among different > targets > > On Tue, 13 Nov 2012, Terry Guo wrote: > > > +@findex MULTILIB_REUSE > > +@item MULTILIB_REUSE > > +Sometimes it is desirable to reuse one existing multilib among > different > > +targets. Such kind of reuse can minimize the number of multilib > variants. > > I don't think "among different targets" is the right wording here. > "for > different sets of options"? >
Updated with your comments.
> > +A typical reuse rule is comprised of two parts connected by equality
> sign.
>
> Not just a typical reuse rule, but *any* reuse rule, as I understand it.
>
You are right. Removed the word "typical".
> > +The left part of the rule are the options used to build multilib and
> the right
> > +part are the options representing target that will reuse this
> multilib. The
> > +equality sign in both parts should be replaced with period.
>
> Is the order of the options significant, on either or both sides? Can
> the
> options on the right hand side be options that aren't used to build any
> multilibs? I think the documentation should answer that sort of
> question.
>
Yes, the option order in left part matters. More explanations are added as
in patch.
> > @@ -7475,10 +7484,16 @@ set_multilib_dir (void)
> >
> > first = 1;
> > p = multilib_select;
> > +
> > + /* Append multilib reuse rules if any. With those rules, we can
> reuse
> > + one multilib for certain different targets. */
> > + if (strlen(multilib_reuse) > 0)
>
> Missing space before '('.
>
Added the missing space.
> > - /* Ignore newlines. */
> > - if (*p == '\n')
> > + /* Ignore newlinesi and spaces. */
>
> Typo "newlinesi". And why the change to ignore spaces as well - what's
> different about this new feature to require that?
>
Typo is corrected. I once wanted to enable user to change multilib spec in
gcc driver on the fly by defining own spec file with content:
*multilib:
OWN RULES
or
*multilib:
+ OWN RULES
For the latter case, an extra space will be involved and break the parse of
multilib spec. So I ignore space here.
But as your said we had better not touch those internal data structure, I
give up this idea now.
> > @@ -7491,8 +7506,8 @@ set_multilib_dir (void)
> > if (*p == '\0')
> > {
> > invalid_select:
> > - fatal_error ("multilib select %qs is invalid",
> > - multilib_select);
> > + fatal_error ("multilib select %qs%qs is invalid",
> > + multilib_select, multilib_reuse);
>
> Printing two quoted strings with no space between the closing quote of
> one
> and the opening quote of the other certainly doesn't seem right.
> (Really
> this whole error message seems pretty bad - it won't make sense to
> users -
> but that's a pre-existing condition.)
>
An extra space is inserted.
> > +rm -rf tmpmultilib3
> > +cat >tmpmultilib3 <<\EOF
>
> As I understand it, this is a refactoring of existing code. The patch
> might be easier to review if the bits that just refactored existing
> code
> into sub-scripts (without any changes to that code) were sent as a
> separate self-contained patch, and then the new feature patch was sent
> as
> a patch applying on top of those.
>
Your understanding is correct. Now I put code refactor part into patch 00.
Patch 01 is supposed to be applied on top of it.
> > + # We only care rule that has concrete multilib.
>
> "care about", I think, but this sentence still doesn't really make
> sense
> to me. What are the cases that aren't being cared about here, and why
> are
> they valid inputs? Surely, given a proper MULTILIB_REUSE setting,
> every
> rule in that setting should do something meaningful and rules that
> don't
> should result in errors?
>
Now an error will be generated once the rule tries to reuse nonexistent
multilib.
Thank you again, Joseph.
BR,
Terry
2012-12-07 Terry Guo <terry....@arm.com>
* gcc/Makefile.in (s-mlib): New argument MULTILIB_REUSE.
* gcc/doc/fragments.texi: Document MULTILIB_REUSE.
* gcc/gcc.c (multilib_reuse): New internal spec.
(set_multilib_dir): Also search multilib from multilib_reuse.
* gcc/genmultilib (tmpmultilib3): Refactor code.
(tmpmultilib4): Ditto.
(multilib_reuse): New multilib argument.
00-multilib-reuse-v5.patch
Description: Binary data
01-multilib-reuse-v5.patch
Description: Binary data
