Re: PR79286, ira combine_and_move_insns in loops

[Jeff Law]

On 02/22/2017 09:32 AM, Dominique d'Humières wrote:

Let me stand up an i686 linux instance and see if I can twiddle things without 
compromising the test.


Also darwin is a -fpic platform.
Which is the key here :-)  The test works fine without PIC, but once PIC 
is introduced, it blows up.


It's pretty easy to see in the assembly code and the dumps.

Prior to IRA we have:

bb3 (loop header):
[ ... ]
(insn 8 7 9 3 (set (mem:SI (pre_dec:SI (reg/f:SI 7 sp)) [1  S4 A32])
(reg/v:SI 89 [ e ])) "j.c":9 56 {*pushsi2}
 (expr_list:REG_ARGS_SIZE (const_int 8 [0x8])
(nil)))
[ ... ]
bb5 (conditional within the loop):
(insn 28 26 30 5 (set (reg/v:SI 89 [ e ])
(mem/u:SI (plus:SI (reg:SI 87)
(const:SI (plus:SI (unspec:SI [
(symbol_ref:SI ("d") [flags 0x2] 
)

] UNSPEC_GOTOFF)
(const_int 331350016 [0x13c0] [1 
d+331350016 S4 A32])) "j.c":11 75 {*movsi_internal}

 (expr_list:REG_DEAD (reg:SI 87)
(nil)))


After IRA we have:

bb3 (loop header)

(insn 8 7 9 3 (set (mem:SI (pre_dec:SI (reg/f:SI 7 sp)) [1  S4 A32])
(mem/u:SI (plus:SI (reg:SI 87)
(const:SI (plus:SI (unspec:SI [
(symbol_ref:SI ("d") [flags 0x2] 
)

] UNSPEC_GOTOFF)
(const_int 331350016 [0x13c0] [1 
d+331350016 S4 A32])) "j.c":9 56 {*pushsi2}

 (expr_list:REG_ARGS_SIZE (const_int 8 [0x8])
(nil)))

And of course that goes boom.

Note that we don't have a REG_EQUAL prior to IRA.  That allows us to 
zero in on this code within IRA which adds the note to insn 28:


3485  /* cse sometimes generates function invariants, but 
doesn't put a
3486 REG_EQUAL note on the insn.  Since this note would 
be redundant,

3487 there's no point creating it earlier than here.  */
3488  if (! note && ! rtx_varies_p (src, 0))
3489note = set_unique_reg_note (insn, REG_EQUAL, 
copy_rtx (src));


may_trap_p returns false for the note:

expr_list:REG_EQUAL (mem/u:SI (plus:SI (reg:SI 87)
(const:SI (plus:SI (unspec:SI [
(symbol_ref:SI ("d") [flags 0x2] 0xb79450fc d>)

] UNSPEC_GOTOFF)
(const_int 331350016 [0x13c0] [1 
d+331350016 S4 A32])



And thus we keep the equivalence.  Ultimately may_trap_p considers a PIC 
memory reference as non-trapping.


I really wonder if we should just drop the may_trap_p test and always do 
the domination check.


jeff

Re: [PATCH 1/2] [ARM] Refactor costs calculation for MEM.

[Bernhard Reutner-Fischer]

On 21 February 2017 17:54:23 CET, charles.bay...@linaro.org wrote:
>From: Charles Baylis 
>

>+/* Convert fron bytes to ints.  */

s/fron/from/

>+#define ARM_NUM_INTS(X) (((X) + UNITS_PER_WORD - 1) / UNITS_PER_WORD)
>+
>+
> /* RTX costs.  Make an estimate of the cost of executing the operation
>X, which is contained with an operation with code OUTER_CODE.
>SPEED_P indicates whether the cost desired is the performance cost,

s/contained with /contained within /
while you are at it.

thanks,

Re: PR rtl-optimization/64081: Enable RTL loop unrolling for duplicated exit blocks and back edges (with AIX fixes)

[Jeff Law]

On 02/15/2017 03:49 AM, Aldy Hernandez wrote:

On 02/13/2017 07:15 PM, Jeff Law wrote:


So it seems in your updated patch there is only one call where we ask
for LOOP_EXIT_COMPLEX, specifically the call from get_loop_location.

But I don't see how asking for LOOP_EXIT_COMPLEX from that location
would change whether or not we unroll any given loop (which is the core
of bz64081).

Am I missing something?


Ughh, only the spaghetti that is this code? ;-).

get_loop_location is only called once in the compiler, in
decide_unrolling().  This call to get_loop_location() will set the loop
description, particularly desc->simple_p where you point out.
The "desc" persists, hanging off the loop structure and its the 
existence (or lack thereof) which enables/disables unrolling.


ie, we ask for complex exits via the get_loop_location calls.  So loops 
with complex exits will get a DESC and thus will get unrolled.  Other 
unpleasant things may happen as a result of having a cached DESC as well.


If we're going to go this direction, I think we need to store the exit 
type in the DESC, then verify that if we ask for simple exits, then only 
give back a DESC for loops with simple exits.  If we ask for complex 
exits, then we can give back any cached DESC.


Then every instance where we ask for a desc has to be checked for 
whether or not it is safe in the presence of complex exits and ask for 
the appropriate style.






Later on down in decide_unrolling(), we decide the number of iterations,
and use desc->simple_p to ignore the loop if it is not simple.

  decide_unroll_constant_iterations (loop, flags);
  if (loop->lpt_decision.decision == LPT_NONE)
decide_unroll_runtime_iterations (loop, flags);
  if (loop->lpt_decision.decision == LPT_NONE)
decide_unroll_stupid (loop, flags);

Any one of these functions will bail if the loop description was not
simple_p:
I don't think so.  If that were the case, then we'd never unroll the 
loop in the testcase.  In fact, decide_unroll_constant_iterations 
unrolls the loop with the complex exit.






Now a problem I see here is that decide_unroll_*_iterations all call
get_simple_loop_desc() which is basically LOOP_EXIT_SIMPLE, but since
the value is already cached we return the previous call that was
LOOP_EXIT_COMPLEX.  So the code works because we will already have a
cached value.
Right.  And ISTM it's the existence of the DESC structure that's the key 
here.  Right?




I think to make it clearer we could:

1. Add an assert in get_loop_desc to make sure that if we're returning a
cached loop description, that the LOOP_EXIT_TYPEs match.  Just in case...
Probably not a bad idea.  But you don't save the loop exit type AFAICT. 
In fact, if you did, you'd quickly see that the calls via decide_* would 
likely trigger your assert.






2. Change all the decide_unroll_*_iterations variants to specifically
ask for a LOOP_EXIT_TYPE, not just  the simple variant. And have this
set to LOOP_EXIT_COMPLEX from decide_unrolling.  Right now, this is all
working because we have only one call to get_loop_location, but I assume
that could change.
But the simple variants all ask for LOOP_EXIT_SIMPLE, so I don't see 
this as improving things significantly.




3. And finally, what the heck is get_loop_location doing in cfgloop,
when it's only used once within loop-unroll.c?  I say we move it to
loop-unroll.c and mark it static.

Seems like a reasonable cleanup.




Does this help?
It does.  But it really shows that the introduction of simple vs complex 
exits is messed up.  All we've done is add another layer of complexity 
that happens to work IMHO, but it seems to do so more by accident than 
design.


Jeff

Re: [PATCH] Change default of param not being smaller that min.

[Jeff Law]

On 02/21/2017 01:09 AM, Martin Liška wrote:

On 02/21/2017 12:03 AM, Alexandre Oliva wrote:

It's convenient to debug -fcompare-debug errors.  You set the param to a
large number, and then non-debug insns will get the same uid in both
debug and non-debug compilations, so it's easier to compare RTL dumps
(using the attached gcc-diff-dump; I thought it was in contrib, but it's
not; should I put it there?) and locate the differences.

Yes, please commit the script, I'm interested in :)

There's second version, which changes default to 0.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin


0001-Change-default-of-param-not-being-smaller-that-min.patch


From c4d196cb9ad7ace112e9013403323fea3ead69fc Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 17 Feb 2017 16:00:30 +0100
Subject: [PATCH] Change default of param not being smaller that min.

gcc/ChangeLog:

2017-02-17  Martin Liska  

* params.def (PARAM_MIN_NONDEBUG_INSN_UID): Change default to 0.

OK.
jeff

Re: [PATCH] Optimize divmod expansion (PR middle-end/79665)

[Jeff Law]

On 02/22/2017 02:40 PM, Jakub Jelinek wrote:

Hi!

If both arguments of integer division or modulo are known to be non-negative
in corresponding signed type, then signed as well as unsigned division/modulo
shall have the exact same result and therefore we can choose between those
two depending on which one is faster (or shorter for -Os), which varries
a lot depending on target and especially for constant divisors on the exact
divisor.  expand_divmod itself is too complicated and we don't even have
the ability to ask about costs e.g. for highpart multiplication without
actually expanding it, so this patch just in that case tries both sequences,
computes their costs and uses the cheaper (and for equal cost honors the
actual original signedness of the operation).

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

2017-02-22  Jakub Jelinek  

PR middle-end/79665
* internal-fn.c (get_range_pos_neg): Moved to ...
* tree.c (get_range_pos_neg): ... here.  No longer static.
* tree.h (get_range_pos_neg): New prototype.
* expr.c (expand_expr_real_2) : If both arguments
are known to be in between 0 and signed maximum inclusive, try to
expand both unsigned and signed divmod and use the cheaper one from
those.

OK.
jeff

[PATCH][PR tree-optimization/79578] Use operand_equal_p rather than pointer equality for base test

[Jeff Law]

tree-ssa-dse.c needs to verify when two writes have the same base 
address.  Right now it uses pointer equality.  The testcase in BZ79578 
shows that we should have been using operand_equal_p.


This one-liner fixes that oversight.  Bootstrapped and regression tested 
on x86_64-linux-gnu.  Installed on the trunk.


Jeff
commit ef506ec9114a7fe27d9ee892c17edd100f72a963
Author: law 
Date:   Thu Feb 23 05:47:43 2017 +

PR tree-optimization/79578
* tree-ssa-dse.c (clear_bytes_written_by): Use operand_equal_p
to compare base operands.

PR tree-optimization/79578
* g++.dg/tree-ssa/ssa-dse-3.C: New test.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@245675 
138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7155850..6da1d74 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2017-02-22 Jeff Law  
+
+   PR tree-optimization/79578
+   * tree-ssa-dse.c (clear_bytes_written_by): Use operand_equal_p
+   to compare base operands.
+
 2017-02-22  Segher Boessenkool  
 
PR target/79211
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index ea5e251..d900cc3 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-02-22  Jeff Law  
+
+   PR tree-optimization/79578
+   * g++.dg/tree-ssa/ssa-dse-3.C: New test.
+
 2017-02-22  Sameera Deshpande  
 
* gcc.target/mips/msa-fp-cc.c: New test.
diff --git a/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-3.C 
b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-3.C
new file mode 100644
index 000..fe8f309
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/ssa-dse-3.C
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c++14 -O3 -fdump-tree-dse1-details" } */
+
+#include 
+#include 
+
+struct A
+{
+std::uint16_t a, b;
+};
+
+A* f(char* b) __attribute__((noinline));
+
+A* f(char* b) {
+auto a = new(b) A{};
+a->a = 1;
+a->b = 2;
+return a;
+}
+
+int main() {
+char b[sizeof(A)] alignas(A);
+f(b);
+}
+
+
+/* { dg-final { scan-tree-dump "Deleted dead store: " "dse1" } } */
+
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 84c0b11..a82e164 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -176,7 +176,7 @@ clear_bytes_written_by (sbitmap live_bytes, gimple *stmt, 
ao_ref *ref)
   /* Verify we have the same base memory address, the write
  has a known size and overlaps with REF.  */
   if (valid_ao_ref_for_dse ()
-  && write.base == ref->base
+  && operand_equal_p (write.base, ref->base, 0)
   && write.size == write.max_size
   && ((write.offset < ref->offset
   && write.offset + write.size > ref->offset)

Merge to gccgo branch

[Ian Lance Taylor]

I merged trunk revision 245668 to the gccgo branch.  I also committed
the patches that have stacked up in gofrontend but have not yet been
committed to trunk, to avoid destabilizing stage 4.

Ian

[gomp4] delete unused variable inside trans-openmp.c

[Cesar Philippidis]

I've applied this patch to gomp-4-0-branch to remove an unused variable
inside trans-openmp.c. I'm not sure why bootstrapping does catch this
sort of error anymore. Maybe my build script of overriding the build
flags some how.

Cesar
2017-02-22  Cesar Philippidis  

	gcc/fortran/
	* trans-openmp.c (gfc_trans_omp_clauses_1): Remove unused variable.


diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 295f172..0b16af6 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -1947,7 +1947,6 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, gfc_omp_clauses *clauses,
 			  && n->expr->ref->next->u.ar.type == AR_FULL)))
 		{
 		  gfc_ref *ref = n->expr->ref;
-		  tree orig_decl = decl;
 		  gfc_component *c = ref->u.c.component;
 		  tree field;
 		  tree context;

[gomp4] correct the reported line number in fortran combined OpenACC directives

[Cesar Philippidis]

Like its c++ counterpart, the fortran FE incorrectly records the line
locations of combined acc loop directives when it lowers the construct
to gimple. Usually this isn't a problem because the fortran FE is able
to report problems with acc loops itself. However, there will be
inaccuracies if the ME tries to use those locations. I had originally
posted the patch here
, but Thomas
asked me to put it in a separate patch.

This patch has been applied to gomp-4_0-branch.

Cesar
2017-02-22  Cesar Philippidis  

	gcc/fortran/
	* trans-openmp.c (gfc_trans_oacc_combined_directive): Set the
	location of combined acc loops.


diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 0b16af6..8688425 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3818,6 +3818,7 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
   enum tree_code construct_code;
   bool scan_nodesc_arrays = false;
   hash_set *array_set = NULL;
+  location_t loc = input_location;
 
   switch (code->op)
 {
@@ -3849,6 +3850,9 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
 pushlevel ();
   stmt = gfc_trans_omp_do (code, EXEC_OACC_LOOP, pblock, loop_clauses, NULL);
 
+  if (CAN_HAVE_LOCATION_P (stmt))
+SET_EXPR_LOCATION (stmt, loc);
+
   if (array_set && array_set->elements ())
 gfc_add_expr_to_block (, stmt);
 
@@ -3864,8 +3868,7 @@ gfc_trans_oacc_combined_directive (gfc_code *code)
   delete array_set;
 }
 
-  stmt = build2_loc (input_location, construct_code, void_type_node, stmt,
-		 oacc_clauses);
+  stmt = build2_loc (loc, construct_code, void_type_node, stmt, oacc_clauses);
   gfc_add_expr_to_block (, stmt);
 
   gfc_free_omp_clauses (loop_clauses);

Re: [PATCH] PR79584, lra ICE in base_to_reg

[Alan Modra]

On Thu, Feb 23, 2017 at 11:41:09AM +1030, Alan Modra wrote:
> lo_sum is indeed not valid for mem:SD.  simplify_operand_subreg is
> where the subreg disappears.

Richard, doesn't the following say that lra is expecting to reload
exactly the lo_sum address you seem to think it should not handle in
process_address?

  /* We still can reload address and if the address is
 valid, we can remove subreg without reloading its
 inner memory.  */
  && valid_address_p (GET_MODE (subst),
  regno_reg_rtx
  [ira_class_hard_regs
   [base_reg_class (GET_MODE (subst),
MEM_ADDR_SPACE (subst),
ADDRESS, SCRATCH)][0]],
  MEM_ADDR_SPACE (subst

-- 
Alan Modra
Australia Development Lab, IBM

[gomp4] correct the reported line number in c++ combined OpenACC directives

[Cesar Philippidis]

The C++ FE isn't setting the expr_location of the split acc loop in
combined acc parallel/kernels loop directives. This only happens for
with combined directives, otherwise cp_parser_omp_construct would be
responsible for setting the location. After fixing this bug, I was able
to resolve a couple of long standing diagnostics discrepancies between
the c/c++ FEs in the test suite.

This patch has been committed to gomp-4_0-branch. Ultimately, this patch
will make its way to trunk, but it's not a huge regression because this
problem has been around for a while now. I'll see if I can get around to
creating a patch for trunk later this week.

Cesar
2017-02-22  Cesar Philippidis  

	gcc/cp/
	* parser.c (cp_parser_oacc_kernels_parallel): Adjust EXPR_LOCATION
	on the combined acc loop.

	gcc/testsuite/
	* c-c++-common/goacc/combined-directives-3.c: New test.
	* c-c++-common/goacc/loop-2-kernels.c (void K): Adjust test.
	* c-c++-common/goacc/loop-2-parallel.c (void P): Adjust test.
	* c-c++-common/goacc/loop-3.c (void p2): Adjust test.


diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d302685..ddb0ab1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -35790,8 +35790,9 @@ cp_parser_oacc_kernels_parallel (cp_parser *parser, cp_token *pragma_tok,
 	  cp_lexer_consume_token (parser->lexer);
 	  tree block = begin_omp_parallel ();
 	  tree clauses;
-	  cp_parser_oacc_loop (parser, pragma_tok, p_name, mask, ,
-			   if_p);
+	  tree stmt = cp_parser_oacc_loop (parser, pragma_tok, p_name, mask,
+	   , if_p);
+	  protected_set_expr_location (stmt, pragma_tok->location);
 	  return finish_omp_construct (code, block, clauses);
 	}
 }
diff --git a/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c b/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
new file mode 100644
index 000..77d4182
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
@@ -0,0 +1,24 @@
+/* Verify the accuracy of the line number associated with combined
+   constructs.  */
+
+int
+main ()
+{
+  int x, y, z;
+
+#pragma acc parallel loop seq auto /* { dg-error "'seq' overrides other OpenACC loop specifiers" } */
+  for (x = 0; x < 10; x++)
+#pragma acc loop
+for (y = 0; y < 10; y++)
+  ;
+
+#pragma acc parallel loop gang auto /* { dg-error "'auto' conflicts with other OpenACC loop specifiers" } */
+  for (x = 0; x < 10; x++)
+#pragma acc loop worker auto /* { dg-error "'auto' conflicts with other OpenACC loop specifiers" } */
+for (y = 0; y < 10; y++)
+#pragma acc loop vector
+  for (z = 0; z < 10; z++)
+	;
+
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c b/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c
index 01ad32d..3a11ef5f 100644
--- a/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c
+++ b/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c
@@ -145,8 +145,8 @@ void K(void)
 #pragma acc kernels loop worker(num:5)
   for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop seq worker // { dg-error "'seq' overrides" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'seq' overrides" "" { target c++ } }
+#pragma acc kernels loop seq worker // { dg-error "'seq' overrides" }
+  for (i = 0; i < 10; i++)
 { }
 #pragma acc kernels loop gang worker
   for (i = 0; i < 10; i++)
@@ -161,8 +161,8 @@ void K(void)
 #pragma acc kernels loop vector(length:5)
   for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop seq vector // { dg-error "'seq' overrides" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'seq' overrides" "" { target c++ } }
+#pragma acc kernels loop seq vector // { dg-error "'seq' overrides" }
+  for (i = 0; i < 10; i++)
 { }
 #pragma acc kernels loop gang vector
   for (i = 0; i < 10; i++)
@@ -174,16 +174,16 @@ void K(void)
 #pragma acc kernels loop auto
   for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop seq auto // { dg-error "'seq' overrides" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'seq' overrides" "" { target c++ } }
+#pragma acc kernels loop seq auto // { dg-error "'seq' overrides" }
+  for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop gang auto // { dg-error "'auto' conflicts" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'auto' conflicts" "" { target c++ } }
+#pragma acc kernels loop gang auto // { dg-error "'auto' conflicts" }
+  for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop worker auto // { dg-error "'auto' conflicts" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'auto' conflicts" "" { target c++ } }
+#pragma acc kernels loop worker auto // { dg-error "'auto' conflicts" }
+  for (i = 0; i < 10; i++)
 { }
-#pragma acc kernels loop vector auto // { dg-error "'auto' conflicts" "" { target c } }
-  for (i = 0; i < 10; i++) // { dg-error "'auto' conflicts" "" { target c++ } }
+#pragma acc kernels loop vector auto // { dg-error "'auto' conflicts" }
+  for (i = 0; i < 10; i++)
 { }
 }

C++ PATCH for c++/79679, missing destructor for argument

[Jason Merrill]

We need to mask off tf_cleanup when we're dealing with argument
conversions; it should only apply to the top level call.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 833d39a7919caa021848861e2035c8a1ac3e3b51
Author: Jason Merrill 
Date:   Wed Feb 22 16:55:24 2017 -0800

PR c++/79679 - missing destructor for argument
* call.c (build_over_call): Don't pass tf_no_cleanup to argument
conversions.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 93fae0d..f7924f0 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7838,12 +7838,13 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
   if (flags & LOOKUP_NO_CONVERSION)
conv->user_conv_p = true;
 
+  tsubst_flags_t arg_complain = complain & (~tf_no_cleanup);
+  if (!conversion_warning)
+   arg_complain &= ~tf_warning;
+
   val = convert_like_with_context (conv, arg, fn, i - is_method,
-  conversion_warning
-  ? complain
-  : complain & (~tf_warning));
-
-  val = convert_for_arg_passing (type, val, complain);
+  arg_complain);
+  val = convert_for_arg_passing (type, val, arg_complain);

   if (val == error_mark_node)
 return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/init/cleanup4.C 
b/gcc/testsuite/g++.dg/init/cleanup4.C
new file mode 100644
index 000..b9769e3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/cleanup4.C
@@ -0,0 +1,22 @@
+// PR c++/79679
+// { dg-do run }
+
+int count;
+struct S {
+S() { ++count; }
+S(const S&) { ++count; }
+~S() { --count; }
+};
+
+struct T {
+T(S) {}
+};
+
+int main() {
+  {
+S s;
+T u(s);
+  }
+  if (count)
+__builtin_abort();
+}

Re: [PATCH] PR79584, lra ICE in base_to_reg

[Alan Modra]

On Wed, Feb 22, 2017 at 11:01:15PM +, Richard Sandiford wrote:
> Alan Modra  writes:
> > This patch allows lra to reload a lo_sum address.  Given a mem with a
> > lo_sum address as used by ppc32, decompose_mem_address returns an
> > address_info with *base the lo_sum and *base_term the reg within the
> > lo_sum.  When a lo_sum address isn't valid, we want to first try
> > reloading the entire lo_sum to a new reg.  I first fixed that, then
> > hit another ICE, this time in gen_int_mode.  gen_int_mode was being
> > called with the mode of the memory access, not the mode of the
> > address.
> 
> It seems odd that we have a lo_sum address for a mode that doesn't
> accept lo_sums.  I don't think that was one of the "three" cases
> anticapted in:
> 
>   /* There are three cases where the shape of *AD.INNER may now be invalid:
> 
>  1) the original address was valid, but either elimination or
>  equiv_address_substitution was applied and that made
>  the address invalid.
> 
>  2) the address is an invalid symbolic address created by
>  force_const_to_mem.
> 
>  3) the address is a frame address with an invalid offset.
> 
>  4) the address is a frame address with an invalid base.
> 
>  All these cases involve a non-autoinc address, so there is no
>  point revalidating other types.  */
> 
> It sounds from the PR comments like a valid lo_sum in a mem:SI
> somehow becomes an invalid lo_sum in a mem:SD through equivalence,
> is that right?

Yes, that it correct.

>  If so, was there a (subreg:SD (reg:SI ...)) in the
> pre-RA insn that got "simplifed" to a (mem:SD (lo_sum ...)) during LRA?

Yes.  The insn in question out of ira is this one:

(insn 11 7 10 2 (set (reg:SD 33 1)
(subreg:SD (reg:SI 155 [ i.0_1 ]) 0)) 
"/home/alan/src/gcc.git/gcc/testsuite/c-c++-common/dfp/pr35620.c":20 489 
{movsd_hardfloat}
 (nil))

(reg:SI 155) has the equivalence with
(mem/j/c:SI (lo_sum:SI (reg/f:SI 160) (symbol_ref:SI ("u"

At the time we fail valid_address_p, lra is looking at

(insn 11 17 10 2 (set (reg:SD 33 1)
(mem/j/c:SD (lo_sum:SI (reg/f:SI 160)
(symbol_ref:SI ("u") [flags 0x84] )) 
[2 u.b+0 S4 A32])) 
"/home/alan/src/gcc.git/gcc/testsuite/c-c++-common/dfp/pr35620.c":20 489 
{movsd_hardfloat}


> IMO the bug's in that step if so and if lo_sum isn't valid for mem:SD.

lo_sum is indeed not valid for mem:SD.  simplify_operand_subreg is
where the subreg disappears.

[snip]

> I don't think the code was ever intended to handle anything
> that complicated.  The address has to be "basically" valid,
> except in the cases above.

I'm seriously regretting my foray into ira.c, attempting to modernize
just a small part of the code to use the DF infrastructure..

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

[Jason Merrill]

On Wed, Feb 22, 2017 at 3:44 PM, Martin Sebor  wrote:
> On 02/22/2017 11:02 AM, Jason Merrill wrote:
>>
>> On Tue, Feb 21, 2017 at 4:27 PM, Martin Sebor  wrote:

 Ah, I see, your patch changes attribute unused handling for local
 variables from tracking TREE_USED to lookup_attribute.  I'm not
 opposed to this change, but I'd like to understand why the TREE_USED
 handling wasn't working.
>>>
>>>
>>>
>>> In the test case in the bug:
>>>
>>>   template 
>>>   void g ()
>>>   {
>>> T t;   // warning, ok
>>>
>>> typedef T U;
>>> U u;   // no warning, bug
>>>   }
>>>
>>>   template void g();
>>>
>>> both TREE_USED(T) and TREE_USED(t) are zero in initialize_local_var
>>> so the function doesn't set already_used or TREE_USED(t) and we get
>>> a warning as expected.
>>>
>>> But because TREE_USED(U) is set to 1 in maybe_record_typedef_use
>>> (to implement -Wunused-local-typedefs),  initialize_local_var then
>>> sets already_used to 1 and later also TREE_USED(u) to 1, suppressing
>>> the warning.
>>
>> Hmm, I would expect maybe_record_typedef_use to set TREE_USED on the
>> TYPE_DECL, not on the *_TYPE which initialize_local_var checks.
>
> That's what it does:
>
>   void
>   maybe_record_typedef_use (tree t)
>   {
> if (!is_typedef_decl (t))
>   return;
>
> TREE_USED (t) = true;
>   }
>
> Here, t is a TYPE_DECL of the typedef U.

Yes.  It is a TYPE_DECL, not a type.

> It has the effect of TREE_USED (TREE_TYPE (decl)) being set in
> initialize_local_var.  The TREE_USED bit on the type (i.e., on
> TREE_TYPE(decl) where decl is the u in the test case above) is
> set when the function template is instantiated, in
> set_underlying_type called from tsubst_decl.

Aha!  That seems like the problem.  Does removing that copy of
TREE_USED from the decl to the type break anything?

Jason

Re: [PATCH] rs6000: Fix fsel pattern (PR79211)

[Segher Boessenkool]

On Fri, Jan 27, 2017 at 03:11:46PM +, Segher Boessenkool wrote:
> 2017-01-27  Segher Boessenkool  
> 
>   * config/rs6000/rs6000.md (*fsel4): Use
>   gpc_reg_operand instead of fpr_reg_operand.

I committed the patch.


Segher

Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

[Martin Sebor]

On 02/22/2017 11:02 AM, Jason Merrill wrote:

On Tue, Feb 21, 2017 at 4:27 PM, Martin Sebor  wrote:

Ah, I see, your patch changes attribute unused handling for local
variables from tracking TREE_USED to lookup_attribute.  I'm not
opposed to this change, but I'd like to understand why the TREE_USED
handling wasn't working.



In the test case in the bug:

  template 
  void g ()
  {
T t;   // warning, ok

typedef T U;
U u;   // no warning, bug
  }

  template void g();

both TREE_USED(T) and TREE_USED(t) are zero in initialize_local_var
so the function doesn't set already_used or TREE_USED(t) and we get
a warning as expected.

But because TREE_USED(U) is set to 1 in maybe_record_typedef_use
(to implement -Wunused-local-typedefs),  initialize_local_var then
sets already_used to 1 and later also TREE_USED(u) to 1, suppressing
the warning.


Hmm, I would expect maybe_record_typedef_use to set TREE_USED on the
TYPE_DECL, not on the *_TYPE which initialize_local_var checks.


That's what it does:

  void
  maybe_record_typedef_use (tree t)
  {
if (!is_typedef_decl (t))
  return;

TREE_USED (t) = true;
  }

Here, t is a TYPE_DECL of the typedef U.

It has the effect of TREE_USED (TREE_TYPE (decl)) being set in
initialize_local_var.  The TREE_USED bit on the type (i.e., on
TREE_TYPE(decl) where decl is the u in the test case above) is
set when the function template is instantiated, in
set_underlying_type called from tsubst_decl.

Martin

RE: [PATCH, MIPS] Calling convention differs depending on the presence of MSA

[Matthew Fortune]

Moore, Catherine  writes:
> > As this is an ABI fix, just wait a day or so in case Catherine has
> > any comment otherwise OK to commit.
> >
> I have not  further comments on this patch.  Please commit.

Hi Sameera,

I've been considering this patch further and have realised that my review
was preoccupied with o32 behaviour.  The patch fixes o32 but breaks
all other ABIs because it forces floating point vectors with size up to
2*word_size to be in memory after the change when they would have been
in registers before.

Since you mentioned off list you're having issues committing I've gone
ahead and made the necessary changes and committed for you as below.

Thanks for your work on this.

Matthew

gcc/
* config/mips/mips.c (mips_return_in_memory): Force FP
vector types to be returned in memory for o32 ABI.

gcc/testsuite/

* gcc.target/mips/msa-fp-cc.c: New test.

---

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 7974a16..4e13fbe 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -6472,9 +6472,13 @@ mips_function_value_regno_p (const unsigned int regno)
 static bool
 mips_return_in_memory (const_tree type, const_tree fndecl ATTRIBUTE_UNUSED)
 {
-  return (TARGET_OLDABI
- ? TYPE_MODE (type) == BLKmode
- : !IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD));
+  if (TARGET_OLDABI)
+/* Ensure that any floating point vector types are returned via memory
+   even if they are supported through a vector mode with some ASEs.  */
+return (VECTOR_FLOAT_TYPE_P (type)
+   || TYPE_MODE (type) == BLKmode);
+
+  return (!IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD));
 }
 

 /* Implement TARGET_SETUP_INCOMING_VARARGS.  */
diff --git a/gcc/testsuite/gcc.target/mips/msa-fp-cc.c 
b/gcc/testsuite/gcc.target/mips/msa-fp-cc.c
new file mode 100644
index 000..3d293f3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/msa-fp-cc.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=32 -mfp64 -mhard-float -mmsa" } */
+typedef float v4f32 __attribute__((vector_size(16)));
+typedef double v2f64 __attribute__((vector_size(16)));
+
+v4f32
+fcmpOeqVector4 (v4f32 a, v4f32 b)
+{
+  return a + b;
+}
+
+v2f64
+fcmpOeqVector2 (v2f64 a, v2f64 b)
+{
+  return a + b;
+}
+
+/* { dg-final { scan-assembler-not "copy_s" } } */
+/* { dg-final { scan-assembler-not "insert" } } */
-- 
2.2.1

Re: [PATCH] PR79584, lra ICE in base_to_reg

[Richard Sandiford]

Alan Modra  writes:
> This patch allows lra to reload a lo_sum address.  Given a mem with a
> lo_sum address as used by ppc32, decompose_mem_address returns an
> address_info with *base the lo_sum and *base_term the reg within the
> lo_sum.  When a lo_sum address isn't valid, we want to first try
> reloading the entire lo_sum to a new reg.  I first fixed that, then
> hit another ICE, this time in gen_int_mode.  gen_int_mode was being
> called with the mode of the memory access, not the mode of the
> address.

It seems odd that we have a lo_sum address for a mode that doesn't
accept lo_sums.  I don't think that was one of the "three" cases
anticapted in:

  /* There are three cases where the shape of *AD.INNER may now be invalid:

 1) the original address was valid, but either elimination or
 equiv_address_substitution was applied and that made
 the address invalid.

 2) the address is an invalid symbolic address created by
 force_const_to_mem.

 3) the address is a frame address with an invalid offset.

 4) the address is a frame address with an invalid base.

 All these cases involve a non-autoinc address, so there is no
 point revalidating other types.  */

It sounds from the PR comments like a valid lo_sum in a mem:SI
somehow becomes an invalid lo_sum in a mem:SD through equivalence,
is that right?  If so, was there a (subreg:SD (reg:SI ...)) in the
pre-RA insn that got "simplifed" to a (mem:SD (lo_sum ...)) during LRA?
IMO the bug's in that step if so and if lo_sum isn't valid for mem:SD.

The patch instead seems to be moving towards reloading any address
we end up with (which presumably fits one of the options above
for _some_ mode, just not in this case for the current mode).
But then there's no reason why the remaining assert:

> @@ -2916,18 +2916,18 @@ base_to_reg (struct address_info *ad)
>rtx_insn *insn;
>rtx_insn *last_insn = get_last_insn();
>  
> -  lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
> +  lra_assert (ad->disp == ad->disp_term);

would hold either.  We'd need to make base_to_reg handle every address
recognised by decompose_mem_address, including for instance cases in
which an index could be sign-extended for the original mem mode but not
for the current one.

I don't think the code was ever intended to handle anything
that complicated.  The address has to be "basically" valid,
except in the cases above.

Thanks,
Richard

C++ PATCH for C++17 class deduction from empty initializer

[Jason Merrill]

Someone on the C++ committee observed that

std::less l{};

didn't work in G++; this fixes that.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit d04e2de711e69266b24c118eb330100859b2671c
Author: Jason Merrill 
Date:   Tue Feb 21 16:50:29 2017 -0800

* pt.c (do_class_deduction): Handle 0 argument case.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5b0f62d..17175ba 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25126,6 +25126,14 @@ do_class_deduction (tree ptype, tree tmpl, tree init, 
int flags,
 
   if (cands == NULL_TREE)
 {
+  if (args->length() == 0)
+   {
+ /* Try tmpl<>.  */
+ tree t = lookup_template_class (tmpl, NULL_TREE, NULL_TREE,
+ NULL_TREE, false, tf_none);
+ if (t != error_mark_node)
+   return t;
+   }
   error ("cannot deduce template arguments for %qT, as it has "
 "no deduction guides or user-declared constructors", type);
   return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction30.C 
b/gcc/testsuite/g++.dg/cpp1z/class-deduction30.C
new file mode 100644
index 000..e182803
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction30.C
@@ -0,0 +1,6 @@
+// { dg-options -std=c++1z }
+
+template  struct A { };
+
+A a{};
+

Re: [C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

[Jakub Jelinek]

On Wed, Feb 22, 2017 at 01:58:28PM -0800, Jason Merrill wrote:
> On Wed, Feb 22, 2017 at 1:50 PM, Jakub Jelinek  wrote:
> > This patch does a few things:
> > 2) all but one error in potential_constant_expression_1 to error_at with
> > location_of (t)
> 
> I'd prefer to calculate the location once at the top of the function
> rather than at every call to error_at.  Given that, I think
> EXPR_LOC_OR_LOC is better than location_of, as the latter will give
> the wrong result for a decl.  OK with that change.

Ok, here is what I've committed then:

2017-02-22  Jakub Jelinek  

PR c++/79664
* parser.c (cp_parser_omp_teams, cp_parser_omp_target): Use
SET_EXPR_LOCATION on OMP_TARGET/OMP_TEAMS tree.
* constexpr.c (potential_constant_expression_1): Handle
OMP_*, OACC_* and CILK_* trees.  Use error_at with
EXPR_LOC_OR_LOC (t, input_location) computed early
instead of error, or error_at with location_of (t).

* g++.dg/gomp/teams-1.C: Adjust expected diagnostic location.
* g++.dg/cpp1y/constexpr-throw.C: Likewise.
* g++.dg/gomp/pr79664.C: New test.

--- gcc/cp/parser.c.jj  2017-02-22 22:32:36.659695471 +0100
+++ gcc/cp/parser.c 2017-02-22 23:00:21.472093702 +0100
@@ -35526,6 +35526,7 @@ cp_parser_omp_teams (cp_parser *parser,
  OMP_TEAMS_CLAUSES (ret) = clauses;
  OMP_TEAMS_BODY (ret) = body;
  OMP_TEAMS_COMBINED (ret) = 1;
+ SET_EXPR_LOCATION (ret, loc);
  return add_stmt (ret);
}
 }
@@ -35547,6 +35548,7 @@ cp_parser_omp_teams (cp_parser *parser,
   TREE_TYPE (stmt) = void_type_node;
   OMP_TEAMS_CLAUSES (stmt) = clauses;
   OMP_TEAMS_BODY (stmt) = cp_parser_omp_structured_block (parser, if_p);
+  SET_EXPR_LOCATION (stmt, loc);
 
   return add_stmt (stmt);
 }
@@ -35965,6 +35967,7 @@ cp_parser_omp_target (cp_parser *parser,
  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
  OMP_TARGET_BODY (stmt) = body;
  OMP_TARGET_COMBINED (stmt) = 1;
+ SET_EXPR_LOCATION (stmt, pragma_tok->location);
  add_stmt (stmt);
  pc = _TARGET_CLAUSES (stmt);
  goto check_clauses;
--- gcc/cp/constexpr.c.jj   2017-02-22 22:32:34.490723479 +0100
+++ gcc/cp/constexpr.c  2017-02-22 23:03:12.472864242 +0100
@@ -5001,10 +5001,11 @@ potential_constant_expression_1 (tree t,
 return false;
   if (t == NULL_TREE)
 return true;
+  location_t loc = EXPR_LOC_OR_LOC (t, input_location);
   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
 {
   if (flags & tf_error)
-error ("expression %qE has side-effects", t);
+error_at (loc, "expression %qE has side-effects", t);
   return false;
 }
   if (CONSTANT_CLASS_P (t))
@@ -5086,8 +5087,7 @@ potential_constant_expression_1 (tree t,
  {
/* fold_call_expr can't do anything with IFN calls.  */
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
-   "call to internal function %qE", t);
+ error_at (loc, "call to internal function %qE", t);
return false;
  }
  }
@@ -5105,8 +5105,8 @@ potential_constant_expression_1 (tree t,
  {
if (flags & tf_error)
  {
-   error_at (EXPR_LOC_OR_LOC (t, input_location),
- "call to non-constexpr function %qD", fun);
+   error_at (loc, "call to non-constexpr function %qD",
+ fun);
explain_invalid_constexpr_fn (fun);
  }
return false;
@@ -5199,8 +5199,7 @@ potential_constant_expression_1 (tree t,
&& !integer_zerop (from))
  {
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
-   "reinterpret_cast from integer to pointer");
+ error_at (loc, "reinterpret_cast from integer to pointer");
return false;
  }
 return (RECUR (from, TREE_CODE (t) != VIEW_CONVERT_EXPR));
@@ -5266,7 +5265,7 @@ potential_constant_expression_1 (tree t,
&& !DECL_DECLARED_CONSTEXPR_P (DECL_CONTEXT (x)))
  {
if (flags & tf_error)
- error ("use of % in a constant expression");
+ error_at (loc, "use of % in a constant expression");
return false;
  }
return true;
@@ -5354,10 +5353,40 @@ potential_constant_expression_1 (tree t,
 case DELETE_EXPR:
 case VEC_DELETE_EXPR:
 case THROW_EXPR:
+case OMP_PARALLEL:
+case OMP_TASK:
+case OMP_FOR:
+case OMP_DISTRIBUTE:
+case OMP_TASKLOOP:
+case OMP_TEAMS:
+case OMP_TARGET_DATA:
+case OMP_TARGET:
+case OMP_SECTIONS:
+case OMP_ORDERED:

Re: [C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

[Jason Merrill]

On Wed, Feb 22, 2017 at 1:50 PM, Jakub Jelinek  wrote:
> This patch does a few things:
> 2) all but one error in potential_constant_expression_1 to error_at with
> location_of (t)

I'd prefer to calculate the location once at the top of the function
rather than at every call to error_at.  Given that, I think
EXPR_LOC_OR_LOC is better than location_of, as the latter will give
the wrong result for a decl.  OK with that change.

Jason

[C++ PATCH] Fix ICE with OpenMP/OpenACC/Cilk+ constructs in constexpr functions (PR c++/79664)

[Jakub Jelinek]

Hi!

This patch does a few things:
1) replaces ICEs with diagnostics + return false in
   potential_constant_expression_1 for OpenMP/OpenACC/Cilk+ constructs,
   I believe unless the upcoming versions of those standards say otherwise,
   we should treat the constructs as invalid in constexpr
When writing a testcase for 1), I've noticed very odd locus for all the
error messages, so I've changed
2) all but one error in potential_constant_expression_1 to error_at with
location_of (t)
and finally
3) #pragma omp target/teams in some cases had wrong locus too

While 1) and 3) I think I can commit with my OpenMP maintainer hat on,
I'm seeking approval for 2).

The only spot in potential_constant_expression_1 I've kept error
is for division by zero, which breaks several g++.dg/warn/overflow-warn*.C
tests and I have no idea what's going on in dejagnu there (both patched
and unpatched compilers emit 4 different diagnostics on a single line
there, the difference is just in one of those diagnostics (different
column), but the testcases actually try to match only 3 diagnostics and
the wording of some of them is very similar and one of the expected
diagnostics is pruned somewhere during the dejagnu processing).
After fighting with that for a while I gave up.

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

2017-02-22  Jakub Jelinek  

PR c++/79664
* parser.c (cp_parser_omp_teams, cp_parser_omp_target): Use
SET_EXPR_LOCATION on OMP_TARGET/OMP_TEAMS tree.
* constexpr.c (potential_constant_expression_1): Handle
OMP_*, OACC_* and CILK_* trees.  Use error_at with location_of (t)
instead of error, use location_of (t) instead of
EXPR_LOC_OR_LOC (t, input_location).

* g++.dg/gomp/teams-1.C: Adjust expected diagnostic location.
* g++.dg/cpp1y/constexpr-throw.C: Likewise.
* g++.dg/gomp/pr79664.C: New test.

--- gcc/cp/parser.c.jj  2017-02-20 13:43:21.0 +0100
+++ gcc/cp/parser.c 2017-02-22 17:17:39.176918357 +0100
@@ -35519,6 +35519,7 @@ cp_parser_omp_teams (cp_parser *parser,
  OMP_TEAMS_CLAUSES (ret) = clauses;
  OMP_TEAMS_BODY (ret) = body;
  OMP_TEAMS_COMBINED (ret) = 1;
+ SET_EXPR_LOCATION (ret, loc);
  return add_stmt (ret);
}
 }
@@ -35540,6 +35541,7 @@ cp_parser_omp_teams (cp_parser *parser,
   TREE_TYPE (stmt) = void_type_node;
   OMP_TEAMS_CLAUSES (stmt) = clauses;
   OMP_TEAMS_BODY (stmt) = cp_parser_omp_structured_block (parser, if_p);
+  SET_EXPR_LOCATION (stmt, loc);
 
   return add_stmt (stmt);
 }
@@ -35958,6 +35960,7 @@ cp_parser_omp_target (cp_parser *parser,
  OMP_TARGET_CLAUSES (stmt) = cclauses[C_OMP_CLAUSE_SPLIT_TARGET];
  OMP_TARGET_BODY (stmt) = body;
  OMP_TARGET_COMBINED (stmt) = 1;
+ SET_EXPR_LOCATION (stmt, pragma_tok->location);
  add_stmt (stmt);
  pc = _TARGET_CLAUSES (stmt);
  goto check_clauses;
--- gcc/cp/constexpr.c.jj   2017-02-21 18:56:43.0 +0100
+++ gcc/cp/constexpr.c  2017-02-22 17:16:53.413507546 +0100
@@ -5004,7 +5004,8 @@ potential_constant_expression_1 (tree t,
   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
 {
   if (flags & tf_error)
-error ("expression %qE has side-effects", t);
+error_at (location_of (t),
+ "expression %qE has side-effects", t);
   return false;
 }
   if (CONSTANT_CLASS_P (t))
@@ -5086,7 +5087,7 @@ potential_constant_expression_1 (tree t,
  {
/* fold_call_expr can't do anything with IFN calls.  */
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
+ error_at (location_of (t),
"call to internal function %qE", t);
return false;
  }
@@ -5105,7 +5106,7 @@ potential_constant_expression_1 (tree t,
  {
if (flags & tf_error)
  {
-   error_at (EXPR_LOC_OR_LOC (t, input_location),
+   error_at (location_of (t),
  "call to non-constexpr function %qD", fun);
explain_invalid_constexpr_fn (fun);
  }
@@ -5199,7 +5200,7 @@ potential_constant_expression_1 (tree t,
&& !integer_zerop (from))
  {
if (flags & tf_error)
- error_at (EXPR_LOC_OR_LOC (t, input_location),
+ error_at (location_of (t),
"reinterpret_cast from integer to pointer");
return false;
  }
@@ -5266,7 +5267,8 @@ potential_constant_expression_1 (tree t,
&& !DECL_DECLARED_CONSTEXPR_P (DECL_CONTEXT (x)))
  {
if (flags & tf_error)
- error ("use of % in a constant expression");
+ error_at (location_of (t),
+   

[PATCH] Optimize divmod expansion (PR middle-end/79665)

[Jakub Jelinek]

Hi!

If both arguments of integer division or modulo are known to be non-negative
in corresponding signed type, then signed as well as unsigned division/modulo
shall have the exact same result and therefore we can choose between those
two depending on which one is faster (or shorter for -Os), which varries
a lot depending on target and especially for constant divisors on the exact
divisor.  expand_divmod itself is too complicated and we don't even have
the ability to ask about costs e.g. for highpart multiplication without
actually expanding it, so this patch just in that case tries both sequences,
computes their costs and uses the cheaper (and for equal cost honors the
actual original signedness of the operation).

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

2017-02-22  Jakub Jelinek  

PR middle-end/79665
* internal-fn.c (get_range_pos_neg): Moved to ...
* tree.c (get_range_pos_neg): ... here.  No longer static.
* tree.h (get_range_pos_neg): New prototype.
* expr.c (expand_expr_real_2) : If both arguments
are known to be in between 0 and signed maximum inclusive, try to
expand both unsigned and signed divmod and use the cheaper one from
those.

--- gcc/internal-fn.c.jj2017-02-11 09:14:56.0 +0100
+++ gcc/internal-fn.c   2017-02-22 10:09:03.503254909 +0100
@@ -413,86 +413,6 @@ expand_FALLTHROUGH (internal_fn, gcall *
"invalid use of attribute %");
 }
 
-/* Helper function for expand_addsub_overflow.  Return 1
-   if ARG interpreted as signed in its precision is known to be always
-   positive or 2 if ARG is known to be always negative, or 3 if ARG may
-   be positive or negative.  */
-
-static int
-get_range_pos_neg (tree arg)
-{
-  if (arg == error_mark_node)
-return 3;
-
-  int prec = TYPE_PRECISION (TREE_TYPE (arg));
-  int cnt = 0;
-  if (TREE_CODE (arg) == INTEGER_CST)
-{
-  wide_int w = wi::sext (arg, prec);
-  if (wi::neg_p (w))
-   return 2;
-  else
-   return 1;
-}
-  while (CONVERT_EXPR_P (arg)
-&& INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (arg, 0)))
-&& TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (arg, 0))) <= prec)
-{
-  arg = TREE_OPERAND (arg, 0);
-  /* Narrower value zero extended into wider type
-will always result in positive values.  */
-  if (TYPE_UNSIGNED (TREE_TYPE (arg))
- && TYPE_PRECISION (TREE_TYPE (arg)) < prec)
-   return 1;
-  prec = TYPE_PRECISION (TREE_TYPE (arg));
-  if (++cnt > 30)
-   return 3;
-}
-
-  if (TREE_CODE (arg) != SSA_NAME)
-return 3;
-  wide_int arg_min, arg_max;
-  while (get_range_info (arg, _min, _max) != VR_RANGE)
-{
-  gimple *g = SSA_NAME_DEF_STMT (arg);
-  if (is_gimple_assign (g)
- && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (g)))
-   {
- tree t = gimple_assign_rhs1 (g);
- if (INTEGRAL_TYPE_P (TREE_TYPE (t))
- && TYPE_PRECISION (TREE_TYPE (t)) <= prec)
-   {
- if (TYPE_UNSIGNED (TREE_TYPE (t))
- && TYPE_PRECISION (TREE_TYPE (t)) < prec)
-   return 1;
- prec = TYPE_PRECISION (TREE_TYPE (t));
- arg = t;
- if (++cnt > 30)
-   return 3;
- continue;
-   }
-   }
-  return 3;
-}
-  if (TYPE_UNSIGNED (TREE_TYPE (arg)))
-{
-  /* For unsigned values, the "positive" range comes
-below the "negative" range.  */
-  if (!wi::neg_p (wi::sext (arg_max, prec), SIGNED))
-   return 1;
-  if (wi::neg_p (wi::sext (arg_min, prec), SIGNED))
-   return 2;
-}
-  else
-{
-  if (!wi::neg_p (wi::sext (arg_min, prec), SIGNED))
-   return 1;
-  if (wi::neg_p (wi::sext (arg_max, prec), SIGNED))
-   return 2;
-}
-  return 3;
-}
-
 /* Return minimum precision needed to represent all values
of ARG in SIGNed integral type.  */
 
--- gcc/tree.c.jj   2017-02-09 14:55:34.0 +0100
+++ gcc/tree.c  2017-02-22 10:10:36.525062066 +0100
@@ -14205,6 +14205,88 @@ verify_type (const_tree t)
 }
 
 
+/* Return 1 if ARG interpreted as signed in its precision is known to be
+   always positive or 2 if ARG is known to be always negative, or 3 if
+   ARG may be positive or negative.  */
+
+int
+get_range_pos_neg (tree arg)
+{
+  if (arg == error_mark_node)
+return 3;
+
+  int prec = TYPE_PRECISION (TREE_TYPE (arg));
+  int cnt = 0;
+  if (TREE_CODE (arg) == INTEGER_CST)
+{
+  wide_int w = wi::sext (arg, prec);
+  if (wi::neg_p (w))
+   return 2;
+  else
+   return 1;
+}
+  while (CONVERT_EXPR_P (arg)
+&& INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (arg, 0)))
+&& TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (arg, 0))) <= prec)
+{
+  arg = TREE_OPERAND (arg, 0);
+  /* Narrower value zero extended into wider type
+will always result in positive values. 

Re: C PATCH to fix ICE with -Wtraditional-conversion (PR c/79662)

[Joseph Myers]

On Wed, 22 Feb 2017, Marek Polacek wrote:

> We ICEd on the following test while warning for -Wtraditional-conversion
> because at that point VAL had been turned into error_mark_node because we
> require a complete type when converting arguments.  Fixed by checking
> for error_mark_node first so that we don't try to access it later with
> TYPE_PRECISION and similar.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

OK.

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

[PATCH] PR79584, lra ICE in base_to_reg

[Alan Modra]

This patch allows lra to reload a lo_sum address.  Given a mem with a
lo_sum address as used by ppc32, decompose_mem_address returns an
address_info with *base the lo_sum and *base_term the reg within the
lo_sum.  When a lo_sum address isn't valid, we want to first try
reloading the entire lo_sum to a new reg.  I first fixed that, then
hit another ICE, this time in gen_int_mode.  gen_int_mode was being
called with the mode of the memory access, not the mode of the
address.

Bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux bi-arch.  OK to apply?

PR rtl-optimization/79584
* lra-constraints.c (base_to_reg): Reload ad->base, the entire
base, not ad->base_term, the reg within base.  Remove assertion
that ad->base == ad->base_term.  Replace gen_int_mode using
bogus mode with const0_rtx.

diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index bd5fbcd..0e98f22 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -2916,18 +2916,18 @@ base_to_reg (struct address_info *ad)
   rtx_insn *insn;
   rtx_insn *last_insn = get_last_insn();
 
-  lra_assert (ad->base == ad->base_term && ad->disp == ad->disp_term);
+  lra_assert (ad->disp == ad->disp_term);
   cl = base_reg_class (ad->mode, ad->as, ad->base_outer_code,
get_index_code (ad));
-  new_reg = lra_create_new_reg (GET_MODE (*ad->base_term), NULL_RTX,
+  new_reg = lra_create_new_reg (GET_MODE (*ad->base), NULL_RTX,
 cl, "base");
   new_inner = simplify_gen_binary (PLUS, GET_MODE (new_reg), new_reg,
ad->disp_term == NULL
-   ? gen_int_mode (0, ad->mode)
+   ? const0_rtx
: *ad->disp_term);
   if (!valid_address_p (ad->mode, new_inner, ad->as))
 return NULL_RTX;
-  insn = emit_insn (gen_rtx_SET (new_reg, *ad->base_term));
+  insn = emit_insn (gen_rtx_SET (new_reg, *ad->base));
   code = recog_memoized (insn);
   if (code < 0)
 {

-- 
Alan Modra
Australia Development Lab, IBM

RE: [PATCH 3/5] Support WORD_REGISTER_OPERATIONS requirements in simplify_operand_subreg

[Matthew Fortune]

Eric Botcazou  writes:
> > The condition would look like this, What do you think?
> >
> >   if (!(GET_MODE_PRECISION (mode) != GET_MODE_PRECISION (innermode)
> > && GET_MODE_SIZE (mode) <= UNITS_PER_WORD
> > && GET_MODE_SIZE (innermode) <= UNITS_PER_WORD
> > && WORD_REGISTER_OPERATIONS)
> >   && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
> > && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
> >
> >   || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
> >
> >   && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN
> > (reg) return true;
> 
> No regressions on SPARC.

I have committed this change now. The final patch is below. I will respond
promptly if there is some further unexpected fallout from this. I built the
arm-none-eabi target successfully as an additional check and bootstrapped
mips64el-linux-gnu as well.  I'll have to rely on the various auto-builders
people have to cover all the testsuite variations as I'm not set up to cover
them all myself.

Thanks,
Matthew

gcc/
PR target/78660
* lra-constraints.c (simplify_operand_subreg): Handle
WORD_REGISTER_OPERATIONS targets.


diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index 35539a9..224a956 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -1541,11 +1541,22 @@ simplify_operand_subreg (int nop, machine_mode reg_mode)
 subregs as we don't substitute such equiv memory (see processing
 equivalences in function lra_constraints) and because for spilled
 pseudos we allocate stack memory enough for the biggest
-corresponding paradoxical subreg.  */
- if (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
-   && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
- || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
- && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg
+corresponding paradoxical subreg.
+
+However, do not blindly simplify a (subreg (mem ...)) for
+WORD_REGISTER_OPERATIONS targets as this may lead to loading junk
+data into a register when the inner is narrower than outer or
+missing important data from memory when the inner is wider than
+outer.  This rule only applies to modes that are no wider than
+a word.  */
+ if (!(GET_MODE_PRECISION (mode) != GET_MODE_PRECISION (innermode)
+   && GET_MODE_SIZE (mode) <= UNITS_PER_WORD
+   && GET_MODE_SIZE (innermode) <= UNITS_PER_WORD
+   && WORD_REGISTER_OPERATIONS)
+ && (!(MEM_ALIGN (subst) < GET_MODE_ALIGNMENT (mode)
+   && SLOW_UNALIGNED_ACCESS (mode, MEM_ALIGN (subst)))
+ || (MEM_ALIGN (reg) < GET_MODE_ALIGNMENT (innermode)
+ && SLOW_UNALIGNED_ACCESS (innermode, MEM_ALIGN (reg)
return true;
 
  *curr_id->operand_loc[nop] = operand;
-- 
2.2.1

Re: [C++ PATCH] For -gdwarf-5 emit DW_TAG_variable instead of DW_TAG_member for C++ static data members (take 2)

[Jason Merrill]

OK.

On Wed, Feb 22, 2017 at 8:57 AM, Jakub Jelinek  wrote:
> On Tue, Feb 21, 2017 at 12:37:32PM -0800, Jason Merrill wrote:
>> On Sat, Feb 18, 2017 at 12:17 AM, Jakub Jelinek  wrote:
>> > On Fri, Feb 17, 2017 at 09:37:09PM -0500, Jason Merrill wrote:
>> >> On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek  wrote:
>> >> > -  && die->die_tag != DW_TAG_member)
>> >> > +  && die->die_tag != DW_TAG_member
>> >> > +  && (die->die_tag != DW_TAG_variable || !class_scope_p 
>> >> > (die->die_parent)))
>> >>
>> >> How about we only check class_scope_p (die->die_parent), and don't
>> >> consider the TAG at all?  DW_TAG_member should only appear at class
>> >> scope.
>> >
>> > That wouldn't work, because that would avoid adding linkage attributes to
>> > methods.  I could use:
>> >   && (die->die_tag == DW_TAG_subprogram || !class_scope_p 
>> > (die->die_parent))
>> > (not 100% sure if some other tag than those can make it here)
>> > or
>> >   && ((die->die_tag != DW_TAG_member || die->die_tag != 
>> > DW_TAG_variable)
>> >   || !class_scope_p (die->die_parent))
>> > or just use
>> >   && (!VAR_P (decl) || !class_scope_p (die->die_parent))
>> > or similar.
>> >
>> >> > - if (old_die->die_tag == DW_TAG_member)
>> >> > + if (old_die->die_tag == DW_TAG_member
>> >> > + || (dwarf_version >= 5 && class_scope_p 
>> >> > (old_die->die_parent)))
>> >>
>> >> Likewise here.
>> >
>> > This spot probably can be changed as you wrote, it is in gen_variable_die,
>> > so methods shouldn't appear there.
>>
>> Hmm, let's think about the behavior we want here.  I don't see any
>> reason not to put AT_linkage_name on a member DW_TAG_variable; I think
>> the old behavior avoided putting it on DW_TAG_member just because it
>> isn't defined for DW_TAG_member.  So it's not clear to me that we need
>> any changes in add_linkage_name or its call site.
>
> Ah, ok.  But then we need to make sure gen_variable_die doesn't set
> no_linkage_name for -gdwarf-5, because otherwise on the attached testcase
> we don't emit linkage_name on b, c, d anywhere (we used to emit that
> before).
>
> Starting bootstrap/regtest now, does this look ok if that passes?
>
> 2017-02-22  Jakub Jelinek  
>
> * dwarf2out.c (gen_variable_die): For -gdwarf-5, use DW_TAG_variable
> instead of DW_TAG_member for static data member declarations and don't
> set no_linkage_name for static inline data members.
> (gen_member_die): For -gdwarf-5 don't change DW_TAG_variable
> to DW_TAG_member.
>
> * g++.dg/debug/dwarf2/inline-var-2.C: New test.
>
> --- gcc/dwarf2out.c.jj  2017-02-18 17:11:18.759821871 +0100
> +++ gcc/dwarf2out.c 2017-02-22 17:48:27.378223877 +0100
> @@ -22669,7 +22669,8 @@ gen_variable_die (tree decl, tree origin
>&& lang_hooks.decls.decl_dwarf_attribute (decl, DW_AT_inline) != -1)
>  {
>declaration = true;
> -  no_linkage_name = true;
> +  if (dwarf_version < 5)
> +   no_linkage_name = true;
>  }
>
>ultimate_origin = decl_ultimate_origin (decl_or_origin);
> @@ -22820,9 +22821,10 @@ gen_variable_die (tree decl, tree origin
>  }
>
>/* For static data members, the declaration in the class is supposed
> - to have DW_TAG_member tag; the specification should still be
> - DW_TAG_variable referencing the DW_TAG_member DIE.  */
> -  if (declaration && class_scope_p (context_die))
> + to have DW_TAG_member tag in DWARF{3,4} and we emit it for compatibility
> + also in DWARF2; the specification should still be DW_TAG_variable
> + referencing the DW_TAG_member DIE.  */
> +  if (declaration && class_scope_p (context_die) && dwarf_version < 5)
>  var_die = new_die (DW_TAG_member, context_die, decl);
>else
>  var_die = new_die (DW_TAG_variable, context_die, decl);
> @@ -24091,7 +24093,8 @@ gen_member_die (tree type, dw_die_ref co
>   && get_AT (child, DW_AT_specification) == NULL)
> {
>   reparent_child (child, context_die);
> - child->die_tag = DW_TAG_member;
> + if (dwarf_version < 5)
> +   child->die_tag = DW_TAG_member;
> }
>   else
> splice_child_die (context_die, child);
> @@ -24113,7 +24116,7 @@ gen_member_die (tree type, dw_die_ref co
> }
>
>/* For C++ inline static data members emit immediately a 
> DW_TAG_variable
> -DIE that will refer to that DW_TAG_member through
> +DIE that will refer to that DW_TAG_member/DW_TAG_variable through
>  DW_AT_specification.  */
>if (TREE_STATIC (member)
>   && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline)
> --- gcc/testsuite/g++.dg/debug/dwarf2/inline-var-2.C.jj 2017-02-22 
> 17:54:28.019597440 +0100
> +++ gcc/testsuite/g++.dg/debug/dwarf2/inline-var-2.C2017-02-22 
> 17:54:10.0 +0100
> @@ -0,0 

Re: C++ PATCH to fix ICE with __underlying_type and incomplete enum (PR c++/79657)

[Jason Merrill]

Hmm, I suppose we don't need to allow this in SFINAE context.  OK.

On Wed, Feb 22, 2017 at 10:24 AM, Marek Polacek  wrote:
> The problem here is that we aren't properly checking what's been passed to
> __underlying_type.  It needs an enum, which the code checks, but that enum
> mustn't be incomplete, otherwise we crash because ENUM_UNDERLYING_TYPE will
> be null until the definition of the enum is completed by finish_enum.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-02-22  Marek Polacek  
>
> PR c++/79657
> * semantics.c (finish_underlying_type): Bail out for incomplete enums.
>
> * g++.dg/ext/underlying_type12.C: New test.
>
> diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> index 6ba7c13..67f8b92 100644
> --- gcc/cp/semantics.c
> +++ gcc/cp/semantics.c
> @@ -3838,7 +3838,8 @@ finish_underlying_type (tree type)
>return underlying_type;
>  }
>
> -  complete_type (type);
> +  if (!complete_type_or_else (type, NULL_TREE))
> +return error_mark_node;
>
>if (TREE_CODE (type) != ENUMERAL_TYPE)
>  {
> diff --git gcc/testsuite/g++.dg/ext/underlying_type12.C 
> gcc/testsuite/g++.dg/ext/underlying_type12.C
> index e69de29..050ecf2 100644
> --- gcc/testsuite/g++.dg/ext/underlying_type12.C
> +++ gcc/testsuite/g++.dg/ext/underlying_type12.C
> @@ -0,0 +1,6 @@
> +// PR c++/79657
> +// { dg-do compile { target c++11 } }
> +
> +enum A { x };
> +enum B { a = (__underlying_type (A)) 1 };
> +enum C { b = (__underlying_type (C)) 1 }; // { dg-error "incomplete" }
>
> Marek

Re: C++ PATCH to fix ICEs with alignas and template parameter pack (PR c++/79653)

[Jason Merrill]

OK.

On Wed, Feb 22, 2017 at 10:53 AM, Marek Polacek  wrote:
> This patch oughta fix a couple of ice-on-invalids.
>
> The parser.c part: I think it makes no sense for cp_parser_std_attribute_spec
> to create an attribute if something went wrong, e.g. in make_pack_expansion, 
> so
> return error_mark_node to signal an issue.
>
> The pt.c part: tsubst_pack_expansion can return error_mark_node to indicate
> that it couldn't handle something, in which case we cannot use TREE_VEC_LENGTH
> on the result.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-02-22  Marek Polacek  
>
> PR c++/79653
> * parser.c (cp_parser_std_attribute_spec): Don't build the attribute
> if the alignas expression is erroneous.
> * pt.c (tsubst_attribute): If tsubst_pack_expansion fails, return
> error_mark_node.
>
> * g++.dg/cpp0x/alignas10.C: New test.
> * g++.dg/cpp0x/alignas9.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 3992516..e20257c 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -24977,12 +24977,17 @@ cp_parser_std_attribute_spec (cp_parser *parser)
>alignas_expr = cxx_alignas_expr (alignas_expr);
>alignas_expr = build_tree_list (NULL_TREE, alignas_expr);
>
> +  /* Handle alignas (pack...).  */
>if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
> {
>   cp_lexer_consume_token (parser->lexer);
>   alignas_expr = make_pack_expansion (alignas_expr);
> }
>
> +  /* Something went wrong, so don't build the attribute.  */
> +  if (alignas_expr == error_mark_node)
> +   return error_mark_node;
> +
>if (cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN) == 
> NULL)
> {
>   cp_parser_error (parser, "expected %<)%>");
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index 38a01e1..5b0f62d 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -10012,6 +10012,8 @@ tsubst_attribute (tree t, tree *decl_p, tree args,
>/* An attribute pack expansion.  */
>tree purp = TREE_PURPOSE (t);
>tree pack = tsubst_pack_expansion (val, args, complain, in_decl);
> +  if (pack == error_mark_node)
> +   return error_mark_node;
>int len = TREE_VEC_LENGTH (pack);
>tree list = NULL_TREE;
>tree *q = 
> diff --git gcc/testsuite/g++.dg/cpp0x/alignas10.C 
> gcc/testsuite/g++.dg/cpp0x/alignas10.C
> index e69de29..164994e 100644
> --- gcc/testsuite/g++.dg/cpp0x/alignas10.C
> +++ gcc/testsuite/g++.dg/cpp0x/alignas10.C
> @@ -0,0 +1,7 @@
> +// PR c++/79653
> +// { dg-do compile { target c++11 } }
> +
> +template  struct outer {
> +  template  struct alignas(alignof(A) * alignof(B)...) inner 
> {}; // { dg-error "mismatched argument pack lengths" }
> +};
> +outer::inner<> mismatched_packs;
> diff --git gcc/testsuite/g++.dg/cpp0x/alignas9.C 
> gcc/testsuite/g++.dg/cpp0x/alignas9.C
> index e69de29..98fe707 100644
> --- gcc/testsuite/g++.dg/cpp0x/alignas9.C
> +++ gcc/testsuite/g++.dg/cpp0x/alignas9.C
> @@ -0,0 +1,6 @@
> +// PR c++/79653
> +// { dg-do compile { target c++11 } }
> +
> +template 
> +struct A { alignas(int...) char c; }; // { dg-error "no argument 
> packs|expected" }
> +A a;
>
> Marek

C++ PATCH to fix ICEs with alignas and template parameter pack (PR c++/79653)

[Marek Polacek]

This patch oughta fix a couple of ice-on-invalids.

The parser.c part: I think it makes no sense for cp_parser_std_attribute_spec
to create an attribute if something went wrong, e.g. in make_pack_expansion, so
return error_mark_node to signal an issue.

The pt.c part: tsubst_pack_expansion can return error_mark_node to indicate
that it couldn't handle something, in which case we cannot use TREE_VEC_LENGTH
on the result.

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

2017-02-22  Marek Polacek  

PR c++/79653
* parser.c (cp_parser_std_attribute_spec): Don't build the attribute
if the alignas expression is erroneous.
* pt.c (tsubst_attribute): If tsubst_pack_expansion fails, return
error_mark_node.

* g++.dg/cpp0x/alignas10.C: New test.
* g++.dg/cpp0x/alignas9.C: New test.

diff --git gcc/cp/parser.c gcc/cp/parser.c
index 3992516..e20257c 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -24977,12 +24977,17 @@ cp_parser_std_attribute_spec (cp_parser *parser)
   alignas_expr = cxx_alignas_expr (alignas_expr);
   alignas_expr = build_tree_list (NULL_TREE, alignas_expr);
 
+  /* Handle alignas (pack...).  */
   if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
{
  cp_lexer_consume_token (parser->lexer);
  alignas_expr = make_pack_expansion (alignas_expr);
}
 
+  /* Something went wrong, so don't build the attribute.  */
+  if (alignas_expr == error_mark_node)
+   return error_mark_node;
+
   if (cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN) == NULL)
{
  cp_parser_error (parser, "expected %<)%>");
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 38a01e1..5b0f62d 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -10012,6 +10012,8 @@ tsubst_attribute (tree t, tree *decl_p, tree args,
   /* An attribute pack expansion.  */
   tree purp = TREE_PURPOSE (t);
   tree pack = tsubst_pack_expansion (val, args, complain, in_decl);
+  if (pack == error_mark_node)
+   return error_mark_node;
   int len = TREE_VEC_LENGTH (pack);
   tree list = NULL_TREE;
   tree *q = 
diff --git gcc/testsuite/g++.dg/cpp0x/alignas10.C 
gcc/testsuite/g++.dg/cpp0x/alignas10.C
index e69de29..164994e 100644
--- gcc/testsuite/g++.dg/cpp0x/alignas10.C
+++ gcc/testsuite/g++.dg/cpp0x/alignas10.C
@@ -0,0 +1,7 @@
+// PR c++/79653
+// { dg-do compile { target c++11 } }
+
+template  struct outer {
+  template  struct alignas(alignof(A) * alignof(B)...) inner 
{}; // { dg-error "mismatched argument pack lengths" }
+};
+outer::inner<> mismatched_packs;
diff --git gcc/testsuite/g++.dg/cpp0x/alignas9.C 
gcc/testsuite/g++.dg/cpp0x/alignas9.C
index e69de29..98fe707 100644
--- gcc/testsuite/g++.dg/cpp0x/alignas9.C
+++ gcc/testsuite/g++.dg/cpp0x/alignas9.C
@@ -0,0 +1,6 @@
+// PR c++/79653
+// { dg-do compile { target c++11 } }
+
+template 
+struct A { alignas(int...) char c; }; // { dg-error "no argument 
packs|expected" }
+A a;

Marek

C PATCH to fix ICE with -Wtraditional-conversion (PR c/79662)

[Marek Polacek]

We ICEd on the following test while warning for -Wtraditional-conversion
because at that point VAL had been turned into error_mark_node because we
require a complete type when converting arguments.  Fixed by checking
for error_mark_node first so that we don't try to access it later with
TYPE_PRECISION and similar.

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

2017-02-22  Marek Polacek  

PR c/79662
* c-typeck.c (convert_arguments): Handle error_mark_node.

* gcc.dg/enum-incomplete-4.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index ed8ffe4..8c2c561 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3437,15 +3437,18 @@ convert_arguments (location_t loc, vec 
arg_loc, tree typelist,
  /* 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)
+ 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 (TREE_CODE (type) == ENUMERAL_TYPE
- && (TYPE_MAIN_VARIANT (type)
- == TYPE_MAIN_VARIANT (valtype)))
+ 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.  */
;
diff --git gcc/testsuite/gcc.dg/enum-incomplete-4.c 
gcc/testsuite/gcc.dg/enum-incomplete-4.c
index e69de29..03fb9f4 100644
--- gcc/testsuite/gcc.dg/enum-incomplete-4.c
+++ gcc/testsuite/gcc.dg/enum-incomplete-4.c
@@ -0,0 +1,11 @@
+/* PR c/79662 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+extern enum e ve;
+
+int
+f0 (int i)
+{
+  f0 (ve); /* { dg-error "incomplete" } */
+}

Marek

C++ PATCH to fix ICE with __underlying_type and incomplete enum (PR c++/79657)

[Marek Polacek]

The problem here is that we aren't properly checking what's been passed to
__underlying_type.  It needs an enum, which the code checks, but that enum
mustn't be incomplete, otherwise we crash because ENUM_UNDERLYING_TYPE will
be null until the definition of the enum is completed by finish_enum.

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

2017-02-22  Marek Polacek  

PR c++/79657
* semantics.c (finish_underlying_type): Bail out for incomplete enums.

* g++.dg/ext/underlying_type12.C: New test.

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index 6ba7c13..67f8b92 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -3838,7 +3838,8 @@ finish_underlying_type (tree type)
   return underlying_type;
 }
 
-  complete_type (type);
+  if (!complete_type_or_else (type, NULL_TREE))
+return error_mark_node;
 
   if (TREE_CODE (type) != ENUMERAL_TYPE)
 {
diff --git gcc/testsuite/g++.dg/ext/underlying_type12.C 
gcc/testsuite/g++.dg/ext/underlying_type12.C
index e69de29..050ecf2 100644
--- gcc/testsuite/g++.dg/ext/underlying_type12.C
+++ gcc/testsuite/g++.dg/ext/underlying_type12.C
@@ -0,0 +1,6 @@
+// PR c++/79657
+// { dg-do compile { target c++11 } }
+
+enum A { x };
+enum B { a = (__underlying_type (A)) 1 };
+enum C { b = (__underlying_type (C)) 1 }; // { dg-error "incomplete" }

Marek

Re: [PATCH] Use -Wnornalized=[options] instead of -Wnormalized=

[Jeff Law]

On 02/21/2017 03:44 AM, Martin Liška wrote:

Hello.

To make documentation and output of --help consistent, I suggest to replace 
inequality
signs with square brackets.

Ready to be installed?

OK.
jeff

Re: [PATCH] restore -Wunused-variable on a typedef'd variable in a function template (PR 79548)

[Jason Merrill]

On Tue, Feb 21, 2017 at 4:27 PM, Martin Sebor  wrote:
>> Ah, I see, your patch changes attribute unused handling for local
>> variables from tracking TREE_USED to lookup_attribute.  I'm not
>> opposed to this change, but I'd like to understand why the TREE_USED
>> handling wasn't working.
>
>
> In the test case in the bug:
>
>   template 
>   void g ()
>   {
> T t;   // warning, ok
>
> typedef T U;
> U u;   // no warning, bug
>   }
>
>   template void g();
>
> both TREE_USED(T) and TREE_USED(t) are zero in initialize_local_var
> so the function doesn't set already_used or TREE_USED(t) and we get
> a warning as expected.
>
> But because TREE_USED(U) is set to 1 in maybe_record_typedef_use
> (to implement -Wunused-local-typedefs),  initialize_local_var then
> sets already_used to 1 and later also TREE_USED(u) to 1, suppressing
> the warning.

Hmm, I would expect maybe_record_typedef_use to set TREE_USED on the
TYPE_DECL, not on the *_TYPE which initialize_local_var checks.

Jason

Re: [PATCH] Fix PR68644

[Bill Schmidt]

Thanks, Jeff -- I caught it as soon as I hit the stupid button... :)

> On Feb 22, 2017, at 11:56 AM, Jeff Law  wrote:
> 
> On 02/22/2017 10:54 AM, Bill Schmidt wrote:
>> Hi,
>> 
>> As discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68644,
>> the test case gcc.dg/tree-ssa/ivopts-lt-2.c does not apply to POWER
>> targets, as the cost model properly shows the pre-increment approach
>> to be preferable and results in ideal code generation.  Thus, adding
>> powerpc*-*-* to the list of targets to skip for this test.
>> 
>> Verified on powerpc64le-unknown-linux-gnu.  Is this ok for trunk?
> ENOPATCH, but it's obvious what the patch should be, so OK.
> 
> jeff
> 
> 

Re: [PATCH] Fix PR68644

[Jeff Law]

On 02/22/2017 10:54 AM, Bill Schmidt wrote:

Hi,

As discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68644,
the test case gcc.dg/tree-ssa/ivopts-lt-2.c does not apply to POWER
targets, as the cost model properly shows the pre-increment approach
to be preferable and results in ideal code generation.  Thus, adding
powerpc*-*-* to the list of targets to skip for this test.

Verified on powerpc64le-unknown-linux-gnu.  Is this ok for trunk?

ENOPATCH, but it's obvious what the patch should be, so OK.

jeff

Fwd: [PATCH] Fix PR68644

[Bill Schmidt]

ENOPATCH.  Trying again...

Hi,

As discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68644,
the test case gcc.dg/tree-ssa/ivopts-lt-2.c does not apply to POWER
targets, as the cost model properly shows the pre-increment approach
to be preferable and results in ideal code generation.  Thus, adding
powerpc*-*-* to the list of targets to skip for this test.

Verified on powerpc64le-unknown-linux-gnu.  Is this ok for trunk?

Thanks,
Bill


2017-02-22  Bill Schmidt  

PR tree-optimization/68644
* gcc.dg/tree-ssa/ivopts-lt-2.c: Skip for powerpc*-*-*.


Index: gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt-2.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt-2.c (revision 245653)
+++ gcc/testsuite/gcc.dg/tree-ssa/ivopts-lt-2.c (working copy)
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-options "-O2 -fdump-tree-ivopts" } */
-/* { dg-skip-if "PR68644" { hppa*-*-* } { "*" } { "" } } */
+/* { dg-skip-if "PR68644" { hppa*-*-* powerpc*-*-* } { "*" } { "" } } */
 
 void
 f1 (int *p, unsigned int i)

[PATCH] Fix PR68644

[Bill Schmidt]

Hi,

As discussed in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68644,
the test case gcc.dg/tree-ssa/ivopts-lt-2.c does not apply to POWER
targets, as the cost model properly shows the pre-increment approach
to be preferable and results in ideal code generation.  Thus, adding
powerpc*-*-* to the list of targets to skip for this test.

Verified on powerpc64le-unknown-linux-gnu.  Is this ok for trunk?

Thanks,
Bill

Re: fwprop fix for PR79405

[Jeff Law]

On 02/16/2017 12:41 PM, Bernd Schmidt wrote:

We have two registers being assigned to each other:

 (set (reg 213) (reg 209))
 (set (reg 209) (reg 213))

These being the only definitions, we are happy to forward propagate reg
209 for reg 213 into a third insn, making a new use for reg 209. We are
then happy to forward propagate reg 213 for it in the same insn...
ending up in an infinite loop.

I don't really see an elegant way to prevent this, so the following just
tries to detect the situation (and more general ones) by brute force.
Bootstrapped and tested on x86_64-linux, verified that the test passes
with a ppc cross, ok?


Bernd


79405.diff


PR rtl-optimization/79405
* fwprop.c (forward_propagate_into): Detect potentially cyclic
replacements and bail out for them.

PR rtl-optimization/79405
* gcc.dg/torture/pr79405.c: New test.

OK.
jeff

Re: [PATCH] Small reg-stack improvement (PR target/70465)

[Jeff Law]

On 02/21/2017 02:23 PM, Jakub Jelinek wrote:

Hi!

As reported by Uros, the
fld a
fld b
fxchg %st(1)
optimization to
fld b
fld a
misses several important cases, one is FLOAT_EXTEND memory loads where
the memory is SFmode or DFmode but we extend it to a wider mode, and
the other is when we load a known i?87 constant like 0.0, 1.0, PI etc.

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

2017-02-21  Jakub Jelinek  

PR target/70465
* reg-stack.c (emit_swap_insn): Treat (float_extend:?F (mem:?F))
and (const_double:?F) like (mem:?F) for the purpose of fxch %st(1)
elimination by swapping fld*.

* gcc.target/i386/pr70465-2.c: New test.

OK.
jeff

[C++ PATCH] For -gdwarf-5 emit DW_TAG_variable instead of DW_TAG_member for C++ static data members (take 2)

[Jakub Jelinek]

On Tue, Feb 21, 2017 at 12:37:32PM -0800, Jason Merrill wrote:
> On Sat, Feb 18, 2017 at 12:17 AM, Jakub Jelinek  wrote:
> > On Fri, Feb 17, 2017 at 09:37:09PM -0500, Jason Merrill wrote:
> >> On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek  wrote:
> >> > -  && die->die_tag != DW_TAG_member)
> >> > +  && die->die_tag != DW_TAG_member
> >> > +  && (die->die_tag != DW_TAG_variable || !class_scope_p 
> >> > (die->die_parent)))
> >>
> >> How about we only check class_scope_p (die->die_parent), and don't
> >> consider the TAG at all?  DW_TAG_member should only appear at class
> >> scope.
> >
> > That wouldn't work, because that would avoid adding linkage attributes to
> > methods.  I could use:
> >   && (die->die_tag == DW_TAG_subprogram || !class_scope_p 
> > (die->die_parent))
> > (not 100% sure if some other tag than those can make it here)
> > or
> >   && ((die->die_tag != DW_TAG_member || die->die_tag != DW_TAG_variable)
> >   || !class_scope_p (die->die_parent))
> > or just use
> >   && (!VAR_P (decl) || !class_scope_p (die->die_parent))
> > or similar.
> >
> >> > - if (old_die->die_tag == DW_TAG_member)
> >> > + if (old_die->die_tag == DW_TAG_member
> >> > + || (dwarf_version >= 5 && class_scope_p 
> >> > (old_die->die_parent)))
> >>
> >> Likewise here.
> >
> > This spot probably can be changed as you wrote, it is in gen_variable_die,
> > so methods shouldn't appear there.
> 
> Hmm, let's think about the behavior we want here.  I don't see any
> reason not to put AT_linkage_name on a member DW_TAG_variable; I think
> the old behavior avoided putting it on DW_TAG_member just because it
> isn't defined for DW_TAG_member.  So it's not clear to me that we need
> any changes in add_linkage_name or its call site.

Ah, ok.  But then we need to make sure gen_variable_die doesn't set
no_linkage_name for -gdwarf-5, because otherwise on the attached testcase
we don't emit linkage_name on b, c, d anywhere (we used to emit that
before).

Starting bootstrap/regtest now, does this look ok if that passes?

2017-02-22  Jakub Jelinek  

* dwarf2out.c (gen_variable_die): For -gdwarf-5, use DW_TAG_variable
instead of DW_TAG_member for static data member declarations and don't
set no_linkage_name for static inline data members.
(gen_member_die): For -gdwarf-5 don't change DW_TAG_variable
to DW_TAG_member.

* g++.dg/debug/dwarf2/inline-var-2.C: New test.

--- gcc/dwarf2out.c.jj  2017-02-18 17:11:18.759821871 +0100
+++ gcc/dwarf2out.c 2017-02-22 17:48:27.378223877 +0100
@@ -22669,7 +22669,8 @@ gen_variable_die (tree decl, tree origin
   && lang_hooks.decls.decl_dwarf_attribute (decl, DW_AT_inline) != -1)
 {
   declaration = true;
-  no_linkage_name = true;
+  if (dwarf_version < 5)
+   no_linkage_name = true;
 }
 
   ultimate_origin = decl_ultimate_origin (decl_or_origin);
@@ -22820,9 +22821,10 @@ gen_variable_die (tree decl, tree origin
 }
 
   /* For static data members, the declaration in the class is supposed
- to have DW_TAG_member tag; the specification should still be
- DW_TAG_variable referencing the DW_TAG_member DIE.  */
-  if (declaration && class_scope_p (context_die))
+ to have DW_TAG_member tag in DWARF{3,4} and we emit it for compatibility
+ also in DWARF2; the specification should still be DW_TAG_variable
+ referencing the DW_TAG_member DIE.  */
+  if (declaration && class_scope_p (context_die) && dwarf_version < 5)
 var_die = new_die (DW_TAG_member, context_die, decl);
   else
 var_die = new_die (DW_TAG_variable, context_die, decl);
@@ -24091,7 +24093,8 @@ gen_member_die (tree type, dw_die_ref co
  && get_AT (child, DW_AT_specification) == NULL)
{
  reparent_child (child, context_die);
- child->die_tag = DW_TAG_member;
+ if (dwarf_version < 5)
+   child->die_tag = DW_TAG_member;
}
  else
splice_child_die (context_die, child);
@@ -24113,7 +24116,7 @@ gen_member_die (tree type, dw_die_ref co
}
 
   /* For C++ inline static data members emit immediately a DW_TAG_variable
-DIE that will refer to that DW_TAG_member through
+DIE that will refer to that DW_TAG_member/DW_TAG_variable through
 DW_AT_specification.  */
   if (TREE_STATIC (member)
  && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline)
--- gcc/testsuite/g++.dg/debug/dwarf2/inline-var-2.C.jj 2017-02-22 
17:54:28.019597440 +0100
+++ gcc/testsuite/g++.dg/debug/dwarf2/inline-var-2.C2017-02-22 
17:54:10.0 +0100
@@ -0,0 +1,35 @@
+// { dg-do compile }
+// { dg-options "-O -std=c++1z -gdwarf-5 -dA -gno-strict-dwarf" }
+// { dg-require-weak "" }
+// { dg-final { scan-assembler-not "DW_TAG_member" { xfail *-*-aix* } } }
+
+inline int a;
+struct S
+{
+  static inline 

Re: PR79286, ira combine_and_move_insns in loops

[Dominique d'Humières]

> Le 22 févr. 2017 à 17:06, Jeff Law  a écrit :
> 
> On 02/22/2017 04:32 AM, Dominique d'Humières wrote:
>> 
>>> Le 21 févr. 2017 à 23:48, Alan Modra  a écrit :
>>> 
>>> On Sat, Feb 18, 2017 at 12:39:08PM +0100, Dominique d'Humières wrote:
> I'm slightly concerned about the test and how it'll behave on targets 
> with small address spaces. If it's a problem we can fault in adjustments.
 
 The test fails on x86_64-apple-darwin16 with -m32 and -O1 and above.
>>> 
>>> Have you investigated just why the test fails?
>> 
>> Segmentation fault
>> 
>> Note that the assembly is the same for revisions r245268 and r245564, i.e., 
>> before and after r245521.
> Given the array index, I kindof expected problems on 32bit platforms.
> 
> Let me stand up an i686 linux instance and see if I can twiddle things 
> without compromising the test.

Also darwin is a -fpic platform.

Dominique

> 
> jeff

Re: PR79286, ira combine_and_move_insns in loops

[Jeff Law]

On 02/22/2017 04:32 AM, Dominique d'Humières wrote:



Le 21 févr. 2017 à 23:48, Alan Modra  a écrit :

On Sat, Feb 18, 2017 at 12:39:08PM +0100, Dominique d'Humières wrote:

I'm slightly concerned about the test and how it'll behave on targets with 
small address spaces. If it's a problem we can fault in adjustments.


The test fails on x86_64-apple-darwin16 with -m32 and -O1 and above.


Have you investigated just why the test fails?


Segmentation fault

Note that the assembly is the same for revisions r245268 and r245564, i.e., 
before and after r245521.

Given the array index, I kindof expected problems on 32bit platforms.

Let me stand up an i686 linux instance and see if I can twiddle things 
without compromising the test.


jeff

Re: [PATCH, GCC/x86 mingw32] Add configure option to force wildcard behavior on Windows

[Thomas Preudhomme]

Oh great thanks!

Best regards,

Thomas

On 17/02/17 22:52, JonY wrote:

On 02/17/2017 11:31 AM, Thomas Preudhomme wrote:

Here you are:

2017-01-24  Thomas Preud'homme  

* configure.ac (--enable-mingw-wildcard): Add new configurable
feature.
* configure: Regenerate.
* config.in: Regenerate.
* config/i386/driver-mingw32.c: new file.
* config/i386/x-mingw32: Add rule to build driver-mingw32.o.
* config.host: Link driver-mingw32.o on MinGW host.
* doc/install.texi: Document new --enable-mingw-wildcard configure
option.

Must have forgotten to paste it.


Thanks, I'll stage it locally until stage 1 opens.

Re: [PATCH 0/5] New warning: -Wstring-plus-int (PR c++/62181)

[Jeff Law]

On 02/22/2017 04:00 AM, Xi Ruoyao wrote:

Hi,

I've implemented -Wstring-plus-int (like the one in CLANG) for GCC. I've
bootstrapped GCC with my patches and runned dejaGNU tests.

My patches are for both C and C++ (like CLANG).

Unlike CLANG, this option is not enabled by default (or GCC won't
bootstrap with -Werror).
Note that the GCC project is currently in "stage4" of its 
development/release cycle.  Meaning only regression bugfixes are being 
considered for integration.  This stage will last until the gcc-7 
release is made.


Your new warning will be reviewed once the development team starts 
looking at gcc-8 work.


THanks,
Jeff

[PATCH] Fix PR79673

[Richard Biener]

The following fixes PR79673.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2017-02-22  Richard Biener  

PR tree-optimization/79673
* tree-ssa-pre.c (compute_avail): Use wide_int_to_tree to
convert the [TARGET_]MEM_REF offset INTEGER_CST, scrapping off
irrelevant address-space qualifiers and avoiding a
ADDR_SPACE_CONVERT_EXPR from fold_convert.

* gcc.target/i386/pr79673.c: New testcase.

Index: gcc/tree-ssa-pre.c
===
--- gcc/tree-ssa-pre.c  (revision 245646)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -3986,21 +3990,21 @@ compute_avail (void)
{
  ref->set = set;
  if (ref1->opcode == MEM_REF)
-   ref1->op0 = fold_convert (TREE_TYPE (ref2->op0),
- ref1->op0);
+   ref1->op0 = wide_int_to_tree (TREE_TYPE (ref2->op0),
+ ref1->op0);
  else
-   ref1->op2 = fold_convert (TREE_TYPE (ref2->op2),
- ref1->op2);
+   ref1->op2 = wide_int_to_tree (TREE_TYPE (ref2->op2),
+ ref1->op2);
}
  else
{
  ref->set = 0;
  if (ref1->opcode == MEM_REF)
-   ref1->op0 = fold_convert (ptr_type_node,
- ref1->op0);
+   ref1->op0 = wide_int_to_tree (ptr_type_node,
+ ref1->op0);
  else
-   ref1->op2 = fold_convert (ptr_type_node,
- ref1->op2);
+   ref1->op2 = wide_int_to_tree (ptr_type_node,
+ ref1->op2);
}
  operands.release ();
 
Index: gcc/testsuite/gcc.target/i386/pr79673.c
===
--- gcc/testsuite/gcc.target/i386/pr79673.c (nonexistent)
+++ gcc/testsuite/gcc.target/i386/pr79673.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void used(double x);
+void usel(long x);
+void test(int c)
+{
+  if (c)
+used(*((double __seg_gs *) 0));
+  else
+usel(*((long __seg_gs *) 0));
+}

Re: [PATCH PR77536]Generate correct profiling information for vectorized loop

[Jan Hubicka]

> Hi Honza,
OK,
thanks!

Honz
> There is the 3rd version patch fixing mentioned issues.  Is it OK?
> 
> Thanks,
> bin

> diff --git a/gcc/testsuite/gcc.dg/vect/pr79347.c 
> b/gcc/testsuite/gcc.dg/vect/pr79347.c
> index 586c638..6825420 100644
> --- a/gcc/testsuite/gcc.dg/vect/pr79347.c
> +++ b/gcc/testsuite/gcc.dg/vect/pr79347.c
> @@ -10,4 +10,4 @@ void n(void)
>  a[i]++;
>  }
>  
> -/* { dg-final { scan-tree-dump-times "Invalid sum of " 2 "vect" } } */
> +/* { dg-final { scan-tree-dump-not "Invalid sum of " "vect" } } */
> diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
> index 43df29c..22c832a 100644
> --- a/gcc/tree-ssa-loop-manip.c
> +++ b/gcc/tree-ssa-loop-manip.c
> @@ -1093,6 +1093,33 @@ scale_dominated_blocks_in_loop (struct loop *loop, 
> basic_block bb,
>  }
>  }
>  
> +/* Return estimated niter for LOOP after unrolling by FACTOR times.  */
> +
> +gcov_type
> +niter_for_unrolled_loop (struct loop *loop, unsigned factor)
> +{
> +  gcc_assert (factor != 0);
> +  bool profile_p = false;
> +  gcov_type est_niter = expected_loop_iterations_unbounded (loop, 
> _p);
> +  gcov_type new_est_niter = est_niter / factor;
> +
> +  /* Without profile feedback, loops for which we do not know a better 
> estimate
> + are assumed to roll 10 times.  When we unroll such loop, it appears to
> + roll too little, and it may even seem to be cold.  To avoid this, we
> + ensure that the created loop appears to roll at least 5 times (but at
> + most as many times as before unrolling).  Don't do adjustment if profile
> + feedback is present.  */
> +  if (new_est_niter < 5 && !profile_p)
> +{
> +  if (est_niter < 5)
> + new_est_niter = est_niter;
> +  else
> + new_est_niter = 5;
> +}
> +
> +  return new_est_niter;
> +}
> +
>  /* Unroll LOOP FACTOR times.  DESC describes number of iterations of LOOP.
> EXIT is the exit of the loop to that DESC corresponds.
>  
> @@ -1170,12 +1197,12 @@ tree_transform_and_unroll_loop (struct loop *loop, 
> unsigned factor,
>gimple_stmt_iterator bsi;
>use_operand_p op;
>bool ok;
> -  unsigned est_niter, prob_entry, scale_unrolled, scale_rest, freq_e, freq_h;
> -  unsigned new_est_niter, i, prob;
> +  unsigned i, prob, prob_entry, scale_unrolled, scale_rest;
> +  gcov_type freq_e, freq_h;
> +  gcov_type new_est_niter = niter_for_unrolled_loop (loop, factor);
>unsigned irr = loop_preheader_edge (loop)->flags & EDGE_IRREDUCIBLE_LOOP;
>auto_vec to_remove;
>  
> -  est_niter = expected_loop_iterations (loop);
>determine_exit_conditions (loop, desc, factor,
>_main_cond, _base, _step,
>_cmp, _bound);
> @@ -1207,22 +1234,6 @@ tree_transform_and_unroll_loop (struct loop *loop, 
> unsigned factor,
>gcc_assert (new_loop != NULL);
>update_ssa (TODO_update_ssa);
>  
> -  /* Determine the probability of the exit edge of the unrolled loop.  */
> -  new_est_niter = est_niter / factor;
> -
> -  /* Without profile feedback, loops for that we do not know a better 
> estimate
> - are assumed to roll 10 times.  When we unroll such loop, it appears to
> - roll too little, and it may even seem to be cold.  To avoid this, we
> - ensure that the created loop appears to roll at least 5 times (but at
> - most as many times as before unrolling).  */
> -  if (new_est_niter < 5)
> -{
> -  if (est_niter < 5)
> - new_est_niter = est_niter;
> -  else
> - new_est_niter = 5;
> -}
> -
>/* Prepare the cfg and update the phi nodes.  Move the loop exit to the
>   loop latch (and make its condition dummy, for the moment).  */
>rest = loop_preheader_edge (new_loop)->src;
> @@ -1326,10 +1337,25 @@ tree_transform_and_unroll_loop (struct loop *loop, 
> unsigned factor,
>/* Ensure that the frequencies in the loop match the new estimated
>   number of iterations, and change the probability of the new
>   exit edge.  */
> -  freq_h = loop->header->frequency;
> -  freq_e = EDGE_FREQUENCY (loop_preheader_edge (loop));
> +
> +  freq_h = loop->header->count;
> +  freq_e = (loop_preheader_edge (loop))->count;
> +  /* Use frequency only if counts are zero.  */
> +  if (freq_h == 0 && freq_e == 0)
> +{
> +  freq_h = loop->header->frequency;
> +  freq_e = EDGE_FREQUENCY (loop_preheader_edge (loop));
> +}
>if (freq_h != 0)
> -scale_loop_frequencies (loop, freq_e * (new_est_niter + 1), freq_h);
> +{
> +  gcov_type scale;
> +  /* Avoid dropping loop body profile counter to 0 because of zero count
> +  in loop's preheader.  */
> +  freq_e = MAX (freq_e, 1);
> +  /* This should not overflow.  */
> +  scale = GCOV_COMPUTE_SCALE (freq_e * (new_est_niter + 1), freq_h);
> +  scale_loop_frequencies (loop, scale, REG_BR_PROB_BASE);
> +}
>  
>exit_bb = single_pred (loop->latch);
>new_exit = find_edge (exit_bb, rest);
> diff --git a/gcc/tree-ssa-loop-manip.h 

[PATCH PR79347/02]Bound profiling counter with niter bound for epilogue loop

[Bin Cheng]

Hi,
This is a followup patch fixing a left over issue in vect_do_peeling.  In the 
first patch,
I used scale_loop_frequencies to scale up profiling counter for epilog_loop 
because
scale_loop_profile doesn't allow scaling up for the now.  This patch adds 
another call
to scale_loop_profile after scaling up.  The added call only bounds counters 
with niter
bound of epilogue.  This is necessary in case if the original loop runs for 
many iterations,
otherwise epilogue loop would still have very large count/frequency comparing 
to vectorized
loop.
BTW, this fix follows existing code/logic, specifically, scale_loop_profile 
itself computes
zero (too small) frequency/count for peeled loop in case of many iterations 
loop.
Consequence is prologue/epilogue loop will be marked as "possibly never be 
executed".
I believe the bound logic in scale_loop_profile needs to be changed just like 
patch to pr77536,
i.e, scaling loop's profiling count wrto estimated niter.  Anyway, this is next 
stage 1 work.

Bootstrap and test on x86_64 and AArch64, is it OK if no failures?

Thanks,
bin

2017-02-21  Bin Cheng  

PR tree-optimization/79347
* tree-vect-loop-manip.c (vect_do_peeling): Compute niter bound for
epilogue loop and use it when scaling profile info.diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index 5ee2c38..3cc7381 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -1645,8 +1645,7 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
   tree type = TREE_TYPE (niters), guard_cond;
   basic_block guard_bb, guard_to;
   int prob_prolog, prob_vector, prob_epilog;
-  int bound_prolog = 0, bound_scalar = 0, bound = 0;
-  int vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
+  int bound_prolog = 0, vf = LOOP_VINFO_VECT_FACTOR (loop_vinfo);
   int prolog_peeling = LOOP_VINFO_PEELING_FOR_ALIGNMENT (loop_vinfo);
   bool epilog_peeling = (LOOP_VINFO_PEELING_FOR_NITER (loop_vinfo)
 || LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo));
@@ -1683,7 +1682,8 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
   bool skip_epilog = (prolog_peeling < 0
  || !LOOP_VINFO_NITERS_KNOWN_P (loop_vinfo));
   /* PEELING_FOR_GAPS is special because epilog loop must be executed.  */
-  if (LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo))
+  bool peel_for_gaps = LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo);
+  if (peel_for_gaps)
 skip_epilog = false;
 
   /* Record the anchor bb at which guard should be placed if scalar loop
@@ -1701,7 +1701,7 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
   basic_block bb_before_loop = loop_preheader_edge (loop)->src;
   scale_bbs_frequencies_int (_before_loop, 1, prob_vector,
 REG_BR_PROB_BASE);
-  scale_loop_profile (loop, prob_vector, bound);
+  scale_loop_profile (loop, prob_vector, 0);
 }
 
   tree niters_prolog = build_int_cst (type, 0);
@@ -1772,6 +1772,8 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
 
   if (epilog_peeling)
 {
+  int bound_epilog = peel_for_gaps ? vf - 1 : vf - 2;
+
   e = single_exit (loop);
   if (!slpeel_can_duplicate_loop_p (loop, e))
{
@@ -1795,8 +1797,8 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
 won't be vectorized.  */
   if (skip_vector)
{
+ int bound_scalar = 0;
  /* Additional epilogue iteration is peeled if gap exists.  */
- bool peel_for_gaps = LOOP_VINFO_PEELING_FOR_GAPS (loop_vinfo);
  tree t = vect_gen_scalar_loop_niters (niters_prolog, prolog_peeling,
bound_prolog,
peel_for_gaps ? vf : vf - 1,
@@ -1813,14 +1815,22 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree niters, 
tree nitersm1,
  e = (e != guard_e ? e : EDGE_PRED (guard_to, 1));
  slpeel_update_phi_nodes_for_guard1 (first_loop, epilog, guard_e, e);
 
+ /* We share epilog loop with scalar version loop.  */
+ bound_epilog = MAX (bound_epilog, bound_scalar - 1);
+
  /* Simply propagate profile info from guard_bb to guard_to which is
 a merge point of control flow.  */
  guard_to->frequency = guard_bb->frequency;
  guard_to->count = guard_bb->count;
  single_succ_edge (guard_to)->count = guard_to->count;
- /* Scale probability of epilog loop back.  */
+ /* Scale probability of epilog loop back.  After scaling up, we
+also need to call scale_loop_profile to bound counters for
+epilog loop in case the original loop runs for many times.
+We can't do it in one call because scale_loop_profile does
+not allow scaling up for now.  */
  int scale_up = REG_BR_PROB_BASE * REG_BR_PROB_BASE / prob_vector;
  

Re: [PATCH PR77536]Generate correct profiling information for vectorized loop

[Bin.Cheng]

On Tue, Feb 21, 2017 at 3:49 PM, Jan Hubicka  wrote:
>> 2017-02-21  Bin Cheng  
>>
>> PR tree-optimization/77536
>> * tree-ssa-loop-manip.c (niter_for_unrolled_loop): New function.
>> (tree_transform_and_unroll_loop): Use above function to compute the
>> estimated niter of unrolled loop and use it when scaling profile.
>> * tree-ssa-loop-manip.h niter_for_unrolled_loop(): New declaration.
>> * tree-vect-loop.c (scale_profile_for_vect_loop): New function.
>> (vect_transform_loop): Call above function.
>>
>> gcc/testsuite/ChangeLog
>> 2017-02-21  Bin Cheng  
>>
>> PR tree-optimization/77536
>> * gcc.dg/vect/pr79347.c: Revise testing string.
>> @@ -1329,7 +1339,12 @@ tree_transform_and_unroll_loop (struct loop *loop, 
>> unsigned factor,
>>freq_h = loop->header->frequency;
>>freq_e = EDGE_FREQUENCY (loop_preheader_edge (loop));
>>if (freq_h != 0)
>> -scale_loop_frequencies (loop, freq_e * (new_est_niter + 1), freq_h);
>> +{
>> +  gcov_type scale;
>> +  /* This should not overflow.  */
>> +  scale = GCOV_COMPUTE_SCALE (freq_e * (new_est_niter + 1), freq_h);
>> +  scale_loop_frequencies (loop, scale, REG_BR_PROB_BASE);
>
> You need to use counts counts when new_est_niter is derrived from profile 
> feedback.
> This is because frequencies are capped to 1, so if loop iterates very 
> many times,
> new_est_niter will be large, freq_h will be 1 and freq_e will be 0.
>
> Also watch the case when freq_e==loop_preheader_edge (loop)->count==0 and 
> freq_h
> is non-zero.  Just do MAX (freq_e, 1). This will not drop the loop body 
> profile to 0.
>
>> +/* Scale profiling counters by estimation for LOOP which is vectorized
>> +   by factor VF.  */
>> +
>> +static void
>> +scale_profile_for_vect_loop (struct loop *loop, unsigned vf)
>> +{
>> +  edge preheader = loop_preheader_edge (loop);
>> +  unsigned freq_h = loop->header->frequency;
>> +  unsigned freq_e = EDGE_FREQUENCY (preheader);
>> +  /* Reduce loop iterations by the vectorization factor.  */
>> +  gcov_type new_est_niter = niter_for_unrolled_loop (loop, vf);
>> +
>> +  /* Use profiling count information if frequencies are zero.  */
>> +  if (freq_h == 0 || freq_e == 0)
>> +{
>> +  freq_e = preheader->count;
>> +  freq_h = loop->header->count;
>> +}
>> +
>> +  if (freq_h != 0)
>> +{
>> +  gcov_type scale;
>> +  /* This should not overflow.  */
>> +  scale = GCOV_COMPUTE_SCALE (freq_e * (new_est_niter + 1), freq_h);
>> +  scale_loop_frequencies (loop, scale, REG_BR_PROB_BASE);
>> +}
>
> Similarly here. Use counts when they are non-zero and use MAX (freq_e, 1).
> freq_e/freq_h needs to be gcov_type in that case.
>
> Patch is OK with these changes.  Thanks a lot!
Hi Honza,
There is the 3rd version patch fixing mentioned issues.  Is it OK?

Thanks,
bin
diff --git a/gcc/testsuite/gcc.dg/vect/pr79347.c 
b/gcc/testsuite/gcc.dg/vect/pr79347.c
index 586c638..6825420 100644
--- a/gcc/testsuite/gcc.dg/vect/pr79347.c
+++ b/gcc/testsuite/gcc.dg/vect/pr79347.c
@@ -10,4 +10,4 @@ void n(void)
 a[i]++;
 }
 
-/* { dg-final { scan-tree-dump-times "Invalid sum of " 2 "vect" } } */
+/* { dg-final { scan-tree-dump-not "Invalid sum of " "vect" } } */
diff --git a/gcc/tree-ssa-loop-manip.c b/gcc/tree-ssa-loop-manip.c
index 43df29c..22c832a 100644
--- a/gcc/tree-ssa-loop-manip.c
+++ b/gcc/tree-ssa-loop-manip.c
@@ -1093,6 +1093,33 @@ scale_dominated_blocks_in_loop (struct loop *loop, 
basic_block bb,
 }
 }
 
+/* Return estimated niter for LOOP after unrolling by FACTOR times.  */
+
+gcov_type
+niter_for_unrolled_loop (struct loop *loop, unsigned factor)
+{
+  gcc_assert (factor != 0);
+  bool profile_p = false;
+  gcov_type est_niter = expected_loop_iterations_unbounded (loop, _p);
+  gcov_type new_est_niter = est_niter / factor;
+
+  /* Without profile feedback, loops for which we do not know a better estimate
+ are assumed to roll 10 times.  When we unroll such loop, it appears to
+ roll too little, and it may even seem to be cold.  To avoid this, we
+ ensure that the created loop appears to roll at least 5 times (but at
+ most as many times as before unrolling).  Don't do adjustment if profile
+ feedback is present.  */
+  if (new_est_niter < 5 && !profile_p)
+{
+  if (est_niter < 5)
+   new_est_niter = est_niter;
+  else
+   new_est_niter = 5;
+}
+
+  return new_est_niter;
+}
+
 /* Unroll LOOP FACTOR times.  DESC describes number of iterations of LOOP.
EXIT is the exit of the loop to that DESC corresponds.
 
@@ -1170,12 +1197,12 @@ tree_transform_and_unroll_loop (struct loop *loop, 
unsigned factor,
   gimple_stmt_iterator bsi;
   use_operand_p op;
   bool ok;
-  unsigned est_niter, prob_entry, scale_unrolled, scale_rest, freq_e, freq_h;
-  unsigned new_est_niter, i, prob;
+  unsigned i, prob, prob_entry, scale_unrolled, scale_rest;
+  gcov_type freq_e, freq_h;
+  gcov_type new_est_niter = 

Re: PR79286, ira combine_and_move_insns in loops

[Dominique d'Humières]

> Le 21 févr. 2017 à 23:48, Alan Modra  a écrit :
> 
> On Sat, Feb 18, 2017 at 12:39:08PM +0100, Dominique d'Humières wrote:
>>> I'm slightly concerned about the test and how it'll behave on targets with 
>>> small address spaces. If it's a problem we can fault in adjustments.
>> 
>> The test fails on x86_64-apple-darwin16 with -m32 and -O1 and above.
> 
> Have you investigated just why the test fails?  

Segmentation fault

Note that the assembly is the same for revisions r245268 and r245564, i.e., 
before and after r245521.

Dominique

> I don't know how to go
> about building a cross-toolchain for darwin, due to lack of darwin
> support in gas and ld.
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM

RE: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

[Matthew Fortune]

> > I will keep working on this code post GCC 7 as the topic is bugging me
> now
> > :-)
> 
> I'm OK to lend a hand, but what's left for GCC 7 to make MIPS happy?

I have just successfully bootstrapped MIPS with just the pending (amended)
patch (3). i.e. with this patch (1) reverted so I did not need this one
after all.

I should have gone back and checked that originally. I'll commit the
updated patch (3) later today. After checking an ARM build.

Thanks for everyone's help and patience,
Matthew

[PATCH, wwwdocs] Update list of ARM cpus in readings.html

[Richard Earnshaw (lists)]

The list of ARM CPUs in readings.html is now looking rather dated.  This
patch updates the list to refer to the most commonly available parts (or
at least, the product families by which they are known).

Committed.

R.
? backends.html~
? index.html.~1.866.~
? readings.html.~1.260.~
? svn.html.~1.183.~
? svn.html~
Index: readings.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/readings.html,v
retrieving revision 1.260
diff -u -r1.260 readings.html
--- readings.html   12 Feb 2017 08:10:51 -  1.260
+++ readings.html   22 Feb 2017 11:14:29 -
@@ -80,7 +80,7 @@
 
  ARM
   Manufacturer: Various, by license from ARM.
-  CPUs include: ARM7 and ARM7T series (eg. ARM7TDMI), ARM9 and StrongARM
+  CPUs include: ARM7TDMI, and the Cortex-A, Cortex-R and Cortex-M series.
   http://infocenter.arm.com/help/index.jsp;>ARM 
Documentation
  
 

Re: [PATCH] Fix PR79666

[Eric Botcazou]

> Eric - you added these improvements so you might want to double-check
> effects on Ada code (and see if the above suggestion is worth
> the effort).

I think that all the relevant testcases are in the gnat.dg testsuite.

-- 
Eric Botcazou

Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

[Eric Botcazou]

> I will keep working on this code post GCC 7 as the topic is bugging me now
> :-)

I'm OK to lend a hand, but what's left for GCC 7 to make MIPS happy?

-- 
Eric Botcazou

[PATCH 5/5] New tests for -Wstring-plus-int

[Xi Ruoyao]

This patch adds tests for -Wstring-plus-int.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* testsuite/c-c++-common/Wstring-plus-int.c: New test
* testsuite/g++.dg/Wstring-plus-int-1.C: New test
* testsuite/g++.dg/Wstring-plus-int-2.C: New test
* testsuite/gcc.dg/Wstring-plus-int.c: New test

---
 gcc/testsuite/c-c++-common/Wstring-plus-int.c | 35 +++
 gcc/testsuite/g++.dg/Wstring-plus-int-1.C | 20 +++
 gcc/testsuite/g++.dg/Wstring-plus-int-2.C | 32 
 gcc/testsuite/gcc.dg/Wstring-plus-int.c   | 17 +
 4 files changed, 104 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C
 create mode 100644 gcc/testsuite/gcc.dg/Wstring-plus-int.c

diff --git a/gcc/testsuite/c-c++-common/Wstring-plus-int.c 
b/gcc/testsuite/c-c++-common/Wstring-plus-int.c
new file mode 100644
index 000..f2a10a3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstring-plus-int.c
@@ -0,0 +1,35 @@
+/* Test -Wstring-plus-int.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-int" } */
+
+/* From stdint.h.  */
+typedef __UINT_LEAST32_TYPE__ uint_least32_t;
+
+/* From wchar.h.  */
+#ifndef __cplusplus
+typedef __WCHAR_TYPE__ wchar_t;
+#endif
+
+int getchar(void);
+
+int
+main (void)
+{
+  /* Following code should be warn in both C/C++, for any standards.  */
+  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
+  const wchar_t *b = L"aa" + L'a'; /* { dg-warning "does not append" } */
+  const char *e = "aa" + getchar(); /* { dg-warning "does not append" } */
+
+  a += 'a'; /* { dg-warning "does not append" } */
+  const char *f = a + 'a'; /* { dg-warning "does not append" } */
+  const char *g = 'a' + a; /* { dg-warning "does not append" } */
+
+  /* Following code should never be warned.  */
+  char x = a['x']; /* { dg-bogus "does not append" } */
+  const char *k = a + 1; /* { dg-bogus "does not append" } */
+  a += 1; /* { dg-bogus "does not append" } */
+  a += (uint_least32_t) 1; /* { dg-bogus "does not append" } */
+
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-1.C 
b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C
new file mode 100644
index 000..82631da
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C
@@ -0,0 +1,20 @@
+/* Test -Wstring-plus-int, for C++11 built-in character types.  */
+
+/* { dg-do compile } */
+/* { dg-options "-std=c++11 -Wstring-plus-int" } */
+
+int
+main(void)
+{
+  const wchar_t *a = L"aa";
+  const char16_t *b = u"aa" + u'a'; /* { dg-warning "does not append" } */
+  const char32_t *c = U"aa" + U'a'; /* { dg-warning "does not append" } */
+
+  /* In C++11, wchar_t, char16_t and char32_t are built-in.
+ There should be warnings.  */
+  const wchar_t *h = a + L'a'; /* { dg-warning "does not append" } */
+  const char16_t *i = b + u'a'; /* { dg-warning "does not append" } */
+  const char32_t *j = c + U'a'; /* { dg-warning "does not append" } */
+
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-2.C 
b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C
new file mode 100644
index 000..48fdb4f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C
@@ -0,0 +1,32 @@
+/* Test -Wstring-plus-int.
+   This is a more realistic test case.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-int" } */
+
+class good_string
+{
+public:
+  good_string(const char *);
+  good_string =(const char *);
+  good_string operator+(char) const;
+  operator const char *(void) const;
+};
+
+/* Someone has forgotten operator+ overload...  */
+class bad_string
+{
+public:
+  bad_string(const char *);
+  bad_string =(const char *);
+  operator const char *(void) const;
+};
+
+int main()
+{
+  good_string good = "aa";
+  good = good + 'a'; /* { dg-bogus "does not append" } */
+  bad_string bad = "bb";
+  bad = bad + 'b'; /* { dg-warning "does not append" } */
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/Wstring-plus-int.c 
b/gcc/testsuite/gcc.dg/Wstring-plus-int.c
new file mode 100644
index 000..bc029b8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstring-plus-int.c
@@ -0,0 +1,17 @@
+/* Test -Wstring-plus-int with C11 UTF literals.  */
+
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -Wstring-plus-int" } */
+
+/* From wchar.h and uchar.h.  */
+typedef __CHAR16_TYPE__ char16_t;
+typedef __CHAR32_TYPE__ char32_t;
+
+int
+main (void)
+{
+  const char16_t *a = u"aa" + u'a'; /* { dg-warning "does not append" } */
+  const char32_t *b = U"aa" + U'a'; /* { dg-warning "does not append" } */
+
+  return 0;
+}

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH 3/5] c: New warning option -Wstring-plus-int

[Xi Ruoyao]

This patch adds warning option -Wstring-plus-int for C.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* c/c-typeck.c (parser_build_binary_op, build_modify_expr): checking 
for -Wstring-plus-int

---
 gcc/c/c-typeck.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 49b99b5..288d44b 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3631,6 +3631,24 @@ parser_build_binary_op (location_t location, enum 
tree_code code,
 ? arg2.original_type
 : TREE_TYPE (arg2.value));
 
+  /* Warn about adding string literals/pointers and chars.  */
+  if (warn_string_plus_int && (TREE_CODE (type1) == POINTER_TYPE ||
+   TREE_CODE (type2) == POINTER_TYPE))
+{
+  enum tree_code code_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+ code1 : code2);
+  tree type_ptr = (TREE_CODE (type1) == POINTER_TYPE ?
+   type1 : type2);
+  tree type_int = (TREE_CODE (type1) == POINTER_TYPE ?
+   type2 : type1);
+  enum tree_code code_int = TREE_CODE (TREE_CODE (type1)
+   == POINTER_TYPE ?
+   arg2.value : arg1.value);
+
+     maybe_warn_string_plus_int (location, type_ptr, type_int,
+     code_ptr, code_int);
+}
+
   result.value = build_binary_op (location, code,
      arg1.value, arg2.value, 1);
   result.original_code = code;
@@ -5768,6 +5786,16 @@ build_modify_expr (location_t location, tree lhs, tree 
lhs_origtype,
      rhseval = newrhs;
    }
 
+  /* Warn about adding string literals/pointers and chars.  */
+     if (modifycode == PLUS_EXPR && warn_string_plus_int)
+   {
+     tree lhs_type1 = (lhs_origtype ? lhs_origtype :
+      TREE_TYPE (lhs));
+     tree rhs_type1 = (rhs_origtype ? rhs_origtype :
+      TREE_TYPE (rhs));
+     maybe_warn_string_plus_int (location, lhs_type1, rhs_type1,
+     TREE_CODE (lhs), TREE_CODE (rhs));
+   }
      newrhs = build_binary_op (location,
    modifycode, lhs, newrhs, 1);


-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH 4/5] Document warning option -Wstring-plus-int

[Xi Ruoyao]

This patch adds document of -Wstring-plus-int.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* doc/invoke.texi (Warning Options): Document -Wstring-plus-int

---
 gcc/doc/invoke.texi | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e1a1d13..6e929e8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -307,7 +307,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wsizeof-pointer-memaccess  -Wsizeof-array-argument @gol
 -Wstack-protector  -Wstack-usage=@var{len}  -Wstrict-aliasing @gol
 -Wstrict-aliasing=n  -Wstrict-overflow  -Wstrict-overflow=@var{n} @gol
--Wstringop-overflow=@var{n} @gol
+-Wstringop-overflow=@var{n} -Wstring-plus-int @gol
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
 -Wsuggest-final-types @gol  -Wsuggest-final-methods  -Wsuggest-override @gol
 -Wmissing-format-attribute  -Wsubobject-linkage @gol
@@ -5046,6 +5046,12 @@ whether to issue a warning.  Similarly to 
@option{-Wstringop-overflow=3} this
 setting of the option may result in warnings for benign code.
 @end table
 
+@item -Wstring-plus-int
+@opindex Wstring-plus-int
+@opindex Wno-string-plus-int
+Warn for adding an integer to a string pointer or string literal,
+which seems an ill-formed attempt to append to the string.
+
 @item -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]}
 @opindex Wsuggest-attribute=
 @opindex Wno-suggest-attribute=

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH 2/5] c++: New warning option -Wstring-plus-int

[Xi Ruoyao]

This patch adds warning option -Wstring-plus-int for C++.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* c-family/c-common.h (maybe_warn_string_plus_int): new prototype
* c-family/c-warn.h (maybe_warn_string_plus_int): new function
* c-family/c-opt: new option -Wstring-plus-int
* cp/call.c (build_new_op_1): Checking for -Wstring-plus-int

---
 gcc/c-family/c-common.h |  2 ++
 gcc/c-family/c-warn.c   | 34 ++
 gcc/c-family/c.opt  |  5 +
 gcc/cp/call.c   | 35 +++
 4 files changed, 76 insertions(+)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 72fba56..8c748e9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1503,6 +1503,8 @@ extern void c_do_switch_warnings (splay_tree, location_t, 
tree, tree, bool,
      bool);
 extern void warn_for_omitted_condop (location_t, tree);
 extern void warn_for_restrict (unsigned, vec *);
+extern void maybe_warn_string_plus_int (location_t, tree, tree,
+enum tree_code, enum tree_code);
 
 /* Places where an lvalue, or modifiable lvalue, may be required.
Used to select diagnostic messages in lvalue_error and
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 09c5760..0b038a7 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2290,3 +2290,37 @@ do_warn_duplicated_branches_r (tree *tp, int *, void *)
 do_warn_duplicated_branches (*tp);
   return NULL_TREE;
 }
+
+static inline bool
+char_type_cv_p (tree type)
+{
+  return char_type_p (build_qualified_type (type, TYPE_UNQUALIFIED));
+}
+
+/* Warn about adding string literals/pointers and chars.
+   Produce a warning when:
+ 1) arg1 is a string literal, and arg2 is a non-literal integer;
+ 2) arg1 is a string pointer, and arg2 is a character.  */
+
+void
+maybe_warn_string_plus_int (location_t loc, tree type1, tree type2,
+enum tree_code code1, enum tree_code code2)
+{
+  bool arg2_is_char = char_type_cv_p (type2);
+  /* In C11, char16_t and char32_t are typedefs. Prevent false positives
+ about uint_least16_t and uint_least32_t.  */
+  if (type2 == uint_least16_type_node ||
+     type2 == uint_least32_type_node)
+   arg2_is_char = false;
+
+  if (code1 == STRING_CST && TREE_CODE (type2) == INTEGER_TYPE &&
+  (code2 != INTEGER_CST || arg2_is_char))
+warning_at (loc, OPT_Wstring_plus_int,
+"adding %qT to string literal does not append to "
+"the string", type2);
+  else if (TREE_CODE (type1) == POINTER_TYPE &&
+      char_type_cv_p (TREE_TYPE (type1)) && arg2_is_char)
+warning_at (loc, OPT_Wstring_plus_int,
+"adding %qT to string pointer %qT does not append "
+"to the string", type2, type1);
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a1b3ae5..f0b482f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -716,6 +716,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger 
Var(warn_stringop_overflow) Ini
 Under the control of Object Size type, warn about buffer overflow in string
 manipulation functions like memcpy and strcpy.
 
+Wstring-plus-int
+C ObjC C++ ObjC++ Var(warn_string_plus_int) Warning
+Warn about adding string pointers and integers, which is likely an ill-formed 
attempt
+to append the string.
+
 Wsuggest-attribute=format
 C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
 Warn about functions which might be candidates for format attributes.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 93fae0d..95bda1b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -,6 +,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int 
flags, tree arg1,
   void *p;
   bool strict_p;
   bool any_viable_p;
+  tree orig_arg1 = error_mark_node;
+  tree orig_arg2 = error_mark_node;
 
   if (error_operand_p (arg1)
   || error_operand_p (arg2)
@@ -5605,8 +5607,20 @@ build_new_op_1 (location_t loc, enum tree_code code, int 
flags, tree arg1,
   code_orig_arg2 = TREE_CODE (TREE_TYPE (arg2));
   break;
 
+case PLUS_EXPR:
+  /* These are saved for the sake of warn_string_plus_int.  */
+  orig_arg1 = arg1;
+  orig_arg2 = arg2;
+  break;
+
   /* =, ->, [], () must be non-static member functions.  */
 case MODIFY_EXPR:
+  if (code2 == PLUS_EXPR)
+{
+     /* Like PLUS_EXPR.  */
+     orig_arg1 = arg1;
+     orig_arg2 = arg2;
+}
   if (code2 != NOP_EXPR)
    break;
   /* FALLTHRU */
@@ -5937,9 +5951,30 @@ build_new_op_1 (location_t loc, enum tree_code code, int 
flags, tree arg1,
 return result;
 
  builtin:
+  /* Warn about adding string literals/pointers and chars.  */
+  if (code == PLUS_EXPR && (complain & tf_warning) &&
+  warn_string_plus_int)
+{
+  maybe_warn_string_plus_int (loc, 

[PATCH 1/5] Move char_type_p prototype into c-common.h

[Xi Ruoyao]

This patch moves prototype of char_type_p into c-common.h, so we can
use it in c-family/*.

gcc/ChangeLog:

2017-02-22  Xi Ruoyao  

* c-family/c-common.h (char_type_p): New prototype
* c/c-typeck.c (char_type_p): Make the function extern
* cp/cp-tree.h (char_type_p): Remove prototype
* cp/tree.c (char_type_p): Change the return type to bool

---
 gcc/c-family/c-common.h | 1 +
 gcc/c/c-typeck.c| 4 +++-
 gcc/cp/cp-tree.h| 1 -
 gcc/cp/tree.c   | 2 +-
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 9c28809..72fba56 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -962,6 +962,7 @@ extern tree build_real_imag_expr (location_t, enum 
tree_code, tree);
 extern tree build_unary_op (location_t, enum tree_code, tree, bool);
 extern tree build_binary_op (location_t, enum tree_code, tree, tree, int);
 extern tree perform_integral_promotions (tree);
+extern bool char_type_p (tree);
 
 /* These functions must be defined by each front-end which implements
a variant of the C language.  They are used by port files.  */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index ed8ffe4..49b99b5 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "tree-inline.h"
 #include "omp-general.h"
+#include "c-family/c-common.h"
 #include "c-family/c-objc.h"
 #include "c-family/c-ubsan.h"
 #include "cilk.h"
@@ -3597,7 +3598,7 @@ parser_build_unary_op (location_t loc, enum tree_code 
code, struct c_expr arg)
 
 /* Returns true if TYPE is a character type, *not* including wchar_t.  */
 
-static bool
+bool
 char_type_p (tree type)
 {
   return (type == char_type_node
@@ -5766,6 +5767,7 @@ build_modify_expr (location_t location, tree lhs, tree 
lhs_origtype,
      newrhs = in_late_binary_op ? save_expr (rhs) : c_save_expr (rhs);
      rhseval = newrhs;
    }
+
      newrhs = build_binary_op (location,
    modifycode, lhs, newrhs, 1);
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f53f744..49a36c2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6685,7 +6685,6 @@ extern bool cv_qualified_p
(const_tree);
 extern tree cv_unqualified (tree);
 extern special_function_kind special_function_p (const_tree);
 extern int count_trees (tree);
-extern int char_type_p (tree);
 extern void verify_stmt_tree   (tree);
 extern linkage_kind decl_linkage   (tree);
 extern duration_kind decl_storage_duration (tree);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index d3c63b8..e6c17f8 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4394,7 +4394,7 @@ special_function_p (const_tree decl)
 
 /* Returns nonzero if TYPE is a character type, including wchar_t.  */
 
-int
+bool
 char_type_p (tree type)
 {
   return (same_type_p (type, char_type_node)

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

[PATCH 0/5] New warning: -Wstring-plus-int (PR c++/62181)

[Xi Ruoyao]

Hi,

I've implemented -Wstring-plus-int (like the one in CLANG) for GCC. I've
bootstrapped GCC with my patches and runned dejaGNU tests.

My patches are for both C and C++ (like CLANG).

Unlike CLANG, this option is not enabled by default (or GCC won't
bootstrap with -Werror).


Xi Ruoyao (5):
  Move char_type_p prototype into c-common.h
  c++: New warning option -Wstring-plus-int
  c: New warning option -Wstring-plus-int
  Document warning option -Wstring-plus-int
  New tests for option -Wstring-plus-int

 gcc/c-family/c-common.h   |  3 +++
 gcc/c-family/c-warn.c | 34 ++
 gcc/c-family/c.opt|  5 
 gcc/c/c-typeck.c  | 32 +++-
 gcc/cp/call.c | 35 +++
 gcc/cp/cp-tree.h  |  1 -
 gcc/cp/tree.c |  2 +-
 gcc/doc/invoke.texi   |  8 +-
 gcc/testsuite/c-c++-common/Wstring-plus-int.c | 35 +++
 gcc/testsuite/g++.dg/Wstring-plus-int-1.C | 20 +++
 gcc/testsuite/g++.dg/Wstring-plus-int-2.C | 32 
 gcc/testsuite/gcc.dg/Wstring-plus-int.c   | 17 +
 12 files changed, 220 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C
 create mode 100644 gcc/testsuite/gcc.dg/Wstring-plus-int.c

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 1/5] Handle WORD_REGISTER_OPERATIONS when reloading (subreg(reg))

[Richard Earnshaw (lists)]

On 21/02/17 19:53, Matthew Fortune wrote:
> Eric Botcazou  writes:
>>> Agreed.  I don't think things like WORD_MODE_OPERATIONS should change
>>> rtl semantics, just optimisation decisions.
> 
> Sorry, I did cover two topics in one email.  My point was about whether
> simplifying:
> 
> (subreg:OUTER (mem:INNER ...))
> To:
> (mem:OUTER ...)
> 
> Should ever be a requirement for successful reloading rather than just an
> optimisation that 'could' be applied. There are a few cases it seems that
> require this simplification to happen which I find odd.
> 

If mem:INNER has side effects (volatile, pre/post addressing etc), then
the mem must never be accessed in any mode other than INNER.  So I agree
that if there are places that assume otherwise that is odd; but they may
have already dealt with the side effects problem earlier.

R.

>>> And using the smallest
>>> possible spill size is often good even for RISCy targets.
> 
> Yes, if (subreg(mem)) simplification only ever happened when it was reducing
> the size of the load/store I would understand the code more too but actually
> it will currently happily simplify to a wider mode. Using a wider mode may
> well be beneficial in other situations where a further reload would be needed
> due to insn constraints I guess. It all feels a bit like magic still.
> 
>> I don't think that's correct, WORD_MODE_OPERATIONS does change RTL semantics
>> for SUBREGs smaller than a word since it can make all bits defined, even if
>> only the lower part is assigned.  For example (SUBREG:SI (MEM:QI)) has the
>> higher bits defined according to LOAD_EXTEND_OP if WORD_MODE_OPERATIONS.
> 
> It would almost be simpler if we had another variant of subreg (like we have
> strict_low_part) to explicitly represent the WORD_REGISTER_OPERATIONS 
> behaviour
> with the mode change. I'll stop myself and hold this for later though!
> 
>> LRA simply needs to preserve the semantics, just as reload does.
> 
> I will keep working on this code post GCC 7 as the topic is bugging me now :-)
> 
> Thanks,
> Matthew
> 

[PR 78140] Reuse same IPA bits and VR info

[Martin Jambor]

Hello,

this is a fix for PR 78140 which is about LTO WPA of Firefox taking
1GB memory more than gcc 6.

It works by reusing the ipa_bits and value_range that we previously
had directly in jump functions and which are just too big to be
allocated for each actual argument in all of Firefox.  Reusing is
achieved by two hash table traits derived from ggc_cache_remove which
apparently has been created just for this purpose and once I
understood them they made my life a lot easier.  In future, I will
have a look at applying this method to other parts of jump functions
as well.

According to my measurements, the patch saves about 1.2 GB of memory.
The problem is that some change last week (between revision 245382 and
245595) has more than invalidated this:

  | compiler| WPA mem (GB) |
  |-+--|
  | gcc 6 branch| 3.86 |
  | trunk rev. 245382   | 5.21 |
  | patched rev. 245382 | 4.06 |
  | trunk rev. 245595   | 6.59 |
  | patched rev. 245595 | 5.25 |

(I have verified this by Martin's way of measuring things.)  I will
try to bisect what commit has caused the increase.  Still, the patch
helps a lot.

There is one thing in the patch that intrigues me, I do not understand
why I had to mark value_range with GTY((for_user)) - as opposed to
just GTY(()) that was there before - whereas ipa_bits does not need
it.  If anyone could enlighten me, that would be great.  But I suppose
this is not an indication of anything being wrong under the hood.

I have bootstrapped and LTO-bootstrapped the patch on x86_64-linux and
also bootstrapped (C, C++ and Fortran) on an aarch64 and i686 cfarm
machine.  I have also LTO-built Firefox with the patch and used it to
browse for a while and it seemed fine.

OK for trunk?

Thanks,

Martin


2017-02-20  Martin Jambor  

PR lto/78140
* ipa-prop.h (ipa_bits): Removed field known.
(ipa_jump_func): Removed field vr_known.  Changed fields bits and m_vr
to pointers.  Adjusted their comments to warn about their sharing.
(ipcp_transformation_summary): Change bits to a vector of pointers.
(ipa_check_create_edge_args): Moved to ipa-prop.c, declare.
(ipa_get_ipa_bits_for_value): Declare.
* tree-vrp.h (value_range): Mark as GTY((for_user)).
* ipa-prop.c (ipa_bit_ggc_hash_traits): New.
(ipa_bits_hash_table): Likewise.
(ipa_vr_ggc_hash_traits): Likewise.
(ipa_vr_hash_table): Likewise.
(ipa_print_node_jump_functions_for_edge): Adjust for bits and m_vr
being pointers and vr_known being removed.
(ipa_set_jf_unknown): Likewise.
(ipa_get_ipa_bits_for_value): New function.
(ipa_set_jfunc_bits): Likewise.
(ipa_get_value_range): New overloaded functions.
(ipa_set_jfunc_vr): Likewise.
(ipa_compute_jump_functions_for_edge): Use the above functions to
construct bits and vr parts of jump functions.
(ipa_check_create_edge_args): Move here from ipa-prop.h, also allocate
ipa_bits_hash_table and ipa_vr_hash_table if they do not already
exist.
(ipcp_grow_transformations_if_necessary): Also allocate
ipa_bits_hash_table and ipa_vr_hash_table if they do not already
exist.
(ipa_node_params_t::duplicate): Do not copy bits, just pointers to
them.  Fix too long lines.
(ipa_write_jump_function): Adjust for bits and m_vr being pointers and
vr_known being removed.
(ipa_read_jump_function): Use new setter functions to construct bits
and vr parts of jump functions or set them to NULL.
(write_ipcp_transformation_info): Adjust for bits being pointers.
(read_ipcp_transformation_info): Likewise.
(ipcp_update_bits): Likewise.  Fix excessively long lines a trailing
space.
Include gt-ipa-prop.h.
* ipa-cp.c (propagate_bits_across_jump_function): Adjust for bits
being pointers.
(ipcp_store_bits_results): Likewise.
(propagate_vr_across_jump_function): Adjust for m_vr being a pointer.
Do not write to existing jump functions but use a temporary instead.
---
 gcc/ipa-cp.c   |  49 
 gcc/ipa-prop.c | 381 ++---
 gcc/ipa-prop.h |  32 ++---
 gcc/tree-vrp.h |   2 +-
 4 files changed, 319 insertions(+), 145 deletions(-)

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index aa3c9973a66..16e7e2216ab 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1819,8 +1819,8 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int 
idx,
 and we store it in jump function during analysis stage.  */
 
   if (src_lats->bits_lattice.bottom_p ()
- && jfunc->bits.known)
-   return dest_lattice->meet_with (jfunc->bits.value, jfunc->bits.mask,
+ && jfunc->bits)
+   return dest_lattice->meet_with (jfunc->bits->value, jfunc->bits->mask,
  

[PATCH] Fix PR79666

[Richard Biener]

I am testing the following patch to fix PR79666 - we may not create
a symbolic range with expressions that may introduce (not already present)
undefined overflow.

With a new predicate like operand_not_min_p we can possibly improve
this for symbolic ops with a useful range, but I'm not sure we'd
ever arrive with non-VARYING ops here.

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

Eric - you added these improvements so you might want to double-check
effects on Ada code (and see if the above suggestion is worth
the effort).

Thanks,
Richard.

2017-02-22  Richard Biener  

PR tree-optimization/79666
* tree-vrp.c (extract_range_from_binary_expr_1): Make sure
to not symbolically negate if that may introduce undefined
overflow.

* gcc.dg/torture/pr79666.c: New testcase.

Index: gcc/tree-vrp.c
===
--- gcc/tree-vrp.c  (revision 245646)
+++ gcc/tree-vrp.c  (working copy)
@@ -2631,8 +2631,17 @@ extract_range_from_binary_expr_1 (value_
min = build_symbolic_expr (expr_type, sym_min_op0,
   neg_min_op0, min);
  else if (sym_min_op1)
-   min = build_symbolic_expr (expr_type, sym_min_op1,
-  neg_min_op1 ^ minus_p, min);
+   {
+ /* We may not negate if that might introduce
+undefined overflow.  */
+ if (! minus_p
+ || neg_min_op1
+ || TYPE_OVERFLOW_WRAPS (expr_type))
+   min = build_symbolic_expr (expr_type, sym_min_op1,
+  neg_min_op1 ^ minus_p, min);
+ else
+   min = NULL_TREE;
+   }
 
  /* Likewise for the upper bound.  */
  if (sym_max_op0 == sym_max_op1)
@@ -2641,8 +2650,17 @@ extract_range_from_binary_expr_1 (value_
max = build_symbolic_expr (expr_type, sym_max_op0,
   neg_max_op0, max);
  else if (sym_max_op1)
-   max = build_symbolic_expr (expr_type, sym_max_op1,
-  neg_max_op1 ^ minus_p, max);
+   {
+ /* We may not negate if that might introduce
+undefined overflow.  */
+ if (! minus_p
+ || neg_max_op1
+ || TYPE_OVERFLOW_WRAPS (expr_type))
+   max = build_symbolic_expr (expr_type, sym_max_op1,
+  neg_max_op1 ^ minus_p, max);
+ else
+   max = NULL_TREE;
+   }
}
   else
{
Index: gcc/testsuite/gcc.dg/torture/pr79666.c
===
--- gcc/testsuite/gcc.dg/torture/pr79666.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr79666.c  (working copy)
@@ -0,0 +1,30 @@
+/* { dg-do run } */
+
+struct
+{
+  unsigned a:6;
+} b;
+
+int c, e, g = 7;
+signed char d, f = 6, h = -10;
+
+void fn1 ()
+{
+  for (; c < 9; c++)
+{
+  if (f)
+   g = ~(~0 / (g ^ e));
+  b.a = ~0;
+  d = ~((h ^ b.a) & 132 & (~(f && g) | (d && 1)));
+  e = ~0;
+  if (d < 127 || f < 1)
+   continue;
+  g = 0;
+}
+}
+
+int main ()
+{
+  fn1 ();
+  return 0; 
+}

Re: Miscellaneous optimization group fixes (was: Rename the "openmp" group of optimizations to "omp")

[Martin Jambor]

Hi,

On Wed, Feb 22, 2017 at 08:58:06AM +0100, Thomas Schwinge wrote:
> > 
> > Rename the "openmp" group of optimizations to "omp"
> > 
> > gcc/
> > * dumpfile.h (OPTGROUP_OPENMP): Rename to OPTGROUP_OMP.  Adjust
> > all users.
> > * dumpfile.c (optgroup_options): Instead of "openmp", associate
> > OPTGROUP_OMP with "omp".


I am of course fine with OPTGROUP_OMP.

> 
> On top of that, OK to commit the following (not yet tested) -- these all
> look like oversights to me, but please verify?

The missing documentation is an oversight.  Thanks for spotting it.

Martin


> 
> commit 9865976a121c1bd0fc59ea75e819924733f7ea98
> Author: Thomas Schwinge 
> Date:   Wed Feb 22 08:32:54 2017 +0100
> 
> Miscellaneous optimization group fixes
> 
> gcc/doc/
> * invoke.texi (-fopt-info): Document "omp".
> * optinfo.texi (Optimization groups): Fix option used for
> OPTGROUP_ALL.
> gcc/
> * dumpfile.h: Sort OPTGROUP_OMP before OPTGROUP_VEC.
> * hsa-gen.c (pass_data_gen_hsail): Use OPTGROUP_OMP.
> * ipa-hsa.c (pass_data_ipa_hsa): Likewise.
> * omp-simd-clone.c (pass_data_omp_simd_clone): Likewise.

Re: [PATCH] Look through clobber stmts in uninit (PR tree-optimization/79345)

[Richard Biener]

On Tue, 21 Feb 2017, Jeff Law wrote:

> On 02/21/2017 02:01 AM, Richard Biener wrote:
> > On Mon, 20 Feb 2017, Marc Glisse wrote:
> > 
> > > On Mon, 20 Feb 2017, Jakub Jelinek wrote:
> > > 
> > > > As mentioned by Jason in the PR, we've regressed on the following
> > > > testcase
> > > > since we started emitting CLOBBERs at the start of ctors (and we warn as
> > > > before with -fno-lifetime-dse -Wuninitialized).
> > > > With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
> > > > and thus we warn, but if there are clobbers before that, that is not the
> > > > case and we don't warn.  The patch is quick hack to bypass the initial
> > > > clobbers as long as there aren't really many.  If we wanted to handle
> > > > all initial clobbers, I bet the first time we run into this we could
> > > > recursively walk vop uses from the default def and build say a bitmap
> > > > of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
> > > > before it as vdef stmts.
> > > 
> > >   MEM[(struct  &)] ={v} {CLOBBER};
> > >   _4 = b.x;
> > >   if (_4 != 0)
> > > 
> > > It would be nice (at some point in a distant future) to turn that into
> > > 
> > >   MEM[(struct  &)] ={v} {CLOBBER};
> > >   if (_4(D) != 0)
> > > 
> > > i.e. replace reads from clobbered memory with a default def (with a
> > > gimple_nop
> > > defining statement).
> > > 
> > > IIRC I tried to do it in FRE once, but failed.
> > 
> > Yeah, FRE isn't too happy about VN_TOP in the lattice (which is what
> > you'd use here).  I also have/had patches trying to make it (more) happy
> > about VN_TOP but it still broke stuff like for example tail-merging...
> > 
> > Need to dissect PRE from tail-merging again, and also possibly PRE
> > from value-numbering (run eliminate() before doing PRE).
> Couldn't this be easily modeled as redundant load elimination?  Pretend the
> clobber is a read, which would make the read of b.x redundant.
> 
> I haven't prototyped it, but that'd be the first approach I'd try.

Sure, but that only works for must-be-uninit reads.  The question is
also what to replace the redundant reads with, esp. if you consider
the read could be of aggregate type.  Obvious choices would be
default-defs for regiser type reads and clobbers for aggregate typed
ones (but then clobbers are not valid in call argument context for 
example).  Another choice would be {} (or zero for register types),
same issue for call arg context.

For maybe-uninit reads you could model it as load hoisting -- you'd
hoist towards may-defs and must-uninit points and insert appropriate
PHIs -- again difficult for the aggregate case.

But yes, currently FRE/PRE treat uninitialized as varying, that's 
something we can improve on.

Richard.

Re: [gomp4] add -finform-parallelism

[Thomas Schwinge]

Hi Cesar!

On Mon, 20 Feb 2017 20:42:59 -0800, Cesar Philippidis  
wrote:
> This patch introduces a new -finform-parallelism flag to report any
> detected parallelism encountered by the compiler. Initially, it's being
> used to report how oaccdevlow partitions OpenACC loops. Currently, if
> you want to extract this information, you need to compile the program
> with -fdump-tree-oaccdevlow, then scan the tree dump for lines marked
> Loop and decode the decimal bitmask that represents the parallelism.
> This patch makes this process more user friendly by utilizing inform
> messages to highlight the directives inside the source code, and clearly
> print out the associated parallelism. E.g. given
> 
>   !$acc parallel loop
>   do i = ...
> !$acc parallel loop
>  do j = ...
> 
> -finform-parallelism reports

(Actually that should report that an OpenACC parallel construct nested in
another OpenACC parallel construct is not yet supported?)

>   inform-parallelism.f90: In function ‘MAIN__._omp_fn.0’:
>   inform-parallelism.f90:10:0: note: ACC LOOP GANG
>  !$acc parallel loop
> 
>   inform-parallelism.f90:12:0: note: ACC LOOP WORKER VECTOR
> !$acc loop

Thanks!


> Unfortunately, because this oaccdevlow runs so late, the offloaded
> function name doesn't match the one specified by the user.

It's still useful.  We can later look into how to preserve the "original
name".


> While working on this, I noticed that the fortran FE wasn't recording
> the location of combined loop directives properly, so I fixed that bug.

That's a separate bug fix, please.


> I also removed an unused variable inside trans-openmp.c.

Also a separate bug fix, please.  In
<87sho0c12q.fsf@euler.schwinge.homeip.net">http://mid.mail-archive.com/87sho0c12q.fsf@euler.schwinge.homeip.net>, I
asked you to fix that one.


> This patch still isn't complete because I found a similar bug in the c++
> FE. Thomas, before I fix that bug

Again, a separate bug fix, please.


> do you think this patch is worth
> pursuing for gomp-4_0-branch or maybe even trunk in general?

Definitely!

> Ideally, we
> can extend these diagnostics to report any detected loops inside kernels
> regions.

Right.

> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi

> +@item -finform-parallelism
> +@opindex finform-parallelism
> +Report any parallelism detected by the compiler.  Inside OpenACC
> +offloaded regions, this includes the gang, worker and vector level
> +parallelism associated with any @code{ACC LOOP}s.  This option is disabled
> +by default.

I know Fortran likes to use upper case, but the generic GCC code uses
lower-case names.

> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c

> +/* Provide diagnostics on OpenACC loops LOOP, its siblings and its
> +   children.  */
> +
> +static void
> +inform_oacc_loop (oacc_loop *loop)
> +{
> +  const char *seq = loop->mask == 0 ? " SEQ" : "";
> +  const char *gang = loop->mask & GOMP_DIM_MASK (GOMP_DIM_GANG)
> +? " GANG" : "";
> +  const char *worker = loop->mask & GOMP_DIM_MASK (GOMP_DIM_WORKER)
> +? " WORKER" : "";
> +  const char *vector = loop->mask & GOMP_DIM_MASK (GOMP_DIM_VECTOR)
> +? " VECTOR" : "";
> +
> +  inform (loop->loc, "ACC LOOP%s%s%s%s", seq, gang, worker, vector);

Likewise.

Per
<8737f68y3r.fsf@euler.schwinge.homeip.net">http://mid.mail-archive.com/8737f68y3r.fsf@euler.schwinge.homeip.net>
I'm suggesting this to be done a bit differently: instead of "inform",
this would then use the appropriate "-fopt-info-note-omp" option group
output.


If that's not yet there, possibly there could be some new flag added for
"-fopt-info" to display the "rich" location, which will also print the
original source code?


Grüße
 Thomas