Re: [PATCH][C++] Fix PR71694, store data race with tail-padding

[Richard Biener]

On December 19, 2016 11:25:49 PM GMT+01:00, Jeff Law  wrote:
>On 12/14/2016 03:44 AM, Richard Biener wrote:
>>
>> The following implements Jasons suggestion of using a langhook to
>> return the size of an aggregate without tail padding that might
>> be re-used when it is inherited from.
>>
>> Using this langhook we can fix the size of the representative for the
>> bitfield properly and thus generate correct code for the testcase.
>> Previously on x86_64 it was
>>
>> main:
>> movlc+4(%rip), %eax
>> andl$-258049, %eax
>> orb $16, %ah
>> movl%eax, c+4(%rip)
>> movb$2, c+7(%rip)
>> xorl%eax, %eax
>> ret
>>
>> while after the patch we see
>>
>> main:
>> movzbl  c+5(%rip), %eax
>> andb$-4, c+6(%rip)
>> movb$2, c+7(%rip)
>> andl$15, %eax
>> orl $16, %eax
>> movb%al, c+5(%rip)
>> xorl%eax, %eax
>> ret
>>
>> in particular the store to C::B::d is now properly using a byte
>store.
>>
>> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu, ok for
>> trunk?
>>
>> Thanks,
>> Richard.
>>
>> 2016-12-14  Richard Biener  
>>
>>  PR c++/71694
>>  * langhooks-def.h (lhd_unit_size_without_reusable_padding): Declare.
>>  (LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
>>  (LANG_HOOKS_FOR_TYPES_INITIALIZER): Adjust.
>>  * langhooks.h (struct lang_hooks_for_types): Add
>>  unit_size_without_reusable_padding.
>>  * langhooks.c (lhd_unit_size_without_reusable_padding): New.
>>  * stor-layout.c (finish_bitfield_representative): Use
>>  unit_size_without_reusable_padding langhook to decide on the
>>  last representatives size.
>>
>>  cp/
>>  * cp-objcp-common.h (cp_unit_size_without_reusable_padding):
>Declare.
>>  (LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
>>  * cp-objcp-common.c (cp_unit_size_without_reusable_padding): New.
>>
>>  * g++.dg/pr71694.C: New testcase.
>>
>> Index: gcc/stor-layout.c
>> ===
>> *** gcc/stor-layout.c(revision 243632)
>> --- gcc/stor-layout.c(working copy)
>> *** finish_bitfield_representative (tree rep
>> *** 1864,1876 
>>   }
>> else
>>   {
>> !   /* ???  If you consider that tail-padding of this struct
>might be
>> !  re-used when deriving from it we cannot really do the
>following
>> ! and thus need to set maxsize to bitsize?  Also we cannot
>> ! generally rely on maxsize to fold to an integer constant, so
>> ! use bitsize as fallback for this case.  */
>> !   tree maxsize = size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT
>(field)),
>> !  DECL_FIELD_OFFSET (repr));
>> if (tree_fits_uhwi_p (maxsize))
>>  maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
>>- tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
>> --- 1864,1877 
>>   }
>> else
>>   {
>> !   /* Note that if the C++ FE sets up tail-padding to be re-used
>it
>> !  creates a as-base variant of the type with TYPE_SIZE
>adjusted
>> ! accordingly.  So it is safe to include tail-padding here.  */
>> !   tree aggsize =
>lang_hooks.types.unit_size_without_reusable_padding
>> !(DECL_CONTEXT (field));
>> !   tree maxsize = size_diffop (aggsize, DECL_FIELD_OFFSET
>(repr));
>> !   /* We cannot generally rely on maxsize to fold to an integer
>constant,
>> ! so use bitsize as fallback for this case.  */
>> if (tree_fits_uhwi_p (maxsize))
>>  maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
>>- tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
>Looks reasonable, formatting nit in the comment above the assignment to
>
>AGGSIZE (looks like it was mucked up in the old version too).  I think 
>the second line is using spaces when it should have used a tab.
>
>Do we document langhooks anywhere?

Didn't find anything.  Applied already after Jason's OK.

Richard.

>Jeff

[PATCH] Add testcases to test builtin-expansion of memcmp and strncmp

[Aaron Sawdey]

This patch adds tests gcc.dg/memcmp-1.c and gcc.dg/strncmp-1.c that
test builtin expansion of memcmp and strncmp for short strings and also
varying alignment of one arg. The strncmp test checks that things work
when one of the strings crosses a 4k boundary as well.

I've included interested parties from targets that have a strncmp
builtin.

The tests pass on ppc64le and x86_64. OK for trunk?

-- 
Aaron Sawdey, Ph.D.  acsaw...@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC ToolchainIndex: gcc/testsuite/gcc.dg/memcmp-1.c
===
--- gcc/testsuite/gcc.dg/memcmp-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/memcmp-1.c	(working copy)
@@ -0,0 +1,612 @@
+/* Test memcmp builtin expansion for compilation and proper execution.  */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+#include 
+#include 
+#include 
+
+#define RUN_TEST(SZ, ALIGN) test_memcmp_ ## SZ ## _ ## ALIGN ()
+
+#define DEF_TEST(SZ, ALIGN)\
+static void test_memcmp_ ## SZ ## _ ## ALIGN (void) {  \
+  char one[3 * (SZ > 10 ? SZ : 10)];     	   \
+  char two[3 * (SZ > 10 ? SZ : 10)];   	   \
+  int i,j;   \
+  for (i = 0 ; i < SZ ; i++)			   		   \
+{			   		   \
+  int r1;   \
+  char *a = one + (i & 1) * ALIGN;			   		   \
+  char *b = two + (i & 1) * ALIGN;			   		   \
+  memset (a, '-', SZ);	   	   \
+  memset (b, '-', SZ);	   	   \
+  a[i] = '1';	   		   \
+  b[i] = '2';	   		   \
+  a[SZ] = 0;			   \
+  b[SZ] = 0;	   		   \
+  if (!((r1 = memcmp (b, a, SZ)) > 0))   		   		   \
+{   \
+	  printf("FAIL SZ %d ALIGN %d i=%d b > a r1=%d a='%s' b='%s'n",\
+		 SZ, ALIGN, i, r1, a, b);   \
+	  abort ();			   \
+	}   \
+  if (!((r1 = memcmp (a, b, SZ)) < 0))			   	   \
+{   \
+	  printf("FAIL SZ %d ALIGN %d i=%d a < b r1=%d a='%s' b='%s'n",\
+		 SZ, ALIGN, i, r1, a, b);   \
+	  abort ();			   \
+	}	   \
+  b[i] = '1';	   		   \
+  if (!((r1 = memcmp (a, b, SZ)) == 0))		   		   \
+{   \
+	  printf("FAIL SZ %d ALIGN %d i=%d a == b r1=%d a='%s' b='%s'n",   \
+		 SZ, ALIGN, i, r1, a, b);   \
+	  abort ();			   \
+	}	   \
+  for(j = i; j < SZ ; j++)			   		   \
+	{		   		   \
+	  a[j] = '1';			   		   \
+	  b[j] = '2';			   		   \
+	}		   		   \
+  if (!((r1 = memcmp (b, a, SZ)) > 0))			   	   \
+{   \
+	  printf("FAIL SZ %d ALIGN %d i=%d b > a r1=%d a='%s' b='%s'n",\
+		 SZ, ALIGN, i, r1, a, b);   \
+	  abort ();			   \
+	} 			   \
+  if (!((r1 = memcmp (a, b, SZ)) < 0))			   	   \
+{   \
+	  printf("FAIL SZ %d ALIGN %d i=%d a < b r1=%d a='%s' b='%s'n",\
+		 SZ, ALIGN, i, r1, a, b);   \
+	  abort ();			   \
+	}	   		   \
+}			   \
+}
+
+#ifdef TEST_ALL
+DEF_TEST(1,1)
+DEF_TEST(1,2)
+DEF_TEST(1,4)
+DEF_TEST(1,8)
+DEF_TEST(1,16)
+DEF_TEST(2,1)
+DEF_TEST(2,2)
+DEF_TEST(2,4)
+DEF_TEST(2,8)
+DEF_TEST(2,16)
+DEF_TEST(3,1)
+DEF_TEST(3,2)
+DEF_TEST(3,4)
+DEF_TEST(3,8)
+DEF_TEST(3,16)
+DEF_TEST(4,1)
+DEF_TEST(4,2)
+DEF_TEST(4,4)
+DEF_TEST(4,8)
+DEF_TEST(4,16)
+DEF_TEST(5,1)
+DEF_TEST(5,2)
+DEF_TEST(5,4)
+DEF_TEST(5,8)
+DEF_TEST(5,16)
+DEF_TEST(6,1)
+DEF_TEST(6,2)
+DEF_TEST(6,4)
+DEF_TEST(6,8)
+DEF_TEST(6,16)
+DEF_TEST(7,1)
+DEF_TEST(7,2)
+DEF_TEST(7,4)
+DEF_TEST(7,8)
+DEF_TEST(7,16)
+DEF_TEST(8,1)
+DEF_TEST(8,2)
+DEF_TEST(8,4)
+DEF_TEST(8,8)
+DEF_TEST(8,16)
+DEF_TEST(9,1)
+DEF_TEST(9,2)
+DEF_TEST(9,4)
+DEF_TEST(9,8)
+DEF_TEST(9,16)
+DEF_TEST(10,1)
+DEF_TEST(10,2)
+DEF_TEST(10,4)
+DEF_TEST(10,8)
+DEF_TEST(10,16)
+DEF_TEST(11,1)
+DEF_TEST(11,2)
+DEF_TEST(11,4)
+DEF_TEST(11,8)
+DEF_TEST(11,16)
+DEF_TEST(12,1)
+DEF_TEST(12,2)
+DEF_TEST(12,4)
+DEF_TEST(12,8)
+DEF_TEST(12,16)
+DEF_TEST(13,1)
+DEF_TEST(13,2)
+DEF_TEST(13,4)
+DEF_TEST(13,8)
+DEF_TEST(13,16)
+DEF_TEST(14,1)
+DEF_TEST(14,2)
+DEF_TEST(14,4)
+DEF_TEST(14,8)
+DEF_TEST(14,16)
+DEF_TEST(15,1)
+DEF_TEST(15,2)
+DEF_TEST(15,4)
+DEF_TEST(15,8)
+DEF_TEST(15,16)
+DEF_TEST(16,1)
+DEF_TEST(16,2)
+DEF_TEST(16,4)
+DEF_TEST(16,8)
+DEF_TEST(16,16)
+DEF_TEST(17,1)
+DEF_TEST(17,2)
+DEF_TEST(17,4)
+DEF_TEST(17,8)
+DEF_TEST(17,16)
+DEF_TEST(18,1)
+DEF_TEST(18,2)
+DEF_TEST(18,4)
+DEF_TEST(18,8)
+DEF_TEST(18,16)
+DEF_TEST(19,1)
+DEF_TEST(19,2)
+DEF_TEST(19,4)
+DEF_TEST(19,8)
+DEF_TEST(19,16)
+DEF_TEST(20,1)
+DEF_TEST(20,2)
+DEF_TEST(20,4)
+DEF_TEST(20,8)
+DEF_TEST(20,16)
+DEF_TEST(21,1)
+DEF_TEST(21,2)
+DEF_TEST(21,4)
+DEF_TEST(21,8)
+DEF_TEST(21,16)
+DEF_TEST(22,1)
+DEF_TEST(22,2)
+DEF_TEST(22,4)
+DEF_TEST(22,8)
+DEF_TEST(22,16)
+DEF_TEST(23,1)
+DEF_TEST(23,2)

Re: [PATCH] PR 78534 Change character length from int to size_t

[Bob Deen]

On 12/19/16 11:33 AM, Janne Blomqvist wrote:

On Mon, Dec 19, 2016 at 6:43 PM, Bob Deen  wrote:

Hi all...

I never saw any followup on this...?

It's one thing to break the ABI between the compiler and the gfortran
library; those can generally be expected to be in sync.  It's another to
break the ABI between two *languages*, when there might be no such
expectation (especially if gcc does NOT break their ABI at the same version
number transition).  Yes, the pre-ISO_C_BINDING method may be old-fashioned,
but it is a de-facto standard, and breaking it should not be done lightly.


First: No, it's not done "lightly". And secondly, cross-language
interfacing is always tricky, which is why most languages, including
Fortran with ISO_C_BINDING, have devised standardized ways for
communication with C so users don't need to rely on various cross-call
mechanisms that are not guaranteed to work.


Apologies if I offended (and to Steve too).  I see all the deliberation 
you're doing for breaking the language->library ABI, and appreciate 
that.  It's well-justified.  My point, however, is that with this change 
you are breaking an entirely *different* ABI - that between Fortran and 
C - and the sum total of discussion was one message from Janne pointing 
out that it was breaking (thanks for that heads-up, I had missed it!), 
with no followup.  Janne, you yourself in that message questioned the 
need for large strings, and had no use cases in response to FX's inquiry.


Now that I think about it, it's not even an ABI change, it's an API 
change... requiring a code change, not just a recompile.


So in this case, this change represents (AFAIK) the only breakage in the 
old-style Fortran<->C ABI/API, with no known use cases... and thus my 
question about whether it's justified.  It's a fair question.  I'm not 
arguing the language->library ABI at all.



C changed to use size_t for string lengths instead of int with ANSI C
in, what, 1989.  With 2-socket servers, typically used e.g. in HPC
clusters, today easily having hundreds of gigs of RAM, limiting
GFortran char lengths to 2 GB for all eternity in the name of
compatibility seems quaint at best. Maybe in your organization Fortran
is legacy code that YE SHALL NOT TOUCH, but GFortran also has to cater
to users who have chosen to write new code in Fortran.


I understand that.  It just seems that opening up an entirely *new* 
ABI/API for breakage deserved a little more discussion.  Y'all are the 
ones doing the (mostly volunteer) work on gfortran, and I appreciate it. 
 You're also much more invested in the future of the language than I 
(yeah, it's mostly legacy code for us).  If you end up deciding that it 
needs to be done, then I'll deal with it.  I just wanted to chime in 
that there are users who will be affected.  If I'm the only one, I 
wouldn't want to stand in the way of progress - but also don't want to 
get steamrolled if it's not an important change, or if there are other 
affected users.


So... ARE there any other affected users out there??


Oh, you have macros rather than hard-coded int all over the place?
Shouldn't it be a relatively trivial affair then to define that macro
appropriately depending on which compiler and which version you're
using?


I wouldn't call it trivial by any means... it's tricky code I haven't 
had to look at in 10 years.  But in the end, probably doable.



Steve showed how you can do it for Fortran. From the C side, just
check the version from the __GNUC__ macro.


I dislike having to check for version numbers (feels kludgy) but that's 
a personal preference.  That will probably work, with a bit of futzing.


Thanks for your attention...

-Bob

Bob Deen @ NASA-JPL Multimission Image Processing Lab
bob.d...@jpl.nasa.gov

Re: Reorganise machmode.h headers

[Jeff Law]

On 11/16/2016 09:32 AM, Richard Sandiford wrote:

Later patches will make machmode.h rely on wide-int.h and the
new poly-int.h, so it needs to appear later in the coretypes.h
include list.

Previously machmode.h included insn-modes.h, which as well as
the main mode enum contains configuration information like
MAX_BITSIZE_MODE_ANY_INT.  This still needs to come first,
since files like wide-int.h depend on the configuration
information.

Similarly, later patches will make the auto-generated inline
mode size functions use poly-int.h, so the patch splits them
out into their own header file and includes it after the
integer utilities.

The patch also makes the generator files include machmode.h
via coretypes.h.  Previously they did it by more indirect means.

Finally, the patch makes wide-int-print.h available via coretypes.h
too.  There didn't seem to be any reason to force only the print
routines to be included directly, and it would be painful to extend
that approach to the new polynomial integer classes.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Thanks,
Richard


[ This patch is part of the SVE series posted here:
  https://gcc.gnu.org/ml/gcc/2016-11/msg00030.html ]

gcc/
2016-11-16  Richard Sandiford  
Alan Hayward  
David Sherwood  

* Makefile.in (MACHMODE_H): Remove insn-modes.h
(CORETYPES_H): New define.
(MOSTLYCLEANFILES): Add insn-modes-inline.h.
(insn-modes-inline.h, s-modes-inline-h): New rules.
(generated_files): Add insn-modes-inline.h.
(RTL_BASE_H, TREE_CORE_H): Use CORETYPES_H instead of coretypes.h.
(build/gensupport.o, build/print-rtl.o, build/read-md.o): Likewise.
(build/read-rtl.o, build/rtl.o, build/vec.o, build/hash-table.o)
(build/inchash.o, build/gencondmd.o, build/genattr.o): Likewise.
(build/genattr-common.o, build/genattrtab.o, build/genautomata.o)
(build/gencheck.o, build/gencodes.o, build/genconditions.o): Likewise.
(build/genconfig.o, build/genconstants.o, build/genemit.o): Likewise.
(build/genenums.o, build/genextract.o, build/genflags.o): Likewise.
(build/gentarget-def.o, build/genmddeps.o, build/genopinit.o)
(build/genoutput.o, build/genpeep.o, build/genpreds.o): Likewise.
(build/genrecog.o, build/genmddump.o, build/genmatch.o): Likewise.
(build/gencfn-macros.o, build/gcov-iov.o): Likewise.
* coretypes.h: Include everything up to real.h for generators.
Include insn-modes.h first.  Include wide-int-print.h after
wide-int.h.  Include insn-modes-inline.h and then machmode.h.
* machmode.h: Don't include insn-modes.h here.
* function-tests.c: Remove includes of signop.h, machmode.h,
double-int.h and wide-int.h.
* rtl.h: Likewise.
* gcc-rich-location.c: Remove includes of machmode.h, double-int.h
and wide-int.h.
* optc-save-gen.awk: Likewise.
* gencheck.c (BITS_PER_UNIT): Delete dummy definition.
* godump.c: Remove include of wide-int-print.h.
* pretty-print.h: Likewise.
* wide-int-print.cc: Likewise.
* wide-int.cc: Likewise.
* hash-map-tests.c: Remove include of signop.h.
* hash-set-tests.c: Likewise.
* rtl-tests.c: Likewise.
* mkconfig.sh: Remove include of machmode.h.
* genmodes.c (emit_insn_modes_h): Split emission of inline functions
into...
(emit_insn_modes_inline_h): ...this new function.  Emit the code
into an insn-modes-inline.h header file, adding appropriate
include guards and end comments.
(emit_insn_modes_c_header): Remove include of machmode.h.
(emit_min_insn_modes_c_header): Include coretypes.h rather than
machmode.h.
(main): Handle -i flag and call emit_insn_modes_inline_h when
it is passed.
So I don't see anything here particularly problematical.  My question is 
whether or not there's anything significant to be gained to moving 
forward with this kit, assuming the 67 piece kit is not likely to move 
forward.


I do think you'll need some tweaks to the contrib/header-tools which 
know about the core headers and dependencies.  Hopefully what's in there 
is easy enough to figure out how to twiddle appropriately.



Jeff

Re: [PR tree-optimization/71691] Fix unswitching in presence of maybe-undef SSA_NAMEs (take 2)

[Jeff Law]

On 12/16/2016 07:41 AM, Aldy Hernandez wrote:


BTW, I don't understand why we don't have auto_bitmap's, as we already
have auto_sbitmap's.  I've implemented the former based on
auto_sbitmap's code we already have.
Trevor poked at it a bit.  bitmaps are a bit more complex than sbitmaps 
in terms of implementation details.


https://gcc.gnu.org/ml/gcc/2016-04/msg00013.html

But his suggestion was to first create auto_bitmap, then look to convert 
to using that as opposed to his other approaches.





The attached patch fixes the bug without introducing any regressions.

I also tested the patch by compiling 242 .ii files with -O3.  These were
gathered from a stage1 build with -save-temps.  There is a slight time
degradation of 4 seconds within 27 minutes of user time:

tainted:26:52
orig:26:48

This was the average aggregate time of two runs compiling all 242 .ii
files.  IMO, this looks reasonable.  It is after all, -O3.Is it
acceptable?

Aldy

curr


commit 2310bcd0e2552a40ca1de354faf005ed3e9daf4e
Author: Aldy Hernandez 
Date:   Fri Dec 16 03:44:52 2016 -0500

PR tree-optimization/71691
* bitmap.h (class auto_bitmap): New.
* tree-ssa-loop-unswitch.c (ssa_maybe_undefined_value_p): New.
(tree_may_unswitch_on): Call ssa_maybe_undefined_value_p.
So I'm going to defer to Richi since he was reviewing my original 
attempt in this space.


It probably doesn't matter in practice (when I looked at this I couldn't 
get into the code in question with a -O3 bootstrap or with the 
testsuite, just with the testcase in the BZ) but you might consider 
handling an already visited node slightly differently.


If the the node was visited and resolved as undefined, then we would 
have already exited the loop.


If the node was visited and resolved as defined, then we could just keep 
processing other items on the the worklist.


The case where you want to conservatively return false is when you're 
actively processing the name in question.


Jeff

Re: [PATCH] Fix bug in MEM parsing in patches 8a/8b

[David Malcolm]

On Mon, 2016-12-19 at 15:10 -0700, Jeff Law wrote:
> On 12/08/2016 01:39 PM, David Malcolm wrote:
> > Testing the patch kit on i686 showed numerous failures of this
> > assertion in set_mem_attributes_minus_bitpos in emit-rtl.c:
> > 
> >   1821gcc_assert (!defattrs->offset_known_p);
> > 
> > when expanding "main" in the rtl.exp test files, after parsing
> > an __RTL-tagged function.
> > 
> > Root cause is various assignments within the RTL parser of the
> > form:
> > 
> > 1222  MEM_OFFSET (x) = atoi (name.string);
> > 
> > where a MEM_* macro appears on the left-hand side of an assignment.
> > 
> > These macros are defined as a field lookup on the result of a call
> > to get_mem_attrs, e.g.:
> > 
> >   #define MEM_OFFSET(RTX) (get_mem_attrs (RTX)->offset)
> > 
> > get_mem_attrs can return the struct mem_attrs * of an rtx, but if
> > it isn't set, it returns:
> >mode_mem_attrs[(int) GET_MODE (x)];
> > 
> > which is this field within struct GTY(()) target_rtl:
> >   /* The default memory attributes for each mode.  */
> >   struct mem_attrs *x_mode_mem_attrs[(int) MAX_MACHINE_MODE];
> > 
> > These assignments in the parser were erroneously writing to these
> > default per-mode values, rather than assigning to a unique-per-rtx
> > instance of struct mem_attrs.
> > 
> > The fix is to call the appropriate set_mem_ functions in the
> > parser, e.g. set_mem_offset; the patch below is intended as a tweak
> > to patch 8a of the kit, and would be merged with it before
> > committing.
> > 
> > The patch also adds extra test coverage for MEM parsing.  This
> > extends
> > the target-independent selftests, and so would go into patch 8b.
> > 
> > Tested for targets x86_64-pc-linux-gnu, i686-pc-linux-gnu,
> > and aarch64-linux-gnu, and on powerpc-ibm-aix7.1.3.0.
> > 
> > OK as adjustments to patches 8a and 8b?
> > 
> > For patch 8a:
> >   gcc/ChangeLog:
> > * read-rtl-function.c
> > (function_reader::handle_any_trailing_information): Replace
> > writes
> > through macros MEM_ALIAS_SET, MEM_OFFSET, MEM_SIZE, MEM_ALIGN,
> > and MEM_ADDR_SPACE with calls to set_mem_ functions.  Add
> > missing
> > call to unread_char when handling "A" for alignment.
> > 
> > For patch 8b:
> >   gcc/ChangeLog:
> > * read-rtl-function.c (selftest::test_loading_mem): New
> > function.
> > (selftest::read_rtl_function_c_tests): Call it.
> >   gcc/testsuite/ChangeLog:
> > * selftests/mem.rtl: New file.
> They seem like reasonable adjustments.
> 
> I know you posted the cc1 patches to add the RTL front-end.  What's
> the 
> status on this kit?
> 
> [ Yes, I keep falling behind... ]

Current status of RTL frontend patch kit:

In summary: patch 8d and patch 9 need review.

In detail:

* patches 1-6 of the kit are committed to trunk

* patch 7 (in extremely minimal form) has been approved, merged into
patch 8a.

* patch 8 (adding function_reader class, and selftests to verify it
works) split into four subpatches, not yet in trunk:
  * patches 8a, 8b, and 8c are approved (the reader itself, selftests
for it that don't depend on any target, and selftests that are aarch64
-specific)
  * patches 8d isn't approved yet; I reposted this today as:
* "[PATCH] Add x86_64-specific selftests for RTL function reader
(v2)"
  https://gcc.gnu.org/ml/gcc-patches/2016-12/msg01616.html
  * I'm successfully run config-list.mk testing across 191 targets to
verify that these selftests don't break the build for anything (there
was some snafu with our dump syntax for pseudos conflicting with hard
reg names on iq2000, but this is resolved now)
  * my plan is to commit 8a-8d as one combined patch once 8d is
approved (assuming it is)

* patch 9 (wiring it up into cc1) needs review:
  * "[PATCH] Add "__RTL" to cc1 (v7)"
* https://gcc.gnu.org/ml/gcc-patches/2016-12/msg01662.html


Dave

Re: [fortran, patch] Remove unused elements in array argument to set_options

[Steven G. Kargl]

On Mon, Dec 19, 2016 at 11:29:45PM +0100, FX wrote:
> > Thinking out loud here.  I wonder, however, if we want
> > to future proof the library against changes to the
> > options passed by having a few spare unused entried 
> > available.  This of course only helps if a new option
> > needs to be added.  It does nothing for removal.
> 
> That’s actually the way gfortran_set_options() works, to ensure that we can 
> add options in the future: in main() we allocate an array of values, and pass 
> that array and its size to gfortran_set_options(). gfortran_set_options() 
> then deals with the number of options passed, meaning if an older program 
> calls a newer library, the runtime will simply use default values for all 
> options there were not passed.
> 
> And when we remove options, we simply pass zero values in their stead — until 
> we break the ABI, when we clean up everything.
> 
> Patch committed, thanks for reviewing.
> 

Thanks for the detailed explanation.  

-- 
Steve
http://troutmask.apl.washington.edu/~kargl/
 2. https://www.youtube.com/watch?v=Py6d6o2jbaE
 1. https://www.youtube.com/watch?v=6hwgPfCcpyQ

Re: [PATCH] fix integer overflow bugs in gimple-ssa-sprintf.c (PR 78608)

[Jeff Law]

On 12/14/2016 09:41 AM, Martin Sebor wrote:



-  if (i < 0)
+  if (HOST_WIDE_INT_MIN == i)

nit.  I think most folks would probably prefer this as
if (i == HOST_WIDE_INT_MIN).

HOST_WIDE_INT_MIN is a constant and when we can write an expression in
either order variable OP const is the preferred order.

You seem to be going back and forth between both styles.  Let' try to
stick with variable OP const.  I don't think you need to go back and fix
all of the existing const OP variable instances right now, but we may in
the future.


I learned the first style (const OP variable, and also using a less
than in favor of other relational operators) many years ago and I'm
trying to unlearn it for GCC.  It's a hard habit to break.

FWIW, the const OP variable style used to be recommended to avoid
subtle bugs due to typos like 'if (var = CST)'  But I realize that
with -Wparentheses warning on this there is no need for the style
when using GCC.
Yea.   And I can see some argument to use the CST == var style.   But 
I'm not up to pushing on that in our coding standards.  I realize it'll 
take time to unlearn :-)





I think the patch for bug 78696 resolves the FIXME (but see below).


If the FIXME was a future thing, then this is OK with the nits fixed. If
the FIXME was a marker for something you intended to address now and
just forgot, then we either need another iteration or a follow-up patch
depending on the severity of the FIXME in your  mind.


The patch for bug 78696 resolves the FIXME but there will need to
be another change here to improve the handling of unknown precisions
and widths:

1) Use get_range_info to constrain non-constant width and precision,
   and if that fails...

2) ...use some reasonable default (e.g., based on the precision of
   the type of the directive).

Without these changes sprintf (d, "%.*f", p, 0.0) causes

  warning: writing a terminating nul past the end of the destination

even at -Wformat-length=1 with no good way to suppress it.  At
-Wformat-length=2, sprintf(d, "%.*i", 0) also causes a similar:

  warning: ‘%.*i’ directive writing 0 or more bytes into a region...

also with no way to suppress it. (The two warnings should also be
worded the same.)

I've started to work on a general fix for both of these in my patch
for bug 78703 - -fprintf-return-value floating point handling
incorrect in locales with a mulltibyte decimal point.  These fit
well together because they both separate the heuristic-based warning
byte counters from the actual counters good for optimization (which
are based on what GCC knows for certain).

Understood.  Thanks for the update.

jeff

Re: [fortran, patch] Remove unused elements in array argument to set_options

[FX]

> Thinking out loud here.  I wonder, however, if we want
> to future proof the library against changes to the
> options passed by having a few spare unused entried 
> available.  This of course only helps if a new option
> needs to be added.  It does nothing for removal.

That’s actually the way gfortran_set_options() works, to ensure that we can add 
options in the future: in main() we allocate an array of values, and pass that 
array and its size to gfortran_set_options(). gfortran_set_options() then deals 
with the number of options passed, meaning if an older program calls a newer 
library, the runtime will simply use default values for all options there were 
not passed.

And when we remove options, we simply pass zero values in their stead — until 
we break the ABI, when we clean up everything.

Patch committed, thanks for reviewing.

FX

Re: [PATCH][C++] Fix PR71694, store data race with tail-padding

[Jeff Law]

On 12/14/2016 03:44 AM, Richard Biener wrote:


The following implements Jasons suggestion of using a langhook to
return the size of an aggregate without tail padding that might
be re-used when it is inherited from.

Using this langhook we can fix the size of the representative for the
bitfield properly and thus generate correct code for the testcase.
Previously on x86_64 it was

main:
movlc+4(%rip), %eax
andl$-258049, %eax
orb $16, %ah
movl%eax, c+4(%rip)
movb$2, c+7(%rip)
xorl%eax, %eax
ret

while after the patch we see

main:
movzbl  c+5(%rip), %eax
andb$-4, c+6(%rip)
movb$2, c+7(%rip)
andl$15, %eax
orl $16, %eax
movb%al, c+5(%rip)
xorl%eax, %eax
ret

in particular the store to C::B::d is now properly using a byte store.

Bootstrap and regtest in progress on x86_64-unknown-linux-gnu, ok for
trunk?

Thanks,
Richard.

2016-12-14  Richard Biener  

PR c++/71694
* langhooks-def.h (lhd_unit_size_without_reusable_padding): Declare.
(LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
(LANG_HOOKS_FOR_TYPES_INITIALIZER): Adjust.
* langhooks.h (struct lang_hooks_for_types): Add
unit_size_without_reusable_padding.
* langhooks.c (lhd_unit_size_without_reusable_padding): New.
* stor-layout.c (finish_bitfield_representative): Use
unit_size_without_reusable_padding langhook to decide on the
last representatives size.

cp/
* cp-objcp-common.h (cp_unit_size_without_reusable_padding): Declare.
(LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
* cp-objcp-common.c (cp_unit_size_without_reusable_padding): New.

* g++.dg/pr71694.C: New testcase.

Index: gcc/stor-layout.c
===
*** gcc/stor-layout.c   (revision 243632)
--- gcc/stor-layout.c   (working copy)
*** finish_bitfield_representative (tree rep
*** 1864,1876 
  }
else
  {
!   /* ???  If you consider that tail-padding of this struct might be
!  re-used when deriving from it we cannot really do the following
!and thus need to set maxsize to bitsize?  Also we cannot
!generally rely on maxsize to fold to an integer constant, so
!use bitsize as fallback for this case.  */
!   tree maxsize = size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT (field)),
! DECL_FIELD_OFFSET (repr));
if (tree_fits_uhwi_p (maxsize))
maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
  - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
--- 1864,1877 
  }
else
  {
!   /* Note that if the C++ FE sets up tail-padding to be re-used it
!  creates a as-base variant of the type with TYPE_SIZE adjusted
!accordingly.  So it is safe to include tail-padding here.  */
!   tree aggsize = lang_hooks.types.unit_size_without_reusable_padding
!   (DECL_CONTEXT (field));
!   tree maxsize = size_diffop (aggsize, DECL_FIELD_OFFSET (repr));
!   /* We cannot generally rely on maxsize to fold to an integer constant,
!so use bitsize as fallback for this case.  */
if (tree_fits_uhwi_p (maxsize))
maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
  - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
Looks reasonable, formatting nit in the comment above the assignment to 
AGGSIZE (looks like it was mucked up in the old version too).  I think 
the second line is using spaces when it should have used a tab.


Do we document langhooks anywhere?

Jeff

Re: [PATCH] correct %g handling with unknown arguments in -fprintf-return-value (PR 78696)

[Jeff Law]

On 12/12/2016 05:06 PM, Martin Sebor wrote:

+/* The lower bound when precision isn't specified is 8 bytes
+   ("1.23456" since precision is taken to be 6).  When precision
+   is zero, the lower bound is 1 byte (e.g., "1").  Otherwise,
+   when precision is greater than zero, then the lower bound
+   is 2 plus precision (plus flags).  */
+res.range.min = (flagmin
+ + (prec != INT_MIN)   /* for decimal point */
+ + (prec == INT_MIN
+? 0 : prec < 0 ? 6 : prec ? prec : -1));

Note for the future, nest/chained ternary operators can sometimes just
be hard to visually parse when they're squashed on a single line.
Formatting like this has often been used in the past to help clarify the
intent:

(flagmin
 + (prec != INT_MIN)
 + (prec == INT_MIN ? 0
: prec < 0 ? 6
: prec ? prec
: -1)


Okay.



If we ignore the flagmin component, I get the following evaluations for
PREC.

PREC   RESULT
INTMIN   0
0  0
negative (but not INTMIN)  7
positive   prec + 1

That doesn't seem in-line with the comment.


Sorry, I think I need a hint.  Which part doesn't seem in line with
which part of the comment?  The numbers you have look correct to me
and I don't see anything wrong with the comment either.

Flagmin is always at least 1 and so RESULT above is 1 when precision
is used and either zero or unknown (because printf ("%.0f", 0) returns
1), and it's 8 when precision is negative because it's taken as if had
been omitted (i.e., it's 6 and printf ("%f", 0) formats "0.00" and
returns 8), and it's prec + 2 when precision is positive because the 2
accounts for the leading "0." (when argument is zero) and precision is
the number of fractional digits.
So I think it's another case me mis-parsing the comment and conflating 
unknown vs unspecified in my mind  I took the "plus flags" as applying 
to all cases, but re-reading it with the additional context you've 
provided, it seems like it actually applies to just the last case.


Try to give the comment a light respin to see if you can clarify.  OK 
with that update.


Jeff

RE: [PATCH, testsuite] MIPS: Relax instruction order check in msa-builtins.c.

[Moore, Catherine]

> -Original Message-
> From: Toma Tabacu [mailto:toma.tab...@imgtec.com]
> Sent: Thursday, December 15, 2016 9:51 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune ; Moore,
> Catherine 
> Subject: [PATCH, testsuite] MIPS: Relax instruction order check in msa-
> builtins.c.
> 
> Hi,
> 
> The 32-bit insert.d case in msa-builtins.c is failing with O2 and Os
> because
> the order of the emitted instructions is slightly different compared to
> the
> other optimization levels.
> 
> This patch tweaks the regular expression for 32-bit insert.d to accept
> the
> alternate instruction order.
> 
> Tested with mips-mti-elf.
> 
> Regards,
> Toma
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/mips/msa-builtins.c (dg-final): Tweak regex for the
> 32-bit
>   insert.d case.

Please change to:
* gcc.target/mips-msa-builtins.c (msa_insert_d): Tweak expected output.

Okay with that change.
Thanks,
Catherine

> 
> diff --git a/gcc/testsuite/gcc.target/mips/msa-builtins.c
> b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> index 6db3d66..a679f06 100644
> --- a/gcc/testsuite/gcc.target/mips/msa-builtins.c
> +++ b/gcc/testsuite/gcc.target/mips/msa-builtins.c
> @@ -481,7 +481,7 @@
>  /* { dg-final { scan-assembler-times
> "msa_insert_h:.*insert\\.h.*msa_insert_h" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insert_w:.*insert\\.w.*msa_insert_w" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insert_d:.*insert\\.d.*msa_insert_d" 1 { target mips64 } } } */
> -/* { dg-final { scan-assembler-times
> "msa_insert_d:.*sra.*insert.w.*insert.w.*msa_insert_d" 1 { target {!
> mips64 } } } } */
> +/* { dg-final { scan-assembler
> "msa_insert_d:.*(sra.*insert.w.*insert.w|insert.w.*sra.*insert.w).*ms
> a_insert_d" { target {! mips64 } } } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_b:.*insve\\.b.*msa_insve_b" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_h:.*insve\\.h.*msa_insve_h" 1 } } */
>  /* { dg-final { scan-assembler-times
> "msa_insve_w:.*insve\\.w.*msa_insve_w" 1 } } */

Re: [fortran, patch] Remove unused elements in array argument to set_options

[Steve Kargl]

On Mon, Dec 19, 2016 at 10:47:09PM +0100, FX wrote:
> For ABI compatibility, we kept some unused elements in the array argument to 
> _gfortran_set_options (options that we have removed). With the current ABI 
> breakage, we might as well remove those.
> 
> Bootstrapped and regtested on x86_64-apple-darwin16.3.0
> OK to commit?
> 
> FX
> 

The patch looks fine to me.

Thinking out loud here.  I wonder, however, if we want
to future proof the library against changes to the
options passed by having a few spare unused entried 
available.  This of course only helps if a new option
needs to be added.  It does nothing for removal.

-- 
Steve

Re: [PATCH] Fix bug in MEM parsing in patches 8a/8b

[Jeff Law]

On 12/08/2016 01:39 PM, David Malcolm wrote:

Testing the patch kit on i686 showed numerous failures of this
assertion in set_mem_attributes_minus_bitpos in emit-rtl.c:

  1821gcc_assert (!defattrs->offset_known_p);

when expanding "main" in the rtl.exp test files, after parsing
an __RTL-tagged function.

Root cause is various assignments within the RTL parser of the
form:

1222  MEM_OFFSET (x) = atoi (name.string);

where a MEM_* macro appears on the left-hand side of an assignment.

These macros are defined as a field lookup on the result of a call
to get_mem_attrs, e.g.:

  #define MEM_OFFSET(RTX) (get_mem_attrs (RTX)->offset)

get_mem_attrs can return the struct mem_attrs * of an rtx, but if
it isn't set, it returns:
   mode_mem_attrs[(int) GET_MODE (x)];

which is this field within struct GTY(()) target_rtl:
  /* The default memory attributes for each mode.  */
  struct mem_attrs *x_mode_mem_attrs[(int) MAX_MACHINE_MODE];

These assignments in the parser were erroneously writing to these
default per-mode values, rather than assigning to a unique-per-rtx
instance of struct mem_attrs.

The fix is to call the appropriate set_mem_ functions in the
parser, e.g. set_mem_offset; the patch below is intended as a tweak
to patch 8a of the kit, and would be merged with it before committing.

The patch also adds extra test coverage for MEM parsing.  This extends
the target-independent selftests, and so would go into patch 8b.

Tested for targets x86_64-pc-linux-gnu, i686-pc-linux-gnu,
and aarch64-linux-gnu, and on powerpc-ibm-aix7.1.3.0.

OK as adjustments to patches 8a and 8b?

For patch 8a:
  gcc/ChangeLog:
* read-rtl-function.c
(function_reader::handle_any_trailing_information): Replace writes
through macros MEM_ALIAS_SET, MEM_OFFSET, MEM_SIZE, MEM_ALIGN,
and MEM_ADDR_SPACE with calls to set_mem_ functions.  Add missing
call to unread_char when handling "A" for alignment.

For patch 8b:
  gcc/ChangeLog:
* read-rtl-function.c (selftest::test_loading_mem): New function.
(selftest::read_rtl_function_c_tests): Call it.
  gcc/testsuite/ChangeLog:
* selftests/mem.rtl: New file.

They seem like reasonable adjustments.

I know you posted the cc1 patches to add the RTL front-end.  What's the 
status on this kit?


[ Yes, I keep falling behind... ]

jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 01:09 PM, Martin Sebor wrote:

By moving the warning earlier, we'll still warn for the most cases, but
won't warn in the more convoluted cases.  We can perhaps work on it
further
in GCC 8.  If we keep it as is, I think most users will just
-Wno-nonnull
as soon as they run into some warning that will be hard to figure out
what
is going on.  At that point they will not get warnings even for the
obvious
cases that we used to warn.  Look at how the Linux kernel folks
disable most
of warnings even for smaller reasons.

But again, the user case is no different than other warnings that are
sensitive to optimizations.

My sense is that we should revert and table until gcc-8 where we can
evaluate the space more thoroughly.


I think that would be unfortunate.  We have a number of alternatives
that seem much preferable.
Definitely unfortunate, but I don't think we have agreement on the key 
issue, namely where the warning belongs.





I have a trivial patch that avoids the sanitizer warnings by disabling
the late -Wnonnull warning when -fsanitize=undefined is specified.
A simple fix for the GCC warnings discovered by profiledbootstrap-O3
and verified on powerpc64 and x86_63 was posted for review last week.
The former seems like a hack.  I can't recall which patch the latter was 
(was it the one that just ripped out the "can't happen code"?




Jakub's patch avoids those warnings and obviates any GCC changes (IIUC).
Yes, but they suffer from missing warnings that are exposed by 
transformations later in the pipeline.




There is also the option of introducing -Wnonnull=2 that warns late
and not including it in -Wall or -Wextra.  All three of this have
been tested.
Understood, but I'm not keep to start adding more levels of warning and 
code to support them until we have reached some kind of agreement.  And 
it's my sense that we need more time to hammer out that kind of agreement.



Jeff

[fortran, patch] Remove unused elements in array argument to set_options

[FX]

For ABI compatibility, we kept some unused elements in the array argument to 
_gfortran_set_options (options that we have removed). With the current ABI 
breakage, we might as well remove those.

Bootstrapped and regtested on x86_64-apple-darwin16.3.0
OK to commit?

FX



set_options.ChangeLog
Description: Binary data


set_options.diff
Description: Binary data

Re: [PATCH] [PR rtl-optimization/65618] Fix MIPS ADA bootstrap failure

[Jeff Law]

On 12/19/2016 08:44 AM, James Cowgill wrote:

Hi,

This patch fixes PR 65618 where ADA cannot be bootstrapped natively on
mips due to a bootstrap comparison failure. The PR is currently in the
target component, but should be in the rtl-optimization component.

The underlying bug is in gcc/emit-rtl.c:try_split and is a result of
the fix for PR rtl-optimization/48826. In that PR, if a call_insn is
split into two instructions, the following NOTE_INSN_CALL_ARG_LOCATION
is moved so that it immediately follows the new call_insn. However,
after doing that the "after" variable was not updated and it could
still point to the old note instruction (the instruction after the
instruction to be split). The "after" variable is later used to obtain
the last instruction in the split and is then passed back to the
delayed branch scheduler influencing how delay slots are assigned. My
patch adjusts the code which handles the NOTE_INSN_CALL_ARG_LOCATION
note so that "after" is updated if necessary.

This bug causes the ADA bootstrap comparison failure in a-except.o
because the branch delay scheduling operates slightly differently for
that file if debug information is turned on.

Thanks,
James

gcc/Changelog:

2016-12-16  James Cowgill  

PR rtl-optimization/65618
* emit-rtl.c (try_split): Update "after" when moving a
NOTE_INSN_CALL_ARG_LOCATION.

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 7de17454037..6be124ac038 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3742,6 +3742,11 @@ try_split (rtx pat, rtx_insn *trial, int last)
   next = NEXT_INSN (next))
if (NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
  {
+   /* Advance after to the next instruction if it is about to
+  be removed.  */
+   if (after == next)
+ after = NEXT_INSN (after);
+
remove_insn (next);
add_insn_after (next, insn, NULL);
break;

So the thing I don't like when looking at this code is we set AFTER 
immediately upon entry to try_split.  But we don't use it until near the 
very end of try_split.  That's just asking for trouble.


Can we reasonably initialize AFTER just before it's used?

Jeff

Re: [PATCH] detect null sprintf pointers (PR 78519)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 01:24:34PM -0700, Jeff Law wrote:
> > PR middle-end/78519 - missing warning for sprintf %s with null pointer
> > 
> > gcc/ChangeLog:
> > 
> > PR middle-end/78519
> > * gimple-ssa-sprintf.c (format_string): Handle null pointers.
> > (format_directive): Diagnose null pointer arguments.
> > (pass_sprintf_length::handle_gimple_call): Diagnose null destination
> > pointers.  Correct location of null format string in diagnostics.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR middle-end/78519
> > * gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test.
> So I think we should defer this given the vigorous discussion around the
> other NULL checks.  This has the same issues that we're discussing in the
> other, rather heated, thread.

If this would be only warned if !fold_return_value, then it wouldn't be
having the same issues.  Though of course, it would diagnose fewer cases.
Conceptually it isn't much dependent on the gimple-ssa-sprintf.c stuff,
so could be warned without too much effort from other pass, perhaps
by using a few helpers from gimple-ssa-sprintf.c.

Jakub

[committed] print_rtx_function: update example in comment

[David Malcolm]

The patch updates the example dump in the comment for
print_rtx_function to reflect various changes:
- r241593: addition of insn UIDs
- r241908: removal of trailing "(nil)" and other default values
- r242023: addition of "param" directives
- r243798: change of format of regnos in non-virtual pseudos
(from "$2" to "<2>")

Committed to trunk (as r243812) under the "obvious" rule.

gcc/ChangeLog:
* print-rtl-function.c (print_rtx_function): Update
example in comment to reflect current format.
---
 gcc/print-rtl-function.c | 44 +---
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/gcc/print-rtl-function.c b/gcc/print-rtl-function.c
index dea84fe..74d8e9c 100644
--- a/gcc/print-rtl-function.c
+++ b/gcc/print-rtl-function.c
@@ -175,38 +175,36 @@ print_param (FILE *outfile, rtx_writer , tree arg)
Example output (with COMPACT==true):
 
(function "times_two"
+ (param "i"
+   (DECL_RTL (mem/c:SI (plus:DI (reg/f:DI virtual-stack-vars)
+  (const_int -4)) [1 i+0 S4 A32]))
+   (DECL_RTL_INCOMING (reg:SI di [ i ])))
  (insn-chain
-   (cnote NOTE_INSN_DELETED)
+   (cnote 1 NOTE_INSN_DELETED)
(block 2
 (edge-from entry (flags "FALLTHRU"))
-(cnote [bb 2] NOTE_INSN_BASIC_BLOCK)
-(cinsn (set (mem/c:SI (plus:DI (reg/f:DI virtual-stack-vars)
+(cnote 4 [bb 2] NOTE_INSN_BASIC_BLOCK)
+(cinsn 2 (set (mem/c:SI (plus:DI (reg/f:DI virtual-stack-vars)
   (const_int -4)) [1 i+0 S4 A32])
-  (reg:SI di [ i ])) "t.c":2
-  (nil))
-(cnote NOTE_INSN_FUNCTION_BEG)
-(cinsn (set (reg:SI %2)
+  (reg:SI di [ i ])) "t.c":2)
+(cnote 3 NOTE_INSN_FUNCTION_BEG)
+(cinsn 6 (set (reg:SI <2>)
   (mem/c:SI (plus:DI (reg/f:DI virtual-stack-vars)
-  (const_int -4)) [1 i+0 S4 A32])) "t.c":3
-  (nil))
-(cinsn (parallel [
-  (set (reg:SI %0 [ _2 ])
-  (ashift:SI (reg:SI %2)
+  (const_int -4)) [1 i+0 S4 A32])) "t.c":3)
+(cinsn 7 (parallel [
+  (set (reg:SI <0> [ _2 ])
+  (ashift:SI (reg:SI <2>)
   (const_int 1)))
   (clobber (reg:CC flags))
   ]) "t.c":3
-  (expr_list:REG_EQUAL (ashift:SI (mem/c:SI (plus:DI (reg/f:DI 
virtual-stack-vars)
+   (expr_list:REG_EQUAL (ashift:SI (mem/c:SI (plus:DI 
(reg/f:DI virtual-stack-vars)
   (const_int -4)) [1 i+0 S4 A32])
-  (const_int 1))
-  (nil)))
-(cinsn (set (reg:SI %1 [  ])
-  (reg:SI %0 [ _2 ])) "t.c":3
-  (nil))
-(cinsn (set (reg/i:SI ax)
-  (reg:SI %1 [  ])) "t.c":4
-  (nil))
-(cinsn (use (reg/i:SI ax)) "t.c":4
-  (nil))
+  (const_int 1
+(cinsn 10 (set (reg:SI <1> [  ])
+  (reg:SI <0> [ _2 ])) "t.c":3)
+(cinsn 14 (set (reg/i:SI ax)
+  (reg:SI <1> [  ])) "t.c":4)
+(cinsn 15 (use (reg/i:SI ax)) "t.c":4)
 (edge-to exit (flags "FALLTHRU"))
) ;; block 2
  ) ;; insn-chain
-- 
1.8.5.3

Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers in GIMPLE.

[Jeff Law]

On 12/15/2016 03:14 AM, Tamar Christina wrote:



On a high level, presumably there's no real value in keeping the old
code to "fold" fpclassify.  By exposing those operations as integer
logicals for the fast path, if the FP value becomes a constant during
the optimization pipeline we'll see the reinterpreted values flowing
into the new integer logical tests and they'll simplify just like
anything else.  Right?


Yes, if it becomes a constant it will be folded away, both in the integer and 
the fp case.

Thanks for clarifying.





The old IBM format is still supported, though they are expected to be
moveing towards a standard ieee 128 bit format.  So my only concern is
that we preserve correct behavior for those cases -- I don't really care
about optimizing them.  So I think you need to keep them.


Yes, I re-added them. It's mostly a copy paste from what they were in the
other functions. But I have no way of testing it.

Understood.


 > +  const HOST_WIDE_INT type_width = TYPE_PRECISION (type);

+  return (format->is_binary_ieee_compatible
+   && FLOAT_WORDS_BIG_ENDIAN == WORDS_BIG_ENDIAN
+   /* We explicitly disable quad float support on 32 bit systems.  */
+   && !(UNITS_PER_WORD == 4 && type_width == 128)
+   && targetm.scalar_mode_supported_p (mode));
+}

Presumably this is why you needed the target.h inclusion.

Note that on some systems we even disable 64bit floating point support.
I suspect this check needs a little re-thinking as I don't think that
checking for a specific UNITS_PER_WORD is correct, nor is checking the
width of the type.  I'm not offhand sure what the test should be, just
that I think we need something better here.


I think what I really wanted to test here is if there was an integer mode 
available
which has the exact width as the floating point one. So I have replaced this 
with
just a call to int_mode_for_mode. Which is probably more correct.
I'll need to think about it, but would inherently think that 
int_mode_for_mode is better than an explicit check of UNITS_PER_WORD and 
typewidth.






+
+/* Determines if the given number is a NaN value.
+   This function is the last in the chain and only has to
+   check if it's preconditions are true.  */
+static tree
+is_nan (gimple_seq *seq, tree arg, location_t loc)

So in the old code we checked UNGT_EXPR, in the new code's slow path you
check UNORDERED.  Was that change intentional?


The old FP code used UNORDERED and the new one was using ORDERED and negating 
the result.
I've replaced it with UNORDERED, but both are correct.

OK.  Just wanted to make sure.

jeff

[PATCH, i386] Add *popcounthi2_1 insn and splitter

[Uros Bizjak]

Hello!

This patch just adds missing popcounthi2 insn and splitter to enable
POPCNTW generation from _builtin_popcount builtin with zero-extended
unsigned short argument.

2016-12-19  Uros Bizjak  

* config/i386/i386.md (*popcounthi2_1): New insn_and_split pattern.

testsuite/ChangeLog:

2016-12-19  Uros Bizjak  

* gcc.target/i386/pr59874-3.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
Index: config/i386/i386.md
===
--- config/i386/i386.md (revision 243799)
+++ config/i386/i386.md (working copy)
@@ -13221,6 +13221,24 @@
(set_attr "type" "bitmanip")
(set_attr "mode" "")])
 
+(define_insn_and_split "*popcounthi2_1"
+  [(set (match_operand:SI 0 "register_operand")
+   (popcount:SI
+ (zero_extend:SI (match_operand:HI 1 "nonimmediate_operand"
+   (clobber (reg:CC FLAGS_REG))]
+  "TARGET_POPCNT
+   && can_create_pseudo_p ()"
+  "#"
+  "&& 1"
+  [(const_int 0)]
+{
+  rtx tmp = gen_reg_rtx (HImode);
+
+  emit_insn (gen_popcounthi2 (tmp, operands[1]));
+  emit_insn (gen_zero_extendhisi2 (operands[0], tmp));
+  DONE;
+})
+
 (define_insn "popcounthi2"
   [(set (match_operand:HI 0 "register_operand" "=r")
(popcount:HI
Index: testsuite/gcc.target/i386/pr59874-3.c
===
--- testsuite/gcc.target/i386/pr59874-3.c   (nonexistent)
+++ testsuite/gcc.target/i386/pr59874-3.c   (working copy)
@@ -0,0 +1,10 @@
+/* PR target/59874 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mpopcnt -masm=att" } */
+/* { dg-final { scan-assembler "popcntw" } } */
+
+unsigned int
+foo (unsigned short x)
+{
+  return __builtin_popcount (x);
+}

[PATCH] Add "__RTL" to cc1 (v7)

[David Malcolm]

This is the final part of the RTL "frontend" patch kit, implemented as
a special case for functions marked with __RTL within the C frontend.

Successfully bootstrapped on x86_64-pc-linux-gnu on top
of the rest of the RTL frontend patch kit.

OK for trunk?

Changed in v7:
- remove i?86-*-* from gcc.dg/rtl/x86_64/final.c
- updated to "<3>" syntax for pseudos (rather than "$3")
- removed unnecessary #includes from run-rtl-passes.c

Changed in v6:
- Handle EOF in c_parser_parse_rtl_body
- Various fixups from review

Changed in v5:
- rebased from r242065 to r242392.  In particular, this
  brings in the gimple FE; rewrote the "startwith" pass-skipping
  mechanism to work with both __GIMPLE and __RTL.

- rewrote the "__RTL" parser so that it shares code with the
  "__GIMPLE" parser, within c_parser_declspecs.
  Updated all the test cases to use the 'startwith' syntax.

  The gimple FE has a "flag_gimple" to enable __GIMPLE; should
  I add an equivalent for __RTL?

- eliminated the new "native_RTL" field, instead just using
  curr_properties & PROP_rtl.

- added original_copy_tables_initialized_p and reinstate:
gcc_assert (original_copy_bb_pool);
  within free_original_copy_tables.

- added a test of multiple __RTL-marked functions within one test
  case.

Links to previous versions:
  https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00263.html
  https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00500.html



gcc/ChangeLog:
* Makefile.in (OBJS): Add run-rtl-passes.o.

gcc/c-family/ChangeLog:
* c-common.c (c_common_reswords): Add "__RTL".
* c-common.h (enum rid): Add RID_RTL.

gcc/c/ChangeLog:
* c-parser.c: Include "read-rtl-function.h" and
"run-rtl-passes.h".
(c_parser_declaration_or_fndef): Rename "gimple-pass-list" in
grammar to gimple-or-rtl-pass-list.  Add rtl-function-definition
production.  Update for renaming of field "gimple_pass" to
"gimple_or_rtl_pass".  If __RTL was seen, call
c_parser_parse_rtl_body.  Convert a timevar_push/pop pair
to an auto_timevar, to cope with early exit.
(c_parser_declspecs): Update RID_GIMPLE handling for renaming of
field "gimple_pass" to "gimple_or_rtl_pass", and for renaming of
c_parser_gimple_pass_list to c_parser_gimple_or_rtl_pass_list.
Handle RID_RTL.
(c_parser_parse_rtl_body): New function.
* c-tree.h (enum c_declspec_word): Add cdw_rtl.
(struct c_declspecs): Rename field "gimple_pass" to
"gimple_or_rtl_pass".  Add field "rtl_p".
* gimple-parser.c (c_parser_gimple_pass_list): Rename to...
(c_parser_gimple_or_rtl_pass_list): ...this, updating accordingly.
* gimple-parser.h (c_parser_gimple_pass_list): Rename to...
(c_parser_gimple_or_rtl_pass_list): ...this.

gcc/ChangeLog:
* cfg.c (original_copy_tables_initialized_p): New function.
* cfg.h (original_copy_tables_initialized_p): New decl.
* cfgrtl.c (relink_block_chain): Guard the call to
free_original_copy_tables with a call to
original_copy_tables_initialized_p.
* cgraph.h (symtab_node::native_rtl_p): New decl.
* cgraphunit.c (symtab_node::native_rtl_p): New function.
(symtab_node::needed_p): Don't assert for early assembly output
for __RTL functions.
(cgraph_node::finalize_function): Set "force_output" for __RTL
functions.
(cgraph_node::analyze): Bail out early for __RTL functions.
(analyze_functions): Update assertion to support __RTL functions.
(cgraph_node::expand): Bail out early for __RTL functions.
* final.c (rest_of_clean_state): Don't call delete_tree_ssa for
_RTL functions.
* gimple-expr.c: Include "tree-pass.h".
(gimple_has_body_p): Return false for __RTL functions.
* pass_manager.h (gcc::pass_manager::get_rest_of_compilation): New
accessor.
(gcc::pass_manager::get_clean_slate): New accessor.
* passes.c: Include "insn-addr.h".
(execute_one_pass): Split out logic for skipping passes into...
(determine_pass_name_match): ...this new function, ...
(should_skip_pass_p): ...and this new function, adding logging
and special-casing dfinit.
(skip_pass): New function.
* read-md.c (md_reader::read_char): Support filtering
the input to a subset of line numbers.
(md_reader::md_reader): Initialize fields
m_first_line and m_last_line.
(md_reader::read_file_fragment): New function.
* read-md.h (md_reader::read_file_fragment): New decl.
(md_reader::m_first_line): New field.
(md_reader::int m_last_line): New field.
* read-rtl-function.c (function_reader::create_function): Only
create cfun if it doesn't already exist.  Set PROP_rtl on cfun's
curr_properties.  Set DECL_INITIAL 

Re: [PATCH] detect null sprintf pointers (PR 78519)

[Jeff Law]

On 12/14/2016 09:21 PM, Martin Sebor wrote:

I suppose setting a range seemed better than giving up.  Then again,
since with this patch GCC will warn on null %s pointers there may
not be much point in trying to see if there's also some other
problem after that, except perhaps in code that deliberately relies
on the Glibc feature.  I'd be fine with just stopping at this point
if you prefer.

I think I'd rather just stop at this point.  I don't think we're gaining
much, if anything and encoding the glibc-ism in here seems like a
mistake.


That's fine.  Attached is an updated patch with this change.

Thanks
Martin


gcc-78519.diff


PR middle-end/78519 - missing warning for sprintf %s with null pointer

gcc/ChangeLog:

PR middle-end/78519
* gimple-ssa-sprintf.c (format_string): Handle null pointers.
(format_directive): Diagnose null pointer arguments.
(pass_sprintf_length::handle_gimple_call): Diagnose null destination
pointers.  Correct location of null format string in diagnostics.

gcc/testsuite/ChangeLog:

PR middle-end/78519
* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test.
So I think we should defer this given the vigorous discussion around the 
other NULL checks.  This has the same issues that we're discussing in 
the other, rather heated, thread.




Jeff

Re: [PATCH] Optimiza aggregate a = b = c = {} (PR c/78408, take 2)

[Jeff Law]

On 12/16/2016 05:50 AM, Richard Biener wrote:


+  gimple *defstmt = SSA_NAME_DEF_STMT (vuse);
+  tree src2 = NULL_TREE, len2 = NULL_TREE;
+  HOST_WIDE_INT offset, offset2;
+  tree val = integer_zero_node;
+  if (gimple_store_p (defstmt)
+  && gimple_assign_single_p (defstmt)
+  && TREE_CODE (gimple_assign_rhs1 (defstmt)) == CONSTRUCTOR
+  && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (defstmt)) == 0


Should be always true for stores from constructors.
Seems like I should drop similar checks from the in-progress DSE 
changes.  I thought I saw something in SRA to cover when this wasn't 
true as well that could probably be simplified.


jeff

[doc, committed] CPP manual cleanup, part 1

[Sandra Loosemore]

I've checked in this patch to do some initial cleanup of bit-rotten 
content in the CPP manual.  I've rewritten some passages that made it 
sound like C99 support is a brand-new thing, and removed text that 
describes how the preprocessor used to work in ancient versions of GCC. 
 This is all routine cleanup  I've done no checking on whether the 
remaining documentation is correct, whether the set of documented 
features corresponds to those currently implemented, etc.


-Sandra
2016-12-19  Sandra Loosemore  

	gcc/
	* doc/cpp.texi: Clean up anachronistic C99 references and remove 
	discussion of very old GCC versions.
	(Differences from previous versions): Delete entire section.
Index: gcc/doc/cpp.texi
===
--- gcc/doc/cpp.texi	(revision 243537)
+++ gcc/doc/cpp.texi	(working copy)
@@ -163,7 +163,6 @@ Implementation Details
 * Implementation-defined behavior::
 * Implementation limits::
 * Obsolete Features::
-* Differences from previous versions::
 
 Obsolete Features
 
@@ -523,8 +522,8 @@ with an optional period, a required deci
 with any sequence of letters, digits, underscores, periods, and
 exponents.  Exponents are the two-character sequences @samp{e+},
 @samp{e-}, @samp{E+}, @samp{E-}, @samp{p+}, @samp{p-}, @samp{P+}, and
-@samp{P-}.  (The exponents that begin with @samp{p} or @samp{P} are new
-to C99.  They are used for hexadecimal floating-point constants.)
+@samp{P-}.  (The exponents that begin with @samp{p} or @samp{P} are 
+used for hexadecimal floating-point constants.)
 
 The purpose of this unusual definition is to isolate the preprocessor
 from the full complexity of numeric constants.  It does not have to
@@ -562,10 +561,8 @@ closing quote or angle bracket.  The pre
 file in different places depending on which form you use.  @xref{Include
 Operation}.
 
-No string literal may extend past the end of a line.  Older versions
-of GCC accepted multi-line string constants.  You may use continued
-lines instead, or string constant concatenation.  @xref{Differences
-from previous versions}.
+No string literal may extend past the end of a line.  You may use continued
+lines instead, or string constant concatenation.
 
 @cindex punctuators
 @cindex digraphs
@@ -1754,39 +1751,23 @@ eprintf ("success!\n")
 The above explanation is ambiguous about the case where the only macro
 parameter is a variable arguments parameter, as it is meaningless to
 try to distinguish whether no argument at all is an empty argument or
-a missing argument.  In this case the C99 standard is clear that the
-comma must remain, however the existing GCC extension used to swallow
-the comma.  So CPP retains the comma when conforming to a specific C
-standard, and drops it otherwise.
+a missing argument.  
+CPP retains the comma when conforming to a specific C
+standard.  Otherwise the comma is dropped as an extension to the standard.
 
-C99 mandates that the only place the identifier @code{@w{__VA_ARGS__}}
+The C standard 
+mandates that the only place the identifier @code{@w{__VA_ARGS__}}
 can appear is in the replacement list of a variadic macro.  It may not
 be used as a macro name, macro argument name, or within a different type
 of macro.  It may also be forbidden in open text; the standard is
 ambiguous.  We recommend you avoid using it except for its defined
 purpose.
 
-Variadic macros are a new feature in C99.  GNU CPP has supported them
-for a long time, but only with a named variable argument
-(@samp{args@dots{}}, not @samp{@dots{}} and @code{@w{__VA_ARGS__}}).  If you are
-concerned with portability to previous versions of GCC, you should use
-only named variable arguments.  On the other hand, if you are concerned
-with portability to other conforming implementations of C99, you should
-use only @code{@w{__VA_ARGS__}}.
-
-Previous versions of CPP implemented the comma-deletion extension
-much more generally.  We have restricted it in this release to minimize
-the differences from C99.  To get the same effect with both this and
-previous versions of GCC, the token preceding the special @samp{##} must
-be a comma, and there must be white space between that comma and
-whatever comes immediately before it:
-
-@smallexample
-#define eprintf(format, args@dots{}) fprintf (stderr, format , ##args)
-@end smallexample
-
-@noindent
-@xref{Differences from previous versions}, for the gory details.
+Variadic macros became a standard part of the C language with C99.  
+GNU CPP previously supported them
+with a named variable argument
+(@samp{args@dots{}}, not @samp{@dots{}} and @code{@w{__VA_ARGS__}}), which
+is still supported for backward compatibility.
 
 @node Predefined Macros
 @section Predefined Macros
@@ -1854,7 +1835,7 @@ processing moves to the line after the @
 A @samp{#line} directive changes @code{__LINE__}, and may change
 @code{__FILE__} as well.  @xref{Line Control}.
 
-C99 introduces @code{__func__}, and GCC has provided 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Martin Sebor]

By moving the warning earlier, we'll still warn for the most cases, but
won't warn in the more convoluted cases.  We can perhaps work on it
further
in GCC 8.  If we keep it as is, I think most users will just -Wno-nonnull
as soon as they run into some warning that will be hard to figure out
what
is going on.  At that point they will not get warnings even for the
obvious
cases that we used to warn.  Look at how the Linux kernel folks
disable most
of warnings even for smaller reasons.

But again, the user case is no different than other warnings that are
sensitive to optimizations.

My sense is that we should revert and table until gcc-8 where we can
evaluate the space more thoroughly.


I think that would be unfortunate.  We have a number of alternatives
that seem much preferable.

I have a trivial patch that avoids the sanitizer warnings by disabling
the late -Wnonnull warning when -fsanitize=undefined is specified.
A simple fix for the GCC warnings discovered by profiledbootstrap-O3
and verified on powerpc64 and x86_63 was posted for review last week.

Jakub's patch avoids those warnings and obviates any GCC changes (IIUC).

There is also the option of introducing -Wnonnull=2 that warns late
and not including it in -Wall or -Wextra.  All three of this have
been tested.

All of these seem safe to me and give interested users the ability to
experiment with the warning and give us feedback.  With alternative
(1) users have the option of turning -Wnonnull off.  Since the early
warning detects just a trivial subset of these problems (and is still
prone to false positives) they would be giving up little by doing that.

Martin

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/16/2016 10:10 AM, Martin Sebor wrote:

On 12/16/2016 09:46 AM, Jakub Jelinek wrote:

On Fri, Dec 16, 2016 at 09:36:25AM -0700, Martin Sebor wrote:

It does for me with an allmodconf. At -O2 I get three warnings, and at
-O3 I get two additional warnings. Now these additional ones happen way
too deep into the pipeline to be reliable. (For a reduced testcase see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8)


The warning looks quite justified in this case.  The first call
to sm_read_sector exits with the pointer being null and so the
second call is diagnosed.  That seems like a great example of
when the warning is useful.


No.  The first call to sm_read_sector just doesn't exit.  So it is
warning
about dead code.


If the code is dead then GCC should eliminate it.  With it eliminated
the warning would be gone.  The warning isn't smart and it doesn't
try to be.  It only works with what GCC gives it.  In this case the
dump shows that GCC thinks the code is reachable.  If it isn't that
would seem to highlight a missed optimization opportunity, not a need
to make the warning smarter than the optimizer.
It's almost always the case that a false positive warning of this nature 
represents a missed optimization somewhere.



However, detecting the missed optimization can be extremely difficult or 
borderline impossible. They often arise out of unfortunate API practices 
(our vec implementation and NULL pointer dereferences show many great 
examples).


And in other cases we may detect the potential for optimizing away the 
path, but to do so simply isn't profitable.  Many of these opportunities 
are path specific.  To be able to take advantage of the path specific 
properties you have to isolate the path (ie, create a duplicate that you 
can munge).  The cost of duplication often exceeds the value in removing 
the redundancy/dead code.


I often wonder if we should look at some of the schemes out there that 
allow marking of infeasible paths through the CFG.  Then we could ignore 
those paths through the CFG (you essentially build def-use pairs that 
are ignored).  It sounds good in theory, but I don't know if it's ever 
been tried in practice.




They do to me.  There are calls in gengtype.c to a function decorated
with attribute nonnull (lbasename) that pass to it a pointer that's
potentially null.  For example below.  get_input_file_name returns
null when inpf is null. There are other calls to get_input_file_name
in the file that check its return value (e.g., get_file_basename)
and so those that don't suggest bugs.  If there is no way for the
get_input_file_name function to return null then that suggests
the function should be declared returns_nonnull (and the return
NULL statement from it removed), and all those callers that check
for null simplified.  In any case, at a minimum the warning points
out an inconsistency that suggests a possible improvement.  That,
in my view, is one of the things that make warnings valuable even
if they don't identify the smoking gun.
I think this touches on another (gcc-8) issue.  Namely using the IPA 
propagation engine to propagate more of these attributes so that we're 
not always mucking around trying to annotate things internally, but 
instead let the compiler do much of the heavy lifting.


jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 12:50:00PM -0700, Jeff Law wrote:
> > Unrelated to where the warning is issued, it might be a good idea to use
> > %K to emit it with inlining stack, otherwise figuring out why it warns
> > will be harder than needed.
> I would think that would apply to any warning triggered once we've started
> optimizing the code.  Don't you?

Probably just any warning post-einline, yes.
Note %K takes a tree, which we have in the expansion (the CALL_EXPR), but
for gimple either we need to build some random tree with the location_t
of gimple_location (stmt), or add another modifier that takes a location_t
and otherwise does the similar thing as %K.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 12:12 PM, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 07:58:54PM +0100, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote:

I don't claim it can't be improved but it seems pretty good as
it is already.  Among the 6 instances it's found in GCC three
look like real bugs.


None look like real bugs to me.

But is the warning rate so high that we need to revert/reject the warning as
implemented.  That's my question.  6 across GCC doesn't sound bad across a
multi-million line codebase.


It isn't 6 across GCC, it is 6 across a single target and single set of
compiler options.  Other targets and other options have different sets,
there is some overlap, but only partial.


Unrelated to where the warning is issued, it might be a good idea to use
%K to emit it with inlining stack, otherwise figuring out why it warns
will be harder than needed.
I would think that would apply to any warning triggered once we've 
started optimizing the code.  Don't you?

jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 11:56 AM, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 11:46:24AM -0700, Jeff Law wrote:

But I don't see that as inherently blocking this patch.  It's pointing out a
bad API interface.  It's no different than when I added teh NULL pointer
dereference warnings a while ago -- we had the exact same kinds of problems.

The question is how many of them are there.  We *know* this kind of thing is
going to happen.  Again, at this point I don't see 78859 as inherently
meaning Martin's patch should be reverted.


profiledbootstrap is meant to be supported without --disable-werror, has
been working that way for at least last 10 years.
But we also have to twiddle things to deal with profiledbootstrap 
selecting different jump threads to realize and as a result we get 
different Wuninitialized warnings.   This happens all the time



 And so is normal

bootstrap on powerpc* or hppa*.
Agreed, but we need to investigate those at a level deep enough to 
understand the real problem.



So broken bootstrap is a

strong reason for having to do something.
Certainly we have to do something.  But that doesn't mean flat out 
reversion and rejection of the patch.  It means careful analysis of the 
costs & benefits.  Jumping to reversion & rejection because it triggers 
some warnings in cases we don't like is too hasty.





By moving the warning earlier, we'll still warn for the most cases, but
won't warn in the more convoluted cases.  We can perhaps work on it further
in GCC 8.  If we keep it as is, I think most users will just -Wno-nonnull
as soon as they run into some warning that will be hard to figure out what
is going on.  At that point they will not get warnings even for the obvious
cases that we used to warn.  Look at how the Linux kernel folks disable most
of warnings even for smaller reasons.
But again, the user case is no different than other warnings that are 
sensitive to optimizations.


My sense is that we should revert and table until gcc-8 where we can 
evaluate the space more thoroughly.


Jeff

Re: [PATCH] PR 78534 Change character length from int to size_t

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 6:43 PM, Bob Deen  wrote:
> Hi all...
>
> I never saw any followup on this...?
>
> It's one thing to break the ABI between the compiler and the gfortran
> library; those can generally be expected to be in sync.  It's another to
> break the ABI between two *languages*, when there might be no such
> expectation (especially if gcc does NOT break their ABI at the same version
> number transition).  Yes, the pre-ISO_C_BINDING method may be old-fashioned,
> but it is a de-facto standard, and breaking it should not be done lightly.

First: No, it's not done "lightly". And secondly, cross-language
interfacing is always tricky, which is why most languages, including
Fortran with ISO_C_BINDING, have devised standardized ways for
communication with C so users don't need to rely on various cross-call
mechanisms that are not guaranteed to work.

That the charlen is a hidden argument added at the end of type int is
AFAIK a fairly common implementation choice, though I'm not sure if it
can be called a de-facto standard.  Considering that Intel Fortran has
switched to size_t several years ago (5-ish?), and AFAIU it's the most
used Fortran compiler around in addition to GFortran, and the world
hasn't crashed down due to it, I suspect users can adapt to the change
with relatively little drama.

That gcc would change it's ABI at all, and especially in conjunction
with gfortran, is a pipe dream.

C changed to use size_t for string lengths instead of int with ANSI C
in, what, 1989.  With 2-socket servers, typically used e.g. in HPC
clusters, today easily having hundreds of gigs of RAM, limiting
GFortran char lengths to 2 GB for all eternity in the name of
compatibility seems quaint at best. Maybe in your organization Fortran
is legacy code that YE SHALL NOT TOUCH, but GFortran also has to cater
to users who have chosen to write new code in Fortran.

> If you do proceed with changing the size, I would request that there at
> least be a facility to reliably tell at compile time (on the C side) which
> definition is being used, so I can adjust our macros accordingly.

Oh, you have macros rather than hard-coded int all over the place?
Shouldn't it be a relatively trivial affair then to define that macro
appropriately depending on which compiler and which version you're
using?

Steve showed how you can do it for Fortran. From the C side, just
check the version from the __GNUC__ macro.

> Our code
> does depend on the size, and it has to cross-platform (and now, if this
> change is made, cross-version), so with this change I would have to support
> both int and size_t.

Well, if you add the option to use size_t you should be able to use
ifort as well. :)

> A C-side preprocessor symbol definition would do the trick.  Of course that
> assumes the versions of gcc/g++ and gfortran are in sync, which is never
> guaranteed.  But that assumption is better than nothing.  Unless someone has
> a better idea...?

Yeah, I think that's the best idea.

Another option would be to implement some kind of
-fcharacter-length=[int,size_t] command-line option. But that would
make the patch a lot more complicated since one would need to typecast
the character length argument when calling libgfortran. And, you'd
still have to have some version-dependent checks to see if gfortran
would accept that option. And like other similar options like
-fdefault-this-or-that it would change the ABI, so code compiled with
that option would be incompatible with code compiled without it. So in
the end I'm not convinced such an option would actually make life any
easier for our users.

> Perhaps it might be best to wait until a time when gcc is also breaking
> their ABI, so that there's no question of code (on either side) working
> across the transition...?

AFAIK there is no ABI change planned for gcc. For better or worse, the
C language is relatively stable and doesn't change much.

> P.S.  I'm just a lurker here, but I lurk specifically to look for things
> that will break our code base, like this  ;-)

Well, then you ought to be aware the ABI cleanup page on the wiki,
where the char length issue has been listed for, what, 5 years or so,
so it can't really be a surprise that it will happen at some point,
can it...?

>
> Bob.Deen @ NASA-JPL Multimission Image Processing Lab
> bob.d...@jpl.nasa.gov
>
>
>
> On 12/12/16 10:26 AM, Bob Deen wrote:
>>
>>
>>> However, this will also affect people doing C->Fortran calls the
>>> old-fashioned way without ISO_C_BINDING, as they will have to change
>>> the string length argument from int to size_t in their prototypes.
>>> Then again, Intel Fortran did this some years ago so I guess at least
>>> people who care about portability to several compilers are aware.
>>
>>
>> We do a ton of this (old fashioned c-fortran binding) and changing the
>> string length argument size will have a big impact on us.  We don't use the
>> Intel compiler so we never noticed a change there.
>>
>> Is 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 07:58:54PM +0100, Jakub Jelinek wrote:
> On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote:
> > > > I don't claim it can't be improved but it seems pretty good as
> > > > it is already.  Among the 6 instances it's found in GCC three
> > > > look like real bugs.
> > > 
> > > None look like real bugs to me.
> > But is the warning rate so high that we need to revert/reject the warning as
> > implemented.  That's my question.  6 across GCC doesn't sound bad across a
> > multi-million line codebase.
> 
> It isn't 6 across GCC, it is 6 across a single target and single set of
> compiler options.  Other targets and other options have different sets,
> there is some overlap, but only partial.

Unrelated to where the warning is issued, it might be a good idea to use
%K to emit it with inlining stack, otherwise figuring out why it warns
will be harder than needed.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 11:54:06AM -0700, Jeff Law wrote:
> > > I don't claim it can't be improved but it seems pretty good as
> > > it is already.  Among the 6 instances it's found in GCC three
> > > look like real bugs.
> > 
> > None look like real bugs to me.
> But is the warning rate so high that we need to revert/reject the warning as
> implemented.  That's my question.  6 across GCC doesn't sound bad across a
> multi-million line codebase.

It isn't 6 across GCC, it is 6 across a single target and single set of
compiler options.  Other targets and other options have different sets,
there is some overlap, but only partial.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 11:46:24AM -0700, Jeff Law wrote:
> But I don't see that as inherently blocking this patch.  It's pointing out a
> bad API interface.  It's no different than when I added teh NULL pointer
> dereference warnings a while ago -- we had the exact same kinds of problems.
> 
> The question is how many of them are there.  We *know* this kind of thing is
> going to happen.  Again, at this point I don't see 78859 as inherently
> meaning Martin's patch should be reverted.

profiledbootstrap is meant to be supported without --disable-werror, has
been working that way for at least last 10 years.  And so is normal
bootstrap on powerpc* or hppa*.  So broken bootstrap is a
strong reason for having to do something.  Richard expressed his
dissatisfaction with the vec.h change:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c17
By moving the warning earlier, we'll still warn for the most cases, but
won't warn in the more convoluted cases.  We can perhaps work on it further
in GCC 8.  If we keep it as is, I think most users will just -Wno-nonnull
as soon as they run into some warning that will be hard to figure out what
is going on.  At that point they will not get warnings even for the obvious
cases that we used to warn.  Look at how the Linux kernel folks disable most
of warnings even for smaller reasons.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/16/2016 09:46 AM, Jakub Jelinek wrote:

On Fri, Dec 16, 2016 at 09:36:25AM -0700, Martin Sebor wrote:

It does for me with an allmodconf. At -O2 I get three warnings, and at
-O3 I get two additional warnings. Now these additional ones happen way
too deep into the pipeline to be reliable. (For a reduced testcase see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8)


The warning looks quite justified in this case.  The first call
to sm_read_sector exits with the pointer being null and so the
second call is diagnosed.  That seems like a great example of
when the warning is useful.


No.  The first call to sm_read_sector just doesn't exit.  So it is warning
about dead code.
Correct.  It's a false positive.  Effectively the loop is never going to 
terminate.






I don't claim it can't be improved but it seems pretty good as
it is already.  Among the 6 instances it's found in GCC three
look like real bugs.


None look like real bugs to me.
But is the warning rate so high that we need to revert/reject the 
warning as implemented.  That's my question.  6 across GCC doesn't sound 
bad across a multi-million line codebase.


jeff

Re: [gimplefe] reject invalid pass name in startwith

[Joseph Myers]

The message passed to error_at should not end in \n; the diagnostics 
machinery deals with inserting the newline.

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

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 11:30 AM, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 10:52:13AM -0700, Jeff Law wrote:

No, it highlights that the warning is done in a wrong place where it suffers
from too many false positives.

I don't inherently see this as generating "too many false positives". And as
Martin says, the warning works with precisely what it is presented.

I think the particular stumbling point is path isolation at some point as
resulted in a NULL explicitly in calls at various places.  That is a *GOOD*
thing to detect and warn against as it represents cases that are often well
hidden and often difficult for a human to analyze (based on my work with
NULL pointer dereference warnings).


Please see e.g. PR78859 for just two recently reported issues (there are more
from gathering what has been said on IRC etc., David said powerpc* bootstrap
is still broken, ...).
One has the non-NULL tests just as a weirdo programming style, not actually
a sign that NULL will ever show there.  So this one could be fixed in theory 
rather
than adding hacks to assert it is non-NULL just remove all those NULL tests.
The other is just too cryptic, there is not even locus printed for the
strlen, so nobody can guess where it is coming from, guessing why will be
hard even if location is provided.
But I don't see that as inherently blocking this patch.  It's pointing 
out a bad API interface.  It's no different than when I added teh NULL 
pointer dereference warnings a while ago -- we had the exact same kinds 
of problems.


The question is how many of them are there.  We *know* this kind of 
thing is going to happen.  Again, at this point I don't see 78859 as 
inherently meaning Martin's patch should be reverted.




Jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 11:00 AM, Martin Sebor wrote:

On 12/19/2016 10:31 AM, Jeff Law wrote:

On 12/17/2016 02:55 PM, Martin Sebor wrote:

On 12/17/2016 01:01 PM, Markus Trippelsdorf wrote:

I agree that these warnings should probably not be issued, though
it's interesting to see where they come from.  The calls are in
the code emitted by GCC, are reachable, and end up taking place
with the right Ubsan runtime recovery options.  It turns out that
Ubsan transforms calls to nonnull functions into conditional
branches testing the argument for null, like so:

if (s == 0)
  __builtin___ubsan_handle_nonnull_arg();
n = strlen (s);

and GCC then transforms those into

if (s == 0)
  {
__builtin___ubsan_handle_nonnull_arg();
n = strlen (NULL);
  }

When the ubsan_handle_nonnull_arg function returns to the caller
the call to strlen(NULL) is made.

So I'd like to see more complete dumps here.


The -Wnonnull warning can be reproduced with this C test case and
-fsantize=undefined:

  char* f (const char *s)
  {
unsigned n = __builtin_strlen (s) + 1;
char *d = __builtin_malloc (n);

if (!d)
  return 0;

__builtin_memcpy (d, s, n);
return d;
  }

The sanitizer emits the following code (I snipped the rest after
the call to malloc):

[ snip.]

THanks.

So this is a clear case of jump threading doing exactly what it is 
supposed to do.  We've duplicated code on the cold path to enable 
removal of a test on the hot path.  That is *exactly* what we want.


The fact that doing so creates a false positive is inherent in the 
transformation.  The question is does this happen enough to warrant 
moving the warning early in the pipeline -- knowing that doing so will 
likely reduce this kind of false positive, but may also introduce missed 
warnings.


--- details follow --



So the key thing to note is that sanitization wants to add the test s != 
NULL before the strlen call and the memcpy call.


Looking at the .thread2 dump:

;;   basic block 2, loop depth 0, count 0, freq 1, maybe hot
;;prev block 0, next block 3, flags: (NEW, REACHABLE, VISITED)
;;pred:   ENTRY [100.0%]  (FALLTHRU,EXECUTABLE)
  if (s_7(D) == 0B)
goto ; [0.0%]
  else
goto ; [100.0%]
;;succ:   4 [100.0%]  (FALSE_VALUE,EXECUTABLE)
;;3 [0.0%]  (TRUE_VALUE,EXECUTABLE)

;;   basic block 3, loop depth 0, count 0, freq 4
;;prev block 2, next block 4, flags: (NEW, REACHABLE, VISITED)
;;pred:   2 [0.0%]  (TRUE_VALUE,EXECUTABLE)
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data0);
;;succ:   4 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 4, loop depth 0, count 0, freq 1, maybe hot
;;prev block 3, next block 5, flags: (NEW, REACHABLE, VISITED)
;;pred:   2 [100.0%]  (FALSE_VALUE,EXECUTABLE)
;;3 [100.0%]  (FALLTHRU,EXECUTABLE)
  _1 = __builtin_strlen (s_7(D));
  _2 = (unsigned int) _1;
  n_8 = _2 + 1;
  _3 = (long unsigned int) n_8;
  d_10 = __builtin_malloc (_3);
  if (d_10 == 0B)
goto ; [4.1%]
  else
goto ; [95.9%]
;;succ:   8 [4.1%]  (TRUE_VALUE,EXECUTABLE)
;;5 [95.9%]  (FALSE_VALUE,EXECUTABLE)

;;   basic block 5, loop depth 0, count 0, freq 9593, maybe hot
;;prev block 4, next block 6, flags: (NEW, REACHABLE, VISITED)
;;pred:   4 [95.9%]  (FALSE_VALUE,EXECUTABLE)
  if (s_7(D) == 0B)
goto ; [0.0%]
  else
goto ; [100.0%]
;;succ:   7 [100.0%]  (FALSE_VALUE,EXECUTABLE)
;;6 [0.0%]  (TRUE_VALUE,EXECUTABLE)

;;   basic block 6, loop depth 0, count 0, freq 4
;;prev block 5, next block 7, flags: (NEW, REACHABLE, VISITED)
;;pred:   5 [0.0%]  (TRUE_VALUE,EXECUTABLE)
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data2);
;;succ:   7 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 7, loop depth 0, count 0, freq 9593, maybe hot
;;prev block 6, next block 8, flags: (NEW, REACHABLE, VISITED)
;;pred:   5 [100.0%]  (FALSE_VALUE,EXECUTABLE)
;;6 [100.0%]  (FALLTHRU,EXECUTABLE)
  __builtin_memcpy (d_10, s_7(D), _3);
;;succ:   8 [100.0%]  (FALLTHRU,EXECUTABLE)

;;   basic block 8, loop depth 0, count 0, freq 1, maybe hot
;;prev block 7, next block 1, flags: (NEW, REACHABLE, VISITED)
;;pred:   4 [4.1%]  (TRUE_VALUE,EXECUTABLE)
;;7 [100.0%]  (FALLTHRU,EXECUTABLE)
  # _4 = PHI <0B(4), d_10(7)>
  return _4;
;;succ:   EXIT [100.0%]

}

We can see the redundant test showing up in bb2 and bb5.  Note the tests 
are on the hot path.


Also note that 2->4->5  will result in a control transfer to 7 and 
3->4->5 will result in a control transfer to 6.  As seen in the .dom2 dump:


  Registering jump thread: (3, 4) incoming edge;  (4, 5) joiner;  (5, 
6) nocopy;


  Registering jump thread: (2, 4) incoming edge;  (4, 5) joiner;  (5, 
7) nocopy;


The "joiner" note means that we don't know where BB4 will go.  But if it 
goes to 5, then we will unconditionally 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 10:52:13AM -0700, Jeff Law wrote:
> > No, it highlights that the warning is done in a wrong place where it suffers
> > from too many false positives.
> I don't inherently see this as generating "too many false positives". And as
> Martin says, the warning works with precisely what it is presented.
> 
> I think the particular stumbling point is path isolation at some point as
> resulted in a NULL explicitly in calls at various places.  That is a *GOOD*
> thing to detect and warn against as it represents cases that are often well
> hidden and often difficult for a human to analyze (based on my work with
> NULL pointer dereference warnings).

Please see e.g. PR78859 for just two recently reported issues (there are more
from gathering what has been said on IRC etc., David said powerpc* bootstrap
is still broken, ...).
One has the non-NULL tests just as a weirdo programming style, not actually
a sign that NULL will ever show there.  So this one could be fixed in theory 
rather
than adding hacks to assert it is non-NULL just remove all those NULL tests.
The other is just too cryptic, there is not even locus printed for the
strlen, so nobody can guess where it is coming from, guessing why will be
hard even if location is provided.

Jakub

Re: [PATCH] PR 78534 Change character length from int to size_t

[Steve Kargl]

On Mon, Dec 19, 2016 at 08:43:01AM -0800, Bob Deen wrote:
> 
> It's one thing to break the ABI between the compiler and the gfortran 
> library; those can generally be expected to be in sync.  It's another to 
> break the ABI between two *languages*, when there might be no such 
> expectation (especially if gcc does NOT break their ABI at the same 
> version number transition).  Yes, the pre-ISO_C_BINDING method may be 
> old-fashioned, but it is a de-facto standard, and breaking it should not 
> be done lightly.

Do you really think that those of us who actively contribute to 
gfortran development take breaking the ABI lightly?  We have put 
off changes to gfortran's library for several years to specifically 
avoid ABI breakage.  It seems that there is never a "Good Time" to
break the ABI.  However, in this case, support for F2008 9.6.4.8,
Defined Input/Output, necessitates a change in the ABI.  Instead of
breaking the ABI multiple times, it has been decided to try to cleanup
some long standing issues with libgfortran.

> If you do proceed with changing the size, I would request that there at 
> least be a facility to reliably tell at compile time (on the C side) 
> which definition is being used, so I can adjust our macros accordingly. 
> Our code does depend on the size, and it has to cross-platform (and now, 
> if this change is made, cross-version), so with this change I would have 
> to support both int and size_t.

As the breakage is going to occur with gfortran 7.0, you do

% cat a.F90
#if defined(__GFORTRAN__) && (__GNUC__ > 6)
print *, '7'
#else
print *, 'not 7'
#endif
end
% gfc7 -E a.F90 | cat -s
] gfc7 -E a.F90 | cat -s
# 1 "a.F90"
# 1 ""
# 1 ""
# 1 "a.F90"

print *, '7'

end
% gfortran6 -E a.F90 | cat -s
# 1 "a.F90"
# 1 ""
# 1 ""
# 1 "a.F90"

print *, 'not 7'

end

> Perhaps it might be best to wait until a time when gcc is also breaking 
> their ABI, so that there's no question of code (on either side) working 
> across the transition...?

There is never a good time.  If we are to wait for gcc, should
we remove support for Defined Input/Output from the compiler?

-- 
Steve

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 10:52:13AM -0700, Jeff Law wrote:
> > > > None look like real bugs to me.
> > > 
> > > They do to me.  There are calls in gengtype.c to a function decorated
> > > with attribute nonnull (lbasename) that pass to it a pointer that's
> > > potentially null.  For example below.  get_input_file_name returns
> > 
> > Most pointers passed to functions with nonnull attributes are, from the
> > compiler POV, potentially NULL.  Usually the compiler just can't prove it
> > can't be non-NULL, it is an exception if it can.
> True.  But what's happening here IIUC is that there is an explict NULL for
> those arguments in the IL, most likely due to path isolation.
> 
> I would agree that a random pointer which we know nothing about could be
> NULL, but we shouldn't warn for it.  That would generate too many false
> positives.
> 
> What those guidelines do mean is that various transformations and
> optimizations may make warnings appear or disappear seemingly randomly.
> That's unfortunate, but inherent in this class of problems until we have a
> real static analyzer.

>From the testcases posted, there is a clear difference between "pointer is
compared to NULL in the current function and soon after that is passed
to a function which expects non-NULL", everything in one function,
from a NULL pointer checked in inline function called from 3 other inline
functions.  If you test for a NULL pointer in the same function, the
likelyhood that you actually do it because NULL could be passed in is much
higher, over when somebody else wrote some other function that just happens
to test for NULL somewhere.
For path isolation it is the same thing, but IMHO not so for
warnings.  So, if we want to catch the first case, we shouldn't rely on path
isolation to sometimes trigger if it is beneficial, but rather than have
infrastructure which allows us to answer whether there is a high probability
user expects a pointer might be NULL (or value might be 0 or whatever other
constant), and use that in the warning for some -Wmaybe-* variant of
selected warnings.  We'd then warn regardless if path isolation is
beneficial or not, but wouldn't warn if it is unlikely useful to warn (or
there could be different levels between what we want to catch and what
amount of false positives we allow).

Jakub

[PATCH] Add support for Fuchsia (OS)

[Josh Conner via gcc-patches]

Ping?


On 12/12/16 1:31 PM, Josh Conner wrote:

On 12/10/16 3:26 AM, Richard Earnshaw wrote:

On 08/12/16 22:55, Josh Conner wrote:

+arm*-*-fuchsia*)
+  tm_file="${tm_file} fuchsia.h arm/fuchsia-elf.h glibc-stdint.h"
+  tmake_file="${tmake_file} arm/t-bpabi"
+  ;;


This will leave the default cpu as arm7tdmi.  Is that what you want?
It's fine if it is, but if not, you should consider setting
target_cpu_cname here as well.


Mmm, probably not. Thanks for pointing that out.
I've attached an updated patch addressing all of the concerns so far.

OK for mainline?

Thanks -

Josh


2016-12-08  Joshua Conner

* config/arm/fuchsia-elf.h: New file.
* config/fuchsia.h: New file.
* config.gcc (*-*-fuchsia*): Set native_system_header_dir.
(aarch64*-*-fuchsia*, arm*-*-fuchsia*, x86_64-*-fuchsia*): Add to
targets.
* config.host: (aarch64*-*-fuchsia*, arm*-*-fuchsia*): Add to hosts.

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Martin Sebor]

On 12/19/2016 10:31 AM, Jeff Law wrote:

On 12/17/2016 02:55 PM, Martin Sebor wrote:

On 12/17/2016 01:01 PM, Markus Trippelsdorf wrote:

I agree that these warnings should probably not be issued, though
it's interesting to see where they come from.  The calls are in
the code emitted by GCC, are reachable, and end up taking place
with the right Ubsan runtime recovery options.  It turns out that
Ubsan transforms calls to nonnull functions into conditional
branches testing the argument for null, like so:

if (s == 0)
  __builtin___ubsan_handle_nonnull_arg();
n = strlen (s);

and GCC then transforms those into

if (s == 0)
  {
__builtin___ubsan_handle_nonnull_arg();
n = strlen (NULL);
  }

When the ubsan_handle_nonnull_arg function returns to the caller
the call to strlen(NULL) is made.

So I'd like to see more complete dumps here.


The -Wnonnull warning can be reproduced with this C test case and
-fsantize=undefined:

  char* f (const char *s)
  {
unsigned n = __builtin_strlen (s) + 1;
char *d = __builtin_malloc (n);

if (!d)
  return 0;

__builtin_memcpy (d, s, n);
return d;
  }

The sanitizer emits the following code (I snipped the rest after
the call to malloc):

   [0.00%]:
  if (s_8(D) == 0B)
goto ; [0.04%]
  else
goto ; [99.96%]

   [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data0);

   [0.00%]:
  _1 = __builtin_strlen (s_8(D));
  _2 = (unsigned int) _1;
  n_9 = _2 + 1;
  _3 = (long unsigned int) n_9;
  d_11 = __builtin_malloc (_3);
  ...

This is then transformed by the third thread jumping pass into:

   [100.00%]:
  if (s_7(D) == 0B)
goto ; [0.04%]
  else
goto ; [99.96%]

   [0.04%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data0);
  _24 = __builtin_strlen (0B);
  _25 = (unsigned int) _24;
  n_26 = _25 + 1;
  _27 = (long unsigned int) n_26;
  d_29 = __builtin_malloc (_27);
  if (d_29 == 0B)
goto ; [4.07%]
  else
goto ; [95.93%]

   [4.07%]:
  goto ; [100.00%]

   [0.04%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data2);

   [95.93%]:
  # _30 = PHI <_19(8), _27(5)>
  # d_31 = PHI 
  __builtin_memcpy (d_31, s_7(D), _30);

   [100.00%]:
  # _4 = PHI <0B(4), d_31(6)>
  return _4;

   [99.96%]:
  _16 = __builtin_strlen (s_7(D));
  _21 = (unsigned int) _16;
  n_20 = _21 + 1;
  _19 = (long unsigned int) n_20;
  d_22 = __builtin_malloc (_19);
  if (d_22 == 0B)
goto ; [4.07%]
  else
goto ; [95.93%]

(If you'd like to see more context please let me know.)

Martin

libgo patch committed: Copy/rewrite cgo support code from Go 1.7 runtime

[Ian Lance Taylor]

This patch copies the cgo support code from the Go 1.7 runtime to
libgo.  The cgo support in gccgo is rather different, so all the code
in cgo_gccgo.go is gccgo-specific.  The rest of the code is similar
but slightly different.  This drops _cgo_allocate, which was removed
from the gc toolchain back in 1.5.  Bootstrapped and ran Go testsuite
on x86_64-pc-linux-gnu.  Committed to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 243766)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e6fb629c5b246bceab5fc8e8613cf2cf82b1e98f
+4a0bb435bbb1d1516b486d1998e8dc184576db61
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/cgo_gccgo.go
===
--- libgo/go/runtime/cgo_gccgo.go   (revision 0)
+++ libgo/go/runtime/cgo_gccgo.go   (working copy)
@@ -0,0 +1,110 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+import (
+   "runtime/internal/atomic"
+   _ "unsafe"
+)
+
+// For historical reasons these functions are called as though they
+// were in the syscall package.
+//go:linkname Cgocall syscall.Cgocall
+//go:linkname CgocallDone syscall.CgocallDone
+//go:linkname CgocallBack syscall.CgocallBack
+//go:linkname CgocallBackDone syscall.CgocallBackDone
+
+// A routine that may be called by SWIG.
+//go:linkname _cgo_panic _cgo_panic
+
+// iscgo is set to true if the cgo tool sets the C variable runtime_iscgo
+// to true.
+var iscgo bool
+
+// cgoHasExtraM is set on startup when an extra M is created for cgo.
+// The extra M must be created before any C/C++ code calls cgocallback.
+var cgoHasExtraM bool
+
+// Cgocall prepares to call from code written in Go to code written in
+// C/C++. This takes the current goroutine out of the Go scheduler, as
+// though it were making a system call. Otherwise the program can
+// lookup if the C code blocks. The idea is to call this function,
+// then immediately call the C/C++ function. After the C/C++ function
+// returns, call cgocalldone. The usual Go code would look like
+// syscall.Cgocall()
+// defer syscall.Cgocalldone()
+// cfunction()
+func Cgocall() {
+   lockOSThread()
+   mp := getg().m
+   mp.ncgocall++
+   mp.ncgo++
+   entersyscall(0)
+}
+
+// CgocallDone prepares to return to Go code from C/C++ code.
+func CgocallDone() {
+   gp := getg()
+   if gp == nil {
+   throw("no g in CgocallDone")
+   }
+   gp.m.ncgo--
+
+   // If we are invoked because the C function called _cgo_panic,
+   // then _cgo_panic will already have exited syscall mode.
+   if gp.atomicstatus == _Gsyscall {
+   exitsyscall(0)
+   }
+
+   unlockOSThread()
+}
+
+// CgocallBack is used when calling from C/C++ code into Go code.
+// The usual approach is
+// syscall.CgocallBack()
+// defer syscall.CgocallBackDone()
+// gofunction()
+//go:nosplit
+func CgocallBack() {
+   if getg() == nil || getg().m == nil {
+   needm(0)
+   mp := getg().m
+   mp.dropextram = true
+   }
+
+   exitsyscall(0)
+
+   if getg().m.ncgo == 0 {
+   // The C call to Go came from a thread created by C.
+   // The C call to Go came from a thread not currently running
+   // any Go. In the case of -buildmode=c-archive or c-shared,
+   // this call may be coming in before package initialization
+   // is complete. Wait until it is.
+   <-main_init_done
+   }
+
+   mp := getg().m
+   if mp.needextram || atomic.Load() > 0 {
+   mp.needextram = false
+   newextram()
+   }
+}
+
+// CgocallBackDone prepares to return to C/C++ code that has called
+// into Go code.
+func CgocallBackDone() {
+   entersyscall(0)
+   mp := getg().m
+   if mp.dropextram && mp.ncgo == 0 {
+   mp.dropextram = false
+   dropm()
+   }
+}
+
+// _cgo_panic may be called by SWIG code to panic.
+func _cgo_panic(p *byte) {
+   exitsyscall(0)
+   panic(gostringnocopy(p))
+}
Index: libgo/go/runtime/cgo_mmap.go
===
--- libgo/go/runtime/cgo_mmap.go(revision 243084)
+++ libgo/go/runtime/cgo_mmap.go(working copy)
@@ -1,43 +0,0 @@
-// Copyright 2015 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build ignore
-
-// Support for memory sanitizer. See runtime/cgo/mmap.go.
-
-// +build linux,amd64
-
-package runtime
-
-import "unsafe"
-
-// _cgo_mmap is filled in by 

[Patch] Turn -fexcess-precision=fast on when in -ffast-math

[James Greenhalgh]

> On Thu, Dec 8, 2016 at 10:44 PM, Uros Bizjak  wrote:
> > Hello!
> >
> > Attached patch fixes fall-out from excess-precision improvements
> > patch. As shown in the PR, the code throughout the compiler assumes
> > FLAG_PRECISION_FAST when flag_unsafe_math_optimizations flag is in
> > effect. The patch puts back two lines, removed by excess-precision
> > improvements patch.
> >
> > 2016-12-08  Uros Bizjak  
> >
> > PR middle-end/78738
> > * toplev.c (init_excess_precision): Initialize flag_excess_precision
> > to EXCESS_PRECISION_FAST for flag_unsafe_math_optimizations.
> >
> > testsuite/ChangeLog:
> >
> > 2016-12-08  Uros Bizjak  
> >
> > PR middle-end/78738
> > * gcc.target/i386/pr78738.c: New test.
> >
> > Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> >
> > OK for mainline?
>
> Hmm, I think it belongs to set_unsafe_math_optimization_flags instead
> (and be consistent if -fexcess-precision was manually specified).

I think it would be better if this were implied by -ffast-math/-Ofast
than by -funsafe-math-optimizations . That's what I've implemented here,
and tagged the option as SetByCombined to allow us to honour what the
user requests.

This should give us the behaviour you were looking for Uros.

I've bootstrapped and tested the behaviour on x86_64, and I've hacked up
the AArch64 backend to validate that we're setting the flag in the right
circumstances (but that meant changing the AArch64 behaviour, so isn't
something we'd want on trunk, and therefore I can't write a testcase for
this patch).

OK?

Thanks,
James

---
2016-12-19  James Greenhalgh  

* common.opt (excess_precision): Tag as SetByCombined.
* opts.c (set_fast_math_flags): Also set
flag_excess_precision_cmdline.
(fast_math_flags_set_p): Also check flag_excess_precision_cmdline.

diff --git a/gcc/common.opt b/gcc/common.opt
index de06844..6ebaf9c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1317,7 +1317,7 @@ Common Report Var(flag_expensive_optimizations) Optimization
 Perform a number of minor, expensive optimizations.
 
 fexcess-precision=
-Common Joined RejectNegative Enum(excess_precision) Var(flag_excess_precision_cmdline) Init(EXCESS_PRECISION_DEFAULT)
+Common Joined RejectNegative Enum(excess_precision) Var(flag_excess_precision_cmdline) Init(EXCESS_PRECISION_DEFAULT) SetByCombined
 -fexcess-precision=[fast|standard]	Specify handling of excess floating-point precision.
 
 Enum
diff --git a/gcc/opts.c b/gcc/opts.c
index 890da03..5844190 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2342,6 +2342,10 @@ set_fast_math_flags (struct gcc_options *opts, int set)
 opts->x_flag_errno_math = !set;
   if (set)
 {
+  if (opts->frontend_set_flag_excess_precision_cmdline
+	  == EXCESS_PRECISION_DEFAULT)
+	opts->x_flag_excess_precision_cmdline
+	  = set ? EXCESS_PRECISION_FAST : EXCESS_PRECISION_DEFAULT;
   if (!opts->frontend_set_flag_signaling_nans)
 	opts->x_flag_signaling_nans = 0;
   if (!opts->frontend_set_flag_rounding_math)
@@ -2374,7 +2378,9 @@ fast_math_flags_set_p (const struct gcc_options *opts)
 	  && opts->x_flag_unsafe_math_optimizations
 	  && opts->x_flag_finite_math_only
 	  && !opts->x_flag_signed_zeros
-	  && !opts->x_flag_errno_math);
+	  && !opts->x_flag_errno_math
+	  && opts->x_flag_excess_precision_cmdline
+	 == EXCESS_PRECISION_FAST);
 }
 
 /* Return true iff flags are set as if -ffast-math but using the flags stored

Re: [PATCH] libstdc++: Allow using without lock free atomic int

[Jonathan Wakely]

On 16/12/16 17:52 +, Jonathan Wakely wrote:

On 09/11/16 23:26 +0200, Pauli wrote:

Compiling programs using std::future for old arm processors fails. The
problem is caused by preprocessor check for atomic lock free int.

Future can be changed to work correctly without lock free atomics with
minor changes to exception_ptr implementation.

Without lock free atomics there is question if deadlock can happen. But
atomic operations can't call outside code preventing any ABBA or
recursive mutex acquiring deadlocks.
Deadlock could happen if throwing an exception or access
is_lock_free() == false atomic from asynchronous signal handler.
Throwing from signal handler is undefined behavior. I don't know about
accessing atomics from asynchronous signal handler but that feels like
undefined behavior if is_lock_free returns false.

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

differences to current if atomic builtins available:
* Race detector annotations that are empty by default
* Check for __gthread_active_p
* Generate x86 code uses xadd instead of xsub
This makes code a bit worse. But problem is duplicated to any other user
of __exchange_and_add. The internal API implementation should be fixed
to generate better code for all cases. But that is a follow up patch.


I'd prefer to do it so we don't change anything for the targets that
already work. Your follow-up patch missed the deadline for GCC 7 and
so will have to wait for GCC 8 now, and we don't want to pessimize
x86.

Also, I think your patch actually breaks some GNU/Linux targets,
because you removed the  header from
, which means that in libsupc++/guard.cc the macro
ATOMIC_INT_LOCK_FREE is no longer defined, and so _GLIBCXX_USE_FUTEX
doesn't get defined. Now arguably guard.cc should have been including
the header directly, but it still shows why such an invasive patch is
a bad idea at this stage of the GCC 7 process.

The attached patch attempts to make exception propagation work for all
targets, without changing anything if it already works.

Do you see any problems with this alternative approach?
Could you please test it for armv5te?

It passes all tests for x86_64-linux and ppc64le-linux.

For your follow-up patch, do you already have a copyright assignment
for contributions to GCC? We'll probably need that before it can be
accepted. We don't need one for this patch, because what remains of
your original patch is just the testsuite changes, which are
mechanical and not copyrightable.


We also need to adjust the linker script to avoid adding new exports
to old symbol versions, revised patch attached. I think it would be
better to make configure define a macro like
HAVE_EXCEPTION_PTR_SINCE_GCC6 and use that in the linker script
instead of testing __GCC_ATOMIC_INT_LOCK_FREE directly. I'll work on
that.




commit a19855aecf301a30bb1d6f6c39fe2a458baddd48
Author: Jonathan Wakely 
Date:   Fri Dec 16 15:22:21 2016 +

PR64735 support exception propagation without atomics

2016-12-19  Pauli Nieminen  
	Jonathan Wakely  

	PR libstdc++/64735
	* config/abi/pre/gnu.ver [__GCC_ATOMIC_INT_LOCK_FREE > 1]
	(GLIBCXX_3.4.15, GLIBCXX_3.4.21, CXXABI_1.3.3, CXXABI_1.3.5): Make
	exports for exception_ptr, nested_exception, and future conditional.
	[__GCC_ATOMIC_INT_LOCK_FREE <= 1] (GLIBCXX_3.4.23, CXXABI_1.3.11): Add
	exports for exception_ptr, nested_exception, and future conditional.
	* include/std/future: Remove check for ATOMIC_INT_LOCK_FREE
	* libsupc++/eh_atomics.h: New file for internal use only.
	(__eh_atomic_inc, __eh_atomic_dec): New.
	* libsupc++/eh_ptr.cc (exception_ptr::_M_addref)
	(exception_ptr::_M_release) (__gxx_dependent_exception_cleanup)
	(rethrow_exception): Use eh_atomics.h reference counting helpers.
	* libsupc++/eh_throw.cc (__gxx_exception_cleanup): Likewise.
	* libsupc++/eh_tm.cc (free_any_cxa_exception): Likewise.
	* libsupc++/exception: Remove check for ATOMIC_INT_LOCK_FREE.
	* libsupc++/exception_ptr.h: Likewise.
	* libsupc++/guard.cc: Include header for ATOMIC_INT_LOCK_FREE macro.
	* libsupc++/nested_exception.cc: Remove check for
	ATOMIC_INT_LOCK_FREE.
	* libsupc++/nested_exception.h: Likewise.
	* src/c++11/future.cc: Likewise.
	* testsuite/18_support/exception_ptr/*: Remove atomic builtins checks.
	* testsuite/18_support/nested_exception/*: Likewise.
	* testsuite/30_threads/async/*: Likewise.
	* testsuite/30_threads/future/*: Likewise.
	* testsuite/30_threads/headers/future/types_std_c++0x.cc: Likewise.
	* testsuite/30_threads/packaged_task/*: Likewise.
	* testsuite/30_threads/promise/*: Likewise.
	* testsuite/30_threads/shared_future/*: Likewise.

diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index 8b0f67b..5da698e 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/16/2016 10:27 AM, Jakub Jelinek wrote:

On Fri, Dec 16, 2016 at 10:10:00AM -0700, Martin Sebor wrote:

No.  The first call to sm_read_sector just doesn't exit.  So it is warning
about dead code.


If the code is dead then GCC should eliminate it.  With it eliminated


There is (especially with jump threading, but not limited to that, other
optimizations may result in that too), code that even very smart optimizing
compiler isn't able to prove that is dead.


the warning would be gone.  The warning isn't smart and it doesn't
try to be.  It only works with what GCC gives it.  In this case the
dump shows that GCC thinks the code is reachable.  If it isn't that
would seem to highlight a missed optimization opportunity, not a need
to make the warning smarter than the optimizer.


No, it highlights that the warning is done in a wrong place where it suffers
from too many false positives.
I don't inherently see this as generating "too many false positives". 
And as Martin says, the warning works with precisely what it is presented.


I think the particular stumbling point is path isolation at some point 
as resulted in a NULL explicitly in calls at various places.  That is a 
*GOOD* thing to detect and warn against as it represents cases that are 
often well hidden and often difficult for a human to analyze (based on 
my work with NULL pointer dereference warnings).






None look like real bugs to me.


They do to me.  There are calls in gengtype.c to a function decorated
with attribute nonnull (lbasename) that pass to it a pointer that's
potentially null.  For example below.  get_input_file_name returns


Most pointers passed to functions with nonnull attributes are, from the
compiler POV, potentially NULL.  Usually the compiler just can't prove it
can't be non-NULL, it is an exception if it can.
True.  But what's happening here IIUC is that there is an explict NULL 
for those arguments in the IL, most likely due to path isolation.


I would agree that a random pointer which we know nothing about could be 
NULL, but we shouldn't warn for it.  That would generate too many false 
positives.


What those guidelines do mean is that various transformations and 
optimizations may make warnings appear or disappear seemingly randomly. 
That's unfortunate, but inherent in this class of problems until we have 
a real static analyzer.


jeff

Re: [PATCH, rs6000] Fold vector multiply built-ins in GIMPLE

[Segher Boessenkool]

Hi Will,

On Mon, Dec 19, 2016 at 11:01:19AM -0600, Will Schmidt wrote:
>   This patch implements folding of the vector Multiply built-ins.
> 
> As part of this patch, I have also marked variables in an existing
> testcase (mult-even-odd-be-order.c) as volatile, to prevent their being
> optimized out, which happens once this vector multiply folding was able
> to occur. 
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?


> 2016-12-19  Will Schmidt  
> 
> *  config/rs6000/rs6000.c: Add handling for early expansion of
> vector multiply builtins.
> 
> [gcc/testsuite]
> 
> 2016-12-19  Will Schmidt  
> 
> *  testsuite/gcc.dg/vmx/mult-even-odd-be-order.c : Mark
> variables as volatile.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-char.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-float.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-floatdouble.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-int.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-int128-p8.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c : New.
> *  testsuite/gcc.target/powerpc/fold-vec-mult-short.c : New.

No space before colon.  No "testsuite/" in the names here; names are
relative to do directory the changelog file is in.

Otherwise okay for trunk.  Thanks!


Segher

Re: [PATCH, rs6000] Fold vector subtract built-ins in GIMPLE

[Segher Boessenkool]

Hi, uh, Will,

On Mon, Dec 19, 2016 at 11:01:08AM -0600, Will Schmidt wrote:
>   This patch implements folding of the vector subtract built-ins.  This
> follows the form used by Bill in his previous "Fold vector addition
> built-ins in GIMPLE" patch. :-)
> 
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
> regressions.  Is this ok for trunk?

This looks fine.  Yes, okay for trunk.  Thanks,


Segher


> 2016-12-19  Will Schmidt  
> 
> * config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling 
> for
> early expansion of vector subtract builtins.
> 
> [gcc/testsuite]
> 
> 2016-12-19  Will Schmidt  
> 
>   * gcc.target/powerpc/fold-vec-sub-char.c: New.
>   * gcc.target/powerpc/fold-vec-sub-float.c: New.
>   * gcc.target/powerpc/fold-vec-sub-floatdouble.c: New.
>   * gcc.target/powerpc/fold-vec-sub-int.c: New.
>   * gcc.target/powerpc/fold-vec-sub-int128.c: New.
>   * gcc.target/powerpc/fold-vec-sub-longlong.c: New.
>   * gcc.target/powerpc/fold-vec-sub-short.c: New.

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/16/2016 01:17 PM, Jakub Jelinek wrote:

On Fri, Dec 16, 2016 at 01:01:13PM -0700, Jeff Law wrote:

Thanks.  Reduced to something like:
int
foo (const char *name)
{
  if (name)
return 6;
  return __builtin_strlen (name);
}
This is warned about both with Martin's late warning and my after ccp2
warning version.  We should include it in gcc testsuite.

I'll note this is an example of a case where Andrew's work would likely help
because it allows us to ask for a range of name_XX at the return statement
and it'll give us back a range that is constrained by the path(s) which
reach the return statement.

Contrast to the current VRP work where each SSA_NAME has a range, but that
range must be valid for every context in which that SSA_NAME appears.


Well, inside the current VRP it has separate ranges for the different paths
and actually replaces the name in the strlen argument with NULL during evrp,
so doesn't suffer from the current VRP limitations.
It'll do that sometimes, but it's not consistent and its a conscious 
design decision (which I may not necessarily agree with, but I'm not 
going to open that can-o-worms).




Anyway, let's consider the warning from the linux kernel with the closedir,
I guess it can be simplified to something along the lines of:
void baz (char *) __attribute__((nonnull));
char *bar (int);

int
foo (void)
{
  char *p = bar (1);
  int ret = 0;
  if (p == 0)
{
  bar (2);
  bar (3);
  bar (4);
  ret = 1;
  goto out;
}
  bar (5);
  bar (6);
  bar (7);
  bar (8);
 out:
  baz (p);
  if (ret)
bar (10);
  return ret;
}

Here we jump thread it and with Martin's warning position warn, with
my patch don't warn.  But if the if (ret) bar (10); is removed, the
code has the same problem that on the error path p will be NULL, but it is
not going to be diagnosed.  For -Wmaybe-nonnull we could e.g. look at if
the argument is a PHI that has NULL at any of the edges; but that doesn't
cover the above case, because p has just one def and so there will be no
PHIs.
Yea, and so what if the warning changes if that statement is removed. 
That kind of change is inherent in any late running warning.  But late 
warnings, in general, generate fewer false positives than early warnings.


I don't see this as a reason to summarily reject Martin's work.

This whole discussion highlights the primary reason why I stopped 
working on uninitialized warnings many years ago and focused strictly on 
the optimization side of the question.


Everyone has a different view on what is acceptable and what is not for 
a false positive.  Everyone has a different view on whether or not it is 
acceptable that a warning disappear when seemingly innocent changes are 
made to the source.  Should we warn early and generate more false 
positives, or late, reducing false positives, but missing warnings in 
unreachable code.   There is no single right answer and the debates are 
endless and frustrating as hell.



Jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 02:42 AM, Jakub Jelinek wrote:


The ubsan pass runs before IPA, so not sure how do you want to do that
(and it is needed to run it early).

One question is if we should perform path isolation in this case at all,
since the branches to __builtin___ubsan_handle_nonnull_arg are with
PROB_VERY_UNLIKELY, so the question is if we should be growing the
paths which is really cold.  It has some small benefit (removing one
cheap comparison from the hot path), but the grows cold block.
Growing a cold path isn't terribly problematical, particularly if it 
allows optimization on the hot path.  In an ideal world these uber-cold 
paths belong on their own pages and are never even paged in.


But I'd really like to see the full dump in this case.  I've got a 
fragment above and it looks like dumb duplication.  ie, why did we 
isolate the path.




BTW, in the testcase from the Linux kernel it is also path isolation
for error recovery path, something that ought to be predicted unlikely
(though, probably not very unlikely unlike this case), so the question is
if we want to isolate that or not too.

Agreed.

jeff

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/17/2016 02:55 PM, Martin Sebor wrote:

On 12/17/2016 01:01 PM, Markus Trippelsdorf wrote:

I agree that these warnings should probably not be issued, though
it's interesting to see where they come from.  The calls are in
the code emitted by GCC, are reachable, and end up taking place
with the right Ubsan runtime recovery options.  It turns out that
Ubsan transforms calls to nonnull functions into conditional
branches testing the argument for null, like so:

if (s == 0)
  __builtin___ubsan_handle_nonnull_arg();
n = strlen (s);

and GCC then transforms those into

if (s == 0)
  {
__builtin___ubsan_handle_nonnull_arg();
n = strlen (NULL);
  }

When the ubsan_handle_nonnull_arg function returns to the caller
the call to strlen(NULL) is made.

So I'd like to see more complete dumps here.



This doesn't happen when -fno-sanitize-recover=undefined is used
when compiling the file because then Ubsan inserts calls to
__builtin___ubsan_handle_nonnull_arg_abort instead which is declared
noreturn.
Right.  That's what I would expect.  If we're going to halt the process 
at first UB, then we want to abort.  Obviously in that case we're 
calling a noreturn function and the strlen never executes.


Otherwise the strlen still needs to be called and whateve action strlen 
has when passed a NULL must be preserved.


So the only question in my mind is what was the larger context so that 
we can look at why we isolated the paths (which brings the strlen into 
the conditional rather than leaving it at the merge point).


jeff

Re: [PATCH] builtin expansion of strncmp for rs6000

[Segher Boessenkool]

Hi Aaron,

On Mon, Dec 19, 2016 at 09:57:07AM -0600, Aaron Sawdey wrote:
> Bootstrap/regtest in progress on ppc64le -mcpu=power8, ok for trunk if
> results are clean?


> +/* Generate alignment check and branch code to set up for
> +   strncmp when we don't have DI alignment.
> +   STRNCMP_LABEL is the label to branch if there is a page crossing.
> +   SRC is the string pointer to be examined.
> +   BYTES is the max number of bytes to compare.   */

You have "tab, space" before the */ .

> +static void
> +expand_strncmp_align_check (rtx strncmp_label, rtx src, HOST_WIDE_INT bytes)
> +{
> +  rtx lab_ref = gen_rtx_LABEL_REF (VOIDmode, strncmp_label);
> +  rtx src_check = copy_addr_to_reg (XEXP (src, 0));
> +  if (GET_MODE (src_check) == SImode)
> +emit_insn (gen_andsi3 (src_check, src_check, GEN_INT (0xfff)));
> +  else
> +emit_insn (gen_anddi3 (src_check, src_check, GEN_INT (0xfff)));
> +  rtx cond = gen_reg_rtx (CCmode);
> +  emit_move_insn (cond, gen_rtx_COMPARE (CCmode, src_check,
> +  GEN_INT (4096 - bytes)));
>  

And the page break here still.

> +  /* SLOW_UNALIGNED_ACCESS -- don't do unaligned stuff.   */

Tab space.

> +{
> +  /* Compare sequence:
> +  check each 8B with: ld/ld cmpd bne
> +  cleanup code at end:
> +  cmpb  get byte that differs
> +  cmpb  look for zero byte
> +  orc   combine
> +  cntlzdget bit of first zero/diff byte
> +  subficconvert for rldcl use
> +  rldcl rldcl   extract diff/zero byte
> +  subf  subtract for final result

Mixed tabs and spaces here.

> +  /* We must always left-align the data we read, and
> +  clear any bytes to the right that are beyond the string.
> +  Otherwise the cmpb sequence won't produce the correct
> +  results.  The beginning of the compare will be done
> +  with word_mode so will not have any extra shifts or
> +  clear rights.  */

You have just a tab before */ here.

> +  /* Generate the final sequence that identifies the differing
> + byte and generates the final result, taking into account
> + zero bytes:
> +
> + cmpb   cmpb_result1, src1, src2
> + cmpb   cmpb_result2, src1, zero
> + orccmpb_result1, cmp_result1, cmpb_result2
> + cntlzd get bit of first zero/diff byte
> + addi   convert for rldcl use
> + rldcl rldcl   extract diff/zero byte
> + subf   subtract for final result
> +  */

Mixed tabs and spaces.

The rest looks good.  Please fix those remaining whitespace errors, and
it is okay for trunk.  Thanks,


Segher

[PATCH] Externalize definition of create_tmp_reg_or_ssa_name

[Will Schmidt]

Hi,

For some future rs6000 vector folding patches, I will be needing
access to the create_tmp_reg_or_ssa_name() function in rs6000.c.
Thus...
Externalize the definition of create_tmp_reg_or_ssa_name
for use in rs6000.c.  The actual usage will show up in later patches.

I'll note that I do not have any 'usage' patches quite ready to go, so
I may just sit on committing this until one of those usage patches is ready.

I've bootstrapped and make check with this patch applied in conjunction with
other patches. 

OK for trunk?


[gcc]

2016-12-19  Will Schmidt  

*  gimple-fold.c (create_tmp_reg_or_ssa_name): remove static 
declaration.
*  gimple-fold.h: add prototype.

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index d00625b..ac4498f 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -161,8 +161,8 @@ can_refer_decl_in_current_unit_p (tree decl, tree from_decl)
is in SSA form, a SSA name is created.  Otherwise a temporary register
is made.  */
 
-static tree
-create_tmp_reg_or_ssa_name (tree type, gimple *stmt = NULL)
+tree
+create_tmp_reg_or_ssa_name (tree type, gimple *stmt)
 {
   if (gimple_in_ssa_p (cfun))
 return make_ssa_name (type, stmt);
diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h
index 5add30c..e7f8fe2 100644
--- a/gcc/gimple-fold.h
+++ b/gcc/gimple-fold.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #ifndef GCC_GIMPLE_FOLD_H
 #define GCC_GIMPLE_FOLD_H
 
+extern tree create_tmp_reg_or_ssa_name (tree, gimple *stmt = NULL);
 extern tree canonicalize_constructor_val (tree, tree);
 extern tree get_symbol_constant_value (tree);
 extern void get_range_strlen (tree, tree[2]);

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jeff Law]

On 12/19/2016 09:51 AM, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 09:34:44AM -0700, Martin Sebor wrote:

That would be just weird, have one behavior for selected subset of functions
and another for the rest?  Ugh.


The selected set of the string built-ins are special -- they are
known not to recover from null pointers so I think treating them
differently would be reasonable (and useful) irrespective of
the -Wnonnull warning.  We don't know what any arbitrary user-
defined nonnull function might do when it gets a null pointer so
skipping those may not make as much sense.


The problem is that then -fsanitize=undefined changes behavior of the
program, which wasn't part of the design.  It should either terminate the
program after reporting (and before it happens) the first fatal UB, or
just report UB before they happen and continue working as without the
instrumentation.  If the program segfaults without instrumentation, so be it
even with instrumentation.
Right.   I think as a fundamental design decision UB sanitization 
shouldn't change the behavior of the code.  Report and terminate at 
first UB just remote UBs.


Deviations from that design should be looked at as bugs.

jeff

Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers in GIMPLE.

[Tamar Christina]

Hi All,

I've respun the patch with the feedback from Jeff and Joseph.

> I think an integer mode should always exist - even in the case of TFmode
> on 32-bit systems (32-bit sparc / s390, for example, use TFmode long
> double for GNU/Linux, and it's supported as _Float128 and __float128 on
> 32-bit x86).  It just be not be usable for arithmetic or declaring
> variables of that type.

You're right, so I test the integer mode I receive with scalar_mode_supported_p.
And this seems to do the right thing.

Thanks for all the comments so far!

Kind Regards,
Tamar

From: Joseph Myers 
Sent: Thursday, December 15, 2016 7:03:27 PM
To: Tamar Christina
Cc: Jeff Law; GCC Patches; Wilco Dijkstra; rguent...@suse.de; Michael Meissner; 
nd
Subject: Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers 
in GIMPLE.

On Thu, 15 Dec 2016, Tamar Christina wrote:

> > Note that on some systems we even disable 64bit floating point support.
> > I suspect this check needs a little re-thinking as I don't think that
> > checking for a specific UNITS_PER_WORD is correct, nor is checking the
> > width of the type.  I'm not offhand sure what the test should be, just
> > that I think we need something better here.
>
> I think what I really wanted to test here is if there was an integer
> mode available which has the exact width as the floating point one. So I
> have replaced this with just a call to int_mode_for_mode. Which is
> probably more correct.

I think an integer mode should always exist - even in the case of TFmode
on 32-bit systems (32-bit sparc / s390, for example, use TFmode long
double for GNU/Linux, and it's supported as _Float128 and __float128 on
32-bit x86).  It just be not be usable for arithmetic or declaring
variables of that type.

I don't know whether TImode bitwise operations, such as generated by this
fpclassify work, will get properly lowered to operations on supported
narrower modes, but I hope so (clearly it's simpler if you can write
things straightforwardly and have them cover this case of TFmode on 32-bit
systems automatically through lowering elsewhere in the compiler, than if
covering that case would require additional code - the more cases you
cover, the more opportunity there is for glibc to use the built-in
functions even with -fsignaling-nans).

--
Joseph S. Myers
jos...@codesourcery.com
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 3ac2d44148440b124559ba7cd3de483b7a74b72d..d8ff9c70ae6b9e72e09b8cbd9a0bd41b6830b83e 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -160,7 +160,6 @@ static tree fold_builtin_0 (location_t, tree);
 static tree fold_builtin_1 (location_t, tree, tree);
 static tree fold_builtin_2 (location_t, tree, tree, tree);
 static tree fold_builtin_3 (location_t, tree, tree, tree, tree);
-static tree fold_builtin_varargs (location_t, tree, tree*, int);
 
 static tree fold_builtin_strpbrk (location_t, tree, tree, tree);
 static tree fold_builtin_strstr (location_t, tree, tree, tree);
@@ -2202,19 +2201,8 @@ interclass_mathfn_icode (tree arg, tree fndecl)
   switch (DECL_FUNCTION_CODE (fndecl))
 {
 CASE_FLT_FN (BUILT_IN_ILOGB):
-  errno_set = true; builtin_optab = ilogb_optab; break;
-CASE_FLT_FN (BUILT_IN_ISINF):
-  builtin_optab = isinf_optab; break;
-case BUILT_IN_ISNORMAL:
-case BUILT_IN_ISFINITE:
-CASE_FLT_FN (BUILT_IN_FINITE):
-case BUILT_IN_FINITED32:
-case BUILT_IN_FINITED64:
-case BUILT_IN_FINITED128:
-case BUILT_IN_ISINFD32:
-case BUILT_IN_ISINFD64:
-case BUILT_IN_ISINFD128:
-  /* These builtins have no optabs (yet).  */
+  errno_set = true;
+  builtin_optab = ilogb_optab;
   break;
 default:
   gcc_unreachable ();
@@ -2233,8 +2221,7 @@ interclass_mathfn_icode (tree arg, tree fndecl)
 }
 
 /* Expand a call to one of the builtin math functions that operate on
-   floating point argument and output an integer result (ilogb, isinf,
-   isnan, etc).
+   floating point argument and output an integer result (ilogb, etc).
Return 0 if a normal call should be emitted rather than expanding the
function in-line.  EXP is the expression that is a call to the builtin
function; if convenient, the result should be placed in TARGET.  */
@@ -5997,11 +5984,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
 CASE_FLT_FN (BUILT_IN_ILOGB):
   if (! flag_unsafe_math_optimizations)
 	break;
-  gcc_fallthrough ();
-CASE_FLT_FN (BUILT_IN_ISINF):
-CASE_FLT_FN (BUILT_IN_FINITE):
-case BUILT_IN_ISFINITE:
-case BUILT_IN_ISNORMAL:
+
   target = expand_builtin_interclass_mathfn (exp, target);
   if (target)
 	return target;
@@ -6281,8 +6264,25 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
 	}
   break;
 
+CASE_FLT_FN (BUILT_IN_ISINF):
+case BUILT_IN_ISNAND32:
+case BUILT_IN_ISNAND64:
+case BUILT_IN_ISNAND128:
+case 

Re: [PATCH, rs6000] Fold vector addition built-ins in GIMPLE

[Will Schmidt]

On Tue, 2016-12-06 at 09:59 +0100, Andreas Schwab wrote:
> On Dez 05 2016, Bill Schmidt  wrote:
> 
> > What's your target triple?
> 
> http://gcc.gnu.org/ml/gcc-testresults/2016-12/msg00471.html
> 
> Andreas.
> 

I *suspect* this is fixable with the addition of this dg- directive to
the fold-vec-add-7.c test.  
+/* { dg-require-effective-target int128 } */

Still working on a recreate locally to see if I can verify that.. 

Looks like Andreas' build is configured with " 
configure flags: --prefix=/usr --build=powerpc64-suse-linux
--enable-checking=release --enable-shared --with-system-zlib CFLAGS='-O2
-g' CXXFLAGS='-O2 -g' --with-cpu-64=power4 --enable-secureplt
--with-long-double-128  "



Thanks,
-Will

[PATCH, rs6000] Fold vector multiply built-ins in GIMPLE

[Will Schmidt]

Hi, 
  This patch implements folding of the vector Multiply built-ins.

As part of this patch, I have also marked variables in an existing
testcase (mult-even-odd-be-order.c) as volatile, to prevent their being
optimized out, which happens once this vector multiply folding was able
to occur. 

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?

Thanks,
-Will
  
[gcc]

2016-12-19  Will Schmidt  

*  config/rs6000/rs6000.c: Add handling for early expansion of
vector multiply builtins.

[gcc/testsuite]

2016-12-19  Will Schmidt  

*  testsuite/gcc.dg/vmx/mult-even-odd-be-order.c : Mark
variables as volatile.
*  testsuite/gcc.target/powerpc/fold-vec-mult-char.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-float.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-floatdouble.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-int.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-int128-p8.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-int128-p9.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-longlong.c : New.
*  testsuite/gcc.target/powerpc/fold-vec-mult-short.c : New.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 0ab8de3..0d777e8 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -16509,6 +16509,36 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
gsi_replace (gsi, g, true);
return true;
   }
+/* Even element flavors of vec_mul (signed). */
+case ALTIVEC_BUILTIN_VMULESB:
+case ALTIVEC_BUILTIN_VMULESH:
+/* Even element flavors of vec_mul (unsigned).  */
+case ALTIVEC_BUILTIN_VMULEUB:
+case ALTIVEC_BUILTIN_VMULEUH:
+  {
+   arg0 = gimple_call_arg (stmt, 0);
+   arg1 = gimple_call_arg (stmt, 1);
+   lhs = gimple_call_lhs (stmt);
+   gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_EVEN_EXPR, arg0, 
arg1);
+   gimple_set_location (g, gimple_location (stmt));
+   gsi_replace (gsi, g, true);
+   return true;
+  }
+/* Odd element flavors of vec_mul (signed).  */
+case ALTIVEC_BUILTIN_VMULOSB:
+case ALTIVEC_BUILTIN_VMULOSH:
+/* Odd element flavors of vec_mul (unsigned). */
+case ALTIVEC_BUILTIN_VMULOUB:
+case ALTIVEC_BUILTIN_VMULOUH:
+  {
+   arg0 = gimple_call_arg (stmt, 0);
+   arg1 = gimple_call_arg (stmt, 1);
+   lhs = gimple_call_lhs (stmt);
+   gimple *g = gimple_build_assign (lhs, VEC_WIDEN_MULT_ODD_EXPR, arg0, 
arg1);
+   gimple_set_location (g, gimple_location (stmt));
+   gsi_replace (gsi, g, true);
+   return true;
+  }
 
 default:
   break;
diff --git a/gcc/testsuite/gcc.dg/vmx/mult-even-odd-be-order.c 
b/gcc/testsuite/gcc.dg/vmx/mult-even-odd-be-order.c
index ff30474..6ba12d0 100644
--- a/gcc/testsuite/gcc.dg/vmx/mult-even-odd-be-order.c
+++ b/gcc/testsuite/gcc.dg/vmx/mult-even-odd-be-order.c
@@ -4,18 +4,18 @@
 
 static void test()
 {
-  vector unsigned char vuca = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-  vector unsigned char vucb = {2,3,2,3,2,3,2,3,2,3,2,3,2,3,2,3};
-  vector signed char vsca = {-8,-7,-6,-5,-4,-3,-2,-1,0,1,2,3,4,5,6,7};
-  vector signed char vscb = {2,-3,2,-3,2,-3,2,-3,2,-3,2,-3,2,-3,2,-3};
-  vector unsigned short vusa = {0,1,2,3,4,5,6,7};
-  vector unsigned short vusb = {2,3,2,3,2,3,2,3};
-  vector signed short vssa = {-4,-3,-2,-1,0,1,2,3};
-  vector signed short vssb = {2,-3,2,-3,2,-3,2,-3};
-  vector unsigned short vuse, vuso;
-  vector signed short vsse, vsso;
-  vector unsigned int vuie, vuio;
-  vector signed int vsie, vsio;
+  volatile vector unsigned char vuca = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+  volatile vector unsigned char vucb = {2,3,2,3,2,3,2,3,2,3,2,3,2,3,2,3};
+  volatile vector signed char vsca = {-8,-7,-6,-5,-4,-3,-2,-1,0,1,2,3,4,5,6,7};
+  volatile vector signed char vscb = {2,-3,2,-3,2,-3,2,-3,2,-3,2,-3,2,-3,2,-3};
+  volatile vector unsigned short vusa = {0,1,2,3,4,5,6,7};
+  volatile vector unsigned short vusb = {2,3,2,3,2,3,2,3};
+  volatile vector signed short vssa = {-4,-3,-2,-1,0,1,2,3};
+  volatile vector signed short vssb = {2,-3,2,-3,2,-3,2,-3};
+  volatile vector unsigned short vuse, vuso;
+  volatile vector signed short vsse, vsso;
+  volatile vector unsigned int vuie, vuio;
+  volatile vector signed int vsie, vsio;
 
   vuse = vec_mule (vuca, vucb);
   vuso = vec_mulo (vuca, vucb);
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-char.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-char.c
new file mode 100644
index 000..3f946e5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mult-char.c
@@ -0,0 +1,23 @@
+/* Verify that overloaded built-ins for vec_mul with char
+   inputs produce the right results.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target 

[PATCH, rs6000] Fold vector subtract built-ins in GIMPLE

[Will Schmidt]

Hi, 
  This patch implements folding of the vector subtract built-ins.  This
follows the form used by Bill in his previous "Fold vector addition
built-ins in GIMPLE" patch. :-)

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  Is this ok for trunk?

Thanks,
-Will


[gcc]

2016-12-19  Will Schmidt  

* config/rs6000/rs6000.c (rs6000_gimple_fold_builtin): Add handling for
early expansion of vector subtract builtins.

[gcc/testsuite]

2016-12-19  Will Schmidt  

  * gcc.target/powerpc/fold-vec-sub-char.c: New.
  * gcc.target/powerpc/fold-vec-sub-float.c: New.
  * gcc.target/powerpc/fold-vec-sub-floatdouble.c: New.
  * gcc.target/powerpc/fold-vec-sub-int.c: New.
  * gcc.target/powerpc/fold-vec-sub-int128.c: New.
  * gcc.target/powerpc/fold-vec-sub-longlong.c: New.
  * gcc.target/powerpc/fold-vec-sub-short.c: New.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index f0c1354..0ab8de3 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -16492,6 +16492,24 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi)
gsi_replace (gsi, g, true);
return true;
   }
+/* Flavors of vec_sub.  We deliberately don't expand
+   P8V_BUILTIN_VSUBUQM. */
+case ALTIVEC_BUILTIN_VSUBUBM:
+case ALTIVEC_BUILTIN_VSUBUHM:
+case ALTIVEC_BUILTIN_VSUBUWM:
+case P8V_BUILTIN_VSUBUDM:
+case ALTIVEC_BUILTIN_VSUBFP:
+case VSX_BUILTIN_XVSUBDP:
+  {
+   arg0 = gimple_call_arg (stmt, 0);
+   arg1 = gimple_call_arg (stmt, 1);
+   lhs = gimple_call_lhs (stmt);
+   gimple *g = gimple_build_assign (lhs, MINUS_EXPR, arg0, arg1);
+   gimple_set_location (g, gimple_location (stmt));
+   gsi_replace (gsi, g, true);
+   return true;
+  }
+
 default:
   break;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-char.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-char.c
new file mode 100644
index 000..5063bd8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-char.c
@@ -0,0 +1,46 @@
+/* Verify that overloaded built-ins for vec_sub with char
+   inputs produce the right results.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec" } */
+
+#include 
+
+vector signed char
+test1 (vector bool char x, vector signed char y)
+{
+  return vec_sub (x, y);
+}
+
+vector signed char
+test2 (vector signed char x, vector bool char y)
+{
+  return vec_sub (x, y);
+}
+
+vector signed char
+test3 (vector signed char x, vector signed char y)
+{
+  return vec_sub (x, y);
+}
+
+vector unsigned char
+test4 (vector bool char x, vector unsigned char y)
+{
+  return vec_sub (x, y);
+}
+
+vector unsigned char
+test5 (vector unsigned char x, vector bool char y)
+{
+  return vec_sub (x, y);
+}
+
+vector unsigned char
+test6 (vector unsigned char x, vector unsigned char y)
+{
+  return vec_sub (x, y);
+}
+
+/* { dg-final { scan-assembler-times "vsububm" 6 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-float.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-float.c
new file mode 100644
index 000..8a29def
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-float.c
@@ -0,0 +1,17 @@
+/* Verify that overloaded built-ins for vec_sub with float
+   inputs produce the right results.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -mno-vsx" } */
+
+#include 
+
+vector float
+test1 (vector float x, vector float y)
+{
+  return vec_sub (x, y);
+}
+
+/* { dg-final { scan-assembler-times "vsubfp" 1 } } */
+
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-floatdouble.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-floatdouble.c
new file mode 100644
index 000..c29acc9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-floatdouble.c
@@ -0,0 +1,23 @@
+/* Verify that overloaded built-ins for vec_sub with float and
+   double inputs for VSX produce the right results.  */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-maltivec -mvsx" } */
+
+#include 
+
+vector float
+test1 (vector float x, vector float y)
+{
+  return vec_sub (x, y);
+}
+
+vector double
+test2 (vector double x, vector double y)
+{
+  return vec_sub (x, y);
+}
+
+/* { dg-final { scan-assembler-times "xvsubsp" 1 } } */
+/* { dg-final { scan-assembler-times "xvsubdp" 1 } } */
diff --git a/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-int.c 
b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-int.c
new file mode 100644
index 000..1fac1dc
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-sub-int.c
@@ -0,0 +1,47 @@
+/* Verify that overloaded built-ins for vec_sub with int
+   inputs produce the right results.  */
+
+/* { dg-do compile } */
+/* { 

Re: [PATCH v2] Run tests only if the machine supports the instruction set.

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 05:50:40PM +0100, Dominik Vogt wrote:
>   * config/s390/s390-c.c (s390_cpu_cpp_builtins_internal): Define
>   __S390_ARCH_LEVEL__.
> gcc/testsuite/ChangeLog-setmem
> 
>   * gcc.target/s390/md/setmem_long-1.c: Use "runnable".
>   * gcc.target/s390/md/rXsbg_mode_sXl.c: Likewise.
>   * gcc.target/s390/md/andc-splitter-1.c: Likewise.
>   * gcc.target/s390/md/andc-splitter-2.c: Likewise.
>   * lib/gcc-dg.exp (gcc-dg-runtest): Export torture_current_flags.
>   * gcc.target/s390/s390.exp: Import torture_current_flags.
>   (check_effective_target_runnable): New.

Unless you want to add support for all targets in the runnable
effective target, I think it would be better to call it less generically,
s390_runnable or similar.

Jakub

Re: [PATCH] S/390: Run md tests with -march=native instead of -march=z13.

[Dominik Vogt]

On Tue, Dec 13, 2016 at 11:42:40AM +0100, Jakub Jelinek wrote:
> On Tue, Dec 13, 2016 at 11:18:31AM +0100, Dominik Vogt wrote:
> > > IMHO you want something like x86 avx_runtime effective target
> > > (z13_runtime?), which would stand for running on z13 capable hw and
> > > with z13 assembler support.
> > 
> > Something like that, yes, but it's not so easy because the kernel
> > has to support it too.  Some features are disabled in a VM
> > although the hardware supports them.  What we really need is
> 
> The z13_runtime or whatever effective target routine can always
> just try to compile/link a simple testcase with at least one z13 specific
> instruction and try to run it.
> 
> >   Run test if the test system (not just the hardware) supports the
> >   instruction set of the -march= option the test was compiled
> >   with, otherwise just compile it.
> > 
> > I.e. derive the "effective_targt..." option from the "-march=..."
> > option set by the torture test.
> > 
> > > Or choose what options to include based on such effective target tests,
> > > and perhaps also select a default action, like we do on some targets e.g. 
> > > in
> > > the vectorizer.
> > 
> > Can you give an example test file, please?
> 
> Look e.g. at gcc.dg/vect/vect.exp and testsuite/lib/*.exp for
> dg-do-what-default and the games done with it.  E.g. if you set
> (and saved/restored) in addition to dg-do-what-default also EFFECTIVE_TARGETS
> then perhaps you could use et-dg-runtest or similar.

Thanks.  New patch with different approach here:
https://gcc.gnu.org/ml/gcc-patches/2016-12/msg01621.html

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] Offer suggestions for misspelled attributes (PR c/70186)

[Jakub Jelinek]

Hi!

On Mon, Dec 19, 2016 at 11:51:29AM -0500, David Malcolm wrote:
> +/* Look for near matches for the scoped attribute with namespace NS and
> +   name NAME.
> +   Return the best matching attribute name, or NULL if none is found.
> +   If it returns non-NULL then *UNDERSCORES is written to, with true
> +   iff leading and trailing underscores were stripped from NAME
> +   before the match.  */
> +
> +static const char *
> +fuzzy_lookup_scoped_attribute_spec (const_tree ns, const_tree name,
> + bool *underscores)
> +{
> +  struct substring attr;
> +  scoped_attributes *attrs;
> +
> +  const char *ns_str = (ns != NULL_TREE) ? IDENTIFIER_POINTER (ns): NULL;
> +
> +  attrs = find_attribute_namespace (ns_str);
> +
> +  if (attrs == NULL)
> +return NULL;
> +
> +  attr.str = IDENTIFIER_POINTER (name);
> +  attr.length = IDENTIFIER_LENGTH (name);
> +  *underscores = extract_attribute_substring ();
> +
> +  best_match  bm ();
> +
> +  hash_table *h = attrs->attribute_hash;
> +  for (hash_table::iterator iter = h->begin ();
> +   iter != h->end (); ++iter)
> +bm.consider ((*iter)->name);
> +  return bm.get_best_meaningful_candidate ();

Does this consider the decl_req, type_req and fn_type_req flags in the
attribute tables (i.e. that for attribute on a VAR_DECL it doesn't suggest
attributes that apply to functions only, on FUNCTION_DECL suggest attributes
that apply only to non-function decls, etc.)?
Does it ignore internal only attributes (i.e. attributes containing spaces
or * characters in their names)?
Say
void foo (void) __attribute ((omp_declare_target));
should not suggest the omp declare target attribute, since users can't type
it in.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 09:34:44AM -0700, Martin Sebor wrote:
> > That would be just weird, have one behavior for selected subset of functions
> > and another for the rest?  Ugh.
> 
> The selected set of the string built-ins are special -- they are
> known not to recover from null pointers so I think treating them
> differently would be reasonable (and useful) irrespective of
> the -Wnonnull warning.  We don't know what any arbitrary user-
> defined nonnull function might do when it gets a null pointer so
> skipping those may not make as much sense.

The problem is that then -fsanitize=undefined changes behavior of the
program, which wasn't part of the design.  It should either terminate the
program after reporting (and before it happens) the first fatal UB, or
just report UB before they happen and continue working as without the
instrumentation.  If the program segfaults without instrumentation, so be it
even with instrumentation.

Jakub

Re: [PATCH v2] Run tests only if the machine supports the instruction set.

[Dominik Vogt]

On Mon, Dec 19, 2016 at 03:28:06PM +0100, Dominik Vogt wrote:
> The attached patch is specific to S/390 but contains a small
> common code change in gcc-dg.exp.  It fixes the notorious problem
> of md tests running on an S/390 machine that does not support the
> z13 instruction set.
> 
> Bootstrapped and tested on s390x biarch.

Version 2 with results of internal discussion:

 * Renamed __s390_arh_level__ to upper case.
 * Replaced __VECTOR__ with new macro __S390_VX__.
 * Added individual (but currently unused) effective-target
   functions for the various architectures.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog-archlevel

* config/s390/s390-c.c (s390_cpu_cpp_builtins_internal): Define
__S390_ARCH_LEVEL__.
gcc/testsuite/ChangeLog-setmem

* gcc.target/s390/md/setmem_long-1.c: Use "runnable".
* gcc.target/s390/md/rXsbg_mode_sXl.c: Likewise.
* gcc.target/s390/md/andc-splitter-1.c: Likewise.
* gcc.target/s390/md/andc-splitter-2.c: Likewise.
* lib/gcc-dg.exp (gcc-dg-runtest): Export torture_current_flags.
* gcc.target/s390/s390.exp: Import torture_current_flags.
(check_effective_target_runnable): New.
(check_effective_target_z900_runnable): New.
(check_effective_target_z990_runnable): New.
(check_effective_target_z9_ec_runnable): New.
(check_effective_target_z10_runnable): New.
(check_effective_target_z196_runnable): New.
(check_effective_target_zEC12_runnable): New.
(check_effective_target_z13_runnable): New.
(check_effective_target_z10_instructions): Removed.
(MD_TEST_OPTS): Add optimization level without -march=.
>From 7c74ce8212fa85272e1928b2d2df44b74cba5d0a Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Tue, 13 Dec 2016 10:21:08 +0100
Subject: [PATCH] S/390: Run md tests only if the machine supports the
 instruction set.

---
 gcc/config/s390/s390-c.c   |  17 +++
 gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c |  19 ++--
 gcc/testsuite/gcc.target/s390/md/andc-splitter-2.c |  19 ++--
 gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c  |   4 +-
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c   |   7 +-
 gcc/testsuite/gcc.target/s390/s390.exp | 126 ++---
 gcc/testsuite/lib/gcc-dg.exp   |   2 +
 7 files changed, 155 insertions(+), 39 deletions(-)

diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c
index fcf7477..e841365 100644
--- a/gcc/config/s390/s390-c.c
+++ b/gcc/config/s390/s390-c.c
@@ -320,6 +320,8 @@ s390_cpu_cpp_builtins_internal (cpp_reader *pfile,
 {
   s390_def_or_undef_macro (pfile, MASK_OPT_HTM, old_opts, opts,
   "__HTM__", "__HTM__");
+  s390_def_or_undef_macro (pfile, MASK_OPT_VX, old_opts, opts,
+  "__S390_VX__", "__S390_VX__");
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
   "__VEC__=10301", "__VEC__");
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
@@ -328,6 +330,21 @@ s390_cpu_cpp_builtins_internal (cpp_reader *pfile,
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
   "__bool=__attribute__((s390_vector_bool)) unsigned",
   "__bool");
+  {
+char macro_def[64];
+int arch_level;
+gcc_assert (s390_arch != PROCESSOR_NATIVE);
+arch_level = (int)s390_arch + 3;
+if (s390_arch >= PROCESSOR_2094_Z9_EC)
+  /* Z9_EC has the same level as Z9_109.  */
+  arch_level--;
+/* Review when a new arch is added and increase the value.  */
+char dummy[23 - 2 * PROCESSOR_max] __attribute__((unused));
+sprintf (macro_def, "__S390_ARCH_LEVEL__=%d", arch_level);
+cpp_undef (pfile, "__S390_ARCH_LEVEL__");
+cpp_define (pfile, macro_def);
+  }
+
   if (!flag_iso)
 {
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
diff --git a/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c 
b/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
index ed78921..9c41ac4 100644
--- a/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
+++ b/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
@@ -1,7 +1,8 @@
 /* Machine description pattern tests.  */
 
-/* { dg-do run { target { lp64 } } } */
+/* { dg-do compile { target { lp64 } } } */
 /* { dg-options "-mzarch -save-temps -dP" } */
+/* { dg-do run { target { lp64 && runnable } } } */
 /* Skip test if -O0 is present on the command line:
 
 { dg-skip-if "" { *-*-* } { "-O0" } { "" } }
@@ -13,26 +14,26 @@
 __attribute__ ((noinline))
 unsigned long andc_vv(unsigned long a, unsigned long b)
 { return ~b & a; }
-/* { dg-final { scan-assembler ":15 .\* \{\\*anddi3\}" } } */
-/* { dg-final { scan-assembler ":15 .\* \{\\*xordi3\}" } } */
+/* { dg-final { scan-assembler ":16 .\* \{\\*anddi3\}" } } */
+/* { dg-final { scan-assembler ":16 .\* \{\\*xordi3\}" } } */
 
 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 05:43:38PM +0100, Markus Trippelsdorf wrote:
> On 2016.12.19 at 09:34 -0700, Martin Sebor wrote:
> > On 12/19/2016 09:17 AM, Jakub Jelinek wrote:
> > > Or apply the patch I've posted which doesn't suffer from this problem,
> > > or revert the -Wnonnull changes and resolve somehow in GCC 8.
> > 
> > I would prefer your patch if it solves the problem.  In fact,
> > as I said before, I'm fine with your patch in any case. 
> 
> Then please lets go with Jakub's patch. And also please revert r243736
> as Richi suggested, because it isn't necessary anymore.

Well, somebody has to ack that, I can't review my own patches in this area.

Jakub

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Markus Trippelsdorf]

On 2016.12.19 at 09:34 -0700, Martin Sebor wrote:
> On 12/19/2016 09:17 AM, Jakub Jelinek wrote:
> > Or apply the patch I've posted which doesn't suffer from this problem,
> > or revert the -Wnonnull changes and resolve somehow in GCC 8.
> 
> I would prefer your patch if it solves the problem.  In fact,
> as I said before, I'm fine with your patch in any case. 

Then please lets go with Jakub's patch. And also please revert r243736
as Richi suggested, because it isn't necessary anymore.

-- 
Markus

Re: [PATCH] PR 78534 Change character length from int to size_t

[Bob Deen]

Hi all...

I never saw any followup on this...?

It's one thing to break the ABI between the compiler and the gfortran 
library; those can generally be expected to be in sync.  It's another to 
break the ABI between two *languages*, when there might be no such 
expectation (especially if gcc does NOT break their ABI at the same 
version number transition).  Yes, the pre-ISO_C_BINDING method may be 
old-fashioned, but it is a de-facto standard, and breaking it should not 
be done lightly.


If you do proceed with changing the size, I would request that there at 
least be a facility to reliably tell at compile time (on the C side) 
which definition is being used, so I can adjust our macros accordingly. 
Our code does depend on the size, and it has to cross-platform (and now, 
if this change is made, cross-version), so with this change I would have 
to support both int and size_t.


A C-side preprocessor symbol definition would do the trick.  Of course 
that assumes the versions of gcc/g++ and gfortran are in sync, which is 
never guaranteed.  But that assumption is better than nothing.  Unless 
someone has a better idea...?


Perhaps it might be best to wait until a time when gcc is also breaking 
their ABI, so that there's no question of code (on either side) working 
across the transition...?


Thanks...

-Bob

P.S.  I'm just a lurker here, but I lurk specifically to look for things 
that will break our code base, like this  ;-)


Bob.Deen @ NASA-JPL Multimission Image Processing Lab
bob.d...@jpl.nasa.gov


On 12/12/16 10:26 AM, Bob Deen wrote:



However, this will also affect people doing C->Fortran calls the
old-fashioned way without ISO_C_BINDING, as they will have to change
the string length argument from int to size_t in their prototypes.
Then again, Intel Fortran did this some years ago so I guess at least
people who care about portability to several compilers are aware.


We do a ton of this (old fashioned c-fortran binding) and changing the string 
length argument size will have a big impact on us.  We don't use the Intel 
compiler so we never noticed a change there.

Is there really a use case for strings > 2 GB that justifies the breakage?  I certainly 
understand wanting to do it "right" but I'm probably not the only one with 
practical considerations that argue against it if there are no compelling use cases.

Thanks...

-Bob

Bob Deen @ NASA-JPL Multimission Image Processing Lab
bob.d...@jpl.nasa.gov

[PATCH] Add x86_64-specific selftests for RTL function reader (v2)

[David Malcolm]

Note to i386 maintainters: this patch is part of the RTL frontend.
It adds selftests for verifying that the RTL dump reader works as
expected, with a mixture of real and hand-written "dumps" to
exercise various aspects of the loader.   Many RTL dumps contain
target-specific features (e.g. names of hard regs), and so these
selftests need to be target-specific, and hence this patch puts
them in i386.c.

Tested on i686-pc-linux-gnu and x86_64-pc-linux-gnu.

OK for trunk, assuming bootstrap?
(this is dependent on patch 8a within the kit).

Changed in v2:
- fixed selftest failures on i686:
* config/i386/i386.c
(selftest::ix86_test_loading_dump_fragment_1): Fix handling of
"frame" reg.
(selftest::ix86_test_loading_call_insn): Require TARGET_SSE.
- updated to use "<3>" syntax for pseudos, rather than "$3"

Blurb from v1:
This patch adds more selftests for class function_reader, where
the dumps to be read contain x86_64-specific features.

In an earlier version of the patch kit, these were handled using
preprocessor conditionals.
This version instead runs them via a target hook for running
target-specific selftests, thus putting them within i386.c.

gcc/ChangeLog:
* config/i386/i386.c
(selftest::ix86_test_loading_dump_fragment_1): New function.
(selftest::ix86_test_loading_call_insn): New function.
(selftest::ix86_test_loading_full_dump): New function.
(selftest::ix86_test_loading_unspec): New function.
(selftest::ix86_run_selftests): Call the new functions.

gcc/testsuite/ChangeLog:
* selftests/x86_64: New subdirectory.
* selftests/x86_64/call-insn.rtl: New file.
* selftests/x86_64/copy-hard-reg-into-frame.rtl: New file.
* selftests/x86_64/times-two.rtl: New file.
* selftests/x86_64/unspec.rtl: New file.

---
 gcc/config/i386/i386.c | 210 +
 gcc/testsuite/selftests/x86_64/call-insn.rtl   |  17 ++
 .../selftests/x86_64/copy-hard-reg-into-frame.rtl  |  15 ++
 gcc/testsuite/selftests/x86_64/times-two.rtl   |  51 +
 gcc/testsuite/selftests/x86_64/unspec.rtl  |  20 ++
 5 files changed, 313 insertions(+)
 create mode 100644 gcc/testsuite/selftests/x86_64/call-insn.rtl
 create mode 100644 gcc/testsuite/selftests/x86_64/copy-hard-reg-into-frame.rtl
 create mode 100644 gcc/testsuite/selftests/x86_64/times-two.rtl
 create mode 100644 gcc/testsuite/selftests/x86_64/unspec.rtl

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1cd1cd8..dc1a86f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -51200,6 +51200,209 @@ ix86_test_dumping_memory_blockage ()
"] UNSPEC_MEMORY_BLOCKAGE)))\n", pat, );
 }
 
+/* Verify loading an RTL dump; specifically a dump of copying
+   a param on x86_64 from a hard reg into the frame.
+   This test is target-specific since the dump contains target-specific
+   hard reg names.  */
+
+static void
+ix86_test_loading_dump_fragment_1 ()
+{
+  rtl_dump_test t (SELFTEST_LOCATION,
+  locate_file ("x86_64/copy-hard-reg-into-frame.rtl"));
+
+  rtx_insn *insn = get_insn_by_uid (1);
+
+  /* The block structure and indentation here is purely for
+ readability; it mirrors the structure of the rtx.  */
+  tree mem_expr;
+  {
+rtx pat = PATTERN (insn);
+ASSERT_EQ (SET, GET_CODE (pat));
+{
+  rtx dest = SET_DEST (pat);
+  ASSERT_EQ (MEM, GET_CODE (dest));
+  /* Verify the "/c" was parsed.  */
+  ASSERT_TRUE (RTX_FLAG (dest, call));
+  ASSERT_EQ (SImode, GET_MODE (dest));
+  {
+   rtx addr = XEXP (dest, 0);
+   ASSERT_EQ (PLUS, GET_CODE (addr));
+   ASSERT_EQ (DImode, GET_MODE (addr));
+   {
+ rtx lhs = XEXP (addr, 0);
+ /* Verify that the "frame" REG was consolidated.  */
+ ASSERT_RTX_PTR_EQ (frame_pointer_rtx, lhs);
+   }
+   {
+ rtx rhs = XEXP (addr, 1);
+ ASSERT_EQ (CONST_INT, GET_CODE (rhs));
+ ASSERT_EQ (-4, INTVAL (rhs));
+   }
+  }
+  /* Verify the "[1 i+0 S4 A32]" was parsed.  */
+  ASSERT_EQ (1, MEM_ALIAS_SET (dest));
+  /* "i" should have been handled by synthesizing a global int
+variable named "i".  */
+  mem_expr = MEM_EXPR (dest);
+  ASSERT_NE (mem_expr, NULL);
+  ASSERT_EQ (VAR_DECL, TREE_CODE (mem_expr));
+  ASSERT_EQ (integer_type_node, TREE_TYPE (mem_expr));
+  ASSERT_EQ (IDENTIFIER_NODE, TREE_CODE (DECL_NAME (mem_expr)));
+  ASSERT_STREQ ("i", IDENTIFIER_POINTER (DECL_NAME (mem_expr)));
+  /* "+0".  */
+  ASSERT_TRUE (MEM_OFFSET_KNOWN_P (dest));
+  ASSERT_EQ (0, MEM_OFFSET (dest));
+  /* "S4".  */
+  ASSERT_EQ (4, MEM_SIZE (dest));
+  /* "A32.  */
+  ASSERT_EQ (32, MEM_ALIGN (dest));
+}
+{
+  rtx src = SET_SRC (pat);
+  ASSERT_EQ (REG, GET_CODE (src));
+  ASSERT_EQ (SImode, GET_MODE (src));
+  ASSERT_EQ (5, REGNO (src));
+  

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Martin Sebor]

On 12/19/2016 09:17 AM, Jakub Jelinek wrote:

On Mon, Dec 19, 2016 at 08:52:44AM -0700, Martin Sebor wrote:

Another thing is that what the compiler does can very well just happen
in some generic function that is called by the function that calls these
strlen/strcpy etc. functions (fns with nonnull attribute).


For the string built-ins (though perhaps not for user-defined
nonnull functions), I wonder if it would make sense to have Ubsan
emit an if (p == null) ubsan_handle_nonnull; else strfunc(p); to


That would be just weird, have one behavior for selected subset of functions
and another for the rest?  Ugh.


The selected set of the string built-ins are special -- they are
known not to recover from null pointers so I think treating them
differently would be reasonable (and useful) irrespective of
the -Wnonnull warning.  We don't know what any arbitrary user-
defined nonnull function might do when it gets a null pointer so
skipping those may not make as much sense.


BTW, in the testcase from the Linux kernel it is also path isolation
for error recovery path, something that ought to be predicted unlikely
(though, probably not very unlikely unlike this case), so the question is
if we want to isolate that or not too.


I don't expect to have the cycles to do significant work on this
before stage 3 ends.  For GCC 7, to avoid the Ubsan warnings,
the late -Wnonnull warnings could simply be suppressed when
-fsanitize=undefined is used.  It wouldn't be ideal but it would
be no worse than what GCC does today.  In 8 the warning could be
made smarter to avoid the problem in general.


Or apply the patch I've posted which doesn't suffer from this problem,
or revert the -Wnonnull changes and resolve somehow in GCC 8.


I would prefer your patch if it solves the problem.  In fact,
as I said before, I'm fine with your patch in any case.  I also
still have the patch that I never posted that adds the two levels
to -Wnonnull (keeping -Wnonnull=1 as the default).  And I now have
another patch that simply suppresses the late -Wnonnull warning
when Ubsan checks for null pointers.  I could see about putting
together yet another one to implement the approach I suggested
above but I hesitate to put too much more time into it before
knowing which approach is preferable.

Martin

Re: [v3 PATCH] Make the perfect-forwarding constructor of a two-element tuple sfinae away when the first argument is an allocator_arg.

[Jonathan Wakely]

On 19/12/16 14:34 +0200, Ville Voutilainen wrote:

On 19 December 2016 at 12:19, Jonathan Wakely  wrote:

On 18/12/16 13:33 +0200, Ville Voutilainen wrote:


Andrzej Krzemienski pointed this out in a discussion related to any and
tags.
Our two-element tuple specialization doesn't make the perfect-forwarding
constructor and the allocator constructor properly mutually exclusive;
this
patch fixes that.

Tested on Linux-x64, ok for trunk, gcc-6 and gcc-5?



gcc-6-branch is frozen, so not there.


Not now, but when that branch reopens, presumably then?


Yes please, for the 6.4 release.


Should this be reported as a defect in the standard?


I don't think so, the standard doesn't specify a two-argument
specialization and the variadic
signature specified doesn't run into this problem. We can certainly
give the other vendors a heads-up
in case their implementations suffer from the same problem but the
standard itself is not defective.


Ah of course, the 2-argument specialization is only implied by the
"only if sizeof...(Types) == 2" comments but not actually specified.

Re: [PATCH] Remove unused libgfortran functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 6:15 PM, FX  wrote:
>> Thanks, committed as r243799.
>
> I think something went wrong in your commit, as none of the “removed” files 
> were removed: https://gcc.gnu.org/viewvc/gcc?view=revision=243799

Indeed, thanks for bringing it up. Fixed by r243804.


-- 
Janne Blomqvist

Re: [libgfortran, patch] Rename numeric STOP runtime functions

[FX]

> The patch you posted contains some apparently unrelated changes to
> gfortran.map. Without those, Ok.

Thanks for the reviews. Patches committed. Wiki updated. I’ll work tonight on 
some of the remaining items on the “abi cleanup” list.


> As a minor cosmetic improvement, you could fold_convert the argument
> to integer_type_node instead of gfc_int4_type_node and change the
> library functions to take plain C int arguments, since the exit()
> argument is a C int anyways.

Then this would need to be done with coarray functions too, so I would not feel 
too comfortable. I’ll leave that as an improvement if there is agreement on 
that side.

FX

[PATCH] Offer suggestions for misspelled attributes (PR c/70186)

[David Malcolm]

Our -Wattributes warnings can be rather cryptic.  The following patch
improves this warning:

../../src/pr70186.c:1:8: warning: 'visbility' attribute directive ignored 
[-Wattributes]
 struct S *foo __attribute__ ((visbility("hidden")));
^

by adding suggestions when unrecognized attributes are encountered,
turning it into this:

../../src/pr70186.c:1:8: warning: 'visbility' attribute directive ignored; did 
you mean 'visibility'? [-Wattributes]
 struct S *foo __attribute__ ((visbility("hidden")));
^

Successfully bootstrapped on x86_64-pc-linux-gnu.

Rather late for gcc 7, sorry.  Is this OK for next stage 1,
or OK for stage 3 now? (if this looks low-enough risk)

gcc/ChangeLog:
PR c/70186
* attribs.c: Include "spellcheck.h".
(extract_attribute_substring): Fix leading comment.  Return a bool
signifying if truncation for underscores occurred.
(struct edit_distance_traits): New class.
(fuzzy_lookup_scoped_attribute_spec): New function, based on
lookup_scoped_attribute_spec.
(decl_attributes): Move warning about ignored attributes to..
(warn_about_unidentified_attribute): ...this new function, and
potentially offer suggestions for misspelled attributes.

gcc/testsuite/ChangeLog:
PR c/70186
* c-c++-common/spellcheck-attributes.c: New test case.
* g++.dg/cpp0x/spellcheck-attributes.C: New test case.
---
 gcc/attribs.c  | 115 ++---
 gcc/testsuite/c-c++-common/spellcheck-attributes.c |   5 +
 gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C |  18 
 3 files changed, 126 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-attributes.c
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/spellcheck-attributes.C

diff --git a/gcc/attribs.c b/gcc/attribs.c
index e66349a..ff5ce0d 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stor-layout.h"
 #include "langhooks.h"
 #include "plugin.h"
+#include "spellcheck.h"
 
 /* Table of the tables of attributes (common, language, format, machine)
searched.  */
@@ -97,10 +98,11 @@ static const struct attribute_spec empty_attribute_table[] =
   { NULL, 0, 0, false, false, false, NULL, false }
 };
 
-/* Return base name of the attribute.  Ie '__attr__' is turned into 'attr'.
-   To avoid need for copying, we simply return length of the string.  */
+/* Extract base name of the attribute.  Ie '__attr__' is turned into 'attr'.
+   To avoid need for copying, we simply update length of the string.
+   Return true if the string was truncated, false otherwise.  */
 
-static void
+static bool
 extract_attribute_substring (struct substring *str)
 {
   if (str->length > 4 && str->str[0] == '_' && str->str[1] == '_'
@@ -108,7 +110,9 @@ extract_attribute_substring (struct substring *str)
 {
   str->length -= 4;
   str->str += 2;
+  return true;
 }
+  return false;
 }
 
 /* Insert an array of attributes ATTRIBUTES into a namespace.  This
@@ -343,6 +347,101 @@ get_attribute_namespace (const_tree attr)
   return get_identifier ("gnu");
 }
 
+/* Specialization of edit_distance_traits for struct substring.  */
+
+template <>
+struct edit_distance_traits
+{
+  static size_t get_length (struct substring *substr)
+  {
+gcc_assert (substr);
+return substr->length;
+  }
+
+  static const char *get_string (struct substring *substr)
+  {
+gcc_assert (substr);
+return substr->str;
+  }
+};
+
+/* Look for near matches for the scoped attribute with namespace NS and
+   name NAME.
+   Return the best matching attribute name, or NULL if none is found.
+   If it returns non-NULL then *UNDERSCORES is written to, with true
+   iff leading and trailing underscores were stripped from NAME
+   before the match.  */
+
+static const char *
+fuzzy_lookup_scoped_attribute_spec (const_tree ns, const_tree name,
+   bool *underscores)
+{
+  struct substring attr;
+  scoped_attributes *attrs;
+
+  const char *ns_str = (ns != NULL_TREE) ? IDENTIFIER_POINTER (ns): NULL;
+
+  attrs = find_attribute_namespace (ns_str);
+
+  if (attrs == NULL)
+return NULL;
+
+  attr.str = IDENTIFIER_POINTER (name);
+  attr.length = IDENTIFIER_LENGTH (name);
+  *underscores = extract_attribute_substring ();
+
+  best_match  bm ();
+
+  hash_table *h = attrs->attribute_hash;
+  for (hash_table::iterator iter = h->begin ();
+   iter != h->end (); ++iter)
+bm.consider ((*iter)->name);
+  return bm.get_best_meaningful_candidate ();
+}
+
+/* Warn about attribute A being unrecognized with name NAME (an identifier),
+   within namespace NS (an identifier, or NULL_TREE).
+   Issue a hint if it appears to be misspelled.  */
+
+static void
+warn_about_unidentified_attribute (tree a, tree ns, tree name)
+{
+  bool underscores;
+  const char *hint
+= fuzzy_lookup_scoped_attribute_spec 

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Jakub Jelinek]

On Mon, Dec 19, 2016 at 08:52:44AM -0700, Martin Sebor wrote:
> > Another thing is that what the compiler does can very well just happen
> > in some generic function that is called by the function that calls these
> > strlen/strcpy etc. functions (fns with nonnull attribute).
> 
> For the string built-ins (though perhaps not for user-defined
> nonnull functions), I wonder if it would make sense to have Ubsan
> emit an if (p == null) ubsan_handle_nonnull; else strfunc(p); to

That would be just weird, have one behavior for selected subset of functions
and another for the rest?  Ugh.

> > BTW, in the testcase from the Linux kernel it is also path isolation
> > for error recovery path, something that ought to be predicted unlikely
> > (though, probably not very unlikely unlike this case), so the question is
> > if we want to isolate that or not too.
> 
> I don't expect to have the cycles to do significant work on this
> before stage 3 ends.  For GCC 7, to avoid the Ubsan warnings,
> the late -Wnonnull warnings could simply be suppressed when
> -fsanitize=undefined is used.  It wouldn't be ideal but it would
> be no worse than what GCC does today.  In 8 the warning could be
> made smarter to avoid the problem in general.

Or apply the patch I've posted which doesn't suffer from this problem,
or revert the -Wnonnull changes and resolve somehow in GCC 8.

Jakub

Re: [PATCH] Remove unused libgfortran functions

[FX]

> Thanks, committed as r243799.

I think something went wrong in your commit, as none of the “removed” files 
were removed: https://gcc.gnu.org/viewvc/gcc?view=revision=243799

FX

Re: [PATCH] Remove unused libgfortran functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 12:59 PM, FX  wrote:
>> Yes, I agree (in general, though I was thinking of making the new one
>> "GFORTRAN_7" to match the release series).
>
> Given that there will not be a 1-to-1 mapping of release series with major 
> ABI versions (hopefully!), I don’t think this is a good idea. It will make 
> people confused.

I don't understand. Why would it imply a 1:1 mapping of release series
with major ABI versions?

Say we release GCC 7 providing libgfortran.so.4 (that is, major
version 4) and gfortran.map has the symbols under the GFORTRAN_7 node.
Later on we release GCC 8 and new library symbols there would have the
GFORTRAN_8 node while keeping the GFORTRAN_7 node for existing symbols
that are backwards compatible. Just like we currently have with
GFORTRAN_1.0, 1.1, etc. (remember that the nodes in .map files are
arbitrary identifiers, the number have no meaning per se).

Then if a user has some code compiled against GCC 8 and tries to run
the binary on a system providing only libgfortran from GCC 7, the user
will get an error message that symbol node GFORTRAN_8 is not found.
IMHO that error message is clearer than an error message saying that
GFORTRAN_2.0 not found (to which the user replies, but I have GFortran
8 which is bigger than 2.0, WTF?? And yes, I've seen exactly this
happen).

>> There's also other things,
>> like e.g. ISO_C_BINDING helper functions living under the
>> __iso_c_binding namespace, instead of under _gfortran like everything
>> else.
>
> Agreed.

Which seems to be a moot point, since you just removed all those
symbols entirely. :)

>> And while we're at it, should we place everything under
>> "__gfortran" or "_GFortran", that is, with two underscores or one
>> underscore followed by a capital letter which in the C world is
>> reserved for the implementation? Though it's not clear to me whether
>> libgfortran can claim to be part of "the implementation" vs. being
>> generic user code.
>
> Another issue is that we have some documented, user-callable functions that 
> currently live in the __gfortran_ “namespace”, e.g. the mixed-language 
> routines 
> (https://gcc.gnu.org/onlinedocs/gfortran/Non-Fortran-Main-Program.html). We 
> want to avoid changing those for no reason, and so for consistency I think we 
> should keep everything under __gfortran_

Currently we have _gfortran_, that is with a single underscore in the
beginning, so it's not in the "C/POSIX reserved for the implementation
namespace". But yes, I agree that at least those functions documented
under the non-Fortran main program section in the manual should be
kept as is.


-- 
Janne Blomqvist

Re: [libgfortran, patch] Remove runtime TRANSPOSE support functions

[FX]

> No, this was actually part of r243799 which I just committed (after you Ok'd 
> it. :) )

Oops. Sorry!

Re: [PATCH] builtin expansion of strncmp for rs6000

[Aaron Sawdey]

On Fri, 2016-12-16 at 17:14 -0600, Segher Boessenkool wrote:
> Please repost.  Thanks,

Hi Segher,
  Thanks for the review. Attached is an updated patch that should
address the issues you noted. 

Bootstrap/regtest in progress on ppc64le -mcpu=power8, ok for trunk if
results are clean?

Thanks,
   Aaron

-- 
Aaron Sawdey, Ph.D.  acsaw...@linux.vnet.ibm.com
050-2/C113  (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC ToolchainIndex: gcc/config/rs6000/rs6000-protos.h
===
--- gcc/config/rs6000/rs6000-protos.h	(revision 243799)
+++ gcc/config/rs6000/rs6000-protos.h	(working copy)
@@ -78,6 +78,7 @@
 extern int expand_block_clear (rtx[]);
 extern int expand_block_move (rtx[]);
 extern bool expand_block_compare (rtx[]);
+extern bool expand_strn_compare (rtx[]);
 extern const char * rs6000_output_load_multiple (rtx[]);
 extern bool rs6000_is_valid_mask (rtx, int *, int *, machine_mode);
 extern bool rs6000_is_valid_and_mask (rtx, machine_mode);
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c	(revision 243799)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -19382,7 +19382,388 @@
   return true;
 }
 
+/* Generate alignment check and branch code to set up for
+   strncmp when we don't have DI alignment.
+   STRNCMP_LABEL is the label to branch if there is a page crossing.
+   SRC is the string pointer to be examined.
+   BYTES is the max number of bytes to compare.	 */
+static void
+expand_strncmp_align_check (rtx strncmp_label, rtx src, HOST_WIDE_INT bytes)
+{
+  rtx lab_ref = gen_rtx_LABEL_REF (VOIDmode, strncmp_label);
+  rtx src_check = copy_addr_to_reg (XEXP (src, 0));
+  if (GET_MODE (src_check) == SImode)
+emit_insn (gen_andsi3 (src_check, src_check, GEN_INT (0xfff)));
+  else
+emit_insn (gen_anddi3 (src_check, src_check, GEN_INT (0xfff)));
+  rtx cond = gen_reg_rtx (CCmode);
+  emit_move_insn (cond, gen_rtx_COMPARE (CCmode, src_check,
+	 GEN_INT (4096 - bytes)));
 
+  rtx cmp_rtx = gen_rtx_LT (VOIDmode, cond, const0_rtx);
+
+  rtx ifelse = gen_rtx_IF_THEN_ELSE (VOIDmode, cmp_rtx,
+ pc_rtx, lab_ref);
+  rtx j = emit_jump_insn (gen_rtx_SET (pc_rtx, ifelse));
+  JUMP_LABEL (j) = strncmp_label;
+  LABEL_NUSES (strncmp_label) += 1;
+}
+
+/* Expand a string compare operation with length, and return
+   true if successful. Return false if we should let the
+   compiler generate normal code, probably a strncmp call.
+
+   OPERANDS[0] is the target (result).
+   OPERANDS[1] is the first source.
+   OPERANDS[2] is the second source.
+   OPERANDS[3] is the length.
+   OPERANDS[4] is the alignment in bytes.  */
+bool
+expand_strn_compare (rtx operands[])
+{
+  rtx target = operands[0];
+  rtx orig_src1 = operands[1];
+  rtx orig_src2 = operands[2];
+  rtx bytes_rtx = operands[3];
+  rtx align_rtx = operands[4];
+  HOST_WIDE_INT cmp_bytes = 0;
+  rtx src1 = orig_src1;
+  rtx src2 = orig_src2;
+
+  /* If this is not a fixed size compare, just call strncmp.  */
+  if (!CONST_INT_P (bytes_rtx))
+return false;
+
+  /* This must be a fixed size alignment.  */
+  if (!CONST_INT_P (align_rtx))
+return false;
+
+  int base_align = INTVAL (align_rtx);
+  int align1 = MEM_ALIGN (orig_src1) / BITS_PER_UNIT;
+  int align2 = MEM_ALIGN (orig_src2) / BITS_PER_UNIT;
+
+  /* SLOW_UNALIGNED_ACCESS -- don't do unaligned stuff.	 */
+  if (SLOW_UNALIGNED_ACCESS (word_mode, align1)
+  || SLOW_UNALIGNED_ACCESS (word_mode, align2))
+return false;
+
+  gcc_assert (GET_MODE (target) == SImode);
+
+  HOST_WIDE_INT bytes = INTVAL (bytes_rtx);
+
+  /* If we have an LE target without ldbrx and word_mode is DImode,
+ then we must avoid using word_mode.  */
+  int word_mode_ok = !(!BYTES_BIG_ENDIAN && !TARGET_LDBRX
+		   && word_mode == DImode);
+
+  int word_mode_size = GET_MODE_SIZE (word_mode);
+
+  int offset = 0;
+  machine_mode load_mode =
+select_block_compare_mode (offset, bytes, base_align, word_mode_ok);
+  int load_mode_size = GET_MODE_SIZE (load_mode);
+
+  /* We don't want to generate too much code.  Also if bytes is
+ 4096 or larger we always want the library strncmp anyway.  */
+  int groups = ROUND_UP (bytes, load_mode_size) / load_mode_size;
+  if (bytes >= 4096 || groups > rs6000_string_compare_inline_limit)
+return false;
+
+  rtx result_reg = gen_reg_rtx (word_mode);
+  rtx final_move_label = gen_label_rtx ();
+  rtx final_label = gen_label_rtx ();
+  rtx begin_compare_label = NULL;
+
+  if (base_align < 8)
+{
+  /* Generate code that checks distance to 4k boundary for this case.  */
+  begin_compare_label = gen_label_rtx ();
+  rtx strncmp_label = gen_label_rtx ();
+  rtx jmp;
+
+  /* Strncmp for power8 in glibc does this:
+	 rldicl	r8,r3,0,52
+	 cmpldi	cr7,r8,4096-16
+	 bgt	cr7,L(pagecross) */
+
+  if (align1 < 8)
+	expand_strncmp_align_check (strncmp_label, src1, bytes);
+  

Re: [PATCH] fix powerpc64le bootstrap failure caused by r243661 (PR 78817)

[Martin Sebor]

On 12/19/2016 02:42 AM, Jakub Jelinek wrote:

On Sat, Dec 17, 2016 at 02:55:15PM -0700, Martin Sebor wrote:

I agree that these warnings should probably not be issued, though
it's interesting to see where they come from.  The calls are in
the code emitted by GCC, are reachable, and end up taking place
with the right Ubsan runtime recovery options.  It turns out that
Ubsan transforms calls to nonnull functions into conditional
branches testing the argument for null, like so:

if (s == 0)
  __builtin___ubsan_handle_nonnull_arg();
n = strlen (s);

and GCC then transforms those into

if (s == 0)
  {
__builtin___ubsan_handle_nonnull_arg();
n = strlen (NULL);
  }

When the ubsan_handle_nonnull_arg function returns to the caller
the call to strlen(NULL) is made.

This doesn't happen when -fno-sanitize-recover=undefined is used
when compiling the file because then Ubsan inserts calls to
__builtin___ubsan_handle_nonnull_arg_abort instead which is declared
noreturn.

It would be easy to suppress these warnings when -fsantize=undefined
is used.  Distinguishing the Ubsan-inserted calls from those in the
source code would be more challenging.  Implementing the warning as
a pass that runs before the Ubsan pass gets around the problem.


The ubsan pass runs before IPA, so not sure how do you want to do that
(and it is needed to run it early).

One question is if we should perform path isolation in this case at all,
since the branches to __builtin___ubsan_handle_nonnull_arg are with
PROB_VERY_UNLIKELY, so the question is if we should be growing the
paths which is really cold.  It has some small benefit (removing one
cheap comparison from the hot path), but the grows cold block.

Another thing is that what the compiler does can very well just happen
in some generic function that is called by the function that calls these
strlen/strcpy etc. functions (fns with nonnull attribute).


For the string built-ins (though perhaps not for user-defined
nonnull functions), I wonder if it would make sense to have Ubsan
emit an if (p == null) ubsan_handle_nonnull; else strfunc(p); to
let the program continue instead of almost certainly crash right
after the handler returns.  That would solve the warning problem
and also achieve the goal of allowing the handler to return and
uncovering subsequent instances of undefined behavior.


BTW, in the testcase from the Linux kernel it is also path isolation
for error recovery path, something that ought to be predicted unlikely
(though, probably not very unlikely unlike this case), so the question is
if we want to isolate that or not too.


I don't expect to have the cycles to do significant work on this
before stage 3 ends.  For GCC 7, to avoid the Ubsan warnings,
the late -Wnonnull warnings could simply be suppressed when
-fsanitize=undefined is used.  It wouldn't be ideal but it would
be no worse than what GCC does today.  In 8 the warning could be
made smarter to avoid the problem in general.

Martin

Re: [libgfortran, patch] Remove runtime clz() and ctz() bit intrisic functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 4:48 PM, FX  wrote:
> We implement the LEADZ and TRAILZ intrinsics in terms of the built-in clz() 
> and ctz() functions. Since these are not available for 128-bit integer types, 
> we used to have support functions _gfortran_clz128() and _gfortran_ctz128() 
> in libgfortran. In 2010, I applied a patch to avoid this and emit the 
> necessary arithmetic directly from the front-end 
> (https://gcc.gnu.org/ml/fortran/2010-09/msg00181.html). The support functions 
> were kept in libgfortran for backward compatibility, but we can now remove 
> them as we break the ABI.
>
> Attached patch is bootstrapped and regtested on x86_64-apple-darwin13.6.0.
> OK to commit?

Hmm, I distinctly remember that this was part of my big-ish patch I
just committed, but on closer inspection, no, maybe I just had it
queued up in my own branch..

So yes, Ok!


-- 
Janne Blomqvist

Re: [libgfortran, patch] Remove iso_c_binding runtime functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 3:54 PM, FX  wrote:
> The ISO_C_BINDING procedures have been emitted directly by the front-end 
> since 2012 (see for example 
> https://gcc.gnu.org/ml/fortran/2012-06/msg00152.html and 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32600). Having now broken the 
> library ABI, we can remove them from the library, where they were kept only 
> for compatibility.
>
> Bootstrapped and regtested on x86_64-apple-darwin16.3.0
> OK to commit?
>
> FX
>

Ok, thanks!

-- 
Janne Blomqvist

[PATCH] [PR rtl-optimization/65618] Fix MIPS ADA bootstrap failure

[James Cowgill]

Hi,

This patch fixes PR 65618 where ADA cannot be bootstrapped natively on
mips due to a bootstrap comparison failure. The PR is currently in the
target component, but should be in the rtl-optimization component.

The underlying bug is in gcc/emit-rtl.c:try_split and is a result of
the fix for PR rtl-optimization/48826. In that PR, if a call_insn is
split into two instructions, the following NOTE_INSN_CALL_ARG_LOCATION
is moved so that it immediately follows the new call_insn. However,
after doing that the "after" variable was not updated and it could
still point to the old note instruction (the instruction after the
instruction to be split). The "after" variable is later used to obtain
the last instruction in the split and is then passed back to the
delayed branch scheduler influencing how delay slots are assigned. My
patch adjusts the code which handles the NOTE_INSN_CALL_ARG_LOCATION
note so that "after" is updated if necessary.

This bug causes the ADA bootstrap comparison failure in a-except.o
because the branch delay scheduling operates slightly differently for
that file if debug information is turned on.

Thanks,
James

gcc/Changelog:

2016-12-16  James Cowgill  

PR rtl-optimization/65618
* emit-rtl.c (try_split): Update "after" when moving a
NOTE_INSN_CALL_ARG_LOCATION.

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 7de17454037..6be124ac038 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3742,6 +3742,11 @@ try_split (rtx pat, rtx_insn *trial, int last)
   next = NEXT_INSN (next))
if (NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
  {
+   /* Advance after to the next instruction if it is about to
+  be removed.  */
+   if (after == next)
+ after = NEXT_INSN (after);
+
remove_insn (next);
add_insn_after (next, insn, NULL);
break;

Re: [libgfortran, patch] Rename numeric STOP runtime functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 4:13 PM, FX  wrote:
> When support for F2008 requirements on numeric STOP statements was 
> implemented, the old _gfortran_stop_numeric() runtime function was made 
> obsolete and a new _gfortran_stop_numeric_f08() function was created, which 
> is the only one used in the front-end nowadays. The old 
> _gfortran_stop_numeric() was kept in libgfortran for ABI compatibility.
>
> Now that we are breaking the ABI, the attached patch removes the older 
> _gfortran_stop_numeric() function, and renames the 
> _gfortran_stop_numeric_f08() function into _gfortran_stop_numeric(). That 
> way, it is in line with the names of all other PAUSE/STOP/ERROR STOP runtime 
> functions.
>
> Bootstrapped and regtested on x86_64-apple-darwin16.3.0.
> OK to commit?

The patch you posted contains some apparently unrelated changes to
gfortran.map. Without those, Ok.

As a minor cosmetic improvement, you could fold_convert the argument
to integer_type_node instead of gfc_int4_type_node and change the
library functions to take plain C int arguments, since the exit()
argument is a C int anyways.



-- 
Janne Blomqvist

Re: [libgfortran, patch] Remove runtime TRANSPOSE support functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 4:31 PM, FX  wrote:
> Since 2010, gfortran does not rely on library support functions to handle 
> TRANSPOSE, but instead emits code directly in the front-end 
> (https://gcc.gnu.org/ml/fortran/2010-09/msg00109.html). We have since kept 
> the various _gfortran_transpose_* functions in libgfortran for ABI 
> compatbility, but we can now remove them.
>
> Attached patch bootstrapped and regtested on x86_64-apple-darwin16.3.0.
> OK to commit?

No, this was actually part of r243799 which I just committed (after
you Ok'd it. :) )


-- 
Janne Blomqvist

Re: [PATCH] Remove unused libgfortran functions

[Janne Blomqvist]

On Mon, Dec 19, 2016 at 4:44 PM, FX  wrote:
>> Now that the libgfortran ABI major version has been bumped, we can
>> remove functions for which the frontend nowadays generates inline
>> code.
>>
>> This removes the malloc, free, exponent, fraction, nearest, rrspacing,
>> spacing, set_exponent and transpose intrinsics. Also the unused
>> store_exe_path function is removed.
>>
>> Regtested on x86_64-pc-linux-gnu, Ok for trunk?
>
> I’m okaying the patch as is. While we continue cleaning up the library (I 
> have submitted a number of patches now) we should discuss how we will arrange 
> the symbols in gfortran.map at the end.

Thanks, committed as r243799.


-- 
Janne Blomqvist

Re: [PATCH v3] add -fprolog-pad=N,M option

[Bernd Schmidt]

I'll consider myself agnostic as to whether this is a feature we want or 
need, so I'll just comment on some style questions. There's a fair 
amount of coding style violations, I'll point some of them out but 
please read the documents we have linked on this page:

  https://gcc.gnu.org/contribute.html

On 12/16/2016 03:14 PM, Torsten Duwe wrote:

Signed-off-by: Torsten Duwe 


This is meaningless for the GCC project. We require a copyright 
assignment; I assume SuSE has a blanket one that covers you. You should 
also write a ChangeLog entry.



diff --git a/gcc/attribs.c b/gcc/attribs.c
index e66349a..6ff81a8 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -365,6 +365,28 @@ decl_attributes (tree *node, tree attributes, int flags)
   if (!attributes_initialized)
 init_attributes ();

+  /* If we're building NOP pads because of a command line arg, note the size
+ for LTO builds, unless the attribute has already been overridden. */


Two spaces at the end of a sentence, including at the end of a comment.


+  if (TREE_CODE (*node) == FUNCTION_DECL &&
+  prolog_nop_pad_size > 0)


Operators go on the next line when wrapping.


+),


Don't put closing parens on their own line.


diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index db293fe..7f3e558 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -2322,6 +2322,34 @@ merge_decls (tree newdecl, tree olddecl, tree newtype, 
tree oldtype)
   TREE_ASM_WRITTEN (olddecl) = 0;
 }

+  /* Prolog pad size may be set wrongly by a forward declaration;
+ fix it up by pulling the final value in front.
+  */


The "*/" should go on the previous line.


+  for (it = _ATTRIBUTES (newdecl); *it; it = _CHAIN(*it))


Space before opening parentheses (many occurrences).


+void
+default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
+ bool record_p)


Every function needs a comment describing its purpose and that of its 
arguments. Look around for examples. There are cases in this patch of 
hook implementations which ignore all their args, there I believe we can 
relax this rule a little (but a comment saying which hook is implemented 
would still be good).




+extern void default_print_prolog_pad (FILE *, unsigned HOST_WIDE_INT , bool);


Careful with stray whitespace.

+  if (tree_fits_uhwi_p (prolog_pad_value1) )


Here too.

+   {
+ pad_size = tree_to_uhwi (prolog_pad_value1);
+   }


Lose { } braces around single statements. Several cases.

Am I missing it or is there no actual implementation of the target hook 
anywhere?



Bernd

Re: [PATCH][ARM] PR target/71436: Restrict *load_multiple pattern till after LRA

[Jakub Jelinek]

On Thu, Dec 15, 2016 at 10:00:14AM +, Richard Earnshaw (lists) wrote:
> sorry, pasted the wrong bit of code.
> 
> That should read when we generate:
> 
> (insn 55 19 67 3 (parallel [
> (set (reg:SI 0 r0)
> (mem/u/c:SI (reg/f:SI 147) [2 c+0 S4 A32]))
> (set (reg:SI 158)
> (mem/u/c:SI (plus:SI (reg/f:SI 147)
> (const_int 4 [0x4])) [2 c+4 S4 A32]))
> ]) "ldm.c":25 404 {*load_multiple}
>  (expr_list:REG_UNUSED (reg:SI 0 r0)
> (nil)))
> 
> ie when we put a pseudo into the register load list.

We put a pseudo there because the predicate on the insn allows it:
(define_special_predicate "load_multiple_operation"
  (match_code "parallel")
{
 return ldm_stm_operation_p (op, /*load=*/true, SImode,
 /*consecutive=*/false,
 /*return_pc=*/false);
})
and the consecutive = false argument says that (almost) no verification
is performed on the SET_DEST, just that it is a REG and doesn't have
REGNO smaller than the first reg.
That said, RA is still not able to cope with such instructions, because
only the first set is represented with constraints, so if such an insn
needs any kind of reloading, it just will not happen.
So I think the posted patch makes lots of sense, otherwise if you use
such a pattern before reload, you just have to hope no reloading will be
needed on it.

Jakub

[libgfortran, patch] Remove runtime clz() and ctz() bit intrisic functions

[FX]

We implement the LEADZ and TRAILZ intrinsics in terms of the built-in clz() and 
ctz() functions. Since these are not available for 128-bit integer types, we 
used to have support functions _gfortran_clz128() and _gfortran_ctz128() in 
libgfortran. In 2010, I applied a patch to avoid this and emit the necessary 
arithmetic directly from the front-end 
(https://gcc.gnu.org/ml/fortran/2010-09/msg00181.html). The support functions 
were kept in libgfortran for backward compatibility, but we can now remove them 
as we break the ABI.

Attached patch is bootstrapped and regtested on x86_64-apple-darwin13.6.0.
OK to commit?

FX



bitint.ChangeLog
Description: Binary data


bitint.diff
Description: Binary data

Re: [PATCH] Remove unused libgfortran functions

[FX]

> Now that the libgfortran ABI major version has been bumped, we can
> remove functions for which the frontend nowadays generates inline
> code.
> 
> This removes the malloc, free, exponent, fraction, nearest, rrspacing,
> spacing, set_exponent and transpose intrinsics. Also the unused
> store_exe_path function is removed.
> 
> Regtested on x86_64-pc-linux-gnu, Ok for trunk?

I’m okaying the patch as is. While we continue cleaning up the library (I have 
submitted a number of patches now) we should discuss how we will arrange the 
symbols in gfortran.map at the end.

FX

[libgfortran, patch] Remove runtime TRANSPOSE support functions

[FX]

Since 2010, gfortran does not rely on library support functions to handle 
TRANSPOSE, but instead emits code directly in the front-end 
(https://gcc.gnu.org/ml/fortran/2010-09/msg00109.html). We have since kept the 
various _gfortran_transpose_* functions in libgfortran for ABI compatbility, 
but we can now remove them.

Attached patch bootstrapped and regtested on x86_64-apple-darwin16.3.0.
OK to commit?

FX



transpose.ChangeLog
Description: Binary data


transpose.diff
Description: Binary data

Re: [PATCH 1/2] print-rtl.c: use '<' and '>' rather than % for pseudos in compact mode

[Bernd Schmidt]

On 12/16/2016 09:18 PM, David Malcolm wrote:


The following patch implements the change for print-rtl.c.

OK for trunk assuming it passes bootstrap?


Yes.


Bernd

Run tests only if the machine supports the instruction set.

[Dominik Vogt]

The attached patch is specific to S/390 but contains a small
common code change in gcc-dg.exp.  It fixes the notorious problem
of md tests running on an S/390 machine that does not support the
z13 instruction set.

Bootstrapped and tested on s390x biarch.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog-archlevel

* config/s390/s390-c.c (s390_cpu_cpp_builtins_internal): Define
__s390_arch_level__.
gcc/testsuite/ChangeLog-setmem

* gcc.target/s390/md/setmem_long-1.c: Use "runnable".
* gcc.target/s390/md/rXsbg_mode_sXl.c: Likewise.
* gcc.target/s390/md/andc-splitter-1.c: Likewise.
* gcc.target/s390/md/andc-splitter-2.c: Likewise.
* lib/gcc-dg.exp (gcc-dg-runtest): Export torture_current_flags.
* gcc.target/s390/s390.exp: Import torture_current_flags.
(check_effective_target_runnable): New.
(check_effective_target_z10_instructions): Removed.
(MD_TEST_OPTS): Add optimization level without -march=.
>From 21e99fb09c5e8350892d99e6c351515333594aae Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Tue, 13 Dec 2016 10:21:08 +0100
Subject: [PATCH] S/390: Run md tests only if the machine supports the
 instruction set.

---
 gcc/config/s390/s390-c.c   | 15 ++
 gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c | 19 +++
 gcc/testsuite/gcc.target/s390/md/andc-splitter-2.c | 19 +++
 gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c  |  4 +-
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c   |  7 +--
 gcc/testsuite/gcc.target/s390/s390.exp | 60 --
 gcc/testsuite/lib/gcc-dg.exp   |  2 +
 7 files changed, 87 insertions(+), 39 deletions(-)

diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c
index fcf7477..8a9ea79 100644
--- a/gcc/config/s390/s390-c.c
+++ b/gcc/config/s390/s390-c.c
@@ -328,6 +328,21 @@ s390_cpu_cpp_builtins_internal (cpp_reader *pfile,
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
   "__bool=__attribute__((s390_vector_bool)) unsigned",
   "__bool");
+  {
+char macro_def[64];
+int arch_level;
+gcc_assert (s390_arch != PROCESSOR_NATIVE);
+arch_level = (int)s390_arch + 3;
+if (s390_arch >= PROCESSOR_2094_Z9_EC)
+  /* Z9_EC has the same level as Z9_109.  */
+  arch_level--;
+/* Review when a new arch is added and increase the value.  */
+char dummy[23 - 2 * PROCESSOR_max] __attribute__((unused));
+sprintf (macro_def, "__s390_arch_level__=%d", arch_level);
+cpp_undef (pfile, "__s390_arch_level__");
+cpp_define (pfile, macro_def);
+  }
+
   if (!flag_iso)
 {
   s390_def_or_undef_macro (pfile, MASK_ZVECTOR, old_opts, opts,
diff --git a/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c 
b/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
index ed78921..9c41ac4 100644
--- a/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
+++ b/gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c
@@ -1,7 +1,8 @@
 /* Machine description pattern tests.  */
 
-/* { dg-do run { target { lp64 } } } */
+/* { dg-do compile { target { lp64 } } } */
 /* { dg-options "-mzarch -save-temps -dP" } */
+/* { dg-do run { target { lp64 && runnable } } } */
 /* Skip test if -O0 is present on the command line:
 
 { dg-skip-if "" { *-*-* } { "-O0" } { "" } }
@@ -13,26 +14,26 @@
 __attribute__ ((noinline))
 unsigned long andc_vv(unsigned long a, unsigned long b)
 { return ~b & a; }
-/* { dg-final { scan-assembler ":15 .\* \{\\*anddi3\}" } } */
-/* { dg-final { scan-assembler ":15 .\* \{\\*xordi3\}" } } */
+/* { dg-final { scan-assembler ":16 .\* \{\\*anddi3\}" } } */
+/* { dg-final { scan-assembler ":16 .\* \{\\*xordi3\}" } } */
 
 __attribute__ ((noinline))
 unsigned long andc_pv(unsigned long *a, unsigned long b)
 { return ~b & *a; }
-/* { dg-final { scan-assembler ":21 .\* \{\\*anddi3\}" } } */
-/* { dg-final { scan-assembler ":21 .\* \{\\*xordi3\}" } } */
+/* { dg-final { scan-assembler ":22 .\* \{\\*anddi3\}" } } */
+/* { dg-final { scan-assembler ":22 .\* \{\\*xordi3\}" } } */
 
 __attribute__ ((noinline))
 unsigned long andc_vp(unsigned long a, unsigned long *b)
 { return ~*b & a; }
-/* { dg-final { scan-assembler ":27 .\* \{\\*anddi3\}" } } */
-/* { dg-final { scan-assembler ":27 .\* \{\\*xordi3\}" } } */
+/* { dg-final { scan-assembler ":28 .\* \{\\*anddi3\}" } } */
+/* { dg-final { scan-assembler ":28 .\* \{\\*xordi3\}" } } */
 
 __attribute__ ((noinline))
 unsigned long andc_pp(unsigned long *a, unsigned long *b)
 { return ~*b & *a; }
-/* { dg-final { scan-assembler ":33 .\* \{\\*anddi3\}" } } */
-/* { dg-final { scan-assembler ":33 .\* \{\\*xordi3\}" } } */
+/* { dg-final { scan-assembler ":34 .\* \{\\*anddi3\}" } } */
+/* { dg-final { scan-assembler ":34 .\* \{\\*xordi3\}" } } */
 
 /* { dg-final { scan-assembler-times "\tngr\?k\?\t" 4 } } */
 /* { dg-final { scan-assembler-times 

[libgfortran, patch] Rename numeric STOP runtime functions

[FX]

When support for F2008 requirements on numeric STOP statements was implemented, 
the old _gfortran_stop_numeric() runtime function was made obsolete and a new 
_gfortran_stop_numeric_f08() function was created, which is the only one used 
in the front-end nowadays. The old _gfortran_stop_numeric() was kept in 
libgfortran for ABI compatibility.

Now that we are breaking the ABI, the attached patch removes the older 
_gfortran_stop_numeric() function, and renames the _gfortran_stop_numeric_f08() 
function into _gfortran_stop_numeric(). That way, it is in line with the names 
of all other PAUSE/STOP/ERROR STOP runtime functions.

Bootstrapped and regtested on x86_64-apple-darwin16.3.0.
OK to commit?

FX



stop.ChangeLog
Description: Binary data


stop.diff
Description: Binary data

[v3 PATCH] Implement LWG 2842, in_place_t check for optional::optional(U&&) should decay U.

[Ville Voutilainen]

Tested on Linux-x64.

The perfect forwarder needs to sfinae out of the way of the in_place_t
signature,
and the in_place_t signature needs to check is_constructible. I also
did some housekeeping
to get rid of the int-pack constraints, because in case of empty packs
it's unclear
what a compiler is supposed to do for them.

2016-12-19  Ville Voutilainen  

Implement LWG 2842, in_place_t check for optional::optional(U&&)
should decay U.
* include/std/optional (_Optional_base(in_place_t, _Args&&...)):
Constrain.
(_Optional_base(in_place_t, initializer_list<_Up>, _Args&&...)):
Turn the int-pack constraint hack into a saner bool.
(_Optional_base<_Tp, false>::_Optional_base(in_place_t, _Args&&...)):
Constrain.
(_Optional_base<_Tp, false>::_Optional_base(in_place_t,
initializer_list<_Up>, _Args&&...)):
Turn the int-pack constraint hack into a saner bool.
(optional(_Up&&)): Constrain against in_place_t.
(optional(in_place_t, _Args&&...)): Constrain.
(constexpr optional(in_place_t, initializer_list<_Up>, _Args&&...)):
Turn the int-pack constraint hack into a saner bool.
* testsuite/20_util/optional/cons/value_neg.cc: Add a test for
a type that is constructible from in_place.
diff --git a/libstdc++-v3/include/std/optional 
b/libstdc++-v3/include/std/optional
index 3d69e10..73bc2b4 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -128,15 +128,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   : _Optional_base{} { }
 
   // Constructors for engaged optionals.
-  template
+  template, bool> = false>
 constexpr explicit _Optional_base(in_place_t, _Args&&... __args)
 : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { }
 
   template&,
-_Args&&...>::value,
-   int>...>
+   enable_if_t&,
+ _Args&&...>, bool> = false>
 constexpr explicit _Optional_base(in_place_t,
   initializer_list<_Up> __il,
   _Args&&... __args)
@@ -264,15 +264,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   constexpr _Optional_base(nullopt_t) noexcept
   : _Optional_base{} { }
 
-  template
+  template, bool> = false>
 constexpr explicit _Optional_base(in_place_t, _Args&&... __args)
 : _M_payload(std::forward<_Args>(__args)...), _M_engaged(true) { }
 
   template&,
-_Args&&...>::value,
-  int>...>
+   enable_if_t&,
+ _Args&&...>, bool> = false>
 constexpr explicit _Optional_base(in_place_t,
   initializer_list<_Up> __il,
   _Args&&... __args)
@@ -432,6 +432,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template , decay_t<_Up>>>,
+ __not_>>,
  is_constructible<_Tp, _Up&&>,
  is_convertible<_Up&&, _Tp>
  >::value, bool> = true>
@@ -441,6 +442,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template , decay_t<_Up>>>,
+ __not_>>,
  is_constructible<_Tp, _Up&&>,
  __not_>
  >::value, bool> = false>
@@ -499,15 +501,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
  emplace(std::move(*__t));
   }
 
-  template
+  template, bool> = false>
   explicit constexpr optional(in_place_t, _Args&&... __args)
 : _Base(std::in_place, std::forward<_Args>(__args)...) { }
 
   template&,
-_Args&&...>::value,
-   int>...>
+   enable_if_t&,
+ _Args&&...>, bool> = false>
   explicit constexpr optional(in_place_t,
  initializer_list<_Up> __il,
  _Args&&... __args)
diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc 
b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc
index 21e86c5..6a2827e 100644
--- a/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/cons/value_neg.cc
@@ -35,5 +35,10 @@ int main()
 std::optional ox2 = 42; // { dg-error "conversion" }
 

[Ping^2][1/9][RFC][DWARF] Reserve three DW_OP numbers in vendor extension space

[Jiong Wang]

Jiong Wang writes:

> Jiong Wang writes:
>
>> On 16/11/16 14:02, Jakub Jelinek wrote:
>>> On Wed, Nov 16, 2016 at 02:54:56PM +0100, Mark Wielaard wrote:
 On Wed, 2016-11-16 at 10:00 +, Jiong Wang wrote:
>   The two operations DW_OP_AARCH64_paciasp and 
> DW_OP_AARCH64_paciasp_deref were
> designed as shortcut operations when LR is signed with A key and using
> function's CFA as salt.  This is the default behaviour of return address
> signing so is expected to be used for most of the time.  
> DW_OP_AARCH64_pauth
> is designed as a generic operation that allow describing pointer signing 
> on
> any value using any salt and key in case we can't use the shortcut 
> operations
> we can use this.

 I admit to not fully understand the salting/keying involved. But given
 that the DW_OP space is really tiny, so we would like to not eat up too
 many of them for new opcodes. And given that introducing any new DW_OPs
 using for CFI unwinding will break any unwinder anyway causing us to
 update them all for this new feature. Have you thought about using a new
 CIE augmentation string character for describing that the return
 address/link register used by a function/frame is salted/keyed?

 This seems a good description of CIE records and augmentation
 characters:http://www.airs.com/blog/archives/460

 It obviously also involves updating all unwinders to understand the new
 augmentation character (and possible arguments). But it might be more
 generic and saves us from using up too many DW_OPs.
>>>
>>> From what I understood, the return address is not always scrambled, so
>>> it doesn't apply to the whole function, just to most of it (except for
>>> an insn in the prologue and some in the epilogue).  So I think one op is
>>> needed.  But can't it be just a toggable flag whether the return address
>>> is scrambled + some arguments to it?
>>> Thus DW_OP_AARCH64_scramble .uleb128 0 would mean that the default
>>> way of scrambling starts here (if not already active) or any kind of
>>> scrambling ends here (if already active), and
>>> DW_OP_AARCH64_scramble .uleb128 non-zero would be whatever encoding you need
>>> to represent details of the less common variants with details what to do.
>>> Then you'd just hook through some MD_* macro in the unwinder the
>>> descrambling operation if the scrambling is active at the insns you unwind
>>> on.
>>>
>>>   Jakub
>>
>> Hi Mark, Jakub:
>>
>>Thanks very much for the suggestions.
>>
>>I have done some experiments on your ideas and am thinking it's good to
>>combine them together.  The use of DW_CFA instead of DW_OP can avoid 
>> building
>>all information from scratch at each unwind location, while we can 
>> indicate
>>the signing key index through new AArch64 CIE augmentation 'B'. This new
>>approach reduce the unwind table size overhead from ~25% to ~5% when 
>> return
>>address signing enabled, it also largely simplified dwarf generation code 
>> for
>>return address signing.
>>
>>As one new DWARF call frame instruction is needed for AArch64, I want to 
>> reuse
>>DW_CFA_GNU_window_save to save the space.  It is in vendor extension 
>> space and
>>used for Sparc only, I think it make sense to reuse it for AArch64. On
>>AArch64, DW_CFA_GNU_window_save toggle return address sign status which 
>> kept
>>in a new boolean type column in DWARF table,  so DW_CFA_GNU_window_save 
>> takes
>>no argument on AArch64, the same as on Sparc, this makes no difference to 
>> those
>>existed encoding, length calculation code.
>>
>>Meanwhile one new DWARF expression operation number is still needed for
>>AArch64, it's useful for describing those complex pointer signing 
>> scenarios
>>and it will be used to multiplex some further extensions on AArch64.
>>
>>OK on this proposal and to install this patch to gcc trunk?
>>
>> Hi GDB, Binutils maintainer:
>>
>>OK on this proposal and install this patch to binutils-gdb master?
>>
>> include/
>> 2016-11-29   Richard Earnshaw  
>>   Jiong Wang  
>>
>>  * dwarf2.def (DW_OP_AARCH64_operation): Reserve the number 0xea.
>
> Ping~

Ping^2

-- 
Regards,
Jiong

[libgfortran, patch] Remove iso_c_binding runtime functions

[FX]

The ISO_C_BINDING procedures have been emitted directly by the front-end since 
2012 (see for example https://gcc.gnu.org/ml/fortran/2012-06/msg00152.html and 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32600). Having now broken the 
library ABI, we can remove them from the library, where they were kept only for 
compatibility.

Bootstrapped and regtested on x86_64-apple-darwin16.3.0
OK to commit?

FX



iso_c_binding.ChangeLog
Description: Binary data


iso_c_binding.diff
Description: Binary data

[committed, libgfortran]

[FX]

In the case where CHMOD is called with a numeric mode, the current code assumes 
that “mode_t” corresponds to an unsigned int. This is true on linux/glibc, but 
not true on macOS (where mode_t is an unsigned short) and thus creates a 
warning and possibly a runtime error. This had been spotted earlier on Windows 
and was corrected, but behind a __MINWG32__ check, although the fixed code is 
actually correct for all platforms.

I have thus committed the attached fix (revision 243796), removing the 
target-specific branch. Will work on all platforms now.

FX



chmod.ChangeLog
Description: Binary data


chmod.diff
Description: Binary data

[PATCH] c++/78771 ICE with inheriting ctor

[Nathan Sidwell]

Jason,
this patch fixes 78771, were an assert fires due to recursive 
instantiation of an inheriting ctor.  Normally when a recursive 
instantiation is needed, we've already constructed and registered the 
declaration, so simply return it.  For ctors though we need to construct 
the clones after we've instantiated the the master pattern (later in 
instantiate_template_1).  Hence any recursive instantiation of a cloned 
fn will barf, as we do.


Now, with an inherited ctor we have to deduce its exception spec and 
deletedness (deduce_inheriting_ctor).  That's fine, until one gets the 
perverse testcase here.  In figuring out what Middle ctor is needed by 
Middle(0), we end up trying to instantiate Derived::Derived (int) to see 
if Middle::Middle (Derived) is a viable candidate.  And that's the 
recursion, as Derived::Derived inherits from Middle::Middle.


Fixed by checking if the cloned instantiations actually exist before 
looking for them.  I think the only case this can occur is when SPEC is 
an inherited ctor, hence the assert.  Also, I don't think this can get 
us the wrong exept spec and deletedness -- it will be other (member) 
ctors that could change it, and we reconstruct the clones later anyway 
in the usual path.


(I tried creating the clones earlier, immediately after construction and 
registering the main function, but that didn't work)


ok?

nathan
--
Nathan Sidwell
2016-12-19  Nathan Sidwell  

	PR c++/78771
	* pt.c (instantiate_template_1): Check for recursive instantiation
	of inheriting constructor.

	PR c++/78771
	* g++.dg/cpp0x/pr78771.C: New.

Index: cp/pt.c
===
--- cp/pt.c	(revision 243746)
+++ cp/pt.c	(working copy)
@@ -17717,10 +17717,22 @@ instantiate_template_1 (tree tmpl, tree
   if (spec == error_mark_node)
 	return error_mark_node;
 
+  /* If this is an inherited ctor, we can recursively clone it
+	 when deducing the validity of the ctor.  But we won't have
+	 cloned the function yet, so do it now.  We'll redo this
+	 later, but any recursive information learnt here can't
+	 change the validity.  */
+  if (!TREE_CHAIN (spec))
+	{
+	  gcc_assert (DECL_INHERITED_CTOR (spec));
+	  clone_function_decl (spec, /*update_method_vec_p=*/0);
+	}
+
   /* Look for the clone.  */
   FOR_EACH_CLONE (clone, spec)
 	if (DECL_NAME (clone) == DECL_NAME (tmpl))
 	  return clone;
+
   /* We should always have found the clone by now.  */
   gcc_unreachable ();
   return NULL_TREE;
Index: testsuite/g++.dg/cpp0x/pr78771.C
===
--- testsuite/g++.dg/cpp0x/pr78771.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr78771.C	(working copy)
@@ -0,0 +1,27 @@
+// PR c++/78771
+// { dg-do compile { target c++11 } }
+
+// ICE instantiating a deleted inherited ctor
+
+struct Base
+{
+  template  Base (U);
+
+  Base (int);
+};
+
+struct Derived;
+
+struct Middle : Base
+{
+  using Base::Base;
+
+  Middle (Derived);
+};
+
+struct Derived : Middle
+{
+  using Middle::Middle;
+};
+
+Middle::Middle (Derived) : Middle (0) {}

Re: [PATCH][ARM] Remove movdi_vfp_cortexa8

[Wilco Dijkstra]

Ramana Radhakrishnan wrote:
> On Wed, Dec 14, 2016 at 5:43 PM, Wilco Dijkstra  
> wrote:

> > Yes, the reason to split the pattern was to introduce the '!' to discourage 
> > Neon->int moves on Cortex-A8 (https://patches.linaro.org/patch/541/). I am 
> > not removing the optimization for Cortex-A8, however  I  haven't been able 
> > to find an example where it makes a difference, even on high register 
> > pressure code.
>
>
> even on crafty  with -mtune=cortex-a8 ?

Indeed the '!' makes no difference on crafty either with -mcpu=cortex-a8 
-mfpu=neon.
Even if it did make a difference in the past, it is either totally ignored by 
the register
allocator or has no useful effect on deciding whether to use a Neon or integer 
register.

Wilco

RE: [PATCH,gcc/MIPS] Make loongson3a use fused madd.d

[Matthew Fortune]

Hi Paul,

Apologies for the delay in responding.

> I get the copyright assignment, it's ok for commit.

Thanks for going through copyright assignment, I can see you listed and
also you have commit access now. Is the trunk build failure still
present for you, if it is now resolved then please go ahead and commit.

Thanks,
Matthew