Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438

2018-11-19 Thread Eric Botcazou
> The blockage was introduced as a fix for PR14381 [1] in r79265 [2].
> Later, the blockage was moved after return label as a fix for PR25176
> [3] in r107871 [4].
> 
> After that, r122626 [5] moves the blockage after the label for the
> naked return from the function. Relevant posts from gcc-patches@ ML
> are at [6], [7]. However, in the posts, there are no concrete
> examples, how scheduler moves instructions from different BB around
> blockage insn, the posts just show that there is a jump around
> blockage when __builtin_return is used. I was under impression that
> scheduler is unable to move instructions over BB boundaries.

The scheduler works on extended basic blocks.  The [7] post gives a rather 
convincing explanation and there is a C++ testcase under PR rtl-opt/14381.

> A mystery is the tree-ssa merge [8] that copies back the hunk, moved
> in r122626 [5] to its original position. From this revision onwards,
> we emit two blockages.

It's the dataflow merge, not the tree-ssa merge.  The additional blockage 
might be needed for DF.

Given that the current PR is totally artificial, I think that we need to be 
quite conservative and only do something on mainline.  And even there I'd be 
rather conservative and remove the kludge only for targets that emit unwind 
information in the epilogue (among which there is x86 I presume).

-- 
Eric Botcazou


[RS6000] num_insns_constant ICE

2018-11-19 Thread Alan Modra
This patch came about from investigating an ICE that appeared when I
was retesting an old half-baked patch of mine to rs6000_rtx_costs.
The num_insns_constant ICE was introduced with git commit f337168d97.
If a const_double is fed to rs6000_is_valid_and_mask and from there to
rs6000_is_valid_mask where INTVAL is used, gcc will ICE.

However, the code was buggy before that.  When const_double no longer
was used for integers there was no point in testing for a mask since
the mask predicates only handle const_int.  In fact, I don't think the
function ever handled floating point constants that might match a load
of minus one and mask.  It does now.  I've added a few comments
regarding splitters so the next person looking at this code can see
how this works.

I also looked at all places (*) where num_insns_constant is called.
That lead to finding two cases where the "G" and "H" constraints were
used incorrectly.  Their purpose is calculating insn lengths.  Thus it
never makes sense to put "GH" together or with "F" in an insn
alternative.  movdi_internal32 used "GHF" in an alternative (and
always selected 64 byte insn length) so I replaced that with "F".
The FMOVE128 version of mov_softfloat also had "GHF" in an
alternative but from what I see in rs6000_emit_move, I believe TFmode
insns loading constants will be splt.  So this insn doesn't need to
handle FP constants (it also doesn't need an iterator as only TFmode
will be selected - not fixed with this patch).

*) Besides the G and H constraint use of num_insns_constant, there
are three uses of num_insns_constant in rs6000.md each only dealing
with const_int or const_wide_int operands.  The rs6000.c call in
rs6000_emit_move only deals with const_int, so no const_double code is
touched.  easy_fp_constant in predicates.md does appear to need a
num_insns_constant that handles const_double in DImode.
easy_fp_constant is called from easy_vector_constant, input_operand
and vector.md mov pattern but not with a DImode operand.  In rs6000.c,
the first call in rs6000_emit_move is also not DImode.  The second
call might indeed match, so there's one possible use.  The
easy_fp_constant call in rs6000_legitimate_constant_p also might call
num_insns_constant, but only in 32-bit mode.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux.  I'm running the powerpc64-linux tests again as I
forgot to test -m32 on the first run.  OK for trunk, assuming the -m32
tests don't regress?

* config/rs6000/rs6000.c (num_insns_constant_wide): Make static.
(num_insns_constant ): Don't ICE on call to
rs6000_is_valid_and_mask.  Formatting.  Simplify and extract
code common to both CONST_INT and CONST_DOUBLE.  Handle
floating point constants in SImode/DImode.  Add gcc_unreachable
for unhandled const_double modes.
* config/rs6000/rs6000-protos.h (num_insns_const_wide): Delete.
* config/rs6000/rs6000.md (movdi_internal32): Remove "GH" from
alternative handling "F".
(mov_softfloat ): Delete "GHF" from alternative.
* config/rs6000/contraints.md (G, H): Comment.

diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
index 90aba1e77d3..21c7a808919 100644
--- a/gcc/config/rs6000/constraints.md
+++ b/gcc/config/rs6000/constraints.md
@@ -252,7 +252,8 @@ (define_constraint "P"
   (and (match_code "const_int")
(match_test "((- (unsigned HOST_WIDE_INT) ival) + 0x8000) < 0x1")))
 
-;; Floating-point constraints
+;; Floating-point constraints.  These two are defined so that insn
+;; length attributes can be calculated exactly.
 
 (define_constraint "G"
   "Constant that can be copied into GPR with two insns for DF/DI
diff --git a/gcc/config/rs6000/rs6000-protos.h 
b/gcc/config/rs6000/rs6000-protos.h
index 1dfe7995ff1..dfee1f28aa9 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -36,7 +36,6 @@ extern int vspltis_shifted (rtx);
 extern HOST_WIDE_INT const_vector_elt_as_int (rtx, unsigned int);
 extern bool macho_lo_sum_memory_operand (rtx, machine_mode);
 extern int num_insns_constant (rtx, machine_mode);
-extern int num_insns_constant_wide (HOST_WIDE_INT);
 extern int small_data_operand (rtx, machine_mode);
 extern bool mem_operand_gpr (rtx, machine_mode);
 extern bool mem_operand_ds_form (rtx, machine_mode);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 479dae547fa..1eabf9cd612 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5818,7 +5818,7 @@ direct_return (void)
 /* Return the number of instructions it takes to form a constant in an
integer register.  */
 
-int
+static int
 num_insns_constant_wide (HOST_WIDE_INT value)
 {
   /* signed constant loadable with addi */
@@ -5856,16 +5856,13 @@ num_insns_constant_wide (HOST_WIDE_INT value)
 int
 num_insns_constant (rtx op, machine_mode mode)
 {
-  HOST_WIDE_INT low, high;
+  HOST_WIDE_INT val;
 
   switch (GET_CODE (

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

2018-11-19 Thread Umesh Kalappa
Hi Jakub ,

the attached patch is good to commit ?

Thank you
~Umesh
On Mon, Nov 19, 2018 at 4:00 PM Lokesh Janghel
 wrote:
>
> Thank you Jakub, I update the patch with your comments and tested it.
> Please let me know your thoughts/suggestions.
>
>
> Thanks
> Lokesh
>
> On Fri, Nov 16, 2018 at 4:57 PM Jakub Jelinek  wrote:
>>
>> On Fri, Nov 16, 2018 at 04:21:25PM +0530, Umesh Kalappa wrote:
>> > My bad ,
>> > attached the same now .
>>
>> +2018-11-15  Lokesh Janghel 
>>
>> Two spaces before < instead of just one.
>> +
>> +   PR  target/85667
>>
>> Only a single space between PR and target.
>>
>> +   * i386.c (function_value_ms_64): ms_abi insist to use the eax
>>
>> The filename is relative to the directory with ChangeLog file, so
>> * config/i386/i386.c
>> in this case.  The description should say what you've changed, so
>> something like:
>> * config/i386/i386.c (function_value_ms_64): Return AX_REG instead
>> of FIRST_SSE_REG for 4 or 8 byte modes.
>>
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -9008,7 +9008,7 @@ function_value_ms_64 (machine_mode orig_mode, 
>> machine_mode mode,
>> case 8:
>> case 4:
>>   if (mode == SFmode || mode == DFmode)
>> -   regno = FIRST_SSE_REG;
>> +   regno = AX_REG;
>>   break;
>>
>> Is there something to back that up, say godbolt.org link with some testcases
>> showing how does MSVC, clang etc. handle those?
>> And, because the function starts with:
>>   unsigned int regno = AX_REG;
>> the change isn't right, you should remove all of:
>> case 8:
>> case 4:
>>   if (mode == SFmode || mode == DFmode)
>> regno = FIRST_SSE_REG;
>>   break;
>> because the default will do what you want.
>>
>> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
>> index 50e53f0..ec54330 100644
>> --- a/gcc/testsuite/ChangeLog
>> +++ b/gcc/testsuite/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2018-11-15 Lokesh Janghel  
>>
>> Two spaces between date and name.
>>
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr85667.c
>> @@ -0,0 +1,29 @@
>> +/* { dg-options "-O2 " } */
>> +/* { dg-final { scan-assembler-times "movl\[\t 
>> \]\.\[a-zA-Z\]\[a-zA-Z\]\[0-9\]\\(%rip\\), %eax" 1} } */
>>
>> First of all, the test is misplaced, it is clearly x86_64 specific
>> and probably lp64 only, so it shouldn't be in gcc.dg/ where it will
>> be run on all targets, but in gcc.target/i386/ and be guarded with
>> { target lp64 }.
>>
>> Second, seems like you'd like to run the testcase, so you'd better have it
>> /* { dg-do run { target lp64 } } */
>>
>> The assembler scanning will work only with -masm=att, not -masm=intel
>> and seems to be very fragile, so I'd suggest have one runtime test and one
>> compile time test in which you put just the fn1 function.  Why two arbitrary
>> letters after dot?  That makes on sense.  Either you are looking for 
>> .LC\[0-9]*
>> specifically, or for arbitrary symbol, then use something like
>> "movl\[^\n\r]*, %eax"
>> or so (and make sure to use -masm=intel).
>>
>> More interesting would be
>> make check ALT_CC_UNDER_TEST=msvc ALT_CXX_UNDER_TEST=msvc++ 
>> RUNTESTFLAGS='compat.exp struct-layout-1.exp'
>> (or whatever MSVC driver names are), i.e. try to run the compat testsuites
>> between MSVC and newly built gcc.
>>
>> Jakub
>
>
>
> --
> Thanks & Regards
> Lokesh Janghel
> +91-9752984749


[v3 PATCH] Housekeeping for the effective targets of optional's tests.

2018-11-19 Thread Ville Voutilainen
Tested on Linux-x64 for optional's tests only. Ok for trunk?

2018-11-20  Ville Voutilainen  

Housekeeping for the effective targets of optional's tests.
* testsuite/20_util/optional/77288.cc: Adjust.
* testsuite/20_util/optional/84601.cc: Likewise.
* testsuite/20_util/optional/assignment/1.cc: Likewise.
* testsuite/20_util/optional/assignment/2.cc: Likewise.
* testsuite/20_util/optional/assignment/3.cc: Likewise.
* testsuite/20_util/optional/assignment/4.cc: Likewise.
* testsuite/20_util/optional/assignment/5.cc: Likewise.
* testsuite/20_util/optional/assignment/6.cc: Likewise.
* testsuite/20_util/optional/assignment/7.cc: Likewise.
* testsuite/20_util/optional/assignment/8.cc: Likewise.
* testsuite/20_util/optional/cons/77727.cc: Likewise.
* testsuite/20_util/optional/cons/copy.cc: Likewise.
* testsuite/20_util/optional/cons/deduction.cc: Likewise.
* testsuite/20_util/optional/cons/default.cc: Likewise.
* testsuite/20_util/optional/cons/move.cc: Likewise.
* testsuite/20_util/optional/cons/trivial.cc: Likewise.
* testsuite/20_util/optional/cons/value.cc: Likewise.
* testsuite/20_util/optional/cons/value_neg.cc: Likewise.
* testsuite/20_util/optional/constexpr/cons/default.cc: Likewise.
* testsuite/20_util/optional/constexpr/cons/value.cc: Likewise.
* testsuite/20_util/optional/constexpr/in_place.cc: Likewise.
* testsuite/20_util/optional/constexpr/nullopt.cc: Likewise.
* testsuite/20_util/optional/constexpr/observers/1.cc: Likewise.
* testsuite/20_util/optional/constexpr/observers/4.cc: Likewise.
* testsuite/20_util/optional/constexpr/observers/5.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/1.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/2.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/3.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/4.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/5.cc: Likewise.
* testsuite/20_util/optional/constexpr/relops/6.cc: Likewise.
* testsuite/20_util/optional/in_place.cc: Likewise.
* testsuite/20_util/optional/make_optional.cc: Likewise.
* testsuite/20_util/optional/nullopt.cc: Likewise.
* testsuite/20_util/optional/observers/1.cc: Likewise.
* testsuite/20_util/optional/observers/2.cc: Likewise.
* testsuite/20_util/optional/observers/3.cc: Likewise.
* testsuite/20_util/optional/observers/4.cc: Likewise.
* testsuite/20_util/optional/observers/5.cc: Likewise.
* testsuite/20_util/optional/observers/6.cc: Likewise.
* testsuite/20_util/optional/relops/1.cc: Likewise.
* testsuite/20_util/optional/relops/2.cc: Likewise.
* testsuite/20_util/optional/relops/3.cc: Likewise.
* testsuite/20_util/optional/relops/4.cc: Likewise.
* testsuite/20_util/optional/relops/5.cc: Likewise.
* testsuite/20_util/optional/relops/6.cc: Likewise.
* testsuite/20_util/optional/relops/7.cc: Likewise.
* testsuite/20_util/optional/requirements.cc: Likewise.
* testsuite/20_util/optional/swap/1.cc: Likewise.
* testsuite/20_util/optional/swap/2.cc: Likewise.
* testsuite/20_util/optional/typedefs.cc: Likewise.
diff --git a/libstdc++-v3/testsuite/20_util/optional/77288.cc b/libstdc++-v3/testsuite/20_util/optional/77288.cc
index 6f3f7f4..cd3dc0e 100644
--- a/libstdc++-v3/testsuite/20_util/optional/77288.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/77288.cc
@@ -1,5 +1,5 @@
 // { dg-options "-std=gnu++17" }
-// { dg-do run }
+// { dg-do run { target c++17 }  }
 
 // Copyright (C) 2016-2018 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/20_util/optional/84601.cc b/libstdc++-v3/testsuite/20_util/optional/84601.cc
index e86d39e..b8a1706 100644
--- a/libstdc++-v3/testsuite/20_util/optional/84601.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/84601.cc
@@ -1,5 +1,5 @@
 // { dg-options "-std=gnu++17" }
-// { dg-do compile }
+// { dg-do compile { target c++17 }  }
 
 #include 
 
diff --git a/libstdc++-v3/testsuite/20_util/optional/assignment/1.cc b/libstdc++-v3/testsuite/20_util/optional/assignment/1.cc
index f9aaae3..ec0832b 100644
--- a/libstdc++-v3/testsuite/20_util/optional/assignment/1.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/1.cc
@@ -1,5 +1,5 @@
 // { dg-options "-std=gnu++17" }
-// { dg-do run }
+// { dg-do run { target c++17 }  }
 
 // Copyright (C) 2013-2018 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/20_util/optional/assignment/2.cc b/libstdc++-v3/testsuite/20_util/optional/assignment/2.cc
index c3832e9..772f7bf 100644
--- a/libstdc++-v3/testsuite/20_util/optional/assignment/2.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/2.cc
@@ -1,5 +1,5 @@
 // { dg-options "-std=gnu++17" }
-// { dg-do run }
+// { dg-do run { target c++17 }  }
 
 // Copyright (C) 2013-2018 Free Software Foundation, Inc.
 //
diff --git a/libstdc++-v3/testsuite/20_util/optional/assignment/3.

Re: [PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)

2018-11-19 Thread Joseph Myers
On Mon, 19 Nov 2018, David Malcolm wrote:

> +/* C implementation of same_type_p.
> +   Returns true iff TYPE1 and TYPE2 are the same type, in the usual
> +   sense of `same'.  */
> +
> +bool
> +same_type_p (tree type1, tree type2)
> +{
> +  return comptypes (type1, type2) == 1;
> +}

I don't think "compatible" and "same" are the same concept.  Normally in C 
you'd be concerned with compatibility; "same type" is only used for the 
rule on duplicate typedefs, which uses comptypes_check_different_types.

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


[PATCH, testsuite] Fix pdp11 test failures

2018-11-19 Thread Paul Koning
This corrects a number of pdp11 test suite failures.  The first set fail due to 
larger than supported alignment.  The next two look for .ascii directives in 
the assembly output which pdp11 does not use.  The last one needs adjustment 
because CSE loads subroutine addresses into registers in this test.

Committed.

paul

testsuite/ChangeLog:

2018-11-19  Paul Koning  

* gcc.c-torture/execute/align-3.c: Skip if pdp11.
* gcc.c-torture/execute/pr23467.c: Ditto.
* gcc.c-torture/execute/pr36093.c: Ditto.
* gcc.c-torture/execute/pr43783.c: Ditto.
* gcc.dg/const-elim-2.c: Xfail if pdp11.
* gcc.dg/torture/pr36400.c: Ditto.
* gcc.dg/tree-ssa/loop-1.c: Xfail for pdp11.  Add pdp11 to check
for jsr.

Index: gcc.c-torture/execute/align-3.c
===
--- gcc.c-torture/execute/align-3.c (revision 266295)
+++ gcc.c-torture/execute/align-3.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
 void func(void) __attribute__((aligned(256)));
 
 void func(void) 
Index: gcc.c-torture/execute/pr23467.c
===
--- gcc.c-torture/execute/pr23467.c (revision 266295)
+++ gcc.c-torture/execute/pr23467.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
 struct s1
 {
   int __attribute__ ((aligned (8))) a;
Index: gcc.c-torture/execute/pr36093.c
===
--- gcc.c-torture/execute/pr36093.c (revision 266295)
+++ gcc.c-torture/execute/pr36093.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
 extern void abort (void);
 
 typedef struct Bar {
Index: gcc.c-torture/execute/pr43783.c
===
--- gcc.c-torture/execute/pr43783.c (revision 266295)
+++ gcc.c-torture/execute/pr43783.c (working copy)
@@ -1,3 +1,5 @@
+/* { dg-skip-if "small alignment" { pdp11-*-* } } */
+
 typedef __attribute__((aligned(16)))
 struct {
   unsigned long long w[3];
Index: gcc.dg/const-elim-2.c
===
--- gcc.dg/const-elim-2.c   (revision 266295)
+++ gcc.dg/const-elim-2.c   (working copy)
@@ -1,7 +1,7 @@
 /* The string constant in this test case should be emitted exactly once.  */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-times "hi there" 1 { xfail nvptx-*-* } } } */
+/* { dg-final { scan-assembler-times "hi there" 1 { xfail nvptx-*-* pdp11-*-* 
} } } */
 
 static inline int returns_23() { return 23; }
 
Index: gcc.dg/torture/pr36400.c
===
--- gcc.dg/torture/pr36400.c(revision 266295)
+++ gcc.dg/torture/pr36400.c(working copy)
@@ -14,4 +14,4 @@ void baz()
   barptr->some_string = "Everything OK";
 }
 
-/* { dg-final { scan-assembler "Everything OK" { xfail nvptx-*-* } } } */
+/* { dg-final { scan-assembler "Everything OK" { xfail nvptx-*-* pdp11-*-* } } 
} */
Index: gcc.dg/tree-ssa/loop-1.c
===
--- gcc.dg/tree-ssa/loop-1.c(revision 266295)
+++ gcc.dg/tree-ssa/loop-1.c(working copy)
@@ -46,7 +46,7 @@ int xxx(void)
 /* CRIS keeps the address in a register.  */
 /* m68k sometimes puts the address in a register, depending on CPU and PIC.  */
 
-/* { dg-final { scan-assembler-times "foo" 5 { xfail hppa*-*-* ia64*-*-* 
sh*-*-* cris-*-* crisv32-*-* fido-*-* m68k-*-* i?86-*-mingw* i?86-*-cygwin* 
x86_64-*-mingw* visium-*-* nvptx*-*-* } } } */
+/* { dg-final { scan-assembler-times "foo" 5 { xfail hppa*-*-* ia64*-*-* 
sh*-*-* cris-*-* crisv32-*-* fido-*-* m68k-*-* i?86-*-mingw* i?86-*-cygwin* 
x86_64-*-mingw* visium-*-* nvptx*-*-* pdp11*-*-* } } } */
 /* { dg-final { scan-assembler-times "foo,%r" 5 { target hppa*-*-* } } } */
 /* { dg-final { scan-assembler-times "= foo"  5 { target ia64*-*-* } } } */
 /* { dg-final { scan-assembler-times "call\[ \t\]*_foo" 5 { target 
i?86-*-mingw* i?86-*-cygwin* } } } */
@@ -53,6 +53,6 @@ int xxx(void)
 /* { dg-final { scan-assembler-times "call\[ \t\]*foo" 5 { target 
x86_64-*-mingw* } } } */
 /* { dg-final { scan-assembler-times "jsr|bsrf|blink\ttr?,r18"  5 { target 
sh*-*-* } } } */
 /* { dg-final { scan-assembler-times "Jsr \\\$r" 5 { target cris-*-* } } } */
-/* { dg-final { scan-assembler-times "\[jb\]sr" 5 { target fido-*-* m68k-*-* } 
} } */
+/* { dg-final { scan-assembler-times "\[jb\]sr" 5 { target fido-*-* m68k-*-* 
pdp11-*-* } } } */
 /* { dg-final { scan-assembler-times "bra *tr,r\[1-9\]*,r21" 5 { target 
visium-*-* } } } */
 /* { dg-final { scan-assembler-times "(?n)\[ \t\]call\[ \t\].*\[ \t\]foo," 5 { 
target nvptx*-*-* } } } */



Re: [PATCH, testsuite] indicate no "weak" support in pdp11

2018-11-19 Thread Paul Koning



> On Nov 19, 2018, at 5:20 PM, Jeff Law  wrote:
> 
> On 11/19/18 3:18 PM, Paul Koning wrote:
>> This patch changes check_weak_available to report that pdp11 does not 
>> support "weak".  A number of test case failures are caused by attempts to 
>> use weak support which is missing in the pdp11 flavor of a.out.
>> 
>> I'm not sure if this is covered by target maintainer authority so I figure 
>> it's best to ask for approval.
>> 
>> Ok for trunk?
>> 
>>  paul
>> 
>> testsuite/ChangeLog:
>> 
>> 2018-11-19  Paul Koning  
>> 
>>  * lib/target-supports.exp (check_weak_available): Return "no" for
>>  pdp11.
> Yes.  And FWIW this kind of change falls into what maintainers can make
> and self-approve.
> 
> jeff

Thanks for the clarification.  Committed.

paul



[PATCH] correct handling of EXCESS_PRECISION_EXPR in function calls (PR 88091)

2018-11-19 Thread Martin Sebor

The recent -Wbuiltin-declaration-mismatch enhancement to detect
calls with incompatible arguments to built-ins declared without
a prototype introduced a subtle bug in the detection of floating
conversion errors when passing EXCESS_PRECISION_EXPR arguments
to floating function parameters with different precision.
The bug was caught by tests on a handful of targets, but
unfortunately not on x86_64-linux.

The attached patch corrects this bug.  Because it also changes
the indentation of the affected function to make the control flow
easier to follow I include two diffs: one that can be applied as
is and another with whitespace changes ignored.

Tested on x86_64-linux with -m32/-m64 and with an i386-solaris2.11
cross-compiler.

Martin
PR c/88091 - c-c++-common/Wconversion-real.c etc. FAIL

gcc/c/ChangeLog:

	PR c/88091
	* c-typeck.c (convert_argument): Add a parameter.  Adjust indentation.
	(convert_arguments): Add comments.  Pass additional argument to
	the function above.

Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(revision 266284)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3190,14 +3190,10 @@ c_build_function_call_vec (location_t loc, vec "
+			"rather than % due to prototype",
+			argnum, rname);
 
-	  if (INTEGRAL_TYPE_P (type)
-	  && TREE_CODE (valtype) == REAL_TYPE)
+	  /* Warn if mismatch between argument and prototype
+	 for decimal float types.  Warn of conversions with
+	 binary float types and of precision narrowing due to
+	 prototype.  */
+	  else if (type != valtype
+		   && (type == dfloat32_type_node
+		   || type == dfloat64_type_node
+		   || type == dfloat128_type_node
+		   || valtype == dfloat32_type_node
+		   || valtype == dfloat64_type_node
+		   || valtype == dfloat128_type_node)
+		   && (formal_prec
+		   <= TYPE_PRECISION (valtype)
+		   || (type == dfloat128_type_node
+			   && (valtype
+			   != dfloat64_type_node
+			   && (valtype
+   != dfloat32_type_node)))
+		   || (type == dfloat64_type_node
+			   && (valtype
+			   != dfloat32_type_node
+	warning_at (ploc, 0,
+			"passing argument %d of %qE as %qT "
+			"rather than %qT due to prototype",
+			argnum, rname, type, valtype);
+
+	}
+  /* Detect integer changing in width or signedness.
+	 These warnings are only activated with
+	 -Wtraditional-conversion, not with -Wtraditional.  */
+  else if (warn_traditional_conversion
+	   && INTEGRAL_TYPE_P (type)
+	   && INTEGRAL_TYPE_P (valtype))
+	{
+	  tree would_have_been = default_conversion (val);
+	  tree type1 = TREE_TYPE (would_have_been);
+
+	  if (val == error_mark_node)
+	/* VAL could have been of incomplete type.  */;
+	  else if (TREE_CODE (type) == ENUMERAL_TYPE
+		   && (TYPE_MAIN_VARIANT (type)
+		   == TYPE_MAIN_VARIANT (valtype)))
+	/* No warning if function asks for enum
+	   and the actual arg is that enum type.  */
+	;
+	  else if (formal_prec != TYPE_PRECISION (type1))
 	warning_at (ploc, OPT_Wtraditional_conversion,
-			"passing argument %d of %qE as integer rather "
-			"than floating due to prototype",
+			"passing argument %d of %qE "
+			"with different width due to prototype",
 			argnum, rname);
-	  if (INTEGRAL_TYPE_P (type)
-	  && TREE_CODE (valtype) == COMPLEX_TYPE)
+	  else if (TYPE_UNSIGNED (type) == TYPE_UNSIGNED (type1))
+	;
+	  /* Don't complain if the formal parameter type
+	 is an enum, because we can't tell now whether
+	 the value was an enum--even the same enum.  */
+	  else if (TREE_CODE (type) == ENUMERAL_TYPE)
+	;
+	  else if (TREE_CODE (val) == INTEGER_CST
+		   && int_fits_type_p (val, type))
+	/* Change in signedness doesn't matter
+	   if a constant value is unaffected.  */
+	;
+	  /* If the value is extended from a narrower
+	 unsigned type, it doesn't matter whether we
+	 pass it as signed or unsigned; the value
+	 certainly is the same either way.  */
+	  else if (TYPE_PRECISION (valtype) < TYPE_PRECISION (type)
+		   && TYPE_UNSIGNED (valtype))
+	;
+	  else if (TYPE_UNSIGNED (type))
 	warning_at (ploc, OPT_Wtraditional_conversion,
-			"passing argument %d of %qE as integer rather "
-			"than complex due to prototype",
+			"passing argument %d of %qE "
+			"as unsigned due to prototype",
 			argnum, rname);
-	  else if (TREE_CODE (type) == COMPLEX_TYPE
-		   && TREE_CODE (valtype) == REAL_TYPE)
+	  else
 	warning_at (ploc, OPT_Wtraditional_conversion,
-			"passing argument %d of %qE as complex rather "
-			"than floating due to prototype",
+			"passing argument %d of %qE "
+			"as signed due to prototype",
 			argnum, rname);
-	  else if (TREE_CODE (type) == REAL_TYPE
-		   && INTEGRAL_TYPE_P (valtype))
-	warning_at (ploc, OPT_Wtraditional_conversion,
-			"passing argument %d of %qE as floating rather "
-			"than integer due to prototype",
-			argnum, rname);
-	  else if (TREE_CODE (type) == COMPLE

Re: [PATCH] avoid error_mark_node in -Wsizeof-pointer-memaccess (PR 88065)

2018-11-19 Thread Martin Sebor

On 11/19/2018 04:10 PM, Jeff Law wrote:

On 11/17/18 3:45 PM, Martin Sebor wrote:

-Wsizeof-pointer-memaccess fails with an ICE when one of
the arguments is ill-formed (error_mark_node).  To avoid
the error the attached patch has the function bail in this
case.

Martin

gcc-88065.diff

PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy

gcc/c-family/ChangeLog:

PR c/88065
* c-warn.c (sizeof_pointer_memaccess_warning): Bail if source
or destination is an error.

gcc/testsuite/ChangeLog:

PR c/88065
* gcc.dg/Wsizeof-pointer-memaccess2.c: New test.

This is probably OK.  But before final ACK, is there a point earlier
where we could/should have bailed out?

ie, when does the ERROR_MARK get created and if you were to look at the
flow from that point to the offending call to
sizeof_pointer_memaccess_warning is there a better place to bail?


sizeof_pointer_memaccess_warning() has a big switch statement
with the built-in code as a controlling expression that it
uses to determine which argument is the source, which one is
the destination, and which one is the size/bound.  To do
the checking anywhere else up the stack the caller would have
to replicate the same switch statement, so I think this is
the right spot to do it.

I can move the test if it's important.

FWIW, I tend to only add rests to c-c++-common if they exercise
different code between the two front-ends.  Otherwise it seems
like unnecessarily increasing the already excessive testsuite
runtimes (each of the C++ tests runs once for each C++ revision,
i.e., C++ 98, C++ 11, C++ 14, and C++ 17).  I do realize that
if the implementation were to diverge between the two front-ends
having test coverage for both would be a good thing.  So it's
a trade-off.  While griping about c-c++-common: sometimes (though
not in the case of ICEs), because of subtle differences between
the two front-ends (like missing or inaccurate location
information) it can also be a pain to get the tests to produce
the same output even for a shared implementation of a warning,
let alone for distinct ones.

Martin


Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438

2018-11-19 Thread Uros Bizjak
On Mon, Nov 19, 2018 at 11:42 PM Eric Botcazou  wrote:
>
> > The extra instruction is generated as a kludge in expand_function_end
> > to prevent instructions that may trap to be scheduled into function
> > epilogue. However, the same blockage is generated under exactly the
> > same conditions earlier in the expand_function_end. The first blockage
> > should prevent unwanted scheduling into the epilogue, so there is
> > actually no need for the second one.
>
> But there are instructions emitted after the first blockage, aren't there?

Yes, but they don't generate exceptions. At least all memory accesses
have  /c flag.

> Did you check the history of the code?

I did.

The blockage was introduced as a fix for PR14381 [1] in r79265 [2].
Later, the blockage was moved after return label as a fix for PR25176
[3] in r107871 [4].

After that, r122626 [5] moves the blockage after the label for the
naked return from the function. Relevant posts from gcc-patches@ ML
are at [6], [7]. However, in the posts, there are no concrete
examples, how scheduler moves instructions from different BB around
blockage insn, the posts just show that there is a jump around
blockage when __builtin_return is used. I was under impression that
scheduler is unable to move instructions over BB boundaries.

A mystery is the tree-ssa merge [8] that copies back the hunk, moved
in r122626 [5] to its original position. From this revision onwards,
we emit two blockages.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=14381
[2] https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=79265
[3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25176
[4] https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=107871
[5] https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=122626
[6] https://gcc.gnu.org/ml/gcc-patches/2007-02/msg01143.html
[7] https://gcc.gnu.org/ml/gcc-patches/2007-02/msg01623.html
[8] 
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/function.c?limit_changes=0&r1=125624&r2=125623&pathrev=125624

Uros.


Re: [PATCH] avoid error_mark_node in -Wsizeof-pointer-memaccess (PR 88065)

2018-11-19 Thread Jakub Jelinek
On Mon, Nov 19, 2018 at 04:10:09PM -0700, Jeff Law wrote:
> > PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
> > 
> > gcc/c-family/ChangeLog:
> > 
> > PR c/88065
> > * c-warn.c (sizeof_pointer_memaccess_warning): Bail if source
> > or destination is an error.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > PR c/88065
> > * gcc.dg/Wsizeof-pointer-memaccess2.c: New test.
> This is probably OK.  But before final ACK, is there a point earlier
> where we could/should have bailed out?

IMHO it is a good point, but it should use error_operand_p predicate instead
of == error_mark_node checks to also catch the case where the argument is
not error_mark_node, but has error_mark_node type.  And, the testcase
shouldn't be in gcc.dg, but in c-c++-common and cover also C++ testing.

Jakub


Fix ICE in ipa-devirt

2018-11-19 Thread Jan Hubicka
Hi,
this patch fixes the second ICE exposed by the new testcase Marxin
added. Again the ODR warning needs work, so I commit it after getting
it right.

Bootstrapped/regtested x86_64-linux, comitted.

PR lto/87957
* ipa-devirt.c (free_enum_values): Do not ICE on ODR vilations.
Index: ipa-devirt.c
===
--- ipa-devirt.c(revision 266235)
+++ ipa-devirt.c(working copy)
@@ -2268,12 +2268,14 @@ free_enum_values ()
   enum_values_freed = true;
   unsigned int i;
   for (i = 0; i < odr_types.length (); i++)
-if (odr_types[i] && TREE_CODE (odr_types[i]->type) == ENUMERAL_TYPE)
+if (odr_types[i])
   {
-   TYPE_VALUES (odr_types[i]->type) = NULL;
+   if (TREE_CODE (odr_types[i]->type) == ENUMERAL_TYPE)
+ TYPE_VALUES (odr_types[i]->type) = NULL;
if (odr_types[i]->types)
   for (unsigned int j = 0; j < odr_types[i]->types->length (); j++)
-   TYPE_VALUES ((*odr_types[i]->types)[j]) = NULL;
+   if (TREE_CODE ((*odr_types[i]->types)[j]) == ENUMERAL_TYPE)
+ TYPE_VALUES ((*odr_types[i]->types)[j]) = NULL;
   }
   enum_values_freed = true;
 }


Re: [PATCH] avoid error_mark_node in -Wsizeof-pointer-memaccess (PR 88065)

2018-11-19 Thread Jeff Law
On 11/17/18 3:45 PM, Martin Sebor wrote:
> -Wsizeof-pointer-memaccess fails with an ICE when one of
> the arguments is ill-formed (error_mark_node).  To avoid
> the error the attached patch has the function bail in this
> case.
> 
> Martin
> 
> gcc-88065.diff
> 
> PR c/88065 - ICE in -Wsizeof-pointer-memaccess on an invalid strncpy
> 
> gcc/c-family/ChangeLog:
> 
>   PR c/88065
>   * c-warn.c (sizeof_pointer_memaccess_warning): Bail if source
>   or destination is an error.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/88065
>   * gcc.dg/Wsizeof-pointer-memaccess2.c: New test.
This is probably OK.  But before final ACK, is there a point earlier
where we could/should have bailed out?

ie, when does the ERROR_MARK get created and if you were to look at the
flow from that point to the offending call to
sizeof_pointer_memaccess_warning is there a better place to bail?

jeff


Re: [PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438

2018-11-19 Thread Eric Botcazou
> The extra instruction is generated as a kludge in expand_function_end
> to prevent instructions that may trap to be scheduled into function
> epilogue. However, the same blockage is generated under exactly the
> same conditions earlier in the expand_function_end. The first blockage
> should prevent unwanted scheduling into the epilogue, so there is
> actually no need for the second one.

But there are instructions emitted after the first blockage, aren't there?

Did you check the history of the code?

-- 
Eric Botcazou


Re: [PATCH] Fix PR83215, remove alias-set zero case from component_uses_parent_alias_set_from

2018-11-19 Thread Eric Botcazou
> Eric, do you know of any cases in Ada where a alias-set zero base
> has non-alias-set zero children?  The testsuite seems to be clean
> but you never know...

No, at least not off the top of my head; that would be weird in any case.

-- 
Eric Botcazou


Re: [C++ Patch] PR 84636 ("internal compiler error: Segmentation fault (identifier_p()/grokdeclarator())")

2018-11-19 Thread Marek Polacek
On Mon, Nov 19, 2018 at 08:03:24PM +0100, Paolo Carlini wrote:
> @@ -12245,8 +12246,9 @@ grokdeclarator (const cp_declarator *declarator,
>   error ("invalid use of %<::%>");
>   return error_mark_node;
> }
> - else if (TREE_CODE (type) == FUNCTION_TYPE
> -  || TREE_CODE (type) == METHOD_TYPE)
> + else if ((TREE_CODE (type) == FUNCTION_TYPE
> +   || TREE_CODE (type) == METHOD_TYPE)

I know it's preexisting but we have FUNC_OR_METHOD_TYPE_P for this.

Marek


Re: [PATCH, testsuite] indicate no "weak" support in pdp11

2018-11-19 Thread Jeff Law
On 11/19/18 3:18 PM, Paul Koning wrote:
> This patch changes check_weak_available to report that pdp11 does not support 
> "weak".  A number of test case failures are caused by attempts to use weak 
> support which is missing in the pdp11 flavor of a.out.
> 
> I'm not sure if this is covered by target maintainer authority so I figure 
> it's best to ask for approval.
> 
> Ok for trunk?
> 
>   paul
> 
> testsuite/ChangeLog:
> 
> 2018-11-19  Paul Koning  
> 
>   * lib/target-supports.exp (check_weak_available): Return "no" for
>   pdp11.
Yes.  And FWIW this kind of change falls into what maintainers can make
and self-approve.

jeff


[PATCH, testsuite] indicate no "weak" support in pdp11

2018-11-19 Thread Paul Koning
This patch changes check_weak_available to report that pdp11 does not support 
"weak".  A number of test case failures are caused by attempts to use weak 
support which is missing in the pdp11 flavor of a.out.

I'm not sure if this is covered by target maintainer authority so I figure it's 
best to ask for approval.

Ok for trunk?

paul

testsuite/ChangeLog:

2018-11-19  Paul Koning  

* lib/target-supports.exp (check_weak_available): Return "no" for
pdp11.

Index: lib/target-supports.exp
===
--- lib/target-supports.exp (revision 266288)
+++ lib/target-supports.exp (working copy)
@@ -314,6 +314,12 @@ proc check_weak_available { } {
return 1
 }
 
+# pdp11 doesn't support it
+
+if { [istarget pdp11*-*-*] } {
+   return 0
+}
+
 # ELF and ECOFF support it. a.out does with gas/gld but may also with
 # other linkers, so we should try it
 



Re: C++ PATCH to implement P1094R2, Nested inline namespaces

2018-11-19 Thread Marek Polacek
On Mon, Nov 19, 2018 at 10:33:17PM +0100, Jakub Jelinek wrote:
> On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> > 2018-11-19  Marek Polacek  
> > 
> > Implement P1094R2, Nested inline namespaces.
> > * g++.dg/cpp2a/nested-inline-ns1.C: New test.
> > * g++.dg/cpp2a/nested-inline-ns2.C: New test.
> > * g++.dg/cpp2a/nested-inline-ns3.C: New test.
> 
> Just a small testsuite comment.
> 
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> > @@ -0,0 +1,26 @@
> > +// P1094R2
> > +// { dg-do compile { target c++2a } }
> 
> Especially because 2a testing isn't included by default, but also
> to make sure it works right even with -std=c++17, wouldn't it be better to
> drop the nested-inline-ns3.C test, make this test c++17 or
> even better always enabled, add dg-options "-Wpedantic" and
> just add dg-warning with c++17_down and c++14_down what should be
> warned on the 3 lines (with .-1 for c++14_down)?
> 
> Or if you want add some further testcases that will test how
> c++17 etc. will dg-error on those with -pedantic-errors etc.

Sure, I've made it { target c++11 } and dropped the third test:

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

2018-11-19  Marek Polacek  

Implement P1094R2, Nested inline namespaces.
* parser.c (cp_parser_namespace_definition): Parse the optional inline
keyword in a nested-namespace-definition.  Adjust push_namespace call.
Formatting fix.

* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 292cce15676..f39e9d753d2 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18872,6 +18872,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   cp_ensure_no_oacc_routine (parser);
 
   bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+  const bool topmost_inline_p = is_inline;
 
   if (is_inline)
 {
@@ -18890,6 +18891,17 @@ cp_parser_namespace_definition (cp_parser* parser)
 {
   identifier = NULL_TREE;
   
+  bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+RID_INLINE);
+  if (nested_inline_p && nested_definition_count != 0)
+   {
+ if (cxx_dialect < cxx2a)
+   pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+OPT_Wpedantic, "nested inline namespace definitions only "
+"available with -std=c++2a or -std=gnu++2a");
+ cp_lexer_consume_token (parser->lexer);
+   }
+
   if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
{
  identifier = cp_parser_identifier (parser);
@@ -18904,7 +18916,12 @@ cp_parser_namespace_definition (cp_parser* parser)
}
 
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
-   break;
+   {
+ /* Don't forget that the innermost namespace might have been
+marked as inline.  */
+ is_inline |= nested_inline_p;
+ break;
+   }
   
   if (!nested_definition_count && cxx_dialect < cxx17)
 pedwarn (input_location, OPT_Wpedantic,
@@ -18913,7 +18930,9 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   /* Nested namespace names can create new namespaces (unlike
 other qualified-ids).  */
-  if (int count = identifier ? push_namespace (identifier) : 0)
+  if (int count = (identifier
+  ? push_namespace (identifier, nested_inline_p)
+  : 0))
nested_definition_count += count;
   else
cp_parser_error (parser, "nested namespace name required");
@@ -18926,7 +18945,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   if (nested_definition_count && attribs)
 error_at (token->location,
  "a nested namespace definition cannot have attributes");
-  if (nested_definition_count && is_inline)
+  if (nested_definition_count && topmost_inline_p)
 error_at (token->location,
  "a nested namespace definition cannot be inline");
 
@@ -18935,7 +18954,7 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
-  warning  (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
+  warning (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
 
   /* Look for the `{' to validate starting the namespace.  */
   matching_braces braces;
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C 
gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
new file mode 100644
index 000..8c9573ea5db
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,29 @@
+// P1094R2
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpedantic" }
+
+namespace A::inline B::C { // { dg-warning "nested inline namespace 
definitions only" "" { target c++17_down } }
+// { dg-warning "ne

[doc, committed] mention shared libraries in docs for -l option

2018-11-19 Thread Sandra Loosemore
I've checked in this patch for PR 50250, following the suggestion in the 
issue to point to the linker documentation for definitive info on -l 
instead of trying to explain everything in great detail.


-Sandra
2018-11-19  Sandra Loosemore  

	PR driver/50250

	gcc/
	* doc/invoke.texi (Link Options): Mention shared libraries
	in documentation for the -l option.  Simplify discussion and
	point to the system linker documentation for details.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi	(revision 266286)
+++ gcc/doc/invoke.texi	(working copy)
@@ -12988,28 +12988,27 @@ Search the library named @var{library} w
 alternative with the library as a separate argument is only for
 POSIX compliance and is not recommended.)
 
+The @option{-l} option is passed directly to the linker by GCC.  Refer
+to your linker documentation for exact details.  The general
+description below applies to the GNU linker.  
+
+The linker searches a standard list of directories for the library.
+The directories searched include several standard system directories
+plus any that you specify with @option{-L}.
+
+Static libraries are archives of object files, and have file names
+like @file{lib@var{library}.a}.  Some targets also support shared
+libraries, which typically have names like @file{lib@var{library}.so}.
+If both static and shared libraries are found, the linker gives
+preference to linking with the shared library unless the
+@option{-static} option is used.
+
 It makes a difference where in the command you write this option; the
 linker searches and processes libraries and object files in the order they
 are specified.  Thus, @samp{foo.o -lz bar.o} searches library @samp{z}
 after file @file{foo.o} but before @file{bar.o}.  If @file{bar.o} refers
 to functions in @samp{z}, those functions may not be loaded.
 
-The linker searches a standard list of directories for the library,
-which is actually a file named @file{lib@var{library}.a}.  The linker
-then uses this file as if it had been specified precisely by name.
-
-The directories searched include several standard system directories
-plus any that you specify with @option{-L}.
-
-Normally the files found this way are library files---archive files
-whose members are object files.  The linker handles an archive file by
-scanning through it for members which define symbols that have so far
-been referenced but not defined.  But if the file that is found is an
-ordinary object file, it is linked in the usual fashion.  The only
-difference between using an @option{-l} option and specifying a file name
-is that @option{-l} surrounds @var{library} with @samp{lib} and @samp{.a}
-and searches several directories.
-
 @item -lobjc
 @opindex lobjc
 You need this special case of the @option{-l} option in order to


[PATCH] handle unusual targets in -Wbuiltin-declaration-mismatch (PR 88098)

2018-11-19 Thread Martin Sebor

The gcc.dg/Wbuiltin-declaration-mismatch-4.c test added with
the recent -Wbuiltin-declaration-mismatch enhancement to detect
calls with incompatible arguments to built-ins declared without
a prototype fails on a few targets due to incorrect assumptions
hardcoded into the test.  Besides removing those assumptions
(or adding appropriate { target } attributes, the attached patch
also adjusts the implementation of the warning to avoid triggering
for enum promotion to int on short_enums targets.

Since the fix is trivial I plan to commit it tomorrow if there
are no concerns.

Tested on x86_64-linux and with an arm-none-eabi cross-compiler.
I also did a little bit of testing with sparc-solaris2.11 cross
compiler but there the test harness fails due to the -m32 option
so the Wbuiltin-declaration-mismatch-4.c still has unexpected
FAILs.  I've raised bug 88104 for the outstanding problem on
sparc-solaris2.11.

Martin
PR testsuite/88098 - FAIL: gcc.dg/Wbuiltin-declaration-mismatch-4.c (test for warnings

gcc/c/ChangeLog:

	PR testsuite/88098
	* c-typeck.c (maybe_warn_builtin_no_proto_arg): Handle short enum
	to int promotion.

gcc/testsuite/ChangeLog:

	PR testsuite/88098
	* gcc.dg/Wbuiltin-declaration-mismatch-4.c: Adjust.
	* gcc.dg/Wbuiltin-declaration-mismatch-5.c: New test.


Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c	(revision 266284)
+++ gcc/c/c-typeck.c	(working copy)
@@ -6461,7 +6461,9 @@ maybe_warn_builtin_no_proto_arg (location_t loc, t
   && TYPE_MODE (parmtype) == TYPE_MODE (argtype))
 return;
 
-  if (parmcode == argcode
+  if ((parmcode == argcode
+   || (parmcode == INTEGER_TYPE
+	   && argcode == ENUMERAL_TYPE))
   && TYPE_MAIN_VARIANT (parmtype) == TYPE_MAIN_VARIANT (promoted))
 return;
 
Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c
===
--- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c	(revision 266284)
+++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c	(working copy)
@@ -77,9 +77,9 @@ void test_integer_conversion_memset (void *d)
   /* Passing a ptrdiff_t where size_t is expected may not be unsafe
  but because GCC may emits suboptimal code for such calls warning
  for them helps improve efficiency.  */
-  memset (d, 0, diffi);   /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .long int.} where .long unsigned int. is expected" } */
+  memset (d, 0, diffi);   /* { dg-warning ".memset. argument 3 promotes to .ptrdiff_t. {aka .\(long \)?int.} where .\(long \)?unsigned int. is expected" } */
 
-  memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where 'long unsigned int' is expected" } */
+  memset (d, 0, 2.0); /* { dg-warning ".memset. argument 3 type is .double. where '\(long \)?unsigned int' is expected" } */
 
   /* Verify that the same call as above but to the built-in doesn't
  trigger a warning.  */
@@ -95,7 +95,7 @@ double fabs ();   /* { dg-message "built-i
 /* Expect a warning for fabsf below because even a float argument promotes
to double.  Unfortunately, invalid calls to fabsf() are not diagnosed.  */
 float fabsf ();   /* { dg-warning "conflicting types for built-in function .fabsf.; expected .float\\\(float\\\)." } */
-long double fabsl (); /* { dg-message "built-in .fabsl. declared here" } */
+long double fabsl (); /* { dg-message "built-in .fabsl. declared here" "large long double" { target large_long_double } } */
 
 void test_real_conversion_fabs (void)
 {
@@ -108,7 +108,8 @@ void test_real_conversion_fabs (void)
   /* In C, the type of an enumeration constant is int.  */
   d = fabs (e0);/* { dg-warning ".fabs. argument 1 type is .int. where .double. is expected in a call to built-in function declared without prototype" } */
 
-  d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" } */
+  d = fabs (e); /* { dg-warning ".fabs. argument 1 type is .enum E. where .double. is expected in a call to built-in function declared without prototype" "ordinary enum" { target { ! short_enums } } } */
+  /* { dg-warning ".fabs. argument 1 promotes to .int. where .double. is expected in a call to built-in function declared without prototype" "size 1 enum" { target short_enums } .-1 } */
 
   /* No warning here since float is promoted to double.  */
   d = fabs (f);
@@ -117,9 +118,9 @@ void test_real_conversion_fabs (void)
 
   d = fabsf (c);/* { dg-warning ".fabsf. argument 1 promotes to .int. where .float. is expected in a call to built-in function declared without prototype" "pr87890" { xfail *-*-* } } */
 
-  d = fabsl (c);/* { dg-warning ".fabsl. argument 1 promotes to .int. where .long double. is expected in a call to built-in function declared without prototype" } */
+  d = fabsl (c); 

Re: C++ PATCH to implement P1094R2, Nested inline namespaces

2018-11-19 Thread Jakub Jelinek
On Mon, Nov 19, 2018 at 04:21:19PM -0500, Marek Polacek wrote:
> 2018-11-19  Marek Polacek  
> 
>   Implement P1094R2, Nested inline namespaces.
>   * g++.dg/cpp2a/nested-inline-ns1.C: New test.
>   * g++.dg/cpp2a/nested-inline-ns2.C: New test.
>   * g++.dg/cpp2a/nested-inline-ns3.C: New test.

Just a small testsuite comment.

> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
> @@ -0,0 +1,26 @@
> +// P1094R2
> +// { dg-do compile { target c++2a } }

Especially because 2a testing isn't included by default, but also
to make sure it works right even with -std=c++17, wouldn't it be better to
drop the nested-inline-ns3.C test, make this test c++17 or
even better always enabled, add dg-options "-Wpedantic" and
just add dg-warning with c++17_down and c++14_down what should be
warned on the 3 lines (with .-1 for c++14_down)?

Or if you want add some further testcases that will test how
c++17 etc. will dg-error on those with -pedantic-errors etc.

> +
> +namespace A::inline B::C {
> +  int i;
> +}
> +
> +namespace D::E::inline F {
> +  int j;
> +}
> +
> +inline namespace X {
> +  int x;
> +}

> +// Make sure the namespaces are marked inline.
> +void
> +g ()
> +{
> +  A::B::C::i++;
> +  A::C::i++;
> +  D::E::j++;
> +  D::E::F::j++;
> +  X::x++;
> +  x++;
> +}

Jakub


Fix hashtable memory leak

2018-11-19 Thread François Dumont
There a memory leak when move assigning between 2 instances with 
different bucket count and non-propagating and unequal allocator 
instances (see new test03).


I reduced code duplication to fix the issue as 1 of the 2 
implementations was fine.


    * include/bits/hashtable.h (_Hashtable<>::_M_replicate): New.
    (_Hashtable<>::operator=(const _Hashtable&)): Use latter.
    (_Hashtable<>::_M_move_assign(_Hashtable&&, false_type)): Likewise.
    * testsuite/23_containers/unordered_set/allocator/move_assign.cc
    (test03): New.

I still need to run all tests but new move_assign.cc works fine.

Ok to commit once fully tested ?

François

diff --git a/libstdc++-v3/include/bits/hashtable.h b/libstdc++-v3/include/bits/hashtable.h
index efc4c4ab94f..61e177abb45 100644
--- a/libstdc++-v3/include/bits/hashtable.h
+++ b/libstdc++-v3/include/bits/hashtable.h
@@ -398,6 +398,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   _M_begin() const
   { return static_cast<__node_type*>(_M_before_begin._M_nxt); }
 
+  template
+	void
+	_M_replicate(_Ht&&, const _NodeGenerator&);
+
   template
 	void
 	_M_assign(const _Hashtable&, const _NodeGenerator&);
@@ -1058,6 +1062,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
 
   // Reuse allocated buckets and nodes.
+  __reuse_or_alloc_node_type __roan(_M_begin(), *this);
+  _M_replicate(__ht,
+		   [&__roan](const __node_type* __n)
+		   { return __roan(__n->_M_v()); });
+  return *this;
+}
+
+  template
+template
+  void
+  _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
+		 _H1, _H2, _Hash, _RehashPolicy, _Traits>::
+  _M_replicate(_Ht&& __ht, const _NodeGenerator& __node_gen)
+  {
 	__bucket_type* __former_buckets = nullptr;
 	std::size_t __former_bucket_count = _M_bucket_count;
 	const __rehash_state& __former_state = _M_rehash_policy._M_state();
@@ -1074,14 +1095,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 	__try
 	  {
-	  __hashtable_base::operator=(__ht);
+	__hashtable_base::operator=(std::forward<_Ht>(__ht));
 	_M_element_count = __ht._M_element_count;
 	_M_rehash_policy = __ht._M_rehash_policy;
-	  __reuse_or_alloc_node_type __roan(_M_begin(), *this);
 	_M_before_begin._M_nxt = nullptr;
-	  _M_assign(__ht,
-		[&__roan](const __node_type* __n)
-		{ return __roan(__n->_M_v()); });
+	_M_assign(__ht, __node_gen);
 	if (__former_buckets)
 	  _M_deallocate_buckets(__former_buckets, __former_bucket_count);
 	  }
@@ -1099,7 +1117,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 			 _M_bucket_count * sizeof(__bucket_type));
 	__throw_exception_again;
 	  }
-  return *this;
   }
 
   template_M_v())); });
 	  __ht.clear();
 	}
-	  __catch(...)
-	{
-	  if (__former_buckets)
-		{
-		  _M_deallocate_buckets();
-		  _M_rehash_policy._M_reset(__former_state);
-		  _M_buckets = __former_buckets;
-		  _M_bucket_count = __former_bucket_count;
-		}
-	  __builtin_memset(_M_buckets, 0,
-			   _M_bucket_count * sizeof(__bucket_type));
-	  __throw_exception_again;
-	}
-	}
 }
 
   template
+#include 
+
 #include 
 #include 
 #include 
@@ -27,7 +29,9 @@ using __gnu_test::counter_type;
 
 void test01()
 {
-  typedef propagating_allocator alloc_type;
+  {
+typedef propagating_allocator> alloc_type;
 typedef __gnu_test::counter_type_hasher hash;
 typedef std::unordered_set,
@@ -50,9 +54,15 @@ void test01()
 VERIFY( counter_type::destructor_count == 2 );
   }
 
+  // Check there's nothing left allocated or constructed.
+  __gnu_cxx::annotate_base::check();
+}
+
 void test02()
 {
-  typedef propagating_allocator alloc_type;
+  {
+typedef propagating_allocator> alloc_type;
 typedef __gnu_test::counter_type_hasher hash;
 typedef std::unordered_set,
@@ -80,9 +90,48 @@ void test02()
 VERIFY( it == v2.begin() );
   }
 
+  // Check there's nothing left allocated or constructed.
+  __gnu_cxx::annotate_base::check();
+}
+
+void test03()
+{
+  {
+typedef propagating_allocator> alloc_type;
+typedef __gnu_test::counter_type_hasher hash;
+typedef std::unordered_set,
+			   alloc_type> test_type;
+
+test_type v1(alloc_type(1));
+v1.emplace(0);
+
+test_type v2(alloc_type(2));
+int i = 0;
+v2.emplace(i++);
+for (; v2.bucket_count() == v1.bucket_count(); ++i)
+  v2.emplace(i);
+
+counter_type::reset();
+
+v2 = std::move(v1);
+
+VERIFY( 1 == v1.get_allocator().get_personality() );
+VERIFY( 2 == v2.get_allocator().get_personality() );
+
+VERIFY( counter_type::move_count == 1  );
+VERIFY( counter_type::destructor_count == i + 1 );
+  }
+
+  // Check there's nothing left allocated or constructed.
+  __gnu_cxx::annotate_base::check();
+}
+
 int main()
 {
   test01();
   test02();
+  test03();
   return 0;
 }



[PATCH] v3: C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850)

2018-11-19 Thread David Malcolm
On Fri, 2018-11-16 at 13:13 -0500, Jason Merrill wrote:
> On Thu, Nov 15, 2018 at 4:48 PM David Malcolm 
> wrote:
> > On Tue, 2018-11-13 at 16:34 -0500, Jason Merrill wrote:
> > > On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor 
> > > wrote:
> > > > On 11/11/2018 02:02 PM, David Malcolm wrote:
> > > > > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote:
> > > > > > On 11/10/2018 12:01 AM, Eric Gallager wrote:
> > > > > > > On 11/9/18, David Malcolm  wrote:
> > > > > > > > This patch adds a fix-it hint to various pointer-vs-
> > > > > > > > non-
> > > > > > > > pointer
> > > > > > > > diagnostics, suggesting the addition of a leading '&'
> > > > > > > > or
> > > > > > > > '*'.
> > > > > > > > 
> > > > > > > > For example, note the ampersand fix-it hint in the
> > > > > > > > following:
> > > > > > > > 
> > > > > > > > demo.c:5:22: error: invalid conversion from
> > > > > > > > 'pthread_key_t'
> > > > > > > > {aka
> > > > > > > > 'unsigned
> > > > > > > > int'}
> > > > > > > >to 'pthread_key_t*' {aka 'unsigned int*'} [-
> > > > > > > > fpermissive]
> > > > > > > > 5 |   pthread_key_create(key, NULL);
> > > > > > > >   |  ^~~
> > > > > > > >   |  |
> > > > > > > >   |  pthread_key_t {aka
> > > > > > > > unsigned
> > > > > > > > int}
> > > > > > > >   |  &
> > > > > > > 
> > > > > > > Having both the type and the fixit underneath the caret
> > > > > > > looks
> > > > > > > kind
> > > > > > > of confusing
> > > > > > 
> > > > > > I agree it's rather subtle.  Keeping the diagnostics
> > > > > > separate
> > > > > > from
> > > > > > the suggested fix should avoid the confusion.
> > > > > 
> > > > > FWIW, the fix-it hint is in a different color (assuming that
> > > > > gcc
> > > > > is
> > > > > invoked in an environment that prints that...)
> > > > 
> > > > I figured it would be, but I'm still not sure it's good design
> > > > to be relying on color alone to distinguish between the problem
> > > > and the suggested fix.  Especially when they are so close to
> > > > one
> > > > another and the fix is just a single character with no obvious
> > > > relationship to the rest of the text on the screen.  In other
> > > > warnings there's at least the "did you forget the '@'?" part
> > > > to give a clue, even though even there the connection between
> > > > the "did you forget" and the & several lines down wouldn't
> > > > necessarily be immediately apparent.
> > > 
> > > Agreed, something along those lines would help to understand why
> > > the
> > > compiler is throwing a random & into the diagnostic.
> > > 
> > > Jason
> > 
> > Here's an updated version which adds a note, putting the fix-it
> > hint
> > on that instead (I attempted adding the text to the initial error,
> > but there was something of a combinatorial explosion of messages).
> > 
> > The above example becomes:
> > 
> > demo.c: In function 'int main()':
> > demo.c:5:22: error: invalid conversion from 'pthread_key_t' {aka
> > 'unsigned int'}
> >to 'pthread_key_t*' {aka 'unsigned int*'} [-fpermissive]
> > 5 |   pthread_key_create(key, NULL);
> >   |  ^~~
> >   |  |
> >   |  pthread_key_t {aka unsigned int}
> > demo.c:5:22: note: possible fix: take the address with '&'
> > 5 |   pthread_key_create(key, NULL);
> >   |  ^~~
> >   |  &
> > In file included from demo.c:1:
> > /usr/include/pthread.h:1122:47: note:   initializing argument 1 of
> >'int pthread_key_create(pthread_key_t*, void (*)(void*))'
> >  1122 | extern int pthread_key_create (pthread_key_t *__key,
> >   |~~~^
> > 
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > 
> > OK for trunk?
> > 
> > gcc/c-family/ChangeLog:
> > PR c++/87850
> > * c-common.c: Include "gcc-rich-location.h".
> > (maybe_emit_indirection_note): New function.
> > * c-common.h (maybe_emit_indirection_note): New decl.
> > 
> > gcc/c/ChangeLog:
> > PR c++/87850
> > * c-typeck.c (convert_for_assignment): Call
> > maybe_emit_indirection_note for pointer vs non-pointer
> > diagnostics.
> > 
> > gcc/cp/ChangeLog:
> > PR c++/87850
> > * call.c (convert_like_real): Call
> > maybe_emit_indirection_note for "invalid conversion"
> > diagnostic.
> > 
> > gcc/testsuite/ChangeLog:
> > PR c++/87850
> > * c-c++-common/indirection-fixits.c: New test.
> > ---
> >  gcc/c-family/c-common.c |  33 +++
> >  gcc/c-family/c-common.h |   2 +
> >  gcc/c/c-typeck.c|  10 +-
> >  gcc/cp/call.c   |   2 +
> >  gcc/testsuite/c-c++-common/indirection-fixits.c | 270
> > 
> >  5 files changed, 315 insertions(+), 2 del

C++ PATCH to implement P1094R2, Nested inline namespaces

2018-11-19 Thread Marek Polacek
This patch implements another C++20 feature, nested inline namespaces.

This was fairly simple, one just has to be careful not to blithely consume the
inline keyword, making a non-valid program valid.  Another minor gotcha was
to handle the innermost 'inline' correctly.

Note that 

  inline namespace A::B { ... }

continues to be invalid.

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

2018-11-19  Marek Polacek  

Implement P1094R2, Nested inline namespaces.
* parser.c (cp_parser_namespace_definition): Parse the optional inline
keyword in a nested-namespace-definition.  Adjust push_namespace call.
Formatting fix.

* g++.dg/cpp2a/nested-inline-ns1.C: New test.
* g++.dg/cpp2a/nested-inline-ns2.C: New test.
* g++.dg/cpp2a/nested-inline-ns3.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 88fc426102b..8b8304acca7 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -18864,6 +18864,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   cp_ensure_no_oacc_routine (parser);
 
   bool is_inline = cp_lexer_next_token_is_keyword (parser->lexer, RID_INLINE);
+  const bool topmost_inline_p = is_inline;
 
   if (is_inline)
 {
@@ -18882,6 +18883,17 @@ cp_parser_namespace_definition (cp_parser* parser)
 {
   identifier = NULL_TREE;
   
+  bool nested_inline_p = cp_lexer_next_token_is_keyword (parser->lexer,
+RID_INLINE);
+  if (nested_inline_p && nested_definition_count != 0)
+   {
+ if (cxx_dialect < cxx2a)
+   pedwarn (cp_lexer_peek_token (parser->lexer)->location,
+OPT_Wpedantic, "nested inline namespace definitions only "
+"available with -std=c++2a or -std=gnu++2a");
+ cp_lexer_consume_token (parser->lexer);
+   }
+
   if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
{
  identifier = cp_parser_identifier (parser);
@@ -18896,7 +18908,12 @@ cp_parser_namespace_definition (cp_parser* parser)
}
 
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_SCOPE))
-   break;
+   {
+ /* Don't forget that the innermost namespace might have been
+marked as inline.  */
+ is_inline |= nested_inline_p;
+ break;
+   }
   
   if (!nested_definition_count && cxx_dialect < cxx17)
 pedwarn (input_location, OPT_Wpedantic,
@@ -18905,7 +18922,9 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   /* Nested namespace names can create new namespaces (unlike
 other qualified-ids).  */
-  if (int count = identifier ? push_namespace (identifier) : 0)
+  if (int count = (identifier
+  ? push_namespace (identifier, nested_inline_p)
+  : 0))
nested_definition_count += count;
   else
cp_parser_error (parser, "nested namespace name required");
@@ -18918,7 +18937,7 @@ cp_parser_namespace_definition (cp_parser* parser)
   if (nested_definition_count && attribs)
 error_at (token->location,
  "a nested namespace definition cannot have attributes");
-  if (nested_definition_count && is_inline)
+  if (nested_definition_count && topmost_inline_p)
 error_at (token->location,
  "a nested namespace definition cannot be inline");
 
@@ -18927,7 +18946,7 @@ cp_parser_namespace_definition (cp_parser* parser)
 
   bool has_visibility = handle_namespace_attrs (current_namespace, attribs);
 
-  warning  (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
+  warning (OPT_Wnamespaces, "namespace %qD entered", current_namespace);
 
   /* Look for the `{' to validate starting the namespace.  */
   matching_braces braces;
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C 
gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
new file mode 100644
index 000..95b4d3378d1
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns1.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
+
+namespace A::inline B::C {
+  int i;
+}
+
+namespace D::E::inline F {
+  int j;
+}
+
+inline namespace X {
+  int x;
+}
+
+// Make sure the namespaces are marked inline.
+void
+g ()
+{
+  A::B::C::i++;
+  A::C::i++;
+  D::E::j++;
+  D::E::F::j++;
+  X::x++;
+  x++;
+}
diff --git gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C 
gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
new file mode 100644
index 000..9b5f2cab47b
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/nested-inline-ns2.C
@@ -0,0 +1,26 @@
+// P1094R2
+// { dg-do compile { target c++2a } }
+
+inline namespace A::B { // { dg-error "a nested namespace definition cannot be 
inline" }
+  int i;
+}
+
+namespace inline C::D { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+namespace E::F inline { // { dg-error "expected" }
+  int i;
+}
+
+namespace inline G { // { dg-error "expected|does not name a type" }
+  int i;
+}
+
+inline namespace i

Re: Fix hashtable node deallocation

2018-11-19 Thread François Dumont

On 11/19/18 1:34 PM, Jonathan Wakely wrote:

On 10/11/18 22:40 +0100, François Dumont wrote:
While working on a hashtable enhancement I noticed that we are not 
using the correct method to deallocate node if the constructor throws 
in _ReuseOrAllocNode operator(). I had to introduce a new 
_M_deallocate_node_ptr for that as node value shall not be destroy 
again.


I also check other places and noticed that a __node_type destructor 
call was missing.


That's intentional. The type has a trivial destructor, so its storage
can just be reused, we don't need to destroy it.



Ok, do you want to also remove the other call to ~__node_type() then ?

Here is the updated patch and the right ChangeLog entry:

    * include/bits/hashtable_policy.h
(_Hashtable_alloc<>::_M_deallocate_node_ptr(__node_type*)): New.
    (_Hashtable_alloc<>::_M_deallocate_node(__node_type*)): Use latter.
(_ReuseOrAllocNode<>::operator<_Arg>()(_Arg&&)): Likewise.
    (_Hashtable_alloc<>::_M_allocate_node): Add ~__node_type call.
    * libstdc++-v3/testsuite/util/testsuite_allocator.h
    (CustomPointerAlloc<>::allocate(size_t, pointer)): Replace by...
    (CustomPointerAlloc<>::allocate(size_t, const_void_pointer)): ...this.
    * testsuite/23_containers/unordered_set/allocator/ext_ptr.cc: Add
    check.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index d8e8c13858d..acc6f41a0ed 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -148,8 +148,7 @@ namespace __detail
 		}
 	  __catch(...)
 		{
-		  __node->~__node_type();
-		  __node_alloc_traits::deallocate(__a, __node, 1);
+		  _M_h._M_deallocate_node_ptr(__node);
 		  __throw_exception_again;
 		}
 	  return __node;
@@ -2116,6 +2115,9 @@ namespace __detail
   void
   _M_deallocate_node(__node_type* __n);
 
+  void
+  _M_deallocate_node_ptr(__node_type* __n);
+
   // Deallocate the linked list of nodes pointed to by __n
   void
   _M_deallocate_nodes(__node_type* __n);
@@ -2154,10 +2156,17 @@ namespace __detail
   template
 void
 _Hashtable_alloc<_NodeAlloc>::_M_deallocate_node(__node_type* __n)
+{
+  __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
+  _M_deallocate_node_ptr(__n);
+}
+
+  template
+void
+_Hashtable_alloc<_NodeAlloc>::_M_deallocate_node_ptr(__node_type* __n)
 {
   typedef typename __node_alloc_traits::pointer _Ptr;
   auto __ptr = std::pointer_traits<_Ptr>::pointer_to(*__n);
-  __node_alloc_traits::destroy(_M_node_allocator(), __n->_M_valptr());
   __n->~__node_type();
   __node_alloc_traits::deallocate(_M_node_allocator(), __ptr, 1);
 }
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
index 7e4a6e02900..3b2a9a1ab35 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/allocator/ext_ptr.cc
@@ -41,6 +41,9 @@ void test01()
   test_type v;
   v.insert(T());
   VERIFY( ++v.begin() == v.end() );
+
+  v = { { 1 }, { 2 }, { 3 }};
+  VERIFY( v.size() == 3 );
 }
 
 int main()
diff --git a/libstdc++-v3/testsuite/util/testsuite_allocator.h b/libstdc++-v3/testsuite/util/testsuite_allocator.h
index b0fecfb59a3..c18223475c9 100644
--- a/libstdc++-v3/testsuite/util/testsuite_allocator.h
+++ b/libstdc++-v3/testsuite/util/testsuite_allocator.h
@@ -582,7 +582,7 @@ namespace __gnu_test
   typedef Ptr		void_pointer;
   typedef Ptr	const_void_pointer;
 
-  pointer allocate(std::size_t n, pointer = {})
+  pointer allocate(std::size_t n, const_void_pointer = {})
   { return pointer(std::allocator::allocate(n)); }
 
   void deallocate(pointer p, std::size_t n)


Re: C++ PATCH for c++/87781, detect invalid elaborated-type-specifier

2018-11-19 Thread Jason Merrill
OK, thanks.
On Wed, Nov 14, 2018 at 3:32 PM Marek Polacek  wrote:
>
> On Wed, Nov 14, 2018 at 10:03:50AM -0500, Jason Merrill wrote:
> > On Wed, Nov 14, 2018 at 9:55 AM Marek Polacek  wrote:
> > >
> > > In elaborated-type-specifier, the typename keyword can only follow a
> > > nested-name-specifier:
> > >
> > >   class-key nested-name-specifier template[opt] simple-template-id
> > >
> > > but we weren't detecting it.
> > >
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > >
> > > 2018-11-14  Marek Polacek  
> > >
> > > PR c++/87781 - detect invalid elaborated-type-specifier.
> > > * parser.c (cp_parser_elaborated_type_specifier): Ensure that
> > > typename follows a nested-name-specifier.
> > >
> > > * g++.dg/parse/elab3.C: New test.
> > >
> > > diff --git gcc/cp/parser.c gcc/cp/parser.c
> > > index e9e49b15702..0ab44ab93e3 100644
> > > --- gcc/cp/parser.c
> > > +++ gcc/cp/parser.c
> > > @@ -17986,6 +17986,10 @@ cp_parser_elaborated_type_specifier (cp_parser* 
> > > parser,
> > >  template-id or not.  */
> > >if (!template_p)
> > > cp_parser_parse_tentatively (parser);
> > > +  /* The `template' keyword must follow a nested-name-specifier.  */
> > > +  else if (!nested_name_specifier)
> > > +   return error_mark_node;
> >
> > Don't we want a diagnostic here?
>
> We'd get "invalid declarator" even without a diagnostic there but I guess it'd
> be nicer so say what the actual problem is.  Unsure which diagnostic to go 
> with,
> this patch uses cp_parser_error though.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-11-14  Marek Polacek  
>
> PR c++/87781 - detect invalid elaborated-type-specifier.
> * parser.c (cp_parser_elaborated_type_specifier): Ensure that
> typename follows a nested-name-specifier.
>
> * g++.dg/parse/elab3.C: New test.
> * g++.dg/template/crash115.C: Adjust dg-error.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index e9e49b15702..bfcf42b0f39 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -17986,6 +17986,14 @@ cp_parser_elaborated_type_specifier (cp_parser* 
> parser,
>  template-id or not.  */
>if (!template_p)
> cp_parser_parse_tentatively (parser);
> +  /* The `template' keyword must follow a nested-name-specifier.  */
> +  else if (!nested_name_specifier)
> +   {
> + cp_parser_error (parser, "% must follow a nested-"
> +  "name-specifier");
> + return error_mark_node;
> +   }
> +
>/* Parse the template-id.  */
>token = cp_lexer_peek_token (parser->lexer);
>decl = cp_parser_template_id (parser, template_p,
> diff --git gcc/testsuite/g++.dg/parse/elab3.C 
> gcc/testsuite/g++.dg/parse/elab3.C
> new file mode 100644
> index 000..61338fb7ac4
> --- /dev/null
> +++ gcc/testsuite/g++.dg/parse/elab3.C
> @@ -0,0 +1,6 @@
> +// PR c++/87781
> +// { dg-do compile }
> +// { dg-options "" }
> +
> +template class A;
> +class template A *p; // { dg-error ".template. must follow|invalid" }
> diff --git gcc/testsuite/g++.dg/template/crash115.C 
> gcc/testsuite/g++.dg/template/crash115.C
> index 5c9f525cd64..80f8683a136 100644
> --- gcc/testsuite/g++.dg/template/crash115.C
> +++ gcc/testsuite/g++.dg/template/crash115.C
> @@ -1,3 +1,3 @@
>  // PR c++/56534
>
> -template < struct template rebind < > // { dg-error "expected" }
> +template < struct template rebind < > // { dg-error "expected|must follow" }


Re: Tweak ALAP calculation in SCHED_PRESSURE_MODEL

2018-11-19 Thread Pat Haugen
On 11/19/18 11:54 AM, Kyrill Tkachov wrote:
> On 16/11/18 18:19, Pat Haugen wrote:
>> On 11/8/18 6:10 AM, Kyrill Tkachov wrote:
>>> The attached patch avoids that by making the alap calculation only
>>> look at true dependencies.  This shouldn't be too bad, since we use
>>> INSN_PRIORITY as the final tie-breaker than that does take
>>> anti-dependencies into account.
>>>
>>> This reduces the number of spills in the hot function from 436.cactusADM
>>> by 14% on aarch64 at -O3 (and the number of instructions in general).
>>> SPEC2017 shows a minor improvement on Cortex-A72 (about 0.1% overall).
>>> Thanks to Wilco for the benchmarking.
>> I tried the patch on PowerPC since it also uses SCHED_PRESSURE_MODEL 
>> algorithm. For CPU2006 only cactusADM had a noticeable difference, but I'm 
>> seeing a 5% degradation. Looking at the generated asm for function 
>> bench_staggeredleapfrog2_(), I see about a 1% increase in number of loads 
>> and stores generated and an extra 100 bytes allocated on the stack.
>>
>> -Pat
>>
> 
> This is a follow-up from 
> https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01525.html
> This version introduces an "artificial" property of the dependencies produced 
> in
> sched-deps.c that is recorded when they are created due to 
> MAX_PENDING_LIST_LENGTH
> and they are thus ignored in the model_analyze_insns ALAP calculation.
> 
> This approach gives most of the benefits of the original patch [1] on aarch64.
> I tried it on the cactusADM hot function (bench_staggeredleapfrog2_) on 
> powerpc64le-unknown-linux-gnu
> with -O3 and found that the initial version proposed did indeed increase the 
> instruction count
> and stack space. This version gives a small improvement on powerpc in terms 
> of instruction count
> (number of st* instructions stays the same), so I'm hoping this version 
> addresses Pat's concerns.
> Pat, could you please try this version out if you've got the chance?
> 

I tried the new verison on cactusADM, it's showing a 2% degradation. I've 
kicked off a full CPU2006 run just to see if any others are affected.

-Pat



[PATCH, middle-end]: Fix PR 88070, ICE in create_pre_exit, at mode-switching.c:438

2018-11-19 Thread Uros Bizjak
Hello!

The assert in create_pre_exit at mode-switching.c expects return copy
pair with nothing in between. However, the compiler starts mode
switching pass with the following sequence:

(insn 19 18 16 2 (set (reg:V2SF 21 xmm0)
(mem/c:V2SF (plus:DI (reg/f:DI 7 sp)
(const_int -72 [0xffb8])) [0  S8 A64]))
"pr88070.c":8 1157 {*movv2sf_internal}
 (nil))
(insn 16 19 20 2 (set (reg:V2SF 0 ax [orig:91  ] [91])
(reg:V2SF 0 ax [89])) "pr88070.c":8 1157 {*movv2sf_internal}
 (nil))
(insn 20 16 21 2 (unspec_volatile [
(const_int 0 [0])
] UNSPECV_BLOCKAGE) "pr88070.c":8 710 {blockage}
 (nil))
(insn 21 20 23 2 (use (reg:V2SF 21 xmm0)) "pr88070.c":8 -1
 (nil))

Please note how (insn 16) interferes with (insn 19)-(insn 21) return copy pair.

The culprit for this is the blockage instruction (insn 20), which
causes sched1 pass (pre reload scheduler) to skip marking (insn 19) as
unmovable instruction (as a dependent insn on return use insn), so the
scheduler is free to schedule (insn 16) between return copy pair (insn
19)-(insn 21).

The extra instruction is generated as a kludge in expand_function_end
to prevent instructions that may trap to be scheduled into function
epilogue. However, the same blockage is generated under exactly the
same conditions earlier in the expand_function_end. The first blockage
should prevent unwanted scheduling into the epilogue, so there is
actually no need for the second one.

Attached patch removes the kludge.

BTW: The extra blockage would crash compilation for all mode-switching
targets, also in the pre-reload mode switching; the vzeroupper
post-reload insertion just trips x86 target on a generic problem in
the middle-end.

2018-11-19  Uros Bizjak  

PR middle-end/88070
* function.c (expand_function_end): Remove kludge that
generates second blockage insn.

testsuite/ChangeLog:

2018-11-19  Uros Bizjak  

PR middle-end/88070
* gcc.target/i386/pr88070.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu
{,-m32} for all default languages, obj-c++ and go.

OK for mainline and release branches?

Uros.
Index: function.c
===
--- function.c  (revision 266278)
+++ function.c  (working copy)
@@ -5447,13 +5447,6 @@ expand_function_end (void)
   if (naked_return_label)
 emit_label (naked_return_label);
 
-  /* @@@ This is a kludge.  We want to ensure that instructions that
- may trap are not moved into the epilogue by scheduling, because
- we don't always emit unwind information for the epilogue.  */
-  if (cfun->can_throw_non_call_exceptions
-  && targetm_common.except_unwind_info (&global_options) != UI_SJLJ)
-emit_insn (gen_blockage ());
-
   /* If stack protection is enabled for this function, check the guard.  */
   if (crtl->stack_protect_guard && targetm.stack_protect_runtime_enabled_p ())
 stack_protect_epilogue ();
Index: testsuite/gcc.target/i386/pr88070.c
===
--- testsuite/gcc.target/i386/pr88070.c (nonexistent)
+++ testsuite/gcc.target/i386/pr88070.c (working copy)
@@ -0,0 +1,12 @@
+/* PR target/88070 */
+/* { dg-do compile } */
+/* { dg-options "-O -fexpensive-optimizations -fnon-call-exceptions 
-fschedule-insns -fno-dce -fno-dse -mavx" } */
+
+typedef float vfloat2 __attribute__ ((__vector_size__ (2 * sizeof (float;
+
+vfloat2
+test1float2 (float c)
+{
+  vfloat2 v = { c, c };
+  return v;
+}


Re: [PATCH][LRA] Fix PR88033: ICE in remove_some_program_points_and_update_live_ranges

2018-11-19 Thread Peter Bergner
On 11/19/18 1:17 PM, Vladimir Makarov wrote:
> On 11/16/2018 02:15 PM, Peter Bergner wrote:
>> PR88033 shows a problem when handling simple copies from a register to 
>> itself:
>>
>>    (insn (set (reg:DI NNN) (reg:DI NNN)))
>>
>> This was causing confusion in the code that performs liveness and conflict
>> updates and program point updates in lra-lives.c.  Trying to handle these
>> types of copies would add some ugly code.  It's easier to just bail on them
>> in non_conflicting_reg_copy_p() altogether, since by definition, a register
>> does not conflict with itself and so needs no special handling.  The patch
>> below implements that and fixes the ICE.
>>
>> This is currently bootstrapping and regtesting on x86_64-linux.
>> Ok for mainline assuming the tests show no regressions?
>>
> OK.  Thank you, Peter.

Thanks, this is now committed.

Peter



Re: [PATCH][LRA] Fix PR88033: ICE in remove_some_program_points_and_update_live_ranges

2018-11-19 Thread Vladimir Makarov




On 11/16/2018 02:15 PM, Peter Bergner wrote:

PR88033 shows a problem when handling simple copies from a register to itself:

   (insn (set (reg:DI NNN) (reg:DI NNN)))

This was causing confusion in the code that performs liveness and conflict
updates and program point updates in lra-lives.c.  Trying to handle these
types of copies would add some ugly code.  It's easier to just bail on them
in non_conflicting_reg_copy_p() altogether, since by definition, a register
does not conflict with itself and so needs no special handling.  The patch
below implements that and fixes the ICE.

This is currently bootstrapping and regtesting on x86_64-linux.
Ok for mainline assuming the tests show no regressions?


OK.  Thank you, Peter.



gcc/
PR rtl-optimization/88033
* ira-lives.c (non_conflicting_reg_copy_p): Skip copies from a register
to itself.  Use HARD_REGISTER_NUM_P.

gcc/testsuite/
PR rtl-optimization/88033
* gcc.target/i386/pr88033.c: New test.


Index: gcc/ira-lives.c
===
--- gcc/ira-lives.c (revision 266207)
+++ gcc/ira-lives.c (working copy)
@@ -1083,11 +1083,17 @@ non_conflicting_reg_copy_p (rtx_insn *insn)
int src_regno = REGNO (SET_SRC (set));
machine_mode mode = GET_MODE (SET_DEST (set));
  
+  /* By definition, a register does not conflict with itself, therefore we

+ do not have to handle it specially.  Returning NULL_RTX now, helps
+ simplify the callers of this function.  */
+  if (dst_regno == src_regno)
+return NULL_RTX;
+
/* Computing conflicts for register pairs is difficult to get right, so
   for now, disallow it.  */
-  if ((dst_regno < FIRST_PSEUDO_REGISTER
+  if ((HARD_REGISTER_NUM_P (dst_regno)
 && hard_regno_nregs (dst_regno, mode) != 1)
-  || (src_regno < FIRST_PSEUDO_REGISTER
+  || (HARD_REGISTER_NUM_P (src_regno)
  && hard_regno_nregs (src_regno, mode) != 1))
  return NULL_RTX;
  
Index: gcc/testsuite/gcc.target/i386/pr88033.c

===
--- gcc/testsuite/gcc.target/i386/pr88033.c (nonexistent)
+++ gcc/testsuite/gcc.target/i386/pr88033.c (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int
+main (long a, long *b, long c)
+{
+  if (!c)
+return 0;
+  int g;
+  *b = (g & ~30) < 0 ? a : a - g;
+  while (1)
+;
+  return 0;
+}





[C++ Patch] PR 84636 ("internal compiler error: Segmentation fault (identifier_p()/grokdeclarator())")

2018-11-19 Thread Paolo Carlini

Hi,

while working on some additional location fixes to grokbitfield - 
preparing testcases - I stumbled into this ICE on invalid, where in 
grokdeclarator we try to use identifier_p on a null unqualified_id. In 
practice, clang and edg behave in different ways and since I couldn't 
decide which one I liked best I prepared two different patches which, 
respectively, implement similar behaviors: the former avoids the 
grokdeclarator ICE and delegates to the checks in grokbitfield for the 
diagnostics; the latter issues a specific error about the missing 
identifier after a function type (note, in any case we want to do that 
when staticp == 2 too, otherwise we crash later for a static version of 
the declaration). Tested both onx86_64-linux, as usual.


Thanks, Paolo.



Index: cp/decl.c
===
--- cp/decl.c   (revision 266268)
+++ cp/decl.c   (working copy)
@@ -12165,7 +12165,8 @@ grokdeclarator (const cp_declarator *declarator,
 }
 
   if (ctype && TREE_CODE (type) == FUNCTION_TYPE && staticp < 2
-  && !(identifier_p (unqualified_id)
+  && !(unqualified_id
+  && identifier_p (unqualified_id)
   && IDENTIFIER_NEWDEL_OP_P (unqualified_id)))
 {
   cp_cv_quals real_quals = memfn_quals;
@@ -12245,8 +12246,9 @@ grokdeclarator (const cp_declarator *declarator,
error ("invalid use of %<::%>");
return error_mark_node;
  }
-   else if (TREE_CODE (type) == FUNCTION_TYPE
-|| TREE_CODE (type) == METHOD_TYPE)
+   else if ((TREE_CODE (type) == FUNCTION_TYPE
+ || TREE_CODE (type) == METHOD_TYPE)
+&& !bitfield)
  {
int publicp = 0;
tree function_context;
Index: testsuite/g++.dg/parse/bitfield3.C
===
--- testsuite/g++.dg/parse/bitfield3.C  (revision 266263)
+++ testsuite/g++.dg/parse/bitfield3.C  (working copy)
@@ -5,5 +5,5 @@ typedef void (func_type)();
 
 struct A
 {
-  friend func_type f : 2; /* { dg-error "with non-integral type" } */
+  friend func_type f : 2; /* { dg-error "20:.f. is neither function nor member 
function" } */
 };
Index: testsuite/g++.dg/parse/bitfield6.C
===
--- testsuite/g++.dg/parse/bitfield6.C  (nonexistent)
+++ testsuite/g++.dg/parse/bitfield6.C  (working copy)
@@ -0,0 +1,6 @@
+// PR c++/84636
+
+typedef void a();
+struct A {
+a: 1;  // { dg-error "bit-field .\\. with non-integral type" }
+};
Index: cp/decl.c
===
--- cp/decl.c   (revision 266268)
+++ cp/decl.c   (working copy)
@@ -12164,15 +12164,24 @@ grokdeclarator (const cp_declarator *declarator,
type = build_pointer_type (type);
 }
 
-  if (ctype && TREE_CODE (type) == FUNCTION_TYPE && staticp < 2
-  && !(identifier_p (unqualified_id)
-  && IDENTIFIER_NEWDEL_OP_P (unqualified_id)))
+  if (ctype && TREE_CODE (type) == FUNCTION_TYPE)
 {
-  cp_cv_quals real_quals = memfn_quals;
-  if (cxx_dialect < cxx14 && constexpr_p
- && sfk != sfk_constructor && sfk != sfk_destructor)
-   real_quals |= TYPE_QUAL_CONST;
-  type = build_memfn_type (type, ctype, real_quals, rqual);
+  if (!unqualified_id)
+   {
+ error_at (declspecs->locations[ds_type_spec],
+   "declarator requires an identifier");
+ type = error_mark_node;
+   }
+  else if (staticp < 2
+  && !(identifier_p (unqualified_id)
+   && IDENTIFIER_NEWDEL_OP_P (unqualified_id)))
+   {
+ cp_cv_quals real_quals = memfn_quals;
+ if (cxx_dialect < cxx14 && constexpr_p
+ && sfk != sfk_constructor && sfk != sfk_destructor)
+   real_quals |= TYPE_QUAL_CONST;
+ type = build_memfn_type (type, ctype, real_quals, rqual);
+   }
 }
 
   {
Index: testsuite/g++.dg/parse/bitfield6.C
===
--- testsuite/g++.dg/parse/bitfield6.C  (nonexistent)
+++ testsuite/g++.dg/parse/bitfield6.C  (working copy)
@@ -0,0 +1,6 @@
+// PR c++/84636
+
+typedef void a();
+struct A {
+  a: 1;  // { dg-error "3:declarator requires an identifier" }
+};


[PATCH, ARM] Improve robustness of -mslow-flash-data

2018-11-19 Thread Thomas Preudhomme

Hi,

Current code to handle -mslow-flash-data in machine description files
suffers from a number of issues which this patch fixes:

1) The insn_and_split in vfp.md to load a generic floating-point
constant via GPR first and move it to VFP register are guarded by
!reload_completed which is forbidden explicitely in the GCC internals
documentation section 17.2 point 3;

2) A number of testcase in the testsuite ICEs under -mslow-flash-data
when targeting the hardfloat ABI [1];

3) Instructions performing load from literal pool are not disabled.

These problems are addressed by 2 separate actions:

1) Making the splitters take a clobber and changing the expanders
accordingly to generate a mov with clobber in cases where a literal
pool would be used. The splitter can thus be enabled after reload since
it does not call gen_reg_rtx anymore;

2) Adding new predicates and constraints to disable literal pool loads
in existing instructions when -mslow-flash-data is in effect.

The patch also rework the splitter for DFmode slightly to generate an
intermediate DI load instead of 2 intermediate SI loads, thus relying on
the existing DI splitters instead of redoing their job. At last, the
patch adds some missing arm_fp_ok effective target to some of the
slow-flash-data testcases.

[1]
c-c++-common/Wunused-var-3.c
gcc.c-torture/compile/pr72771.c
gcc.c-torture/compile/vector-5.c
gcc.c-torture/compile/vector-6.c
gcc.c-torture/execute/20030914-1.c
gcc.c-torture/execute/20050316-1.c
gcc.c-torture/execute/pr59643.c
gcc.dg/builtin-tgmath-1.c
gcc.dg/debug/pr55730.c
gcc.dg/graphite/interchange-7.c
gcc.dg/pr56890-2.c
gcc.dg/pr68474.c
gcc.dg/pr80286.c
gcc.dg/torture/pr35227.c
gcc.dg/torture/pr65077.c
gcc.dg/torture/pr86363.c
g++.dg/torture/pr81112.C
g++.dg/torture/pr82985.C
g++.dg/warn/Wunused-var-7.C
and a lot more in libstdc++ in special_functions/*_comp_ellint_* and
special_functions/*_ellint_* directories.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-11-14  Thomas Preud'homme  

* config/arm/arm.md (arm_movdi): Split if -mslow-flash-data and
source is a constant that would be loaded by literal pool.
(movsf expander): Generate a no_literal_pool_sf_immediate insn if
-mslow-flash-data is present, targeting hardfloat ABI and source is a
float constant that cannot be loaded via vmov.
(movdf expander): Likewise but generate a no_literal_pool_df_immediate
insn.
(arm_movsf_soft_insn): Split if -mslow-flash-data and source is a
float constant that would be loaded by literal pool.
(softfloat constant movsf splitter): Splitter for the above case.
(movdf_soft_insn): Split if -mslow-flash-data and source is a float
constant that would be loaded by literal pool.
(softfloat constant movdf splitter): Splitter for the above case.
* config/arm/constraints.md (Pz): Document existing constraint.
(Ha): Define constraint.
(Tu): Likewise.
* config/arm/predicates.md (hard_sf_operand): New predicate.
(hard_df_operand): Likewise.
* config/arm/thumb2.md (thumb2_movsi_insn): Split if
-mslow-flash-data and constant would be loaded by literal pool.
* constant/arm/vfp.md (thumb2_movsi_vfp): Likewise and disable constant
load in VFP register.
(movdi_vfp): Likewise.
(thumb2_movsf_vfp): Use hard_sf_operand as predicate for source to
prevent match for a constant load if -mslow-flash-data and constant
cannot be loaded via vmov.  Adapt constraint accordingly by
using Ha instead of E for generic floating-point constant load.
(thumb2_movdf_vfp): Likewise using hard_df_operand predicate instead.
(no_literal_pool_df_immediate): Add a clobber to use as the
intermediate general purpose register and also enable it after reload
but disable it constant is a valid FP constant.  Add constraints and
generate a DI intermediate load rather than 2 SI loads.
(no_literal_pool_sf_immediate): Add a clobber to use as the
intermediate general purpose register and also enable it after
reload.

*** gcc/testsuite/ChangeLog ***

2018-11-14  Thomas Preud'homme  

* gcc.target/arm/thumb2-slow-flash-data-2.c: Require arm_fp_ok
effective target.
* gcc.target/arm/thumb2-slow-flash-data-3.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-4.c: Likewise.
* gcc.target/arm/thumb2-slow-flash-data-5.c: Likewise.

Testing: Built arm-none-eabi cross compilers for Armv7E-M defaulting to
softfloat and hardfloat ABI which showed no regression and some
FAIL->PASS for hardfloat ABI. Bootstraped on Arm and Thumb-2 without any
regression. Compiled SPEC2k6 without -mslow-flash-data and checked that
code generation didn't change.

Is this ok for stage3?

Best regards,

Thomas
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index a773518cefaf8451e77fead9e072ee8ef39f

Re: Tweak ALAP calculation in SCHED_PRESSURE_MODEL

2018-11-19 Thread Kyrill Tkachov


On 16/11/18 18:19, Pat Haugen wrote:

On 11/8/18 6:10 AM, Kyrill Tkachov wrote:

The attached patch avoids that by making the alap calculation only
look at true dependencies.  This shouldn't be too bad, since we use
INSN_PRIORITY as the final tie-breaker than that does take
anti-dependencies into account.

This reduces the number of spills in the hot function from 436.cactusADM
by 14% on aarch64 at -O3 (and the number of instructions in general).
SPEC2017 shows a minor improvement on Cortex-A72 (about 0.1% overall).
Thanks to Wilco for the benchmarking.

I tried the patch on PowerPC since it also uses SCHED_PRESSURE_MODEL algorithm. 
For CPU2006 only cactusADM had a noticeable difference, but I'm seeing a 5% 
degradation. Looking at the generated asm for function 
bench_staggeredleapfrog2_(), I see about a 1% increase in number of loads and 
stores generated and an extra 100 bytes allocated on the stack.

-Pat



This is a follow-up from 
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01525.html
This version introduces an "artificial" property of the dependencies produced in
sched-deps.c that is recorded when they are created due to 
MAX_PENDING_LIST_LENGTH
and they are thus ignored in the model_analyze_insns ALAP calculation.

This approach gives most of the benefits of the original patch [1] on aarch64.
I tried it on the cactusADM hot function (bench_staggeredleapfrog2_) on 
powerpc64le-unknown-linux-gnu
with -O3 and found that the initial version proposed did indeed increase the 
instruction count
and stack space. This version gives a small improvement on powerpc in terms of 
instruction count
(number of st* instructions stays the same), so I'm hoping this version 
addresses Pat's concerns.
Pat, could you please try this version out if you've got the chance?

In terms of implementation, it required extending the various add_dependency 
functions/helpers to
take an extra argument marking the dependency as "artificial", which is what 
most of the patch diff
does. It is otherwise a fairly simple patch.

Bootstrapped and tested on aarch64-none-linux-gnu.

Is this ok for trunk if the PPC performance results look ok?


[1] https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01514.html

2018-11-19  Richard Sandiford  
Kyrylo Tkachov  

* haifa-sched.c (model_analyze_insns): Avoid counting artificial
anti-dependencies.
* sched-int.h (struct _dep): Add artificial field.
(DEP_ARTIFICIAL): Define accessor.
(DEP_ANTI_ARTIFICIAL): Define.
(DEP_POSTPONED): Adjust definition.
(add_dependence): Add default bool argument to prototype.
* sched-deps.c (init_dep_1): Initialize artificial field.
(add_dependence_1): Add default bool parameter.  Handle in definition.
(add_dependence_list): Likewise.
(add_dependence_list_and_free): Likewise.
(flush_pending_lists): Likewise.
(haifa_note_dep): Handle DEP_ANTI_ARTIFICIAL in ds.
(sched_analyze_1): Mark new dependencies created as part of handling
MAX_PENDING_LIST_LENGTH limit as artificial.
(sched_analyze_2): Likewise.
(sched_analyze_insn): Likewise.
(deps_analyze_insn): Likewise.
(dump_ds): Handle DEP_ANTI_ARTIFICIAL.

diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 2c84ce3814357b30e1aaed57f1de3034d99afd57..c1787d01c5f4765a63986efe04c59748792e4932 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -3504,8 +3504,13 @@ model_analyze_insns (void)
 	FOR_EACH_DEP (iter, SD_LIST_FORW, sd_it, dep)
 	  {
 	con = MODEL_INSN_INFO (DEP_CON (dep));
-	if (con->insn && insn->alap < con->alap + 1)
-	  insn->alap = con->alap + 1;
+	/* Consider all dependencies except those introduced artificially
+	   by the scheduler as part of adhering to the
+	   MAX_PENDING_LIST_LENGTH limit.  */
+	unsigned int min_alap
+	  = con->alap + !DEP_ARTIFICIAL (dep);
+	if (con->insn && insn->alap < min_alap)
+	  insn->alap = min_alap;
 	  }
 
 	insn->old_queue = QUEUE_INDEX (iter);
diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index f89f28269fd5ecf96688ed255d07b6976d2180c4..a83b779086099e9a1a0690b480230f4d7ad3e0ba 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -100,6 +100,7 @@ init_dep_1 (dep_t dep, rtx_insn *pro, rtx_insn *con, enum reg_note type, ds_t ds
   DEP_COST (dep) = UNKNOWN_DEP_COST;
   DEP_NONREG (dep) = 0;
   DEP_MULTIPLE (dep) = 0;
+  DEP_ARTIFICIAL (dep) = 0;
   DEP_REPLACE (dep) = NULL;
 }
 
@@ -472,16 +473,18 @@ static int cache_size;
 static bool mark_as_hard;
 
 static int deps_may_trap_p (const_rtx);
-static void add_dependence_1 (rtx_insn *, rtx_insn *, enum reg_note);
+static void add_dependence_1 (rtx_insn *, rtx_insn *, enum reg_note,
+			  bool = false);
 static void add_dependence_list (rtx_insn *, rtx_insn_list *, int,
- enum reg_note, bool);
+ enum reg_note, bool, bool = false);
 static void add_dependence_list_and_free (struct deps_desc *, rtx_insn *,
 	  rtx_insn_list **, int, enum reg_note,
-	  bool);
+	  bool, bo

Re: [PATCH] diagnose unsupported uses of hardware register variables (PR 88000)

2018-11-19 Thread Segher Boessenkool
On Mon, Nov 19, 2018 at 12:59:29PM +, Michael Matz wrote:
> Hi,
> 
> On Fri, 16 Nov 2018, Segher Boessenkool wrote:
> 
> > > I.e. like volatile they can arbitrarily change their value.  I don't 
> > > know if other peoples mind model is similar, but it certainly is the 
> > > model that is implemented in the GIMPLE pipeline (and if memory serves 
> > > well of the RTL pipeline as well).
> > > 
> > > Copying outof/into pseudos around asms serves no purpose with that 
> > > model, it rather makes regvars somewhat "safer" to use, without 
> > > actually making them safer (there _is_ nothing safe about them).
> > 
> > The only supported case is for inputs and outputs to extended asm.  
> > Maybe we should just come up with a syntax so you can specify hard regs 
> > for that directly (without needing a separate regclass for every reg).
> 
> I would like that, yes.  (Of course backward compat would have us support 
> the historic usage for some time, but still ...)

Right, or for ten years at least (there are header files that use
register asm!)

I was thinking something like

  asm("smth %0,%1" : "=*r0"(x) : "*r1"(y));

where then "*" means the rest of the constraint is just a register name.

I don't know how easy this really is to make work, but do you think this
is a sensible syntax?


Segher


[PATCH] [aarch64] Add CPU support for Ampere Computing's eMAG.

2018-11-19 Thread Christoph Muellner
*** gcc/ChangeLog ***

2018-xx-xx  Christoph Muellner 

* config/aarch64/aarch64-cores.def: Define emag
* config/aarch64/aarch64-tune.md: Regenerated with emag
* config/aarch64/aarch64.c: Defining tuning struct
* doc/invoke.texi: Document mtune value
---
 gcc/config/aarch64/aarch64-cores.def |  1 +
 gcc/config/aarch64/aarch64-tune.md   |  2 +-
 gcc/config/aarch64/aarch64.c | 25 +
 gcc/doc/invoke.texi  |  2 +-
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/gcc/config/aarch64/aarch64-cores.def 
b/gcc/config/aarch64/aarch64-cores.def
index 1f3ac56..6e6800e 100644
--- a/gcc/config/aarch64/aarch64-cores.def
+++ b/gcc/config/aarch64/aarch64-cores.def
@@ -63,6 +63,7 @@ AARCH64_CORE("thunderxt83",   thunderxt83,   thunderx,  8A,  
AARCH64_FL_FOR_ARCH
 
 /* APM ('P') cores. */
 AARCH64_CORE("xgene1",  xgene1,xgene1,8A,  AARCH64_FL_FOR_ARCH8, 
xgene1, 0x50, 0x000, -1)
+AARCH64_CORE("emag",emag,  xgene1,8A,  AARCH64_FL_FOR_ARCH8, 
emag, 0x50, 0x000, -1)
 
 /* Qualcomm ('Q') cores. */
 AARCH64_CORE("falkor",  falkor,falkor,8A,  AARCH64_FL_FOR_ARCH8 | 
AARCH64_FL_CRC | AARCH64_FL_CRYPTO | AARCH64_FL_RDMA, qdf24xx,   0x51, 0xC00, 
-1)
diff --git a/gcc/config/aarch64/aarch64-tune.md 
b/gcc/config/aarch64/aarch64-tune.md
index fade1d4..408976a 100644
--- a/gcc/config/aarch64/aarch64-tune.md
+++ b/gcc/config/aarch64/aarch64-tune.md
@@ -1,5 +1,5 @@
 ;; -*- buffer-read-only: t -*-
 ;; Generated automatically by gentune.sh from aarch64-cores.def
 (define_attr "tune"
-   
"cortexa35,cortexa53,cortexa57,cortexa72,cortexa73,thunderx,thunderxt88p1,thunderxt88,thunderxt81,thunderxt83,xgene1,falkor,qdf24xx,exynosm1,phecda,thunderx2t99p1,vulcan,thunderx2t99,cortexa55,cortexa75,cortexa76,ares,tsv110,saphira,cortexa57cortexa53,cortexa72cortexa53,cortexa73cortexa35,cortexa73cortexa53,cortexa75cortexa55,cortexa76cortexa55"
+   
"cortexa35,cortexa53,cortexa57,cortexa72,cortexa73,thunderx,thunderxt88p1,thunderxt88,thunderxt81,thunderxt83,xgene1,emag,falkor,qdf24xx,exynosm1,phecda,thunderx2t99p1,vulcan,thunderx2t99,cortexa55,cortexa75,cortexa76,ares,tsv110,saphira,cortexa57cortexa53,cortexa72cortexa53,cortexa73cortexa35,cortexa73cortexa53,cortexa75cortexa55,cortexa76cortexa55"
(const (symbol_ref "((enum attr_tune) aarch64_tune)")))
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index f7f88a9..995aafe 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -957,6 +957,31 @@ static const struct tune_params xgene1_tunings =
   &xgene1_prefetch_tune
 };
 
+static const struct tune_params emag_tunings =
+{
+  &xgene1_extra_costs,
+  &xgene1_addrcost_table,
+  &xgene1_regmove_cost,
+  &xgene1_vector_cost,
+  &generic_branch_cost,
+  &xgene1_approx_modes,
+  6, /* memmov_cost  */
+  4, /* issue_rate  */
+  AARCH64_FUSE_NOTHING, /* fusible_ops  */
+  "16",/* function_align.  */
+  "16",/* jump_align.  */
+  "16",/* loop_align.  */
+  2,   /* int_reassoc_width.  */
+  4,   /* fp_reassoc_width.  */
+  1,   /* vec_reassoc_width.  */
+  2,   /* min_div_recip_mul_sf.  */
+  2,   /* min_div_recip_mul_df.  */
+  17,  /* max_case_values.  */
+  tune_params::AUTOPREFETCHER_OFF, /* autoprefetcher_model.  */
+  (AARCH64_EXTRA_TUNE_NO_LDP_STP_QREGS),   /* tune_flags.  */
+  &xgene1_prefetch_tune
+};
+
 static const struct tune_params qdf24xx_tunings =
 {
   &qdf24xx_extra_costs,
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e016dce..ac81fb2 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15288,7 +15288,7 @@ Specify the name of the target processor for which GCC 
should tune the
 performance of the code.  Permissible values for this option are:
 @samp{generic}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55},
 @samp{cortex-a57}, @samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75},
-@samp{cortex-a76}, @samp{ares}, @samp{exynos-m1}, @samp{falkor},
+@samp{cortex-a76}, @samp{ares}, @samp{exynos-m1}, @samp{emag}, @samp{falkor},
 @samp{qdf24xx}, @samp{saphira}, @samp{phecda}, @samp{xgene1}, @samp{vulcan},
 @samp{thunderx}, @samp{thunderxt88}, @samp{thunderxt88p1}, @samp{thunderxt81},
 @samp{tsv110}, @samp{thunderxt83}, @samp{thunderx2t99},
-- 
2.9.5



[PING] Re: [PATCH 1/2] C++: more location wrapper nodes (PR c++/43064, PR c++/43486)

2018-11-19 Thread David Malcolm
Ping, for these patches:

[PATCH 1/2] C++: more location wrapper nodes (PR c++/43064, PR c++/43486)
  https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00304.html

[PATCH 2/2] C++: improvements to binary operator diagnostics (PR c++/87504)
  https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00303.html


Thanks
Dave

On Mon, 2018-11-05 at 15:31 -0500, David Malcolm wrote:
> The C++ frontend gained various location wrapper nodes in r256448
> (GCC 8).
> That patch:
>   https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00799.html
> added wrapper nodes around all nodes with !CAN_HAVE_LOCATION_P for:
> 
> * arguments at callsites, and for
> 
> * typeid, alignof, sizeof, and offsetof.
> 
> This is a followup to that patch, adding many more location wrappers
> to the C++ frontend.  It adds location wrappers for nodes with
> !CAN_HAVE_LOCATION_P to:
> 
> * all literal nodes (in cp_parser_primary_expression)
> 
> * all id-expression nodes (in finish_id_expression), except within a
>   decltype.
> 
> * all mem-initializer nodes within a mem-initializer-list
>   (in cp_parser_mem_initializer)
> 
> However, the patch also adds some suppressions: regions in the parser
> for which wrapper nodes will not be created:
> 
> * within a template-parameter-list or template-argument-list (in
>   cp_parser_template_parameter_list and
> cp_parser_template_argument_list
>   respectively), to avoid encoding the spelling location of the nodes
>   in types.  For example, "array<10>" and "array<10>" are the same
> type,
>   despite the fact that the two different "10" tokens are spelled in
>   different locations in the source.
> 
> * within a gnu-style attribute (none of are handlers are set up to
> cope
>   with location wrappers yet)
> 
> * within various OpenMP clauses
> 
> The patch enables various improvements to locations for bad
> initializations, for -Wchar-subscripts, and enables various other
> improvements in the followup patch.
> 
> For example, given the followup buggy mem-initializer:
> 
> class X {
>   X() : bad(42),
> good(42)
>   { }
>   void* bad;
>   int good;
> };
> 
> previously, our diagnostic was on the final close parenthesis of the
> mem-initializer-list, leaving it unclear where the problem is:
> 
> t.cc: In constructor 'X::X()':
> t.cc:3:16: error: invalid conversion from 'int' to 'void*' [-
> fpermissive]
> 3 | good(42)
>   |^
>   ||
>   |int
> 
> whereas with the patch we highlight which expression is bogus:
> 
> t.cc: In constructor 'X::X()':
> t.cc:2:13: error: invalid conversion from 'int' to 'void*' [-
> fpermissive]
> 2 |   X() : bad(42),
>   | ^~
>   | |
>   | int
> 
> Similarly, the diagnostic for this bogus initialization:
> 
> i.cc:1:44: error: initializer-string for array of chars is too long
> [-fpermissive]
> 1 | char test[3][4] = { "ok", "too long", "ok" };
>   |^
> 
> is improved by the patch so that it indicates which string is too
> long:
> 
> i.cc:1:27: error: initializer-string for array of chars is too long
> [-fpermissive]
> 1 | char test[3][4] = { "ok", "too long", "ok" };
>   |   ^~
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, in
> conjunction with the followup patch [1]
> 
> I did some light performance testing, comparing release builds with
> and
> without the patch on kdecore.cc (preprocessed all-of-KDE) and a test
> file
> that includes all of the C++ stdlib (but does nothing) with it, in
> both
> cases compiling at -O3 -g.  In both cases there was no significant
> difference in the overall wallclock time for all of compilation:
> 
> kdecode.c total wallclock time:
> 
> http://chart.apis.google.com/chart?cht=lc&chs=700x400&chxt=x,y,x,y&ch
> xr=1,58.26,61.79&chco=FF,FF&chdl=control|experiment&chds=58.2
> 6,61.79&chd=t:59.55,60.26,60.53,60.35,60.17,60.27,59.26,60.01,60.21,6
> 0.23,60.1,60.2,60.12,60.48,60.32,60.18,60.01,60.01,60.04,59.96,60.1,6
> 0.11,60.21,60.36,60.08,60.1,60.16,60.01,60.21,60.15,60.12,60.09,59.96
> ,60.12,60.06,60.12,60.05,60.11,59.93,59.99|59.6,59.3,60.03,60.1,60.49
> ,60.35,60.03,60.1,59.87,60.39,60.1,59.96,60.19,60.45,59.97,59.91,60.0
> ,59.99,60.09,60.15,60.79,59.98,60.16,60.09,60.02,60.05,60.32,60.01,59
> .95,59.88,60.1,60.07,60.22,59.87,60.04,60.11,60.01,60.09,59.86,59.86&
> chxl=0:|1|8|16|24|32|40|2:||Iteration|3:||Time+(secs)&chtt=Compilatio
> n+of+kdecore.cc+at+-O3+with+-g+for+x86_64-pc-linux-gnu:+total:+wall
> 
> cp-stdlib.cc total wallclock time:
> 
> http://chart.apis.google.com/chart?cht=lc&chs=700x400&chxt=x,y,x,y&ch
> xr=1,1.88,4.59&chco=FF,FF&chdl=control|experiment&chds=1.88,4
> .59&chd=t:3.59,2.94,2.95,2.94,2.94,2.93,2.92,2.94,2.93,2.94,2.94,2.88
> ,2.94,2.9,2.94,2.9,2.94,2.93,2.94,2.93,2.95,2.93,2.9,2.9,2.94,2.99,2.
> 95,3.0,2.94,3.0,2.94,2.99,2.95,2.95,2.9,2.99,2.94,2.99,2.94,2.96|3.54
> ,2.92,2.93

Re: [PING #4] [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

2018-11-19 Thread Martin Sebor

On 11/16/2018 01:46 AM, Richard Biener wrote:

On Fri, Nov 16, 2018 at 4:09 AM Martin Sebor  wrote:


Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01934.html

Do I need to change something in this patch to make it acceptable
or should I assume we will leave this bug in GCC unfixed?


So the issue is the next_stmt handling because for the _next_ stmt
we did not yet replace uses with lattice values.  This information
was missing all the time along (and absent from patch context).

I notice the next_stmt handling is incredibly fragile and it doesn't
even check the store you identify as thouching the same object
writes a '\0', nor does it verify the store writes to a position after
the strncpy accessed area (but eventually anywhere is OK...).


Yes, this is being tracked in bug 84396.  I have been planing
to tighten it up to check that it is, in fact, the NUL character
being inserted but other things have been getting in the way (like
trying to fix this bug).


So I really wonder why there's the restriction on 1:1 equality of the
base.  That relies on proper CSE (as you saw and tried to work-around
in your patch) and more.

So what I'd do is the attached.  Apply more leeway (and less at the
same time) and restrict it to stores from zero but allow any aliasing
one.  One could build up more precision by, instead of using
ptr_may_alias_ref_p use refs_may_alias_p and build up a ao_ref
for the strncpy destination using ao_ref_from_ptr_and_size, but
I didn't bother to figure out what constraint on len the function
computed up to this point.

The patch fixes the testcase.


It does, but it also introduces two regressions into the test
suite (false negatives).  The code your patch removes is there
to handle these cases.  I'll look into your suggestion to use
refs_may_alias_p to avoid these regressions.

Martin

PS With the suggested patch GCC fails to detect the following:

  struct A { char str[3]; };

  struct B { struct A a[3]; int i; };
  struct C { struct B b[3]; int i; };

  void f (struct B *p, const char *s)
  {
// write into p->a[0]:
__builtin_strncpy (p->a[0].str, s, sizeof p->a[0].str);

// write into p->a[1]:
p->a[1].str[sizeof p->a[1].str - 1] = 0;
  }


Fix ICE in cp_var_mod_type_p

2018-11-19 Thread Jan Hubicka
Hi,
enable-checking compiler crashes in free_lang_data because verify_type
is called too early (after we free data in types but before we clear the
langhooks) and it ends up calling cp_var_mod_type_p which ICEs.

This is fixed by moving type checking after hooks updates.  It would be
also possible to move hook update into free_lang_data_in_cgraph but it
seems to me that it is better to keep such a global cahgne in the
toplevel function (free_lang_data).

lto-bootstrapped/retested x86_64-linux, OK?

Honza
* tree.c (free_lang_data_in_cgraph): Add argument fld; break out
type checking to...
(free_lang_data) ... here; update call of free_lang_data_in_cgraph.
Index: tree.c
===
--- tree.c  (revision 266235)
+++ tree.c  (working copy)
@@ -6014,44 +6014,38 @@ assign_assembler_name_if_needed (tree t)
been set up.  */
 
 static void
-free_lang_data_in_cgraph (void)
+free_lang_data_in_cgraph (struct free_lang_data_d *fld)
 {
   struct cgraph_node *n;
   varpool_node *v;
-  struct free_lang_data_d fld;
   tree t;
   unsigned i;
   alias_pair *p;
 
   /* Find decls and types in the body of every function in the callgraph.  */
   FOR_EACH_FUNCTION (n)
-find_decls_types_in_node (n, &fld);
+find_decls_types_in_node (n, fld);
 
   FOR_EACH_VEC_SAFE_ELT (alias_pairs, i, p)
-find_decls_types (p->decl, &fld);
+find_decls_types (p->decl, fld);
 
   /* Find decls and types in every varpool symbol.  */
   FOR_EACH_VARIABLE (v)
-find_decls_types_in_var (v, &fld);
+find_decls_types_in_var (v, fld);
 
   /* Set the assembler name on every decl found.  We need to do this
  now because free_lang_data_in_decl will invalidate data needed
  for mangling.  This breaks mangling on interdependent decls.  */
-  FOR_EACH_VEC_ELT (fld.decls, i, t)
+  FOR_EACH_VEC_ELT (fld->decls, i, t)
 assign_assembler_name_if_needed (t);
 
   /* Traverse every decl found freeing its language data.  */
-  FOR_EACH_VEC_ELT (fld.decls, i, t)
-free_lang_data_in_decl (t, &fld);
+  FOR_EACH_VEC_ELT (fld->decls, i, t)
+free_lang_data_in_decl (t, fld);
 
   /* Traverse every type found freeing its language data.  */
-  FOR_EACH_VEC_ELT (fld.types, i, t)
-free_lang_data_in_type (t, &fld);
-  if (flag_checking)
-{
-  FOR_EACH_VEC_ELT (fld.types, i, t)
-   verify_type (t);
-}
+  FOR_EACH_VEC_ELT (fld->types, i, t)
+free_lang_data_in_type (t, fld);
 }
 
 
@@ -6061,6 +6055,7 @@ static unsigned
 free_lang_data (void)
 {
   unsigned i;
+  struct free_lang_data_d fld;
 
   /* If we are the LTO frontend we have freed lang-specific data already.  */
   if (in_lto_p
@@ -6081,7 +6076,7 @@ free_lang_data (void)
 
   /* Traverse the IL resetting language specific information for
  operands, expressions, etc.  */
-  free_lang_data_in_cgraph ();
+  free_lang_data_in_cgraph (&fld);
 
   /* Create gimple variants for common types.  */
   for (unsigned i = 0;
@@ -6102,6 +6097,15 @@ free_lang_data (void)
 
   lang_hooks.tree_inlining.var_mod_type_p = hook_bool_tree_tree_false;
 
+  if (flag_checking)
+{
+  int i;
+  tree t;
+
+  FOR_EACH_VEC_ELT (fld.types, i, t)
+   verify_type (t);
+}
+
   /* We do not want the default decl_assembler_name implementation,
  rather if we have fixed everything we want a wrapper around it
  asserting that all non-local symbols already got their assembler
Index: testsuite/g++.dg/torture/pr87997.C
===
--- testsuite/g++.dg/torture/pr87997.C  (nonexistent)
+++ testsuite/g++.dg/torture/pr87997.C  (working copy)
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+template  struct a;
+template  class b, typename c, typename f, typename... d>
+struct a, c> {
+  using e = b;
+};
+template  class h {
+public:
+  typedef f g;
+};
+template  using k = typename a::e;
+template  struct l { template  using m = k; };
+template  struct n {
+  typedef typename j::g o;
+  template  struct p {
+typedef typename l::template m other;
+  };
+};
+template  struct F {
+  typedef typename n::template p::other q;
+};
+template > class r {
+public:
+  typename n::q>::o operator[](long);
+  f *t() noexcept;
+};
+class s {
+  void m_fn2();
+  r u;
+};
+void s::m_fn2() try {
+  for (int i;;)
+(this->*u[i])();
+} catch (...) {
+}


[PATCH] S/390: Skip LT(G) peephole when literal pool is involved

2018-11-19 Thread Ilya Leoshkevich
Bootstrapped and regtested on s390x-redhat-linux.

By the time peephole optimizations run, we've already made up our mind
whether to use base-register or relative addressing for literal pool
entries.  LT(G) supports only base-register addressing, and so it is
too late to convert L(G)RL + compare to LT(G).  This change should not
make the code worse unless building with e.g. -fno-dce, since comparing
literal pool entries to zero should be optimized away during earlier
passes.

gcc/ChangeLog:

2018-11-19  Ilya Leoshkevich  

PR target/88083
* config/s390/s390.md: Skip LT(G) peephole when literal pool is
involved.
* rtl.h (contains_constant_pool_address_p): New function.
* rtlanal.c (contains_constant_pool_address_p): Likewise.

gcc/testsuite/ChangeLog:

2018-11-19  Ilya Leoshkevich  

PR target/88083
* gcc.target/s390/pr88083.c: New test.
---
 gcc/config/s390/s390.md |  3 ++-
 gcc/rtl.h   |  1 +
 gcc/rtlanal.c   | 14 ++
 gcc/testsuite/gcc.target/s390/pr88083.c |  9 +
 4 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/s390/pr88083.c

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 7a556d40224..721222d221f 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -941,7 +941,8 @@
(compare (match_dup 0) (match_operand:GPR 1 "const0_operand")))]
   "s390_match_ccmode(insn, CCSmode) && TARGET_EXTIMM
&& GENERAL_REG_P (operands[0])
-   && satisfies_constraint_T (operands[2])"
+   && satisfies_constraint_T (operands[2])
+   && !contains_constant_pool_address_p (operands[2])"
   [(parallel
 [(set (reg:CCS CC_REGNUM)
  (compare:CCS (match_dup 2) (match_dup 1)))
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 68d3ceab29f..4f28afcf841 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3385,6 +3385,7 @@ extern void set_insn_deleted (rtx_insn *);
 extern rtx single_set_2 (const rtx_insn *, const_rtx);
 extern bool contains_symbol_ref_p (const_rtx);
 extern bool contains_symbolic_reference_p (const_rtx);
+extern bool contains_constant_pool_address_p (const_rtx);
 
 /* Handle the cheap and common cases inline for performance.  */
 
diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index e8b6b9c7a42..0bae21e14c5 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -6550,6 +6550,20 @@ contains_symbolic_reference_p (const_rtx x)
   return false;
 }
 
+/* Return true if RTL X contains a constant pool address.  */
+
+bool
+contains_constant_pool_address_p (const_rtx x)
+{
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
+if (SYMBOL_REF_P (*iter) && CONSTANT_POOL_ADDRESS_P (*iter))
+  return true;
+
+  return false;
+}
+
+
 /* Return true if X contains a thread-local symbol.  */
 
 bool
diff --git a/gcc/testsuite/gcc.target/s390/pr88083.c 
b/gcc/testsuite/gcc.target/s390/pr88083.c
new file mode 100644
index 000..d5e530eef83
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr88083.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-fno-sched-last-insn-heuristic -fno-dce -march=z196 -O2" } */
+
+void *a, *b;
+
+void c(void)
+{
+  __builtin_memcpy(a, b, -1);  /* { dg-warning "exceeds maximum object size" } 
*/
+}
-- 
2.19.1



Re: [PATCH] make function_args_iterator a proper iterator

2018-11-19 Thread Martin Sebor

On 11/19/2018 03:48 AM, Eric Botcazou wrote:

Eventually, when GCC moves to more a recent C++ revision, it will
become possible to simplify the for loops to make use of the range
based for loop syntax along the lines of:

   for (auto argtype: function_args (functype))
 {
   ...
 }

Tested on x86_64-linux, and (lightly) on powerpc64le-linux using
a cross-compiler.  I'll test the changes to the other back ends
before committing.


How does this interact with debugging?  Because, in my experience, the more
you convert things to modernish C++, the less you can easily debug them...


There are no unexpected interactions that I'm aware of.  The C++
11 range for loop is just convenient syntax that the front end
"expands" to a conventional loop using ordinary iterators.  Calls
to the iterator members that implicitly take place can be stepped
into using the debugger just like any other functions.  In terms
of the user experience, I'd say the range for loop is pretty close
to a macro that expands to the use iterators.

Martin


Re: [PATCH] make function_args_iterator a proper iterator

2018-11-19 Thread Martin Sebor

On 11/19/2018 03:32 AM, Richard Biener wrote:

On Sat, Nov 17, 2018 at 12:05 AM Martin Sebor  wrote:


To encourage and simplify the adoption of iterator classes in
GCC the attached patch turns the function_args_iterator struct
into an (almost) proper C++ iterator class that can be used
the same way as traditional forward iterators.

The patch also replaces all of the 26 uses of the legacy
FOREACH_FUNCTION_ARGS macro with ordinary for loops that use
function_args_iterator directly, and also poisons both
FOREACH_FUNCTION_ARGS and the unused FOREACH_FUNCTION_ARGS_PTR
macros.

The few dozen (hundred?) existing uses of for loops that iterate
over function parameter types using the TREE_CHAIN() macro can
be relatively easily modified to adopt the iterator approach over
time.  (The patch stops of short of making this change.)

Eventually, when GCC moves to more a recent C++ revision, it will
become possible to simplify the for loops to make use of the range
based for loop syntax along the lines of:

   for (auto argtype: function_args (functype))
 {
   ...
 }

Tested on x86_64-linux, and (lightly) on powerpc64le-linux using
a cross-compiler.  I'll test the changes to the other back ends
before committing.


This isn't stage3 material.


In the response referenced below Jeff requested I make use of
iterators in my patch.  This simply does what he asked for,
except throughout all of GCC.

Martin



Richard.



Martin

PS For some additional background on this change see:
   https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00493.html




[PATCH] Fix PR83215, remove alias-set zero case from component_uses_parent_alias_set_from

2018-11-19 Thread Richard Biener


The PR complains that with TYPE_TYPELESS_STORAGE in C++ now too much
TBAA aliasing happens (not that this was unexpected).  The reason
for this with the testcase is the alias-set zero case in
component_uses_parent_alias_set_from which history tells me was added
for attribute((may_alias)) purposes - but that case is now handled
in the caller via ref_all_alias_ptr_type_p.  The TYPE_TYPELESS_STORAGE
case is also not needed given the flag is propagated in the type hierarchy
so if T is TYPE_TYPELESS_STORAGE then all bases are so as well.

Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages.

Eric, do you know of any cases in Ada where a alias-set zero base
has non-alias-set zero children?  The testsuite seems to be clean
but you never know...

Richard.

2018-11-19  Richard Biener  

PR middle-end/83215
* alias.c (component_uses_parent_alias_set_from): Remove
alias-set zero and TYPE_TYPELESS_STORAGE case both already
handled in other ways.

* g++.dg/tree-ssa/pr83215.C: New testcase.

Index: gcc/alias.c
===
--- gcc/alias.c (revision 266278)
+++ gcc/alias.c (working copy)
@@ -601,8 +601,7 @@ objects_must_conflict_p (tree t1, tree t
 /* Return the outermost parent of component present in the chain of
component references handled by get_inner_reference in T with the
following property:
- - the component is non-addressable, or
- - the parent has alias set zero,
+ - the component is non-addressable
or NULL_TREE if no such parent exists.  In the former cases, the alias
set of this parent is the alias set that must be used for T itself.  */
 
@@ -611,10 +610,6 @@ component_uses_parent_alias_set_from (co
 {
   const_tree found = NULL_TREE;
 
-  if (AGGREGATE_TYPE_P (TREE_TYPE (t))
-  && TYPE_TYPELESS_STORAGE (TREE_TYPE (t)))
-return const_cast  (t);
-
   while (handled_component_p (t))
 {
   switch (TREE_CODE (t))
@@ -652,9 +647,6 @@ component_uses_parent_alias_set_from (co
  gcc_unreachable ();
}
 
-  if (get_alias_set (TREE_TYPE (TREE_OPERAND (t, 0))) == 0)
-   found = t;
-
   t = TREE_OPERAND (t, 0);
 }
  
Index: gcc/testsuite/g++.dg/tree-ssa/pr83215.C
===
--- gcc/testsuite/g++.dg/tree-ssa/pr83215.C (nonexistent)
+++ gcc/testsuite/g++.dg/tree-ssa/pr83215.C (working copy)
@@ -0,0 +1,19 @@
+// { dg-do compile }
+// { dg-options "-O2 -fdump-tree-fre1" }
+
+struct mytest
+{
+  float a;
+  char buf[256];
+};
+
+int foo(mytest *m, int *i)
+{
+  int tmp = *i;
+  m->a = 10.0f;
+  return tmp + *i;
+}
+
+// we should be able to CSE *i despite mytest having a cbar[] buffer
+// and thus being subject to TYPE_TYPELESS_STORAGE
+// { dg-final { scan-tree-dump-times "\\*i" 1 "fre1" } }


Re: [PING #4] [PATCH] look harder for MEM_REF operand equality to avoid -Wstringop-truncation (PR 84561)

2018-11-19 Thread Jeff Law
On 11/16/18 1:46 AM, Richard Biener wrote:
> On Fri, Nov 16, 2018 at 4:09 AM Martin Sebor  wrote:
>>
>> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01934.html
>>
>> Do I need to change something in this patch to make it acceptable
>> or should I assume we will leave this bug in GCC unfixed?
> 
> So the issue is the next_stmt handling because for the _next_ stmt
> we did not yet replace uses with lattice values.  This information
> was missing all the time along (and absent from patch context).
> 
> I notice the next_stmt handling is incredibly fragile and it doesn't
> even check the store you identify as thouching the same object
> writes a '\0', nor does it verify the store writes to a position after
> the strncpy accessed area (but eventually anywhere is OK...).
> 
> So I really wonder why there's the restriction on 1:1 equality of the
> base.  That relies on proper CSE (as you saw and tried to work-around
> in your patch) and more.
> 
> So what I'd do is the attached.  Apply more leeway (and less at the
> same time) and restrict it to stores from zero but allow any aliasing
> one.  One could build up more precision by, instead of using
> ptr_may_alias_ref_p use refs_may_alias_p and build up a ao_ref
> for the strncpy destination using ao_ref_from_ptr_and_size, but
> I didn't bother to figure out what constraint on len the function
> computed up to this point.
So FWIW I threw this into the tester and it had a consistent regression
on one of the stringop truncation tests. I think it was
stringop-truncation-2 (logs lost due to stupidity on my part).  It was
visible on x86_64 native as well as other configurations.

It may will be a viable option once that regression is investigated.

jeff




Re: [PATCH] Disable unrolling for loops vectorised with non-constant VF (was: [PATCH][cunroll] Add unroll-known-loop-iterations-only param and use it in aarch64)

2018-11-19 Thread Richard Biener
On Fri, Nov 16, 2018 at 7:02 PM Kyrill Tkachov
 wrote:
>
> Hi all,
>
> This is an alternative to 
> https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00694.html
> As richi suggested, this disables unrolling of loops vectorised with 
> variable-length SVE
> in the vectoriser itself through the loop->unroll member.
>
> It took me a few tries to get it right, as it needs to be set to '1' to 
> disable unrolling,
> the rationale for that mechanism is described in the comment in cfgloop.h.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> Is this ok for trunk?

OK.

Richard.

> Thanks,
> Kyrill
>
> 2018-11-15  Kyrylo Tkachov  
>
>  * tree-vect-loop.c (vect_transform_loop): Disable further unrolling
>  of the loop if vf is non-constant.
>
> 2018-11-15  Kyrylo Tkachov  
>
>  * gcc.target/aarch64/sve/unroll-1.c: New test.


Re: [PATCH] Fix condition in lto-symtab.c (PR lto/88077).

2018-11-19 Thread Richard Biener
On Mon, Nov 19, 2018 at 1:12 PM Martin Liška  wrote:
>
> Hi.
>
> This patch fixes what I screwed in r256989, where I wrongly replaced
> TREE_CODE (TREE_TYPE (decl)) with TYPE_SIZE (decl), which is wrong.
>
> Patch survives testing on x86_64-linux-gnu.
>
> Ready for trunk?

OK.

> Thanks,
> Martin
>
> gcc/lto/ChangeLog:
>
> 2018-11-19  Martin Liska  
>
> PR lto/88077
> * lto-symtab.c (lto_symtab_merge): Transform the
> condition before r256989.
>
> gcc/testsuite/ChangeLog:
>
> 2018-11-19  Martin Liska  
>
> PR lto/88077
> * gcc.dg/lto/pr88077_0.c: New test.
> * gcc.dg/lto/pr88077_1.c: New test.
> ---
>  gcc/lto/lto-symtab.c | 5 +++--
>  gcc/testsuite/gcc.dg/lto/pr88077_0.c | 3 +++
>  gcc/testsuite/gcc.dg/lto/pr88077_1.c | 6 ++
>  3 files changed, 12 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr88077_0.c
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr88077_1.c
>
>


Re: [PATCH] Fix stmt removal in vectorize_fold_left_reduction (PR tree-optimization/88071)

2018-11-19 Thread Richard Biener
On Mon, 19 Nov 2018, Jakub Jelinek wrote:

> Hi!
> 
> In these cases the stmts vectorize_fold_left_reduction is removing may have
> EH region of -2 attached to them, but because they are removed with false
> as remove_permanently or update_eh_info, the old stmts are kept in the EH
> tables and cause checking ICE.
> 
> From what I understand, the stmts are really removed permanently, so fixed
> thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2018-11-19  Jakub Jelinek  
> 
>   PR tree-optimization/88071
>   * tree-vect-loop.c (vectorize_fold_left_reduction): Pass true instead
>   of false as last argument to gsi_remove.
>   * tree-vect-stmts.c (vect_finish_replace_stmt): Pass true instead of
>   false as last argument to gsi_replace.
> 
>   * gcc.dg/pr88071.c: New test.
> 
> --- gcc/tree-vect-loop.c.jj   2018-11-16 10:22:20.037235568 +0100
> +++ gcc/tree-vect-loop.c  2018-11-19 11:12:48.463267700 +0100
> @@ -5861,7 +5861,7 @@ vectorize_fold_left_reduction (stmt_vec_
> /* Remove the statement, so that we can use the same code paths
>as for statements that we've just created.  */
> gimple_stmt_iterator tmp_gsi = gsi_for_stmt (new_stmt);
> -   gsi_remove (&tmp_gsi, false);
> +   gsi_remove (&tmp_gsi, true);
>   }
>  
>if (i == vec_num - 1)
> --- gcc/tree-vect-stmts.c.jj  2018-11-16 10:22:20.061235172 +0100
> +++ gcc/tree-vect-stmts.c 2018-11-19 11:47:41.480260582 +0100
> @@ -1720,7 +1720,7 @@ vect_finish_replace_stmt (stmt_vec_info
>gcc_assert (gimple_get_lhs (stmt_info->stmt) == gimple_get_lhs (vec_stmt));
>  
>gimple_stmt_iterator gsi = gsi_for_stmt (stmt_info->stmt);
> -  gsi_replace (&gsi, vec_stmt, false);
> +  gsi_replace (&gsi, vec_stmt, true);
>  
>return vect_finish_stmt_generation_1 (stmt_info, vec_stmt);
>  }
> --- gcc/testsuite/gcc.dg/pr88071.c.jj 2018-11-19 11:55:11.364945562 +0100
> +++ gcc/testsuite/gcc.dg/pr88071.c2018-11-19 11:55:05.682037075 +0100
> @@ -0,0 +1,5 @@
> +/* PR tree-optimization/88071 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fexceptions -fnon-call-exceptions -fopenmp-simd 
> -ftrapv -ftree-loop-vectorize" } */
> +
> +#include "gomp/openmp-simd-2.c"
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


[PATCH] Fix stmt removal in vectorize_fold_left_reduction (PR tree-optimization/88071)

2018-11-19 Thread Jakub Jelinek
Hi!

In these cases the stmts vectorize_fold_left_reduction is removing may have
EH region of -2 attached to them, but because they are removed with false
as remove_permanently or update_eh_info, the old stmts are kept in the EH
tables and cause checking ICE.

>From what I understand, the stmts are really removed permanently, so fixed
thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-11-19  Jakub Jelinek  

PR tree-optimization/88071
* tree-vect-loop.c (vectorize_fold_left_reduction): Pass true instead
of false as last argument to gsi_remove.
* tree-vect-stmts.c (vect_finish_replace_stmt): Pass true instead of
false as last argument to gsi_replace.

* gcc.dg/pr88071.c: New test.

--- gcc/tree-vect-loop.c.jj 2018-11-16 10:22:20.037235568 +0100
+++ gcc/tree-vect-loop.c2018-11-19 11:12:48.463267700 +0100
@@ -5861,7 +5861,7 @@ vectorize_fold_left_reduction (stmt_vec_
  /* Remove the statement, so that we can use the same code paths
 as for statements that we've just created.  */
  gimple_stmt_iterator tmp_gsi = gsi_for_stmt (new_stmt);
- gsi_remove (&tmp_gsi, false);
+ gsi_remove (&tmp_gsi, true);
}
 
   if (i == vec_num - 1)
--- gcc/tree-vect-stmts.c.jj2018-11-16 10:22:20.061235172 +0100
+++ gcc/tree-vect-stmts.c   2018-11-19 11:47:41.480260582 +0100
@@ -1720,7 +1720,7 @@ vect_finish_replace_stmt (stmt_vec_info
   gcc_assert (gimple_get_lhs (stmt_info->stmt) == gimple_get_lhs (vec_stmt));
 
   gimple_stmt_iterator gsi = gsi_for_stmt (stmt_info->stmt);
-  gsi_replace (&gsi, vec_stmt, false);
+  gsi_replace (&gsi, vec_stmt, true);
 
   return vect_finish_stmt_generation_1 (stmt_info, vec_stmt);
 }
--- gcc/testsuite/gcc.dg/pr88071.c.jj   2018-11-19 11:55:11.364945562 +0100
+++ gcc/testsuite/gcc.dg/pr88071.c  2018-11-19 11:55:05.682037075 +0100
@@ -0,0 +1,5 @@
+/* PR tree-optimization/88071 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fexceptions -fnon-call-exceptions -fopenmp-simd -ftrapv 
-ftree-loop-vectorize" } */
+
+#include "gomp/openmp-simd-2.c"

Jakub


Re: [doc, committed] clarify that "packed" attribute can apply to C++ classes

2018-11-19 Thread Sandra Loosemore

On 11/19/18 2:52 AM, Jonathan Wakely wrote:

On 15/11/18 17:52 -0700, Sandra Loosemore wrote:
I've checked in this patch for PR 25759, another minor documentation 
improvement.


I'll commit this patch as obvious as soon as svn stops being slow.


Thank you.  My eyes were kind of glazing over when I did this one, I 
guess.  :-(



I wonder if the last sentence would be further improved by simply
saying "not on a @code{typedef} that does not also define the type"
instead of spelling out the kinds of types for a second time.


If you think that would be adequately clear to readers, that would be 
fine with me.  I remember hesitating over that one and deciding to leave 
it closer to the original text rather than rewrite.


-Sandra


[committed] Add pr60994.C testcase (PR c++/60994)

2018-11-19 Thread Jakub Jelinek
Hi!

This is a testcase from PR60994 #c7, which got fixed with PR77812
but the testcases from that PR look different from this one, so I've tested
and committed this to trunk in order to close the PR.

2018-11-19  Jakub Jelinek  

PR c++/60994
* g++.dg/lookup/pr60994.C: New test.

--- gcc/testsuite/g++.dg/lookup/pr60994.C.jj2018-11-19 14:57:16.095718277 
+0100
+++ gcc/testsuite/g++.dg/lookup/pr60994.C   2018-11-19 14:55:52.138070817 
+0100
@@ -0,0 +1,13 @@
+// PR c++/60994
+// { dg-do compile }
+
+struct s
+{
+  static int i;
+};
+
+template 
+int s()
+{
+  return s::i; // { dg-bogus "is not a class" }
+}

Jakub


Re: [PATCH] Fix PR88031

2018-11-19 Thread Christophe Lyon
On Thu, 15 Nov 2018 at 14:41, Richard Biener  wrote:
>
>
> With one of my last changes we regressed here so this goes all the
> way cleaning up things so we only have a single flag to
> vectorizable_condition whetehr we are called from reduction context.
> In theory the !multiple-types restriction could be easily lifted now
> (just remove the check).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Richard.
>
> 2018-11-15  Richard Biener  
>
> PR tree-optimization/88031
> * tree-vect-loop.c (vectorizable_reduction): Move check
> for multiple types earlier so we get the expected dump.
> Simplify calls to vectorizable_condition.
> * tree-vect-stmts.h (vectorizable_condition): Update prototype.
> * tree-vect-stmts.c (vectorizable_condition): Instead of
> reduc_def and reduc_index take just a flag.  Simplify
> code-generation now that we can rely on the defs being set up.
> (vectorizable_comparison): Remove unused argument.
>
> * gcc.dg/pr88031.c: New testcase.
>

Hi Richard,

Since you committed this patch (r266182),
I've noticed regressions on aarch64:
gcc.target/aarch64/sve/clastb_1.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_2.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_3.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_4.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_5.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_6.c -march=armv8.2-a+sve (internal
compiler error)
gcc.target/aarch64/sve/clastb_7.c -march=armv8.2-a+sve (internal
compiler error)

during GIMPLE pass: vect
/gcc/testsuite/gcc.target/aarch64/sve/clastb_1.c: In function
'condition_reduction':
/gcc/testsuite/gcc.target/aarch64/sve/clastb_1.c:9:1: internal
compiler error: in vect_get_vec_def_for_operand_1, at
tree-vect-stmts.c:1485
0xe915ab vect_get_vec_def_for_operand_1(_stmt_vec_info*, vect_def_type)
/gcc/tree-vect-stmts.c:1485
0xe987ea vect_get_vec_def_for_operand(tree_node*, _stmt_vec_info*, tree_node*)
/gcc/tree-vect-stmts.c:1547
0xe9f944 vectorizable_condition(_stmt_vec_info*,
gimple_stmt_iterator*, _stmt_vec_info**, bool, _slp_tree*,
vec*)
/gcc/tree-vect-stmts.c:8931
0xebed1d vectorizable_reduction(_stmt_vec_info*,
gimple_stmt_iterator*, _stmt_vec_info**, _slp_tree*, _slp_instance*,
vec*)
/gcc/tree-vect-loop.c:6964
0xeb2640 vect_transform_stmt(_stmt_vec_info*, gimple_stmt_iterator*,
_slp_tree*, _slp_instance*)
/gcc/tree-vect-stmts.c:9691
0xeb4029 vect_transform_loop_stmt
/gcc/tree-vect-loop.c:8185
0xec1ff2 vect_transform_loop(_loop_vec_info*)
/gcc/tree-vect-loop.c:8405
0xee5950 try_vectorize_loop_1
/gcc/tree-vectorizer.c:969
0xee62f1 vectorize_loops()
/gcc/tree-vectorizer.c:1102
Please submit a full bug report,

Christophe

>
> diff --git a/gcc/testsuite/gcc.dg/pr88031.c b/gcc/testsuite/gcc.dg/pr88031.c
> new file mode 100644
> index 000..2c1d1c1391d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr88031.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +int a[512];
> +int b;
> +void d()
> +{
> +  unsigned char c;
> +  for (; b; b++) {
> +  c = 1;
> +  for (; c; c <<= 1) {
> + a[b] <<= 8;
> + if (b & c)
> +   a[b] = 1;
> +  }
> +  }
> +}
> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
> index eb01acdf717..88b980bb9d7 100644
> --- a/gcc/tree-vect-loop.c
> +++ b/gcc/tree-vect-loop.c
> @@ -6531,14 +6531,24 @@ vectorizable_reduction (stmt_vec_info stmt_info, 
> gimple_stmt_iterator *gsi,
>  double_reduc = true;
>  }
>
> +  vect_reduction_type reduction_type
> += STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info);
> +  if ((double_reduc || reduction_type != TREE_CODE_REDUCTION)
> +  && ncopies > 1)
> +{
> +  if (dump_enabled_p ())
> +   dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +"multiple types in double reduction or condition "
> +"reduction.\n");
> +  return false;
> +}
> +
>if (code == COND_EXPR)
>  {
>/* Only call during the analysis stage, otherwise we'll lose
> -STMT_VINFO_TYPE.  We'll pass ops[0] as reduc_op, it's only
> -used as a flag during analysis.  */
> +STMT_VINFO_TYPE.  */
>if (!vec_stmt && !vectorizable_condition (stmt_info, gsi, NULL,
> -   ops[0], 0, NULL,
> -   cost_vec))
> +   true, NULL, cost_vec))
>  {
>if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> @@ -6638,8 +6648,6 @@ vectorizable_reduction (stmt_vec_info stmt_info, 
> gimple_stmt_iterato

Re: [PATCH PR84648]Adjust loop exit conditions for loop-until-wrap cases.

2018-11-19 Thread Christophe Lyon
On Wed, 14 Nov 2018 at 11:10, bin.cheng  wrote:
>
> --
> Sender:Richard Biener 
> Sent at:2018 Nov 13 (Tue) 23:03
> To:bin.cheng 
> Cc:GCC Patches 
> Subject:Re: [PATCH PR84648]Adjust loop exit conditions for loop-until-wrap 
> cases.
>
> >
> > On Sun, Nov 11, 2018 at 9:02 AM bin.cheng  
> > wrote:
> >>
> >> Hi,
> >> This patch fixes PR84648 by adjusting exit conditions for loop-until-wrap 
> >> cases.
> >> It only handles simple cases in which IV.base are constants because we 
> >> rely on
> >> current niter analyzer which doesn't handle parameterized bound in wrapped
> >> case.  It could be relaxed in the future.
> >>
> >> Bootstrap and test on x86_64 in progress.
> >
> > Please use TYPE_MIN/MAX_VALUE or wi::min/max_value consistently.
> > Either tree_int_cst_equal (iv0->base, TYPE_MIN_VALUE (type)) or
> > wide_int_to_tree (niter_type, wi::max_value (TYPE_PRECISION (type),
> > TYPE_SIGN (type))).
> >
> > Also
> >
> > +  iv0->base = low;
> > +  iv0->step = fold_convert (niter_type, integer_one_node);
> >
> > build_int_cst (niter_type, 1);
> >
> > +  iv1->base = high;
> > +  iv1->step = integer_zero_node;
> >
> > build_int_cst (niter_type, 0);
> Fixed, thanks for reviewing.
>
> >
> > With the code, what happens to signed IVs?  I suppose we figure out things
> > earlier by means of undefined overflow?
> The code takes advantage of signed undefined overflow and handle it as wrap.
> In the reported test case, we have following IL:
>:
>   goto ; [INV]
>
>:
>   i_4 = i_2 + 1;
>
>:
>   # i_2 = PHI <0(2), i_4(3)>
>   i.0_1 = (signed int) i_2;
>   if (i.0_1 >= 0)
> goto ; [INV]
>   else
> goto ; [INV]
>
> So the IV is actually transformed into signed int, we rely on scev to 
> understand
> type conversion correctly and generate (int){0, 1} for i.0_1.  Is this 
> reasonable?
>
> Updated patch attached, bootstrap and test on x86_64.
>
> Thanks,
> bin
>
> 2018-11-11  Bin Cheng  
>
> PR tree-optimization/84648
> * tree-ssa-loop-niter.c (adjust_cond_for_loop_until_wrap): New.
> (number_of_iterations_cond): Adjust exit cond for loop-until-wrap case
> by calling adjust_cond_for_loop_until_wrap.
>
> 2018-11-11  Bin Cheng  
>
> PR tree-optimization/84648
> * gcc.dg/tree-ssa/pr84648.c: New test.
> * gcc.dg/pr68317.c: Add warning check on overflow.


Hi Bin,

Since you committed this patch (r266171), I've noticed a regression
in fortran:
FAIL: gfortran.dg/transfer_intrinsic_3.f90   -O3 -fomit-frame-pointer
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution
test
FAIL: gfortran.dg/transfer_intrinsic_3.f90   -O3 -g  execution test
on arm-none-linux-gnueabihf
--with-cpu cortex-a5
--with-fpu vfpv3-d16-fp16

cortex-a9+neon-fp16, cortex-a15+neon-vfpv4 and
cortex-a57+crypto-neon-fp-armv8 are still OK.

Christophe


Re: [PATCH] Fix debuginfo in -fopenmp code (PR debug/87039)

2018-11-19 Thread Richard Biener
On Sat, 17 Nov 2018, Jakub Jelinek wrote:

> On Sat, Nov 17, 2018 at 05:51:05PM +0100, Richard Biener wrote:
> > On November 17, 2018 4:14:58 PM GMT+01:00, Jakub Jelinek  
> > wrote:
> > >On Sat, Nov 17, 2018 at 08:31:32AM +0100, Richard Biener wrote:
> > >> On November 16, 2018 10:10:00 PM GMT+01:00, Jakub Jelinek
> > >>  wrote: Can you add a comment why doing it
> > >differently
> > >> for this case is necessary or do it the same way in all cases?
> > >
> > >Do you mean in omp-expand.c or dwarf2out.c?  I believe I'm doing it the
> > >same
> > 
> > I meant in dwarf2out.c.  IIRC we have multiple calls into dwarf2out_decl 
> > and you adjust only one? 
> 
> We don't need to change the case where decl isn't a FUNCTION_DECL,
> and must not change the case where we call dwarf2out_decl (decl), that would
> lead to infinite recursion.
> 
> The only case might be replace
>   current_function_decl = origin;
>   dwarf2out_decl (origin);
> with the dwarf2out_early_global_decl (origin); call, not really sure if that
> is needed though.  Do we ever have functions where
> decl_function_context (decl) != decl_function_context (DECL_ABSTRACT_ORIGIN 
> (decl))
> ?
> 
> The other possibility is to move this decl_function_context handling code
> from dwarf2out_early_global_decl to dwarf2out_decl, guarded with 
>   if (early_dwarf
>   && decl_function_context (decl)))
>...
> 
> Something like (completely untested):
> 
> --- gcc/dwarf2out.c.jj2018-11-16 17:33:42.899215778 +0100
> +++ gcc/dwarf2out.c   2018-11-17 20:11:26.847301806 +0100
> @@ -26390,22 +26390,6 @@ dwarf2out_early_global_decl (tree decl)
>   {
> tree save_fndecl = current_function_decl;
>  
> -   /* For nested functions, make sure we have DIEs for the parents first
> -  so that all nested DIEs are generated at the proper scope in the
> -  first shot.  */
> -   tree context = decl_function_context (decl);
> -   if (context != NULL)
> - {
> -   dw_die_ref context_die = lookup_decl_die (context);
> -   current_function_decl = context;
> -
> -   /* Avoid emitting DIEs multiple times, but still process CONTEXT
> -  enough so that it lands in its own context.  This avoids type
> -  pruning issues later on.  */
> -   if (context_die == NULL || is_declaration_die (context_die))
> - dwarf2out_decl (context);
> - }
> -
> /* Emit an abstract origin of a function first.  This happens
>with C++ constructor clones for example and makes
>dwarf2out_abstract_function happy which requires the early
> @@ -26716,11 +26700,31 @@ dwarf2out_decl (tree decl)
>we're a method, it will be ignored, since we already have a DIE.
>Avoid doing this late though since clones of class methods may
>otherwise end up in limbo and create type DIEs late.  */
> -  if (early_dwarf
> -   && decl_function_context (decl)
> -   /* But if we're in terse mode, we don't care about scope.  */
> -   && debug_info_level > DINFO_LEVEL_TERSE)
> - context_die = NULL;
> +  if (early_dwarf)
> + {
> +   /* For nested functions, make sure we have DIEs for the parents first
> +  so that all nested DIEs are generated at the proper scope in the
> +  first shot.  */
> +   tree context = decl_function_context (decl);
> +   if (context != NULL)
> + {
> +   dw_die_ref ctx_die = lookup_decl_die (context);
> +   tree save_fndecl = current_function_decl;
> +   current_function_decl = context;
> +
> +   /* Avoid emitting DIEs multiple times, but still process CONTEXT
> +  enough so that it lands in its own context.  This avoids type
> +  pruning issues later on.  */
> +   if (ctx_die == NULL || is_declaration_die (ctx_die))
> + dwarf2out_decl (context);
> +
> +   current_function_decl = save_fndecl;
> +
> +   /* But if we're in terse mode, we don't care about scope.  */
> +   if (debug_info_level > DINFO_LEVEL_TERSE)
> + context_die = NULL;
> + }
> + }

Ugh, I'd like to keep the ordering "hacks" in dwarf2out_early_global_decl.

Reviewing the original code now the only case we need the recursion
is that of your original change (though the recursion shouldn't
be necessary for the is_declaration_die case).

Thus the original dwarf2out.c hunk is OK.

Thanks and sorry for the confusion coming from reviewing patches
from a mobile phone w/o SVN access...

Richard.


>break;
>  
>  case VAR_DECL:
> 
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)


Re: [v3 PATCH] PR libstdc++/87855

2018-11-19 Thread Jonathan Wakely

On 19/11/18 12:28 +, Jonathan Wakely wrote:

On 15/11/18 14:00 +0200, Ville Voutilainen wrote:

Tested on Linux-PPC64. Ok for trunk? Backports?


One problem ...


--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/9.cc
@@ -0,0 +1,98 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile }


This needs to be:

// { dg-do compile { target c++17 } }

Otherwise it FAILs with DejaGnu 1.5.2 and older when using something
like --target_board=unix/-std=c++14 (because old versions of DejaGnu
override the explicit dg-options in the testcase with the
--target_board options).

If you use { target c++17 } then when the old DejaGnu overrides your
{ dg-options "-std=gnu++17" } setting it still correctly skips the
test for older dialects, instead of getting a FAIL.


I'm wrong about that. The check_effective_target_c++* checks in
gcc/testsuite/lib/target-supports.exp say:

# This assumes that the default for the compiler is $cxx_default, and that
# there will never be multiple -std= arguments on the command line.

So if dg-options adds -std=gnu++17 then the testsuite thinks that
option is in effect, even if there's a later -std=c++14 on the final
command-line that came from --target_board. Drat. So using an old
DejaGnu means setting -std=* via --target_board will give lots of
FAILures.

But having the { target c++17 } present is still preferable, as it
makes the test's requirements explicit, and makes the test more
futureproof. If we later change the compiler default to -std=gnu++17
then we'll want to remove the { dg-options "-std=gnu++17" } from these
tests, so that they can also be tested in C++2a and C++2b modes. If we
do that, we'll need the { target c++17 } so it doesn't get tested for
c++14 and older.



Re: [PATCH] diagnose unsupported uses of hardware register variables (PR 88000)

2018-11-19 Thread Michael Matz
Hi,

On Fri, 16 Nov 2018, Segher Boessenkool wrote:

> > I.e. like volatile they can arbitrarily change their value.  I don't 
> > know if other peoples mind model is similar, but it certainly is the 
> > model that is implemented in the GIMPLE pipeline (and if memory serves 
> > well of the RTL pipeline as well).
> > 
> > Copying outof/into pseudos around asms serves no purpose with that 
> > model, it rather makes regvars somewhat "safer" to use, without 
> > actually making them safer (there _is_ nothing safe about them).
> 
> The only supported case is for inputs and outputs to extended asm.  
> Maybe we should just come up with a syntax so you can specify hard regs 
> for that directly (without needing a separate regclass for every reg).

I would like that, yes.  (Of course backward compat would have us support 
the historic usage for some time, but still ...)


Ciao,
Michael.


Re: Fix __gnu_cxx::throw_allocator 2 * O(log(N)) complexity

2018-11-19 Thread Jonathan Wakely

On 13/11/18 07:15 +0100, François Dumont wrote:

Oops, it was not the tested patch. Here it is.

On 11/12/18 7:43 AM, François Dumont wrote:
When doing some debugging session I noticed that the 
__gnu_cxx::throw_allocator doubles all lookup on both insert and 
erase.


Using map::insert result and erasing the found iterator avoids this 
double lookup.


    * include/ext/throw_allocator.h
    (annotate_base::insert(void*, size_t)): Use insert result to 
check for

    double insert attempt.
    (annotate_base::insert_construct(void*)): Likewise.
    (annotate_base::check_allocated(void*, size_t)): Return found 
iterator.

    (annotate_base::erase(void*, size_t)): Use latter method returned
    iterator.
    (annotate_base::check_constructed(void*, size_t)): Return found 
iterator.

    (annotate_base::erase_construct(void*)): Use latter method returned
    iterator.

Tested under linux x86_64.

Ok to commit ?


OK, thanks.




Re: Fix hashtable node deallocation

2018-11-19 Thread Jonathan Wakely

On 17/11/18 22:01 +0100, François Dumont wrote:
Here is the same patch but this time with a test change which is 
supposed to show the problem.


However it doesn't because of:
  _Pointer_adapter(element_type* __arg = 0)
  { _Storage_policy::set(__arg); }

which is not explicit.

So is this patch really necessary ? If it isn't, is usage of 
pointer_traits<>::pointer_to really necessary ?


Yes. Just because our _Pointer_adapter allows implicit conversions
from raw pointers doesn't mean all fancy pointers allow that.

Note that I also found a bug in the 
__gnu_test::CustomPointerAlloc::allocate, the signature with hint is 
wrong.


Yes, that's a bug, thanks.


    * include/ext/throw_allocator.h
    (annotate_base::insert(void*, size_t)): Use insert result to check for
    double insert attempt.
    (annotate_base::insert_construct(void*)): Likewise.
    (annotate_base::check_allocated(void*, size_t)): Return found iterator.
    (annotate_base::erase(void*, size_t)): Use latter method returned
    iterator.
    (annotate_base::check_constructed(void*, size_t)): Return found 
iterator.

    (annotate_base::erase_construct(void*)): Use latter method returned
    iterator.


This looks like the wrong ChangeLog.



    * libstdc++-v3/testsuite/util/testsuite_allocator.h
    (CustomPointerAlloc<>::allocate(size_t, pointer)): Replace by...
    (CustomPointerAlloc<>::allocate(size_t, const_void_pointer)): ...this.
    * testsuite/23_containers/unordered_set/allocator/ext_ptr.cc: Add
    check.


Re: Fix hashtable node deallocation

2018-11-19 Thread Jonathan Wakely

On 10/11/18 22:40 +0100, François Dumont wrote:
While working on a hashtable enhancement I noticed that we are not 
using the correct method to deallocate node if the constructor throws 
in _ReuseOrAllocNode operator(). I had to introduce a new 
_M_deallocate_node_ptr for that as node value shall not be destroy 
again.


I also check other places and noticed that a __node_type destructor 
call was missing.


That's intentional. The type has a trivial destructor, so its storage
can just be reused, we don't need to destroy it.



Re: [PATCH] Support simd function declarations via a pre-include.

2018-11-19 Thread Jakub Jelinek
On Mon, Nov 19, 2018 at 01:15:43PM +0100, Martin Liška wrote:
> 2018-11-15  Martin Liska  
> 
>   * config/gnu-user.h (TARGET_F951_OPTIONS): New.
>   * gcc.c (find_fortran_preinclude_file): New function
>   to handle Fortran pre-include.
> 
> gcc/fortran/ChangeLog:
> 
> 2018-11-15  Martin Liska  
> 
>   * decl.c (gfc_match_gcc_builtin): New function.
>   * gfortran.h (struct vect_builtin_tuple): New.
>   (gfc_adjust_builtins): Likewise.
>   * lang-specs.h (TARGET_F951_OPTIONS): New.
>   (F951_OPTIONS): Use it.
>   * lang.opt: Add new option -fpre-include.
>   * match.h (gfc_match_gcc_builtin): Declare new function.
>   * parse.c (decode_gcc_attribute): Handle builtin.
>   (parse_progunit): Call gfc_adjust_builtins.
>   * scanner.c (gfc_new_file): Load pre-included header file
>   when provided.
>   * trans-intrinsic.c (add_simd_flag_for_built_in): New.
>   (gfc_adjust_builtins): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-11-15  Martin Liska  
> 
>   * gfortran.dg/simd-builtins-1.f90: New test.
>   * gfortran.dg/simd-builtins-1.h: New test.
>   * gfortran.dg/simd-builtins-2.f90: New test.
>   * gfortran.dg/simd-builtins-3.f90: New test.
>   * gfortran.dg/simd-builtins-3.h: New test.
>   * gfortran.dg/simd-builtins-4.f: New test.
>   * gfortran.dg/simd-builtins-4.h: New test.
>   * gfortran.dg/simd-builtins-5.f: New test.
>   * gfortran.dg/simd-builtins-6.f90: New test.

LGTM.  Please give Fortran maintainers 2 days so that they can comment on it
before committing.

Jakub


Re: [v3 PATCH] PR libstdc++/87855

2018-11-19 Thread Jonathan Wakely

On 15/11/18 14:00 +0200, Ville Voutilainen wrote:

Tested on Linux-PPC64. Ok for trunk? Backports?


One problem ...


--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/assignment/9.cc
@@ -0,0 +1,98 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile }


This needs to be:

// { dg-do compile { target c++17 } }

Otherwise it FAILs with DejaGnu 1.5.2 and older when using something
like --target_board=unix/-std=c++14 (because old versions of DejaGnu
override the explicit dg-options in the testcase with the
--target_board options).

If you use { target c++17 } then when the old DejaGnu overrides your
{ dg-options "-std=gnu++17" } setting it still correctly skips the
test for older dialects, instead of getting a FAIL.

OK for trunk with that change, thanks.

I think it's OK to backport, but maybe let's wait a few weeks. Keep
the bugzilla open for now, until you do that backport.



Re: [PATCH] Support simd function declarations via a pre-include.

2018-11-19 Thread Martin Liška
On 11/16/18 4:12 PM, Martin Liška wrote:
> On 11/16/18 2:49 PM, Jakub Jelinek wrote:
>> On Fri, Nov 16, 2018 at 02:24:42PM +0100, Martin Liška wrote:
>>> +  if (gfc_match (" (%n) attributes simd", builtin) != MATCH_YES)
>>> +return MATCH_ERROR;
>>> +
>>> +  int builtin_kind = 0;
>>> +  if (gfc_match (" (notinbranch)") == MATCH_YES)
>>
>> I think you need " ( notinbranch )" here.
>>
>>> +builtin_kind = -1;
>>> +  else if (gfc_match (" (inbranch)") == MATCH_YES)
>>> +builtin_kind = 1;
>>
>> And similarly here (+ testsuite coverage for whether you can in free form
>> insert spaces in all the spots that should be allowed).
>> !gcc$ builtin ( sinf ) attributes simd ( notinbranch )  ! comment
>> e.g. should be valid in free form (and fixed form too).
>>
>>> --- a/gcc/fortran/gfortran.h
>>> +++ b/gcc/fortran/gfortran.h
>>> @@ -2764,6 +2764,18 @@ bool gfc_in_match_data (void);
>>>  match gfc_match_char_spec (gfc_typespec *);
>>>  extern int directive_unroll;
>>>  
>>> +/* Tuple for parsing of vectorized built-ins.  */
>>> +struct vect_builtin_tuple
>>> +{
>>> +  vect_builtin_tuple (const char *n, int t): name (n), simd_type (t)
>>
>> gfc_vect_builtin_tuple ?
>> + document what the simd_type is (or make it enum or whatever).
>> One option would be enum omp_clause_code and use OMP_CLAUSE_ERROR for
>> the case where the argument isn't specified, but I think generally
>> gfortran.h doesn't depend on tree* stuff and wants to have its own
>> enums etc.
>>
>>> +extern vec vectorized_builtins;
>>
>> gfc_vectorized_builtins ?
>>
>>> --- a/gcc/fortran/trans-intrinsic.c
>>> +++ b/gcc/fortran/trans-intrinsic.c
>>> @@ -597,7 +597,61 @@ define_quad_builtin (const char *name, tree type, bool 
>>> is_const)
>>>return fndecl;
>>>  }
>>>  
>>> +/* Add SIMD attribute for FNDECL built-in if the built-in
>>> +   name is in VECTORIZED_BUILTINS.  */
>>> +#include "print-tree.h"
>>
>> If you need to include a header, include it at the start of the file.
>>
>>> +static void
>>> +add_simd_flag_for_built_in (tree fndecl)
>>> +{
>>> +  if (fndecl == NULL_TREE)
>>> +return;
>>> +
>>> +  const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
>>> +  for (unsigned i = 0; i < vectorized_builtins.length (); i++)
>>> +if (strcmp (vectorized_builtins[i].name, name) == 0)
>>
>> How many add_simd_flag_for_built_in calls are we expecting and how many
>> vectorized_builtins.length ()?  If it is too much, perhaps e.g. sort
>> the vector by name and do a binary search.  At least if it turns out to be
>> non-trivial compile time.
>>> +
>>> +  vectorized_builtins.truncate (0);
>>
>> That is a memory leak, right?  The names are malloced.
>> And why truncate rather than release?
>>> +  const char *path = find_a_file (&include_prefixes, argv[1], R_OK, true);
>>> +  if (path != NULL)
>>> +  return concat (argv[0], path, NULL);
>>
>> Formatting.
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gfortran.dg/simd-builtins-1.h
>>> @@ -0,0 +1,4 @@
>>> +!GCC$ builtin (sinf) attributes simd
>>> +!GCC$ builtin (sinf) attributes simd (inbranch)
>>> +!GCC$ builtin (sinf) attributes simd (notinbranch)
>>> +!GCC$ builtin (cosf) attributes simd (notinbranch)
>>
>> Are you sure it is a good idea to have the 3 first lines for the same
>> builtin, rather than different?
>>
>> It should be testsuite covered what we do in that case, but with the above
>> you don't cover what happens e.g. with notinbranch alone, or no argument.
>>
>> Plus, as I said, I think you should have one *.f and one *.f90 test where
>> you just use many of those !gcc$ builtin lines with spaces in various spots
>> to verify it is parsed properly.
>>
>>  Jakub
>>
> 
> Hi.
> 
> I'm sending version, I changed the container to hash_map that should provide
> faster look up.
> 
> I've been testing the patch right now.
> 
> Martin
> 

Hi.

I'm sending one another tested version on x86_64-linux-gnu. I fixed issues 
spotted
by Jakub.

Martin
>From 0d39d1dc7067849cec0fbd35382bb18f98ed29ba Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 7 Nov 2018 12:41:19 +0100
Subject: [PATCH] Support simd function declarations via a pre-include.

gcc/ChangeLog:

2018-11-15  Martin Liska  

	* config/gnu-user.h (TARGET_F951_OPTIONS): New.
	* gcc.c (find_fortran_preinclude_file): New function
	to handle Fortran pre-include.

gcc/fortran/ChangeLog:

2018-11-15  Martin Liska  

	* decl.c (gfc_match_gcc_builtin): New function.
	* gfortran.h (struct vect_builtin_tuple): New.
	(gfc_adjust_builtins): Likewise.
	* lang-specs.h (TARGET_F951_OPTIONS): New.
	(F951_OPTIONS): Use it.
	* lang.opt: Add new option -fpre-include.
	* match.h (gfc_match_gcc_builtin): Declare new function.
	* parse.c (decode_gcc_attribute): Handle builtin.
	(parse_progunit): Call gfc_adjust_builtins.
	* scanner.c (gfc_new_file): Load pre-included header file
	when provided.
	* trans-intrinsic.c (add_simd_flag_for_built_in): New.
	(gfc_adjust_builtins): Likewise.

gcc/testsuite/ChangeLog:

2018-11-15  Martin Liska  

	* gfortran.dg/simd-bu

Re: [PATCH] Support simd function declarations via a pre-include.

2018-11-19 Thread Martin Liška
On 11/17/18 7:16 PM, Bernhard Reutner-Fischer wrote:
> Hi
> 
>> I'm sending version, I changed the container to hash_map that should
>> provide
>> faster look up.
>>
>> I've been testing the patch right now.
> 
> In find_fortran_preinclude_file() you allocate the filename.

Sure, but that's driver allocation and apparently valgrind
is not complaining about the memory allocation.

> 
> diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
> index 55d6dafdb5d..4e500f88174 100644
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> @@ -2428,6 +2428,10 @@ gfc_new_file (void)
>  {
>bool result;
>  
> +  if (flag_pre_include != NULL
> +  && !load_file (flag_pre_include, NULL, false))
> +exit (FATAL_EXIT_CODE);
> +
>if (gfc_cpp_enabled ())
>  {
>result = gfc_cpp_preprocess (gfc_source_file);
> 
> 
> Don't you leak the filename here?

This string is in FE, which lives in a option string pool.

Martin

> I.e.
> else free()
> 
> thanks,
> 



[PATCH] Fix condition in lto-symtab.c (PR lto/88077).

2018-11-19 Thread Martin Liška
Hi.

This patch fixes what I screwed in r256989, where I wrongly replaced
TREE_CODE (TREE_TYPE (decl)) with TYPE_SIZE (decl), which is wrong.

Patch survives testing on x86_64-linux-gnu.

Ready for trunk?
Thanks,
Martin

gcc/lto/ChangeLog:

2018-11-19  Martin Liska  

PR lto/88077
* lto-symtab.c (lto_symtab_merge): Transform the
condition before r256989.

gcc/testsuite/ChangeLog:

2018-11-19  Martin Liska  

PR lto/88077
* gcc.dg/lto/pr88077_0.c: New test.
* gcc.dg/lto/pr88077_1.c: New test.
---
 gcc/lto/lto-symtab.c | 5 +++--
 gcc/testsuite/gcc.dg/lto/pr88077_0.c | 3 +++
 gcc/testsuite/gcc.dg/lto/pr88077_1.c | 6 ++
 3 files changed, 12 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr88077_0.c
 create mode 100644 gcc/testsuite/gcc.dg/lto/pr88077_1.c


diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c
index 18437eb2841..d018a16bd42 100644
--- a/gcc/lto/lto-symtab.c
+++ b/gcc/lto/lto-symtab.c
@@ -374,8 +374,9 @@ lto_symtab_merge (symtab_node *prevailing, symtab_node *entry)
 	 int a[]={1,2,3};
 	 here the first declaration is COMMON
 	 and sizeof(a) == sizeof (int).  */
-	else if (TREE_CODE (type) == ARRAY_TYPE)
-	  return (TYPE_SIZE (decl) == TYPE_SIZE (TREE_TYPE (type)));
+	else if (TREE_CODE (type) != ARRAY_TYPE
+		 || (TYPE_SIZE (type) != TYPE_SIZE (TREE_TYPE (type
+	  return false;
   }
 
   return true;
diff --git a/gcc/testsuite/gcc.dg/lto/pr88077_0.c b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
new file mode 100644
index 000..9e464b6ad4a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr88077_0.c
@@ -0,0 +1,3 @@
+/* { dg-lto-do link } */
+
+int HeaderStr;
diff --git a/gcc/testsuite/gcc.dg/lto/pr88077_1.c b/gcc/testsuite/gcc.dg/lto/pr88077_1.c
new file mode 100644
index 000..fd3de3e77a6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr88077_1.c
@@ -0,0 +1,6 @@
+char HeaderStr[1];
+
+int main()
+{
+  return 0;
+}



[PATCH] Fix PR87229

2018-11-19 Thread Richard Biener


The following fixes and ICE in the LTO streamer by releasing 
non-gimple-val sizepos values in free-lang-data.  This works around(?)
the C++ FE leaving non-gimplified abstract origins around.

LTO bootstrapped & tested on x86_64-unknown-linux-gnu, applied to trunk 
sofar.

Richard.

>From f7ddb8be8a059b084a1d46bde9a33ba365886b6e Mon Sep 17 00:00:00 2001
From: Richard Guenther 
Date: Mon, 19 Nov 2018 09:28:24 +0100
Subject: [PATCH] fix-pr87229

2018-11-19  Richard Biener  

PR lto/87229
* tree.c (free_lang_data_in_one_sizepos): Free non-gimple-val
sizepos values.

* g++.dg/lto/pr87229_0.C: New testcase.

diff --git a/gcc/testsuite/g++.dg/lto/pr87229_0.C 
b/gcc/testsuite/g++.dg/lto/pr87229_0.C
new file mode 100644
index 000..1c20e805291
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr87229_0.C
@@ -0,0 +1,7 @@
+// { dg-lto-do assemble }
+
+struct Main { Main(char* x); };
+
+Main::Main(char* x) {
+char cfg[__builtin_strlen(x)];
+}
diff --git a/gcc/tree.c b/gcc/tree.c
index be89897d43a..48de9cf350f 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -5254,6 +5254,13 @@ free_lang_data_in_one_sizepos (tree *expr_p)
   tree expr = *expr_p;
   if (CONTAINS_PLACEHOLDER_P (expr))
 *expr_p = build0 (PLACEHOLDER_EXPR, TREE_TYPE (expr));
+  /* ???  We have to reset all non-GIMPLE sizepos because those eventually
+ refer to trees we cannot stream.  See for example PR87229 which
+ shows an example with non-gimplified abstract origins in C++.
+ Note this should only happen for abstract copies so setting sizes
+ to NULL is OK (but we cannot easily assert this).  */
+  else if (expr && !is_gimple_val (expr))
+*expr_p = NULL_TREE;
 }
 
 


[PATCH] Fix how we match for count(n*) in gcov tests.

2018-11-19 Thread Martin Liška
Hi.

A small tweak in tests that fixes pattern of following form: /* count(1*). */
I'm going to install the patch.

Martin

gcc/testsuite/ChangeLog:

2018-11-19  Martin Liska  

* g++.dg/gcov/pr84548.C: Remove remove-gcda.
* g++.dg/gcov/ternary.C: Likewise.
* lib/gcov.exp: Support pattern of following form:
count(1*).
---
 gcc/testsuite/g++.dg/gcov/pr84548.C | 2 +-
 gcc/testsuite/g++.dg/gcov/ternary.C | 2 +-
 gcc/testsuite/lib/gcov.exp  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/gcc/testsuite/g++.dg/gcov/pr84548.C b/gcc/testsuite/g++.dg/gcov/pr84548.C
index 6c22c1902f2..3b60b90e2a2 100644
--- a/gcc/testsuite/g++.dg/gcov/pr84548.C
+++ b/gcc/testsuite/g++.dg/gcov/pr84548.C
@@ -16,4 +16,4 @@ int main()
   return 0;
 }
 
-// { dg-final { run-gcov remove-gcda pr84548.C } }
+// { dg-final { run-gcov pr84548.C } }
diff --git a/gcc/testsuite/g++.dg/gcov/ternary.C b/gcc/testsuite/g++.dg/gcov/ternary.C
index d055928c295..9b8e34644b4 100644
--- a/gcc/testsuite/g++.dg/gcov/ternary.C
+++ b/gcc/testsuite/g++.dg/gcov/ternary.C
@@ -9,4 +9,4 @@ int main()
   return a;
 }
 
-// { dg-final { run-gcov remove-gcda ternary.C } }
+// { dg-final { run-gcov ternary.C } }
diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
index a7b8c0a1ef4..dbd3c8e8e44 100644
--- a/gcc/testsuite/lib/gcov.exp
+++ b/gcc/testsuite/lib/gcov.exp
@@ -59,7 +59,7 @@ proc verify-lines { testname testcase file } {
 while { [gets $fd line] >= 0 } {
 # We want to match both "-" and "#" as count as well as numbers,
 # since we want to detect lines that shouldn't be marked as covered.
-	if [regexp "^ *(\[^:]*): *(\[0-9\\-#]+):.*count\\((\[0-9\\-#=\\.kMGTPEZY]+)\\)(.*)" \
+	if [regexp "^ *(\[^:]*): *(\[0-9\\-#]+):.*count\\((\[0-9\\-#=\\.kMGTPEZY\*]+)\\)(.*)" \
 		"$line" all is n shouldbe rest] {
 	if [regexp "^ *{(.*)}" $rest all xfailed] {
 		switch [dg-process-target $xfailed] {



Re: [PATCH, libstdc++] Implement P0415 More constexpr for std::complex.

2018-11-19 Thread Jonathan Wakely

On 16/11/18 19:39 -0500, Ed Smith-Rowland wrote:

@@ -322,67 +323,43 @@
  //@{
  ///  Return new complex value @a x plus @a y.
  template
-inline complex<_Tp>
+inline _GLIBCXX20_CONSTEXPR complex<_Tp>
operator+(const complex<_Tp>& __x, const complex<_Tp>& __y)
-{
-  complex<_Tp> __r = __x;
-  __r += __y;
-  return __r;
-}
+{ return complex<_Tp>(__x.real() + __y.real(), __x.imag() + __y.imag()); }


Is this change (and all the similar ones) really needed?

Doesn't the fact that all the constructors and member operators of
std::complex mean that the original definition is also valid in a
constexpr function?


@@ -1163,50 +1143,43 @@
#endif

  template
-complex&
+_GLIBCXX20_CONSTEXPR complex&
operator=(const complex<_Tp>&  __z)
{
- __real__ _M_value = __z.real();
- __imag__ _M_value = __z.imag();
+ _M_value = __z.__rep();


These changes look OK, but I wonder if we shouldn't ask the compiler
to make it possible to use __real__ and __imag__ in constexpr
functions instead.

I assume it doesn't, and that's why you made this change. But if it
Just Worked, and the other changes I commented on above are also
unnecessary, then this patch would *mostly* just be adding
_GLIBCXX20_CONSTEXPR which is OK for stage 3 (as it doesn't affect any
dialects except C++2a).



@@ -1872,7 +1831,7 @@
{ return _Tp(); }

  template
-inline typename __gnu_cxx::__promote<_Tp>::__type
+_GLIBCXX_CONSTEXPR inline typename __gnu_cxx::__promote<_Tp>::__type


This should be _GLIBCXX20_CONSTEXPR.


Index: testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
===
--- testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
(nonexistent)
+++ testsuite/26_numerics/complex/comparison_operators/more_constexpr.cc
(working copy)
@@ -0,0 +1,51 @@
+// { dg-do compile { target c++2a } }


All the tests with { target c++2a} should also have:

// { dg-options "-std=gnu++2a" }

Because otherwise they are skipped by default, and only get run when
RUNTESTFLAGS explicitly includes something like
--target_board=unix/-std=gnu++2a

The dg-options needs to come first, or it doesn't apply before the
check for { target c++2a }.




[PATCH] PR libstdc++/88084 - Implement LWG 2777

2018-11-19 Thread Jonathan Wakely

* include/std/string_view (basic_string_view::copy): Use traits to
copy.
* testsuite/21_strings/basic_string_view/operations/copy/char/2.cc:
New test.
* testsuite/21_strings/basic_string_view/operations/copy/wchar_t/2.cc:
New test.

Tested x86_64-linux, committed to trunk.


commit 4fd4c4c6ad0d6cfa6d07402dab9705917fc9e101
Author: Jonathan Wakely 
Date:   Mon Nov 19 10:11:23 2018 +

PR libstdc++/88084 - Implement LWG 2777

* include/std/string_view (basic_string_view::copy): Use traits to
copy.
* testsuite/21_strings/basic_string_view/operations/copy/char/2.cc:
New test.
* 
testsuite/21_strings/basic_string_view/operations/copy/wchar_t/2.cc:
New test.

diff --git a/libstdc++-v3/include/std/string_view 
b/libstdc++-v3/include/std/string_view
index 9e0f6a723e4..dc2478d8d0e 100644
--- a/libstdc++-v3/include/std/string_view
+++ b/libstdc++-v3/include/std/string_view
@@ -236,9 +236,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__glibcxx_requires_string_len(__str, __n);
__pos = _M_check(__pos, "basic_string_view::copy");
const size_type __rlen = std::min(__n, _M_len - __pos);
-   for (auto __begin = this->_M_str + __pos,
-__end = __begin + __rlen; __begin != __end;)
- *__str++ = *__begin++;
+   // _GLIBCXX_RESOLVE_LIB_DEFECTS
+   // 2777. basic_string_view::copy should use char_traits::copy
+   traits_type::copy(__str, data() + __pos, __rlen);
return __rlen;
   }
 
diff --git 
a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/char/2.cc 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/char/2.cc
new file mode 100644
index 000..b03edf77a4f
--- /dev/null
+++ 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/char/2.cc
@@ -0,0 +1,56 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target c++17 } }
+
+#include 
+#include 
+
+using char_type = char;
+
+// PR libstdc++/88084
+// LWG 2777. basic_string_view::copy should use char_traits::copy
+
+struct traits : std::char_traits
+{
+  static char_type*
+  copy(char_type* s, const char_type* p, std::size_t n)
+  {
+while (n--)
+  *s++ = 'X';
+return s;
+  }
+};
+
+void
+test01()
+{
+  std::basic_string_view s = "abc";
+  char_type buf[3] = { '1', '2', '3' };
+  auto len = s.copy(buf, 3, 1);
+  VERIFY( len == 2 );
+  VERIFY( buf[0] == 'X' );
+  VERIFY( buf[1] == 'X' );
+  VERIFY( buf[2] == '3' );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git 
a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/wchar_t/2.cc
 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/wchar_t/2.cc
new file mode 100644
index 000..bf192aa8870
--- /dev/null
+++ 
b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/copy/wchar_t/2.cc
@@ -0,0 +1,56 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target c++17 } }
+
+#include 
+#include 
+
+using char_type = wchar_t;
+
+// PR libstdc++/88084
+// LWG 2777. basic_string_view::copy should use char_traits::copy
+
+struct traits : std::char_traits
+{
+  static char_type*
+  copy(char_type* s, const char_type* p, std::size_t n)
+  {
+while (n--)
+  *s++ = 'X';
+return s;
+  }
+};
+
+void
+test01()
+{
+  std::basic_string_view s = L"abc";
+  char_type buf[3] = { L'1', L'2', L'3' };
+  auto

Re: [PATCH, libgcc/ARM & testsuite] Optimize executable size when using softfloat fmul/dmul

2018-11-19 Thread Thomas Preudhomme
FWIW, the testcases were taken from
https://gcc.gnu.org/ml/gcc-patches/2015-01/msg01026.html

Previous approach for fixing tying of fmul to fdiv can be seen in
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01971.html. As mentioned
in the cover letter, this patch went for a completely different
approach and does not share any code besides the testcases.

Best regards,

Thomas
On Mon, 19 Nov 2018 at 09:57, Thomas Preudhomme
 wrote:
>
> Softfloat single precision and double precision floating-point
> multiplication routines in libgcc share some code with the
> floating-point division of their corresponding precision. As the code
> is structured now, this leads to *all* division code being pulled in an
> executable in softfloat mode even if only multiplication is
> performed.
>
> This patch create some new LIB1ASMFUNCS macros to also build files with
> just the multiplication and shared code as weak symbols. By putting
> these earlier in the static library, they can then be picked up when
> only multiplication is used and they are overriden by the global
> definition in the existing file containing both multiplication and
> division code when division is needed.
>
> The patch also removes changes made to the FUNC_START and ARM_FUNC_START
> macros in r218124 since the intent was to put multiplication and
> division code into their own section in a later patch to achieve the
> same size optimization. That approach relied on specific section layout
> to ensure multiplication and division were not too far from the shared
> bit of code in order to the branches to be within range. Due to lack of
> guarantee regarding section layout, in particular with all the
> possibility of linker scripts, this approach was chosen instead. This
> patch keeps the two testcases that were posted by Tony Wang (an Arm
> employee at the time) on the mailing list to implement this approach
> and adds a new one, hence the attribution.
>
> ChangeLog entries are as follows:
>
> *** gcc/ChangeLog ***
>
> 2018-11-14  Thomas Preud'homme  
>
> * config/arm/elf.h: Update comment about condition that need to
> match with libgcc/config/arm/lib1funcs.S to also include
> libgcc/config/arm/t-arm.
> * doc/sourcebuild.texi (output-exists, output-exists-not): Rename
> subsubsection these directives are in to "Check for output files".
> Move scan-symbol to that section and add to it new scan-symbol-not
> directive.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2018-11-16  Tony Wang  
> Thomas Preud'homme  
>
> * lib/lto.exp (lto-execute): Define output_file and testname_with_flags
> to same value as execname.
> (scan-symbol): Move and rename to ...
> * lib/gcc-dg.exp (scan-symbol-common): This.  Adapt into a
> helper function returning true or false if a symbol is present.
> (scan-symbol): New procedure.
> (scan-symbol-not): Likewise.
> * gcc.target/arm/size-optimization-ieee-1.c: New testcase.
> * gcc.target/arm/size-optimization-ieee-2.c: Likewise.
> * gcc.target/arm/size-optimization-ieee-3.c: Likewise.
>
> *** libgcc/ChangeLog ***
>
> 2018-11-16  Thomas Preud'homme  
>
> * /config/arm/lib1funcs.S (FUNC_START): Remove unused sp_section
> parameter and corresponding code.
> (ARM_FUNC_START): Likewise in both definitions.
> Also update footer comment about condition that need to match with
> gcc/config/arm/elf.h to also include libgcc/config/arm/t-arm.
> * config/arm/ieee754-df.S (muldf3): Also build it if L_arm_muldf3 is
> defined.  Weakly define it in this case.
> * config/arm/ieee754-sf.S (mulsf3): Likewise with L_arm_mulsf3.
> * config/arm/t-elf (LIB1ASMFUNCS): Build _arm_muldf3.o and
> _arm_mulsf3.o before muldiv versions if targeting Thumb-1 only. Add
> comment to keep condition in sync with the one in
> libgcc/config/arm/lib1funcs.S and gcc/config/arm/elf.h.
>
> Testing: Bootstrapped on arm-linux-gnueabihf (Arm & Thumb-2) and
> testsuite shows no
> regression. Also built an arm-none-eabi cross compiler targeting
> soft-float which also shows no regression. In particular newly added
> tests and gcc.dg/lto/20081212-1 test pass.
>
> Is this ok for stage3?
>
> Best regards,
>
> Thomas


Re: [PATCH] make function_args_iterator a proper iterator

2018-11-19 Thread Eric Botcazou
> Eventually, when GCC moves to more a recent C++ revision, it will
> become possible to simplify the for loops to make use of the range
> based for loop syntax along the lines of:
> 
>for (auto argtype: function_args (functype))
>  {
>...
>  }
> 
> Tested on x86_64-linux, and (lightly) on powerpc64le-linux using
> a cross-compiler.  I'll test the changes to the other back ends
> before committing.

How does this interact with debugging?  Because, in my experience, the more 
you convert things to modernish C++, the less you can easily debug them...

-- 
Eric Botcazou


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

2018-11-19 Thread Lokesh Janghel
Thank you Jakub, I update the patch with your comments and tested it.
Please let me know your thoughts/suggestions.

On Mon, Nov 19, 2018 at 4:00 PM Lokesh Janghel
 wrote:
>
> Thank you Jakub, I update the patch with your comments and tested it.
> Please let me know your thoughts/suggestions.
>
>
> Thanks
> Lokesh
>
> On Fri, Nov 16, 2018 at 4:57 PM Jakub Jelinek  wrote:
>>
>> On Fri, Nov 16, 2018 at 04:21:25PM +0530, Umesh Kalappa wrote:
>> > My bad ,
>> > attached the same now .
>>
>> +2018-11-15  Lokesh Janghel 
>>
>> Two spaces before < instead of just one.
>> +
>> +   PR  target/85667
>>
>> Only a single space between PR and target.
>>
>> +   * i386.c (function_value_ms_64): ms_abi insist to use the eax
>>
>> The filename is relative to the directory with ChangeLog file, so
>> * config/i386/i386.c
>> in this case.  The description should say what you've changed, so
>> something like:
>> * config/i386/i386.c (function_value_ms_64): Return AX_REG instead
>> of FIRST_SSE_REG for 4 or 8 byte modes.
>>
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -9008,7 +9008,7 @@ function_value_ms_64 (machine_mode orig_mode, 
>> machine_mode mode,
>> case 8:
>> case 4:
>>   if (mode == SFmode || mode == DFmode)
>> -   regno = FIRST_SSE_REG;
>> +   regno = AX_REG;
>>   break;
>>
>> Is there something to back that up, say godbolt.org link with some testcases
>> showing how does MSVC, clang etc. handle those?
>> And, because the function starts with:
>>   unsigned int regno = AX_REG;
>> the change isn't right, you should remove all of:
>> case 8:
>> case 4:
>>   if (mode == SFmode || mode == DFmode)
>> regno = FIRST_SSE_REG;
>>   break;
>> because the default will do what you want.
>>
>> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
>> index 50e53f0..ec54330 100644
>> --- a/gcc/testsuite/ChangeLog
>> +++ b/gcc/testsuite/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2018-11-15 Lokesh Janghel  
>>
>> Two spaces between date and name.
>>
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr85667.c
>> @@ -0,0 +1,29 @@
>> +/* { dg-options "-O2 " } */
>> +/* { dg-final { scan-assembler-times "movl\[\t 
>> \]\.\[a-zA-Z\]\[a-zA-Z\]\[0-9\]\\(%rip\\), %eax" 1} } */
>>
>> First of all, the test is misplaced, it is clearly x86_64 specific
>> and probably lp64 only, so it shouldn't be in gcc.dg/ where it will
>> be run on all targets, but in gcc.target/i386/ and be guarded with
>> { target lp64 }.
>>
>> Second, seems like you'd like to run the testcase, so you'd better have it
>> /* { dg-do run { target lp64 } } */
>>
>> The assembler scanning will work only with -masm=att, not -masm=intel
>> and seems to be very fragile, so I'd suggest have one runtime test and one
>> compile time test in which you put just the fn1 function.  Why two arbitrary
>> letters after dot?  That makes on sense.  Either you are looking for 
>> .LC\[0-9]*
>> specifically, or for arbitrary symbol, then use something like
>> "movl\[^\n\r]*, %eax"
>> or so (and make sure to use -masm=intel).
>>
>> More interesting would be
>> make check ALT_CC_UNDER_TEST=msvc ALT_CXX_UNDER_TEST=msvc++ 
>> RUNTESTFLAGS='compat.exp struct-layout-1.exp'
>> (or whatever MSVC driver names are), i.e. try to run the compat testsuites
>> between MSVC and newly built gcc.
>>
>> Jakub
>
>
>
> --
> Thanks & Regards
> Lokesh Janghel
> +91-9752984749



-- 
Thanks & Regards
Lokesh Janghel
+91-9752984749


85667.patch
Description: Binary data


Re: [PATCH 2/2] Fix -fsave-optimization-record ICE (PR tree-optimization/87025)

2018-11-19 Thread Richard Biener
On Sat, Nov 17, 2018 at 1:12 AM David Malcolm  wrote:
>
> PR tree-optimization/87025 reports an ICE within
> -fsave-optimization-record's optrecord_json_writer.
>
> The issue is that dump_context::begin_scope creates an optinfo
> of kind OPTINFO_KIND_SCOPE, but fails to call
> dump_context::end_any_optinfo, so the optinfo for the scope remains
> pending.
>
> The JSON writer would normally push a JSON array for the "scope" optinfo
> when the latter is emitted.  However, if a dump_* call happens that
> doesn't flush the "scope" optinfo e.g. dump_printf (as opposed to
> dump_printf_loc), that dump_ call is added to the pending optinfo, and
> optinfo::handle_dump_file_kind changes the pending optinfo's m_kind
> (e.g. to OPTINFO_KIND_NOTE).  Hence when the pending optinfo is
> eventually emitted, it isn't OPTINFO_KIND_SCOPE anymore, and hence
> the JSON writer doesn't create and push a JSON array for it, leading
> to dump_context's view of scopes getting out-of-sync with that of
> the JSON writer's.
>
> Later, dump_context::end_scope unconditionally tries to pop the JSON scope
> array, but no JSON scope array was added, leading to an assertion
> failure (or crash).
>
> The fix is to call dump_context::end_any_optinfo immediately after
> creating the scope optinfo, so that it is emitted immediately, ensuring
> that the JSON writer stays in-sync with the dump_context.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> (in conjuction with the previous patch)
>
> OK for trunk?

OK.

> gcc/ChangeLog:
> PR tree-optimization/87025
> * dumpfile.c (dump_context::begin_scope): Call end_any_optinfo
> immediately after creating the scope optinfo.
> (selftest::test_pr87025): New function.
> (selftest::dumpfile_c_tests): Call it.
> * optinfo-emit-json.cc (optrecord_json_writer::pop_scope): Assert
> that we're not popping the top-level records array.
> * optinfo.cc (optinfo::handle_dump_file_kind): Assert that we're
> not changing the kind of a "scope" optinfo.
>
> gcc/testsuite/ChangeLog:
> PR tree-optimization/87025
> * gcc.dg/pr87025.c: New test.
> ---
>  gcc/dumpfile.c | 16 
>  gcc/optinfo-emit-json.cc   |  3 +++
>  gcc/optinfo.cc |  3 +++
>  gcc/testsuite/gcc.dg/pr87025.c | 22 ++
>  4 files changed, 44 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/pr87025.c
>
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 014acf1..e125650 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -1131,6 +1131,7 @@ dump_context::begin_scope (const char *name, const 
> dump_location_t &loc)
>optinfo &info = begin_next_optinfo (loc);
>info.m_kind = OPTINFO_KIND_SCOPE;
>info.add_item (item);
> +  end_any_optinfo ();
>  }
>else
>  delete item;
> @@ -2575,6 +2576,20 @@ test_capture_of_dump_calls (const line_table_case 
> &case_)
>}
>  }
>
> +static void
> +test_pr87025 ()
> +{
> +  dump_user_location_t loc
> += dump_user_location_t::from_location_t (UNKNOWN_LOCATION);
> +
> +  temp_dump_context tmp (true, true,
> +MSG_ALL_KINDS | MSG_PRIORITY_USER_FACING);
> +  {
> +AUTO_DUMP_SCOPE ("outer scope", loc);
> +dump_printf (MSG_NOTE, "msg1\n");
> +  }
> +}
> +
>  /* Run all of the selftests within this file.  */
>
>  void
> @@ -2582,6 +2597,7 @@ dumpfile_c_tests ()
>  {
>test_impl_location ();
>for_each_line_table_case (test_capture_of_dump_calls);
> +  test_pr87025 ();
>  }
>
>  } // namespace selftest
> diff --git a/gcc/optinfo-emit-json.cc b/gcc/optinfo-emit-json.cc
> index 4fa6708..841a13b 100644
> --- a/gcc/optinfo-emit-json.cc
> +++ b/gcc/optinfo-emit-json.cc
> @@ -169,6 +169,9 @@ void
>  optrecord_json_writer::pop_scope ()
>  {
>m_scopes.pop ();
> +
> +  /* We should never pop the top-level records array.  */
> +  gcc_assert (m_scopes.length () > 0);
>  }
>
>  /* Create a JSON object representing LOC.  */
> diff --git a/gcc/optinfo.cc b/gcc/optinfo.cc
> index f8e08de..f76da45 100644
> --- a/gcc/optinfo.cc
> +++ b/gcc/optinfo.cc
> @@ -133,6 +133,9 @@ optinfo::emit_for_opt_problem () const
>  void
>  optinfo::handle_dump_file_kind (dump_flags_t dump_kind)
>  {
> +  /* Any optinfo for a "scope" should have been emitted separately.  */
> +  gcc_assert (m_kind != OPTINFO_KIND_SCOPE);
> +
>if (dump_kind & MSG_OPTIMIZED_LOCATIONS)
>  m_kind = OPTINFO_KIND_SUCCESS;
>else if (dump_kind & MSG_MISSED_OPTIMIZATION)
> diff --git a/gcc/testsuite/gcc.dg/pr87025.c b/gcc/testsuite/gcc.dg/pr87025.c
> new file mode 100644
> index 000..059313c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr87025.c
> @@ -0,0 +1,22 @@
> +/* Ensure we don't ICE when tracking optimization record scopes within
> +   the vectorizer.  */
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fsave-optimization-record -ftree-vectorize 
> -fno-tree-scev-cprop -fno-tree-sink" } */
> +
> +void

Re: [PATCH 1/2] Eliminate global state from -fsave-optimization-record

2018-11-19 Thread Richard Biener
On Sat, Nov 17, 2018 at 1:12 AM David Malcolm  wrote:
>
> As work towards fixing PR tree-optimization/87025, this patch
> eliminates global state from optinfo-emit-json.cc in favor
> of adding an optional m_json_writer field to dump_context,
> replacing the m_forcibly_enable_optinfo flag.
>
> This allows for writing selftests for the interaction of the
> JSON-building code with the dumpfile.c code.
> In particular, the existing selftest that created optinfo
> instances now exercise the JSON-building code (although no
> JSON is actually written out).
>
> The patch also simplifies the layering by replacing optinfo::emit ()
> with dump_context::emit_optinfo, so that dump_context has
> responsibility for keeping track of dump destinations.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
> (in conjuction with the followup patch)
>
> OK for trunk?

OK.

Richard.

> gcc/ChangeLog:
> PR tree-optimization/87025
> * dump-context.h: Include "optinfo.h".
> (class optrecord_json_writer): New forward decl.
> (dump_context::forcibly_enable_optinfo_p): Delete.
> (dump_context::optinfo_enabled_p): New member function.
> (dump_context::optimization_records_enabled_p): New member
> function.
> (dump_context::set_json_writer): New member function.
> (dump_context::emit_optinfo): New member function.
> (dump_context::m_forcibly_enable_optinfo): Delete.
> (dump_context::m_json_writer): New member data.
> * dumpfile.c (dump_context::set_json_writer): New member function.
> (dump_context::finish_any_json_writer): New member function.
> (dump_context::end_scope): Replace call to
> optimization_records_maybe_pop_dump_scope with call to
> m_json_writer->pop_scope.
> (dump_context::optinfo_enabled_p): New member function.
> (dump_context::end_any_optinfo): Replace call to optinfo::emit with 
> call
> to dump_context::emit_optinfo.
> (dump_context::emit_optinfo): New member function.
> (temp_dump_context::temp_dump_context): Replace
> m_forcibly_enable_optinfo with call to set_json_writer.
> (temp_dump_context::~temp_dump_context): Clean up any json writer.
> * optinfo-emit-json.cc (class optrecord_json_writer): Move to
> optinfo-emit-json.h
> (the_json_writer): Delete.
> (optimization_records_start): Delete.
> (optimization_records_finish): Delete.
> (optimization_records_enabled_p): Delete, in favor of
> dump_context::optimization_records_enabled_p.
> (optimization_records_maybe_record_optinfo): Delete.
> (optimization_records_maybe_pop_dump_scope): Delete.
> * optinfo-emit-json.h: Include "json.h".  Delete forward
> decl of opt_pass.
> (optimization_records_start): Delete.
> (optimization_records_finish): Delete.
> (optimization_records_enabled_p): Delete.
> (optimization_records_maybe_record_optinfo): Delete.
> (optimization_records_maybe_pop_dump_scope): Delete.
> (class optrecord_json_writer): Move here from
> optinfo-emit-json.cc.
> * optinfo.cc (optinfo::emit_for_opt_problem): Replace call
> to optinfo::emit with call to dump_context::emit_optinfo.
> (optinfo::emit): Delete, in favor of dump_context::emit_optinfo.
> (optinfo_enabled_p): Delete, in favor of
> dump_context::optinfo_enabled_p.
> (optinfo_wants_inlining_info_p): Update for conversion o
> optimization_records_enabled_p to a member function of
> dump_context.
> * optinfo.h (optinfo_enabled_p): Delete, in favor of
> dump_context::optinfo_enabled_p.
> (optinfo::emit): Delete, in favor of dump_context::emit_optinfo.
> * toplev.c: Include "dump-context.h".
> (compile_file): Replace call to optimization_records_finish with
> dump_context::finish_any_json_writer.
> (do_compile): Replace call to optimization_records_start with
> conditionally creating a optrecord_json_writer for the
> dump_context.
> ---
>  gcc/dump-context.h   |  22 ---
>  gcc/dumpfile.c   |  55 --
>  gcc/optinfo-emit-json.cc | 101 
> ---
>  gcc/optinfo-emit-json.h  |  42 +++-
>  gcc/optinfo.cc   |  25 +---
>  gcc/optinfo.h|   8 
>  gcc/toplev.c |   8 +++-
>  7 files changed, 109 insertions(+), 152 deletions(-)
>
> diff --git a/gcc/dump-context.h b/gcc/dump-context.h
> index ace139c..2016ce7 100644
> --- a/gcc/dump-context.h
> +++ b/gcc/dump-context.h
> @@ -25,7 +25,9 @@ along with GCC; see the file COPYING3.  If not see
>  #include "dumpfile.h"
>  #include "pretty-print.h"
>  #include "selftest.h"
> +#include "optinfo.h"
>
> +class optrecord_json_writer;
>  namespace selftest { class te

Re: [PATCH] make function_args_iterator a proper iterator

2018-11-19 Thread Richard Biener
On Sat, Nov 17, 2018 at 12:05 AM Martin Sebor  wrote:
>
> To encourage and simplify the adoption of iterator classes in
> GCC the attached patch turns the function_args_iterator struct
> into an (almost) proper C++ iterator class that can be used
> the same way as traditional forward iterators.
>
> The patch also replaces all of the 26 uses of the legacy
> FOREACH_FUNCTION_ARGS macro with ordinary for loops that use
> function_args_iterator directly, and also poisons both
> FOREACH_FUNCTION_ARGS and the unused FOREACH_FUNCTION_ARGS_PTR
> macros.
>
> The few dozen (hundred?) existing uses of for loops that iterate
> over function parameter types using the TREE_CHAIN() macro can
> be relatively easily modified to adopt the iterator approach over
> time.  (The patch stops of short of making this change.)
>
> Eventually, when GCC moves to more a recent C++ revision, it will
> become possible to simplify the for loops to make use of the range
> based for loop syntax along the lines of:
>
>for (auto argtype: function_args (functype))
>  {
>...
>  }
>
> Tested on x86_64-linux, and (lightly) on powerpc64le-linux using
> a cross-compiler.  I'll test the changes to the other back ends
> before committing.

This isn't stage3 material.

Richard.

>
> Martin
>
> PS For some additional background on this change see:
>https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00493.html


Fix bad interaction between ce and cmpelim passes on Visium

2018-11-19 Thread Eric Botcazou
This is a code quality regression that appeared during GCC 8 development on 
Visium in the form of the failure of gcc.target/visium/overflow*.c tests and 
that I swept under the rug back then.

On architectures which do not provide scc instructions like Visium, you can 
nevertheless implement a few variants of cstore by using the carry, which is 
traditionally modeled as sltu, and some tricks (for example, on Visium, we 
implement seq/sne/sltu/sgtu/sleu/sgeu this way); the only counterpart is that 
the patterns are a bit convoluted if you want to have exact RTL semantics.

So generating cstore instructions, usually done by the if-conversion pass, can 
disable the elimination of redundant comparison instructions, only done by the 
compare-elim pass on some RISC architectures, because the latter pass is based 
on generic pattern matching.  That's what happened here for UNEGV operations.

The regression is fixed by teaching the compare-elim pass to handle a NOT in 
the first operand of comparisons and adjusting the neg2_insn_set_carry 
of the Visium to make it consistent with the unegv3 pattern.

Tested on visium-elf, sparc-sun-solaris2.11 and x86_64-suse-linux, applied on 
the mainline.


2018-11-19  Eric Botcazou  

* compare-elim.c (struct comparison): Add not_in_a field.
(is_not): New static function.
(strip_not): Likewise.
(conforming_compare): Handle a NOT in the first operand.
(can_eliminate_compare): Likewise.
(find_comparison_dom_walker::before_dom_children): Likewise.
(try_eliminate_compare): Likewise.
* config/visium/visium.md (negsi2_insn_set_carry): Turn into...
(neg2_insn_set_carry): ...this and add missing NEG operation.


2018-11-19  Eric Botcazou  

* gcc.target/visium/overflow8.c: Remove -fno-if-conversion and
unrelated final test.
* gcc.target/visium/overflow16: Likewise.
* gcc.target/visium/overflow32.c: Likewise.

-- 
Eric BotcazouIndex: compare-elim.c
===
--- compare-elim.c	(revision 266178)
+++ compare-elim.c	(working copy)
@@ -123,10 +123,32 @@ struct comparison
 
   /* True if its inputs are still valid at the end of the block.  */
   bool inputs_valid;
+
+  /* Whether IN_A is wrapped in a NOT before being compared.  */
+  bool not_in_a;
 };
   
 static vec all_compares;
 
+/* Return whether X is a NOT unary expression.  */
+
+static bool
+is_not (rtx x)
+{
+  return GET_CODE (x) == NOT;
+}
+
+/* Strip a NOT unary expression around X, if any.  */
+
+static rtx
+strip_not (rtx x)
+{
+  if (is_not (x))
+return XEXP (x, 0);
+
+  return x;
+}
+
 /* Look for a "conforming" comparison, as defined above.  If valid, return
the rtx for the COMPARE itself.  */
 
@@ -147,7 +169,7 @@ conforming_compare (rtx_insn *insn)
   if (!REG_P (dest) || REGNO (dest) != targetm.flags_regnum)
 return NULL;
 
-  if (!REG_P (XEXP (src, 0)))
+  if (!REG_P (strip_not (XEXP (src, 0
 return NULL;
 
   if (CONSTANT_P (XEXP (src, 1)) || REG_P (XEXP (src, 1)))
@@ -278,10 +300,13 @@ can_eliminate_compare (rtx compare, rtx
 return false;
 
   /* Make sure the compare is redundant with the previous.  */
-  if (!rtx_equal_p (XEXP (compare, 0), cmp->in_a)
+  if (!rtx_equal_p (strip_not (XEXP (compare, 0)), cmp->in_a)
   || !rtx_equal_p (XEXP (compare, 1), cmp->in_b))
 return false;
 
+  if (is_not (XEXP (compare, 0)) != cmp->not_in_a)
+return false;
+
   /* New mode must be compatible with the previous compare mode.  */
   machine_mode new_mode
 = targetm.cc_modes_compatible (GET_MODE (compare), cmp->orig_mode);
@@ -365,8 +390,9 @@ find_comparison_dom_walker::before_dom_c
 	  last_cmp = XCNEW (struct comparison);
 	  last_cmp->insn = insn;
 	  last_cmp->prev_clobber = last_clobber;
-	  last_cmp->in_a = XEXP (src, 0);
+	  last_cmp->in_a = strip_not (XEXP (src, 0));
 	  last_cmp->in_b = XEXP (src, 1);
+	  last_cmp->not_in_a = is_not (XEXP (src, 0));
 	  last_cmp->eh_note = eh_note;
 	  last_cmp->orig_mode = GET_MODE (src);
 	  if (last_cmp->in_b == const0_rtx
@@ -837,7 +863,9 @@ try_eliminate_compare (struct comparison
 flags = gen_rtx_REG (cmp->orig_mode, targetm.flags_regnum);
 
   /* Generate a new comparison for installation in the setter.  */
-  rtx y = copy_rtx (cmp_a);
+  rtx y = cmp->not_in_a
+	  ? gen_rtx_NOT (GET_MODE (cmp_a), copy_rtx (cmp_a))
+	  : copy_rtx (cmp_a);
   y = gen_rtx_COMPARE (GET_MODE (flags), y, copy_rtx (cmp_b));
   y = gen_rtx_SET (flags, y);
 
Index: config/visium/visium.md
===
--- config/visium/visium.md	(revision 266178)
+++ config/visium/visium.md	(working copy)
@@ -1208,14 +1208,14 @@ (define_insn "*neg2_insn   %0,r0,%1"
   [(set_attr "type" "arith")])
 
-(define_insn "negsi2_insn_set_carry"
+(define_insn "neg2_insn_set_carry"
   [(set (reg:CCC R_FLAGS)
-	(compare:CCC (not:SI (match_operand:SI 1 "register_operand" "r"))

[PATCH, libgcc/ARM & testsuite] Optimize executable size when using softfloat fmul/dmul

2018-11-19 Thread Thomas Preudhomme
Softfloat single precision and double precision floating-point
multiplication routines in libgcc share some code with the
floating-point division of their corresponding precision. As the code
is structured now, this leads to *all* division code being pulled in an
executable in softfloat mode even if only multiplication is
performed.

This patch create some new LIB1ASMFUNCS macros to also build files with
just the multiplication and shared code as weak symbols. By putting
these earlier in the static library, they can then be picked up when
only multiplication is used and they are overriden by the global
definition in the existing file containing both multiplication and
division code when division is needed.

The patch also removes changes made to the FUNC_START and ARM_FUNC_START
macros in r218124 since the intent was to put multiplication and
division code into their own section in a later patch to achieve the
same size optimization. That approach relied on specific section layout
to ensure multiplication and division were not too far from the shared
bit of code in order to the branches to be within range. Due to lack of
guarantee regarding section layout, in particular with all the
possibility of linker scripts, this approach was chosen instead. This
patch keeps the two testcases that were posted by Tony Wang (an Arm
employee at the time) on the mailing list to implement this approach
and adds a new one, hence the attribution.

ChangeLog entries are as follows:

*** gcc/ChangeLog ***

2018-11-14  Thomas Preud'homme  

* config/arm/elf.h: Update comment about condition that need to
match with libgcc/config/arm/lib1funcs.S to also include
libgcc/config/arm/t-arm.
* doc/sourcebuild.texi (output-exists, output-exists-not): Rename
subsubsection these directives are in to "Check for output files".
Move scan-symbol to that section and add to it new scan-symbol-not
directive.

*** gcc/testsuite/ChangeLog ***

2018-11-16  Tony Wang  
Thomas Preud'homme  

* lib/lto.exp (lto-execute): Define output_file and testname_with_flags
to same value as execname.
(scan-symbol): Move and rename to ...
* lib/gcc-dg.exp (scan-symbol-common): This.  Adapt into a
helper function returning true or false if a symbol is present.
(scan-symbol): New procedure.
(scan-symbol-not): Likewise.
* gcc.target/arm/size-optimization-ieee-1.c: New testcase.
* gcc.target/arm/size-optimization-ieee-2.c: Likewise.
* gcc.target/arm/size-optimization-ieee-3.c: Likewise.

*** libgcc/ChangeLog ***

2018-11-16  Thomas Preud'homme  

* /config/arm/lib1funcs.S (FUNC_START): Remove unused sp_section
parameter and corresponding code.
(ARM_FUNC_START): Likewise in both definitions.
Also update footer comment about condition that need to match with
gcc/config/arm/elf.h to also include libgcc/config/arm/t-arm.
* config/arm/ieee754-df.S (muldf3): Also build it if L_arm_muldf3 is
defined.  Weakly define it in this case.
* config/arm/ieee754-sf.S (mulsf3): Likewise with L_arm_mulsf3.
* config/arm/t-elf (LIB1ASMFUNCS): Build _arm_muldf3.o and
_arm_mulsf3.o before muldiv versions if targeting Thumb-1 only. Add
comment to keep condition in sync with the one in
libgcc/config/arm/lib1funcs.S and gcc/config/arm/elf.h.

Testing: Bootstrapped on arm-linux-gnueabihf (Arm & Thumb-2) and
testsuite shows no
regression. Also built an arm-none-eabi cross compiler targeting
soft-float which also shows no regression. In particular newly added
tests and gcc.dg/lto/20081212-1 test pass.

Is this ok for stage3?

Best regards,

Thomas
From 8740697791f99b7175e188f049663883c39e51b0 Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme 
Date: Fri, 26 Oct 2018 16:21:09 +0100
Subject: [PATCH] [PATCH, libgcc/ARM] Optimize executable size when using
 softfloat fmul/dmul

Softfloat single precision and double precision floating-point
multiplication routines in libgcc share some code with the
floating-point division of their corresponding precision. As the code
is structured now, this leads to *all* division code being pulled in an
executable in softfloat mode even if only multiplication is
performed.

This patch create some new LIB1ASMFUNCS macros to also build files with
just the multiplication and shared code as weak symbols. By putting
these earlier in the static library, they can then be picked up when
only multiplication is used and they are overriden by the global
definition in the existing file containing both multiplication and
division code when division is needed.

The patch also removes changes made to the FUNC_START and ARM_FUNC_START
macros in r218124 since the intent was to put multiplication and
division code into their own section in a later patch to achieve the
same size optimization. That approach relied on specific section layout
to ensure multiplication and division were not too far from the shared
bit of code in order to the branches to be within range

Re: [doc, committed] clarify that "packed" attribute can apply to C++ classes

2018-11-19 Thread Jonathan Wakely

On 15/11/18 17:52 -0700, Sandra Loosemore wrote:
I've checked in this patch for PR 25759, another minor documentation 
improvement.


I'll commit this patch as obvious as soon as svn stops being slow.

I wonder if the last sentence would be further improved by simply
saying "not on a @code{typedef} that does not also define the type"
instead of spelling out the kinds of types for a second time.


commit 546daacf7086b3f7b79ba067f4660b912ba28a09
Author: Jonathan Wakely 
Date:   Mon Nov 19 09:39:33 2018 +

Fix typos in packed attribute documentation

* doc/extend.texi (Common Type Attributes): Fix typos.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index d230da977d4..0dff5e86ed1 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7317,7 +7317,7 @@ or @code{__pointer__} for the mode used to represent pointers.
 
 @item packed
 @cindex @code{packed} type attribute
-This attribute, attached to @code{struct}, @code{union}, or C++ @code{class}
+This attribute, attached to a @code{struct}, @code{union}, or C++ @code{class}
 type definition, specifies that each of its members (other than zero-width
 bit-fields) is placed to minimize the memory required.  This is equivalent
 to specifying the @code{packed} attribute on each of the members.
@@ -7349,7 +7349,7 @@ struct __attribute__ ((__packed__)) my_packed_struct
   @};
 @end smallexample
 
-You may only specify the @code{packed} attribute attribute on the definition
+You may only specify the @code{packed} attribute on the definition
 of an @code{enum}, @code{struct}, @code{union}, or @code{class}, 
 not on a @code{typedef} that does not also define the enumerated type,
 structure, union, or class.


[Ada] Remove obsolete code in UI_From_gnu

2018-11-19 Thread Eric Botcazou
Now that HOST_BITS_PER_WIDE_INT is always 64, we can get rid of disabled code 
in the UI_From_gnu routine.

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


2018-11-19  Eric Botcazou  

* gcc-interface/cuintp.c (UI_From_gnu): Remove code for 32-bit hosts.

-- 
Eric BotcazouIndex: gcc-interface/cuintp.c
===
--- gcc-interface/cuintp.c	(revision 266178)
+++ gcc-interface/cuintp.c	(working copy)
@@ -142,13 +142,16 @@ UI_From_gnu (tree Input)
   /* UI_Base is defined so that 5 Uint digits is sufficient to hold the
  largest possible signed 64-bit value.  */
   const int Max_For_Dint = 5;
-  int v[Max_For_Dint], i;
+  int v[Max_For_Dint];
   Vector_Template temp;
   Int_Vector vec;
 
-#if HOST_BITS_PER_WIDE_INT == 64
-  /* On 64-bit hosts, tree_fits_shwi_p tells whether the input fits in a
- signed 64-bit integer.  Then a truncation tells whether it fits
+#if HOST_BITS_PER_WIDE_INT < 64
+#error unsupported HOST_BITS_PER_WIDE_INT setting
+#endif
+
+  /* On 64-bit hosts, tree_fits_shwi_p tells whether the input fits in
+ a signed 64-bit integer.  Then a truncation tells whether it fits
  in a signed 32-bit integer.  */
   if (tree_fits_shwi_p (Input))
 {
@@ -158,24 +161,11 @@ UI_From_gnu (tree Input)
 }
   else
 return No_Uint;
-#else
-  /* On 32-bit hosts, tree_fits_shwi_p tells whether the input fits in a
- signed 32-bit integer.  Then a sign test tells whether it fits
- in a signed 64-bit integer.  */
-  if (tree_fits_shwi_p (Input))
-return UI_From_Int (tree_to_shwi (Input));
-
-  gcc_assert (TYPE_PRECISION (gnu_type) <= 64);
-  if (TYPE_UNSIGNED (gnu_type)
-  && TYPE_PRECISION (gnu_type) == 64
-  && wi::neg_p (Input, SIGNED))
-return No_Uint;
-#endif
 
   gnu_base = build_int_cst (gnu_type, UI_Base);
   gnu_temp = Input;
 
-  for (i = Max_For_Dint - 1; i >= 0; i--)
+  for (int i = Max_For_Dint - 1; i >= 0; i--)
 {
   v[i] = tree_to_shwi (fold_build1 (ABS_EXPR, gnu_type,
 	fold_build2 (TRUNC_MOD_EXPR, gnu_type,


Re: [PATCH]Come up with -flive-patching master option.

2018-11-19 Thread Martin Liška
On 11/16/18 5:04 PM, Qing Zhao wrote:
> 
>> On Nov 16, 2018, at 9:26 AM, Martin Liška > > wrote:
>>
>> On 11/16/18 2:36 AM, Qing Zhao wrote:
>>> Hi,
>>>
>>> this is the new version of the patch.
>>>
>>> I have bootstrapped it on both aarch64 and x86,  no regression.
>>>
>>> please take a look.
>>
>> Thanks for the updated version of the patch.
>> I have last small nits I see:
>>
>> - gcc/common.opt: when running --help=common, the line is too long
> 
> the following is the output for ./gcc —help=common:
>   -flive-patching             Same as -flive-patching=.  Use the latter option
>                               instead.
>   -flive-patching=[inline-only-static|inline-clone] Control IPA optimizations
>                               to provide a safe compilation for live-patching.
>                               At the same time, provides multiple-level 
> control
>                               on the enabled IPA optimizations.
>  
> Not sure what’s you mean of “the line is too long”? could you please specify 
> the above which line?

You are probably using a console that has quite small column limit, so that you 
see it automatically
wrapped.

I see:

...
  -flimit-function-alignment  This option lacks documentation.
  -flive-patching Same as -flive-patching=.  Use the latter option 
instead.
  -flive-patching=[inline-only-static|inline-clone] Control IPA optimizations 
to provide a safe compilation for live-patching. At the same time, provides 
multiple-level control on the enabled IPA optimizations.

^--- the long line

  -flive-range-shrinkage  Relief of register pressure through live range 
shrinkage.
  -floop-blockEnable loop nest transforms.  Same as 
-floop-nest-optimize.  Same as -floop-nest-optimize.
...

> 
>> - gcc/doc/invoke.texi - 2 spaces in between sentences + better gol
>> - gcc/opts.c - do not mix spaces + tabs
> 
> I have used contrib/check_GNU_style.sh to check the patch, I did see one 
> place that complains about 2 spaces in between sentences, fixed it.

I see it:

=== ERROR type #3: dot, space, space, new sentence (3 error(s)) ===
gcc/common.opt:2190:62:optimizations to provide a safe compilation for 
live-patching.█At the same
gcc/doc/invoke.texi:9291:14:optimizations.█For example, inlining a function 
into its caller, cloning
gcc/doc/invoke.texi:9297:37:impacted functions for each function.█In order to 
control the number of

> but I didn’t see spaces + tabs mix issue with the script. could you please 
> specify?

This is a new check that I've just installed:

=== ERROR type #1: a space should not precede a tab (1 error(s)) ===
gcc/opts.c:2350:0:control_optimizations_for_live_patching (opts, 
opts_set,

Martin

> 
> thanks.
> 
> Qing
> *
> *
> 



[PATCH] Detect mixed usage of spaces and tabs.

2018-11-19 Thread Martin Liška
Hi.

It's enhancement of the check_GNU_style_lib library.
Detect situations where a space precedes a tab at
the beginning of a line.

I'm going to install the patch.

Martin

contrib/ChangeLog:

2018-11-19  Martin Liska  

* check_GNU_style_lib.py: Detect mixed usage
of spaces and tabs.
---
 contrib/check_GNU_style_lib.py | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)


diff --git a/contrib/check_GNU_style_lib.py b/contrib/check_GNU_style_lib.py
index 63d0538aa57..ac3682fb2af 100755
--- a/contrib/check_GNU_style_lib.py
+++ b/contrib/check_GNU_style_lib.py
@@ -99,6 +99,18 @@ class SpacesCheck:
 line.replace(self.expanded_tab, error_string(ws_char * ts)),
 'blocks of 8 spaces should be replaced with tabs', i)
 
+class SpacesAndTabsMixedCheck:
+def __init__(self):
+self.re = re.compile('\ \t')
+
+def check(self, filename, lineno, line):
+stripped = line.lstrip()
+start = line[:len(line) - len(stripped)]
+if self.re.search(line):
+return CheckError(filename, lineno,
+error_string(start.replace('\t', ws_char * ts)) + line[len(start):],
+'a space should not precede a tab', 0)
+
 class TrailingWhitespaceCheck:
 def __init__(self):
 self.re = re.compile('(\s+)$')
@@ -236,12 +248,27 @@ class TrailingWhitespaceTest(unittest.TestCase):
 r = self.check.check('foo', 123, 'a = 123;\t')
 self.assertIsNotNone(r)
 
+class SpacesAndTabsMixedTest(unittest.TestCase):
+def setUp(self):
+self.check = SpacesAndTabsMixedCheck()
+
+def test_trailing_whitespace_check_basic(self):
+r = self.check.check('foo', 123, '   \ta = 123;')
+self.assertEqual('foo', r.filename)
+self.assertEqual(0, r.column)
+self.assertIsNotNone(r.console_error)
+r = self.check.check('foo', 123, '   \t  a = 123;')
+self.assertIsNotNone(r.console_error)
+r = self.check.check('foo', 123, '\t  a = 123;')
+self.assertIsNone(r)
+
 def check_GNU_style_file(file, file_encoding, format):
 checks = [LineLengthCheck(), SpacesCheck(), TrailingWhitespaceCheck(),
 SentenceSeparatorCheck(), SentenceEndOfCommentCheck(),
 SentenceDotEndCheck(), FunctionParenthesisCheck(),
 SquareBracketCheck(), ClosingParenthesisCheck(),
-BracesOnSeparateLineCheck(), TrailinigOperatorCheck()]
+BracesOnSeparateLineCheck(), TrailinigOperatorCheck(),
+SpacesAndTabsMixedCheck()]
 errors = []
 
 patch = PatchSet(file, encoding=file_encoding)