Re: [PATCH 2/6] detect unterminated const arrays in strlen calls (PR 86552)

2018-09-13 Thread Jeff Law 
On 9/9/18 3:57 AM, Bernd Edlinger wrote:
> On 09/09/18 01:47, Jeff Law wrote:
>> On 9/8/18 3:47 PM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>>
 -fold_builtin_strlen (location_t loc, tree type, tree arg)
 +fold_builtin_strlen (location_t loc, tree fndecl, tree type, tree arg)
   {
 if (!validate_arg (arg, POINTER_TYPE))
   return NULL_TREE;
 else
   {
 -  tree len = c_strlen (arg, 0);
 -
 +  tree nonstr = NULL_TREE;
 +  tree len = c_strlen (arg, 0, );
 if (len)
 -   return fold_convert_loc (loc, type, len);
 +   {
 + if (loc == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (arg))
 +   loc = EXPR_LOCATION (arg);
 +
 + /* To avoid warning multiple times about unterminated
 +arrays only warn if its length has been determined
 +and is being folded to a constant.  */
 + if (nonstr)
 +   warn_string_no_nul (loc, NULL_TREE, fndecl, nonstr);
 +
 + return fold_convert_loc (loc, type, len);
 +   }

 return NULL_TREE;
>>>
>>> If I see that right, this will do a wrong folding,
>>> just to suppress duplicate warnings.
>>>
>>> But it will re-introduce a path to PR87053, since c_strlen
>>> is supposed to return the wrong value because nonstr
>>> suggests the caller is able to handle this.
>>>
>>> I think c_strlen should never return anything that is invalid.
>>> Returning len and nonstr should be mutually exclusive events.
>> Conceptually I agree and have already been looking at this.  But in
>> practice I'm still on the fence.
>>
>> I'm fairly unhappy with the APIs we're using here and the inconsistency
>> in what nonstr means in terms of the return value from various
>> functions.   We see this when we start layering the warnings on top of
>> the trunk (which has the 87053 fix).  In some cases we want nonstr to
>> force the function to return an "I don't know value, typically
>> NULL_TREE", but in other cases we really want the function to still
>> return the length and let the caller decide what to do about the
>> termination problem.
>>
>> It doesn't help that we also have memsize as well.
>>
>> Rationalizing the APIs here is likely going to be a prereq to move forward.
>>
> 
> Yes, I agree.  The API is trying to solve everything at once, that is not 
> good.
> 
> I have given Martin's part 2/6 patch a little massage tonight,
> and reached this nice result:
> 
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 91)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 95)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 98)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 100)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 107)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 109)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 110)
> XPASS: gcc.dg/warn-strlen-no-nul.c pr? (test for warnings, line 118)
> XPASS: gcc.dg/warn-strlen-no-nul.c bug (test for warnings, line 122)
> XPASS: gcc.dg/warn-strlen-no-nul.c bug (test for warnings, line 123)
> 
> I left the test case as-is, just removed the ugly
> "Prune out warnings with no location (pr?)."
> at the end of the test.
> 
> How do you like that?
It's pretty good.  I updated the testcase constructed a ChangeLog from
this, the bits of the 1/6 patch and your changes.  Bootstrapped and
regression tested on x86_64.

It subsumes the remainder of the 1/6 patch from Martin as well as the
2/6 patch in its entirety.

I'll be committing it to the trunk momentarily.  I'm going to stop here
for the night and let the various automatic testers to their thing.

Jeff

Re: [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic

2018-09-13 Thread Jeff Law 
On 9/13/18 7:30 AM, Bernd Edlinger wrote:
> On 08/31/18 19:45, Bernd Edlinger wrote:
>> On 08/31/18 16:42, Jeff Law wrote:
>>> On 08/31/2018 01:08 AM, Bernd Edlinger wrote:
 Hi,


 when re-testing the new STRING_CST semantic patch here:
 https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html

 I noticed that the (my) fix for PR 87053 does still use
 semantic properties of the STRING_CST that is not compatible
 with the new proposed STRING_CST semantics.

 That means that c_strlen needs to handle the case
 where strings are not zero terminated.

 When this is fixed, fortran.dg/pr45636.f90 starts to regress,
 because the check in gimple_fold_builtin_memory_op here:

     if (tree_fits_uhwi_p (len)
     && compare_tree_int (len, MOVE_MAX) <= 0
     /* ???  Don't transform copies from strings with known length 
 this
    confuses the tree-ssa-strlen.c.  This doesn't handle
    the case in gcc.dg/strlenopt-8.c which is XFAILed for that
    reason.  */
     && !c_strlen (src, 2))
 does now return NULL_TREE, because the fortran string is not null 
 terminated.
 However that allows the folding which makes the fortran test case fail.

 I fixed that by pulling in the c_getstr patch again, and use
 it to make another exception for string constants without any embedded NUL.
 That is what tree-ssa-forwprop.c can handle, and should make this work in
 fortran again.


 Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
 Is it OK for trunk?
>>> I've gone the rounds on pr45636 a couple times and it's part of the
>>> reason why there's a FIXME in the bits I committed earlier this week.
>>>
>> Yes, the rationale here is that tree-ssa-forwprop will likely choke if there
>> is a NUL byte in the string:
>>
>>    /* Neither builtin_strncpy_read_str nor builtin_memcpy_read_str
>>   handle embedded '\0's.  */
>>    if (strlen (src_buf) != src_len)
>>      break;
>>
>> Prior to this the c_strlen (src, 2) returned 2, thus this folding was not 
>> done,
>> but since the string does not contain any NUL bytes this returns now NULL.
>> However it is easy to add an exception that triggers only in a fortran string
>> in this way.
>>
>>> I'll look at this closely in conjunction with the (unposted) patch which
>>> addresses the FIXME.
>>>
>>> Jeff
>>>
> Hi,
> 
> this is a minor update to the previous patch version, in that it adds
> the following to c_getstr, in order to be bootstrapped with or without
> the other STRING_CST-v2 semantic patches:
> 
> @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I
> unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src);
> unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size);
>   
> +  /* Ideally this would turn into a gcc_checking_assert over time.  */
> +  if (string_length > string_size)
> +string_length = string_size;
> +
> const char *string = TREE_STRING_POINTER (src);
>   
> if (string_length == 0
> 
> 
> So this patch can be boot-strapped alone,
> or together with the following patches:
> 
> [PATCHv2] Handle overlength strings in the C FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html
> 
> [PATCHv2] Handle overlength strings in C++ FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
> => Apporoved, without the part in vtable-class-hierarchy.c (!)
> 
> [PATCHv2] Handle overlength string literals in the fortan FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
> => Approved.
> 
> [PATCH] Check the STRING_CSTs in varasm.c
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
> 
> 
> 
> 
> Bernd.
> 
> 
> patch-c-strlen-v2.diff
> 
> 2018-08-30  Bernd Edlinger  
> 
>   * builtins.c (c_strlen): Handle not zero terminated STRING_CSTs
>   correctly.
>   * fold-const.c (c_getstr): Fix function comment.  Remove unused third
>   argument.  Fix range checks.
>   * fold-const.h (c_getstr): Adjust protoype.
>   * gimple-fold.c (gimple_fold_builtin_memory_op): Avoid folding when
>   string is constant but contains no NUL byte.
And I've committed the rest of this patch (a piece of the fold-const.c
changes were already committed).

jeff

Re: [PATCH] Adjust string_constant to new STRING_CST semantics

2018-09-13 Thread Jeff Law 
On 8/24/18 3:24 PM, Bernd Edlinger wrote:
> Hi,
> 
> I noticed that the latest patches to make STRING_CST
> semantics for literals and for initializers consistent
> also fixes PR 86711 and PR 86714 (but not PR 87053).
> 
> The code change in the string_constant does not change
> any test case, the new test cases are already fixed
> once we have the unified STRING_CST semantics.
> Thus https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
> would be the pre-condition for this patch.
> 
> If the pre-condition patch is applied then my earlier patch
> to fix PR 86711/86714 is no longer necessary.
> Likely neither Martin's patch (at least that part about
> PR 86711/86714).
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-stringconst.diff
> 
> gcc:
> 2018-08-24  Bernd Edlinger  
> 
>   * expr.c (string_constant): Adjust function comment.
>   Remove bogus check for zero termination.
> 
> testsuite:
> 2018-08-24  Bernd Edlinger  
> 
>   PR middle-end/86711
>   PR middle-end/86714
>   * gcc.c-torture/execute/pr86711.c: New test.
>   * gcc.c-torture/execute/pr86714.c: New test.
So I applied the expr.c part of this patch.  We've already got these
tests in the testsuite.

jeff

Re: [PATCH,rs6000] Add _MM_SHUFFLE definitions for rs6000

2018-09-13 Thread Segher Boessenkool 
Hi Carl,

On Thu, Sep 13, 2018 at 12:08:01PM -0700, Carl Love wrote:
> The _MM_SHUFFLE and _MM_SHUFFLE2 macros are currently defined in the
> i386 branch.  This patch adds the two macro definitions to the rs6000
> config directory as requested.  


> @@ -1163,8 +1166,8 @@ _mm_cvtss_sd (__m128d __A, __m128 __B)
>    res [0] = ((__v4sf)__B) [0];
>    return (__m128d) res;
>  #endif
> -}
>  
> +}

Please don't do this (accidental) change.

Other than that it is fine.  Thanks!  Okay for trunk and 8.


Segher

Go patch committed: Call gcWrwiteBarrier instead of writebarrierptr

2018-09-13 Thread Ian Lance Taylor 
In the Go 1.11 release writebarrierptr is going away, so change the Go
frontend to call gcWriteBarrier instead.  We weren't using
gcWriteBarrier before; adjust the implementation to use the putFast
method.

This revealed a problem in the kickoff function.  When using cgo,
kickoff can be called on the g0 of an m allocated by newExtraM.  In
that case the m will generally have a p, but systemstack may be called
by wbBufFlush as part of flushing the write barrier buffer.  At that
point the buffer is full, so we can not do a write barrier.  So adjust
the existing code in kickoff so that in the case where we are g0,
don't do any write barrier at all.

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 264294)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-82d7205ba9e5c1fe38fd24f89a45caf2e974975b
+218c9159635e06e39ae43d0efe1ac1e694fead2e
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/runtime.def
===
--- gcc/go/gofrontend/runtime.def   (revision 264290)
+++ gcc/go/gofrontend/runtime.def   (working copy)
@@ -302,7 +302,7 @@ DEF_GO_RUNTIME(IFACEEFACEEQ, "runtime.if
 
 
 // Set *dst = src where dst is a pointer to a pointer and src is a pointer.
-DEF_GO_RUNTIME(WRITEBARRIERPTR, "runtime.writebarrierptr",
+DEF_GO_RUNTIME(GCWRITEBARRIER, "runtime.gcWriteBarrier",
   P2(POINTER, POINTER), R0())
 
 // Set *dst = *src for an arbitrary type.
Index: gcc/go/gofrontend/wb.cc
===
--- gcc/go/gofrontend/wb.cc (revision 264283)
+++ gcc/go/gofrontend/wb.cc (working copy)
@@ -380,7 +380,7 @@ Gogo::propagate_writebarrierrec()
 // This is compatible with the definition in the runtime package.
 //
 // For types that are pointer shared (pointers, maps, chans, funcs),
-// we replaced the call to typedmemmove with writebarrierptr(, B).
+// we replaced the call to typedmemmove with gcWriteBarrier(, B).
 // As far as the GC is concerned, all pointers are the same, so it
 // doesn't need the type descriptor.
 //
@@ -391,7 +391,7 @@ Gogo::propagate_writebarrierrec()
 // runtime package, so we could optimize by only testing it once
 // between function calls.
 //
-// A slice could be handled with a call to writebarrierptr plus two
+// A slice could be handled with a call to gcWriteBarrier plus two
 // integer moves.
 
 // Traverse the IR adding write barriers.
@@ -824,7 +824,7 @@ Gogo::assign_with_write_barrier(Function
 case Type::TYPE_MAP:
 case Type::TYPE_CHANNEL:
   // These types are all represented by a single pointer.
-  call = Runtime::make_call(Runtime::WRITEBARRIERPTR, loc, 2, lhs, rhs);
+  call = Runtime::make_call(Runtime::GCWRITEBARRIER, loc, 2, lhs, rhs);
   break;
 
 case Type::TYPE_STRING:
Index: libgo/go/runtime/mgc_gccgo.go
===
--- libgo/go/runtime/mgc_gccgo.go   (revision 264276)
+++ libgo/go/runtime/mgc_gccgo.go   (working copy)
@@ -11,6 +11,11 @@ import (
"unsafe"
 )
 
+// For gccgo, use go:linkname to rename compiler-called functions to
+// themselves, so that the compiler will export them.
+//
+//go:linkname gcWriteBarrier runtime.gcWriteBarrier
+
 // gcRoot is a single GC root: a variable plus a ptrmask.
 //go:notinheap
 type gcRoot struct {
@@ -188,12 +193,7 @@ func checkPreempt() {
 //go:nowritebarrier
 func gcWriteBarrier(dst *uintptr, src uintptr) {
buf := ().m.p.ptr().wbBuf
-   next := buf.next
-   np := next + 2*sys.PtrSize
-   buf.next = np
-   *(*uintptr)(unsafe.Pointer(next)) = src
-   *(*uintptr)(unsafe.Pointer(next + sys.PtrSize)) = *dst
-   if np >= buf.end {
+   if !buf.putFast(src, *dst) {
wbBufFlush(dst, src)
}
*dst = src
Index: libgo/go/runtime/proc.go
===
--- libgo/go/runtime/proc.go(revision 264282)
+++ libgo/go/runtime/proc.go(working copy)
@@ -1146,24 +1146,29 @@ func kickoff() {
 
fv := gp.entry
param := gp.param
-   gp.entry = nil
 
// When running on the g0 stack we can wind up here without a p,
-   // for example from mcall(exitsyscall0) in exitsyscall.
-   // Setting gp.param = nil will call a write barrier, and if
-   // there is no p that write barrier will crash. When called from
-   // mcall the gp.param value will be a *g, which we don't need to
-   // shade since we know it will be kept alive elsewhere. In that
-   // case clear the field using uintptr so that the write barrier
-   // does nothing.
-   if gp.m.p == 0 {
-   if gp ==

libgo patch committed: Correct counters when sweeping span

2018-09-13 Thread Ian Lance Taylor 
Since the gofrontend uses conservative garbage collection on the
stack, pointers that appeared dead can be revived while scanning the
stack.  This can cause the allocation counts seen when sweeping a span
to increase.  We already accept that.  This patch changes the code to
update the counters when we find a discrepancy.  It also fixes the
free index, and changes the span allocation code to retry if it gets a
span that has become full.  This gives us slightly more correct
numbers in MemStats, helps avoid a rare failure in TestReadMemStats,
and helps avoid some very rare crashes.  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 264290)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-f2cd046a4e0d681c3d21ee547b437d3eab8af268
+82d7205ba9e5c1fe38fd24f89a45caf2e974975b
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/mcentral.go
===
--- libgo/go/runtime/mcentral.go(revision 264245)
+++ libgo/go/runtime/mcentral.go(working copy)
@@ -56,6 +56,15 @@ retry:
c.empty.insertBack(s)
unlock()
s.sweep(true)
+
+   // With gccgo's conservative GC, the returned span may
+   // now be full. See the comments in mspan.sweep.
+   if uintptr(s.allocCount) == s.nelems {
+   s.freeindex = s.nelems
+   lock()
+   goto retry
+   }
+
goto havespan
}
if s.sweepgen == sg-1 {
Index: libgo/go/runtime/mgcsweep.go
===
--- libgo/go/runtime/mgcsweep.go(revision 264245)
+++ libgo/go/runtime/mgcsweep.go(working copy)
@@ -296,7 +296,7 @@ func (s *mspan) sweep(preserve bool) boo
}
nfreed := s.allocCount - nalloc
 
-   // This test is not reliable with gccgo, because of
+   // This check is not reliable with gccgo, because of
// conservative stack scanning. The test boils down to
// checking that no new bits have been set in gcmarkBits since
// the span was added to the sweep count. New bits are set by
@@ -309,16 +309,23 @@ func (s *mspan) sweep(preserve bool) boo
// check to be inaccurate, and it will keep an object live
// unnecessarily, but provided the pointer is not really live
// it is not otherwise a problem. So we disable the test for gccgo.
-   if false && nalloc > s.allocCount {
-   print("runtime: nelems=", s.nelems, " nalloc=", nalloc, " 
previous allocCount=", s.allocCount, " nfreed=", nfreed, "\n")
-   throw("sweep increased allocation count")
+   nfreedSigned := int(nfreed)
+   if nalloc > s.allocCount {
+   // print("runtime: nelems=", s.nelems, " nalloc=", nalloc, " 
previous allocCount=", s.allocCount, " nfreed=", nfreed, "\n")
+   // throw("sweep increased allocation count")
+
+   // For gccgo, adjust the freed count as a signed number.
+   nfreedSigned = int(s.allocCount) - int(nalloc)
+   if uintptr(nalloc) == s.nelems {
+   s.freeindex = s.nelems
+   }
}
 
s.allocCount = nalloc
wasempty := s.nextFreeIndex() == s.nelems
s.freeindex = 0 // reset allocation index to start of span.
if trace.enabled {
-   getg().m.p.ptr().traceReclaimed += uintptr(nfreed) * s.elemsize
+   getg().m.p.ptr().traceReclaimed += uintptr(nfreedSigned) * 
s.elemsize
}
 
// gcmarkBits becomes the allocBits.
@@ -334,7 +341,7 @@ func (s *mspan) sweep(preserve bool) boo
// But we need to set it before we make the span available for 
allocation
// (return it to heap or mcentral), because allocation code assumes 
that a
// span is already swept if available for allocation.
-   if freeToHeap || nfreed == 0 {
+   if freeToHeap || nfreedSigned <= 0 {
// The span must be in our exclusive ownership until we update 
sweepgen,
// check for potential races.
if s.state != mSpanInUse || s.sweepgen != sweepgen-1 {
@@ -347,8 +354,11 @@ func (s *mspan) sweep(preserve bool) boo
atomic.Store(, sweepgen)
}
 
-   if nfreed > 0 && spc.sizeclass() != 0 {
-   c.local_nsmallfree[spc.sizeclass()] += uintptr(nfreed)
+   if spc.sizeclass() != 0 {
+   c.local_nsmallfree[spc.sizeclass()] += uintptr(nfreedSigned)
+   }
+
+   if nfreedSigned > 0

Re: [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic

2018-09-13 Thread Jeff Law 
On 9/13/18 7:30 AM, Bernd Edlinger wrote:
> On 08/31/18 19:45, Bernd Edlinger wrote:
>> On 08/31/18 16:42, Jeff Law wrote:
>>> On 08/31/2018 01:08 AM, Bernd Edlinger wrote:
 Hi,


 when re-testing the new STRING_CST semantic patch here:
 https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html

 I noticed that the (my) fix for PR 87053 does still use
 semantic properties of the STRING_CST that is not compatible
 with the new proposed STRING_CST semantics.

 That means that c_strlen needs to handle the case
 where strings are not zero terminated.

 When this is fixed, fortran.dg/pr45636.f90 starts to regress,
 because the check in gimple_fold_builtin_memory_op here:

     if (tree_fits_uhwi_p (len)
     && compare_tree_int (len, MOVE_MAX) <= 0
     /* ???  Don't transform copies from strings with known length 
 this
    confuses the tree-ssa-strlen.c.  This doesn't handle
    the case in gcc.dg/strlenopt-8.c which is XFAILed for that
    reason.  */
     && !c_strlen (src, 2))
 does now return NULL_TREE, because the fortran string is not null 
 terminated.
 However that allows the folding which makes the fortran test case fail.

 I fixed that by pulling in the c_getstr patch again, and use
 it to make another exception for string constants without any embedded NUL.
 That is what tree-ssa-forwprop.c can handle, and should make this work in
 fortran again.


 Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
 Is it OK for trunk?
>>> I've gone the rounds on pr45636 a couple times and it's part of the
>>> reason why there's a FIXME in the bits I committed earlier this week.
>>>
>>
>> Yes, the rationale here is that tree-ssa-forwprop will likely choke if there
>> is a NUL byte in the string:
>>
>>    /* Neither builtin_strncpy_read_str nor builtin_memcpy_read_str
>>   handle embedded '\0's.  */
>>    if (strlen (src_buf) != src_len)
>>      break;
>>
>> Prior to this the c_strlen (src, 2) returned 2, thus this folding was not 
>> done,
>> but since the string does not contain any NUL bytes this returns now NULL.
>> However it is easy to add an exception that triggers only in a fortran string
>> in this way.
>>
>>> I'll look at this closely in conjunction with the (unposted) patch which
>>> addresses the FIXME.
>>>
>>> Jeff
>>>
>>
> 
> Hi,
> 
> this is a minor update to the previous patch version, in that it adds
> the following to c_getstr, in order to be bootstrapped with or without
> the other STRING_CST-v2 semantic patches:
> 
> @@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I
> unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src);
> unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size);
>   
> +  /* Ideally this would turn into a gcc_checking_assert over time.  */
> +  if (string_length > string_size)
> +string_length = string_size;> +
> const char *string = TREE_STRING_POINTER (src);
>   
> if (string_length == 0
I've bootstrapped and regression tested this along with the other
patches in the kit to update STRING_CST semantics.   This re-fixes the
pr87053 regression.  Installed on the trunk.

Jeff

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Jeff Law 
On 8/24/18 2:18 PM, Bernd Edlinger wrote:
> Hi!
> 
> 
> This is an alternative approach in making STRING_CST semantics
> consistent.
> 
> This means it makes STRING_CST used for literals and for initializers
> use exactly the same semantics.
> 
> It makes sure that all STRING_CST have a TYPE_SIZE_UNIT, and that it is
> always larger than TREE_STRING_LENGTH, these and certain other properties
> are checked in varasm.c, so the currently observed consistency issues
> in the middle-end can be resolved.
> 
> 
> It depends on the following pre-condition patches:
> 
> [PATCH] Use complete_array_type on flexible array member initializers
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01538.html
> 
> PATCHv2] Call braced_list_to_string after array size is fixed
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01565.html
> 
> [PATCHv2] Handle overlength strings in the C FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html
> 
> [PATCHv2] Handle overlength strings in C++ FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
> 
> [PATCHv2] Handle overlength string literals in the fortan FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
> 
> The Ada and GO patches are no longer necessary.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-varasm-v2.diff
> 
> 2018-08-24  Bernd Edlinger  
> 
>   * varasm.c (compare_constant): Compare type size of STRING_CSTs.
>   (get_constant_size): Don't make STRING_CSTs larger than they are.
>   (check_string_literal): New check function for STRING_CSTs.
>   (output_constant): Use it.
The prereqs above are all installed and I've installed this patch.
We've still got the pr87053 regression, but I'll commit a fix for that
momentarily.

Jeff

Re: [PATCHv2] Handle overlength strings in the C FE

2018-09-13 Thread Jeff Law 
On 8/24/18 1:59 PM, Bernd Edlinger wrote:
> Hi!
> 
> 
> This is an alternative approach handle overlength strings in the C FE.
> 
> The difference to the previous version is that overlength
> STRING_CST never have a longer TREE_STRING_LENGTH than the TYPE_DOMAIN.
> And those STRING_CSTs are thus no longer zero terminated.
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> patch-c-fe-v2.diff
> 
> 2018-08-01  Bernd Edlinger  
> 
>   * c-typeck.c (digest_init): Shorten overlength strings.
I've installed this on the the trunk.  As we know this will regress
pr87053 temporarily.

Jeff

Go patch committed: Open code select statement setup

2018-09-13 Thread Ian Lance Taylor 
This patch to the Go frontend and libgo changes the compiler to open
code the select statement setup, rather than calling a runtime
function for each case.  This is a version of a change done in the gc
runtime.  I'm bringing it in now to simplify upgrading libgo to the
1.11 release.  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 264283)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-f68c03e509b26e7f483f2800eb70a5fbf3f74d0b
+f2cd046a4e0d681c3d21ee547b437d3eab8af268
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/escape.cc
===
--- gcc/go/gofrontend/escape.cc (revision 264245)
+++ gcc/go/gofrontend/escape.cc (working copy)
@@ -1361,7 +1361,13 @@ Escape_analysis_assign::statement(Block*
   {
 Expression* init = s->temporary_statement()->init();
 if (init != NULL)
-  this->assign(Node::make_node(s), Node::make_node(init));
+ {
+   Node* n = Node::make_node(init);
+   if (s->temporary_statement()->value_escapes())
+ this->assign(this->context_->sink(), n);
+   else
+ this->assign(Node::make_node(s), n);
+ }
   }
   break;
 
@@ -1616,15 +1622,6 @@ Escape_analysis_assign::expression(Expre
 }
 break;
 
-  case Runtime::SELECTSEND:
-{
-  // Send to a channel, lose track. The last argument is
-  // the address of the value to send.
-  Node* arg_node = Node::make_node(call->args()->back());
-  this->assign_deref(this->context_->sink(), arg_node);
-}
-break;
-
   case Runtime::IFACEE2T2:
   case Runtime::IFACEI2T2:
 {
@@ -2228,8 +2225,12 @@ Escape_analysis_assign::assign(Node* dst
 case Expression::EXPRESSION_TEMPORARY_REFERENCE:
   {
 // Temporary is tracked through the underlying Temporary_statement.
-Statement* t = 
dst->expr()->temporary_reference_expression()->statement();
-dst = Node::make_node(t);
+Temporary_statement* t =
+ dst->expr()->temporary_reference_expression()->statement();
+   if (t->value_escapes())
+ dst = this->context_->sink();
+   else
+ dst = Node::make_node(t);
   }
   break;
 
Index: gcc/go/gofrontend/runtime.def
===
--- gcc/go/gofrontend/runtime.def   (revision 264245)
+++ gcc/go/gofrontend/runtime.def   (working copy)
@@ -152,22 +152,10 @@ DEF_GO_RUNTIME(CHANRECV1, "runtime.chanr
 DEF_GO_RUNTIME(CHANRECV2, "runtime.chanrecv2", P2(CHAN, POINTER), R1(BOOL))
 
 
-// Start building a select statement.
-DEF_GO_RUNTIME(NEWSELECT, "runtime.newselect", P3(POINTER, INT64, INT32), R0())
-
-// Add a default clause to a select statement.
-DEF_GO_RUNTIME(SELECTDEFAULT, "runtime.selectdefault", P1(POINTER), R0())
-
-// Add a send clause to a select statement.
-DEF_GO_RUNTIME(SELECTSEND, "runtime.selectsend", P3(POINTER, CHAN, POINTER),
-  R0())
-
-// Add a receive clause to a select statement.
-DEF_GO_RUNTIME(SELECTRECV, "runtime.selectrecv",
-  P4(POINTER, CHAN, POINTER, BOOLPTR), R0())
-
-// Run a select, returning the index of the selected clause.
-DEF_GO_RUNTIME(SELECTGO, "runtime.selectgo", P1(POINTER), R1(INT))
+// Run a select, returning the index of the selected clause and
+// whether that channel received a value.
+DEF_GO_RUNTIME(SELECTGO, "runtime.selectgo", P3(POINTER, POINTER, INT),
+  R2(INT, BOOL))
 
 
 // Panic.
Index: gcc/go/gofrontend/statements.cc
===
--- gcc/go/gofrontend/statements.cc (revision 264259)
+++ gcc/go/gofrontend/statements.cc (working copy)
@@ -4548,17 +4548,19 @@ Select_clauses::Select_clause::traverse(
 
 void
 Select_clauses::Select_clause::lower(Gogo* gogo, Named_object* function,
-Block* b, Temporary_statement* sel)
+Block* b, Temporary_statement* scases,
+size_t index, Temporary_statement* recvok)
 {
   Location loc = this->location_;
 
-  Expression* selref = Expression::make_temporary_reference(sel, loc);
-  selref = Expression::make_unary(OPERATOR_AND, selref, loc);
+  Expression* scase = Expression::make_temporary_reference(scases, loc);
+  Expression* index_expr = Expression::make_integer_ul(index, NULL, loc);
+  scase = Expression::make_array_index(scase, index_expr, NULL, NULL, loc);
 
   if

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Jeff Law 
On 9/13/18 1:41 PM, Bernd Edlinger wrote:
> On 09/13/18 20:44, Jeff Law wrote:
>> On 8/24/18 2:18 PM, Bernd Edlinger wrote:
>>> Hi!
>>>
>>>
>>> This is an alternative approach in making STRING_CST semantics
>>> consistent.
>>>
>>> This means it makes STRING_CST used for literals and for initializers
>>> use exactly the same semantics.
>>>
>>> It makes sure that all STRING_CST have a TYPE_SIZE_UNIT, and that it is
>>> always larger than TREE_STRING_LENGTH, these and certain other properties
>>> are checked in varasm.c, so the currently observed consistency issues
>>> in the middle-end can be resolved.
>>>
>>>
>>> It depends on the following pre-condition patches:
>> [ ... ]
>>
>>>
>>> [PATCHv2] Handle overlength strings in C++ FE
>>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
>> I've committed this patch to the trunk.
>>
> 
> Note, however this leaves pr87503.c semi-broken:
> 
> gcc -fpermissive -x c++ -O2 pr87053.c
> pr87053.c:11:23: warning: initializer-string for array of chars is too long 
> [-fpermissive]
> 11 | } u = {{"1234", "567"}};
> |   ^
> $ ./a.out
> Aborted (core dumped)
> 
> 
> This would be fixed by the patch I posted earlier today:
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00690.html
I'm not done for the day :-)

jeff

[Patch, fortran] PR87284 - [7/8/9 Regression] Allocation of class arrays with mold results in "conditional jump or move depends on uninitialised value"

2018-09-13 Thread Paul Richard Thomas 
The above has been fixed as 'obvious' on all three branches. Revisions
264249, 264251 and 264288.

Cheers

Paul

2018-09-13  Paul Thomas  

PR fortran/87284
* trans-expr.c (gfc_trans_class_init_assign): Access to
to array elements of the dynamic type requires that the array
reference be added to the class expression and not the _data
component, unlike scalar expressions.

2018-09-13  Paul Thomas  

PR fortran/87284
* gfortran.dg/allocate_with_mold_2.f90: New test.

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Bernd Edlinger 
On 09/13/18 20:44, Jeff Law wrote:
> On 8/24/18 2:18 PM, Bernd Edlinger wrote:
>> Hi!
>>
>>
>> This is an alternative approach in making STRING_CST semantics
>> consistent.
>>
>> This means it makes STRING_CST used for literals and for initializers
>> use exactly the same semantics.
>>
>> It makes sure that all STRING_CST have a TYPE_SIZE_UNIT, and that it is
>> always larger than TREE_STRING_LENGTH, these and certain other properties
>> are checked in varasm.c, so the currently observed consistency issues
>> in the middle-end can be resolved.
>>
>>
>> It depends on the following pre-condition patches:
> [ ... ]
> 
>>
>> [PATCHv2] Handle overlength strings in C++ FE
>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
> I've committed this patch to the trunk.
> 

Note, however this leaves pr87503.c semi-broken:

gcc -fpermissive -x c++ -O2 pr87053.c
pr87053.c:11:23: warning: initializer-string for array of chars is too long 
[-fpermissive]
11 | } u = {{"1234", "567"}};
|   ^
$ ./a.out
Aborted (core dumped)


This would be fixed by the patch I posted earlier today:

https://gcc.gnu.org/ml/gcc-patches/2018-09/msg00690.html



Thanks
Bernd.

Re: [PATCH,rs6000] Add _MM_SHUFFLE definitions for rs6000

2018-09-13 Thread Carl Love 
Bill:

Ah, I read your note as Power 8, Power 7 and the IBM AT 11.  Thanks for
the clarification.

Carl Love

On Thu, 2018-09-13 at 14:11 -0500, Bill Schmidt wrote:
> On 9/13/18 2:08 PM, Carl Love wrote:
> > GCC maintainers:
> > 
> > The _MM_SHUFFLE and _MM_SHUFFLE2 macros are currently defined in
> > the
> > i386 branch.  This patch adds the two macro definitions to the
> > rs6000
> > config directory as requested.  
> > 
> > The patch has been tested on 
> > 
> > powerpc64le-unknown-linux-gnu (Power 8 LE)
> > 
> > With no regressions.
> > 
> > The request includes backporting the macro definitions to GCC 7 and
> > GCC
> > 8.  The backport to GCC7 will require creating
> > the config/rs6000/emmintrin.h, config/rs6000/xmmintrin.h and
> > config/rs6000/x86intrin.h files with the standard header comments.
> > 
> > Please let me know if the patch is acceptable for mainline and for
> > backporting to GCC 7 and 8.  Thanks
> 
> Hi Carl,
> 
> No, it should not be backported to GCC 7.  To clarify what I was
> saying:
> these header files exist in branches/ibm/gcc-7-branch but do NOT
> exist
> in branches/gcc-7-branch.  That IBM-private branch (used for AT 11.0)
> should have these macros added, but the main gcc-7-branch should not.
> We don't want any new files added anywhere as part of this work.
> 
> Thanks,
> Bill
> 
> > 
> >    Carl Love
> > -
> > ---
> > 
> > gcc/ChangeLog:
> > 
> > 2018-09-11  Carl Love  
> > 
> > * config/rs6000/emmintrin.h: Add _MM_SHUFFLE2.
> > * config/rs6000/xmmintrin.h: Add _MM_SHUFFLE.
> > ---
> >  gcc/config/rs6000/emmintrin.h | 5 -
> >  gcc/config/rs6000/xmmintrin.h | 3 +++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gcc/config/rs6000/emmintrin.h
> > b/gcc/config/rs6000/emmintrin.h
> > index 412ece7..d4543fb 100644
> > --- a/gcc/config/rs6000/emmintrin.h
> > +++ b/gcc/config/rs6000/emmintrin.h
> > @@ -85,6 +85,9 @@ typedef double __m128d __attribute__
> > ((__vector_size__ (16), __may_alias__));
> >  typedef long long __m128i_u __attribute__ ((__vector_size__ (16),
> > __may_alias__, __aligned__ (1)));
> >  typedef double __m128d_u __attribute__ ((__vector_size__ (16),
> > __may_alias__, __aligned__ (1)));
> >  
> > +/* Define two value permute mask */
> > +#define _MM_SHUFFLE2(x,y) (((x) << 1) | (y))
> > +
> >  /* Create a vector with element 0 as F and the rest zero.  */
> >  extern __inline __m128d __attribute__((__gnu_inline__,
> > __always_inline__, __artificial__))
> >  _mm_set_sd (double __F)
> > @@ -1163,8 +1166,8 @@ _mm_cvtss_sd (__m128d __A, __m128 __B)
> >    res [0] = ((__v4sf)__B) [0];
> >    return (__m128d) res;
> >  #endif
> > -}
> >  
> > +}
> >  extern __inline __m128d __attribute__((__gnu_inline__,
> > __always_inline__, __artificial__))
> >  _mm_shuffle_pd(__m128d __A, __m128d __B, const int __mask)
> >  {
> > diff --git a/gcc/config/rs6000/xmmintrin.h
> > b/gcc/config/rs6000/xmmintrin.h
> > index 43d03ea..11ecbd8 100644
> > --- a/gcc/config/rs6000/xmmintrin.h
> > +++ b/gcc/config/rs6000/xmmintrin.h
> > @@ -57,6 +57,9 @@
> >  #ifndef _XMMINTRIN_H_INCLUDED
> >  #define _XMMINTRIN_H_INCLUDED
> >  
> > +/* Define four value permute mask */
> > +#define _MM_SHUFFLE(w,x,y,z) (((w) << 6) | ((x) << 4) | ((y) << 2)
> > | (z))
> > +
> >  #include 
> >  
> >  /* Avoid collisions between altivec.h and strict adherence to C++
> > and
> > -- 
> > 2.7.4
> > 
> 
>

Re: [PATCH,rs6000] Add _MM_SHUFFLE definitions for rs6000

2018-09-13 Thread Bill Schmidt 
On 9/13/18 2:08 PM, Carl Love wrote:
> GCC maintainers:
>
> The _MM_SHUFFLE and _MM_SHUFFLE2 macros are currently defined in the
> i386 branch.  This patch adds the two macro definitions to the rs6000
> config directory as requested.  
>
> The patch has been tested on 
>
> powerpc64le-unknown-linux-gnu (Power 8 LE)
>
> With no regressions.
>
> The request includes backporting the macro definitions to GCC 7 and GCC
> 8.  The backport to GCC7 will require creating
> the config/rs6000/emmintrin.h, config/rs6000/xmmintrin.h and
> config/rs6000/x86intrin.h files with the standard header comments.
>
> Please let me know if the patch is acceptable for mainline and for
> backporting to GCC 7 and 8.  Thanks

Hi Carl,

No, it should not be backported to GCC 7.  To clarify what I was saying:
these header files exist in branches/ibm/gcc-7-branch but do NOT exist
in branches/gcc-7-branch.  That IBM-private branch (used for AT 11.0)
should have these macros added, but the main gcc-7-branch should not.
We don't want any new files added anywhere as part of this work.

Thanks,
Bill

>
>Carl Love
> 
>
> gcc/ChangeLog:
>
> 2018-09-11  Carl Love  
>
>   * config/rs6000/emmintrin.h: Add _MM_SHUFFLE2.
>   * config/rs6000/xmmintrin.h: Add _MM_SHUFFLE.
> ---
>  gcc/config/rs6000/emmintrin.h | 5 -
>  gcc/config/rs6000/xmmintrin.h | 3 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/rs6000/emmintrin.h b/gcc/config/rs6000/emmintrin.h
> index 412ece7..d4543fb 100644
> --- a/gcc/config/rs6000/emmintrin.h
> +++ b/gcc/config/rs6000/emmintrin.h
> @@ -85,6 +85,9 @@ typedef double __m128d __attribute__ ((__vector_size__ 
> (16), __may_alias__));
>  typedef long long __m128i_u __attribute__ ((__vector_size__ (16), 
> __may_alias__, __aligned__ (1)));
>  typedef double __m128d_u __attribute__ ((__vector_size__ (16), 
> __may_alias__, __aligned__ (1)));
>  
> +/* Define two value permute mask */
> +#define _MM_SHUFFLE2(x,y) (((x) << 1) | (y))
> +
>  /* Create a vector with element 0 as F and the rest zero.  */
>  extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm_set_sd (double __F)
> @@ -1163,8 +1166,8 @@ _mm_cvtss_sd (__m128d __A, __m128 __B)
>    res [0] = ((__v4sf)__B) [0];
>    return (__m128d) res;
>  #endif
> -}
>  
> +}
>  extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, 
> __artificial__))
>  _mm_shuffle_pd(__m128d __A, __m128d __B, const int __mask)
>  {
> diff --git a/gcc/config/rs6000/xmmintrin.h b/gcc/config/rs6000/xmmintrin.h
> index 43d03ea..11ecbd8 100644
> --- a/gcc/config/rs6000/xmmintrin.h
> +++ b/gcc/config/rs6000/xmmintrin.h
> @@ -57,6 +57,9 @@
>  #ifndef _XMMINTRIN_H_INCLUDED
>  #define _XMMINTRIN_H_INCLUDED
>  
> +/* Define four value permute mask */
> +#define _MM_SHUFFLE(w,x,y,z) (((w) << 6) | ((x) << 4) | ((y) << 2) | (z))
> +
>  #include 
>  
>  /* Avoid collisions between altivec.h and strict adherence to C++ and
> -- 
> 2.7.4
>

[PATCH,rs6000] Add _MM_SHUFFLE definitions for rs6000

2018-09-13 Thread Carl Love 
GCC maintainers:

The _MM_SHUFFLE and _MM_SHUFFLE2 macros are currently defined in the
i386 branch.  This patch adds the two macro definitions to the rs6000
config directory as requested.  

The patch has been tested on 

powerpc64le-unknown-linux-gnu (Power 8 LE)

With no regressions.

The request includes backporting the macro definitions to GCC 7 and GCC
8.  The backport to GCC7 will require creating
the config/rs6000/emmintrin.h, config/rs6000/xmmintrin.h and
config/rs6000/x86intrin.h files with the standard header comments.

Please let me know if the patch is acceptable for mainline and for
backporting to GCC 7 and 8.  Thanks

   Carl Love


gcc/ChangeLog:

2018-09-11  Carl Love  

* config/rs6000/emmintrin.h: Add _MM_SHUFFLE2.
* config/rs6000/xmmintrin.h: Add _MM_SHUFFLE.
---
 gcc/config/rs6000/emmintrin.h | 5 -
 gcc/config/rs6000/xmmintrin.h | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/config/rs6000/emmintrin.h b/gcc/config/rs6000/emmintrin.h
index 412ece7..d4543fb 100644
--- a/gcc/config/rs6000/emmintrin.h
+++ b/gcc/config/rs6000/emmintrin.h
@@ -85,6 +85,9 @@ typedef double __m128d __attribute__ ((__vector_size__ (16), 
__may_alias__));
 typedef long long __m128i_u __attribute__ ((__vector_size__ (16), 
__may_alias__, __aligned__ (1)));
 typedef double __m128d_u __attribute__ ((__vector_size__ (16), __may_alias__, 
__aligned__ (1)));
 
+/* Define two value permute mask */
+#define _MM_SHUFFLE2(x,y) (((x) << 1) | (y))
+
 /* Create a vector with element 0 as F and the rest zero.  */
 extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
 _mm_set_sd (double __F)
@@ -1163,8 +1166,8 @@ _mm_cvtss_sd (__m128d __A, __m128 __B)
   res [0] = ((__v4sf)__B) [0];
   return (__m128d) res;
 #endif
-}
 
+}
 extern __inline __m128d __attribute__((__gnu_inline__, __always_inline__, 
__artificial__))
 _mm_shuffle_pd(__m128d __A, __m128d __B, const int __mask)
 {
diff --git a/gcc/config/rs6000/xmmintrin.h b/gcc/config/rs6000/xmmintrin.h
index 43d03ea..11ecbd8 100644
--- a/gcc/config/rs6000/xmmintrin.h
+++ b/gcc/config/rs6000/xmmintrin.h
@@ -57,6 +57,9 @@
 #ifndef _XMMINTRIN_H_INCLUDED
 #define _XMMINTRIN_H_INCLUDED
 
+/* Define four value permute mask */
+#define _MM_SHUFFLE(w,x,y,z) (((w) << 6) | ((x) << 4) | ((y) << 2) | (z))
+
 #include 
 
 /* Avoid collisions between altivec.h and strict adherence to C++ and
-- 
2.7.4

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Bernd Edlinger 
On 09/13/18 20:42, Jeff Law wrote:
> On 8/24/18 2:18 PM, Bernd Edlinger wrote:
>>
>> [PATCHv2] Handle overlength string literals in the fortan FE
>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
> I've committed this patch to the trunk.
> 


Hi Jeff,

I fixed the ChangeLog entry,
and committed as obvious:

Index: gcc/cp/ChangeLog
===
--- gcc/cp/ChangeLog(Revision 264286)
+++ gcc/cp/ChangeLog(Revision 264287)
@@ -1,8 +1,6 @@
  2018-09-13  Bernd Edlinger  
  
* typeck2.c (digest_init_r): Fix overlength strings.
-   * vtable-class-hierarchy.c (build_key_buffer_arg): Make string literal
-   NUL terminated.
  
  2018-09-13  Ville Voutilainen  
  

Thanks
Bernd.

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Jeff Law 
On 8/24/18 2:18 PM, Bernd Edlinger wrote:
> Hi!
> 
> 
> This is an alternative approach in making STRING_CST semantics
> consistent.
> 
> This means it makes STRING_CST used for literals and for initializers
> use exactly the same semantics.
> 
> It makes sure that all STRING_CST have a TYPE_SIZE_UNIT, and that it is
> always larger than TREE_STRING_LENGTH, these and certain other properties
> are checked in varasm.c, so the currently observed consistency issues
> in the middle-end can be resolved.
> 
> 
> It depends on the following pre-condition patches:
[ ... ]

> 
> [PATCHv2] Handle overlength strings in C++ FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
I've committed this patch to the trunk.

jeff

Re: [PATCH] Check the STRING_CSTs in varasm.c

2018-09-13 Thread Jeff Law 
On 8/24/18 2:18 PM, Bernd Edlinger wrote:
> 
> [PATCHv2] Handle overlength string literals in the fortan FE
> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
I've committed this patch to the trunk.

jeff

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
On 13 September 2018 at 20:41, Jason Merrill  wrote:
>> Okay. Do you think we should have an sfk_kind for non-canonical
>> copy/move operations? That would presumably make it a tad more 
>> straightforward to go from
>> fndecl to whatever class bits, instead of what's currently there, where we 
>> say "yeah I had a fndecl,
>> now I turned it into an sfk_kind that says it's a copy constructor, but 
>> guess which one when you're
>> deeming its triviality". ;)
>
> I suppose it would be possible to have a more detailed sfk_kind for
> distinguishing between different signatures, but I'm inclined instead
> to stop using sfk_kind in trivial_fn_p.  Even if having an enumeration
> is convenient for dispatch (or bitmapping), it doesn't need to be the
> same enum.

Yeah, the idea of using a different enum dawned on me straight after
sending that email. ;)
I'll give this approach a spin, more bits into the lang_type and a
different mapping, that way we should indeed
get correct answers for all cases.

Go patch committed: Implement //go:nowritebarrierrec

2018-09-13 Thread Ian Lance Taylor 
The libgo runtime package uses special comments //go:nowritebarrier,
//go:nowritebarrierrec, and //go:yeswritebarrierrec to record whether
write barriers are permitted in a function or not.  The Go frontend
already implements //go:nowritebarrier, but it has been treating
//go:nowritebarrierrec as equivalent to //go:nowritebarrier.  The
//go:nowritebarrierrec comment means not only that the function itself
may not have any write barriers, but also that any functions that it
calls directly may not have any write barriers--unless the function
called is marked //go:yeswritebarrierrec.  This patch implements
support for these comments in the Go frontend.  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 264282)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-baf07c40960dc4f8df9da97281870d80d4245b18
+f68c03e509b26e7f483f2800eb70a5fbf3f74d0b
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 264245)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -941,6 +941,9 @@ class Gogo
std::vector&,
Bfunction* init_bfunction);
 
+  void
+  propagate_writebarrierrec();
+
   Named_object*
   write_barrier_variable();
 
Index: gcc/go/gofrontend/lex.cc
===
--- gcc/go/gofrontend/lex.cc(revision 264245)
+++ gcc/go/gofrontend/lex.cc(working copy)
@@ -1922,9 +1922,15 @@ Lex::skip_cpp_comment()
   // function that it calls, needs to use any write barriers, it
   // should emit an error instead.
   // FIXME: Should only work when compiling the runtime package.
-  // FIXME: currently treated the same as go:nowritebarrier
   this->pragmas_ |= GOPRAGMA_NOWRITEBARRIERREC;
 }
+  else if (verb == "go:yeswritebarrierrec")
+{
+  // Applies to the next function.  Disables go:nowritebarrierrec
+  // when looking at callees; write barriers are permitted here.
+  // FIXME: Should only work when compiling the runtime package.
+  this->pragmas_ |= GOPRAGMA_YESWRITEBARRIERREC;
+}
   else if (verb == "go:cgo_unsafe_args")
 {
   // Applies to the next function.  Taking the address of any
Index: gcc/go/gofrontend/lex.h
===
--- gcc/go/gofrontend/lex.h (revision 264245)
+++ gcc/go/gofrontend/lex.h (working copy)
@@ -63,9 +63,11 @@ enum GoPragma
   GOPRAGMA_SYSTEMSTACK = 1 << 5,   // Must run on system stack.
   GOPRAGMA_NOWRITEBARRIER = 1 << 6,// No write barriers.
   GOPRAGMA_NOWRITEBARRIERREC = 1 << 7, // No write barriers here or callees.
-  GOPRAGMA_CGOUNSAFEARGS = 1 << 8, // Pointer to arg is pointer to all.
-  GOPRAGMA_UINTPTRESCAPES = 1 << 9,// uintptr(p) escapes.
-  GOPRAGMA_NOTINHEAP = 1 << 10 // type is not in heap.
+  GOPRAGMA_YESWRITEBARRIERREC = 1 << 8,// Stops nowritebarrierrec.
+  GOPRAGMA_MARK = 1 << 9,  // Marker for nowritebarrierrec.
+  GOPRAGMA_CGOUNSAFEARGS = 1 << 10,// Pointer to arg is pointer to all.
+  GOPRAGMA_UINTPTRESCAPES = 1 << 11,   // uintptr(p) escapes.
+  GOPRAGMA_NOTINHEAP = 1 << 12 // type is not in heap.
 };
 
 // A token returned from the lexer.
Index: gcc/go/gofrontend/parse.cc
===
--- gcc/go/gofrontend/parse.cc  (revision 264245)
+++ gcc/go/gofrontend/parse.cc  (working copy)
@@ -2360,7 +2360,10 @@ Parse::function_decl(unsigned int pragma
{ GOPRAGMA_NOINLINE, "noinline", false, true, true },
{ GOPRAGMA_SYSTEMSTACK, "systemstack", false, true, true },
{ GOPRAGMA_NOWRITEBARRIER, "nowritebarrier", false, true, true },
-   { GOPRAGMA_NOWRITEBARRIERREC, "nowritebarrierrec", false, true, true },
+   { GOPRAGMA_NOWRITEBARRIERREC, "nowritebarrierrec", false, true,
+ true },
+   { GOPRAGMA_YESWRITEBARRIERREC, "yeswritebarrierrec", false, true,
+ true },
{ GOPRAGMA_CGOUNSAFEARGS, "cgo_unsafe_args", false, true, true },
{ GOPRAGMA_UINTPTRESCAPES, "uintptrescapes", true, true, true },
   };
Index: gcc/go/gofrontend/wb.cc
===
--- gcc/go/gofrontend/wb.cc (revision 264259)
+++ gcc/go/gofrontend/wb.cc (working copy)
@@ -231,6 +231,133 @@ Check_escape::expression(Expression** pe
   return TRAVERSE_CONTINUE;
 }
 
+// Collect all writebarrierrec functions.  This is used when compiling
+// the runtime package, to propagate //go:nowritebarrierrec.
+
+class Collect_writebarrierrec_functions : public Traverse
+{
+ public:
+

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Jason Merrill 
On Thu, Sep 13, 2018 at 12:42 PM, Ville Voutilainen
 wrote:
> On 13 September 2018 at 19:36, Jason Merrill  wrote:
>> On Thu, Sep 13, 2018 at 9:41 AM, Ville Voutilainen
>>  wrote:
>>> On 13 September 2018 at 13:41, Ville Voutilainen
>>>  wrote:
> How does this work when:
>   unsigned has_complex_copy_ctor : 1;
 It doesn't. I need more bits. Luckily, we have some available.
>>>
>>> Here. I suppose this could theoretically be done in some later stage
>>> of class completion,
>>> but that would presumably be an additional member function loop that
>>> looks at the constructors,
>>> weeds out copy constructors, and calculates the triviality bit (and it
>>> should probably then also
>>> look at fields and bases, again). So while I continue to have a minor
>>> distaste for this whole approach,
>>> and how it wastes two perfectly good bits for a dark corner case, I
>>> think I can learn to live with it.
>>
>> Really, the problem is that trivial_fn_p shouldn't use
>> type_has_trivial_fn, and also that the function named
>> "type_has_trivial_fn" actually returns "type has no non-trivial fn".
>> These flags are relics of C++98 semantics.  Your test should also
>> check that !__is_trivially_constructible(M,M&) and
>> !__is_trivially_constructible(M2,M2&).
>
> Yeah, this patch doesn't handle those trivialities correctly, that
> needs further work.
>
>> I suppose that given the limited number of possibly trivial
>> signatures, we can still use flag bits on the class, as your patch is
>> heading toward: one bit for each of the possibly trivial signatures,
>> and TYPE_HAS_COMPLEX_COPY_CTOR; then TYPE_HAS_COPY_CTOR is the union
>> of these.  And similarly for the other possibly trivial functions.
>
> Okay. Do you think we should have an sfk_kind for non-canonical
> copy/move operations? That would presumably make it a tad more 
> straightforward to go from
> fndecl to whatever class bits, instead of what's currently there, where we 
> say "yeah I had a fndecl,
> now I turned it into an sfk_kind that says it's a copy constructor, but guess 
> which one when you're
> deeming its triviality". ;)

I suppose it would be possible to have a more detailed sfk_kind for
distinguishing between different signatures, but I'm inclined instead
to stop using sfk_kind in trivial_fn_p.  Even if having an enumeration
is convenient for dispatch (or bitmapping), it doesn't need to be the
same enum.

Jason

Re: [PATCH] Add a dwarf unit type to represent 24 bit values.

2018-09-13 Thread Jason Merrill 
On Thu, Sep 13, 2018 at 1:35 AM, John Darrington
 wrote:
> On Mon, Sep 10, 2018 at 04:40:58PM +0100, Jason Merrill wrote:
>  On Mon, Sep 10, 2018 at 3:42 PM, John Darrington
>   wrote:
>  > On Mon, Sep 10, 2018 at 03:36:26PM +0100, Jason Merrill wrote:
>  >  On Mon, Aug 27, 2018 at 8:20 PM, John Darrington
>  >   wrote:
>  >  > * include/dwarf2.h (enum dwarf_unit_type) 
> [DE_EH_PE_udata3]: New member.
>  >
>  >
>  >  What's the rationale?  Do you have a separate patch that uses 
> this new macro?
>  >
>  > Yes.   I there is an upcoming patch for GDB.  See
>  > https://sourceware.org/ml/gdb-patches/2018-08/msg00731.html
>
>  This looks like support for reading fixed 3-byte values from the
>  exception handling unwind information.  Do you expect this information
>  to ever need to store 3-byte values?
>
> Yes,  I've already come across that (which is why I created the patch).
> Without this patch, GDB will barf.  See the attached backtrace.

Well, that's curious, given that GCC doesn't have that address type either.

Ah, looking closer, I see that we aren't dealing with the EH unwind
information, but rather the normal DWARF unwind information, which
uses

  /* The encoding for FDE's in a normal .debug_frame section
 depends on the target address size.  */
  cie->encoding = DW_EH_PE_absptr;

it seems strange that GDB then wants to use one of the other codes as
a proxy for loading a particular number of bytes.

I also notice that the default definition of DWARF2_ADDR_SIZE in GCC
has a comment,

/* The size of addresses as they appear in the Dwarf 2 data.
   Some architectures use word addresses to refer to code locations,
   but Dwarf 2 info always uses byte addresses.  On such machines,
   Dwarf 2 addresses need to be larger than the architecture's
   pointers.  */
#define DWARF2_ADDR_SIZE ((POINTER_SIZE + BITS_PER_UNIT - 1) / BITS_PER_UNIT)

And many targets with smaller pointers override this in GCC and GAS, e.g.

./gas/config/tc-msp430.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-s12z.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-wasm32.h:#define DWARF2_ADDR_SIZE(bfd)   4
./gas/config/tc-xgate.h:#define DWARF2_ADDR_SIZE(bfd) 4
./gas/config/tc-aarch64.c:/* Implement DWARF2_ADDR_SIZE.  */
./gas/config/tc-csky.h:#define DWARF2_ADDR_SIZE(bfd)   4
./gas/config/tc-avr.h:#define DWARF2_ADDR_SIZE(bfd) 4

Is this appropriate for your target, as well?

Perhaps GCC should double check that DWARF2_ADDR_SIZE is 2, 4, or 8.

Jason

libgo patch committed: Avoid write barriers with traceback info

2018-09-13 Thread Ian Lance Taylor 
This patch to libgo's runtime package avoids write barriers with traceback info.

Unlike the gc runtime, libgo stores traceback information in location
structs, which contain strings.  Therefore, copying location structs
around appears to require write barriers, although in fact write
barriers are never important because the strings are never allocated
in Go memory.  They come from libbacktrace.

Some of the generated write barriers come at times when write barriers
are not permitted.  For example, exitsyscall, marked
nowritebarrierrec, calls exitsyscallfast which calls traceGoSysExit
which calls traceEvent which calls traceStackID which calls
trace.stackTab.put which copies location values into memory allocated
by tab.newStack.  This write barrier can be invoked when there is no
p, causing a crash.

This change fixes the problem by ensuring that location values are
copied around in the tracing code with no write barriers.

This was found by fixing the compiler to fully implement
//go:nowritebarrierrec; patch to follow.

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 264276)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-70bd9801911f8ed27df410d36a928166c37a68fd
+baf07c40960dc4f8df9da97281870d80d4245b18
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/go/runtime/proc.go
===
--- libgo/go/runtime/proc.go(revision 264276)
+++ libgo/go/runtime/proc.go(working copy)
@@ -1140,7 +1140,7 @@ func startTheWorldWithSema(emitTraceEven
 func kickoff() {
gp := getg()
 
-   if gp.traceback != nil {
+   if gp.traceback != 0 {
gtraceback(gp)
}
 
@@ -3097,7 +3097,7 @@ func newproc(fn uintptr, arg unsafe.Poin
} else {
resetNewG(newg, , )
}
-   newg.traceback = nil
+   newg.traceback = 0
 
if readgstatus(newg) != _Gdead {
throw("newproc1: new g is not Gdead")
Index: libgo/go/runtime/runtime2.go
===
--- libgo/go/runtime/runtime2.go(revision 264245)
+++ libgo/go/runtime/runtime2.go(working copy)
@@ -431,7 +431,7 @@ type g struct {
 
isSystemGoroutine bool // whether goroutine is a "system" goroutine
 
-   traceback *tracebackg // stack traceback buffer
+   traceback uintptr // stack traceback buffer
 
context  g_ucontext_t // saved context for setcontext
stackcontext [10]uintptr  // split-stack context
Index: libgo/go/runtime/trace.go
===
--- libgo/go/runtime/trace.go   (revision 264245)
+++ libgo/go/runtime/trace.go   (working copy)
@@ -135,6 +135,7 @@ var trace struct {
 }
 
 // traceBufHeader is per-P tracing buffer.
+//go:notinheap
 type traceBufHeader struct {
link  traceBufPtr  // in trace.empty/full
lastTicks uint64   // when we wrote the last event
@@ -747,7 +748,8 @@ func (tab *traceStackTable) put(pcs []lo
stk.n = len(pcs)
stkpc := stk.stack()
for i, pc := range pcs {
-   stkpc[i] = pc
+   // Use memmove to avoid write barrier.
+   memmove(unsafe.Pointer([i]), unsafe.Pointer(), 
unsafe.Sizeof(pc))
}
part := int(hash % uintptr(len(tab.tab)))
stk.link = tab.tab[part]
Index: libgo/go/runtime/traceback_gccgo.go
===
--- libgo/go/runtime/traceback_gccgo.go (revision 264245)
+++ libgo/go/runtime/traceback_gccgo.go (working copy)
@@ -186,7 +186,7 @@ func tracebackothers(me *g) {
if gp != nil && gp != me {
print("\n")
goroutineheader(gp)
-   gp.traceback = (*tracebackg)(noescape(unsafe.Pointer()))
+   gp.traceback = (uintptr)(noescape(unsafe.Pointer()))
getTraceback(me, gp)
printtrace(tb.locbuf[:tb.c], nil)
printcreatedby(gp)
@@ -220,7 +220,7 @@ func tracebackothers(me *g) {
print("\tgoroutine in C code; stack unavailable\n")
printcreatedby(gp)
} else {
-   gp.traceback = 
(*tracebackg)(noescape(unsafe.Pointer()))
+   gp.traceback = (uintptr)(noescape(unsafe.Pointer()))
getTraceback(me, gp)
printtrace(tb.locbuf[:tb.c], nil)
printcreatedby(gp)
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 264245)
+++ libgo/runtime/proc.c(working

[visium] Define TARGET_HAVE_SPECULATION_SAFE_VALUE

2018-09-13 Thread Eric Botcazou 
Tested on visium-elf, applied on the mainline.


2018-09-13  Eric Botcazou  

PR target/86812
* config/visium/visium.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): Define.

-- 
Eric BotcazouIndex: config/visium/visium.c
===
--- config/visium/visium.c	(revision 264202)
+++ config/visium/visium.c	(working copy)
@@ -280,17 +280,19 @@ static HOST_WIDE_INT visium_constant_ali
 #undef  TARGET_LEGITIMATE_CONSTANT_P
 #define TARGET_LEGITIMATE_CONSTANT_P visium_legitimate_constant_p
 
-#undef TARGET_LRA_P
+#undef  TARGET_LRA_P
 #define TARGET_LRA_P hook_bool_void_false
 
 #undef  TARGET_LEGITIMATE_ADDRESS_P
 #define TARGET_LEGITIMATE_ADDRESS_P visium_legitimate_address_p
 
-#undef TARGET_PRINT_OPERAND_PUNCT_VALID_P
+#undef  TARGET_PRINT_OPERAND_PUNCT_VALID_P
 #define TARGET_PRINT_OPERAND_PUNCT_VALID_P visium_print_operand_punct_valid_p
-#undef TARGET_PRINT_OPERAND
+
+#undef  TARGET_PRINT_OPERAND
 #define TARGET_PRINT_OPERAND visium_print_operand
-#undef TARGET_PRINT_OPERAND_ADDRESS
+
+#undef  TARGET_PRINT_OPERAND_ADDRESS
 #define TARGET_PRINT_OPERAND_ADDRESS visium_print_operand_address
 
 #undef  TARGET_ATTRIBUTE_TABLE
@@ -347,27 +349,30 @@ static HOST_WIDE_INT visium_constant_ali
 #undef  TARGET_TRAMPOLINE_INIT
 #define TARGET_TRAMPOLINE_INIT visium_trampoline_init
 
-#undef TARGET_MD_ASM_ADJUST
+#undef  TARGET_MD_ASM_ADJUST
 #define TARGET_MD_ASM_ADJUST visium_md_asm_adjust
 
-#undef TARGET_FLAGS_REGNUM
+#undef  TARGET_FLAGS_REGNUM
 #define TARGET_FLAGS_REGNUM FLAGS_REGNUM
 
-#undef TARGET_HARD_REGNO_NREGS
+#undef  TARGET_HARD_REGNO_NREGS
 #define TARGET_HARD_REGNO_NREGS visium_hard_regno_nregs
 
-#undef TARGET_HARD_REGNO_MODE_OK
+#undef  TARGET_HARD_REGNO_MODE_OK
 #define TARGET_HARD_REGNO_MODE_OK visium_hard_regno_mode_ok
 
-#undef TARGET_MODES_TIEABLE_P
+#undef  TARGET_MODES_TIEABLE_P
 #define TARGET_MODES_TIEABLE_P visium_modes_tieable_p
 
-#undef TARGET_CAN_CHANGE_MODE_CLASS
+#undef  TARGET_CAN_CHANGE_MODE_CLASS
 #define TARGET_CAN_CHANGE_MODE_CLASS visium_can_change_mode_class
 
-#undef TARGET_CONSTANT_ALIGNMENT
+#undef  TARGET_CONSTANT_ALIGNMENT
 #define TARGET_CONSTANT_ALIGNMENT visium_constant_alignment
 
+#undef  TARGET_HAVE_SPECULATION_SAFE_VALUE
+#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 namespace {

[Ada] Use uniform EH setting on ARM/Linux platforms

2018-09-13 Thread Eric Botcazou 
As pointed out by Joseph.  Applied on the mainline.


2018-09-13  Eric Botcazou  

* Makefile.rtl (arm% linux-gnueabi%): Always set EH_MECHANISM to -arm.

-- 
Eric Botcazou

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Paul Koning 



> On Sep 13, 2018, at 10:58 AM, Andrew Stubbs  wrote:
> 
> On 13/09/18 15:49, Paul Koning wrote:
>> It's ambiguous, because the last sentence of that paragraph says "addm3 is 
>> used if addptrm3 is not defined."
> 
> I didn't read that as ambiguous; I read it as addm3 is assumed to work fine 
> when addptr is not defined.
> 
>> I don't know of any change in this area.  All I know is that pdp11 has adds 
>> that clobber CC and it doesn't define addptrm3, relying on that last 
>> sentence.  I've tried LRA and for the most part it compiles successfully, I 
>> suppose I should verify the generated code based on the point you raised.  
>> If I really have to define addptr, I'm in trouble because  save/restore CC 
>> is not easy on pdp11.
> 
> The code was added because we had a number of testcases that failed at 
> runtime without it.
> 
> Admittedly, that was in a GCC 7 code-base, and I can't reproduce the failure 
> with one of those test cases now (with addptr deleted), but possibly that's 
> just noise.

Possibly relevant is that pdp11 is a "type 2" CC setting target, one where the 
machine description doesn't mention CC until after reload.  So if reload (LRA) 
is generating adds, the CC effect of that is invisible anyway until later 
passes that deal with the resulting clobbers and elimination, or not, of 
compares.

If that's what this is all about, some documentation clarification would help.  
Can someone confirm (or refute) my guess?

paul

[Ada] Fix PR ada/81103

2018-09-13 Thread Eric Botcazou 
This removes the inclusion of the unused termio.h from terminals.c.

Tested on x86_64-suse-linux, applied on the mainline.


2018-09-13  Eric Botcazou  

PR ada/81103
* terminals.c: Do not include termio.h.

-- 
Eric BotcazouIndex: terminals.c
===
--- terminals.c	(revision 264202)
+++ terminals.c	(working copy)
@@ -1107,14 +1107,6 @@ __gnat_setup_winsize (void *desc, int ro
 #include 
 #include 
 #include 
-
-/* On some system termio is either absent or including it will disable termios
-   (HP-UX) */
-#if !defined (__hpux__) && !defined (BSD) && !defined (__APPLE__) \
-  && !defined (__rtems__) && !defined (__QNXNTO__)
-#   include 
-#endif
-
 #include 
 #include 
 #include 
@@ -1130,7 +1122,6 @@ __gnat_setup_winsize (void *desc, int ro
 #   include 
 #endif
 #if defined (__hpux__)
-#   include 
 #   include 
 #endif

Re: [PATCH][Middle-end]Add a new option to finer control inlining based on function's visibility

2018-09-13 Thread Qing Zhao 


> On Sep 13, 2018, at 2:06 AM, Richard Biener  wrote:
> 
> On Wed, 12 Sep 2018, Alexander Monakov wrote:
> 
>> On Wed, 12 Sep 2018, Richard Biener wrote:
>>> With LTO "static" is too blurry - would -finline-only-hidden be OK
>>> with you?
>> 
>> This doesn't sound right - in non-pic, everything is hidden, effectively.
>> And the intended use is with Linux kernel, which does not use -fpic.
>> 
>> I agree with LTO this option makes less sense, but I wouldn't expect LTO
>> to be used for livepatching-capable kernels.
> 
> The issue is that with LTO this option probably cannot be reliably
> implemented (well, I guess we could stick 'noinline' attributes
> onto all non-static declared functions…).

currently, I am using 

callee->externally_visible 

i.e.:

  /* Set when function is visible by other units.  */
  unsigned externally_visible : 1;

to distinguish whether the function is static (visible ONLY inside the file) or 
global (visible by other files)

if externally_visible is TRUE and the option -finline-only-static is specified, 
disable the inlining.


> 
> Btw, what about
> 
> inline T foo() {}
> 
> in C99?

for this case, foo is not visible outside of the file? right?

the current implementation based on externally_visible can correctly treat this 
routine as “static” (visible ONLY inside the file), and inline the call to foo.
this seems the correct behavior.


>  Those are not declared static (in fact there may be
> extern T foo () declarations somewhere).  I also think we
> have to continue to inline always-inline functions.

for always-inline functions, if they are NOT static, I think that the option 
-finline-no-static should NOT inline them for help hot-patching purpose. 

> 
> That is, is it really "-fimplicit-inline-only-static"?  Would
> it make more sense to have a -fno-implicit-inline switch which
> will not inline any function that is not declared inline?

not quite understand this, why adding “implicit”?
the new option -finline-no-static will control inlining of functions with or 
without “declared inline”.

thanks.

Qing


> That might be already available via -fno-inline-small-functions
> [-fno-inline-functions].
> 
> Richard.

libgo patch committed: Build GC roots index

2018-09-13 Thread Ian Lance Taylor 
This libgo patch by Than McIntosh build a GC roots index to speed up
write barrier processing in bulkBarrierPreWrite and elsewhere.  The GC
roots index is basically a sorted list of all roots, so as to allow
more efficient lookups of gcdata structures for globals.  The previous
implementation worked on the raw (unsorted) roots list itself, which
did not scale well.  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 264259)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-acf852f838e6b99f907d84648be853fa2c374393
+70bd9801911f8ed27df410d36a928166c37a68fd
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc   (revision 264245)
+++ gcc/go/gofrontend/gogo.cc   (working copy)
@@ -1535,7 +1535,8 @@ Gogo::write_globals()
  // Avoid putting runtime.gcRoots itself on the list.
  if (this->compiling_runtime()
  && this->package_name() == "runtime"
- && Gogo::unpack_hidden_name(no->name()) == "gcRoots")
+ && (Gogo::unpack_hidden_name(no->name()) == "gcRoots"
+   || Gogo::unpack_hidden_name(no->name()) == "gcRootsIndex"))
;
  else
var_gc.push_back(no);
Index: libgo/go/runtime/cgocall.go
===
--- libgo/go/runtime/cgocall.go (revision 264245)
+++ libgo/go/runtime/cgocall.go (working copy)
@@ -243,17 +243,21 @@ func cgoCheckUnknownPointer(p unsafe.Poi
return
}
 
-   roots := gcRoots
-   for roots != nil {
-   for j := 0; j < roots.count; j++ {
-   pr := roots.roots[j]
-   addr := uintptr(pr.decl)
-   if cgoInRange(p, addr, addr+pr.size) {
-   cgoCheckBits(pr.decl, pr.gcdata, 0, pr.ptrdata)
-   return
-   }
+   lo := 0
+   hi := len(gcRootsIndex)
+   for lo < hi {
+   m := lo + (hi-lo)/2
+   pr := gcRootsIndex[m]
+   addr := uintptr(pr.decl)
+   if cgoInRange(p, addr, addr+pr.size) {
+   cgoCheckBits(pr.decl, pr.gcdata, 0, pr.ptrdata)
+   return
+   }
+   if uintptr(p) < addr {
+   hi = m
+   } else {
+   lo = m + 1
}
-   roots = roots.next
}
 
return
Index: libgo/go/runtime/mbitmap.go
===
--- libgo/go/runtime/mbitmap.go (revision 264245)
+++ libgo/go/runtime/mbitmap.go (working copy)
@@ -575,19 +575,23 @@ func bulkBarrierPreWrite(dst, src, size
if !inheap(dst) {
// If dst is a global, use the data or BSS bitmaps to
// execute write barriers.
-   roots := gcRoots
-   for roots != nil {
-   for i := 0; i < roots.count; i++ {
-   pr := roots.roots[i]
-   addr := uintptr(pr.decl)
-   if addr <= dst && dst < addr+pr.size {
-   if dst < addr+pr.ptrdata {
-   bulkBarrierBitmap(dst, src, 
size, dst-addr, pr.gcdata)
-   }
-   return
+   lo := 0
+   hi := len(gcRootsIndex)
+   for lo < hi {
+   m := lo + (hi-lo)/2
+   pr := gcRootsIndex[m]
+   addr := uintptr(pr.decl)
+   if addr <= dst && dst < addr+pr.size {
+   if dst < addr+pr.ptrdata {
+   bulkBarrierBitmap(dst, src, size, 
dst-addr, pr.gcdata)
}
+   return
+   }
+   if dst < addr {
+   hi = m
+   } else {
+   lo = m + 1
}
-   roots = roots.next
}
return
}
Index: libgo/go/runtime/mgc_gccgo.go
===
--- libgo/go/runtime/mgc_gccgo.go   (revision 264245)
+++ libgo/go/runtime/mgc_gccgo.go   (working copy)
@@ -12,6 +12,7 @@ import (
 )
 
 // gcRoot is a single GC root: a variable plus a ptrmask.
+//go:notinheap
 type gcRoot struct {

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
On 13 September 2018 at 19:36, Jason Merrill  wrote:
> On Thu, Sep 13, 2018 at 9:41 AM, Ville Voutilainen
>  wrote:
>> On 13 September 2018 at 13:41, Ville Voutilainen
>>  wrote:
 How does this work when:
   unsigned has_complex_copy_ctor : 1;
>>> It doesn't. I need more bits. Luckily, we have some available.
>>
>> Here. I suppose this could theoretically be done in some later stage
>> of class completion,
>> but that would presumably be an additional member function loop that
>> looks at the constructors,
>> weeds out copy constructors, and calculates the triviality bit (and it
>> should probably then also
>> look at fields and bases, again). So while I continue to have a minor
>> distaste for this whole approach,
>> and how it wastes two perfectly good bits for a dark corner case, I
>> think I can learn to live with it.
>
> Really, the problem is that trivial_fn_p shouldn't use
> type_has_trivial_fn, and also that the function named
> "type_has_trivial_fn" actually returns "type has no non-trivial fn".
> These flags are relics of C++98 semantics.  Your test should also
> check that !__is_trivially_constructible(M,M&) and
> !__is_trivially_constructible(M2,M2&).

Yeah, this patch doesn't handle those trivialities correctly, that
needs further work.

> I suppose that given the limited number of possibly trivial
> signatures, we can still use flag bits on the class, as your patch is
> heading toward: one bit for each of the possibly trivial signatures,
> and TYPE_HAS_COMPLEX_COPY_CTOR; then TYPE_HAS_COPY_CTOR is the union
> of these.  And similarly for the other possibly trivial functions.

Okay. Do you think we should have an sfk_kind for non-canonical
copy/move operations?
That would presumably make it a tad more straightforward to go from
fndecl to whatever class bits,
instead of what's currently there, where we say "yeah I had a fndecl,
now I turned it into an sfk_kind
that says it's a copy constructor, but guess which one when you're
deeming its triviality". ;)

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Jason Merrill 
On Thu, Sep 13, 2018 at 9:41 AM, Ville Voutilainen
 wrote:
> On 13 September 2018 at 13:41, Ville Voutilainen
>  wrote:
>>> How does this work when:
>>>   unsigned has_complex_copy_ctor : 1;
>> It doesn't. I need more bits. Luckily, we have some available.
>
> Here. I suppose this could theoretically be done in some later stage
> of class completion,
> but that would presumably be an additional member function loop that
> looks at the constructors,
> weeds out copy constructors, and calculates the triviality bit (and it
> should probably then also
> look at fields and bases, again). So while I continue to have a minor
> distaste for this whole approach,
> and how it wastes two perfectly good bits for a dark corner case, I
> think I can learn to live with it.

Really, the problem is that trivial_fn_p shouldn't use
type_has_trivial_fn, and also that the function named
"type_has_trivial_fn" actually returns "type has no non-trivial fn".
These flags are relics of C++98 semantics.  Your test should also
check that !__is_trivially_constructible(M,M&) and
!__is_trivially_constructible(M2,M2&).

I suppose that given the limited number of possibly trivial
signatures, we can still use flag bits on the class, as your patch is
heading toward: one bit for each of the possibly trivial signatures,
and TYPE_HAS_COMPLEX_COPY_CTOR; then TYPE_HAS_COPY_CTOR is the union
of these.  And similarly for the other possibly trivial functions.

Jason

[PR debug/87295] ICE in dwarf2out

2018-09-13 Thread Nathan Sidwell 

Richard, Jan,  as early debug experts perhaps you can clue me in?

87295 is an ICE in lookup_external_ref, we strcmp a NULL 
die->die_id.die_symbol.


The die is being cloned in clone_as_declaration during a 
copy_ancestor_tree walk ultimately coming from 
copy_decls_for_unworthy_types.


The incoming die is DW_TAG_structure_type with comdat_type_p set.  We 
create a new_die_raw, which has NULL die_id.die_symbol and then never 
set that field (or another piece of the die_id union).


How is this supposed to work?

nathan
--
Nathan Sidwell

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Andrew Stubbs 

On 13/09/18 15:49, Paul Koning wrote:

It's ambiguous, because the last sentence of that paragraph says "addm3 is used if 
addptrm3 is not defined."


I didn't read that as ambiguous; I read it as addm3 is assumed to work 
fine when addptr is not defined.



I don't know of any change in this area.  All I know is that pdp11 has adds 
that clobber CC and it doesn't define addptrm3, relying on that last sentence.  
I've tried LRA and for the most part it compiles successfully, I suppose I 
should verify the generated code based on the point you raised.  If I really 
have to define addptr, I'm in trouble because  save/restore CC is not easy on 
pdp11.


The code was added because we had a number of testcases that failed at 
runtime without it.


Admittedly, that was in a GCC 7 code-base, and I can't reproduce the 
failure with one of those test cases now (with addptr deleted), but 
possibly that's just noise.


Andrew

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Paul Koning 



> On Sep 13, 2018, at 10:39 AM, Andrew Stubbs  wrote:
> 
> On 13/09/18 15:16, Paul Koning wrote:
>> If you don't have machine operations that add without messing with
>> condition codes, wouldn't it make sense to omit the definition of the
>> add-pointer patterns?  GCC will build things out of normal
>> (CC-clobbering) adds if there are no add-pointer operations, which
>> may well be more efficient in most cases than explicitly
>> saving/restoring a CC that may in fact not matter right at that
>> spot.
> 
> I thought the whole point of addptr is that it *is* needed when add
> clobbers CC? As in, LRA spills are malformed without this.
> 
> Did something change? The internals manual still says "It only needs to
> be defined if addm3 sets the condition code."

It's ambiguous, because the last sentence of that paragraph says "addm3 is used 
if addptrm3 is not defined."  

I don't know of any change in this area.  All I know is that pdp11 has adds 
that clobber CC and it doesn't define addptrm3, relying on that last sentence.  
I've tried LRA and for the most part it compiles successfully, I suppose I 
should verify the generated code based on the point you raised.  If I really 
have to define addptr, I'm in trouble because  save/restore CC is not easy on 
pdp11.

paul

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Andrew Stubbs 

On 13/09/18 15:16, Paul Koning wrote:

If you don't have machine operations that add without messing with
condition codes, wouldn't it make sense to omit the definition of the
add-pointer patterns?  GCC will build things out of normal
(CC-clobbering) adds if there are no add-pointer operations, which
may well be more efficient in most cases than explicitly
saving/restoring a CC that may in fact not matter right at that
spot.


I thought the whole point of addptr is that it *is* needed when add
clobbers CC? As in, LRA spills are malformed without this.

Did something change? The internals manual still says "It only needs to
be defined if addm3 sets the condition code."

Andrew

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Paul Koning 



> On Sep 13, 2018, at 10:08 AM, Andrew Stubbs  wrote:
> 
> On 13/09/18 11:01, Andrew Stubbs wrote:
>> The assert is caused because the def-use chains indicate that SCC conflicts 
>> with itself. I suppose the question is why is it doing that, but it's 
>> probably do do with that being a special register that gets used in split2 
>> (particularly by the addptrdi3 pattern). Although, those patterns are 
>> careful to save SCC to one side and then restore it again after, so I'd have 
>> thought the DF analysis would work out?
> 
> I think I may have a theory on this one now
> 
> The addptrdi3 pattern must use two 32-bit adds with a carry in SCC, but 
> addptr patterns are not allowed to clobber SCC, so the splitter carefully 
> saves and restores the old value.

If you don't have machine operations that add without messing with condition 
codes, wouldn't it make sense to omit the definition of the add-pointer 
patterns?  GCC will build things out of normal (CC-clobbering) adds if there 
are no add-pointer operations, which may well be more efficient in most cases 
than explicitly saving/restoring a CC that may in fact not matter right at that 
spot.

paul

[PATCH] Fix PR87263

2018-09-13 Thread Richard Biener 


I'm using this PR as an excuse to put in the pending non-iteration
"iteration" rewrite.  This ensures that we've at least visited one
predecessor of a block and re-instantiates only visiting reachable
blocks.

{O1,}-Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

It probably doesn't fully fix PR87263 but the regression police will
come up with more testcases soonish.

Richard.

2018-09-13  Richard Biener  

PR tree-optimization/87263
* tree-ssa-sccvn.c (visit_phi): Revert some earlier changes.
(struct unwind_state): Add max_rpo field.
(do_rpo_vn): Allow up-to-date loop state to be used when not iterating.
Compute max_rpo, the max RPO number a block can be backwards reached
from.  Re-write non-iterating mode to a RPO ordered worklist approach,
separating it from the iterating mode.

* gcc.dg/torture/pr87263.c: New testcase.
* gcc.dg/torture/ssa-fre-2.c: Likewise.
* gcc.dg/torture/ssa-fre-3.c: Likewise.
* gcc.dg/torture/ssa-fre-4.c: Likewise.

Index: gcc/testsuite/gcc.dg/torture/pr87263.c
===
--- gcc/testsuite/gcc.dg/torture/pr87263.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr87263.c  (working copy)
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+
+int a, b, c;
+
+int main ()
+{ 
+  int g, *h[3] = {, , };
+  if (h[2] == 0)
+;
+  else
+{ 
+  int i[1];
+  if (a)
+   while (a)
+ L:;
+  else
+   {
+ int k = b;
+   }
+}
+  if ((b < c) > b)
+goto L;
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-2.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-2.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-2.c(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+
+int x;
+int main()
+{
+  int i = 0;
+  x = 0;
+  if (x)
+{
+  for (; i < 10; ++i)
+   {
+doit:
+ x = i;
+   }
+}
+  if (!x)
+goto doit;
+  if (x != 9)
+__builtin_abort ();
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-3.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-3.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-3.c(working copy)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fdump-tree-fre1" } */
+
+int x;
+int main()
+{
+  x = 0;
+  int z = x;
+  int w = 1;
+  for (int i = 0; i < 32; ++i)
+{
+  if (z)
+   w = 2;
+  else
+   w = 1;
+  if (w == 2)
+   __builtin_abort ();
+}
+  return w;
+}
+
+/* { dg-final { scan-tree-dump-not "abort" "fre1" } } */
Index: gcc/testsuite/gcc.dg/torture/ssa-fre-4.c
===
--- gcc/testsuite/gcc.dg/torture/ssa-fre-4.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/ssa-fre-4.c(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+/* { dg-additional-options "-fdump-tree-fre1" } */
+
+int x;
+int main()
+{
+  x = 0;
+  if (x)
+{
+  for (int i = 0; i < 10; ++i)
+   x = i;
+}
+  return x;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 264267)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -4198,10 +4198,7 @@ visit_phi (gimple *phi, bool *inserted,
  }
   }
 
-  /* If the value we want to use is the backedge and that wasn't visited
- yet or if we should take it as VARYING but it has a non-VARYING
- value drop to VARYING.  This only happens when not iterating.
- If we value-number a virtual operand never value-number to the
+  /* If we value-number a virtual operand never value-number to the
  value from the backedge as that confuses the alias-walking code.
  See gcc.dg/torture/pr87176.c.  If the value is the same on a
  non-backedge everything is OK though.  */
@@ -4209,9 +4206,7 @@ visit_phi (gimple *phi, bool *inserted,
   && !seen_non_backedge
   && TREE_CODE (backedge_val) == SSA_NAME
   && sameval == backedge_val
-  && (SSA_NAME_IS_VIRTUAL_OPERAND (backedge_val)
- || !SSA_VISITED (backedge_val)
- || SSA_VAL (backedge_val) != backedge_val))
+  && SSA_NAME_IS_VIRTUAL_OPERAND (backedge_val))
 /* Note this just drops to VARYING without inserting the PHI into
the hashes.  */
 result = PHI_RESULT (phi);
@@ -6226,6 +6221,9 @@ struct unwind_state
   /* Whether to handle this as iteration point or whether to treat
  incoming backedge PHI values as varying.  */
   bool iterate;
+  /* Maximum RPO index this block is reachable from.  */
+  int max_rpo;
+  /* Unwind state.  */
   void *ob_top;
   vn_reference_t

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Andrew Stubbs 

On 13/09/18 11:01, Andrew Stubbs wrote:
The assert is caused because the def-use chains indicate that SCC 
conflicts with itself. I suppose the question is why is it doing that, 
but it's probably do do with that being a special register that gets 
used in split2 (particularly by the addptrdi3 pattern). Although, those 
patterns are careful to save SCC to one side and then restore it again 
after, so I'd have thought the DF analysis would work out?


I think I may have a theory on this one now

The addptrdi3 pattern must use two 32-bit adds with a carry in SCC, but 
addptr patterns are not allowed to clobber SCC, so the splitter 
carefully saves and restores the old value.


This is correct at runtime, and looks correct in RTL dumps, but it means 
that there's still a single rtx REG instance holding the live SCC 
register, and its still live before and after the new add instruction.


Would I be right in thinking that the dataflow analysis doesn't like this?

I think I have a work-around (by using different instructions), but is 
there a correct way to do this if there weren't an alternative?


Andrew

[PATCH] Improve PR63155, SSA coalescing conflict merging

2018-09-13 Thread Richard Biener 


A non-trivial amount of time is spent in ssa_conflicts_merge,
in particular the

  /* Add a conflict between X and every one Y has.  If the bitmap doesn't
 exist, then it has already been coalesced, and we don't need to add a
 conflict.  */
  EXECUTE_IF_SET_IN_BITMAP (by, 0, z, bi)
{
  bitmap bz = ptr->conflicts[z];
  if (bz)
bitmap_set_bit (bz, x);
}

loop because it is cache unfriendly.  The observation is that while
it adds a conflict to X where a conflict to Y existed it doesn't
actually remove the no longer needed conflict to Y.

Doing that, while more work, improves compile-time on the 2nd
testcase in the PR from 46s to 22s (leaving memory use alone).
The first testcase improves from 108s to 39s (leaving memory use alone).
Memory use is still a big issue for both testcases.

The explanation is possibly that when doing a lot of coalescing into
some variable the above walks get longer and longer because we add
more conflicts overall.

Bootstrap & regtest running on x86_64-unknown-linux-gnu.

Richard.

2018-09-13  Richard Biener  

PR middle-end/63155
* tree-ssa-coalesce.c (ssa_conflicts_merge): Remove conflict
bits for the merged partition.

Index: gcc/tree-ssa-coalesce.c
===
--- gcc/tree-ssa-coalesce.c (revision 264259)
+++ gcc/tree-ssa-coalesce.c (working copy)
@@ -620,7 +620,11 @@ ssa_conflicts_merge (ssa_conflicts *ptr,
 {
   bitmap bz = ptr->conflicts[z];
   if (bz)
-   bitmap_set_bit (bz, x);
+   {
+ bool was_there = bitmap_clear_bit (bz, y);
+ gcc_checking_assert (was_there);
+ bitmap_set_bit (bz, x);
+   }
 }
 
   if (bx)

Re: VRP: undefined shifting calculation should not need sign bit

2018-09-13 Thread Aldy Hernandez 




On 09/13/2018 03:33 AM, Richard Sandiford wrote:

Aldy Hernandez  writes:

On 09/12/2018 12:57 PM, Richard Sandiford wrote:

Aldy Hernandez  writes:

diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
index 589fdea4df6..e9ee418e5b2 100644
--- a/gcc/wide-int-range.h
+++ b/gcc/wide-int-range.h
@@ -131,7 +131,7 @@ extern bool wide_int_range_div (wide_int , wide_int 
,
   /* Return TRUE if shifting by range [MIN, MAX] is undefined behavior.  */
   
   inline bool

-wide_int_range_shift_undefined_p (signop sign, unsigned prec,
+wide_int_range_shift_undefined_p (unsigned prec,
  const wide_int , const wide_int )
   {
 /* ?? Note: The original comment said this only applied to
@@ -142,7 +142,7 @@ wide_int_range_shift_undefined_p (signop sign, unsigned 
prec,
behavior from the shift operation.  We cannot even trust
SHIFT_COUNT_TRUNCATED at this stage, because that applies to rtl
shifts, and the operation at the tree level may be widened.  */
-  return wi::lt_p (min, 0, sign) || wi::ge_p (max, prec, sign);
+  return wi::sign_mask (min) || wi::ge_p (max, prec, UNSIGNED);


I don't think this is a good idea.  Logically the comparison should
be done relative to the TYPE_SIGN of the type, so I think the original
code was correct.


The operation to calculate undefinedness must be done with the type of
the RHS, as opposed to the type of the entire operation.  This can be
confusing, as most operations use the same type for all operands as well
as for the type of the entire operation.  For example, AFAICT, the
following is valid gimple:

UINT64 = UINT64 << INT32

The original code was doing this (correctly), but since it was confusing
to remember which type to pass, I rewrote the above function to not need
the sign of the RHS.  This came about because in my ranger work, I
passed the wrong type which took forever to find ;-).  My patch avoids
further confusion.

Am I missing a subtle incorrectness in my approach?


The problem is with things like UINT256 << UINT8 vs. UINT256 << INT8.
A range of [128, 131] on the UINT8 would be represented using the same
wide_ints as a range of [-128, -125] on the INT8, but the former is
well-defined while the latter isn't.  Only the TYPE_SIGN tells you
which applies.

The original code got this right, but the new code effectively assumes
all shift amounts are signed, and so would treat UINT8 like INT8.

OK, so no current target actually supports UINT256 AFAIK, so it might
be academic.  But the original point of wide-int.h was to support such
wide types, so they could become a thing in future.
Heh heh.  Academical or not, it seems like finding these UINT256 bugs in 
the future will be harder than me passing the correct inner sign now.


My tree is a mess right now, but I'll submit a fix next week reverting 
the inner sign discrepancy, while keeping the bits that remove the 
vrp_shift_undefined_p wrapper.


Thank you for your explanation.
Aldy

[PATCH] Limit workaround for Clang bug to __clang_major__ <= 7

2018-09-13 Thread Jonathan Wakely 

The bug https://bugs.llvm.org/show_bug.cgi?id=33222 is now fixed on
Clang trunk, so the workaround won't be needed for Clang 8.0 and later.

* include/std/variant (variant) [__clang__]: Limit workaround to
Clang 7 and older.

Tested x86_64-linux, committed to trunk.

I hope somebody will check with Clang trunk soon and verify this
works.

commit 6112f62b32c84edd2e2369116ad233f6fef5b585
Author: Jonathan Wakely 
Date:   Thu Sep 13 14:30:05 2018 +0100

Limit workaround for Clang bug to __clang_major__ <= 7

The bug https://bugs.llvm.org/show_bug.cgi?id=33222 is now fixed on
Clang trunk, so the workaround won't be needed for Clang 8.0 and later.

* include/std/variant (variant) [__clang__]: Limit workaround to
Clang 7 and older.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 2d86a704c63..5a77e9e2d84 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1296,7 +1296,7 @@ namespace __variant
 
 #undef _VARIANT_RELATION_FUNCTION_TEMPLATE
 
-#ifdef __clang__
+#if defined(__clang__) && __clang_major__ <= 7
 public:
   using _Base::_M_u; // See https://bugs.llvm.org/show_bug.cgi?id=31852
 private:

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Jakub Jelinek 
Hi!

On Thu, Sep 13, 2018 at 04:41:25PM +0300, Ville Voutilainen wrote:
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -2046,7 +2046,7 @@ struct GTY(()) lang_type {
>unsigned lazy_copy_assign : 1;
>unsigned lazy_destructor : 1;
>unsigned has_const_copy_ctor : 1;
> -  unsigned has_complex_copy_ctor : 1;
> +  unsigned has_complex_copy_ctor : 2;
>unsigned has_complex_copy_assign : 1;
>  
>unsigned non_aggregate : 1;

Just a formatting nit, there is currently an empty new-line after each group
of 8 bits, so if you add one bit in the middle, the empty lines need to move
as well (i.e. empty line before has_complex_copy_assign instead of after it
and ditto for has_complex_move_ctor).

I'll defer actual review to Jason/Nathan.

> @@ -2070,7 +2070,7 @@ struct GTY(()) lang_type {
>/* There are some bits left to fill out a 32-bit word.  Keep track
>   of this by updating the size of this bitfield whenever you add or
>   remove a flag.  */
> -  unsigned dummy : 4;
> +  unsigned dummy : 3;
>  
>tree primary_base;
>vec *vcall_indices;

Jakub

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
On 13 September 2018 at 13:41, Ville Voutilainen
 wrote:
>> How does this work when:
>>   unsigned has_complex_copy_ctor : 1;
> It doesn't. I need more bits. Luckily, we have some available.

Here. I suppose this could theoretically be done in some later stage
of class completion,
but that would presumably be an additional member function loop that
looks at the constructors,
weeds out copy constructors, and calculates the triviality bit (and it
should probably then also
look at fields and bases, again). So while I continue to have a minor
distaste for this whole approach,
and how it wastes two perfectly good bits for a dark corner case, I
think I can learn to live with it.

Tests pass on Linux-PPC64, although the suite is still running some
library tests, so I think I'll
do a paranoid re-run. OK for trunk? I'm not planning to backport this,
it's not a regression.

2018-09-13  Ville Voutilainen  

gcc/cp

 PR c++/87051
* cp-tree.h (lang_type): Grow has_complex_copy_ctor and shrink
dummy.
* decl.c (grok_special_member_properties): Don't mark the class
as having a non-trivial copy constructor if the copy constructor
we're looking at is not a const-copy and a const-copy was already
found, reset the copy constructor triviality if we found a
trivial const-copy.

testsuite/

 PR c++/87051
* g++.dg/ext/is_trivially_constructible7.C: New.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6cd6e5f..fa39f6a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2046,7 +2046,7 @@ struct GTY(()) lang_type {
   unsigned lazy_copy_assign : 1;
   unsigned lazy_destructor : 1;
   unsigned has_const_copy_ctor : 1;
-  unsigned has_complex_copy_ctor : 1;
+  unsigned has_complex_copy_ctor : 2;
   unsigned has_complex_copy_assign : 1;
 
   unsigned non_aggregate : 1;
@@ -2070,7 +2070,7 @@ struct GTY(()) lang_type {
   /* There are some bits left to fill out a 32-bit word.  Keep track
  of this by updating the size of this bitfield whenever you add or
  remove a flag.  */
-  unsigned dummy : 4;
+  unsigned dummy : 3;
 
   tree primary_base;
   vec *vcall_indices;
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 50b60e8..452e968 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13212,7 +13212,15 @@ grok_special_member_properties (tree decl)
 	 default arguments.  */
 	  TYPE_HAS_COPY_CTOR (class_type) = 1;
 	  if (user_provided_p (decl))
-	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	{
+	  if (ctor > 1)
+		TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	  else if (!TYPE_HAS_CONST_COPY_CTOR (class_type)
+		   && TYPE_HAS_COMPLEX_COPY_CTOR (class_type) < 2)
+		TYPE_HAS_COMPLEX_COPY_CTOR (class_type) += 2;
+	}
+	  else if (ctor > 1 && TYPE_HAS_COMPLEX_COPY_CTOR (class_type) == 2)
+	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 0;
 	  if (ctor > 1)
 	TYPE_HAS_CONST_COPY_CTOR (class_type) = 1;
 	}
diff --git a/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
new file mode 100644
index 000..177eb2e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++11 } }
+
+// PR c++/87051
+
+struct M {
+  M(const M&) = default;
+  M(M&);
+};
+
+struct M2 {
+  M2(M2&);
+  M2(const M2&) = default;
+};
+
+static_assert( __is_trivially_constructible(M, M&&), "");
+static_assert( __is_trivially_constructible(M2, M2&&), "");

Re: [PATCH] Adjust c_getstr/c_strlen to new STRING_CST semantic

2018-09-13 Thread Bernd Edlinger 
On 08/31/18 19:45, Bernd Edlinger wrote:
> On 08/31/18 16:42, Jeff Law wrote:
>> On 08/31/2018 01:08 AM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>>
>>> when re-testing the new STRING_CST semantic patch here:
>>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html
>>>
>>> I noticed that the (my) fix for PR 87053 does still use
>>> semantic properties of the STRING_CST that is not compatible
>>> with the new proposed STRING_CST semantics.
>>>
>>> That means that c_strlen needs to handle the case
>>> where strings are not zero terminated.
>>>
>>> When this is fixed, fortran.dg/pr45636.f90 starts to regress,
>>> because the check in gimple_fold_builtin_memory_op here:
>>>
>>>     if (tree_fits_uhwi_p (len)
>>>     && compare_tree_int (len, MOVE_MAX) <= 0
>>>     /* ???  Don't transform copies from strings with known length 
>>> this
>>>    confuses the tree-ssa-strlen.c.  This doesn't handle
>>>    the case in gcc.dg/strlenopt-8.c which is XFAILed for that
>>>    reason.  */
>>>     && !c_strlen (src, 2))
>>> does now return NULL_TREE, because the fortran string is not null 
>>> terminated.
>>> However that allows the folding which makes the fortran test case fail.
>>>
>>> I fixed that by pulling in the c_getstr patch again, and use
>>> it to make another exception for string constants without any embedded NUL.
>>> That is what tree-ssa-forwprop.c can handle, and should make this work in
>>> fortran again.
>>>
>>>
>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
>> I've gone the rounds on pr45636 a couple times and it's part of the
>> reason why there's a FIXME in the bits I committed earlier this week.
>>
> 
> Yes, the rationale here is that tree-ssa-forwprop will likely choke if there
> is a NUL byte in the string:
> 
>    /* Neither builtin_strncpy_read_str nor builtin_memcpy_read_str
>   handle embedded '\0's.  */
>    if (strlen (src_buf) != src_len)
>      break;
> 
> Prior to this the c_strlen (src, 2) returned 2, thus this folding was not 
> done,
> but since the string does not contain any NUL bytes this returns now NULL.
> However it is easy to add an exception that triggers only in a fortran string
> in this way.
> 
>> I'll look at this closely in conjunction with the (unposted) patch which
>> addresses the FIXME.
>>
>> Jeff
>>
> 

Hi,

this is a minor update to the previous patch version, in that it adds
the following to c_getstr, in order to be bootstrapped with or without
the other STRING_CST-v2 semantic patches:

@@ -14611,6 +14611,10 @@ c_getstr (tree src, unsigned HOST_WIDE_I
unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src);
unsigned HOST_WIDE_INT string_size = tree_to_uhwi (mem_size);
  
+  /* Ideally this would turn into a gcc_checking_assert over time.  */
+  if (string_length > string_size)
+string_length = string_size;
+
const char *string = TREE_STRING_POINTER (src);
  
if (string_length == 0


So this patch can be boot-strapped alone,
or together with the following patches:

[PATCHv2] Handle overlength strings in the C FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01566.html

[PATCHv2] Handle overlength strings in C++ FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01567.html
=> Apporoved, without the part in vtable-class-hierarchy.c (!)

[PATCHv2] Handle overlength string literals in the fortan FE
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01568.html
=> Approved.

[PATCH] Check the STRING_CSTs in varasm.c
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01569.html




Bernd.
2018-08-30  Bernd Edlinger  

	* builtins.c (c_strlen): Handle not zero terminated STRING_CSTs
	correctly.
	* fold-const.c (c_getstr): Fix function comment.  Remove unused third
	argument.  Fix range checks.
	* fold-const.h (c_getstr): Adjust protoype.
	* gimple-fold.c (gimple_fold_builtin_memory_op): Avoid folding when
	string is constant but contains no NUL byte.

diff -Npur gcc/builtins.c gcc/builtins.c
--- gcc/builtins.c	2018-08-30 08:21:13.0 +0200
+++ gcc/builtins.c	2018-08-30 21:46:11.155211333 +0200
@@ -604,12 +604,12 @@ c_strlen (tree src, int only_value, unsi
  In that case, the elements of the array after the terminating NUL are
  all NUL.  */
   HOST_WIDE_INT strelts = TREE_STRING_LENGTH (src);
-  strelts = strelts / eltsize - 1;
+  strelts = strelts / eltsize;
 
   if (!tree_fits_uhwi_p (memsize))
 return NULL_TREE;
 
-  HOST_WIDE_INT maxelts = tree_to_uhwi (memsize) / eltsize - 1;
+  HOST_WIDE_INT maxelts = tree_to_uhwi (memsize) / eltsize;
 
   /* PTR can point to the byte representation of any string type, including
  char* and wchar_t*.  */
@@ -617,10 +617,6 @@ c_strlen (tree src, int only_value, unsi
 
   if (byteoff && TREE_CODE (byteoff) != INTEGER_CST)
 {
-  /* For empty strings the result should be zero.  */
-  if (maxelts == 0)
-	return ssize_int (0);
-
   /* The code below works only

[PATCH] Add myself to MAINTAINERS

2018-09-13 Thread Ilya Leoshkevich 
2018-09-13  Ilya Leoshkevich  

* MAINTAINERS: (write after approval): Add myself.
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c90437180d..f8838fa8e8c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -462,6 +462,7 @@ Georg-Johann Lay
 Vlad Lazar 
 Marc Lehmann   
 James Lemke
+Ilya Leoshkevich   
 Kriang Lerdsuwanakij   
 Renlin Li  
 Xinliang David Li  
-- 
2.19.0

Re: [PATCH v4 01/10] Initial TI PRU GCC port

2018-09-13 Thread Richard Sandiford 
Dimitar Dimitrov  writes:
> +; Specialized IOR/AND patterns for matching setbit/clearbit instructions.
> +;
> +; TODO - allow clrbit and setbit to support (1 << REG) constructs
> +
> +(define_insn "clearbit__"
> +  [(set (match_operand:EQD 0 "register_operand"  "=r")

Nit: stray tab instead of space.

> + (and:EQD
> +   (zero_extend:EQD
> + (match_operand:EQS0 1 "register_operand" "r"))
> +   (match_operand:EQD 2 "single_zero_operand" "n")))]
> +  ""
> +  "clr\\t%0, %1, %V2"
> +  [(set_attr "type" "alu")
> +   (set_attr "length" "4")])

Very minor suggestion, doesn't need to hold up acceptance, but: some
patterns (like this one) have an explicit length of 4, but others have
an implicit length of 4 thanks to the default value in the define_attr.
It would be more consistent to do one or the other everywhere.

Using EQD and EQS0 like this has the unfortunate effect of creating
patterns like:

   ... (zero_extend:SI (match_operand:SI ...)) ...

(which you rely on for the define_substs) and:

   ... (zero_extend:HI (match_operand:SI ...)) ...

even though these are both invalid rtl.  That said, the rtl passes
should never generate this kind of rtl (i.e. they shouldn't rely on
targets to reject it), so that's probably fine in practice.  It just
adds a bit of bloat.  I also don't have a good suggestion how to fix it
without more infrastructure.  Adding:

  GET_MODE_SIZE (mode) >= GET_MODE_SIZE (mode)

should cause gencondmd to remove the cases in which the zero_extend
result is narrower than the input, but doing that everywhere would
be ugly, and would still leave the case in which the zero_extend
result is the same size as the input (which you can't remove without
breaking the define_substs).

So after all that, I think this is fine as-is.

> +(define_insn 
> "sub_impl_"
> +  [(set (match_operand:EQD 0 "register_operand" "=r,r,r")
> + (minus:EQD
> +  (zero_extend:EQD
> +   (match_operand:EQS0 1 "reg_or_ubyte_operand" "r,r,I"))
> +  (zero_extend:EQD
> +   (match_operand:EQS1 2 "reg_or_ubyte_operand" "r,I,r"]
> +  ""
> +  "@
> +   sub\\t%0, %1, %2
> +   sub\\t%0, %1, %2
> +   rsb\\t%0, %2, %1"
> +  [(set_attr "type" "alu")
> +   (set_attr "length" "4")])

By convention, subtraction patterns shouldn't accept constants for
operand 2.  Constants should instead be subtracted using an addition
of the negative.

> +(define_constraint "I"
> +  "An unsigned 8-bit constant."
> +  (and (match_code "const_int")
> +   (match_test "UBYTE_INT (ival)")))

As it stands this will reject QImode constants with the top bit set,
since const_ints are always stored in sign-extended form.  E.g. QImode
128 is stored as (const_int -128) rather than (const_int 128).

Unfortunately this is difficult to fix in a clean way, since
const_ints don't store their mode (a long-standing wart) and unlike
predicates, constraints aren't given a mode from context.  The best
way I can think of coping with it is:

a) have a separate constraint for -128...127
b) add a define_mode_attr that maps QI to the new constraint and
   HI and SI to I
c) use  etc. instead of I in the match_operands

Similar comment for "J" and HImode, although you already have the
"N" as the corresponding signed constraint and so don't need a new one.

> +(define_predicate "const_1_operand"
> +  (and (match_code "const_int")
> +   (match_test "IN_RANGE (INTVAL (op), 1, 1)")))

INTVAL (op) == 1 seems more obvious.

> +(define_predicate "const_ubyte_operand"
> +  (and (match_code "const_int")
> +   (match_test "IN_RANGE (INTVAL (op), 0, 0xff)")))
> +
> +(define_predicate "const_uhword_operand"
> +  (and (match_code "const_int")
> +   (match_test "IN_RANGE (INTVAL (op), 0, 0x)")))

Here you can use "INTVAL (op) & GET_MODE_MASK (mode)" to avoid the
problem above, as long as you always pass a mode to these predicates.

> +(define_predicate "ctable_addr_operand"
> +  (and (match_code "const_int")
> +   (match_test ("(pru_get_ctable_base_index (INTVAL (op)) >= 0)"
> +
> +(define_predicate "ctable_base_operand"
> +  (and (match_code "const_int")
> +   (match_test ("(pru_get_ctable_exact_base_index (INTVAL (op)) >= 
> 0)"

Redundant brackets around the match_test strings.

(I want to rip out that syntax at some point, since allowing brackets
around strings just makes the files harder for scripting tools to parse.)

> +;; Return true if OP is a text segment reference.
> +;; This is needed for program memory address expressions.  Borrowed from AVR.
> +(define_predicate "text_segment_operand"
> +  (match_code "code_label,label_ref,symbol_ref,plus,minus,const")
> +{
> +  switch (GET_CODE (op))
> +{
> +case CODE_LABEL:
> +  return true;
> +case LABEL_REF :
> +  return true;
> +case SYMBOL_REF :
> +  return SYMBOL_REF_FUNCTION_P (op);
> +case PLUS :
> +case MINUS :
> +  /* Assume canonical format of symbol + constant.
> +  Fall through.  */
> +case CONST :
> +

Re: [PATCH, ARM] PR85434: Prevent spilling of stack protector guard's address on ARM

2018-09-13 Thread Thomas Preudhomme 
Hi all,

Ping? This new version changes both the middle-end and back-end part
so will need a review for both of those.

Best regards,

Thomas
On Wed, 29 Aug 2018 at 11:07, Thomas Preudhomme
 wrote:
>
> Forgot another important change in ARM backend:
>
> The expander were causing one too many indirection which was what
> caused the test failure in glibc. The new expanders code skip the
> creation of a move from the memory reference of the guard's address to
> a register since this is done in the insn themselves. I think during
> the initial implementation of the first version of the patch I had
> issues with loading the address and used that to load the address. As
> can be seen from the absence of regression on the runtime stack
> protector test in glibc, this is now working properly, also confirmed
> by manual inspection of the code.
>
> I've attached the interdiff from previous version for reference.
>
> Best regards,
>
> Thomas
> On Wed, 29 Aug 2018 at 10:51, Thomas Preudhomme
>  wrote:
> >
> > Resend hopefully without HTML this time.
> >
> > On Wed, 29 Aug 2018 at 10:49, Thomas Preudhomme
> >  wrote:
> > >
> > > Hi,
> > >
> > > I've reworked the patch fixing PR85434 (spilling of stack protector 
> > > guard's address on ARM) to address the testsuite regression on powerpc 
> > > and x86 as well as glibc testsuite regression on ARM. Issues were due to 
> > > unconditionally attempting to generate the new patterns. The code now 
> > > tests if there is a pattern for them for the target before generating 
> > > them. In the ARM side of the patch, I've also added a more specific 
> > > predicate for the new patterns. The new patch is found below.
> > >
> > >
> > > In case of high register pressure in PIC mode, address of the stack
> > > protector's guard can be spilled on ARM targets as shown in PR85434,
> > > thus allowing an attacker to control what the canary would be compared
> > > against. ARM does lack stack_protect_set and stack_protect_test insn
> > > patterns, defining them does not help as the address is expanded
> > > regularly and the patterns only deal with the copy and test of the
> > > guard with the canary.
> > >
> > > This problem does not occur for x86 targets because the PIC access and
> > > the test can be done in the same instruction. Aarch64 is exempt too
> > > because PIC access insn pattern are mov of UNSPEC which prevents it from
> > > the second access in the epilogue being CSEd in cse_local pass with the
> > > first access in the prologue.
> > >
> > > The approach followed here is to create new "combined" set and test
> > > standard pattern names that take the unexpanded guard and do the set or
> > > test. This allows the target to use an opaque pattern (eg. using UNSPEC)
> > > to hide the individual instructions being generated to the compiler and
> > > split the pattern into generic load, compare and branch instruction
> > > after register allocator, therefore avoiding any spilling. This is here
> > > implemented for the ARM targets. For targets not implementing these new
> > > standard pattern names, the existing stack_protect_set and
> > > stack_protect_test pattern names are used.
> > >
> > > To be able to split PIC access after register allocation, the functions
> > > had to be augmented to force a new PIC register load and to control
> > > which register it loads into. This is because sharing the PIC register
> > > between prologue and epilogue could lead to spilling due to CSE again
> > > which an attacker could use to control what the canary gets compared
> > > against.
> > >
> > > ChangeLog entries are as follows:
> > >
> > > *** gcc/ChangeLog ***
> > >
> > > 2018-08-09  Thomas Preud'homme  
> > >
> > > * target-insns.def (stack_protect_combined_set): Define new standard
> > > pattern name.
> > > (stack_protect_combined_test): Likewise.
> > > * cfgexpand.c (stack_protect_prologue): Try new
> > > stack_protect_combined_set pattern first.
> > > * function.c (stack_protect_epilogue): Try new
> > > stack_protect_combined_test pattern first.
> > > * config/arm/arm.c (require_pic_register): Add pic_reg and compute_now
> > > parameters to control which register to use as PIC register and force
> > > reloading PIC register respectively.  Insert in the stream of insns if
> > > possible.
> > > (legitimize_pic_address): Expose above new parameters in prototype and
> > > adapt recursive calls accordingly.
> > > (arm_legitimize_address): Adapt to new legitimize_pic_address
> > > prototype.
> > > (thumb_legitimize_address): Likewise.
> > > (arm_emit_call_insn): Adapt to new require_pic_register prototype.
> > > * config/arm/arm-protos.h (legitimize_pic_address): Adapt to prototype
> > > change.
> > > * config/arm/predicated.md (guard_operand): New predicate.
> > > * config/arm/arm.md (movsi expander): Adapt to legitimize_pic_address
> > > prototype change.
> > > (stack_protect_combined_set):

Re: [PATCH] DWARF: add DW_AT_count to zero-length arrays

2018-09-13 Thread Tom de Vries 
On 8/17/18 6:29 AM, Omar Sandoval wrote:
> I don't have commit rights (first time contributor), so if this change is okay
> could it please be applied?

Hi,

thanks for the patch, I've committed the approved version.

[ In case you don't have one already ... ] if you want to continue
contributing, it's a good idea to get a copyright assignment in place (
https://gcc.gnu.org/contribute.html#legal ).

Thanks,
- Tom

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
On 13 September 2018 at 13:03, Jakub Jelinek  wrote:
> On Thu, Sep 13, 2018 at 12:56:59PM +0300, Ville Voutilainen wrote:
>> +   else if (!TYPE_HAS_CONST_COPY_CTOR (class_type))
>> + TYPE_HAS_COMPLEX_COPY_CTOR (class_type) += 2;
>> + }
>> +   else if (ctor > 1 && TYPE_HAS_COMPLEX_COPY_CTOR (class_type) == 2)
>> + TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 0;
>
> How does this work when:
>   unsigned has_complex_copy_ctor : 1;


It doesn't. I need more bits. Luckily, we have some available.

Re: [PING][PATCH] DWARF: add DW_AT_count to zero-length arrays

2018-09-13 Thread Richard Biener 
On Thu, 13 Sep 2018, Tom de Vries wrote:

> On 9/4/18 5:59 PM, Tom de Vries wrote:
> > [ Adding Jason as addressee ]
> > 
> > On 08/28/2018 08:20 PM, Omar Sandoval wrote:
> >> On Fri, Aug 17, 2018 at 12:16:07AM -0700, Omar Sandoval wrote:
> >>> On Thu, Aug 16, 2018 at 11:54:53PM -0700, Omar Sandoval wrote:
>  On Thu, Aug 16, 2018 at 10:27:48PM -0700, Andrew Pinski wrote:
> > On Thu, Aug 16, 2018 at 9:29 PM Omar Sandoval  
> > wrote:
> >> Hi,
> >>
> >> This fixes the issue that it is impossible to distinguish a 
> >> zero-length array
> >> type from a flexible array type given the DWARF produced by GCC (which 
> >> I
> >> reported here [1]). We do so by adding a DW_AT_count attribute with a 
> >> value of
> >> zero only for zero-length arrays (this is what clang does in this 
> >> case, too).
> >>
> >> 1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985
> >>
> >> The reproducer from the PR now produces the expected output:
> >>
> >> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c
> >> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c
> >> $ gdb -batch -ex 'ptype baz' zero_length.o
> >> type = struct {
> >> int foo;
> >> int bar[0];
> >> }
> >> $ gdb -batch -ex 'ptype baz' flexible.o
> >> type = struct {
> >> int foo;
> >> int bar[];
> >> }
> >>
> >> This was bootstrapped and tested on x86_64-pc-linux-gnu.
> >>
> >> I don't have commit rights (first time contributor), so if this change 
> >> is okay
> >> could it please be applied?
> >>> [snip]
> >>>
> >>> Here's the patch with the is_c () helper instead.
> >>>
> >>> 2018-08-17  Omar Sandoval  
> >>>
> > I've added the PR number here.
> > 
> >>>   * dwarf2out.c (is_c): New.
> >>>   (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays.
> >>>
> >>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> >>> index 5a74131d332..189f9bb381f 100644
> >>> --- a/gcc/dwarf2out.c
> >>> +++ b/gcc/dwarf2out.c
> >>> @@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum 
> >>> dwarf_attribute);
> >>>  static int get_AT_flag (dw_die_ref, enum dwarf_attribute);
> >>>  static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute);
> >>>  static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
> >>> +static bool is_c (void);
> >>>  static bool is_cxx (void);
> >>>  static bool is_cxx (const_tree);
> >>>  static bool is_fortran (void);
> >>> @@ -5434,6 +5435,19 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute 
> >>> attr_kind)
> >>>return a ? AT_file (a) : NULL;
> >>>  }
> >>>  
> >>> +/* Return TRUE if the language is C.  */
> >>> +
> >>> +static inline bool
> >>> +is_c (void)
> >>> +{
> >>> +  unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
> >>> +
> >>> +  return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99
> >>> +   || lang == DW_LANG_C11 || lang == DW_LANG_ObjC);
> >>> +
> >>> +
> >>> +}
> >>> +
> >>>  /* Return TRUE if the language is C++.  */
> >>>  
> >>>  static inline bool
> >>> @@ -20918,12 +20932,24 @@ add_subscript_info (dw_die_ref type_die, tree 
> >>> type, bool collapse_p)
> >>>  dimension arr(N:*)
> >>>Since the debugger is definitely going to need to know N
> >>>to produce useful results, go ahead and output the lower
> >>> -  bound solo, and hope the debugger can cope.  */
> >>> +  bound solo, and hope the debugger can cope.
> >>> +
> >>> +  For C and C++, if upper is NULL, this may be a zero-length array
> >>> +  or a flexible array; we'd like to be able to distinguish between
> >>> +  the two.  Set DW_AT_count to 0 for the former.  TYPE_SIZE is NULL
> >>> +  for the latter.  */
> >>>
> > I found inserting this comment here confusing, I've moved it down and
> > shortened it.
> > 
> >>> if (!get_AT (subrange_die, DW_AT_lower_bound))
> >>>   add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL);
> >>> -   if (upper && !get_AT (subrange_die, DW_AT_upper_bound))
> >>> - add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
> >>> +   if (!get_AT (subrange_die, DW_AT_upper_bound)
> >>> +   && !get_AT (subrange_die, DW_AT_count))
> >>> + {
> >>> +   if (upper)
> >>> + add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
> >>> +   else if ((is_c () || is_cxx ()) && TYPE_SIZE (type))
> >>> + add_bound_info (subrange_die, DW_AT_count,
> >>> + build_int_cst (TREE_TYPE (lower), 0), NULL);
> >>> + }
> >>>   }
> >>>  
> >>>/* Otherwise we have an array type with an unspecified length.  The
> >> Ping. Does this version look okay for trunk?
> >>
> > Looks ok to me (but I can't approve).
> > 
> > Also, I've added a testcase.
> > 
> > OK for trunk?
> > 
> 
> Ping.
> 
> Thanks,
> - Tom
> > 
> > 
> >

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Jakub Jelinek 
On Thu, Sep 13, 2018 at 12:56:59PM +0300, Ville Voutilainen wrote:
> +   else if (!TYPE_HAS_CONST_COPY_CTOR (class_type))
> + TYPE_HAS_COMPLEX_COPY_CTOR (class_type) += 2;
> + }
> +   else if (ctor > 1 && TYPE_HAS_COMPLEX_COPY_CTOR (class_type) == 2)
> + TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 0;

How does this work when:
  unsigned has_complex_copy_ctor : 1;
...
#define TYPE_HAS_COMPLEX_COPY_CTOR(NODE) (LANG_TYPE_CLASS_CHECK 
(NODE)->has_complex_copy_ctor)
?

> if (ctor > 1)
>   TYPE_HAS_CONST_COPY_CTOR (class_type) = 1;

Jakub

Re: [PATCH 04/25] SPECIAL_REGNO_P

2018-09-13 Thread Andrew Stubbs 

On 12/09/18 12:29, Andrew Stubbs wrote:

I'll report back when I've done more testing.


I reproduced the problem, in the latest sources, with the 
SPECIAL_REGNO_P patch removed (and HARD_REGNO_RENAME_OK adjusted 
accordingly).


Testcase: gcc.c-torture/compile/20020706-2.c -O3 -funroll-loops


during RTL pass: rnreg
dump file: /scratch/astubbs/amd/upstream/tmp/target.290r.rnreg
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/testsuite/gcc.c-torture/compile/20020706-2.c: In function 'crashIt': 
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/testsuite/gcc.c-torture/compile/20020706-2.c:26:1: internal compiler error: in merge_overlapping_regs, at regrename.c:300
26 | }

   | ^
0xef149d merge_overlapping_regs
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/regrename.c:300
0xef17cb find_rename_reg(du_head*, reg_class, unsigned long (*) [7], int, bool)
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/regrename.c:373
0xef1c84 rename_chains
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/regrename.c:497
0xef612b regrename_optimize
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/regrename.c:1951
0xef61ae execute
/scratch/astubbs/amd/upstream/src/gcc-gcn-master/gcc/regrename.c:1986
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.


The register that find_rename_reg is considering is SCC, which is one of 
the "special" registers.  There is a short-cut in rename_chains for 
fixed registers, global registers, and frame pointers.  It does not 
check HARD_REGNO_RENAME_OK.


Presumably the bug is not that it will actually try to rename SCC, but 
that it trips an assert while trying to compute the other parameter for 
the HARD_REGNO_RENAME_OK hook.


The SPECIAL_REGNO_P macro fixed the issue by extending the short-cut to 
include the additional registers.


The assert is caused because the def-use chains indicate that SCC 
conflicts with itself. I suppose the question is why is it doing that, 
but it's probably do do with that being a special register that gets 
used in split2 (particularly by the addptrdi3 pattern). Although, those 
patterns are careful to save SCC to one side and then restore it again 
after, so I'd have thought the DF analysis would work out?


Andrew

[PING][PATCH] DWARF: add DW_AT_count to zero-length arrays

2018-09-13 Thread Tom de Vries 
On 9/4/18 5:59 PM, Tom de Vries wrote:
> [ Adding Jason as addressee ]
> 
> On 08/28/2018 08:20 PM, Omar Sandoval wrote:
>> On Fri, Aug 17, 2018 at 12:16:07AM -0700, Omar Sandoval wrote:
>>> On Thu, Aug 16, 2018 at 11:54:53PM -0700, Omar Sandoval wrote:
 On Thu, Aug 16, 2018 at 10:27:48PM -0700, Andrew Pinski wrote:
> On Thu, Aug 16, 2018 at 9:29 PM Omar Sandoval  wrote:
>> Hi,
>>
>> This fixes the issue that it is impossible to distinguish a zero-length 
>> array
>> type from a flexible array type given the DWARF produced by GCC (which I
>> reported here [1]). We do so by adding a DW_AT_count attribute with a 
>> value of
>> zero only for zero-length arrays (this is what clang does in this case, 
>> too).
>>
>> 1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985
>>
>> The reproducer from the PR now produces the expected output:
>>
>> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c
>> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c
>> $ gdb -batch -ex 'ptype baz' zero_length.o
>> type = struct {
>> int foo;
>> int bar[0];
>> }
>> $ gdb -batch -ex 'ptype baz' flexible.o
>> type = struct {
>> int foo;
>> int bar[];
>> }
>>
>> This was bootstrapped and tested on x86_64-pc-linux-gnu.
>>
>> I don't have commit rights (first time contributor), so if this change 
>> is okay
>> could it please be applied?
>>> [snip]
>>>
>>> Here's the patch with the is_c () helper instead.
>>>
>>> 2018-08-17  Omar Sandoval  
>>>
> I've added the PR number here.
> 
>>> * dwarf2out.c (is_c): New.
>>> (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays.
>>>
>>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>>> index 5a74131d332..189f9bb381f 100644
>>> --- a/gcc/dwarf2out.c
>>> +++ b/gcc/dwarf2out.c
>>> @@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum 
>>> dwarf_attribute);
>>>  static int get_AT_flag (dw_die_ref, enum dwarf_attribute);
>>>  static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute);
>>>  static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
>>> +static bool is_c (void);
>>>  static bool is_cxx (void);
>>>  static bool is_cxx (const_tree);
>>>  static bool is_fortran (void);
>>> @@ -5434,6 +5435,19 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute 
>>> attr_kind)
>>>return a ? AT_file (a) : NULL;
>>>  }
>>>  
>>> +/* Return TRUE if the language is C.  */
>>> +
>>> +static inline bool
>>> +is_c (void)
>>> +{
>>> +  unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language);
>>> +
>>> +  return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99
>>> + || lang == DW_LANG_C11 || lang == DW_LANG_ObjC);
>>> +
>>> +
>>> +}
>>> +
>>>  /* Return TRUE if the language is C++.  */
>>>  
>>>  static inline bool
>>> @@ -20918,12 +20932,24 @@ add_subscript_info (dw_die_ref type_die, tree 
>>> type, bool collapse_p)
>>>dimension arr(N:*)
>>>  Since the debugger is definitely going to need to know N
>>>  to produce useful results, go ahead and output the lower
>>> -bound solo, and hope the debugger can cope.  */
>>> +bound solo, and hope the debugger can cope.
>>> +
>>> +For C and C++, if upper is NULL, this may be a zero-length array
>>> +or a flexible array; we'd like to be able to distinguish between
>>> +the two.  Set DW_AT_count to 0 for the former.  TYPE_SIZE is NULL
>>> +for the latter.  */
>>>
> I found inserting this comment here confusing, I've moved it down and
> shortened it.
> 
>>>   if (!get_AT (subrange_die, DW_AT_lower_bound))
>>> add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL);
>>> - if (upper && !get_AT (subrange_die, DW_AT_upper_bound))
>>> -   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
>>> + if (!get_AT (subrange_die, DW_AT_upper_bound)
>>> + && !get_AT (subrange_die, DW_AT_count))
>>> +   {
>>> + if (upper)
>>> +   add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL);
>>> + else if ((is_c () || is_cxx ()) && TYPE_SIZE (type))
>>> +   add_bound_info (subrange_die, DW_AT_count,
>>> +   build_int_cst (TREE_TYPE (lower), 0), NULL);
>>> +   }
>>> }
>>>  
>>>/* Otherwise we have an array type with an unspecified length.  The
>> Ping. Does this version look okay for trunk?
>>
> Looks ok to me (but I can't approve).
> 
> Also, I've added a testcase.
> 
> OK for trunk?
> 

Ping.

Thanks,
- Tom
> 
> 
> 0001-debug-DWARF-add-DW_AT_count-to-zero-length-arrays.patch
> 
> [debug] DWARF: add DW_AT_count to zero-length arrays
> 
> 2018-09-04  Omar Sandoval  
>   Tom de Vries  
> 
>   PR debug/86985
>   * dwarf2out.c (is_c): New function.
>   (add_subscript_info): Add DW_AT_count of 0 for C zero-length

Re: [C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
On 13 September 2018 at 12:08, Ville Voutilainen
 wrote:
> Curses.. the resetting is over-eager; we might have a non-trivial base
> or a member, and in those cases we shouldn't
> reset the triviality when we see a non-user-provided const copy. I
> think I'll hack around this with a non 0/1 value. :)

Testing the attached. I think it might need a comment, and I'm not
sure how to word it.
Here are some options:

1) /* Yo dawg, we heard you'd like another bit, so we added a second
bit to your bit
so you can read the other bit when you read your bit */
2) /* I'm vewy vewy sowwy */
3) /* We need to attach another bit to the TYPE_HAS_COMPLEX_COPY_CTOR
bit, so let's
quantum-entangle the additional bit and the actual bit */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 50b60e8..ce6ac7f 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13212,7 +13212,14 @@ grok_special_member_properties (tree decl)
 	 default arguments.  */
 	  TYPE_HAS_COPY_CTOR (class_type) = 1;
 	  if (user_provided_p (decl))
-	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	{
+	  if (ctor > 1)
+		TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	  else if (!TYPE_HAS_CONST_COPY_CTOR (class_type))
+		TYPE_HAS_COMPLEX_COPY_CTOR (class_type) += 2;
+	}
+	  else if (ctor > 1 && TYPE_HAS_COMPLEX_COPY_CTOR (class_type) == 2)
+	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 0;
 	  if (ctor > 1)
 	TYPE_HAS_CONST_COPY_CTOR (class_type) = 1;
 	}
diff --git a/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
new file mode 100644
index 000..177eb2e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++11 } }
+
+// PR c++/87051
+
+struct M {
+  M(const M&) = default;
+  M(M&);
+};
+
+struct M2 {
+  M2(M2&);
+  M2(const M2&) = default;
+};
+
+static_assert( __is_trivially_constructible(M, M&&), "");
+static_assert( __is_trivially_constructible(M2, M2&&), "");

Re: [GCC][PATCH v2][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks

2018-09-13 Thread Kyrill Tkachov 



On 13/09/18 10:25, Sam Tebbs wrote:


On 09/11/2018 04:20 PM, James Greenhalgh wrote:
> On Tue, Sep 04, 2018 at 10:13:43AM -0500, Sam Tebbs wrote:
>> Hi James,
>>
>> Thanks for the feedback. Here is an update with the changes you proposed
>> and an updated changelog.
>>
>> gcc/
>> 2018-09-04  Sam Tebbs  
>>
>>   PR target/85628
>>   * config/aarch64/aarch64.md (*aarch64_bfxil):
>>   Define.
>>   * config/aarch64/constraints.md (Ulc): Define
>>   * config/aarch64/aarch64-protos.h (aarch64_high_bits_all_ones_p):
>>   Define.
>>   * config/aarch64/aarch64.c (aarch64_high_bits_all_ones_p): New 
function.
>>
>> gcc/testsuite
>> 2018-09-04  Sam Tebbs  
>>
>>   PR target/85628
>>   * gcc.target/aarch64/combine_bfxil.c: New file.
>>   * gcc.target/aarch64/combine_bfxil_2.c: New file.
>>
>>
> 
>
>> +/* Return true if I's bits are consecutive ones from the MSB.  */
>> +bool
>> +aarch64_high_bits_all_ones_p (HOST_WIDE_INT i)
>> +{
>> +  return exact_log2(-i) != HOST_WIDE_INT_M1;
>> +}
> You need a space in here between the function name and the bracket:
>
>exact_log2 (-i)
>
>
>> +extern void abort(void);
> The same comment applies multiple places in this file.
>
> Likewise; if (
>
> Otherwise, OK, please apply with those fixes.
>
> Thanks,
> James

Thanks for noticing that, here's the fixed version.



Thanks Sam, I've committed the patch on your behalf with r264264.
If you want to get write-after-approval access to the SVN repo to commit 
patches yourself in the future
please fill out the form at https://sourceware.org/cgi-bin/pdw/ps_form.cgi 
putting my address from the MAINTAINERS file as the approver.

Kyrill


Sam

Re: [GCC][PATCH v2][Aarch64] Exploiting BFXIL when OR-ing two AND-operations with appropriate bitmasks

2018-09-13 Thread Sam Tebbs 


On 09/11/2018 04:20 PM, James Greenhalgh wrote:

On Tue, Sep 04, 2018 at 10:13:43AM -0500, Sam Tebbs wrote:

Hi James,

Thanks for the feedback. Here is an update with the changes you proposed
and an updated changelog.

gcc/
2018-09-04  Sam Tebbs  

  PR target/85628
  * config/aarch64/aarch64.md (*aarch64_bfxil):
  Define.
  * config/aarch64/constraints.md (Ulc): Define
  * config/aarch64/aarch64-protos.h (aarch64_high_bits_all_ones_p):
  Define.
  * config/aarch64/aarch64.c (aarch64_high_bits_all_ones_p): New 
function.

gcc/testsuite
2018-09-04  Sam Tebbs  

  PR target/85628
  * gcc.target/aarch64/combine_bfxil.c: New file.
  * gcc.target/aarch64/combine_bfxil_2.c: New file.






+/* Return true if I's bits are consecutive ones from the MSB.  */
+bool
+aarch64_high_bits_all_ones_p (HOST_WIDE_INT i)
+{
+  return exact_log2(-i) != HOST_WIDE_INT_M1;
+}

You need a space in here between the function name and the bracket:

   exact_log2 (-i)



+extern void abort(void);

The same comment applies multiple places in this file.

Likewise; if (

Otherwise, OK, please apply with those fixes.

Thanks,
James


Thanks for noticing that, here's the fixed version.

Sam
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index aae1db45ed69c14e306ccce056861a58d9acd834..b26e46f81a414bf71762527f84fd9ac38b81b829 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -624,4 +624,6 @@ rtl_opt_pass *make_pass_tag_collision_avoidance (gcc::context *);
 
 poly_uint64 aarch64_regmode_natural_size (machine_mode);
 
+bool aarch64_high_bits_all_ones_p (HOST_WIDE_INT);
+
 #endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d088ef7ee0f256ad0d4f59d2735121de2dd67eba..34acc58510110fbc2cb4abd19ec9d7a04bad3f4c 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1432,6 +1432,13 @@ aarch64_hard_regno_caller_save_mode (unsigned regno, unsigned,
 return SImode;
 }
 
+/* Return true if I's bits are consecutive ones from the MSB.  */
+bool
+aarch64_high_bits_all_ones_p (HOST_WIDE_INT i)
+{
+  return exact_log2 (-i) != HOST_WIDE_INT_M1;
+}
+
 /* Implement TARGET_CONSTANT_ALIGNMENT.  Make strings word-aligned so
that strcpy from constants will be faster.  */
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 955769a64d2030839cdb337321a808626188190e..88f66104db31320389f05cdd5d161db9992a77b8 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -5336,6 +5336,31 @@
   [(set_attr "type" "rev")]
 )
 
+(define_insn "*aarch64_bfxil"
+  [(set (match_operand:GPI 0 "register_operand" "=r,r")
+(ior:GPI (and:GPI (match_operand:GPI 1 "register_operand" "r,0")
+		(match_operand:GPI 3 "const_int_operand" "n, Ulc"))
+	(and:GPI (match_operand:GPI 2 "register_operand" "0,r")
+		(match_operand:GPI 4 "const_int_operand" "Ulc, n"]
+  "(INTVAL (operands[3]) == ~INTVAL (operands[4]))
+  && (aarch64_high_bits_all_ones_p (INTVAL (operands[3]))
+|| aarch64_high_bits_all_ones_p (INTVAL (operands[4])))"
+  {
+switch (which_alternative)
+{
+  case 0:
+	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[3])));
+	return "bfxil\\t%0, %1, 0, %3";
+  case 1:
+	operands[3] = GEN_INT (ctz_hwi (~INTVAL (operands[4])));
+	return "bfxil\\t%0, %2, 0, %3";
+  default:
+	gcc_unreachable ();
+}
+  }
+  [(set_attr "type" "bfm")]
+)
+
 ;; There are no canonicalisation rules for the position of the lshiftrt, ashift
 ;; operations within an IOR/AND RTX, therefore we have two patterns matching
 ;; each valid permutation.
diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 72cacdabdac52dcb40b480f7a5bfbf4997c742d8..31fc3eafd8bba03cc773e226223a6293c6dde8d4 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -172,6 +172,13 @@
   A constraint that matches the immediate constant -1."
   (match_test "op == constm1_rtx"))
 
+(define_constraint "Ulc"
+ "@internal
+ A constraint that matches a constant integer whose bits are consecutive ones
+ from the MSB."
+ (and (match_code "const_int")
+  (match_test "aarch64_high_bits_all_ones_p (ival)")))
+
 (define_constraint "Usv"
   "@internal
A constraint that matches a VG-based constant that can be loaded by
diff --git a/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
new file mode 100644
index ..adb0582ed9d8207f7b52c8912d03345369747448
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/combine_bfxil.c
@@ -0,0 +1,98 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps" } */
+
+extern void abort (void);
+
+unsigned long long
+combine_balanced (unsigned long long a, unsigned long long b)
+{
+  return (a &

Re: [PATCH][OBVIOUS] Close file on return from verify-intermediate

2018-09-13 Thread Joey Ye 
Committed as r264202.

gcov-8 still fails at r264226 according to
https://gcc.gnu.org/ml/gcc-testresults/2018-09/msg01478.html

So it is confirmed that this patch doesn't resolve PR85871, as Mike hoped.

Thanks,
Joey
On Mon, Sep 10, 2018 at 3:04 PM Martin Liška  wrote:
>
> On 09/05/2018 03:29 PM, Joey Ye wrote:
> > This is a fix to an obvious issue in gcov.exp, where proc 
> > verify-intermediate returns without closing the open file.
> >
> > This can be a possible fix to PR85871. gcov-8.C diffs to other gcov 
> > testcases that it invokes verify-intermediate. Not closing an open file may 
> > result in random failure quietly.
> >
> > It is only a possible fix as I failed to reproduce the PR85871 random 
> > failure in my local machine despite continuous testing of multiple days. So 
> > I cannot verify if this patch fixes the regression either.
> >
> > To verify, https://gcc.gnu.org/ml/gcc-testresults/ need to be watched 
> > whether gcov-8 regression will disappear completely one month after this 
> > patch committed to trunk.
> >
> > Tested with make check with no new regressions.
> >
> > OK to trunk?
> >
> > testsuite/ChangeLog:
> > 2018-09-05  Joey Ye  
> >
> > * lib/gcov.exp (verify-intermediate): Add missing close.
> >
>
> Hi.
>
> Thanks for the fix, it's obvious. Please install the patch.
>
> Note that gcov-8.C is built multiple times with different -std=* options:
>
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++98 (test for excess errors)
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++98 execution test
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++98  gcov
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++11 (test for excess errors)
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++11 execution test
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++11  gcov
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++14 (test for excess errors)
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++14 execution test
> PASS: g++.dg/gcov/gcov-8.C  -std=gnu++14  gcov
>
> That can cause the collisions seen in the PR.
>
> Martin

Re: [PATCH] [ARC] Fix generation of specs

2018-09-13 Thread Alexey Brodkin 
Hi Claus,

On Thu, 2018-09-13 at 09:59 +0200, Claudiu Zissulescu wrote:
> The patch is missing the entry change log. Otherwise is ok. I'll push it with 
> the mentioned changes.

Thanks for taking care.

Also may we have it back-ported to 8.x branch so it will be a part of 8.3 
release later?

-Alexey

[PATCH] [ARC] Fix generation of specs

2018-09-13 Thread Alexey Brodkin 
With no trailing space in LINK_EH_SPEC linker spec gets generated as:
>8-
%{!r:--build-id} --eh-frame-hdr%{h*} ...
>8-

or even worse if hash style is added:
>8-
%{!r:--build-id} --eh-frame-hdr--hash-style=sysv %{h*} ...
>8-

Now if that spec is really used by LD then it fails inevitably
saying that it doesn't know option "--eh-frame-hdr--hash-style=sysv".

Cc: Andrew Burgess 
Cc: Claudiu Zissulescu 
---
 gcc/config/arc/linux.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/config/arc/linux.h b/gcc/config/arc/linux.h
index 96d548eae341..62ebe4de0fc7 100644
--- a/gcc/config/arc/linux.h
+++ b/gcc/config/arc/linux.h
@@ -98,7 +98,7 @@ along with GCC; see the file COPYING3.  If not see
Signalize that because we have fde-glibc, we don't need all C shared libs
linked against -lgcc_s.  */
 #undef LINK_EH_SPEC
-#define LINK_EH_SPEC "--eh-frame-hdr"
+#define LINK_EH_SPEC "--eh-frame-hdr "
 #endif
 
 #undef SUBTARGET_CPP_SPEC
-- 
2.17.1

[C++ PATCH] PR c++/87051

2018-09-13 Thread Ville Voutilainen 
Howdy! The tricky details of copy constructors, part 76.
In this approach, I let the const-copy "dominate"; that is, if
a const-copy was found, a non-const-copy will not turn off triviality
of the copy constructor, and a const-copy will reinstate triviality
of the copy constructor even if a non-const copy removed said
triviality.

I suppose we could try moving this knowledge from the class level
to the level of special member function decl, but I wonder whether
we have the bits to do that. This approach seems to solve the
problem, certainly.

This has been tested manually on Linux-x64, running full suite
on Linux-PPC64.

Thoughts? OK for trunk if the full suite passes?

2018-09-13  Ville Voutilainen  

gcc/cp

 PR c++/87051
* decl.c (grok_special_member_properties): Don't mark the class
as having a non-trivial copy constructor if the copy constructor
we're looking at is not a const-copy and a const-copy was already
found, reset the copy constructor triviality if we found a
trivial const-copy.

testsuite/

 PR c++/87051
* g++.dg/ext/is_trivially_constructible7.C: New.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 50b60e8..1b09721 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13211,8 +13211,11 @@ grok_special_member_properties (tree decl)
 	 are no other parameters or else all other parameters have
 	 default arguments.  */
 	  TYPE_HAS_COPY_CTOR (class_type) = 1;
-	  if (user_provided_p (decl))
-	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	  if (user_provided_p (decl)
+	  && (ctor > 1 || !TYPE_HAS_CONST_COPY_CTOR (class_type)))
+		TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 1;
+	  else if (ctor > 1)
+	TYPE_HAS_COMPLEX_COPY_CTOR (class_type) = 0;
 	  if (ctor > 1)
 	TYPE_HAS_CONST_COPY_CTOR (class_type) = 1;
 	}
diff --git a/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
new file mode 100644
index 000..177eb2e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/is_trivially_constructible7.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++11 } }
+
+// PR c++/87051
+
+struct M {
+  M(const M&) = default;
+  M(M&);
+};
+
+struct M2 {
+  M2(M2&);
+  M2(const M2&) = default;
+};
+
+static_assert( __is_trivially_constructible(M, M&&), "");
+static_assert( __is_trivially_constructible(M2, M2&&), "");

Re: VRP: undefined shifting calculation should not need sign bit

2018-09-13 Thread Richard Sandiford 
Aldy Hernandez  writes:
> On 09/12/2018 12:57 PM, Richard Sandiford wrote:
>> Aldy Hernandez  writes:
>>> diff --git a/gcc/wide-int-range.h b/gcc/wide-int-range.h
>>> index 589fdea4df6..e9ee418e5b2 100644
>>> --- a/gcc/wide-int-range.h
>>> +++ b/gcc/wide-int-range.h
>>> @@ -131,7 +131,7 @@ extern bool wide_int_range_div (wide_int , 
>>> wide_int ,
>>>   /* Return TRUE if shifting by range [MIN, MAX] is undefined behavior.  */
>>>   
>>>   inline bool
>>> -wide_int_range_shift_undefined_p (signop sign, unsigned prec,
>>> +wide_int_range_shift_undefined_p (unsigned prec,
>>>   const wide_int , const wide_int )
>>>   {
>>> /* ?? Note: The original comment said this only applied to
>>> @@ -142,7 +142,7 @@ wide_int_range_shift_undefined_p (signop sign, unsigned 
>>> prec,
>>>behavior from the shift operation.  We cannot even trust
>>>SHIFT_COUNT_TRUNCATED at this stage, because that applies to rtl
>>>shifts, and the operation at the tree level may be widened.  */
>>> -  return wi::lt_p (min, 0, sign) || wi::ge_p (max, prec, sign);
>>> +  return wi::sign_mask (min) || wi::ge_p (max, prec, UNSIGNED);
>> 
>> I don't think this is a good idea.  Logically the comparison should
>> be done relative to the TYPE_SIGN of the type, so I think the original
>> code was correct.
>
> The operation to calculate undefinedness must be done with the type of 
> the RHS, as opposed to the type of the entire operation.  This can be 
> confusing, as most operations use the same type for all operands as well 
> as for the type of the entire operation.  For example, AFAICT, the 
> following is valid gimple:
>
>   UINT64 = UINT64 << INT32
>
> The original code was doing this (correctly), but since it was confusing 
> to remember which type to pass, I rewrote the above function to not need 
> the sign of the RHS.  This came about because in my ranger work, I 
> passed the wrong type which took forever to find ;-).  My patch avoids 
> further confusion.
>
> Am I missing a subtle incorrectness in my approach?

The problem is with things like UINT256 << UINT8 vs. UINT256 << INT8.
A range of [128, 131] on the UINT8 would be represented using the same
wide_ints as a range of [-128, -125] on the INT8, but the former is
well-defined while the latter isn't.  Only the TYPE_SIGN tells you
which applies.

The original code got this right, but the new code effectively assumes
all shift amounts are signed, and so would treat UINT8 like INT8.

OK, so no current target actually supports UINT256 AFAIK, so it might
be academic.  But the original point of wide-int.h was to support such
wide types, so they could become a thing in future.

Thanks,
Richard

Re: [PATCH][4/4][v2] RPO-style value-numbering for FRE/PRE

2018-09-13 Thread Richard Biener 
On Thu, 13 Sep 2018, Gerald Pfeifer wrote:

> On Mon, 10 Sep 2018, Martin Liška wrote:
> > Works for me! One needed to add --wrapper gdb81,--args. So now 
> > I see a nice back-trace.
> 
> Great! This appears to be a tough one, though breaking FreeBSD/i386
> with clang, GCC 6 and GCC 9 (two weeks old), Solaris, HP/UX, so looks
> like something around glibc vs other libcs?
> 
> 
> On FreeBSD 10.4 I tried malloc debugging setting MALLOC_CONF=zero:true
> in the environment, to zero initialize all allocations, alas that also 
> did not change anything.
> 
> Nor did MALLOC_CONF=tcache:false,junk:true.
> 
> So my guess is it's a memory allocation issue, but perhaps one level 
> higher than the system functions?
> 
> 
> Not sure how I can help; clearly not my area of expertise. :-(

If it reproduces with clang as a host compiler maybe you can
use ASAN with it?  I'm not sure if GCCs ASAN support is up to speed.
IIRC the issues all show up with the stage1 compiler...

Martin's now gone and I have to try set up a VM with FreeBSD to
debug this myself.

Richard.

Re: [PATCH][4/4][v2] RPO-style value-numbering for FRE/PRE

2018-09-13 Thread Gerald Pfeifer 
On Mon, 10 Sep 2018, Martin Liška wrote:
> Works for me! One needed to add --wrapper gdb81,--args. So now 
> I see a nice back-trace.

Great! This appears to be a tough one, though breaking FreeBSD/i386
with clang, GCC 6 and GCC 9 (two weeks old), Solaris, HP/UX, so looks
like something around glibc vs other libcs?


On FreeBSD 10.4 I tried malloc debugging setting MALLOC_CONF=zero:true
in the environment, to zero initialize all allocations, alas that also 
did not change anything.

Nor did MALLOC_CONF=tcache:false,junk:true.

So my guess is it's a memory allocation issue, but perhaps one level 
higher than the system functions?


Not sure how I can help; clearly not my area of expertise. :-(

Gerald

Re: [PATCH][Middle-end]Add a new option to finer control inlining based on function's visibility

2018-09-13 Thread Richard Biener 
On Wed, 12 Sep 2018, Alexander Monakov wrote:

> On Wed, 12 Sep 2018, Richard Biener wrote:
> > With LTO "static" is too blurry - would -finline-only-hidden be OK
> > with you?
> 
> This doesn't sound right - in non-pic, everything is hidden, effectively.
> And the intended use is with Linux kernel, which does not use -fpic.
> 
> I agree with LTO this option makes less sense, but I wouldn't expect LTO
> to be used for livepatching-capable kernels.

The issue is that with LTO this option probably cannot be reliably
implemented (well, I guess we could stick 'noinline' attributes
onto all non-static declared functions...).

Btw, what about

inline T foo() {}

in C99?  Those are not declared static (in fact there may be
extern T foo () declarations somewhere).  I also think we
have to continue to inline always-inline functions.

That is, is it really "-fimplicit-inline-only-static"?  Would
it make more sense to have a -fno-implicit-inline switch which
will not inline any function that is not declared inline?
That might be already available via -fno-inline-small-functions
[-fno-inline-functions].

Richard.

Re: [PATCH] Improve x % c == d signed expansion (PR middle-end/87290)

2018-09-13 Thread Richard Biener 
On Wed, 12 Sep 2018, Jakub Jelinek wrote:

> Hi!
> 
> This patch optimizes signed x % C1 == C2 expansion if it is beneficial.
> x % C1 == 0 should be handled unconditionally in match.pd (previous patch),
> this only handles the cases where C1 is positive power of two and abs (C2)
> is smaller than it and non-zero.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2018-09-12  Jakub Jelinek  
>   Kyrylo Tkachov  
> 
>   PR middle-end/87290
>   * expr.c (maybe_optimize_pow2p_mod_cmp): New function.
>   (maybe_optimize_mod_cmp): Use it if integer_pow2p treeop1.
> 
>   * gcc.target/i386/pr87290.c: New test.
>   * gcc.c-torture/execute/pr87290.c: New test.
> 
> --- gcc/expr.c.jj 2018-09-12 15:20:37.448807135 +0200
> +++ gcc/expr.c2018-09-12 18:16:41.0 +0200
> @@ -11523,6 +11523,98 @@ mod_inv (const wide_int , const wide_i
>return x1;
>  }
>  
> +/* Optimize x % C1 == C2 for signed modulo if C1 is a power of two and C2
> +   is non-zero and C3 ((1<<(prec-1)) | (C1 - 1)):
> +   for C2 > 0 to x & C3 == C2
> +   for C2 < 0 to x & C3 == (C2 & C3).  */
> +enum tree_code
> +maybe_optimize_pow2p_mod_cmp (enum tree_code code, tree *arg0, tree *arg1)
> +{
> +  gimple *stmt = get_def_for_expr (*arg0, TRUNC_MOD_EXPR);
> +  tree treeop0 = gimple_assign_rhs1 (stmt);
> +  tree treeop1 = gimple_assign_rhs2 (stmt);
> +  tree type = TREE_TYPE (*arg0);
> +  scalar_int_mode mode;
> +  if (!is_a  (TYPE_MODE (type), ))
> +return code;
> +  if (GET_MODE_BITSIZE (mode) != TYPE_PRECISION (type)
> +  || TYPE_PRECISION (type) <= 1
> +  || TYPE_UNSIGNED (type)
> +  /* Signed x % c == 0 should have been optimized into unsigned modulo
> +  earlier.  */
> +  || integer_zerop (*arg1)
> +  /* If c is known to be non-negative, modulo will be expanded as 
> unsigned
> +  modulo.  */
> +  || get_range_pos_neg (treeop0) == 1)
> +return code;
> +
> +  /* x % c == d where d < 0 && d <= -c should be always false.  */
> +  if (tree_int_cst_sgn (*arg1) == -1
> +  && -wi::to_widest (treeop1) >= wi::to_widest (*arg1))
> +return code;
> +
> +  int prec = TYPE_PRECISION (type);
> +  wide_int w = wi::to_wide (treeop1) - 1;
> +  w |= wi::shifted_mask (0, prec - 1, true, prec);
> +  tree c3 = wide_int_to_tree (type, w);
> +  tree c4 = *arg1;
> +  if (tree_int_cst_sgn (*arg1) == -1)
> +c4 = wide_int_to_tree (type, w & wi::to_wide (*arg1));
> +
> +  rtx op0 = expand_normal (treeop0);
> +  treeop0 = make_tree (TREE_TYPE (treeop0), op0);
> +
> +  bool speed_p = optimize_insn_for_speed_p ();
> +
> +  do_pending_stack_adjust ();
> +
> +  location_t loc = gimple_location (stmt);
> +  struct separate_ops ops;
> +  ops.code = TRUNC_MOD_EXPR;
> +  ops.location = loc;
> +  ops.type = TREE_TYPE (treeop0);
> +  ops.op0 = treeop0;
> +  ops.op1 = treeop1;
> +  ops.op2 = NULL_TREE;
> +  start_sequence ();
> +  rtx mor = expand_expr_real_2 (, NULL_RTX, TYPE_MODE (ops.type),
> + EXPAND_NORMAL);
> +  rtx_insn *moinsns = get_insns ();
> +  end_sequence ();
> +
> +  unsigned mocost = seq_cost (moinsns, speed_p);
> +  mocost += rtx_cost (mor, mode, EQ, 0, speed_p);
> +  mocost += rtx_cost (expand_normal (*arg1), mode, EQ, 1, speed_p);
> +
> +  ops.code = BIT_AND_EXPR;
> +  ops.location = loc;
> +  ops.type = TREE_TYPE (treeop0);
> +  ops.op0 = treeop0;
> +  ops.op1 = c3;
> +  ops.op2 = NULL_TREE;
> +  start_sequence ();
> +  rtx mur = expand_expr_real_2 (, NULL_RTX, TYPE_MODE (ops.type),
> + EXPAND_NORMAL);
> +  rtx_insn *muinsns = get_insns ();
> +  end_sequence ();
> +
> +  unsigned mucost = seq_cost (muinsns, speed_p);
> +  mucost += rtx_cost (mur, mode, EQ, 0, speed_p);
> +  mucost += rtx_cost (expand_normal (c4), mode, EQ, 1, speed_p);
> +
> +  if (mocost <= mucost)
> +{
> +  emit_insn (moinsns);
> +  *arg0 = make_tree (TREE_TYPE (*arg0), mor);
> +  return code;
> +}
> +
> +  emit_insn (muinsns);
> +  *arg0 = make_tree (TREE_TYPE (*arg0), mur);
> +  *arg1 = c4;
> +  return code;
> +}
> +
>  /* Attempt to optimize unsigned (X % C1) == C2 (or (X % C1) != C2).
> If C1 is odd to:
> (X - C2) * C3 <= C4 (or >), where
> @@ -11561,8 +11653,6 @@ maybe_optimize_mod_cmp (enum tree_code c
>tree treeop1 = gimple_assign_rhs2 (stmt);
>if (TREE_CODE (treeop0) != SSA_NAME
>|| TREE_CODE (treeop1) != INTEGER_CST
> -  /* x % pow2 is handled right already.  */
> -  || integer_pow2p (treeop1)
>/* Don't optimize the undefined behavior case x % 0;
>x % 1 should have been optimized into zero, punt if
>it makes it here for whatever reason;
> @@ -11572,6 +11662,11 @@ maybe_optimize_mod_cmp (enum tree_code c
>|| tree_int_cst_le (treeop1, *arg1))
>  return code;
>  
> +  /* Unsigned x % pow2 is handled right already, for signed
> + modulo handle it in maybe_optimize_pow2p_mod_cmp.  */
> +  if (integer_pow2p

Re: [PATCH, middle-end]: Default the mode of pop and swap insns to the reg_raw_mode of FIRST_STACK_REG

2018-09-13 Thread Jeff Law 
On 9/12/18 12:34 PM, Uros Bizjak wrote:
> Hello!
> 
> Although reg-stack.c is mostly an x86 affair, attached patch decouples
> it from x86 a bit. The patch changes the default mode of pop and swap
> insns to the reg_raw_mode of FIRST_STACK_REG (= XFmode on x86). Also,
> the patch explicitly constructs swap insn, without calling x86
> specific named insn pattern.
> 
> 2018-09-11  Uros Bizjak  
> 
> * reg-stack.c: Include regs.h.
> (replace_reg): Assert that mode is MODE_FLOAT or MODE_COMPLEX_FLOAT.
> (emit_pop_insn): Default pop insn mode to the reg_raw_mode of
> FIRST_STACK_REG, not DFmode.
> (emit_swap_insn): Default swap insn mode to the reg_raw_mode of
> FIRST_STACK_REG, not XFmode.  Explicitly construct swap RTX.
> (change stack): Default register mode to the reg_raw_mode of
> FIRST_STACK_REG, not DFmode.
> * config/i386/i386.md (*swap): Remove insn pattern.
> (*swapxf): Rename from swapxf.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> No functional changes, but the patch needs middle-end approval.
> 
> OK for mainline?
OK.
jeff

Re: [PATCH] Move signed to unsigned x % c == 0 optimization from fold-const to match.pd (PR tree-optimization/87287)

2018-09-13 Thread Jeff Law 
On 9/12/18 12:31 PM, Jakub Jelinek wrote:
> Hi!
> 
> While working on the next PR, I've noticed we only fold this on generic, not
> gimple.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> 2018-09-12  Jakub Jelinek  
> 
>   PR tree-optimization/87287
>   * fold-const.c (fold_binary_loc) : Move signed modulo
>   X % C == 0 to X % (unsigned) C == 0 optimization to ...
>   * match.pd (X % C == 0): ... here.  New optimization.
> 
>   * gcc.dg/tree-ssa/pr87287.c: New test.
OK.
jeff