Patchwork Move -fbuiltin from c.opt to common.opt and change it to common group

login
register
mail settings
Submitter Kito Cheng
Date Aug. 28, 2014, 10:19 a.m.
Message ID <CA+yXCZAFMtz1rxakrRgWC5FAg=aBj4onh=bcry2V3OyscbOtDg@mail.gmail.com>
Download mbox | patch
Permalink /patch/383753/
State New
Headers show

Comments

Kito Cheng - Aug. 28, 2014, 10:19 a.m.
Hi all:

-fno-builtin is seem not only for the c family front-end, but also
used in LTO now, so move it to common.opt and change it to `Common`.
Richard Guenther - Aug. 28, 2014, 10:26 a.m.
On Thu, 28 Aug 2014, Kito Cheng wrote:

> Hi all:
> 
> -fno-builtin is seem not only for the c family front-end, but also
> used in LTO now, so move it to common.opt and change it to `Common`.

Please leave it in c-family and just add LTO to the set of supported
languages.  -fno-builtin isn't meaningful for other frontends
and we just happen to use the flag.

If then it makes more sense to move -fhosted and -ffreestanding
though I don't know how meaningful those are for other frontends.

Or create a "proper" flag to communicate that the middle-end
should avoid creating new calls to builtins at all cost
(well, that's really what -ffreestanding is about).

Richard.
Kito Cheng - Aug. 28, 2014, 3:54 p.m.
Hi Richard:

>> -fno-builtin is seem not only for the c family front-end, but also
>> used in LTO now, so move it to common.opt and change it to `Common`.
>
> Please leave it in c-family and just add LTO to the set of supported
> languages.  -fno-builtin isn't meaningful for other frontends
> and we just happen to use the flag.
> If then it makes more sense to move -fhosted and -ffreestanding
> though I don't know how meaningful those are for other frontends.
>
> Or create a "proper" flag to communicate that the middle-end
> should avoid creating new calls to builtins at all cost
> (well, that's really what -ffreestanding is about).

-fno-builtin is meaningless for other front-end but middle-end,
However `-fno-builtin` is more explicit than -fhosted and  -ffreestanding,
when people see the option `-fno-builtin`, they will know what this
option mean "do not use builtin implicitly", and most important
 -fno-builtin is more well known than -fhosted or -ffreestanding.

and the flag_no_builtin is already used in gcc/lto/lto-lang.c:def_builtin_1,
so my patch is not first user of this option in LTO front-end.

> Richard.
Richard Guenther - Aug. 29, 2014, 7:58 a.m.
On Thu, 28 Aug 2014, Kito Cheng wrote:

> Hi Richard:
> 
> >> -fno-builtin is seem not only for the c family front-end, but also
> >> used in LTO now, so move it to common.opt and change it to `Common`.
> >
> > Please leave it in c-family and just add LTO to the set of supported
> > languages.  -fno-builtin isn't meaningful for other frontends
> > and we just happen to use the flag.
> > If then it makes more sense to move -fhosted and -ffreestanding
> > though I don't know how meaningful those are for other frontends.
> >
> > Or create a "proper" flag to communicate that the middle-end
> > should avoid creating new calls to builtins at all cost
> > (well, that's really what -ffreestanding is about).
> 
> -fno-builtin is meaningless for other front-end but middle-end,

Well, I wouldn't say that.

> However `-fno-builtin` is more explicit than -fhosted and  -ffreestanding,
> when people see the option `-fno-builtin`, they will know what this
> option mean "do not use builtin implicitly", and most important
>  -fno-builtin is more well known than -fhosted or -ffreestanding.

But builtins _are_ used implicitely regardless of -fno-builtin!
At least memcpy, memmove and memset are.

> and the flag_no_builtin is already used in gcc/lto/lto-lang.c:def_builtin_1,
> so my patch is not first user of this option in LTO front-end.

Yeah, but nothing sets that flag in LTO so it is always zero
(so the use is bogus).

It's also that setting that flag dependent on a "merged" -fno-builtin
will break TUs that use builtins.  So I fear it's not that easy.
Hmm.

I don't like all those tri-state options but I guess it would
make sense to have for -ftree-loop-distribute-patterns for this
case (to note it's disabled by -ffreestanding/-fno-builtin).

That said - the whole way we handle option merging in LTO is
somewhat broken and this is just an example where the current
hacky scheme breaks down.

So maybe instead finally fix LTO option merging?

There were two ideas floating around - first is to annotate
all functions with option and target attributes as coming
in from the individual TUs, second is to do LTRANs partitioning
based on options and thus have different "global" options for
different LTRANS units.

Note that for selected options we already annotate struct function
(like for -fnon-call-exceptions), eventually annotating struct
function is more sensible than optimize or target attributes.

The question remains what to do with the options explicitely
specified at link time - this probably means we need to continue
to distinguish flags we have to conservatively carry over from
compile-phase and those we can safely override.

Both above ideas mean that option processing would mostly move from
lto-wrapper to lto1 (WPA phase), maybe apart from computing a
default optimization level in case there was none specified at
link time.

Maybe you have time working towards this?  A first step would be
to move most of the option merging code from lto-wrapper to
lto1.

Richard.
Kito Cheng - Aug. 29, 2014, 4:04 p.m.
Hi Richard:

>> >> -fno-builtin is seem not only for the c family front-end, but also
>> >> used in LTO now, so move it to common.opt and change it to `Common`.
>> >
>> > Please leave it in c-family and just add LTO to the set of supported
>> > languages.  -fno-builtin isn't meaningful for other frontends
>> > and we just happen to use the flag.
>> > If then it makes more sense to move -fhosted and -ffreestanding
>> > though I don't know how meaningful those are for other frontends.
>> >
>> > Or create a "proper" flag to communicate that the middle-end
>> > should avoid creating new calls to builtins at all cost
>> > (well, that's really what -ffreestanding is about).
>>
>> -fno-builtin is meaningless for other front-end but middle-end,
>
> Well, I wouldn't say that.

Sorry for my misunderstand.

>> However `-fno-builtin` is more explicit than -fhosted and  -ffreestanding,
>> when people see the option `-fno-builtin`, they will know what this
>> option mean "do not use builtin implicitly", and most important
>>  -fno-builtin is more well known than -fhosted or -ffreestanding.
>
> But builtins _are_ used implicitely regardless of -fno-builtin!
> At least memcpy, memmove and memset are.

Are you mean the name of `-fno-builtins` is not precise enough?
or we can't disable ALL implicitly built-in function call so `-fno-builtins`
is not the best option name for solve this problem?

And I still prefer use `-fno-builtin` since it's well known:

Here is some investigate for the C library:
Many C library has add `-fno-builtin`, but only musl add `-ffreestanding`
(Sampling: glibc, uClibc, newlib, dietlibc, musl, bionic)

So if we use `-fno-builtin` then the C library can do LTO without
change in future.
(though LTO with C library is not work well today.)

>> and the flag_no_builtin is already used in gcc/lto/lto-lang.c:def_builtin_1,
>> so my patch is not first user of this option in LTO front-end.
>
> Yeah, but nothing sets that flag in LTO so it is always zero
> (so the use is bogus).

Agree.

> It's also that setting that flag dependent on a "merged" -fno-builtin
> will break TUs that use builtins.  So I fear it's not that easy.
> Hmm.
>
> I don't like all those tri-state options but I guess it would
> make sense to have for -ftree-loop-distribute-patterns for this
> case (to note it's disabled by -ffreestanding/-fno-builtin).
>
> That said - the whole way we handle option merging in LTO is
> somewhat broken and this is just an example where the current
> hacky scheme breaks down.

Yes, I understand the root cause indeed is the option merging
problem, I have tried to LTO a program with llibc.a (newlib)
and then got lots of problem for option and built-in functions.

> So maybe instead finally fix LTO option merging?
>
> There were two ideas floating around - first is to annotate
> all functions with option and target attributes as coming
> in from the individual TUs, second is to do LTRANs partitioning
> based on options and thus have different "global" options for
> different LTRANS units.

In my understand the key point is grouping by option and prevent
interaction between different group (eg. never inline or ipa-cp between
 different group).
The second idea seem more feasible at this moment since most
option in gcc still propagate by global variable, so maybe we need
to refactoring `option flags` in gcc for first idea if we doing this approach.

> Note that for selected options we already annotate struct function
> (like for -fnon-call-exceptions), eventually annotating struct
> function is more sensible than optimize or target attributes.
>
> The question remains what to do with the options explicitely
> specified at link time - this probably means we need to continue
> to distinguish flags we have to conservatively carry over from
> compile-phase and those we can safely override.
>
> Both above ideas mean that option processing would mostly move from
> lto-wrapper to lto1 (WPA phase), maybe apart from computing a
> default optimization level in case there was none specified at
> link time.
>
> Maybe you have time working towards this?  A first step would be
> to move most of the option merging code from lto-wrapper to
> lto1.

However seem both idea need to do this thing first, I will take a look
in next days :)
Richard Guenther - Sept. 1, 2014, 9:46 a.m.
On Sat, 30 Aug 2014, Kito Cheng wrote:

> Hi Richard:
> 
> >> >> -fno-builtin is seem not only for the c family front-end, but also
> >> >> used in LTO now, so move it to common.opt and change it to `Common`.
> >> >
> >> > Please leave it in c-family and just add LTO to the set of supported
> >> > languages.  -fno-builtin isn't meaningful for other frontends
> >> > and we just happen to use the flag.
> >> > If then it makes more sense to move -fhosted and -ffreestanding
> >> > though I don't know how meaningful those are for other frontends.
> >> >
> >> > Or create a "proper" flag to communicate that the middle-end
> >> > should avoid creating new calls to builtins at all cost
> >> > (well, that's really what -ffreestanding is about).
> >>
> >> -fno-builtin is meaningless for other front-end but middle-end,
> >
> > Well, I wouldn't say that.
> 
> Sorry for my misunderstand.
> 
> >> However `-fno-builtin` is more explicit than -fhosted and  -ffreestanding,
> >> when people see the option `-fno-builtin`, they will know what this
> >> option mean "do not use builtin implicitly", and most important
> >>  -fno-builtin is more well known than -fhosted or -ffreestanding.
> >
> > But builtins _are_ used implicitely regardless of -fno-builtin!
> > At least memcpy, memmove and memset are.
> 
> Are you mean the name of `-fno-builtins` is not precise enough?
> or we can't disable ALL implicitly built-in function call so `-fno-builtins`
> is not the best option name for solve this problem?
> 
> And I still prefer use `-fno-builtin` since it's well known:
> 
> Here is some investigate for the C library:
> Many C library has add `-fno-builtin`, but only musl add `-ffreestanding`
> (Sampling: glibc, uClibc, newlib, dietlibc, musl, bionic)
> 
> So if we use `-fno-builtin` then the C library can do LTO without
> change in future.
> (though LTO with C library is not work well today.)
> 
> >> and the flag_no_builtin is already used in gcc/lto/lto-lang.c:def_builtin_1,
> >> so my patch is not first user of this option in LTO front-end.
> >
> > Yeah, but nothing sets that flag in LTO so it is always zero
> > (so the use is bogus).
> 
> Agree.
> 
> > It's also that setting that flag dependent on a "merged" -fno-builtin
> > will break TUs that use builtins.  So I fear it's not that easy.
> > Hmm.
> >
> > I don't like all those tri-state options but I guess it would
> > make sense to have for -ftree-loop-distribute-patterns for this
> > case (to note it's disabled by -ffreestanding/-fno-builtin).
> >
> > That said - the whole way we handle option merging in LTO is
> > somewhat broken and this is just an example where the current
> > hacky scheme breaks down.
> 
> Yes, I understand the root cause indeed is the option merging
> problem, I have tried to LTO a program with llibc.a (newlib)
> and then got lots of problem for option and built-in functions.
> 
> > So maybe instead finally fix LTO option merging?
> >
> > There were two ideas floating around - first is to annotate
> > all functions with option and target attributes as coming
> > in from the individual TUs, second is to do LTRANs partitioning
> > based on options and thus have different "global" options for
> > different LTRANS units.
> 
> In my understand the key point is grouping by option and prevent
> interaction between different group (eg. never inline or ipa-cp between
>  different group).
> The second idea seem more feasible at this moment since most
> option in gcc still propagate by global variable, so maybe we need
> to refactoring `option flags` in gcc for first idea if we doing this approach.
> 
> > Note that for selected options we already annotate struct function
> > (like for -fnon-call-exceptions), eventually annotating struct
> > function is more sensible than optimize or target attributes.
> >
> > The question remains what to do with the options explicitely
> > specified at link time - this probably means we need to continue
> > to distinguish flags we have to conservatively carry over from
> > compile-phase and those we can safely override.
> >
> > Both above ideas mean that option processing would mostly move from
> > lto-wrapper to lto1 (WPA phase), maybe apart from computing a
> > default optimization level in case there was none specified at
> > link time.
> >
> > Maybe you have time working towards this?  A first step would be
> > to move most of the option merging code from lto-wrapper to
> > lto1.
> 
> However seem both idea need to do this thing first, I will take a look
> in next days :)

Thanks!

Richard.

Patch

From 47552b58a09ac9d944be1c35bb5c938f4cb8ec0f Mon Sep 17 00:00:00 2001
From: Kito Cheng <kito@andestech.com>
Date: Thu, 14 Aug 2014 11:34:26 +0800
Subject: [PATCH 1/2] Move -fbuiltin from c.opt to common.opt and change it to
 common group

ChangeLog

2014-09-28  Kito Cheng  <kito@0xlab.org>

	c-family/
	* c.opt (fbuiltin): Move to gcc/common.opt

	gcc/
	* common.opt (fbuiltin): Add.
---
 gcc/c-family/c.opt | 4 ----
 gcc/common.opt     | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index d619250..ae04114 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -912,10 +912,6 @@  Recognize the \"asm\" keyword
 fbuilding-libgcc
 C ObjC C++ ObjC++ Undocumented Var(flag_building_libgcc)
 
-fbuiltin
-C ObjC C++ ObjC++ Var(flag_no_builtin, 0)
-Recognize built-in functions
-
 fbuiltin-
 C ObjC C++ ObjC++ Joined
 
diff --git a/gcc/common.opt b/gcc/common.opt
index f7021102..607799d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -923,6 +923,10 @@  fbtr-bb-exclusive
 Common Report Var(flag_btr_bb_exclusive) Optimization
 Restrict target load migration not to re-use registers in any basic block
 
+fbuiltin
+Common Var(flag_no_builtin, 0)
+Recognize built-in functions
+
 fcall-saved-
 Common Joined RejectNegative Var(common_deferred_options) Defer
 -fcall-saved-<register>	Mark <register> as being preserved across functions
-- 
1.9.3