Generator programs can only be built with optimization enabled?

[Mark Mitchell]

Typing "make" in $objdir/gcc (after a bootstrap) sometimes results in
errors like:

build/gencondmd.o: In function `VEC_rtx_heap_reserve':
/net/sparrowhawk/scratch/mitchell/src/lto/gcc/rtl.h:195: undefined
reference to `vec_heap_p_reserve'

For an ordinary "make" the generator programs are built without
optimization.  But, rtl.h use DEF_VEC_*, the expansion of which includes
inline functions.   With optimization disabled, the compiler apparently
emits the inline functions.  The inline functions reference symbols
(like vec_heap_p_reserve) that are not included in the build objects
linked into the generator program.

I'm using a version of mainline that's a few weeks old; is this
something that has been recently fixed?

Thanks,

--
Mark Mitchell
CodeSourcery
mark@...
[phone] x713

[Andrew Pinski]

On Jun 11, 2006, at 1:20 PM, Mark Mitchell wrote:

> I'm using a version of mainline that's a few weeks old; is this
> something that has been recently fixed?

Not really fixed, just hidden and the behavior of cgraph reverted to
what it was in 4.1.0 but you can still reproduce the failure with
-fkeep-inline-functions or compiling with Sun's CC (at default
optimizations).

See PR 18058 for how long this has been going on, there was a patch  
there
but it sounds like it does not fix the full problem any more.

Thanks,
Andrew Pinski

[David Edelsohn]

        This is part of the new build infrastructure.  One cannot simply
go into $objdir/gcc and type "make".  One either needs to use the
appropriate incantation at the top-level build directory or go into
$objdir/gcc and type "make CFLAGS='xxx'", where 'xxx' matches the
optimization options for the current bootstrap phase.

David

[Gabriel Dos Reis-2]

Mark Mitchell <mark@...> writes:

| Typing "make" in $objdir/gcc (after a bootstrap) sometimes results in
| errors like:
|
| build/gencondmd.o: In function `VEC_rtx_heap_reserve':
| /net/sparrowhawk/scratch/mitchell/src/lto/gcc/rtl.h:195: undefined
| reference to `vec_heap_p_reserve'
|
| For an ordinary "make" the generator programs are built without
| optimization.  But, rtl.h use DEF_VEC_*, the expansion of which includes
| inline functions.   With optimization disabled, the compiler apparently
| emits the inline functions.  The inline functions reference symbols
| (like vec_heap_p_reserve) that are not included in the build objects
| linked into the generator program.

this must be a bug, not a feature.  I think we already have PRs about it.

-- Gaby

[Mark Mitchell]

David Edelsohn wrote:
> This is part of the new build infrastructure.  One cannot simply
> go into $objdir/gcc and type "make".  One either needs to use the
> appropriate incantation at the top-level build directory or go into
> $objdir/gcc and type "make CFLAGS='xxx'", where 'xxx' matches the
> optimization options for the current bootstrap phase.

That seems unfortunate, but so be it.

Thanks,

--
Mark Mitchell
CodeSourcery
mark@...
[phone] x713

[David Edelsohn]

>>>>> Mark Mitchell writes:

Mark> That seems unfortunate, but so be it.

        Yes it is very unfortunate and not very convenient for the way
that most developers want to use the build infrastructure.  There no
longer is an equivalent to "make quickstrap".  To rebuild only GCC, one
can use "make all-stageN-gcc" at the top-level.

David

[Paolo Bonzini]

David Edelsohn wrote:
>>>>>> Mark Mitchell writes:
>
> Mark> That seems unfortunate, but so be it.
>
> Yes it is very unfortunate and not very convenient for the way
> that most developers want to use the build infrastructure.  There no
> longer is an equivalent to "make quickstrap".  To rebuild only GCC, one
> can use "make all-stageN-gcc" at the top-level.

Andrew pointed out PR18058, where you can read:

----- Comment #33 From Andrew Pinski  2006-06-11 13:10  [reply] -------

...
This was caused by:
2006-01-22  Zack Weinberg  <zackw@...>

         * genautomata.c: Include vec.h, not varray.h.


The problem that Mark reported happens because (since always) the CFLAGS
of the gcc directory are just "-g", not "-O2 -g".  Optimized builds have
(since always) been done only because the toplevel overrides the "-g"
CFLAGS.  So, when you type "make" in the gcc directory, it triggers a
non-optimized build of everything (generator programs, compiler,
driver), which in turn triggers PR18058.

So, let's please not confuse issues.  I work in the GCC directory daily.
  I type "make" there and it just works.  You can even type "make
quickstrap" if you want:

ifeq ($(BOOTSTRAPPING),yes)
# Provide quickstrap as a target that people can type into the gcc
# directory, and that fails if you're not into it.
quickstrap: all
else
...
endif

I think this was your suggestion, and it was implemented.

If there are requests for improvements to the build infrastructure,
please tell me clearly about them.  However, I don't see how this is
related to the problem that Mark reported.

Paolo

[David Edelsohn]

>>>>> Paolo Bonzini writes:

Paolo> So, let's please not confuse issues.  I work in the GCC directory daily.
Paolo> I type "make" there and it just works.  You can even type "make
Paolo> quickstrap" if you want:

Paolo> I think this was your suggestion, and it was implemented.

        Typing "make" in the gcc subdirectory does not do what I expect.

        Also, if "make quickstrap" is implemented, you should update the
"Top-Level Bootstrap" page in the GCC Wiki to document the current
functionality.

David

[Daniel Jacobowitz-2]

On Mon, Jun 12, 2006 at 10:22:17AM -0400, David Edelsohn wrote:
> Typing "make" in the gcc subdirectory does not do what I expect.

Then could you clarify what happens, and what you expect, please?

--
Daniel Jacobowitz
CodeSourcery

[David Edelsohn]

>>>>> Daniel Jacobowitz writes:

Daniel> On Mon, Jun 12, 2006 at 10:22:17AM -0400, David Edelsohn wrote:
>> Typing "make" in the gcc subdirectory does not do what I expect.

Daniel> Then could you clarify what happens, and what you expect, please?

        The behavior prior to the top-level bootstrap changes that I and
others repeatedly have mentioned in email and IRC: if I type "make cc1" in
the gcc subdirectory, the build should be invoked with the appropriate
options from the current build stage.  In other words, if I have a
completely bootstrapped compiler, change a source file, enter $objdir/gcc,
and type "make cc1", I expect cc1 to be rebuilt with CFLAGS="-O2 -g".
Instead, if I type "make" or "make quickstrap", the compilation uses
CFLAGS=-g.

Thanks, David

[Mark Mitchell]

Paolo Bonzini wrote:

> This was caused by:
> 2006-01-22  Zack Weinberg  <zackw@...>
>
>         * genautomata.c: Include vec.h, not varray.h.
>
>
> The problem that Mark reported happens because (since always) the CFLAGS
> of the gcc directory are just "-g", not "-O2 -g".  Optimized builds have
> (since always) been done only because the toplevel overrides the "-g"
> CFLAGS.  So, when you type "make" in the gcc directory, it triggers a
> non-optimized build of everything (generator programs, compiler,
> driver), which in turn triggers PR18058.

I think that, after Zack's change, the generator programs that include
rtl.h should be linked with build/vec.o.  That may not be necessary when
optimizing, but it would avoid this problem.  Do you agree?

--
Mark Mitchell
CodeSourcery
mark@...
[phone] x713

[Paolo Bonzini]

> The behavior prior to the top-level bootstrap changes that I and
> others repeatedly have mentioned in email and IRC: if I type "make cc1" in
> the gcc subdirectory, the build should be invoked with the appropriate
> options from the current build stage.  In other words, if I have a
> completely bootstrapped compiler, change a source file, enter $objdir/gcc,
> and type "make cc1", I expect cc1 to be rebuilt with CFLAGS="-O2 -g".
> Instead, if I type "make" or "make quickstrap", the compilation uses
> CFLAGS=-g.

I see.  Thanks for the explanation.

Paolo

[Paolo Bonzini]

> I think that, after Zack's change, the generator programs that include
> rtl.h should be linked with build/vec.o.  That may not be necessary when
> optimizing, but it would avoid this problem.  Do you agree?
>  
Well, if it fixes the bug, yes: I prefer to fix this in the makefile
than with #ifdef GENERATOR_FILE's here and there.

An untested patch to do so is attached.  You can try it and, if it
fails, there is also Rainer Orth's patch in comment #14 of the PR.

Paolo

2006-06-13  Paolo Bonzini  <bonzini@...>

        * Makefile.in (BUILD_RTL): Add build/vec.o.
        (build/genextract, build/genautomata): Remove dependency on it.

Index: Makefile.in
===================================================================
--- Makefile.in (revision 114487)
+++ Makefile.in (working copy)
@@ -854,7 +854,7 @@ LDEXP_LIB = @LDEXP_LIB@
 # even if we are cross-building GCC.
 BUILD_LIBS = $(BUILD_LIBIBERTY)
 
-BUILD_RTL = build/rtl.o build/read-rtl.o build/ggc-none.o \
+BUILD_RTL = build/rtl.o build/read-rtl.o build/ggc-none.o build/vec.o \
     build/min-insn-modes.o build/gensupport.o build/print-rtl.o
 BUILD_ERRORS = build/errors.o
 
@@ -3017,9 +3020,6 @@ genprogmd = attr attrtab automata codes
 $(genprogmd:%=build/gen%$(build_exeext)): $(BUILD_RTL) $(BUILD_ERRORS)
 
 # These programs need files over and above what they get from the above list.
-build/genextract$(build_exeext) : build/vec.o
-
-build/genautomata$(build_exeext) : build/vec.o
 build/genautomata$(build_exeext) : BUILD_LIBS += -lm
 
 # These programs are not linked with the MD reader.

[Eric Botcazou]

> An untested patch to do so is attached.  You can try it and, if it
> fails, there is also Rainer Orth's patch in comment #14 of the PR.

Sure, but read the date of the comment. :-)

I'm really wondering what the "Patch URL" field of the PR is for...

IMHO this PR is a striking example of the *major* problems we have been having
in the patch reviewing department for quite some time.

--
Eric Botcazou

[Paolo Bonzini]

Eric Botcazou wrote:
>> An untested patch to do so is attached.  You can try it and, if it
>> fails, there is also Rainer Orth's patch in comment #14 of the PR.
>>    
> Sure, but read the date of the comment. :-)
>  
Yes, OTOH it is the patch that I like the most...

Thanks for chiming in this discussion.  You've clearly given a good deal
of thought to the problem, and if you have suggestions I'm all ears.

Paolo

[Eric Botcazou]

> Thanks for chiming in this discussion.  You've clearly given a good deal
> of thought to the problem, and if you have suggestions I'm all ears.

http://gcc.gnu.org/ml/gcc-patches/2006-03/msg00297.html

--
Eric Botcazou

[Paolo Bonzini]

Eric Botcazou wrote:
>> Thanks for chiming in this discussion.  You've clearly given a good deal
>> of thought to the problem, and if you have suggestions I'm all ears.
>>    
>
> http://gcc.gnu.org/ml/gcc-patches/2006-03/msg00297.html
Cool.  Mark, this is very similar to my patch, but better. :-)

I didn't understand the purpose of:

        (build/gencondmd.o): Filter out -fkeep-inline-functions.

Paolo

[Eric Botcazou]

> I didn't understand the purpose of:
>
> (build/gencondmd.o): Filter out -fkeep-inline-functions.

Read the comment?

--
Eric Botcazou

[Paolo Bonzini]

Eric Botcazou wrote:
>> I didn't understand the purpose of:
>>
>> (build/gencondmd.o): Filter out -fkeep-inline-functions.
>>    
> Read the comment?
It can help indeed.

However, the audit trail of the PR seems to say that now
-fkeep-inline-functions is sort of implied by -O0; I can build
insn-conditions.md with "-O0 -fkeep-inline-functions" so I'm not
affected by the PR.  In this case, will your patch still fix the bug
when using GCC with BOOT_CFLAGS=-O0?

Paolo

[Eric Botcazou]

> However, the audit trail of the PR seems to say that now
> -fkeep-inline-functions is sort of implied by -O0; I can build
> insn-conditions.md with "-O0 -fkeep-inline-functions" so I'm not
> affected by the PR.

Comment #36 seems to say that we're back to the initial state.

--
Eric Botcazou