Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Segher Boessenkool]

On Fri, Sep 02, 2016 at 05:39:49PM -0500, Segher Boessenkool wrote:
> It seems to be a regression, anyway?  Older versions built fine?

I see this warning, but only in stage1 of the build.


Segher

Re: [PATCH] Move class substring_loc from c-family into gcc

[David Malcolm]

On Fri, 2016-09-02 at 16:55 -0600, Martin Sebor wrote:
> I've successfully tested the patch below by incorporating it into
> the -Wformat-length pass I've been working on.  I'd like to submit
> the latest and hopefully close to final version of my work for
> review without duplicating this code and it might be helpful if
> it was possible to build my latest patch without having to find
> and install this one first.  Could someone review and approve
> David's patch?

I'm not quite sure what the boundaries of my "diagnostics"/"libcpp"
maintainer rights are, and on whether I could self-approve this one -
the addition of the langhook gave me pause.  But
 https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02161.html
suggests that maybe I'm being too reticent, along with your positive
feedback on the patch; it is all about diagnostics infrastructure,
after all.  So if no-one complains I'll commit it early next week.

> Thanks
> Martin
> 
> On 08/25/2016 10:09 AM, David Malcolm wrote:
> > This patch is intended to help Martin's new middle-end sprintf
> > format
> > warning.
> > 
> > It moves most of the on-demand locations-within-strings
> > code in c-family into gcc, into a new substring-locations.c file to
> > go with substring-locations.h: class substring_loc, representing
> > a source caret and source range within a STRING_CST, along with
> > format_warning for handling emitting a warning relating to it.
> > 
> > The actual work of reconstructing the source locations within
> > a string seems inherently frontend-specific, so the patch
> > introduces a
> > langhook to do this, implementing it for C using the existing code
> > (and thus hiding the details of string-concatenation as being
> > specific to the c-family).  Attempts to get substring location
> > from other frontends will fail, and the format_warning_* calls
> > handle such failures gracefully by simply using the location of
> > the string as a whole for the warning.
> > 
> > I've tested it with Martin's gimple-ssa-sprintf patch, and I'm able
> > to
> > emit carets and underlines in the correct places in C code from the
> > middle end with this approach (patch to follow).
> > 
> > Successfully bootstrapped on x86_64-pc-linux-gnu.
> > 
> > OK for trunk?
> > 
> > gcc/ChangeLog:
> > * Makefile.in (OBJS): Add substring-locations.o.
> > * langhooks-def.h (class substring_loc): New forward decl.
> > (lhd_get_substring_location): New decl.
> > (LANG_HOOKS_GET_SUBSTRING_LOCATION): New macro.
> > (LANG_HOOKS_INITIALIZER): Add
> > LANG_HOOKS_GET_SUBSTRING_LOCATION.
> > * langhooks.c (lhd_get_substring_location): New function.
> > * langhooks.h (class substring_loc): New forward decl.
> > (struct lang_hooks): Add field get_substring_location.
> > * substring-locations.c: New file, taking definition of
> > format_warning_va and format_warning_at_substring from
> > c-family/c-format.c, making them non-static.
> > * substring-locations.h: Include .
> > (class substring_loc): Move class here from c-family/c
> > -common.h.
> > Add comments.
> > (format_warning_va): New decl.
> > (format_warning_at_substring): New decl.
> > (get_source_location_for_substring): Add comment.
> > 
> > gcc/c-family/ChangeLog:
> > * c-common.c (get_cpp_ttype_from_string_type): Handle being
> > passed
> > a POINTER_TYPE.
> > (substring_loc::get_location): Move to substring-locations.c,
> > keeping implementation as...
> > (c_get_substring_location): New function, from the above,
> > reworked
> > to use accessors rather than member lookup.
> > * c-common.h (class substring_loc): Move to substring
> > -locations.h,
> > replacing with a forward decl.
> > (c_get_substring_location): New decl.
> > * c-format.c: Include "substring-locations.h".
> > (format_warning_va): Move to substring-locations.c.
> > (format_warning_at_substring): Likewise.
> > 
> > gcc/c/ChangeLog:
> > * c-lang.c (LANG_HOOKS_GET_SUBSTRING_LOCATION): Use
> > c_get_substring_location for this new langhook.
> > 
> > gcc/testsuite/ChangeLog:
> > * gcc.dg/plugin/diagnostic_plugin_test_string_literals.c:
> > Include
> > "substring-locations.h".
> > ---
> >   gcc/Makefile.in|   1 +
> >   gcc/c-family/c-common.c|  23 +--
> >   gcc/c-family/c-common.h|  32 +---
> >   gcc/c-family/c-format.c| 157 +---
> > -
> >   gcc/c/c-lang.c |   3 +
> >   gcc/langhooks-def.h|   8 +-
> >   gcc/langhooks.c|   8 +
> >   gcc/langhooks.h|   9 +
> >   gcc/substring-locations.c  | 195
> > +
> >   gcc/substring-locations.h  |  67 +++
> >   .../diagnostic_plugin_test_string_literals.c   |   

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

On 9/2/16, Segher Boessenkool  wrote:
> On Fri, Sep 02, 2016 at 05:08:34PM -0400, Eric Gallager wrote:
>> > The only way to know for sure what GCC is warning about is to look at
>> > the uninit dump.
>>
>> How exactly do I generate an uninit dump? I'm not seeing any relevant
>> options in the GCC manual...
>
> -fdump-tree-uninit
>


I tried that but it doesn't look like it produced any dumpfiles...
although adding -save-temps to the command gave me an extra 'note'
that wasn't there before:

$ /usr/local/bin/g++ -std=gnu++98 -fno-PIE -c -g -mdynamic-no-pic
-DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables
-fdump-tree-uninit -fdump-tree-uninit-details
-fdump-tree-uninit-blocks-details -fdump-tree-cddce2
-fdump-tree-cddce2-blocks -dA -dD -dP -freport-bug -fsched-verbose=5
-fchecking=2 -save-temps -Wall -Wextra -Wc++11-compat -Wnarrowing
-Wwrite-strings -Wcast-qual -Wno-format -Wmissing-format-attribute
-Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros
-Wno-overlength-strings -Wno-implicit-fallthrough -DHAVE_CONFIG_H -I.
-I. -I../../gcc -I../../gcc/. -I../../gcc/../include
-I../../gcc/../libcpp/include
-I/private/var/root/gcc-git/my_oddly_named_builddir/./gmp
-I/private/var/root/gcc-git/gmp
-I/private/var/root/gcc-git/my_oddly_named_builddir/./mpfr/src
-I/private/var/root/gcc-git/mpfr/src
-I/private/var/root/gcc-git/mpc/src -I../../gcc/../libdecnumber
-I../../gcc/../libdecnumber/dpd -I../libdecnumber
-I../../gcc/../libbacktrace
-I/private/var/root/gcc-git/my_oddly_named_builddir/./isl/include
-I/private/var/root/gcc-git/isl/include -o combine.o -MT combine.o
-MMD -MP -MF ./.deps/combine.TPo
../../gcc/combine.c../../gcc/combine.c: In function ‘int
combine_instructions(rtx_insn*, unsigned int)’:
../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
if ((next = try_combine (insn, prev, NULL, NULL,
^~
../../gcc/combine.c:1125:13: note: ‘prev’ was declared here
   rtx_insn *prev;
 ^~~~

I tried again without '-save-temps' and confirmed that the extra
'note' disappeared again.

>> > Moreover, if the warning is bogus and not a regression, it is very
>> > likely that it is reported already here:
>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
>> >
>> > The effort dedicated to report and analyze the report would be better
>> > spent fixing any of the issues already known.
>> >
>> > Nevertheless, assignments within 'if' are one of the things that make
>> > reading GCC code harder than it needs to be (and combine.c is scary).
>>
>> So would a patch to move the assignment out of the 'if' be better then?
>
> Not really; this idiom is used all over the place in combine (including
> a few times with this same variable!)
>
> It seems to be a regression, anyway?  Older versions built fine?
>
>
> Segher
>


I wasn't really paying as close attention with older versions; I was
only looking at the warnings this time to silence the narrowing ones:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02129.html
...and so I noticed this one and figured I'd try to silence it as
well. It still built fine anyways; it was just a warning and not an
error after all.
(IOW I don't know if this is a regression or not)

Re: [PATCH] Move class substring_loc from c-family into gcc

[Martin Sebor]

I've successfully tested the patch below by incorporating it into
the -Wformat-length pass I've been working on.  I'd like to submit
the latest and hopefully close to final version of my work for
review without duplicating this code and it might be helpful if
it was possible to build my latest patch without having to find
and install this one first.  Could someone review and approve
David's patch?

Thanks
Martin

On 08/25/2016 10:09 AM, David Malcolm wrote:

This patch is intended to help Martin's new middle-end sprintf format
warning.

It moves most of the on-demand locations-within-strings
code in c-family into gcc, into a new substring-locations.c file to
go with substring-locations.h: class substring_loc, representing
a source caret and source range within a STRING_CST, along with
format_warning for handling emitting a warning relating to it.

The actual work of reconstructing the source locations within
a string seems inherently frontend-specific, so the patch introduces a
langhook to do this, implementing it for C using the existing code
(and thus hiding the details of string-concatenation as being
specific to the c-family).  Attempts to get substring location
from other frontends will fail, and the format_warning_* calls
handle such failures gracefully by simply using the location of
the string as a whole for the warning.

I've tested it with Martin's gimple-ssa-sprintf patch, and I'm able to
emit carets and underlines in the correct places in C code from the
middle end with this approach (patch to follow).

Successfully bootstrapped on x86_64-pc-linux-gnu.

OK for trunk?

gcc/ChangeLog:
* Makefile.in (OBJS): Add substring-locations.o.
* langhooks-def.h (class substring_loc): New forward decl.
(lhd_get_substring_location): New decl.
(LANG_HOOKS_GET_SUBSTRING_LOCATION): New macro.
(LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_GET_SUBSTRING_LOCATION.
* langhooks.c (lhd_get_substring_location): New function.
* langhooks.h (class substring_loc): New forward decl.
(struct lang_hooks): Add field get_substring_location.
* substring-locations.c: New file, taking definition of
format_warning_va and format_warning_at_substring from
c-family/c-format.c, making them non-static.
* substring-locations.h: Include .
(class substring_loc): Move class here from c-family/c-common.h.
Add comments.
(format_warning_va): New decl.
(format_warning_at_substring): New decl.
(get_source_location_for_substring): Add comment.

gcc/c-family/ChangeLog:
* c-common.c (get_cpp_ttype_from_string_type): Handle being passed
a POINTER_TYPE.
(substring_loc::get_location): Move to substring-locations.c,
keeping implementation as...
(c_get_substring_location): New function, from the above, reworked
to use accessors rather than member lookup.
* c-common.h (class substring_loc): Move to substring-locations.h,
replacing with a forward decl.
(c_get_substring_location): New decl.
* c-format.c: Include "substring-locations.h".
(format_warning_va): Move to substring-locations.c.
(format_warning_at_substring): Likewise.

gcc/c/ChangeLog:
* c-lang.c (LANG_HOOKS_GET_SUBSTRING_LOCATION): Use
c_get_substring_location for this new langhook.

gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic_plugin_test_string_literals.c: Include
"substring-locations.h".
---
  gcc/Makefile.in|   1 +
  gcc/c-family/c-common.c|  23 +--
  gcc/c-family/c-common.h|  32 +---
  gcc/c-family/c-format.c| 157 +
  gcc/c/c-lang.c |   3 +
  gcc/langhooks-def.h|   8 +-
  gcc/langhooks.c|   8 +
  gcc/langhooks.h|   9 +
  gcc/substring-locations.c  | 195 +
  gcc/substring-locations.h  |  67 +++
  .../diagnostic_plugin_test_string_literals.c   |   1 +
  11 files changed, 308 insertions(+), 196 deletions(-)
  create mode 100644 gcc/substring-locations.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 1b7464b..769efcf 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1444,6 +1444,7 @@ OBJS = \
store-motion.o \
streamer-hooks.o \
stringpool.o \
+   substring-locations.o \
target-globals.o \
targhooks.o \
timevar.o \
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..f3e44c2 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -1122,6 +1122,9 @@ static enum cpp_ttype
  get_cpp_ttype_from_string_type (tree string_type)
  {
gcc_assert (string_type);
+  if (TREE_CODE (string_type) 

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Segher Boessenkool]

On Fri, Sep 02, 2016 at 05:08:34PM -0400, Eric Gallager wrote:
> > The only way to know for sure what GCC is warning about is to look at the
> > uninit dump.
> 
> How exactly do I generate an uninit dump? I'm not seeing any relevant
> options in the GCC manual...

-fdump-tree-uninit

> > Moreover, if the warning is bogus and not a regression, it is very likely
> > that it is reported already here:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
> >
> > The effort dedicated to report and analyze the report would be better spent
> > fixing any of the issues already known.
> >
> > Nevertheless, assignments within 'if' are one of the things that make
> > reading GCC code harder than it needs to be (and combine.c is scary).
> 
> So would a patch to move the assignment out of the 'if' be better then?

Not really; this idiom is used all over the place in combine (including
a few times with this same variable!)

It seems to be a regression, anyway?  Older versions built fine?


Segher

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

On 9/2/16, Manuel López-Ibáñez  wrote:
> On 02/09/16 20:27, Segher Boessenkool wrote:
>> On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:
>>> ../../gcc/combine.c: In function ‘int combine_instructions(rtx_insn*,
>>> unsigned int)’:
>>> ../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
>>> in this function [-Wmaybe-uninitialized]
>>> if ((next = try_combine (insn, prev, NULL, NULL,
>>> ^~
>>
>> That is:
>>
>>   if (HAVE_cc0
>>   && JUMP_P (insn)
>>   && (prev = prev_nonnote_insn (insn)) != 0
>>   && NONJUMP_INSN_P (prev)
>>   && sets_cc0_p (PATTERN (prev)))
>> {
>>   if ((next = try_combine (insn, prev, NULL, NULL,
>>_direct_jump_p,
>>last_combined_insn)) != 0)
>>
>> so prev is always initialised here.  Could you try to find out why GCC
>> warns anyway?  Or open a PR?
>>
>> HAVE_cc0 probably expands to 0 (I'm not sure what your target is), that
>> might have something to do with it.
>
> Unfortunately, the location reported by GCC is sometimes wrong, because
> location tracking in the middle-end is far from perfect.
> https://gcc.gnu.org/PR40635 https://gcc.gnu.org/PR53917
>
> The only way to know for sure what GCC is warning about is to look at the
> uninit dump.


How exactly do I generate an uninit dump? I'm not seeing any relevant
options in the GCC manual...

>
> Moreover, if the warning is bogus and not a regression, it is very likely
> that it is reported already here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
>
> The effort dedicated to report and analyze the report would be better spent
> fixing any of the issues already known.
>
> Nevertheless, assignments within 'if' are one of the things that make
> reading GCC code harder than it needs to be (and combine.c is scary).


So would a patch to move the assignment out of the 'if' be better then?
(Also, what about the one in varasm.c?)

Re: [PATCH, ubsan, obvious] Fix typo in string empty check

[Kirill Yukhin]

On 02.09.2016 23:54, Jakub Jelinek wrote:
Sure, sorry.
gcc/
* ubsan.c (ubsan_use_new_style_p): Fix check for empty string.
--
Thanks, K

Re: [PATCH, IPA] Check pointer for 0 before use in `get_odr_type`

[Kirill Yukhin]

On 02.09.2016 23:56, Jakub Jelinek wrote:

On Fri, Sep 02, 2016 at 11:53:01PM +0300, Kirill Yukhin wrote:

gcc/
* gcc/ipa-devirt.c (get_odr_type): Check odr_types_ptr for
zero before dereferencing it.


I've already tested/posted
http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00089.html
for this.

Okay, disregard then.

--
Thanks, K



Jakub

Re: [PATCH, IPA] Check pointer for 0 before use in `get_odr_type`

[Jakub Jelinek]

On Fri, Sep 02, 2016 at 11:53:01PM +0300, Kirill Yukhin wrote:
> Hello,
> Looks like `get_odr_type ()` contains code which dereferences
> pointer before check it for zero. I moved the line under the check.
> 
> Bootstrap/regtest on x?86|x86_64 in progress.
> 
> Is it ok for trunk if pass?
> 
> gcc/
>   * gcc/ipa-devirt.c (get_odr_type): Check odr_types_ptr for
>   zero before dereferencing it.

I've already tested/posted
http://gcc.gnu.org/ml/gcc-patches/2016-09/msg00089.html
for this.

Jakub

Re: [PATCH, ubsan, obvious] Fix typo in string empty check

[Jakub Jelinek]

On Fri, Sep 02, 2016 at 11:22:24PM +0300, Kirill Yukhin wrote:
> Hello,
> Patch in the bottom fixes typo in check of for string emptiness
> 
> gcc/
> * gcc/ubsan.c (ubsan_use_new_style_p): Fix check for empty string.

No gcc/ in the ChangeLog entry.

> I'll bootstrap/regtest the patch and check it into as obvious if no
> objections.
> 
> --
> Thanks, K
> 
> commit 57ad19906b808386220d628a1ba326e043e0d211
> Author: Kirill Yukhin 
> Date:   Fri Sep 2 23:14:05 2016 +0300
> 
> Compare first element of char* instead of pointer.
> 
> diff --git a/gcc/ubsan.c b/gcc/ubsan.c
> index 5cbc98d..d3bd8e3 100644
> --- a/gcc/ubsan.c
> +++ b/gcc/ubsan.c
> @@ -1469,7 +1469,7 @@ ubsan_use_new_style_p (location_t loc)
> 
>expanded_location xloc = expand_location (loc);
>if (xloc.file == NULL || strncmp (xloc.file, "\1", 2) == 0
> -  || xloc.file == '\0' || xloc.file[0] == '\xff'
> +  || xloc.file[0] == '\0' || xloc.file[0] == '\xff'
>|| xloc.file[1] == '\xff')
>  return false;

Yeah, this is obvious.  You should probably mention
PR other/77421
and perhaps credit also Jonathan who wrote that first, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77421#c3 , dunno why
it hasn't been submitted to gcc-patches.

Jakub

[PATCH, IPA] Check pointer for 0 before use in `get_odr_type`

[Kirill Yukhin]

Hello,
Looks like `get_odr_type ()` contains code which dereferences
pointer before check it for zero. I moved the line under the check.

Bootstrap/regtest on x?86|x86_64 in progress.

Is it ok for trunk if pass?

gcc/
* gcc/ipa-devirt.c (get_odr_type): Check odr_types_ptr for
zero before dereferencing it.

--
Thanks, K

commit 9b822dfb4db14ce762a8d55cf76c677f3fae04bc
Author: Kirill Yukhin 
Date:   Fri Sep 2 23:40:55 2016 +0300

Access odr_type only if odr_type_ptr is not 0.

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 2cf018b..cca912c 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -2132,12 +2132,14 @@ get_odr_type (tree type, bool insert)
 }
   else if (base_id > val->id)
 {
-  odr_types[val->id] = 0;
   /* Be sure we did not recorded any derived types; these may need
 renumbering too.  */
   gcc_assert (val->derived_types.length() == 0);
   if (odr_types_ptr)
-   val->id = odr_types.length ();
+   {
+ odr_types[val->id] = 0;
+ val->id = odr_types.length ();
+   }
   vec_safe_push (odr_types_ptr, val);
 }
   return val;

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Segher Boessenkool]

On Fri, Sep 02, 2016 at 09:31:14PM +0100, Manuel López-Ibáñez wrote:
> Nevertheless, assignments within 'if' are one of the things that make 
> reading GCC code harder than it needs to be (and combine.c is scary).

Yes and yes.  But we really should not warn here.


Segher

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Manuel López-Ibáñez]

On 02/09/16 20:27, Segher Boessenkool wrote:

On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:

../../gcc/combine.c: In function ‘int combine_instructions(rtx_insn*,
unsigned int)’:
../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
if ((next = try_combine (insn, prev, NULL, NULL,
^~


That is:

  if (HAVE_cc0
  && JUMP_P (insn)
  && (prev = prev_nonnote_insn (insn)) != 0
  && NONJUMP_INSN_P (prev)
  && sets_cc0_p (PATTERN (prev)))
{
  if ((next = try_combine (insn, prev, NULL, NULL,
   _direct_jump_p,
   last_combined_insn)) != 0)

so prev is always initialised here.  Could you try to find out why GCC
warns anyway?  Or open a PR?

HAVE_cc0 probably expands to 0 (I'm not sure what your target is), that
might have something to do with it.


Unfortunately, the location reported by GCC is sometimes wrong, because 
location tracking in the middle-end is far from perfect. 
https://gcc.gnu.org/PR40635 https://gcc.gnu.org/PR53917


The only way to know for sure what GCC is warning about is to look at the 
uninit dump.


Moreover, if the warning is bogus and not a regression, it is very likely that 
it is reported already here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639


The effort dedicated to report and analyze the report would be better spent 
fixing any of the issues already known.


Nevertheless, assignments within 'if' are one of the things that make reading 
GCC code harder than it needs to be (and combine.c is scary).


Cheers,

Manuel.

[PATCH, ubsan, obvious] Fix typo in string empty check

[Kirill Yukhin]

Hello,
Patch in the bottom fixes typo in check of for string emptiness

gcc/
* gcc/ubsan.c (ubsan_use_new_style_p): Fix check for empty string.

I'll bootstrap/regtest the patch and check it into as obvious if no 
objections.


--
Thanks, K

commit 57ad19906b808386220d628a1ba326e043e0d211
Author: Kirill Yukhin 
Date:   Fri Sep 2 23:14:05 2016 +0300

Compare first element of char* instead of pointer.

diff --git a/gcc/ubsan.c b/gcc/ubsan.c
index 5cbc98d..d3bd8e3 100644
--- a/gcc/ubsan.c
+++ b/gcc/ubsan.c
@@ -1469,7 +1469,7 @@ ubsan_use_new_style_p (location_t loc)

   expanded_location xloc = expand_location (loc);
   if (xloc.file == NULL || strncmp (xloc.file, "\1", 2) == 0
-  || xloc.file == '\0' || xloc.file[0] == '\xff'
+  || xloc.file[0] == '\0' || xloc.file[0] == '\xff'
   || xloc.file[1] == '\xff')
 return false;

[PATCH] PR fortran/77460

[Steve Kargl]

I plan to commit the following patch in the next day or two.
Objections?

2016-09-03  Steven G. Kargl  

PR fortran/77460
* simplify.c (simplify_transformation_to_scalar):  On error, result
may be NULL, simply return.

2016-09-03  Steven G. Kargl  

PR fortran/77460
* gfortran.dg/pr77460.f90: New test.


Index: gcc/fortran/simplify.c
===
--- gcc/fortran/simplify.c  (revision 239797)
+++ gcc/fortran/simplify.c  (working copy)
@@ -489,6 +489,8 @@ simplify_transformation_to_scalar (gfc_e
}
 
   result = op (result, gfc_copy_expr (a));
+  if (!result)
+   return result;
 }
 
   return result;
Index: gcc/testsuite/gfortran.dg/pr77460.f90
===
--- gcc/testsuite/gfortran.dg/pr77460.f90   (nonexistent)
+++ gcc/testsuite/gfortran.dg/pr77460.f90   (working copy)
@@ -0,0 +1,7 @@
+! { dg-do compile }
+  double precision, parameter :: x = huge(1d0)
+  print*, sum((/x,-x/))
+  print*, sum((/x,x,-x,-x/))  ! { dg-error "overflow" }
+  print*, sum((/x,-x,1d0/))
+  print*, sum((/1d0,x,-x/))
+end

-- 
Steve

Re: [PATCH, obvious] Remove double condition from dwarf2out.c

[Kirill Yukhin]

On 02.09.2016 20:00, Jakub Jelinek wrote:

On Fri, Sep 02, 2016 at 07:52:45PM +0300, Kirill Yukhin wrote:

Hi,
Remove identical conditions from AND
in return.
Will check-in after bootstrap/regtest on i386|x86_64 as obvious.

gcc/
 * dwarf2out.c (dw_val_equal_p): Remove redundant condition in
 return statement.

This isn't obvious, val_vms_delta uses both .lbl1 and .lbl2, for equality
I bet you want to compare both.  I.e.
return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)
- && !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1));
+ && !strcmp (a->v.val_vms_delta.lbl2, b->v.val_vms_delta.lbl2));

Jakub

Thanks, reg-testing the patch.

--
Thanks, K

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

On 9/2/16, David Malcolm  wrote:
> On Fri, 2016-09-02 at 15:41 -0400, Eric Gallager wrote:
>> On 9/2/16, Segher Boessenkool  wrote:
>> > On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:
>> > > ../../gcc/combine.c: In function ‘int
>> > > combine_instructions(rtx_insn*,
>> > > unsigned int)’:
>> > > ../../gcc/combine.c:1310:8: warning: ‘prev’ may be used
>> > > uninitialized
>> > > in this function [-Wmaybe-uninitialized]
>> > > if ((next = try_combine (insn, prev, NULL, NULL,
>> > > ^~
>> >
>> > That is:
>> >
>> >   if (HAVE_cc0
>> >   && JUMP_P (insn)
>> >   && (prev = prev_nonnote_insn (insn)) != 0
>> >   && NONJUMP_INSN_P (prev)
>> >   && sets_cc0_p (PATTERN (prev)))
>> > {
>> >   if ((next = try_combine (insn, prev, NULL, NULL,
>> >_direct_jump_p,
>> >last_combined_insn)) != 0)
>> >
>> > so prev is always initialised here.  Could you try to find out why
>> > GCC
>> > warns anyway?  Or open a PR?
>> >
>> > HAVE_cc0 probably expands to 0 (I'm not sure what your target is),
>> > that
>> > might have something to do with it.
>> >
>> >
>> > Segher
>> >
>>
>>
>> My target is i386-apple-darwin9.8.0. If HAVE_cc0 expands to 0, then
>> the code inside the brackets wouldn't even be reached anyways, would
>> it? Where would HAVE_cc0 be defined?
>
> It's defined in insn-config.h, which is generated in build/gcc by
> genconfig from the target's machine description file.
>
>


Okay, yeah, checking there, it has:

#define HAVE_cc0 0

on line 12. So Segher's intuition was correct.
(I'm attaching that whole file for reference)
/* Generated automatically by the program `genconfig'
   from the machine description file `md'.  */

#ifndef GCC_INSN_CONFIG_H
#define GCC_INSN_CONFIG_H

#define MAX_RECOG_OPERANDS 30
#define MAX_DUP_OPERANDS 30
#ifndef MAX_INSNS_PER_SPLIT
#define MAX_INSNS_PER_SPLIT 5
#endif
#define HAVE_cc0 0
#define CC0_P(X) ((X) ? 0 : 0)
#define HAVE_conditional_move 1
#define HAVE_conditional_execution 0
#define HAVE_lo_sum 0
#define HAVE_rotate 1
#define HAVE_rotatert 1
#define HAVE_peephole 0
#define HAVE_peephole2 1
#define MAX_INSNS_PER_PEEP2 4

#endif /* GCC_INSN_CONFIG_H */

Re: PR35503 - warn for restrict pointer

[Manuel López-Ibáñez]

On 02/09/16 18:44, David Malcolm wrote:

Much better would be to have the formatting be done inside the
diagnostics subsystem's call into pp_format, with something like this:

  warning_at_rich_loc_n (, OPT_Wrestrict,
 arg_positions
.length (),
 "passing argument %i to restrict
-qualified"
 " parameter aliases with argument
%FIXME",
 "passing argument %i to restrict
-qualified"
 " parameter aliases with arguments
%FIXME",
 param_pos + 1,

 _positions);


Yes, building up diagnostic messages from pieces is discouraged: 
https://gcc.gnu.org/codingconventions.html#Diagnostics



and have %FIXME (or somesuch) consume _positions in the va_arg,
printing the argument numbers there.  Doing it this way also avoids
building the string for the case where Wrestrict is disabled, since the
pp_format only happens after we know we're going to print the warning.


Is it possible to pass template arguments through ... ? And how does va_arg 
know the type of the particular template passed?



Assuming that there isn't yet a pre-canned way to print a set of
argument numbers that I've missed, the place to add the %FIXME-handling
would presumably be in default_tree_printer in tree-diagnostic.c -
though it's obviously nothing to do with trees. (Or if this is too
single-purpose, perhaps there's a need to temporarily inject one-time
callbacks for consuming custom args??).


I'm surprised we don't have a function pp_vec to print/debug a vec<>, but 
perhaps it is simpler to convert arg_pos to a 'char *' and use %s instead of 
%FIXME or call-backs.


Cheers,

Manuel.

Re: [PATCH 4/4] (v2) Add -fdiagnostics-generate-patch

[David Malcolm]

On Wed, 2016-08-24 at 21:13 -0400, David Malcolm wrote:
> Changed in v2: I dropped -fdiagnostics-apply-fixits
> 
> This patch uses the edit_context machinery to provide a new
> -fdiagnostics-generate-patch option.
> 
> If set an edit_context is created for global_dc, and any
> fix-it hints emitted by diagnostics are added to the edit_context.
> 
> -fdiagnostics-generate-patch writes out a patch to stderr in unified
> diff format.  This is potentially color-coded, following the same
> rules
> as for diagnostics (and affected by -fdiagnostics-color).  The color
> scheme mimics that of git-diff.
> gcc/ChangeLog:
>   * common.opt (fdiagnostics-generate-patch): New option.
>   * diagnostic.c: Include "edit-context.h".
>   (diagnostic_initialize): Initialize context->edit_context_ptr.
>   (diagnostic_finish): Delete context->edit_context_ptr.
>   (diagnostic_report_diagnostic): Add fix-it hints from the
>   diagnostic to context->edit_context_ptr, if any.
>   * diagnostic.h (class edit_context): Add forward decl.
>   (struct diagnostic_context): Add field "edit_context_ptr".
>   * doc/invoke.texi (Diagnostic Message Formatting Options): Add
>   -fdiagnostics-generate-patch.
>   (-fdiagnostics-generate-patch): New item.
>   * toplev.c: Include "edit-context.h".
>   (process_options): Set global_dc->edit_context_ptr to a new
>   edit_context if the options need one.
>   (toplev::main): Handle -fdiagnostics-generate-patch by using
>   global_dc->edit_context_ptr.
> 
> gcc/testsuite/ChangeLog:
>   * gcc.dg/plugin/diagnostic-test-show-locus-generate-patch.c:
> New
>   test case.
>   * gcc.dg/plugin/plugin.exp (plugin_test_list): Add
>   diagnostic-test-show-locus-generate-patch.c to the sources
>   for diagnostic_plugin_test_show_locus.c.


The prerequisites are in, so I've committed this to trunk (as r239965), having 
verified bootstrap on x86_64-pc-linux-gnu.

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[David Malcolm]

On Fri, 2016-09-02 at 15:41 -0400, Eric Gallager wrote:
> On 9/2/16, Segher Boessenkool  wrote:
> > On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:
> > > ../../gcc/combine.c: In function ‘int
> > > combine_instructions(rtx_insn*,
> > > unsigned int)’:
> > > ../../gcc/combine.c:1310:8: warning: ‘prev’ may be used
> > > uninitialized
> > > in this function [-Wmaybe-uninitialized]
> > > if ((next = try_combine (insn, prev, NULL, NULL,
> > > ^~
> > 
> > That is:
> > 
> >   if (HAVE_cc0
> >   && JUMP_P (insn)
> >   && (prev = prev_nonnote_insn (insn)) != 0
> >   && NONJUMP_INSN_P (prev)
> >   && sets_cc0_p (PATTERN (prev)))
> > {
> >   if ((next = try_combine (insn, prev, NULL, NULL,
> >_direct_jump_p,
> >last_combined_insn)) != 0)
> > 
> > so prev is always initialised here.  Could you try to find out why
> > GCC
> > warns anyway?  Or open a PR?
> > 
> > HAVE_cc0 probably expands to 0 (I'm not sure what your target is),
> > that
> > might have something to do with it.
> > 
> > 
> > Segher
> > 
> 
> 
> My target is i386-apple-darwin9.8.0. If HAVE_cc0 expands to 0, then
> the code inside the brackets wouldn't even be reached anyways, would
> it? Where would HAVE_cc0 be defined?

It's defined in insn-config.h, which is generated in build/gcc by
genconfig from the target's machine description file.

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

On 9/2/16, Segher Boessenkool  wrote:
> On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:
>> ../../gcc/combine.c: In function ‘int combine_instructions(rtx_insn*,
>> unsigned int)’:
>> ../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
>> in this function [-Wmaybe-uninitialized]
>> if ((next = try_combine (insn, prev, NULL, NULL,
>> ^~
>
> That is:
>
>   if (HAVE_cc0
>   && JUMP_P (insn)
>   && (prev = prev_nonnote_insn (insn)) != 0
>   && NONJUMP_INSN_P (prev)
>   && sets_cc0_p (PATTERN (prev)))
> {
>   if ((next = try_combine (insn, prev, NULL, NULL,
>_direct_jump_p,
>last_combined_insn)) != 0)
>
> so prev is always initialised here.  Could you try to find out why GCC
> warns anyway?  Or open a PR?
>
> HAVE_cc0 probably expands to 0 (I'm not sure what your target is), that
> might have something to do with it.
>
>
> Segher
>


My target is i386-apple-darwin9.8.0. If HAVE_cc0 expands to 0, then
the code inside the brackets wouldn't even be reached anyways, would
it? Where would HAVE_cc0 be defined?

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Segher Boessenkool]

On Fri, Sep 02, 2016 at 02:21:07PM -0400, Eric Gallager wrote:
> ../../gcc/combine.c: In function ‘int combine_instructions(rtx_insn*,
> unsigned int)’:
> ../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
> in this function [-Wmaybe-uninitialized]
> if ((next = try_combine (insn, prev, NULL, NULL,
> ^~

That is:

  if (HAVE_cc0
  && JUMP_P (insn)
  && (prev = prev_nonnote_insn (insn)) != 0
  && NONJUMP_INSN_P (prev)
  && sets_cc0_p (PATTERN (prev)))
{
  if ((next = try_combine (insn, prev, NULL, NULL,
   _direct_jump_p,
   last_combined_insn)) != 0)

so prev is always initialised here.  Could you try to find out why GCC
warns anyway?  Or open a PR?

HAVE_cc0 probably expands to 0 (I'm not sure what your target is), that
might have something to do with it.


Segher

Re: [patch, libstdc++] std::shuffle: Generate two swap positions at a time if possible

[Eelis]

On 2016-09-02 20:20, Eelis van der Weegen wrote:

On 2016-08-31 14:45, Jonathan Wakely wrote:

Is this significantly faster than just using
uniform_int_distribution<_IntType>{0, __bound - 1}(__g) so we don't
need to duplicate the logic? (And people maintaining the code won't
reconvince themselves it's correct every time they look at it :-)

[..]

Could we hoist this test out of the loop somehow?

If we change the loop condition to be __i+1 < __last we don't need to
test it on every iteration, and then after the loop we can just do
the final swap if (__urange % 2).


Reusing std::uniform_int_distribution seems just as fast, so I've removed 
__generate_random_index_below.

I've hoisted the (__i + 1 == __last) check out of the loop (in a slightly 
different way), and it seems to shave off a couple more cycles, yay!

Updated patch attached.



Please ignore that patch, I used __g()&1 but that's invalid (the new 
"UniformRandomBitGenerator" name is misleading).

Updated patch (which uses a proper distribution even for the [0,1] case) 
attached.
Index: libstdc++-v3/include/bits/stl_algo.h
===
--- libstdc++-v3/include/bits/stl_algo.h	(revision 239895)
+++ libstdc++-v3/include/bits/stl_algo.h	(working copy)
@@ -3772,6 +3772,47 @@
   typedef typename std::make_unsigned<_DistanceType>::type __ud_type;
   typedef typename std::uniform_int_distribution<__ud_type> __distr_type;
   typedef typename __distr_type::param_type __p_type;
+
+  typedef typename std::remove_reference<_UniformRandomNumberGenerator>::type _Gen;
+  typedef typename std::common_type::type __uc_type;
+
+  const __uc_type __urngrange = __g.max() - __g.min();
+  const __uc_type __urange = __uc_type(__last - __first);
+
+  if (__urngrange / __urange >= __urange)
+// I.e. (__urngrange >= __urange * __urange) but without wrap issues.
+  {
+	_RandomAccessIterator __i = __first + 1;
+
+	// Since we know the range isn't empty, an even number of elements
+	// means an uneven number of elements /to swap/, in which case we
+	// do the first one up front:
+
+	if ((__urange % 2) == 0)
+	{
+	  __distr_type __d{0, 1};
+	  std::iter_swap(__i++, __first + __d(__g));
+	}
+
+	// Now we know that __last - __i is even, so we do the rest in pairs,
+	// using a single distribution invocation to produce swap positions
+	// for two successive elements at a time:
+
+	while (__i != __last)
+	{
+	  const __uc_type __swap_range = __uc_type(__i - __first) + 1;
+	  const __uc_type __comp_range = __swap_range * (__swap_range + 1);
+
+	  std::uniform_int_distribution<__uc_type> __d{0, __comp_range - 1};
+	  const __uc_type __pospos = __d(__g);
+
+	  std::iter_swap(__i++, __first + (__pospos % __swap_range));
+	  std::iter_swap(__i++, __first + (__pospos / __swap_range));
+	}
+
+	return;
+  }
+
   __distr_type __d;
 
   for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)

[PATCH] Add a warning for suspicious use of conditional expressions in boolean context

[Bernd Edlinger]

Hi!

As reported in PR77434 and PR77421 there should be a warning for
suspicious uses of conditional expressions with non-boolean arguments.

This warning triggers on conditional expressions in boolean context,
when both possible results are non-zero integer constants, so that
the resulting truth value does in fact not depend on the condition
itself.  Thus something like "if (a == b ? 1 : 2)" is always bogus,
and was most likely meant to be "if (a == (b ? 1 : 2))".


Boot-strap and reg-testing on x86_64-pc-linux-gnu without regressions.
Is it OK for trunk.


Thanks
Bernd.
gcc:
2016-09-02  Bernd Edlinger  

PR c++/77434
* doc/invoke.texi: Document -Wcond-in-bool-context.

PR middle-end/77421
* dwarf2out.c (output_loc_operands): Fix assertion.

c-family:
2016-09-02  Bernd Edlinger  

PR c++/77434
* c.opt (Wcond-in-bool-context): New warning.
* c-common.c (c_common_truthvalue_conversion): Warn on integer
constants in boolean context.

testsuite:
2016-09-02  Bernd Edlinger  

PR c++/77434
* c-c++-common/Wcond-in-bool-context.c: New test.
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c	(revision 239953)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4618,6 +4618,14 @@ c_common_truthvalue_conversion (location_t locatio
 	   TREE_OPERAND (expr, 0));
 
 case COND_EXPR:
+  if (TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST
+	  && TREE_CODE (TREE_OPERAND (expr, 2)) == INTEGER_CST
+	  && !integer_zerop (TREE_OPERAND (expr, 1))
+	  && !integer_zerop (TREE_OPERAND (expr, 2))
+	  && (!integer_onep (TREE_OPERAND (expr, 1))
+	  || !integer_onep (TREE_OPERAND (expr, 2
+	warning_at (EXPR_LOCATION (expr), OPT_Wcond_in_bool_context,
+		"?: using integer constants in boolean context");
   /* Distribute the conversion into the arms of a COND_EXPR.  */
   if (c_dialect_cxx ())
 	{
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt	(revision 239953)
+++ gcc/c-family/c.opt	(working copy)
@@ -350,6 +350,10 @@ Wcomments
 C ObjC C++ ObjC++ Warning Alias(Wcomment)
 Synonym for -Wcomment.
 
+Wcond-in-bool-context
+C ObjC C++ ObjC++ Var(warn_cond_in_bool_context) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn for conditional expressions (?:) using integer constants in boolean context.
+
 Wconditionally-supported
 C++ ObjC++ Var(warn_conditionally_supported) Warning
 Warn for conditionally-supported constructs.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(revision 239953)
+++ gcc/doc/invoke.texi	(working copy)
@@ -259,7 +259,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
 -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
--Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
+-Wchar-subscripts -Wclobbered -Wcomment @gol
+-Wcond-in-bool-context -Wconditionally-supported  @gol
 -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
 -Wdelete-incomplete @gol
 -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
@@ -5179,6 +5180,13 @@ programs.
 Warn for variables that might be changed by @code{longjmp} or
 @code{vfork}.  This warning is also enabled by @option{-Wextra}.
 
+@item -Wcond-in-bool-context
+@opindex Wcond-in-bool-context
+@opindex Wno-cond-in-bool-context
+Warn for conditional expressions (?:) using non-boolean integer constants in
+boolean context, like @code{if (a <= b ? 2 : 3)}.  This warning is enabled
+by @option{-Wall}.
+
 @item -Wconditionally-supported @r{(C++ and Objective-C++ only)}
 @opindex Wconditionally-supported
 @opindex Wno-conditionally-supported
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c	(revision 239953)
+++ gcc/dwarf2out.c	(working copy)
@@ -2051,9 +2051,9 @@ output_loc_operands (dw_loc_descr_ref loc, int for
 	/* Make sure the offset has been computed and that we can encode it as
 	   an operand.  */
 	gcc_assert (die_offset > 0
-		&& die_offset <= (loc->dw_loc_opc == DW_OP_call2)
+		&& die_offset <= (loc->dw_loc_opc == DW_OP_call2
  ? 0x
- : 0x);
+ : 0x));
 	dw2_asm_output_data ((loc->dw_loc_opc == DW_OP_call2) ? 2 : 4,
 			 die_offset, NULL);
   }
Index: gcc/testsuite/c-c++-common/Wcond-in-bool-context.c
===
--- gcc/testsuite/c-c++-common/Wcond-in-bool-context.c	(revision 0)
+++ gcc/testsuite/c-c++-common/Wcond-in-bool-context.c	(working copy)
@@ -0,0 +1,17 @@
+/* PR c++/77434 */
+/* { dg-options 

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

On 9/2/16, Segher Boessenkool  wrote:
> On Fri, Sep 02, 2016 at 09:40:36AM -0400, Eric Gallager wrote:
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -1122,7 +1122,7 @@ static int
>>  combine_instructions (rtx_insn *f, unsigned int nregs)
>>  {
>>rtx_insn *insn, *next;
>> -  rtx_insn *prev;
>> +  rtx_insn *prev = NULL;
>>struct insn_link *links, *nextlinks;
>>rtx_insn *first;
>>basic_block last_bb;
>
> I don't see any place "prev" is used witout an immediately preceding
> assignment to it.  Could you show the warning you see please?
>
>
> Segher
>


Sure. When I remove the initialization I get:


../../gcc/combine.c: In function ‘int combine_instructions(rtx_insn*,
unsigned int)’:
../../gcc/combine.c:1310:8: warning: ‘prev’ may be used uninitialized
in this function [-Wmaybe-uninitialized]
if ((next = try_combine (insn, prev, NULL, NULL,
^~

Re: [patch, libstdc++] std::shuffle: Generate two swap positions at a time if possible

[Eelis van der Weegen]

On 2016-08-31 14:45, Jonathan Wakely wrote:

Is this significantly faster than just using
uniform_int_distribution<_IntType>{0, __bound - 1}(__g) so we don't
need to duplicate the logic? (And people maintaining the code won't
reconvince themselves it's correct every time they look at it :-)

[..]

Could we hoist this test out of the loop somehow?

If we change the loop condition to be __i+1 < __last we don't need to
test it on every iteration, and then after the loop we can just do
the final swap if (__urange % 2).


Reusing std::uniform_int_distribution seems just as fast, so I've removed 
__generate_random_index_below.

I've hoisted the (__i + 1 == __last) check out of the loop (in a slightly 
different way), and it seems to shave off a couple more cycles, yay!

Updated patch attached.

Index: libstdc++-v3/include/bits/stl_algo.h
===
--- libstdc++-v3/include/bits/stl_algo.h	(revision 239895)
+++ libstdc++-v3/include/bits/stl_algo.h	(working copy)
@@ -3772,6 +3772,46 @@
   typedef typename std::make_unsigned<_DistanceType>::type __ud_type;
   typedef typename std::uniform_int_distribution<__ud_type> __distr_type;
   typedef typename __distr_type::param_type __p_type;
+
+  typedef typename std::remove_reference<_UniformRandomNumberGenerator>::type _Gen;
+  typedef typename std::common_type::type __uc_type;
+
+  const __uc_type __urngrange = __g.max() - __g.min();
+  const __uc_type __urange = __uc_type(__last - __first);
+
+  if (__urngrange / __urange >= __urange)
+// I.e. (__urngrange >= __urange * __urange) but without wrap issues.
+  {
+	_RandomAccessIterator __i = __first + 1;
+
+	// Since we know the range isn't empty, an even number of elements
+	// means an uneven number of elements /to swap/, in which case we
+	// do the first one up front:
+
+	if ((__urange % 2) == 0)
+	{
+	  std::iter_swap(__i++, __first + (__g() & 1));
+	}
+
+	// Now we know that __last - __i is even, so we do the rest in pairs,
+	// using a single distribution invocation to produce swap positions
+	// for two successive elements at a time:
+
+	while (__i != __last)
+	{
+	  const __uc_type __swap_range = __uc_type(__i - __first) + 1;
+	  const __uc_type __comp_range = __swap_range * (__swap_range + 1);
+
+	  std::uniform_int_distribution<__uc_type> __d{0, __comp_range - 1};
+	  const __uc_type __pospos = __d(__g);
+
+	  std::iter_swap(__i++, __first + (__pospos % __swap_range));
+	  std::iter_swap(__i++, __first + (__pospos / __swap_range));
+	}
+
+	return;
+  }
+
   __distr_type __d;
 
   for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)

[committed] Introduce class edit_context (v3)

[David Malcolm]

Changes in v3:
- Updated doc/invoke.texi.
- Renamed line_state to edited_line
- I rewrote content-tracking in terms of edited lines, rather than
tracking the whole file.  This avoids the need to load the whole file,
instead fetching it as needed from input.c's cache.  This required a
little fiddly code to handle empty files, and whether or not a file
has a trailing newline.
- Added validation of columns, and rejection of rich_locations that have
rejected fix-it hints.

Successfully bootstrapped on x86_64-pc-linux-gnu.

Committed to trunk as r239963.

gcc/ChangeLog:
* Makefile.in (OBJS-libcommon): Add edit-context.o.
* diagnostic-color.c (color_dict): Add "diff-filename",
"diff-hunk", "diff-delete", and "diff-insert".
(parse_gcc_colors): Update default value of GCC_COLORS in comment
to reflect above changes.
* doc/invoke.texi (-fdiagnostics-color): Update description of
default GCC_COLORS, and of the supported capabilities.
* edit-context.c: New file.
* edit-context.h: New file.
* input.c (struct fcache): Add field "missing_trailing_newline".
(diagnostics_file_cache_forcibly_evict_file): Initialize it to
true.
(add_file_to_cache_tab): Likewise.
(fcache::fcache): Likewise.
(get_next_line): Update c->missing_trailing_newline.
(location_missing_trailing_newline): New function.
* input.h (location_missing_trailing_newline): New decl.
* selftest-run-tests.c (selftest::run_tests): Call
edit_context_c_tests.
* selftest.h (edit_context_c_tests): New decl.

libcpp/ChangeLog:
* include/line-map.h (rich_location::seen_impossible_fixit_p): New
accessor.
---
 gcc/Makefile.in   |1 +
 gcc/diagnostic-color.c|7 +-
 gcc/doc/invoke.texi   |   20 +-
 gcc/edit-context.c| 1511 +
 gcc/edit-context.h|   68 ++
 gcc/input.c   |   47 +-
 gcc/input.h   |1 +
 gcc/selftest-run-tests.c  |1 +
 gcc/selftest.h|1 +
 libcpp/include/line-map.h |1 +
 10 files changed, 1647 insertions(+), 11 deletions(-)
 create mode 100644 gcc/edit-context.c
 create mode 100644 gcc/edit-context.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b38a0c1..7c18285 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1561,6 +1561,7 @@ OBJS = \
 # Objects in libcommon.a, potentially used by all host binaries and with
 # no target dependencies.
 OBJS-libcommon = diagnostic.o diagnostic-color.o diagnostic-show-locus.o \
+   edit-context.o \
pretty-print.o intl.o \
vec.o input.o version.o hash-table.o ggc-none.o memory-block.o \
selftest.o
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index 42aa1b6..0bd8170 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -170,6 +170,10 @@ static struct color_cap color_dict[] =
   { "quote", SGR_SEQ (COLOR_BOLD), 5, false },
   { "fixit-insert", SGR_SEQ (COLOR_FG_GREEN), 12, false },
   { "fixit-delete", SGR_SEQ (COLOR_FG_RED), 12, false },
+  { "diff-filename", SGR_SEQ (COLOR_BOLD), 13, false },
+  { "diff-hunk", SGR_SEQ (COLOR_FG_CYAN), 9, false },
+  { "diff-delete", SGR_SEQ (COLOR_FG_RED), 11, false },
+  { "diff-insert", SGR_SEQ (COLOR_FG_GREEN), 11, false },
   { NULL, NULL, 0, false }
 };
 
@@ -200,7 +204,8 @@ colorize_stop (bool show_color)
 /* Parse GCC_COLORS.  The default would look like:
GCC_COLORS='error=01;31:warning=01;35:note=01;36:\
range1=32:range2=34:locus=01:quote=01:\
-   fixit-insert=32:fixit-delete=31'
+   fixit-insert=32:fixit-delete=31'\
+   diff-filename=01:diff-hunk=32:diff-delete=31:diff-insert=32'
No character escaping is needed or supported.  */
 static bool
 parse_gcc_colors (void)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 87da1f1..986ab43 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3343,7 +3343,9 @@ for 88-color and 256-color modes background colors.
 
 The default @env{GCC_COLORS} is
 @smallexample
-error=01;31:warning=01;35:note=01;36:range1=32:range2=34:locus=01:quote=01:fixit-insert=32:fixit-delete=31
+error=01;31:warning=01;35:note=01;36:range1=32:range2=34:locus=01:quote=01:\
+fixit-insert=32:fixit-delete=31:\
+diff-filename=01:diff-hunk=32:diff-delete=31:diff-insert=32
 @end smallexample
 @noindent
 where @samp{01;31} is bold red, @samp{01;35} is bold magenta,
@@ -3391,6 +3393,22 @@ be inserted or replaced.
 @vindex fixit-delete GCC_COLORS @r{capability}
 SGR substring for fix-it hints suggesting text to
 be deleted.
+
+@item diff-filename=
+@vindex diff-filename GCC_COLORS @r{capability}
+SGR substring for filename headers within generated patches.
+
+@item diff-hunk=
+@vindex diff-hunk GCC_COLORS @r{capability}
+SGR substring for the starts of hunks within generated patches.
+
+@item diff-delete=
+@vindex diff-delete GCC_COLORS @r{capability}
+SGR substring for deleted 

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Thu, 2016-09-01 at 14:55 +0530, Prathamesh Kulkarni wrote:

[...]

> The attached version passes bootstrap+test on ppc64le-linux-gnu.
> Given that it only looks if parameters are restrict qualified and not
> how they're used inside the callee,
> this can have false positives as in above test-cases.
> Should the warning be put in Wextra rather than Wall (I have left it
> in Wall in the patch)  or only enabled with -Wrestrict ?
> 
> Thanks,
> Prathamesh
> > 
> > Richard.


diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..a3dae34 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "substring-locations.h"
 #include "spellcheck.h"
+#include "gcc-rich-location.h"
 
 cpp_reader *parse_in;  /* Declared in c-pragma.h.  */
 
@@ -13057,4 +13058,76 @@ diagnose_mismatched_attributes (tree olddecl,
tree newdecl)
   return warned;
 }
 
+/* Warn if an argument at position param_pos is passed to a
+   restrict-qualified param, and it aliases with another argument.  */
+
+void
+warn_for_restrict (unsigned param_pos, vec *args)
+{
+  tree arg = (*args)[param_pos];
+  if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node,
0))
+return;
+
+  location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
+  gcc_rich_location richloc (loc);
+
+  unsigned i;
+  tree current_arg;
+  auto_vec arg_positions;
+
+  FOR_EACH_VEC_ELT (*args, i, current_arg) 
+{
+  if (i == param_pos)
+   continue;
+
+  tree current_arg = (*args)[i];
+  if (operand_equal_p (arg, current_arg, 0))
+   {
+ TREE_VISITED (current_arg) = 1; 
+ arg_positions.safe_push (i);
+   }
+}
+
+  if (arg_positions.is_empty ())
+return;
+
+  struct obstack fmt_obstack;
+  gcc_obstack_init (_obstack);
+  char *fmt = (char *) obstack_alloc (_obstack, 0);
+
+  char num[32];
+  sprintf (num, "%u", param_pos + 1);
+
+  obstack_grow (_obstack, "passing argument ",
+   strlen ("passing argument "));
+  obstack_grow (_obstack, num, strlen (num));
+  obstack_grow (_obstack,
+   " to restrict-qualified parameter aliases with
argument",
+   strlen (" to restrict-qualified parameter "
+   "aliases with argument"));
+
+  /* make argument plural and append space.  */
+  if (arg_positions.length () > 1)
+obstack_1grow (_obstack, 's');
+  obstack_1grow (_obstack, ' ');
+
+  unsigned pos;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+{
+  tree arg = (*args)[pos];
+  if (EXPR_HAS_LOCATION (arg))
+   richloc.add_range (EXPR_LOCATION (arg), false);
+
+  sprintf (num, "%u", pos + 1);
+  obstack_grow (_obstack, num, strlen (num));
+
+  if (i < arg_positions.length () - 1)
+   obstack_grow (_obstack, ", ",  strlen (", "));
+}
+
+  obstack_1grow (_obstack, 0);
+  warning_at_rich_loc (, OPT_Wrestrict, fmt);
+  obstack_free (_obstack, fmt);
+}

Thanks for working on this.

I'm not a fan of how the patch builds "fmt" here.  If nothing else,
this perhaps should be:

  warning_at_rich_loc (, OPT_Wrestrict, "%s", fmt);

but building up strings like the patch does makes localization
difficult.

Much better would be to have the formatting be done inside the
diagnostics subsystem's call into pp_format, with something like this:

  warning_at_rich_loc_n (, OPT_Wrestrict,
 arg_positions
.length (),
 "passing argument %i to restrict
-qualified"
 " parameter aliases with argument
%FIXME",
 "passing argument %i to restrict
-qualified"
 " parameter aliases with arguments
%FIXME",
 param_pos + 1,

 _positions);


and have %FIXME (or somesuch) consume _positions in the va_arg,
printing the argument numbers there.  Doing it this way also avoids
building the string for the case where Wrestrict is disabled, since the
pp_format only happens after we know we're going to print the warning.

Assuming that there isn't yet a pre-canned way to print a set of
argument numbers that I've missed, the place to add the %FIXME-handling
would presumably be in default_tree_printer in tree-diagnostic.c -
though it's obviously nothing to do with trees. (Or if this is too
single-purpose, perhaps there's a need to temporarily inject one-time
callbacks for consuming custom args??).

We can then have a fun discussion about the usage of the Oxford comma :) [1]

IIRC we don't yet have warning_at_rich_loc_n, but it would be fairly easy to 
add.

[...]

[1] which seems to be locale-dependent itself:
https://en.wikipedia.org/wiki/Serial_comma#Other_languages

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Richard Henderson]

On 09/02/2016 02:35 AM, Uros Bizjak wrote:
> --q--
>   /* Pass float and _Complex float variable arguments by reference.
>  This avoids 64-bit store from a FP register to a pretend args save area
>  and subsequent 32-bit load from the saved location to a FP register.
> 
>  Note that 32-bit loads and stores to/from a FP register on alpha reorder
>  bits to form a canonical 64-bit value in the FP register.  This fact
>  invalidates compiler assumption that 32-bit FP value lives in the lower
>  32-bits of the passed 64-bit FP value, so loading the 32-bit value from
>  the stored 64-bit location using 32-bit FP load is invalid on alpha.
> 
>  This introduces sort of ABI incompatibility, but until _Float32 was
>  introduced, C-family languages promoted 32-bit float variable arg to
>  a 64-bit double, and it was not allowed to pass float as a varible
>  argument.  Passing _Complex float as a variable argument never
>  worked on alpha.  Thus, we have no backward compatibility issues
>  to worry about, and passing un-promoted _Float32 and _Complex float
>  as a variable argument will actually work in the future.  */
> --/q--

This sounds like a good plan to me.

> (I was not able to find the
> authoritative OSF/1 ABI document on the net...)

As far as I know, it was never available online.
I have a paper copy.


r~

Re: cfg.c: redundant second assignment of bb_copy = NULL in free_original_copy_tables()

[Prathamesh Kulkarni]

On 2 September 2016 at 15:49, Richard Biener  wrote:
> On Fri, 2 Sep 2016, Prathamesh Kulkarni wrote:
>
>> Hi,
>> There appears to be a redundant second assignmeent bb_copy = NULL in
>> free_copy_original_tables(). I suppose it should be
>> bb_original = NULL instead ?
>> I found this mentioned on a blog "Bugs found in gcc with help of PVS studio":
>> http://www.viva64.com/en/b/0425/#ID0EHCCK
>
> Ok.
Thanks, committed as r239960 after bootstrap+test on x86_64-unknown-linux-gnu.

Regards,
Prathamesh
>
> Thanks,
> Richard.
2016-09-02  Prathamesh Kulkarni  

* cfg.c (free_original_copy_tables): Replace second assignment of
bb_copy = NULL by bb_original = NULL. 

diff --git a/gcc/cfg.c b/gcc/cfg.c
index 0e31780..cab66c6 100644
--- a/gcc/cfg.c
+++ b/gcc/cfg.c
@@ -1075,7 +1075,7 @@ free_original_copy_tables (void)
   delete bb_copy;
   bb_copy = NULL;
   delete bb_original;
-  bb_copy = NULL;
+  bb_original = NULL;
   delete loop_copy;
   loop_copy = NULL;
   delete original_copy_bb_pool;

Re: [PATCH, obvious] Remove double condition from dwarf2out.c

[Jakub Jelinek]

On Fri, Sep 02, 2016 at 07:52:45PM +0300, Kirill Yukhin wrote:
> Hi,
> Remove identical conditions from AND
> in return.
> Will check-in after bootstrap/regtest on i386|x86_64 as obvious.
> 
> gcc/
> * dwarf2out.c (dw_val_equal_p): Remove redundant condition in
> return statement.

This isn't obvious, val_vms_delta uses both .lbl1 and .lbl2, for equality
I bet you want to compare both.  I.e.
   return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)
- && !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1));
+ && !strcmp (a->v.val_vms_delta.lbl2, b->v.val_vms_delta.lbl2));

Jakub

[PATCH, obvious] Remove double condition from dwarf2out.c

[Kirill Yukhin]

Hi,
Remove identical conditions from AND
in return.
Will check-in after bootstrap/regtest on i386|x86_64 as obvious.

gcc/
* dwarf2out.c (dw_val_equal_p): Remove redundant condition in
return statement.

--
Thanks, K

commit 4c71ef36cda91edaef731e460a58fe09942b2d93
Author: Kirill Yukhin 
Date:   Fri Sep 2 19:39:02 2016 +0300

Remove redundant calculation.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 11e0113..a7aba9d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1425,8 +1425,7 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
   return memcmp (a->v.val_data8, b->v.val_data8, 8) == 0;

 case dw_val_class_vms_delta:
-  return (!strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)
-  && !strcmp (a->v.val_vms_delta.lbl1, 
b->v.val_vms_delta.lbl1));

+  return !strcmp (a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1);

 case dw_val_class_discr_value:
   return (a->v.val_discr_value.pos == b->v.val_discr_value.pos

Re: [PATCH] Fix up -fsanitize=address ctor order (PR sanitizer/77396, take 2)

[Richard Biener]

On September 2, 2016 4:58:20 PM GMT+02:00, Jakub Jelinek  
wrote:
>On Thu, Sep 01, 2016 at 09:58:44AM +0200, Richard Biener wrote:
>> Ah, so it expects sth _before_ before-dynamic-init?  At least it
>seems
>> that globals are not only registered inbetween
>before/after-dynamic-init.
>
>Here is updated patch, bootstrapped/regtested on x86_64-linux and
>i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>> Maybe simply always init dynamic_init_globals?
>
>I've posted a patch to llvm-commits.
>
>2016-09-02  Jakub Jelinek  
>
>   PR sanitizer/77396
>   * sanopt.c: Include gimple-ssa.h, tree-phinodes.h and ssa-iterators.h.
>   (sanopt_optimize_walker): Optimize away
>   __asan_before_dynamic_init (...) followed by
>   __asan_after_dynamic_init () without intervening memory loads/stores.
>   * ipa-pure-const.c (special_builtin_state): Handle
>   BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT and
>   BUILT_IN_ASAN_AFTER_DYNAMIC_INIT.
>
>   * decl2.c (do_static_initialization_or_destruction): Only
>   call asan_dynamic_init_call if INITP is true.
>
>   * g++.dg/asan/pr77396.C: New test.
>
>--- gcc/sanopt.c.jj2016-08-31 20:40:26.224215125 +0200
>+++ gcc/sanopt.c   2016-09-02 13:30:09.954070468 +0200
>@@ -33,6 +33,9 @@ along with GCC; see the file COPYING3.
> #include "ubsan.h"
> #include "params.h"
> #include "tree-hash-traits.h"
>+#include "gimple-ssa.h"
>+#include "tree-phinodes.h"
>+#include "ssa-iterators.h"
> 
> 
> /* This is used to carry information about basic blocks.  It is
>@@ -538,6 +541,28 @@ sanopt_optimize_walker (basic_block bb,
>   if (asan_check_optimize && !nonfreeing_call_p (stmt))
>   info->freeing_call_events++;
> 
>+  /* If __asan_before_dynamic_init ("module"); is followed by
>+   __asan_after_dynamic_init (); without intervening memory
>loads/stores,
>+   there is nothing to guard, so optimize both away.  */
>+  if (asan_check_optimize
>+&& gimple_call_builtin_p (stmt, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
>+  {
>+use_operand_p use;
>+gimple *use_stmt;
>+if (single_imm_use (gimple_vdef (stmt), , _stmt))
>+  {
>+if (is_gimple_call (use_stmt)
>+&& gimple_call_builtin_p (use_stmt,
>+  BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
>+  {
>+unlink_stmt_vdef (use_stmt);
>+gimple_stmt_iterator gsi2 = gsi_for_stmt (use_stmt);
>+gsi_remove (, true);
>+remove = true;
>+  }
>+  }
>+  }
>+
>   if (gimple_call_internal_p (stmt))
>   switch (gimple_call_internal_fn (stmt))
> {
>--- gcc/ipa-pure-const.c.jj2016-08-31 20:40:26.631209934 +0200
>+++ gcc/ipa-pure-const.c   2016-09-02 13:24:55.173990290 +0200
>@@ -508,6 +508,8 @@ special_builtin_state (enum pure_const_s
>   case BUILT_IN_FRAME_ADDRESS:
>   case BUILT_IN_APPLY:
>   case BUILT_IN_APPLY_ARGS:
>+  case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
>+  case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
> *looping = false;
> *state = IPA_CONST;
> return true;
>--- gcc/cp/decl2.c.jj  2016-08-31 20:40:26.577210623 +0200
>+++ gcc/cp/decl2.c 2016-09-02 13:24:55.175990265 +0200
>@@ -3861,7 +3861,7 @@ do_static_initialization_or_destruction
>  in other compilation units, or at least those that haven't been
>  initialized yet.  Variables that need dynamic construction in
>  the current compilation unit are kept accessible.  */
>-  if (flag_sanitize & SANITIZE_ADDRESS)
>+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
> finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
> 
>   node = vars;
>@@ -3914,7 +3914,7 @@ do_static_initialization_or_destruction
> 
>   /* Revert what __asan_before_dynamic_init did by calling
>  __asan_after_dynamic_init.  */
>-  if (flag_sanitize & SANITIZE_ADDRESS)
>+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
> finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/true));
> 
>   /* Finish up the init/destruct if-stmt body.  */
>--- gcc/testsuite/g++.dg/asan/pr77396.C.jj 2016-09-02
>13:24:55.175990265 +0200
>+++ gcc/testsuite/g++.dg/asan/pr77396.C2016-09-02 13:24:55.175990265
>+0200
>@@ -0,0 +1,12 @@
>+// PR sanitizer/77396
>+// { dg-do run }
>+// { dg-set-target-env-var ASAN_OPTIONS
>"check_initialization_order=true" }
>+
>+static int a = 0; 
>+static int b = a; 
>+
>+int
>+main ()
>+{
>+  return 0;
>+}
>
>
>   Jakub

Re: [PATCH] Fix up ivopts POINTER_TYPE_P handling (PR tree-optimization/77444)

[Richard Biener]

On September 2, 2016 5:11:39 PM GMT+02:00, Jakub Jelinek  
wrote:
>Hi!
>
>I've looked a little bit at svn blame and we had:
>  tree steptype = type;
>  if (POINTER_TYPE_P (type))
>steptype = sizetype;
>there initially and the
>  steptype = unsigned_type_for (type);
>has been added later on in r209190, without cleaning up the earlier
>stmts.
>I think for POINTER_TYPE_P it is better to use sizetype instead of
>nonstandard integer type with TYPE_PRECISION of pointer types.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>2016-09-02  Jakub Jelinek  
>   Richard Biener  
>
>   PR tree-optimization/77444
>   * tree-ssa-loop-ivopts.c (cand_value_at): For pointers use sizetype
>   as steptype, remove redundant initialization.
>
>--- gcc/tree-ssa-loop-ivopts.c.jj  2016-07-21 08:59:55.0 +0200
>+++ gcc/tree-ssa-loop-ivopts.c 2016-09-02 14:18:09.0 +0200
>@@ -5168,10 +5168,11 @@ cand_value_at (struct loop *loop, struct
>   aff_tree step, delta, nit;
>   struct iv *iv = cand->iv;
>   tree type = TREE_TYPE (iv->base);
>-  tree steptype = type;
>+  tree steptype;
>   if (POINTER_TYPE_P (type))
> steptype = sizetype;
>-  steptype = unsigned_type_for (type);
>+  else
>+steptype = unsigned_type_for (type);
> 
>   tree_to_aff_combination (iv->step, TREE_TYPE (iv->step), );
>   aff_combination_convert (, steptype);
>
>   Jakub

Re: [PATCH, vec-tails 07/10] Support loop epilogue combining

[Bin.Cheng]

On Fri, Sep 2, 2016 at 3:46 PM, Yuri Rumyantsev  wrote:
> Hi Jeff,
>
> I am trying to reduce cost of repeated call of if-conversion for
> epilogue vectorization. I'd like to clarify your recommendation -
> should I design additional support for versioning in
> vect_do_peeling_for_loop_bound or lightweight version of if-conversion
Hi Yuri,
I didn't read the patch, so please correct me if I mis-understand
anything.  It might be better not to introduce versioning logic in
peeling stuff if possible.  The peeling part is complicated and
generates somehow inefficient CFG.  I am preparing patches rewriting
the peeling stuff.

Thanks,
bin
> is sufficient? Any help in clarification will be appreciated.
>
> Thanks ahead.
> Yuri.
>
> 2016-08-01 19:10 GMT+03:00 Jeff Law :
>> On 08/01/2016 03:09 AM, Ilya Enkovich wrote:
>>>
>>> 2016-07-26 18:38 GMT+03:00 Ilya Enkovich :

 2016-07-26 18:26 GMT+03:00 Jeff Law :
>
> On 07/26/2016 03:57 AM, Ilya Enkovich wrote:
>>>
>>>
>>>
>>> Ilya, what's the fundamental reason why we need to run
>>> if-conversion again? Yes, I know you want to if-convert the
>>> epilogue, but why?
>>>
>>> What are the consequences of not doing if-conversion on the
>>> epilogue? Presumably we miss a vectorization opportunity on the
>>> tail.  But that may be a reasonable limitation to allow the
>>> existing work to move forward while you go back and revamp things a
>>> little.
>>
>>
>>
>> If we have some control-flow in a loop then we have to if-convert it
>> for vectorizer. We need to preserve both versions: if-converted one
>> for vectorizer and the original one to be used if vectorization
>> fails.  For epilogues we have similar situation and need two
>> versions.  I do it by running if-conversion on a copy of original
>> loop. Note that it doesn't run full if-conversion pass. If-conversion
>> is called for epilogue loop only.
>
>
> Right.  So what I think Richi wants you to try is to use the
> if-converted
> loop to construct the if-converted epilogue.  It seems conceptually
> simple
> and low cost -- the question is on the implementation side.  I have no
> clue
> how painful that would be.


 Probably another part of if-conversion may be re-used to build required
 epilogue.  I'll have a look.
>>>
>>>
>>> Hi,
>>>
>>> Yuri will continue my work from this point.
>>
>> Understood.  I'm actually got some comments on #5 and Yuri is already on the
>> CC list for that draft message.
>>
>> Jeff

Re: Fwd: libgo patch committed: Use -fgo-c-header to share between Go and C

[Andreas Schwab]

On Sep 02 2016, Andreas Schwab  wrote:

> On Sep 02 2016, Ian Lance Taylor  wrote:
>
>> On Fri, Sep 2, 2016 at 9:14 AM, Andreas Schwab  wrote:
>>>
>>> That breaks libgo on ia64.  The problem is that _ucontext_t isn't
>>> properly aligned.
>>
>> Interesting.  Thanks for looking into it.  What is the required
>> alignment?  This code should be aligning it to a pointer boundary.
>
> That is too small.  It needs at least 16 byte alignment.

And PowerPC has a similar requirement.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Fwd: libgo patch committed: Use -fgo-c-header to share between Go and C

[Andreas Schwab]

On Sep 02 2016, Ian Lance Taylor  wrote:

> On Fri, Sep 2, 2016 at 9:14 AM, Andreas Schwab  wrote:
>>
>> That breaks libgo on ia64.  The problem is that _ucontext_t isn't
>> properly aligned.
>
> Interesting.  Thanks for looking into it.  What is the required
> alignment?  This code should be aligning it to a pointer boundary.

That is too small.  It needs at least 16 byte alignment.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[committed] Remove redundant stmt in i386.c (PR other/77421)

[Jakub Jelinek]

Hi!

I've committed as obvious the following change.  I've tracked the redundant
stmt to appear in r216794, where just a bunch of new cases were added and
the redundant stmt together with those.

2016-09-02  Jakub Jelinek  

PR other/77421
* config/i386/i386.c (ix86_expanded_args_builtin): Remove redundant
assignment added in r216794.

--- gcc/config/i386/i386.c.jj   2016-09-01 11:43:48.0 +0200
+++ gcc/config/i386/i386.c  2016-09-02 14:13:33.089980709 +0200
@@ -34862,7 +34862,6 @@ ix86_expand_args_builtin (const struct b
 case V4DI_FTYPE_V4DI_V4DI_V4DI_INT_UQI:
 case V4SI_FTYPE_V4SI_V4SI_V4SI_INT_UQI:
 case V2DI_FTYPE_V2DI_V2DI_V2DI_INT_UQI:
-   nargs = 5;
   nargs = 5;
   mask_pos = 1;
   nargs_constant = 1;

Jakub

Re: Fwd: libgo patch committed: Use -fgo-c-header to share between Go and C

[Ian Lance Taylor]

On Fri, Sep 2, 2016 at 9:14 AM, Andreas Schwab  wrote:
>
> That breaks libgo on ia64.  The problem is that _ucontext_t isn't
> properly aligned.

Interesting.  Thanks for looking into it.  What is the required
alignment?  This code should be aligning it to a pointer boundary.

Ian

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Mike Stump]

On Sep 2, 2016, at 6:31 AM, Joseph Myers  wrote:
> 
> On Fri, 2 Sep 2016, Uros Bizjak wrote:
> 
>> It looks to me that we have no tests for _Complex float variable
>> arguments passing in g*.dg/compat/. There are no xfails for alpha* in
>> this directory, and these arguments would fail for sure for this
>> target.
> 
> I suppose compat tests should be added for _Complex double and _Complex 
> long double variable argument passing as well along with _Complex float, 
> if those types aren't tested for variable argument passing either.

I concur.

Re: Fwd: libgo patch committed: Use -fgo-c-header to share between Go and C

[Andreas Schwab]

That breaks libgo on ia64.  The problem is that _ucontext_t isn't
properly aligned.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: libgo/runtime: Fix signal stack size for ia64

[Andreas Schwab]

On Sep 02 2016, Andreas Schwab  wrote:

> On Aug 31 2016, Ian Lance Taylor  wrote:
>
>> Index: libgo/runtime/runtime.c
>> ===
>> --- libgo/runtime/runtime.c  (revision 239872)
>> +++ libgo/runtime/runtime.c  (working copy)
>> @@ -272,7 +272,14 @@ runtime_tickspersecond(void)
>>  void
>>  runtime_mpreinit(M *mp)
>>  {
>> -mp->gsignal = runtime_malg(32*1024, (byte**)>gsignalstack, 
>> >gsignalstacksize);  // OS X wants >=8K, Linux >=2K
>> +int32 stacksize = 32 * 1024;// OS X wants >=8K, Linux >=2K
>> +
>> +#ifdef SIGSTKSZ
>> +if(stacksize < SIGSTKSZ)
>> +stacksize = SIGSTKSZ;
>> +#endif
>
> There is nothing that defines SIGSTKSZ.

Sorry, that is wrong, the regression I see must be something else.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: libgo/runtime: Fix signal stack size for ia64

[Andreas Schwab]

On Aug 31 2016, Ian Lance Taylor  wrote:

> Index: libgo/runtime/runtime.c
> ===
> --- libgo/runtime/runtime.c   (revision 239872)
> +++ libgo/runtime/runtime.c   (working copy)
> @@ -272,7 +272,14 @@ runtime_tickspersecond(void)
>  void
>  runtime_mpreinit(M *mp)
>  {
> - mp->gsignal = runtime_malg(32*1024, (byte**)>gsignalstack, 
> >gsignalstacksize);  // OS X wants >=8K, Linux >=2K
> + int32 stacksize = 32 * 1024;// OS X wants >=8K, Linux >=2K
> +
> +#ifdef SIGSTKSZ
> + if(stacksize < SIGSTKSZ)
> + stacksize = SIGSTKSZ;
> +#endif

There is nothing that defines SIGSTKSZ.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Bernd Edlinger]

Hi,

 > +  r += !a == ~b;
 > +  r += !a == ~(int) b;

I don't understand why ~b should not be warned at -Wall.

Frankly I don't even understand why the above statements are
completely optimized away.

r += !a == ~b;
is optimized away, but

b = ~b;
r += !a == b;

Is not.  Why?



Bernd.

Re: [PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Segher Boessenkool]

On Fri, Sep 02, 2016 at 09:40:36AM -0400, Eric Gallager wrote:
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -1122,7 +1122,7 @@ static int
>  combine_instructions (rtx_insn *f, unsigned int nregs)
>  {
>rtx_insn *insn, *next;
> -  rtx_insn *prev;
> +  rtx_insn *prev = NULL;
>struct insn_link *links, *nextlinks;
>rtx_insn *first;
>basic_block last_bb;

I don't see any place "prev" is used witout an immediately preceding
assignment to it.  Could you show the warning you see please?


Segher

[PATCH] Fix alter_output_for_subst_insn (PR other/77421)

[Jakub Jelinek]

Hi!

The two ports that use define_subst (ix86 and visium) don't do anything in
this function, other than returning early - return insn_out, so all I could
do is look at the intent.
The *insn_out == '*' || *insn_out != '@' got flagged by some tool, the
"*insn_out == '*' || " part is unnecessary, since '*' != '@'.  I guess the
reason for it being there is that template starting with * is C code (which
this function has nothing to do for), template starting with @ is what the
function wants to do something about and other template strings it wants to
ignore too.

But, when I got to this function, I found various weirdo formatting and
other issues, the most important is that it leaks memory and in my
understanding allocates that buffer completely uselessly, as it is pretty
much a strdup of the insn_out after skipping initial spaces (and all @,
which is weird, IMNSHO the only @ that it should skip is the very first
one), except that '\0' isn't there and the length is remembered.  But, we
don't change it at all, so we can as well use the original insn_out.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-02  Jakub Jelinek  

PR other/77421
* gensupport.c (alter_output_for_subst_insn): Remove redundant
*insn_out == '*' test.  Don't copy unnecessary to yet another
memory buffer, and don't leak it.

--- gcc/gensupport.c.jj 2016-08-12 17:33:38.0 +0200
+++ gcc/gensupport.c2016-09-02 14:55:01.217182312 +0200
@@ -1632,33 +1632,30 @@ duplicate_each_alternative (const char *
 static const char *
 alter_output_for_subst_insn (rtx insn, int alt)
 {
-  const char *insn_out, *sp ;
-  char *old_out, *new_out, *cp;
-  int i, j, new_len;
+  const char *insn_out, *old_out;
+  char *new_out, *cp;
+  size_t old_len, new_len;
+  int j;
 
   insn_out = XTMPL (insn, 3);
 
-  if (alt < 2 || *insn_out == '*' || *insn_out != '@')
+  if (alt < 2 || *insn_out != '@')
 return insn_out;
 
-  old_out = XNEWVEC (char, strlen (insn_out)),
-  sp = insn_out;
+  old_out = insn_out + 1;
+  while (ISSPACE (*old_out))
+old_out++;
+  old_len = strlen (old_out);
 
-  while (ISSPACE (*sp) || *sp == '@')
-sp++;
-
-  for (i = 0; *sp;)
-old_out[i++] = *sp++;
-
-  new_len = alt * (i + 1) + 1;
+  new_len = alt * (old_len + 1) + 1;
 
   new_out = XNEWVEC (char, new_len);
   new_out[0] = '@';
 
-  for (j = 0, cp = new_out + 1; j < alt; j++, cp += i + 1)
+  for (j = 0, cp = new_out + 1; j < alt; j++, cp += old_len + 1)
 {
-  memcpy (cp, old_out, i);
-  *(cp+i) = (j == alt - 1) ? '\0' : '\n';
+  memcpy (cp, old_out, old_len);
+  cp[old_len] = (j == alt - 1) ? '\0' : '\n';
 }
 
   return new_out;

Jakub

C/C++ PATCH for c/77423 (bogus warning with -Wlogical-not-parentheses)

[Marek Polacek]

Martin reported that -Wlogical-not-parentheses issues a bogus warning
for bit operations on booleans, because the warning didn't take integer
promotions into account.  The following patch ought to fix this problem.

It's not exactly trivial because I had to handle even more complex
expressions and comparison.  I am aware that given what I do with
CONVERT_EXPR_P in expr_has_boolean_operands_p, it's possible to construct
a pathological testcase where the C FE would warn, but C++ FE wouldn't.
But I'm not convinced it's worth complicating this code further.

I also fixed -Wlogical-not-parentheses wording in docs, following Martin's
suggestion.

Bootstrapped/regtested on x86_64-linux and ppc64-linux, ok for trunk?

2016-09-02  Marek Polacek  

PR c/77423
* doc/invoke.texi: Update -Wlogical-not-parentheses documentation.

* c-common.c (bool_promoted_to_int_p): New function.
(expr_has_boolean_operands_p): New function.
(warn_logical_not_parentheses): Return if expr_has_boolean_operands_p.
(maybe_warn_bool_compare): Use bool_promoted_to_int_p.

* c-c++-common/Wlogical-not-parentheses-3.c: New test.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 399ba97..fbc8fb0 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1479,6 +1479,36 @@ warn_tautological_cmp (location_t loc, enum tree_code 
code, tree lhs, tree rhs)
 }
 }
 
+/* Return true iff T is a boolean promoted to int.  */
+
+static bool
+bool_promoted_to_int_p (tree t)
+{
+  return (CONVERT_EXPR_P (t)
+ && TREE_TYPE (t) == integer_type_node
+ && TREE_CODE (TREE_TYPE (TREE_OPERAND (t, 0))) == BOOLEAN_TYPE);
+}
+
+/* Return true iff EXPR only contains boolean operands, or comparisons.  */
+
+static bool
+expr_has_boolean_operands_p (tree expr)
+{
+  STRIP_NOPS (expr);
+
+  if (CONVERT_EXPR_P (expr))
+return bool_promoted_to_int_p (expr);
+  else if (UNARY_CLASS_P (expr))
+return expr_has_boolean_operands_p (TREE_OPERAND (expr, 0));
+  else if (BINARY_CLASS_P (expr))
+return (expr_has_boolean_operands_p (TREE_OPERAND (expr, 0))
+   && expr_has_boolean_operands_p (TREE_OPERAND (expr, 1)));
+  else if (COMPARISON_CLASS_P (expr))
+return true;
+  else
+return false;
+}
+
 /* Warn about logical not used on the left hand side operand of a comparison.
This function assumes that the LHS is inside of TRUTH_NOT_EXPR.
Do not warn if RHS is of a boolean type, a logical operator, or
@@ -1494,6 +1524,10 @@ warn_logical_not_parentheses (location_t location, enum 
tree_code code,
   || truth_value_p (TREE_CODE (rhs)))
 return;
 
+  /* Don't warn for expression like !x == ~(bool1 | bool2).  */
+  if (expr_has_boolean_operands_p (rhs))
+return;
+
   /* Don't warn for !x == 0 or !y != 0, those are equivalent to
  !(x == 0) or !(y != 0).  */
   if ((code == EQ_EXPR || code == NE_EXPR)
@@ -12405,9 +12439,7 @@ maybe_warn_bool_compare (location_t loc, enum tree_code 
code, tree op0,
 don't want to warn here.  */
   tree noncst = TREE_CODE (op0) == INTEGER_CST ? op1 : op0;
   /* Handle booleans promoted to integers.  */
-  if (CONVERT_EXPR_P (noncst)
- && TREE_TYPE (noncst) == integer_type_node
- && TREE_CODE (TREE_TYPE (TREE_OPERAND (noncst, 0))) == BOOLEAN_TYPE)
+  if (bool_promoted_to_int_p (noncst))
/* Warn.  */;
   else if (TREE_CODE (TREE_TYPE (noncst)) != BOOLEAN_TYPE
   && !truth_value_p (TREE_CODE (noncst)))
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 87da1f1..38d55d4 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5437,8 +5437,8 @@ if (a < 0 && a < 0) @{ @dots{} @}
 @opindex Wlogical-not-parentheses
 @opindex Wno-logical-not-parentheses
 Warn about logical not used on the left hand side operand of a comparison.
-This option does not warn if the RHS operand is of a boolean type.  Its
-purpose is to detect suspicious code like the following:
+This option does not warn if the right operand is considered to be a Boolean
+expression.  Its purpose is to detect suspicious code like the following:
 @smallexample
 int a;
 @dots{}
diff --git gcc/testsuite/c-c++-common/Wlogical-not-parentheses-3.c 
gcc/testsuite/c-c++-common/Wlogical-not-parentheses-3.c
index e69de29..00aa747 100644
--- gcc/testsuite/c-c++-common/Wlogical-not-parentheses-3.c
+++ gcc/testsuite/c-c++-common/Wlogical-not-parentheses-3.c
@@ -0,0 +1,31 @@
+/* PR c/77423 */
+/* { dg-do compile } */
+/* { dg-options "-Wlogical-not-parentheses" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+int
+f (int a, bool b, bool c)
+{
+  int r = 0;
+
+  r += !a == (b | c);
+  r += !a == (b ^ c);
+  r += !a == (b & c);
+  r += !a == ~b;
+  r += !a == ~(int) b;
+  r += !a == ((b & c) | c);
+  r += !a == ((b & c) | (b ^ c));
+  r += !a == (int) (b ^ c);
+  r += !a == (int) ~b;
+  r += !a == ~~b;
+  r += !a == ~(b | c);
+  r += !a == ~(b | (a == 1));
+  r += !a == 

[PATCH] Fix up ivopts POINTER_TYPE_P handling (PR tree-optimization/77444)

[Jakub Jelinek]

Hi!

I've looked a little bit at svn blame and we had:
  tree steptype = type;
  if (POINTER_TYPE_P (type))
steptype = sizetype;
there initially and the
  steptype = unsigned_type_for (type);
has been added later on in r209190, without cleaning up the earlier stmts.
I think for POINTER_TYPE_P it is better to use sizetype instead of
nonstandard integer type with TYPE_PRECISION of pointer types.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-02  Jakub Jelinek  
Richard Biener  

PR tree-optimization/77444
* tree-ssa-loop-ivopts.c (cand_value_at): For pointers use sizetype
as steptype, remove redundant initialization.

--- gcc/tree-ssa-loop-ivopts.c.jj   2016-07-21 08:59:55.0 +0200
+++ gcc/tree-ssa-loop-ivopts.c  2016-09-02 14:18:09.0 +0200
@@ -5168,10 +5168,11 @@ cand_value_at (struct loop *loop, struct
   aff_tree step, delta, nit;
   struct iv *iv = cand->iv;
   tree type = TREE_TYPE (iv->base);
-  tree steptype = type;
+  tree steptype;
   if (POINTER_TYPE_P (type))
 steptype = sizetype;
-  steptype = unsigned_type_for (type);
+  else
+steptype = unsigned_type_for (type);
 
   tree_to_aff_combination (iv->step, TREE_TYPE (iv->step), );
   aff_combination_convert (, steptype);

Jakub

[PATCH] Remove unnecessary conditional in get_odr_type (PR rtl-optimization/77425)

[Jakub Jelinek]

Hi!

As mentioned in the PR, we have
static GTY(()) vec  *odr_types_ptr;
#define odr_types (*odr_types_ptr)
and in the else block do
  odr_types[val->id] = 0;
first (which is actually
  (*odr_types_ptr)[val->id] = 0;
without the obfuscation) and then
  if (odr_types_ptr)
...
odr_types_ptr is known to be non-NULL in this case, otherwise it couldn't be
dereferenced first - it can be NULL only when nothing has been added yet,
but that happens only if the then path is taken.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-02  Jakub Jelinek  

PR rtl-optimization/77425
* ipa-devirt.c (get_odr_type): Set val->id unconditionally.

--- gcc/ipa-devirt.c.jj 2016-04-14 21:20:19.0 +0200
+++ gcc/ipa-devirt.c2016-09-01 12:42:07.077740393 +0200
@@ -2136,8 +2136,7 @@ get_odr_type (tree type, bool insert)
   /* Be sure we did not recorded any derived types; these may need
 renumbering too.  */
   gcc_assert (val->derived_types.length() == 0);
-  if (odr_types_ptr)
-   val->id = odr_types.length ();
+  val->id = odr_types.length ();
   vec_safe_push (odr_types_ptr, val);
 }
   return val;
 
Jakub

[PATCH] Fix UB in sched-int.h iterator (PR rtl-optimization/77425)

[Jakub Jelinek]

Hi!

We have
#define DEPS_LIST_FIRST(L) ((L)->first)
and first is the field of the struct, so for the case when list is
NULL we do linkp = >first; which actually gives us NULL too, but with
UB.  From my analysis of the scheduler code, we should never use linkp (or
anything else in the iterator) after sd_iterator_cond returned false (don't
iterate anymore), so I think it is just fine to keep it pointing to the
previous entry (where *linkp is NULL), instead of setting linkp effectively
to NULL.  All uses of linkp dereference linkp anyway.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-09-02  Jakub Jelinek  

PR rtl-optimization/77425
* sched-int.h (sd_iterator_cond): Don't update it_ptr->linkp if list
is NULL.

--- gcc/sched-int.h.jj  2016-03-15 17:10:19.0 +0100
+++ gcc/sched-int.h 2016-09-01 11:49:52.057719797 +0200
@@ -1624,10 +1624,11 @@ sd_iterator_cond (sd_iterator_def *it_pt
  sd_next_list (it_ptr->insn,
_ptr->types, , _ptr->resolved_p);
 
- it_ptr->linkp = _LIST_FIRST (list);
-
  if (list)
-   continue;
+   {
+ it_ptr->linkp = _LIST_FIRST (list);
+ continue;
+   }
}
 
  *dep_ptr = NULL;

Jakub

[PATCH] Fix up -fsanitize=address ctor order (PR sanitizer/77396, take 2)

[Jakub Jelinek]

On Thu, Sep 01, 2016 at 09:58:44AM +0200, Richard Biener wrote:
> Ah, so it expects sth _before_ before-dynamic-init?  At least it seems
> that globals are not only registered inbetween before/after-dynamic-init.

Here is updated patch, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk?

> Maybe simply always init dynamic_init_globals?

I've posted a patch to llvm-commits.

2016-09-02  Jakub Jelinek  

PR sanitizer/77396
* sanopt.c: Include gimple-ssa.h, tree-phinodes.h and ssa-iterators.h.
(sanopt_optimize_walker): Optimize away
__asan_before_dynamic_init (...) followed by
__asan_after_dynamic_init () without intervening memory loads/stores.
* ipa-pure-const.c (special_builtin_state): Handle
BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT and
BUILT_IN_ASAN_AFTER_DYNAMIC_INIT.

* decl2.c (do_static_initialization_or_destruction): Only
call asan_dynamic_init_call if INITP is true.

* g++.dg/asan/pr77396.C: New test.

--- gcc/sanopt.c.jj 2016-08-31 20:40:26.224215125 +0200
+++ gcc/sanopt.c2016-09-02 13:30:09.954070468 +0200
@@ -33,6 +33,9 @@ along with GCC; see the file COPYING3.
 #include "ubsan.h"
 #include "params.h"
 #include "tree-hash-traits.h"
+#include "gimple-ssa.h"
+#include "tree-phinodes.h"
+#include "ssa-iterators.h"
 
 
 /* This is used to carry information about basic blocks.  It is
@@ -538,6 +541,28 @@ sanopt_optimize_walker (basic_block bb,
   if (asan_check_optimize && !nonfreeing_call_p (stmt))
info->freeing_call_events++;
 
+  /* If __asan_before_dynamic_init ("module"); is followed by
+__asan_after_dynamic_init (); without intervening memory loads/stores,
+there is nothing to guard, so optimize both away.  */
+  if (asan_check_optimize
+ && gimple_call_builtin_p (stmt, BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT))
+   {
+ use_operand_p use;
+ gimple *use_stmt;
+ if (single_imm_use (gimple_vdef (stmt), , _stmt))
+   {
+ if (is_gimple_call (use_stmt)
+ && gimple_call_builtin_p (use_stmt,
+   BUILT_IN_ASAN_AFTER_DYNAMIC_INIT))
+   {
+ unlink_stmt_vdef (use_stmt);
+ gimple_stmt_iterator gsi2 = gsi_for_stmt (use_stmt);
+ gsi_remove (, true);
+ remove = true;
+   }
+   }
+   }
+
   if (gimple_call_internal_p (stmt))
switch (gimple_call_internal_fn (stmt))
  {
--- gcc/ipa-pure-const.c.jj 2016-08-31 20:40:26.631209934 +0200
+++ gcc/ipa-pure-const.c2016-09-02 13:24:55.173990290 +0200
@@ -508,6 +508,8 @@ special_builtin_state (enum pure_const_s
case BUILT_IN_FRAME_ADDRESS:
case BUILT_IN_APPLY:
case BUILT_IN_APPLY_ARGS:
+   case BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT:
+   case BUILT_IN_ASAN_AFTER_DYNAMIC_INIT:
  *looping = false;
  *state = IPA_CONST;
  return true;
--- gcc/cp/decl2.c.jj   2016-08-31 20:40:26.577210623 +0200
+++ gcc/cp/decl2.c  2016-09-02 13:24:55.175990265 +0200
@@ -3861,7 +3861,7 @@ do_static_initialization_or_destruction
  in other compilation units, or at least those that haven't been
  initialized yet.  Variables that need dynamic construction in
  the current compilation unit are kept accessible.  */
-  if (flag_sanitize & SANITIZE_ADDRESS)
+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
 finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
 
   node = vars;
@@ -3914,7 +3914,7 @@ do_static_initialization_or_destruction
 
   /* Revert what __asan_before_dynamic_init did by calling
  __asan_after_dynamic_init.  */
-  if (flag_sanitize & SANITIZE_ADDRESS)
+  if (initp && (flag_sanitize & SANITIZE_ADDRESS))
 finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/true));
 
   /* Finish up the init/destruct if-stmt body.  */
--- gcc/testsuite/g++.dg/asan/pr77396.C.jj  2016-09-02 13:24:55.175990265 
+0200
+++ gcc/testsuite/g++.dg/asan/pr77396.C 2016-09-02 13:24:55.175990265 +0200
@@ -0,0 +1,12 @@
+// PR sanitizer/77396
+// { dg-do run }
+// { dg-set-target-env-var ASAN_OPTIONS "check_initialization_order=true" }
+
+static int a = 0; 
+static int b = a; 
+
+int
+main ()
+{
+  return 0;
+}


Jakub

Re: [PATCH, vec-tails 07/10] Support loop epilogue combining

[Yuri Rumyantsev]

Hi Jeff,

I am trying to reduce cost of repeated call of if-conversion for
epilogue vectorization. I'd like to clarify your recommendation -
should I design additional support for versioning in
vect_do_peeling_for_loop_bound or lightweight version of if-conversion
is sufficient? Any help in clarification will be appreciated.

Thanks ahead.
Yuri.

2016-08-01 19:10 GMT+03:00 Jeff Law :
> On 08/01/2016 03:09 AM, Ilya Enkovich wrote:
>>
>> 2016-07-26 18:38 GMT+03:00 Ilya Enkovich :
>>>
>>> 2016-07-26 18:26 GMT+03:00 Jeff Law :

 On 07/26/2016 03:57 AM, Ilya Enkovich wrote:
>>
>>
>>
>> Ilya, what's the fundamental reason why we need to run
>> if-conversion again? Yes, I know you want to if-convert the
>> epilogue, but why?
>>
>> What are the consequences of not doing if-conversion on the
>> epilogue? Presumably we miss a vectorization opportunity on the
>> tail.  But that may be a reasonable limitation to allow the
>> existing work to move forward while you go back and revamp things a
>> little.
>
>
>
> If we have some control-flow in a loop then we have to if-convert it
> for vectorizer. We need to preserve both versions: if-converted one
> for vectorizer and the original one to be used if vectorization
> fails.  For epilogues we have similar situation and need two
> versions.  I do it by running if-conversion on a copy of original
> loop. Note that it doesn't run full if-conversion pass. If-conversion
> is called for epilogue loop only.


 Right.  So what I think Richi wants you to try is to use the
 if-converted
 loop to construct the if-converted epilogue.  It seems conceptually
 simple
 and low cost -- the question is on the implementation side.  I have no
 clue
 how painful that would be.
>>>
>>>
>>> Probably another part of if-conversion may be re-used to build required
>>> epilogue.  I'll have a look.
>>
>>
>> Hi,
>>
>> Yuri will continue my work from this point.
>
> Understood.  I'm actually got some comments on #5 and Yuri is already on the
> CC list for that draft message.
>
> Jeff

[PATCH] Silence some uninitialized variable warnings that appear when bootstrapping

[Eric Gallager]

While I was silencing some warnings about narrowing conversions, which
was https://gcc.gnu.org/ml/gcc-patches/2016-08/msg02129.html (for
reference), I also silenced some warnings from -Wmaybe-uninitialized
in combine.c and varasm.c while I was at it. I took the easy way out
and simply initialized the variables; there might possibly be a better
way to go about it, I don't know...

Thanks,
Eric Gallager
 gcc/combine.c | 2 +-
 gcc/varasm.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index 1b262f9..b45d991 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1122,7 +1122,7 @@ static int
 combine_instructions (rtx_insn *f, unsigned int nregs)
 {
   rtx_insn *insn, *next;
-  rtx_insn *prev;
+  rtx_insn *prev = NULL;
   struct insn_link *links, *nextlinks;
   rtx_insn *first;
   basic_block last_bb;
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 00a9b30..140052b 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -774,7 +774,7 @@ mergeable_string_section (tree decl ATTRIBUTE_UNUSED,
  unsigned HOST_WIDE_INT align ATTRIBUTE_UNUSED,
  unsigned int flags ATTRIBUTE_UNUSED)
 {
-  HOST_WIDE_INT len;
+  HOST_WIDE_INT len = 0;
 
   if (HAVE_GAS_SHF_MERGE && flag_merge_constants
   && TREE_CODE (decl) == STRING_CST

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Joseph Myers]

On Fri, 2 Sep 2016, Uros Bizjak wrote:

> It looks to me that we have no tests for _Complex float variable
> arguments passing in g*.dg/compat/. There are no xfails for alpha* in
> this directory, and these arguments would fail for sure for this
> target.

I suppose compat tests should be added for _Complex double and _Complex 
long double variable argument passing as well along with _Complex float, 
if those types aren't tested for variable argument passing either.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Uros Bizjak]

On Fri, Sep 2, 2016 at 2:37 PM, Uros Bizjak  wrote:
> On Fri, Sep 2, 2016 at 2:11 PM, Jakub Jelinek  wrote:
>> On Fri, Sep 02, 2016 at 12:09:30PM +, Joseph Myers wrote:
>>> On Fri, 2 Sep 2016, Uros Bizjak wrote:
>>>
>>> >  argument.  Passing _Complex float as a variable argument never
>>> >  worked on alpha.  Thus, we have no backward compatibility issues
>>>
>>> Presumably there should be an architecture-independent execution test of
>>> passing _Complex float in variable arguments - either new, or a
>>> pre-existing one whose XFAIL or skip for alpha can be removed.  (That is,
>>> one in the GCC testsuite rather than relying on a libffi test to test
>>> GCC.)
>>
>> And if it is in g*.dg/compat/, it can even test ABI compatibility between
>> different compilers or their versions.
>
> It looks to me that we have no tests for _Complex float variable
> arguments passing in g*.dg/compat/. There are no xfails for alpha* in
> this directory, and these arguments would fail for sure for this
> target.

Indeed. The only scalar _Complex float processing is in
scalar-by-value-4*, and the test lacks varargs.

Uros.

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Uros Bizjak]

On Fri, Sep 2, 2016 at 2:11 PM, Jakub Jelinek  wrote:
> On Fri, Sep 02, 2016 at 12:09:30PM +, Joseph Myers wrote:
>> On Fri, 2 Sep 2016, Uros Bizjak wrote:
>>
>> >  argument.  Passing _Complex float as a variable argument never
>> >  worked on alpha.  Thus, we have no backward compatibility issues
>>
>> Presumably there should be an architecture-independent execution test of
>> passing _Complex float in variable arguments - either new, or a
>> pre-existing one whose XFAIL or skip for alpha can be removed.  (That is,
>> one in the GCC testsuite rather than relying on a libffi test to test
>> GCC.)
>
> And if it is in g*.dg/compat/, it can even test ABI compatibility between
> different compilers or their versions.

It looks to me that we have no tests for _Complex float variable
arguments passing in g*.dg/compat/. There are no xfails for alpha* in
this directory, and these arguments would fail for sure for this
target.

Uros.

Re: [RFC][IPA-VRP] Add support for IPA VRP in ipa-cp/ipa-prop

[Jan Hubicka]

> Hi Honza,
> 
> Here is a re-based version which also addresses the review comments.
> 
> Do you mean the following, I was following other implementations.
> 
> @@ -2264,6 +2430,11 @@ propagate_constants_accross_call (struct
> cgraph_edge *cs)
> _plats->bits_lattice);
>ret |= propagate_aggs_accross_jump_function (cs, jump_func,
> dest_plats);
> +  if (opt_for_fn (callee->decl, flag_ipa_vrp))
> +ret |= propagate_vr_accross_jump_function (cs,
> +   jump_func, dest_plats);
> +  else
> +ret |= dest_plats->m_value_range.set_to_bottom ();

yes, that looks fine.

Path is OK, thanks!

Honza

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Jakub Jelinek]

On Fri, Sep 02, 2016 at 12:09:30PM +, Joseph Myers wrote:
> On Fri, 2 Sep 2016, Uros Bizjak wrote:
> 
> >  argument.  Passing _Complex float as a variable argument never
> >  worked on alpha.  Thus, we have no backward compatibility issues
> 
> Presumably there should be an architecture-independent execution test of 
> passing _Complex float in variable arguments - either new, or a 
> pre-existing one whose XFAIL or skip for alpha can be removed.  (That is, 
> one in the GCC testsuite rather than relying on a libffi test to test 
> GCC.)

And if it is in g*.dg/compat/, it can even test ABI compatibility between
different compilers or their versions.

Jakub

Re: [RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Joseph Myers]

On Fri, 2 Sep 2016, Uros Bizjak wrote:

>  argument.  Passing _Complex float as a variable argument never
>  worked on alpha.  Thus, we have no backward compatibility issues

Presumably there should be an architecture-independent execution test of 
passing _Complex float in variable arguments - either new, or a 
pre-existing one whose XFAIL or skip for alpha can be removed.  (That is, 
one in the GCC testsuite rather than relying on a libffi test to test 
GCC.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH][AArch64 - v3] Simplify eh_return implementation

[Wilco Dijkstra]

Ramana Radhakrishnan wrote:
> Can you please file a PR for this and add some testcases ?  This sounds like 
> a serious enough problem that needs to be looked at probably going back since 
> the dawn of time.

I've created PR77455. Updated patch below:

This patch simplifies the handling of the EH return value.  We force the use of 
the
frame pointer so the return location is always at FP + 8.  This means we can 
emit
a simple volatile access in EH_RETURN_HANDLER_RTX without needing md
patterns, splitters and frame offset calculations.  The new implementation also
fixes various bugs in aarch64_final_eh_return_addr, which does not work with
-fomit-frame-pointer, alloca or outgoing arguments.

Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backport
this to GCC6.x?

ChangeLog:

2016-09-02  Wilco Dijkstra  

PR77455
gcc/
* config/aarch64/aarch64.md (eh_return): Remove pattern and splitter.
* config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove.
(EH_RETURN_HANDLER_RTX): New define.
* config/aarch64/aarch64.c (aarch64_frame_pointer_required):
Force frame pointer in EH return functions.
(aarch64_expand_epilogue): Add barrier for eh_return.
(aarch64_final_eh_return_addr): Remove.
(aarch64_eh_return_handler_rtx): New function.
* config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr):
Remove.
(aarch64_eh_return_handler_rtx): New prototype.

testsuite/
* gcc.target/aarch64/eh_return.c: New test.
--
diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index 
3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070
 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode);
 int aarch64_hard_regno_nregs (unsigned, machine_mode);
 int aarch64_uxt_size (int, HOST_WIDE_INT);
 int aarch64_vec_fpconst_pow_of_2 (rtx);
-rtx aarch64_final_eh_return_addr (void);
+rtx aarch64_eh_return_handler_rtx (void);
 rtx aarch64_mask_from_zextract_ops (rtx, rtx);
 const char *aarch64_output_move_struct (rtx *operands);
 rtx aarch64_return_addr (int, rtx);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 
003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853daf08842955288861ec7e7acca
 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version;
 #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL) \
   aarch64_declare_function_name (STR, NAME, DECL)
 
-/* The register that holds the return address in exception handlers.  */
-#define AARCH64_EH_STACKADJ_REGNUM (R0_REGNUM + 4)
-#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, AARCH64_EH_STACKADJ_REGNUM)
+/* For EH returns X4 contains the stack adjustment.  */
+#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM)
+#define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
 
 /* Don't use __builtin_setjmp until we've defined it.  */
 #undef DONT_USE_BUILTIN_SETJMP
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
e742c19d76e6c62117aa62a990b9c2945aa06b74..f07d771ea343803e054e03f59c8c1efb698bf474
 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void)
   && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM)))
 return true;
 
+  /* Force a frame pointer for EH returns so the return address is at FP+8.  */
+  if (crtl->calls_eh_return)
+return true;
+
   return false;
 }
 
@@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)
 + cfun->machine->frame.saved_varargs_size) != 0;
 
   /* Emit a barrier to prevent loads from a deallocated stack.  */
-  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca)
+  if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca
+  || crtl->calls_eh_return)
 {
   emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
   need_barrier_p = false;
@@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)
 emit_jump_insn (ret_rtx);
 }
 
-/* Return the place to copy the exception unwinding return address to.
-   This will probably be a stack slot, but could (in theory be the
-   return register).  */
+/* Implement EH_RETURN_HANDLER_RTX.  The return address is stored at FP + 8.
+   The access needs to be volatile to prevent it from being removed.  */
 rtx
-aarch64_final_eh_return_addr (void)
+aarch64_eh_return_handler_rtx (void)
 {
-  HOST_WIDE_INT fp_offset;
-
-  aarch64_layout_frame ();
-
-  fp_offset = cfun->machine->frame.frame_size
- - cfun->machine->frame.hard_fp_offset;
-
-  if (cfun->machine->frame.reg_offset[LR_REGNUM] < 0)
-return gen_rtx_REG (DImode, LR_REGNUM);
-
-  /* 

Re: [PATCH] Use RPO order for fwprop iteration

[Richard Biener]

On Fri, 2 Sep 2016, Robin Dapp wrote:

> This causes a performance regression in the xalancbmk SPECint2006
> benchmark on s390x. At first sight, the produced asm output doesn't look
> too different but I'll have a closer look. Is the fwprop order supposed
> to have major performance implications?

No, the change may allow a more copy propagation in loops
but I didn't expect any major code generation changes from it.
I've mainly done the change for consistency (for a forward SSA
propagation pass RPO oder is the proper one).

So if you can come up with a testcase I can investigate if the
change is undesired.

Thanks,
Richard.

[RFC PATCH, alpha]: ABI change: pass SFmode and SCmode variable arguments by reference

[Uros Bizjak]

Hello!

I would like to propose an ABI adjustment for the aloha OSF/1 ABI. To
quote explanation in the patch:

--q--
  /* Pass float and _Complex float variable arguments by reference.
 This avoids 64-bit store from a FP register to a pretend args save area
 and subsequent 32-bit load from the saved location to a FP register.

 Note that 32-bit loads and stores to/from a FP register on alpha reorder
 bits to form a canonical 64-bit value in the FP register.  This fact
 invalidates compiler assumption that 32-bit FP value lives in the lower
 32-bits of the passed 64-bit FP value, so loading the 32-bit value from
 the stored 64-bit location using 32-bit FP load is invalid on alpha.

 This introduces sort of ABI incompatibility, but until _Float32 was
 introduced, C-family languages promoted 32-bit float variable arg to
 a 64-bit double, and it was not allowed to pass float as a varible
 argument.  Passing _Complex float as a variable argument never
 worked on alpha.  Thus, we have no backward compatibility issues
 to worry about, and passing un-promoted _Float32 and _Complex float
 as a variable argument will actually work in the future.  */
--/q--

Another rationale for the adjustment is, that "other" compilers do not
know about _Float32 and _Complex float, the official ABI pre-dates
some of these types by a decade or more (I was not able to find the
authoritative OSF/1 ABI document on the net...), and lastly ... gcc is
the last compiler that keeps this dead architecture alive, so IMO we
can consider it as a de-facto implementer of the ABI.

Following this ABI adjustment, we can also fix libffi, where
libffi.complex/cls_complex_va_float.c says:

--q--
/* Alpha splits _Complex into two arguments.  It's illegal to pass
   float through varargs, so _Complex float goes badly.  In sort of
   gets passed as _Complex double, but the compiler doesn't agree
   with itself on this issue.  */
/* { dg-do run { xfail alpha*-*-* } } */
--/q--

2016-09-02  Uros Bizjak  

* config/alpha/alpha.c (alpha_pass_by_reference): Pass un-named
SFmode and SCmode arguments by reference.

Patch was bootstrapped and regression tested on alphaev68-linux-gnu
for all default languages plus obj-c++ and go.

Any comments?

Uros.
diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c
index 702cd27..81cef4e 100644
--- a/gcc/config/alpha/alpha.c
+++ b/gcc/config/alpha/alpha.c
@@ -5754,8 +5754,29 @@ static bool
 alpha_pass_by_reference (cumulative_args_t ca ATTRIBUTE_UNUSED,
 machine_mode mode,
 const_tree type ATTRIBUTE_UNUSED,
-bool named ATTRIBUTE_UNUSED)
+bool named)
 {
+  /* Pass float and _Complex float variable arguments by reference.
+ This avoids 64-bit store from a FP register to a pretend args save area
+ and subsequent 32-bit load from the saved location to a FP register.
+
+ Note that 32-bit loads and stores to/from a FP register on alpha reorder
+ bits to form a canonical 64-bit value in the FP register.  This fact
+ invalidates compiler assumption that 32-bit FP value lives in the lower
+ 32-bits of the passed 64-bit FP value, so loading the 32-bit value from
+ the stored 64-bit location using 32-bit FP load is invalid on alpha.
+
+ This introduces sort of ABI incompatibility, but until _Float32 was
+ introduced, C-family languages promoted 32-bit float variable arg to
+ a 64-bit double, and it was not allowed to pass float as a varible
+ argument.  Passing _Complex float as a variable argument never
+ worked on alpha.  Thus, we have no backward compatibility issues
+ to worry about, and passing unpromoted _Float32 and _Complex float
+ as a variable argument will actually work in the future.  */
+
+  if (mode == SFmode || mode == SCmode)
+return !named;
+
   return mode == TFmode || mode == TCmode;
 }
 

Re: [PATCH] Use RPO order for fwprop iteration

[Robin Dapp]

This causes a performance regression in the xalancbmk SPECint2006
benchmark on s390x. At first sight, the produced asm output doesn't look
too different but I'll have a closer look. Is the fwprop order supposed
to have major performance implications?

Regards
 Robin

> This changes it from PRE on the inverted graph to RPO order which works
> better for loops and blocks with no path to exit.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
> 
> Richard.
> 
> 2016-08-22  Richard Biener  
> 
>   * tree-ssa-forwprop.c (pass_forwprop::execute): Use RPO order.
> 
> Index: gcc/tree-ssa-forwprop.c
> ===
> --- gcc/tree-ssa-forwprop.c   (revision 239607)
> +++ gcc/tree-ssa-forwprop.c   (working copy)
> @@ -2099,7 +2099,8 @@ pass_forwprop::execute (function *fun)
>lattice.create (num_ssa_names);
>lattice.quick_grow_cleared (num_ssa_names);
>int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (fun));
> -  int postorder_num = inverted_post_order_compute (postorder);
> +  int postorder_num = pre_and_rev_post_order_compute_fn (cfun, NULL,
> +  postorder, false);
>auto_vec to_fixup;
>to_purge = BITMAP_ALLOC (NULL);
>for (int i = 0; i < postorder_num; ++i)
> 

[ipa-cp] Change option name from -fipa-cp-bit to -fipa-bit-cp in ipcp_store_bits_results()

[Prathamesh Kulkarni]

Committed as obvious after bootstrap+test on x86_64-unknown-linux-gnu.

Thanks,
Prathamesh
2016-09-02  Prathamesh Kulkarni  

* ipa-cp.c (ipcp_store_bits_results): Change option name from
-fipa-cp-bit to -fipa-bit-cp.

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 9514dd1..5ff7bed 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4917,7 +4917,7 @@ ipcp_store_bits_results (void)
{
  if (dump_file)
fprintf (dump_file, "Not considering %s for ipa bitwise propagation 
"
-   "; -fipa-cp-bit: disabled.\n",
+   "; -fipa-bit-cp: disabled.\n",
node->name ());
  continue;
}

Re: [RFC][IPA-VRP] Early VRP Implementation

[Kugan Vivekanandarajah]

Ping ?

Thanks,
Kugan

On 23 August 2016 at 12:11, Kugan Vivekanandarajah
 wrote:
> Hi,
>
> On 19 August 2016 at 21:41, Richard Biener  wrote:
>> On Tue, Aug 16, 2016 at 9:45 AM, kugan
>>  wrote:
>>> Hi Richard,
>>>
>>> On 12/08/16 20:43, Richard Biener wrote:

 On Wed, Aug 3, 2016 at 3:17 AM, kugan 
 wrote:
>>>
>>>
>>> [SNIP]
>>>

 diff --git a/gcc/common.opt b/gcc/common.opt
 index 8a292ed..7028cd4 100644
 --- a/gcc/common.opt
 +++ b/gcc/common.opt
 @@ -2482,6 +2482,10 @@ ftree-vrp
  Common Report Var(flag_tree_vrp) Init(0) Optimization
  Perform Value Range Propagation on trees.

 +fdisable-tree-evrp
 +Common Report Var(flag_disable_early_vrp) Init(0) Optimization
 +Disable Early Value Range Propagation on trees.
 +

 no please, this is automatically supported via -fdisable-
>>>
>>>
>>> I am now having -ftree-evrp which is enabled all the time. But This will
>>> only be used for disabling the early-vrp. That is, early-vrp will be run
>>> when ftree-vrp is enabled and ftree-evrp is not explicitly disabled. Is this
>>> OK?
>>
>> Why would one want to disable early-vrp?  I see you do this in the testsuite
>> for non-early VRP unit-tests but using -fdisable-tree-evrp1 there
>> would be ok as well.
>
> Removed it altogether. I though that you wanted a way to disable
> early-vrp for testing purposes.
>

 @@ -1728,11 +1736,12 @@ extract_range_from_assert (value_range *vr_p, tree
 expr)
  always false.  */

  static void
 -extract_range_from_ssa_name (value_range *vr, tree var)
 +extract_range_from_ssa_name (value_range *vr, bool dom_p, tree var)
  {
value_range *var_vr = get_value_range (var);

 -  if (var_vr->type != VR_VARYING)
 +  if (var_vr->type != VR_VARYING
 +  && (!dom_p || var_vr->type != VR_UNDEFINED))
  copy_value_range (vr, var_vr);
else
  set_value_range (vr, VR_RANGE, var, var, NULL);

 why do you need these changes?  I think I already told you you need to
 initialize the lattice to sth else than VR_UNDEFINED and that you can't
 fully re-use update_value_range.  If you don't want to do that then
 instead
 of doing changes all over the place do it in get_value_range and have a
 global flag.
>>>
>>>
>>> I have now added a global early_vrp_p and use this to initialize
>>> VR_INITIALIZER and get_value_range default to VR_VARYING.
>>
>> ICK.  Ok, I see that this works, but it is quite ugly, so (see below)
>>


 @@ -3594,7 +3643,8 @@ extract_range_from_cond_expr (value_range *vr,
 gassign *stmt)
 on the range of its operand and the expression code.  */

  static void
 -extract_range_from_comparison (value_range *vr, enum tree_code code,
 +extract_range_from_comparison (value_range *vr,
 +  enum tree_code code,
tree type, tree op0, tree op1)
  {
bool sop = false;

 remove these kind of no-op changes.
>>>
>>>
>>> Done.
>>>
>>>

 +/* Initialize local data structures for VRP.  If DOM_P is true,
 +   we will be calling this from early_vrp where value range propagation
 +   is done by visiting stmts in dominator tree.  ssa_propagate engine
 +   is not used in this case and that part of the ininitialization will
 +   be skipped.  */

  static void
 -vrp_initialize (void)
 +vrp_initialize (bool dom_p)
  {
basic_block bb;

 @@ -6949,6 +7010,9 @@ vrp_initialize (void)
vr_phi_edge_counts = XCNEWVEC (int, num_ssa_names);
bitmap_obstack_initialize (_equiv_obstack);

 +  if (dom_p)
 +return;
 +

 split the function instead.

 @@ -7926,7 +7992,8 @@ vrp_visit_switch_stmt (gswitch *stmt, edge
 *taken_edge_p)
 If STMT produces a varying value, return SSA_PROP_VARYING.  */

  static enum ssa_prop_result
 -vrp_visit_stmt (gimple *stmt, edge *taken_edge_p, tree *output_p)
 +vrp_visit_stmt_worker (gimple *stmt, bool dom_p,  edge *taken_edge_p,
 +  tree *output_p)
  {
tree def;
ssa_op_iter iter;
 @@ -7940,7 +8007,7 @@ vrp_visit_stmt (gimple *stmt, edge
 *taken_edge_p, tree *output_p)
if (!stmt_interesting_for_vrp (stmt))
  gcc_assert (stmt_ends_bb_p (stmt));
else if (is_gimple_assign (stmt) || is_gimple_call (stmt))
 -return vrp_visit_assignment_or_call (stmt, output_p);
 +return vrp_visit_assignment_or_call (stmt, dom_p, output_p);
else if (gimple_code (stmt) == GIMPLE_COND)
  return vrp_visit_cond_stmt (as_a  (stmt), taken_edge_p);
else if (gimple_code (stmt) == GIMPLE_SWITCH)
 @@ -7954,6 +8021,12 @@ vrp_visit_stmt (gimple 

Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments

[Kugan Vivekanandarajah]

Hi Richard,

On 25 August 2016 at 22:24, Richard Biener  wrote:
> On Thu, Aug 11, 2016 at 1:09 AM, kugan
>  wrote:
>> Hi,
>>
>>
>> On 10/08/16 20:28, Richard Biener wrote:
>>>
>>> On Wed, Aug 10, 2016 at 10:57 AM, Jakub Jelinek  wrote:

 On Wed, Aug 10, 2016 at 08:51:32AM +1000, kugan wrote:
>
> I see it now. The problem is we are just looking at (-1) being in the
> ops
> list for passing changed to rewrite_expr_tree in the case of
> multiplication
> by negate.  If we have combined (-1), as in the testcase, we will not
> have
> the (-1) and will pass changed=false to rewrite_expr_tree.
>
> We should set changed based on what happens in try_special_add_to_ops.
> Attached patch does this. Bootstrap and regression testing are ongoing.
> Is
> this OK for trunk if there is no regression.


 I think the bug is elsewhere.  In particular in
 undistribute_ops_list/zero_one_operation/decrement_power.
 All those look problematic in this regard, they change RHS of statements
 to something that holds a different value, while keeping the LHS.
 So, generally you should instead just add a new stmt next to the old one,
 and adjust data structures (replace the old SSA_NAME in some ->op with
 the new one).  decrement_power might be a problem here, dunno if all the
 builtins are const in all cases that DSE would kill the old one,
 Richard, any preferences for that?  reset flow sensitive info + reset
 debug
 stmt uses, or something different?  Though, replacing the LHS with a new
 anonymous SSA_NAME might be needed too, in case it is before SSA_NAME of
 a
 user var that doesn't yet have any debug stmts.
>>>
>>>
>>> I'd say replacing the LHS is the way to go, with calling the appropriate
>>> helper
>>> on the old stmt to generate a debug stmt for it / its uses (would need
>>> to look it
>>> up here).
>>>
>>
>> Here is an attempt to fix it. The problem arises when in
>> undistribute_ops_list, we linearize_expr_tree such that NEGATE_EXPR is added
>> (-1) MULT_EXPR (OP). Real problem starts when we handle this in
>> zero_one_operation. Unlike what was done earlier, we now change the stmt
>> (with propagate_op_to_signle use or by directly) such that the value
>> computed by stmt is no longer what it used to be. Because of this, what is
>> computed in undistribute_ops_list and rewrite_expr_tree are also changed.
>>
>> undistribute_ops_list already expects this but rewrite_expr_tree will not if
>> we dont pass the changed as an argument.
>>
>> The way I am fixing this now is, in linearize_expr_tree, I set ops_changed
>> to true if we change NEGATE_EXPR to (-1) MULT_EXPR (OP). Then when we call
>> zero_one_operation with ops_changed = true, I replace all the LHS in
>> zero_one_operation with the new SSA and replace all the uses. I also call
>> the rewrite_expr_tree with changed = false in this case.
>>
>> Does this make sense? Bootstrapped and regression tested for
>> x86_64-linux-gnu without any new regressions.
>
> I don't think this solves the issue.  zero_one_operation associates the
> chain starting at the first *def and it will change the intermediate values
> of _all_ of the stmts visited until the operation to be removed is found.
> Note that this is independent of whether try_special_add_to_ops did anything.
>
> Even for the regular undistribution cases we get this wrong.
>
> So we need to back-track in zero_one_operation, replacing each LHS
> and in the end the op in the opvector of the main chain.  That's basically
> the same as if we'd do a regular re-assoc operation on the sub-chains.
> Take their subops, simulate zero_one_operation by
> appending the cancelling operation and optimizing the oplist, and then
> materializing the associated ops via rewrite_expr_tree.
>
Here is a draft patch which records the stmt chain when in
zero_one_operation and then fixes it when OP is removed. when we
update *def, that will update the ops vector. Does this looks sane?


Bootstrapped and regression tested on x86_64-linux-gnu with no new regressions.

Thanks,
Kugan
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c
index e69de29..049eddc 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c
@@ -0,0 +1,36 @@
+/* PR tree-optimization/72835.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct struct_1 {
+unsigned int m1 : 6 ;
+unsigned int m2 : 24 ;
+unsigned int m3 : 6 ;
+};
+
+unsigned short var_32 = 0x2d10;
+
+struct struct_1 s1;
+
+void init ()
+{
+  s1.m1 = 4;
+  s1.m2 = 0x7ca4b8;
+  s1.m3 = 24;
+}
+
+void foo ()
+{
+  unsigned int c
+= ((unsigned int) s1.m2) * (-((unsigned int) s1.m3))
++ (var_32) * (-((unsigned int) (s1.m1)));
+  if (c != 4098873984)
+__builtin_abort ();
+}
+
+int main ()
+{
+init ();
+   

[Fortran, Patch, PR72832, v1] [6/7 Regression] [OOP] ALLOCATE with SOURCE fails to allocate requested dimensions

[Andre Vehreschild]

Hi all,

attached patch fixes the issue raised by PR72832. The issue was that
the array descriptor of the SOURCE= in an ALLOCATE () was used to
allocate an array object although an explicit array spec had been
given.

The initial test showed a second issue when a class array was copied.
Compiling the code with -fcheck=bounds showed that no boundary check
was generated for class array copying using gfc_copy_class_to_class().
I have added the generation of a runtime boundary check when the
-fcheck=bounds flag is set to locate the current issue. The test
allocate_with_source_23 is compiled with fcheck=bounds and fails as
expected ({ xfail *-*-* } set).

Fixing the both issues unfortunately raised the next one, when trying
to get the size of a class array returned from a function (testcase:
allocate_with_source_11.f08). Here the issue was that for functions
returning class arrays gfc_conv_expr_descriptor () relied on the
descriptor being magicked into the scalarizer, which did not work in
this use case. Due to the first issue this bug did not raise beforehand.
Because I could not figure how to do it right in
gfc_conv_expr_descriptor (), I found a way to circumvent the issue by
getting the reference of the result of the function returning a class
array and massaging it to be ok for size (). This works quite neatly,
but may be someone with better knowledge of conv_expr_descriptor and
the scalarizer might want to fix it there. I suppose there are more
locations in the code, that work around this issue.

Bootstrapped and regtests ok on x86_64-linux-gnu/F23 for trunk and
gcc-6. Ok for both?

- Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 
gcc/fortran/ChangeLog:

2016-09-01  Andre Vehreschild  

PR fortran/72832
* trans-expr.c (gfc_copy_class_to_class): Add generation of
runtime array bounds check.
* trans-intrinsic.c (gfc_conv_intrinsic_size): Add a crutch to
get the descriptor of a function returning a class object.
* trans-stmt.c (gfc_trans_allocate): Use the array spec on the
array to allocate instead of the array spec from source=.

gcc/testsuite/ChangeLog:

2016-09-01  Andre Vehreschild  

PR fortran/72832
* gfortran.dg/allocate_with_source_22.f03: New test.
* gfortran.dg/allocate_with_source_23.f03: New test.  Expected to
fail.


diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 19239fb..4d2fd33 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -1180,6 +1180,7 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
   stmtblock_t body;
   stmtblock_t ifbody;
   gfc_loopinfo loop;
+  tree orig_nelems = nelems; /* Needed for bounds check.  */
 
   gfc_init_block ();
   tmp = fold_build2_loc (input_location, MINUS_EXPR,
@@ -1207,6 +1208,31 @@ gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
 	}
   vec_safe_push (args, to_ref);
 
+  /* Add bounds check.  */
+  if ((gfc_option.rtcheck & GFC_RTCHECK_BOUNDS) > 0 && is_from_desc)
+	{
+	  char *msg;
+	  const char *name = "<>";
+	  tree from_len;
+
+	  if (DECL_P (to))
+	name = (const char *)(DECL_NAME (to)->identifier.id.str);
+
+	  from_len = gfc_conv_descriptor_size (from_data, 1);
+	  tmp = fold_build2_loc (input_location, NE_EXPR,
+  boolean_type_node, from_len, orig_nelems);
+	  msg = xasprintf ("Array bound mismatch for dimension %d "
+			   "of array '%s' (%%ld/%%ld)",
+			   1, name);
+
+	  gfc_trans_runtime_check (true, false, tmp, ,
+   _current_locus, msg,
+			 fold_convert (long_integer_type_node, orig_nelems),
+			   fold_convert (long_integer_type_node, from_len));
+
+	  free (msg);
+	}
+
   tmp = build_call_vec (fcn_type, fcn, args);
 
   /* Build the body of the loop.  */
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index 8167842..d4ff85c 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -5845,9 +5845,20 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr)
   if (actual->expr->ts.type == BT_CLASS)
 gfc_add_class_array_ref (actual->expr);
 
-  argse.want_pointer = 1;
   argse.data_not_needed = 1;
-  gfc_conv_expr_descriptor (, actual->expr);
+  if (gfc_is_alloc_class_array_function (actual->expr))
+{
+  /* For functions that return a class array conv_expr_descriptor is not
+	 able to get the descriptor right.  Therefore this special case.  */
+  gfc_conv_expr_reference (, actual->expr);
+  argse.expr = gfc_build_addr_expr (NULL_TREE,
+	gfc_class_data_get (argse.expr));
+}
+  else
+{
+  argse.want_pointer = 1;
+  gfc_conv_expr_descriptor (, actual->expr);
+}
   gfc_add_block_to_block (>pre, );
   gfc_add_block_to_block (>post, );
   arg1 = gfc_evaluate_now (argse.expr, >pre);
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 

Re: [PATCH, docs] invoke.texi: random copy-editing

[Gerald Pfeifer]

On Wed, 29 Aug 2012, Sandra Loosemore wrote:
>   * doc/invoke.texi: Fix numerous typos and punctuation/grammatical
>   errors throughout the file.  Re-word some awkward sentences and
>   paragraphs.

There are three questions (and to some extent suggestions) on this 
patch and the text covered by it that I'm wondering about.  Hope
that's still fine after all the time.

I'm happy to make any changes myself, but am looking at your expertise.


 Item 11:  Define a copy constructor and an assignment operator for classes
-with dynamically allocated memory.
+with dynamically-allocated memory.

Why the dash here?  Is this because it's seens as a technical term?  
(Usually it's the Germans with those absolutelylongandnonbreaking words. 
;-)


-(C++ only) A base class is not initialized in a derived class' copy
+(C++ only) A base class is not initialized in a derived class's copy
 constructor.

"class's" twists my brain a little.  What do you think about using
"in a copy constructor of a derived class" instead?


 When profile feedback is available (see @option{-fprofile-generate}) the actual
-recursion depth can be guessed from probability that function will recurse via
-given call expression.  This parameter limits inlining only to call expression
-whose probability exceeds given threshold (in percents).  The default value is
-10.
+recursion depth can be guessed from probability that function recurses via a
+given call expression.  This parameter limits inlining only to call expressions
+whose probability exceeds the given threshold (in percents).

This predates your patch, but should this be "the probability"?

Gerald