Re: ICE with precompiled headers (GCC 8.1)
On 5/1/19 10:56 AM, Paul Smith wrote: On Wed, 2019-05-01 at 09:35 -0400, Paul Smith wrote: Unfortunately my GCC is heavily optimized and stripped so backtraces are useless. I will generate a debuggable GCC and see if I can get more info on the ICE. I have created a GCC with debug enabled so I'll see what I find. I was able to reproduce this ICE quite readily with my debuggable GCC 8.1.0. Here's the failure: : internal compiler error: Segmentation fault 0x9cae61 crash_signal /work/src/cc/gcc-8.1.0/gcc/toplev.c:325 0x1293778 apply_vpath /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:127 0x1293acc deps_add_dep(deps*, char const*) /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:258 0x1293fe3 deps_restore(deps*, _IO_FILE*, char const*) /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:432 0x129535b cpp_read_state(cpp_reader*, char const*, _IO_FILE*, save_macro_data*) /work/src/cc/gcc-8.1.0/libcpp/pch.c:884 0x596d59 c_common_read_pch(cpp_reader*, char const*, int, char const*) /work/src/cc/gcc-8.1.0/gcc/c-family/c-pch.c:373 0x12872fe should_stack_file /work/src/cc/gcc-8.1.0/libcpp/files.c:814 0x12874f1 _cpp_stack_file /work/src/cc/gcc-8.1.0/libcpp/files.c:900 0x12876a7 _cpp_stack_include /work/src/cc/gcc-8.1.0/libcpp/files.c:1049 0x1287b22 cpp_push_include(cpp_reader*, char const*) /work/src/cc/gcc-8.1.0/libcpp/files.c:1484 0x5943ec push_command_line_include /work/src/cc/gcc-8.1.0/gcc/c-family/c-opts.c:1483 0x594615 c_finish_options /work/src/cc/gcc-8.1.0/gcc/c-family/c-opts.c:1452 0x5963a2 c_common_parse_file() /work/src/cc/gcc-8.1.0/gcc/c-family/c-opts.c:1126 Unsurprisingly the problem is that the "deps" data member in cpp_reader* is null: #0 apply_vpath (d=d@entry=0x0, t=t@entry=0x2174860 "/src/foo_pch.h") at /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:127 #1 0x01293acd in deps_add_dep (d=d@entry=0x0, t=t@entry=0x2174860 "/src/foo_pch.h") at /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:258 #2 0x01293fe4 in deps_restore (deps=0x0, fd=fd@entry=0x2171750, self=self@entry=0x2106810 "/src/obj/foo_pch.h.gch") at /work/src/cc/gcc-8.1.0/libcpp/mkdeps.c:432 #3 0x0129535c in cpp_read_state (r=r@entry=0x20f4d60, name=name@entry=0x2106810 "/src/obj/foo_pch.h.gch", f=f@entry=0x2171750, data=0x210bce0) at /work/src/cc/gcc-8.1.0/libcpp/pch.c:884 I have no idea whether the problem is that it should never be possible for "deps" to be null, or whether the problem is that we're not checking for that possibility when we should be. I'm building the current GCC 9.0.1 prerelease to see if I can reproduce it there. Once again removing -fpch-deps solves the problem. I can only assume that without that flag we never bother to walk the deps data member at all. I've been playing with dep generation on the modules branch, and noticed there did appear to be something funky going on with its interaction with PCH. I didn't investigate, but have some patches that I'll be merging in the not too distant future. nathan -- Nathan Sidwell
Re: __attribute__((early_branch))
On Tue, Apr 30, 2019 at 9:53 PM Jeff Law wrote:
>
> On 4/30/19 12:34 PM, cmdLP #CODE wrote:
> > Hello GCC-team,
> >
> > I use GCC for all my C and C++ programs. I know how to use GCC, but I am
> > not a contributor to GCC (yet). I often discover some problems C and C++
> > code have in general. There is often the choice between fast or readable
> > code. Some code I have seen performs well but looks ugly (goto, etc.);
> > other code is simple, but has some performance problems. What if we could
> > make the simple code perform well?
> >
> > There is a common problem with conditional branches inside loops. This can
> > decrease the performance of a program. To fix this issue, the conditional
> > branch should be moved outside of the loop. Sometimes this optimization is
> > done by the compiler, but guessing on optimizations done by the compiler is
> > really bad. Often it is not easy to transform the source code to have the
> > conditional branching outside the loop. Instead I propose a new attribute,
> > which forces the compiler to do a conditional branch (based on the
> > annotated parameter) at the beginning of a function. It branches to the
> > corresponding code of the function compiled with the value being constant.
> >
> > Here is a code example, which contains this issue.
> >
> > enum reduce_op
> > {
> > REDUCE_ADD,
> > REDUCE_MULT,
> > REDUCE_MIN,
> > REDUCE_MAX
> > };
> >
> > /* performance critical function */
> > void reduce_data(enum reduce_op reduce,
> > unsigned const* data,
> > unsigned data_size)
> > {
> > unsigned i, result, item;
> >
> > result = reduce == REDUCE_MULT ? 1u
> >: reduce == REDUCE_MIN ? ~0u // ~0u is UINT_MAX
> >: 0u;
> >
> > for(i = 0; i < data_size; ++i)
> > {
> > item = data[i];
> >
> > switch(reduce)
> > {
> > case REDUCE_ADD:
> > result += item;
> > break;
> >
> > case REDUCE_MULT:
> > result *= item;
> > break;
> >
> > case REDUCE_MIN:
> > if(item < result) result = item;
> > // RIP: result > break;
> >
> > case REDUCE_MAX:
> > if(item > result) result = item;
> > // RIP: result >?= item;
> > break;
> > }
> > }
> >
> > return result;
> > }
> >
> > The value of reduce does not change inside the function. For this
> > example, the optimization is trivial. But consider more complex examples.
> > The function should be optimized to:
> [ ]
> This is loop unswitching. It's a standard GCC optimization. If it's
> not working as well as it should, we're far better off determining why
> and fixing the automatic transformation rather than relying on
> attributes to drive the transformation.
It's currently not implemented for switch () stmts, just for conditionals.
This also hurts SPEC cactusADM. There might be a missed-optimization
bug about this. A simple recursive implementation might be possible;
unswitch one case at a time - maybe order by profile probability. We
already recurse on the unswitched bodies (in case multiple conditions
can be unswitched)
>
> > What if the variable changes?
> >
> > When the variable changes there should be a jump to the corresponding
> > parallel code of the compiled code with new value.
> >
> > Unoptimized
> >
> > /* removes comments starting with # and ending in a newline */
> > void remove_comment(char* dst,
> > char const* src)
> > {
> > // initialization nessecary
> > int state __attribute__((early_branch(0, 1))) = 0;
> >
> > char c;
> >
> > while(*src)
> > {
> > c = *src++;
> >
> > switch(state)
> > {
> > case 0:
> > if(c == '#')
> > state = 1;
> > else
> > *dst++ = c;
> >
> > break;
> >
> > case 1:
> > if(c == '\n')
> > {
> > *dst++ = '\n';
> > state = 0;
> > }
> >
> > break;
> > }
> > }
> > *dst = '\0';
> > }
> >
> > changed to
> >
> > void remove_comment(char* dst,
> > char const* src)
> > {
> > char c;
> >
> > switch(0)
> > {
> > case 0:
> > while(*src)
> > {
> > c = *src++;
> > if(c == '#')
> > goto branch1;
> > else
> >
Re: What is the precise definition of NOTE_INSN_FUNCTION_BEG?
On 01/05/19 20:40, Segher Boessenkool wrote: > On Tue, Apr 30, 2019 at 03:48:02PM -0600, Jeff Law wrote: >> On 4/30/19 11:24 AM, Matthew Malcomson wrote: >>> That was why I ended up suggesting multiple notes -- it's currently >>> trying to satisfy more than one criteria and they're not quite compatible. >> Well, we obviously have to keep arg setup, asan, stack protector and >> nonlocal stuff in the same relative order, but I believe they should all >> ultimately land before the NOTE_INSN_FUNCTION_BEG. THe question is how >> to make that happen :-) > > The current meaning of NOTE_INSN_FUNCTION_BEG is > >/* Indicate the beginning of the function body, > as opposed to parm setup. */ >emit_note (NOTE_INSN_FUNCTION_BEG); > > (function.c), and half of the things that use the note think that > everything before it is argument setup, and nothing after it is. > > Just adding extra notes isn't enough afaics; some surgery is needed. > > > Segher > Apologies, I don't follow -- could you elaborate on why an extra note is not enough? If this note is trying to mark the end of the argument setup for those places you mention, and the start of user code for debug output, and those are not the same place then I would have thought splitting it would be necessary. Do you mean that splitting would have to be followed by some extra work so the different uses would use the specific note they want? MM
Re: Second GCC 9.1 Release Candidate available from gcc.gnu.org
Hi Jakub, > The second release candidate for GCC 9.1 is available from > > https://gcc.gnu.org/pub/gcc/snapshots/9.0.1-RC-20190430/ > ftp://gcc.gnu.org/pub/gcc/snapshots/9.0.1-RC-20190430 > > and shortly its mirrors. It has been generated from SVN revision 270689. > > I have so far bootstrapped and tested the release candidate on > x86_64-linux and i686-linux. Please test it and report any issues to > bugzilla. I've now tested this RC on i386-pc-solaris2.1[01] and sparc-sun-solaris2.1[01]. The only issue (apart from the just-fixed spellcheck-options-5.c testcase) is +FAIL: gcc.target/i386/pr90178.c scan-assembler-times xorl[t ]*%eax,[t ]*%eax 1 While this has already been fixed on mainline, it's still present on the gcc-9 branch on every x86 target. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: What is the precise definition of NOTE_INSN_FUNCTION_BEG?
On Thu, May 02, 2019 at 01:02:14PM +, Matthew Malcomson wrote: > On 01/05/19 20:40, Segher Boessenkool wrote: > > On Tue, Apr 30, 2019 at 03:48:02PM -0600, Jeff Law wrote: > >> On 4/30/19 11:24 AM, Matthew Malcomson wrote: > >>> That was why I ended up suggesting multiple notes -- it's currently > >>> trying to satisfy more than one criteria and they're not quite compatible. > >> Well, we obviously have to keep arg setup, asan, stack protector and > >> nonlocal stuff in the same relative order, but I believe they should all > >> ultimately land before the NOTE_INSN_FUNCTION_BEG. THe question is how > >> to make that happen :-) > > > > The current meaning of NOTE_INSN_FUNCTION_BEG is > > > >/* Indicate the beginning of the function body, > > as opposed to parm setup. */ > >emit_note (NOTE_INSN_FUNCTION_BEG); > > > > (function.c), and half of the things that use the note think that > > everything before it is argument setup, and nothing after it is. > > > > Just adding extra notes isn't enough afaics; some surgery is needed. > > Apologies, I don't follow -- could you elaborate on why an extra note is > not enough? > > If this note is trying to mark the end of the argument setup for those > places you mention, and the start of user code for debug output, and > those are not the same place then I would have thought splitting it > would be necessary. Because other things want to use it as the place to put stack checking, for example. And that cannot be after this note, but it can also not be before it. Is there any reason the stack checking code is inserted way before the prologue/epilogue are, btw? Segher
Re: __attribute__((early_branch))
On Thu, May 02, 2019 at 02:17:51PM +0200, Richard Biener wrote: > On Tue, Apr 30, 2019 at 9:53 PM Jeff Law wrote: > > This is loop unswitching. It's a standard GCC optimization. If it's > > not working as well as it should, we're far better off determining why > > and fixing the automatic transformation rather than relying on > > attributes to drive the transformation. > > It's currently not implemented for switch () stmts, just for conditionals. > This also hurts SPEC cactusADM. There might be a missed-optimization > bug about this. A simple recursive implementation might be possible; > unswitch one case at a time - maybe order by profile probability. We > already recurse on the unswitched bodies (in case multiple conditions > can be unswitched) Well, if for some case value we can prove the controlling expression is constant in the loop, we can almost always prove it is constant without looking at the case value? So we can pull the whole switch statement outside just as easily? Segher
Re: Second GCC 9.1 Release Candidate available from gcc.gnu.org
On Thu, May 02, 2019 at 03:47:33PM +0200, Rainer Orth wrote: > I've now tested this RC on i386-pc-solaris2.1[01] and > sparc-sun-solaris2.1[01]. The only issue (apart from the just-fixed > spellcheck-options-5.c testcase) is > > +FAIL: gcc.target/i386/pr90178.c scan-assembler-times xorl[t > ]*%eax,[t ]*%eax 1 > > While this has already been fixed on mainline, it's still present on the > gcc-9 branch on every x86 target. It's not a regression though (the testcase is new). Segher
Re: Second GCC 9.1 Release Candidate available from gcc.gnu.org
Hi Segher, > On Thu, May 02, 2019 at 03:47:33PM +0200, Rainer Orth wrote: >> I've now tested this RC on i386-pc-solaris2.1[01] and >> sparc-sun-solaris2.1[01]. The only issue (apart from the just-fixed >> spellcheck-options-5.c testcase) is >> >> +FAIL: gcc.target/i386/pr90178.c scan-assembler-times xorl[t >> ]*%eax,[t ]*%eax 1 >> >> While this has already been fixed on mainline, it's still present on the >> gcc-9 branch on every x86 target. > > It's not a regression though (the testcase is new). true, but in a way that's even worse: a new target-specific testcase failing on exactly that target (and a primary platform at that) doesn't seem particularly convincing to me: we should be getting rid of testsuite failures now, not introducing new ones at the 11th hour. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: Second GCC 9.1 Release Candidate available from gcc.gnu.org
On Thu, May 02, 2019 at 08:41:29PM +0200, Rainer Orth wrote: > Hi Segher, > > > On Thu, May 02, 2019 at 03:47:33PM +0200, Rainer Orth wrote: > >> I've now tested this RC on i386-pc-solaris2.1[01] and > >> sparc-sun-solaris2.1[01]. The only issue (apart from the just-fixed > >> spellcheck-options-5.c testcase) is > >> > >> +FAIL: gcc.target/i386/pr90178.c scan-assembler-times xorl[t > >> ]*%eax,[t ]*%eax 1 > >> > >> While this has already been fixed on mainline, it's still present on the > >> gcc-9 branch on every x86 target. > > > > It's not a regression though (the testcase is new). > > true, but in a way that's even worse: a new target-specific testcase > failing on exactly that target (and a primary platform at that) doesn't > seem particularly convincing to me: we should be getting rid of > testsuite failures now, not introducing new ones at the 11th hour. If you prefer to not know about the bug, sure, delete the testcase :-/ The problem is fixed on trunk already, fwiw. Segher
gcc-7-20190502 is now available
Snapshot gcc-7-20190502 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/7-20190502/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 7 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-7-branch revision 270827 You'll find: gcc-7-20190502.tar.xzComplete GCC SHA256=5fbd59be5cf4635cf0a0f6de077ebd2e0e365ff2720a39a776ad6224d534aa5c SHA1=26d97a5f8b607e80884feccc7829bac340d39fc6 Diffs from 7-20190425 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-7 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
