This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug driver/69805] [6 Regression] ICE in greater_than_spec_func, at gcc.c:9722


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69805

--- Comment #3 from vries at gcc dot gnu.org ---
(In reply to Jakub Jelinek from comment #2)
> Created attachment 37698 [details]
> gcc6-pr69805.patch
> 
> Untested fix.
>

As author of the patch that introduces the problem, let me do a review.

> 2016-02-16  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR driver/69805
> 	* gcc.c (LINK_COMMAND_SPEC, GOMP_SELF_SPECS): Use
> 	:%* in %:gt() argument.
> 	(greater_than_spec_func): Adjust for expecting only numbers,
> 	if there are more than two numbers, compare the last two.
> 
> 	* testsuite/libgomp.c/pr69805.c: New test.
> 
> --- gcc/gcc.c.jj	2016-02-15 22:22:51.000000000 +0100
> +++ gcc/gcc.c	2016-02-16 09:35:03.579849080 +0100
> @@ -1019,7 +1019,7 @@ proper position among the other output f
>      %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}} \
>      %{static:} %{L*} %(mfwrap) %(link_libgcc) " \
>      VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %o " CHKP_SPEC " \
> -    %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*} 1):\
> +    %{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):\

For:
...
$ gcc -ftree-parallelize-loops=3 -ftree-parallelize-loops=2
...
this passes:
  ("3", "2", "1")
to greater_than_spec_func, instead of:
  ("-", "ftree-parallelize-loops=3", "-", "ftree-parallelize-loops=2", "1")
.

This changes the semantics of greater_than_spec_func slightly. Strictly
speaking not necessary to fix the ICE. But the new semantics will perhaps be
easier to understand.

Hmm, I see I forgot to document the spec function in doc/invocation.texi.

> @@ -9775,29 +9775,13 @@ greater_than_spec_func (int argc, const
>    if (argc == 1)
>      return NULL;
>  
> -  gcc_assert (argc == 3);
> -  gcc_assert (argv[0][0] == '-');
> -  gcc_assert (argv[0][1] == '\0');
> -
> -  /* Point p to <n> in in -opt=<n>.  */
> -  const char *p = argv[1];
> -  while (true)
> -    {
> -      char c = *p;
> -      if (c == '\0')
> -        gcc_unreachable ();
> +  gcc_assert (argc >= 2);
>  
> -      ++p;
> -
> -      if (c == '=')
> -        break;
> -    }
> -

Due to the new semantics, no longer any need to skip '-opt='.

And 'gcc_assert (argc == 3)' is the assert that triggered. The assert was
incorrect because for spec '%{S*}', gcc 'substitutes all the switches specified
to GCC whose names start with -S', and the code assumed that only the last is
substituted.

Patch looks good to me.

I'll follow up with a documentation patch.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]