Re: [patch v2] Get rid of stack trampolines for nested functions (1/4)
On Mär 28 2017, Eric Botcazou wrote: >> That needs to use ptr_mode, not Pmode. > > I don't think so, the whole computation is in Pmode. Could you try something > similar to what is done in the 'else' arm of the big surrounding conditional? Thanks, this gets me further, the ada library now compiles with -mabi=ilp32. But I still see some ICEs while running the testsuite, which I haven't investigated yet. And the original comment before this hunk doesn't make sense at this point. Andreas. diff --git a/gcc/calls.c b/gcc/calls.c index 61caf4ca75..c92e35ea5a 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -206,6 +206,9 @@ prepare_call_address (tree fndecl_or_type, rtx funexp, rtx static_chain_value, DECL_STATIC_CHAIN (fndecl_or_type) = 1; rtx chain = targetm.calls.static_chain (fndecl_or_type, false); + if (GET_MODE (funexp) != Pmode) + funexp = convert_memory_address (Pmode, funexp); + /* Avoid long live ranges around function calls. */ funexp = copy_to_mode_reg (Pmode, funexp); -- 2.12.2 -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
[PATCH][RFC] Fix P1 PR77498
After quite some pondering over this and other related bugs I propose the following for GCC 7 which tames down PRE a bit (back to levels of GCC 6). Technically it's the wrong place to fix this, we do have measures in place during elimination but they are not in effect at -O2. For GCC 8 I'd like to be more aggressive there but that would require to enable predictive commoning at -O2 (with some limits to its unrolling) to not lose optimization opportunities. The other option is to ignore this issue and postpone the solution to GCC 8. Bootstrapped / tested on x86_64-unknown-linux-gnu. Any preference? Thanks, Richard. 2017-03-29 Richard Biener PR tree-optimization/77498 * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications to non-constants over backedges. * gfortran.dg/pr77498.f: New testcase. Index: gcc/tree-ssa-pre.c === *** gcc/tree-ssa-pre.c (revision 246026) --- gcc/tree-ssa-pre.c (working copy) *** phi_translate_1 (pre_expr expr, bitmap_s *** 1468,1477 leader for it. */ if (constant->kind != CONSTANT) { ! unsigned value_id = get_expr_value_id (constant); ! constant = find_leader_in_sets (value_id, set1, set2); ! if (constant) ! return constant; } else return constant; --- 1468,1487 leader for it. */ if (constant->kind != CONSTANT) { ! /* Do not allow simplifications to non-constants over ! backedges as this will likely result in a loop PHI node ! to be inserted and increased register pressure. ! See PR77498 - this avoids doing predcoms work in ! a less efficient way. */ ! if (find_edge (pred, phiblock)->flags & EDGE_DFS_BACK) ! ; ! else ! { ! unsigned value_id = get_expr_value_id (constant); ! constant = find_leader_in_sets (value_id, set1, set2); ! if (constant) ! return constant; ! } } else return constant; Index: gcc/testsuite/gfortran.dg/pr77498.f === --- gcc/testsuite/gfortran.dg/pr77498.f (nonexistent) +++ gcc/testsuite/gfortran.dg/pr77498.f (working copy) @@ -0,0 +1,36 @@ +! { dg-do compile } +! { dg-options "-O2 -ffast-math -fdump-tree-pre" } + + subroutine foo(U,V,R,N,A) + integer N + real*8 U(N,N,N),V(N,N,N),R(N,N,N),A(0:3) + integer I3, I2, I1 +C + do I3=2,N-1 + do I2=2,N-1 +do I1=2,N-1 + R(I1,I2,I3)=V(I1,I2,I3) + * -A(0)*( U(I1, I2, I3 ) ) + * -A(1)*( U(I1-1,I2, I3 ) + U(I1+1,I2, I3 ) + * + U(I1, I2-1,I3 ) + U(I1, I2+1,I3 ) + * + U(I1, I2, I3-1) + U(I1, I2, I3+1) ) + * -A(2)*( U(I1-1,I2-1,I3 ) + U(I1+1,I2-1,I3 ) + * + U(I1-1,I2+1,I3 ) + U(I1+1,I2+1,I3 ) + * + U(I1, I2-1,I3-1) + U(I1, I2+1,I3-1) + * + U(I1, I2-1,I3+1) + U(I1, I2+1,I3+1) + * + U(I1-1,I2, I3-1) + U(I1-1,I2, I3+1) + * + U(I1+1,I2, I3-1) + U(I1+1,I2, I3+1) ) + * -A(3)*( U(I1-1,I2-1,I3-1) + U(I1+1,I2-1,I3-1) + * + U(I1-1,I2+1,I3-1) + U(I1+1,I2+1,I3-1) + * + U(I1-1,I2-1,I3+1) + U(I1+1,I2-1,I3+1) + * + U(I1-1,I2+1,I3+1) + U(I1+1,I2+1,I3+1) ) +enddo + enddo + enddo + return + end + +! PRE shouldn't do predictive commonings job here (and in a bad way) +! ??? It still does but not as bad as it could. Less prephitmps +! would be better, pcom does it with 6. +! { dg-final { scan-tree-dump-times "# prephitmp" 9 "pre" } }
[PATCH][AArch64] Fix missing optimization for CMP+AND
Hi all During combine GCC tries to merge CMP (with zero) and AND into a TST. However, in cases where an ANDS operand is not compatible, this was being missed. Adding a define_split where this operand was moved to a register seems to help out. For example for a test : int f (unsigned char *p) { return p[0] == 50 || p[0] == 52; } int g (unsigned char *p) { return (p[0] >> 4 & 0xfd) == 0; } we are now generating f: ldrbw0, [x0] mov w1, 253 sub w0, w0, #50 tst w0, w1 csetw0, eq ret .size f, .-f .align 2 .p2align 3,,7 .global g .type g, %function g: ldrbw1, [x0] mov w0, 13 tst w0, w1, lsr 4 csetw0, eq ret instead of f: ldrbw0, [x0] sub w0, w0, #50 and w0, w0, -3 and w0, w0, 255 cmp w0, 0 csetw0, eq ret .size f, .-f .align 2 .p2align 3,,7 .global g .type g, %function g: ldrbw0, [x0] lsr w0, w0, 4 and w0, w0, -3 cmp w0, 0 csetw0, eq ret Added this new test and checked for regressions on bootstrapped aarch64-none-linux-gnu Ok for stage 1? Thanks Sudi 2017-03-17 Kyrylo Tkachov Sudakshina Das * config/aarch64/aarch64.md (define_split for and3nr_compare0): Move non aarch64_logical_operand to a register. (define_split for and_3nr_compare0): Move non register immediate operand to a register. * config/aarch64/predicates.md (aarch64_mov_imm_operand): New. 2017-03-17 Kyrylo Tkachov Sudakshina Das * gcc.target/aarch64/tst_imm_split_1.c: New Test.diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 5adc5ed..5e5dbff 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -3881,6 +3881,22 @@ [(set_attr "type" "logics_reg,logics_imm")] ) +(define_split + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (and:GPI (match_operand:GPI 0 "register_operand") + (match_operand:GPI 1 "aarch64_mov_imm_operand")) + (const_int 0))) + (clobber (match_operand:SI 2 "register_operand"))] + "" + [(set (match_dup 2) (match_dup 1)) + (set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (and:GPI (match_dup 0) + (match_dup 2)) + (const_int 0)))] +) + (define_insn "*and3nr_compare0_zextract" [(set (reg:CC_NZ CC_REGNUM) (compare:CC_NZ @@ -3916,6 +3932,26 @@ [(set_attr "type" "logics_shift_imm")] ) +(define_split + [(set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (and:GPI (SHIFT:GPI + (match_operand:GPI 0 "register_operand") + (match_operand:QI 1 "aarch64_shift_imm_")) + (match_operand:GPI 2 "aarch64_mov_imm_operand")) + (const_int 0))) +(clobber (match_operand:SI 3 "register_operand"))] + "" + [(set (match_dup 3) (match_dup 2)) + (set (reg:CC_NZ CC_REGNUM) + (compare:CC_NZ + (and:GPI (SHIFT:GPI + (match_dup 0) + (match_dup 1)) + (match_dup 3)) + (const_int 0)))] +) + ;; --- ;; Shifts ;; --- diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md index e83d45b..ed62b8e 100644 --- a/gcc/config/aarch64/predicates.md +++ b/gcc/config/aarch64/predicates.md @@ -106,6 +106,10 @@ (ior (match_operand 0 "register_operand") (match_operand 0 "aarch64_logical_immediate"))) +(define_predicate "aarch64_mov_imm_operand" + (and (match_code "const_int") + (match_test "aarch64_move_imm (INTVAL (op), mode)"))) + (define_predicate "aarch64_logical_and_immediate" (and (match_code "const_int") (match_test "aarch64_and_bitmask_imm (INTVAL (op), mode)"))) diff --git a/gcc/testsuite/gcc.target/aarch64/tst_imm_split_1.c b/gcc/testsuite/gcc.target/aarch64/tst_imm_split_1.c new file mode 100644 index 000..33a2c0f --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/tst_imm_split_1.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +int +f (unsigned char *p) +{ + return p[0] == 50 || p[0] == 52; +} + +int +g (unsigned char *p) +{ + return (p[0] >> 4 & 0xfd) == 0; +} + +/* { dg-final { scan-assembler-not "and\\t\[xw\]\[0-9\]+, \[xw\]\[0-9\]+.*" } } */ +/* { dg-final { scan-assembler "tst\\t\[xw\]\[0-9\]+, \[xw\]\[0-9\]+" } } */ +/* { dg-final { scan-assembler "tst\\t\[xw\]\[0-9\]+, \[xw\]\[0-9\]+, lsr 4" } } */
[PATCH] [gcc, testsuite] Don't xfail on s390.
The attached patch removes the XFAIL in attr-alloc_size-11.c on s390. (PR 79356). https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79356 Untested. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany gcc/testsuite/ChangeLog-attr-alloc-size-11 PR testsuite/79356 * gcc.dg/attr-alloc_size-11.c: Don't xfail on s390. >From 7a11e38485db879b499099292d7bd29bf3085775 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Wed, 29 Mar 2017 11:21:07 +0100 Subject: [PATCH] [gcc, testsuite] Don't xfail on s390. --- gcc/testsuite/gcc.dg/attr-alloc_size-11.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.dg/attr-alloc_size-11.c b/gcc/testsuite/gcc.dg/attr-alloc_size-11.c index e5f2157..47584f5 100644 --- a/gcc/testsuite/gcc.dg/attr-alloc_size-11.c +++ b/gcc/testsuite/gcc.dg/attr-alloc_size-11.c @@ -47,7 +47,7 @@ typedef __SIZE_TYPE__size_t; /* The following tests fail because of missing range information. The xfail exclusions are PR79356. */ -TEST (signed char, SCHAR_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1 range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info for signed char" { xfail { ! { aarch64*-*-* arm*-*-* ia64-*-* mips*-*-* powerpc*-*-* sparc*-*-* s390x-*-* } } } } */ +TEST (signed char, SCHAR_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1 range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info for signed char" { xfail { ! { aarch64*-*-* arm*-*-* ia64-*-* mips*-*-* powerpc*-*-* sparc*-*-* s390*-*-* } } } } */ TEST (short, SHRT_MIN + 2, ALLOC_MAX); /* { dg-warning "argument 1 range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" "missing range info for short" { xfail { ! { aarch64*-*-* arm*-*-* ia64-*-* mips*-*-* powerpc*-*-* sparc*-*-* s390x-*-* } } } } */ TEST (int, INT_MIN + 2, ALLOC_MAX);/* { dg-warning "argument 1 range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" } */ TEST (int, -3, ALLOC_MAX); /* { dg-warning "argument 1 range \\\[13, \[0-9\]+\\\] exceeds maximum object size 12" } */ -- 2.3.0
[v3 PATCH] Adjust optional's pretty printer for LWG 2900.
Tested on Linux-x64. 2017-03-29 Ville Voutilainen Adjust optional's pretty printer for LWG 2900. * python/libstdcxx/v6/printers.py (StdExpOptionalPrinter::__init__): Look at the nested payload in case of non-experimental optional. diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py index 14025dd..a67b27a 100644 --- a/libstdc++-v3/python/libstdcxx/v6/printers.py +++ b/libstdc++-v3/python/libstdcxx/v6/printers.py @@ -1023,6 +1023,8 @@ class StdExpOptionalPrinter(SingleObjContainerPrinter): valtype = self._recognize (val.type.template_argument(0)) self.typename = re.sub('^std::(experimental::|)(fundamentals_v\d::|)(.*)', r'std::\1\3<%s>' % valtype, typename, 1) self.typename = strip_versioned_namespace(self.typename) +if not self.typename.startswith('std::experimental'): +val = val['_M_payload'] self.val = val contained_value = val['_M_payload'] if self.val['_M_engaged'] else None visualizer = gdb.default_visualizer (val['_M_payload'])
Re: [v3 PATCH] Adjust optional's pretty printer for LWG 2900.
On 29/03/17 14:51 +0300, Ville Voutilainen wrote: Tested on Linux-x64. 2017-03-29 Ville Voutilainen Adjust optional's pretty printer for LWG 2900. * python/libstdcxx/v6/printers.py (StdExpOptionalPrinter::__init__): Look at the nested payload in case of non-experimental optional. OK.
[PATCH, committed] PR80158 cleanup (alternate interpretations)
Hi, The fix for PR80158 is technically correct, but not "future-proof" in the sense that someday we may implement more than one alternate interpretation for a strength reduction candidate. This patch cleans that up. Bootstrapped on powerpc64le-unknown-linux-gnu with no regressions, committed. Thanks, Bill 2017-03-29 Bill Schmidt PR tree-optimization/80158 * gimple-ssa-strength-reduction.c (replace_mult_candidate): Handle possible future case of more than one alternate interpretation. (replace_rhs_if_not_dup): Likewise. (replace_one_candidate): Likewise. Index: gcc/gimple-ssa-strength-reduction.c === --- gcc/gimple-ssa-strength-reduction.c (revision 246554) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2086,11 +2086,15 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ tree lhs = gimple_assign_lhs (c->cand_stmt); gassign *copy_stmt = gimple_build_assign (lhs, basis_name); gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); gsi_replace (&gsi, copy_stmt, false); c->cand_stmt = copy_stmt; - if (c->next_interp) - lookup_cand (c->next_interp)->cand_stmt = copy_stmt; + while (cc->next_interp) + { + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = copy_stmt; + } if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = copy_stmt; } @@ -2116,12 +2120,16 @@ replace_mult_candidate (slsr_cand_t c, tree basis_ else { gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; gimple_assign_set_rhs_with_ops (&gsi, code, basis_name, bump_tree); update_stmt (gsi_stmt (gsi)); c->cand_stmt = gsi_stmt (gsi); - if (c->next_interp) - lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi); + while (cc->next_interp) + { + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = gsi_stmt (gsi); + } if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = gsi_stmt (gsi); } @@ -3406,11 +3414,15 @@ replace_rhs_if_not_dup (enum tree_code new_code, t || !operand_equal_p (new_rhs2, old_rhs1, 0 { gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; gimple_assign_set_rhs_with_ops (&gsi, new_code, new_rhs1, new_rhs2); update_stmt (gsi_stmt (gsi)); c->cand_stmt = gsi_stmt (gsi); - if (c->next_interp) - lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi); + while (cc->next_interp) + { + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = gsi_stmt (gsi); + } if (dump_file && (dump_flags & TDF_DETAILS)) return gsi_stmt (gsi); @@ -3514,11 +3526,15 @@ replace_one_candidate (slsr_cand_t c, unsigned i, || !operand_equal_p (rhs2, orig_rhs2, 0)) { gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; gimple_assign_set_rhs_with_ops (&gsi, MINUS_EXPR, basis_name, rhs2); update_stmt (gsi_stmt (gsi)); c->cand_stmt = gsi_stmt (gsi); - if (c->next_interp) - lookup_cand (c->next_interp)->cand_stmt = gsi_stmt (gsi); + while (cc->next_interp) + { + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = gsi_stmt (gsi); + } if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = gsi_stmt (gsi); @@ -3537,11 +3553,15 @@ replace_one_candidate (slsr_cand_t c, unsigned i, { gassign *copy_stmt = gimple_build_assign (lhs, basis_name); gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); + slsr_cand_t cc = c; gimple_set_location (copy_stmt, gimple_location (c->cand_stmt)); gsi_replace (&gsi, copy_stmt, false); c->cand_stmt = copy_stmt; - if (c->next_interp) - lookup_cand (c->next_interp)->cand_stmt = copy_stmt; + while (cc->next_interp) + { + cc = lookup_cand (cc->next_interp); + cc->cand_stmt = copy_stmt; + } if (dump_file && (dump_flags & TDF_DETAILS)) stmt_to_print = copy_stmt; @@ -3550,11 +3570,15 @@ replace_one_candidate (slsr_cand_t c, unsigned i, { gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt); gassign *cast_stmt = gimple_build_assign (lhs, NOP_EXPR, basis_name); + slsr_cand_t cc = c; gimple_set_location (cast_stmt, gimple_
Re: [patch v2] Get rid of stack trampolines for nested functions (1/4)
> Thanks, this gets me further, the ada library now compiles with > -mabi=ilp32. Great, I think that you can apply it as obvious then, it's code which is only exercised in Ada and on ILP32 64-bit platforms so the risk is very low... > But I still see some ICEs while running the testsuite, > which I haven't investigated yet. And the original comment before this > hunk doesn't make sense at this point. The hunk in the 'else' arm of the big surrounding conditional? -- Eric Botcazou
Re: [patch v2] Get rid of stack trampolines for nested functions (1/4)
On Mär 29 2017, Eric Botcazou wrote: >> But I still see some ICEs while running the testsuite, >> which I haven't investigated yet. And the original comment before this >> hunk doesn't make sense at this point. > > The hunk in the 'else' arm of the big surrounding conditional? Yes. It talks about SYMBOL_REF which it clearly isn't at this point. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH v2] S/390: Optimize atomic_compare_exchange and atomic_compare builtins.
On Mon, Mar 27, 2017 at 09:27:35PM +0100, Dominik Vogt wrote: > The attached patch optimizes the atomic_exchange and > atomic_compare patterns on s390 and s390x (mostly limited to > SImode and DImode). Among general optimizaation, the changes fix > most of the problems reported in PR 80080: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80080 > > Bootstrapped and regression tested on a zEC12 with s390 and s390x > biarch. New version of the patch after internal discussion. Bootstrapped and regression tested on s390 and s390x biarch on a zEC12. v2: * Clean up and correct comments. * Reformat. * Use force_reg. * Remove an experimental, not working splitter. * Use cstorecc4 more often. * Clean up code of the predicate. Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany gcc/ChangeLog * (s390_expand_cs_hqi): Removed. (s390_expand_cs, s390_expand_atomic_exchange_tdsi): New prototypes. (s390_cc_modes_compatible): Export. * config/s390/predicates.md ("memory_nosymref_operand"): New predicate for compare-and-swap. * config/s390/s390.c(s390_emit_compare_and_swap): Handle all integer modes. (s390_cc_modes_compatible): Remove static. (s390_expand_cs_hqi): Make static. (s390_expand_cs_tdsi): Generate an explicit compare before trying compare-and-swap, in some cases. (s390_expand_cs): Wrapper function. (s390_expand_atomic_exchange_tdsi): New backend specific expander for atomic_exchange. * config/s390/s390.md (define_peephole2): New peephole to help combining the load-and-test pattern with volatile memory. ("cstorecc4"): Deal with CCZmode too. ("sne", "sneccz1_ne", "sneccz1_eq"): Renamed and duplicated pattern. ("sneccz_ne", "sneccz_eq"): New. ("atomic_compare_and_swap"): Merge the patterns for small and large integers. Forbid symref memory operands. Move expander to s390.c. ("atomic_compare_and_swap_internal") ("*atomic_compare_and_swap_1") ("*atomic_compare_and_swap_2") ("*atomic_compare_and_swap_3"): Forbid symref memory operands. ("atomic_exchange"): Allow and implement all integer modes. gcc/testsuite/ChangeLog * gcc.target/s390/md/atomic_compare_exchange-1.c: New test. * gcc.target/s390/md/atomic_compare_exchange-1.inc: New test. * gcc.target/s390/md/atomic_exchange-1.inc: New test. >From 1ce8c987f227e3c6095980e52edec07835f6fad7 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Thu, 23 Feb 2017 17:23:11 +0100 Subject: [PATCH] S/390: Optimize atomic_compare_exchange and atomic_compare builtins. 1) Use the load-and-test instructions for atomic_exchange if the value is 0. 2) If IS_WEAK is true, compare the memory contents before a compare-and-swap and skip the CS instructions if the value is not the expected one. --- gcc/config/s390/predicates.md | 10 + gcc/config/s390/s390-protos.h | 5 +- gcc/config/s390/s390.c | 173 ++- gcc/config/s390/s390.md| 199 .../gcc.target/s390/md/atomic_compare_exchange-1.c | 84 ++ .../s390/md/atomic_compare_exchange-1.inc | 336 + .../gcc.target/s390/md/atomic_exchange-1.c | 309 +++ 7 files changed, 1050 insertions(+), 66 deletions(-) create mode 100644 gcc/testsuite/gcc.target/s390/md/atomic_compare_exchange-1.c create mode 100644 gcc/testsuite/gcc.target/s390/md/atomic_compare_exchange-1.inc create mode 100644 gcc/testsuite/gcc.target/s390/md/atomic_exchange-1.c diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md index 0c82efc..c902f96 100644 --- a/gcc/config/s390/predicates.md +++ b/gcc/config/s390/predicates.md @@ -67,6 +67,16 @@ return true; }) +;; Like memory_operand, but rejects symbol references. +(define_predicate "memory_nosymref_operand" + (match_operand 0 "memory_operand") +{ + if (SUBREG_P (op)) +op = XEXP (op, 0); + + return (GET_CODE (op) == MEM && !SYMBOL_REF_P (XEXP (op, 0))); +}) + ;; Return true if OP is a valid operand for the BRAS instruction. ;; Allow SYMBOL_REFs and @PLT stubs. diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h index 7f06a20..81644b9 100644 --- a/gcc/config/s390/s390-protos.h +++ b/gcc/config/s390/s390-protos.h @@ -81,6 +81,7 @@ extern bool s390_overlap_p (rtx, rtx, HOST_WIDE_INT); extern bool s390_offset_p (rtx, rtx, rtx); extern int tls_symbolic_operand (rtx); +extern machine_mode s390_cc_modes_compatible (machine_mode, machine_mode); extern bool s390_match_ccmode (rtx_insn *, machine_mode); extern machine_mode s390_tm_ccmode (rtx, rtx, bool); extern machine_mode s390_select_ccmode (enum rtx_code, rtx, rtx); @@ -112,8 +113,8 @@ extern void s390_expand_vec_strlen (rtx, rtx, rtx); extern void s390_expand_vec_movstr (rtx, rtx,
Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener wrote: > On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng wrote: >> Hi, >> This patch is to fix PR80153. As analyzed in the PR, root cause is >> tree_affine lacks >> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr + >> (unsigned)offset, >> even worse, it always returns the former expression in aff_combination_tree, >> which >> is wrong if the original expression has the latter form. The patch resolves >> the issue >> by always returning the latter form expression, i.e, always trying to >> generate folded >> expression. Also as analyzed in comment, I think this change won't result >> in substantial >> code gen difference. >> I also need to adjust get_computation_aff for test case >> gcc.dg/tree-ssa/reassoc-19.c. >> Well, I think the changed behavior is correct, but for case the original >> pointer candidate >> is chosen, it should be unnecessary to compute in uutype. Also this >> adjustment only >> generates (unsigned)(pointer + offset) which is generated by tree-affine.c. >> Bootstrap and test on x86_64 and AArch64. Is it OK? > Thanks for reviewing. > Hmm. What is the desired goal? To have all elts added have > comb->type as type? Then > the type passed to add_elt_to_tree is redundant with comb->type. It > looks like it > is always passed comb->type now. Yes, except pointer type comb->type, elts are converted to comb->type with this patch. The redundant type is removed in updated patch. > > ISTR from past work in this area that it was important for pointer > combinations to allow > both pointer and sizetype elts at least. Yes, It's still important to allow different types for pointer and offset in pointer type comb. I missed a pointer type check condition in the patch, fixed in updated patch. > > Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P case > elt is sizetype now, not of pointer type. As said above, we are > trying to maintain > both pointer and sizetype elts with like: > > if (scale == 1) > { > if (!expr) > { > if (POINTER_TYPE_P (TREE_TYPE (elt))) > return elt; > else > return fold_convert (type1, elt); > } > > where your earilier fold to type would result in not all cases handled the > same > (depending whether scale was -1 for example). IIUC, it doesn't matter. For comb->type being pointer type, the behavior remains the same. For comb->type being unsigned T, this elt is converted to ptr_offtype, rather than unsigned T, this doesn't matter because ptr_offtype and unsigned T are equal to each other, otherwise tree_to_aff_combination shouldn't distribute it as a single elt. Anyway, this is addressed in updated patch by checking pointer comb->type additionally. BTW, I think "scale==-1" case is a simple heuristic differentiating pointer_base and offset. > > Thus - shouldn't we simply drop the type argument (or rather the comb one? > that wide_int_ext_for_comb looks weird given we get a widest_int as input > and all the other wide_int_ext_for_comb calls around). > > And unconditionally convert to type, simplifying the rest of the code? As said, for pointer type comb, we need to keep current behavior; for other cases, unconditionally convert to comb->type is the goal. Bootstrap and test on x86_64 and AArch64. Is this version OK? Thanks, bin 2017-03-28 Bin Cheng PR tree-optimization/80153 * tree-affine.c (add_elt_to_tree): Remove parameter TYPE. Use type of parameter COMB. Convert elt to type of COMB it COMB is not of pointer type. (aff_combination_to_tree): Update calls to add_elt_to_tree. * tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types. (get_computation_aff): Use utype directly for original candidate. gcc/testsuite/ChangeLog 2017-03-28 Bin Cheng PR tree-optimization/80153 * gcc.c-torture/execute/pr80153.c: New. diff --git a/gcc/testsuite/gcc.c-torture/execute/pr80153.c b/gcc/testsuite/gcc.c-torture/execute/pr80153.c new file mode 100644 index 000..3eed578 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr80153.c @@ -0,0 +1,48 @@ +/* PR tree-optimization/80153 */ + +void check (int, int, int) __attribute__((noinline)); +void check (int c, int c2, int val) +{ + if (!val) { +__builtin_abort(); + } +} + +static const char *buf; +static int l, i; + +void _fputs(const char *str) __attribute__((noinline)); +void _fputs(const char *str) +{ + buf = str; + i = 0; + l = __builtin_strlen(buf); +} + +char _fgetc() __attribute__((noinline)); +char _fgetc() +{ + char val = buf[i]; + i++; + if (i > l) +return -1; + else +return val; +} + +static const char *string = "oops!\n"; + +int main(void) +{ + int i; + int c; + + _fputs(string); + + for (i = 0; i < __builtin_strlen(string); i++) { +c = _fgetc(); +check(c, string[i], c == string[i]); + } + + return 0; +} diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c index e620eea..
Re: [PR 77333] Fix fntypes of calls calling clones
Hi, On Thu, Mar 16, 2017 at 05:57:51PM +0100, Martin Jambor wrote: > Hi, > > On Mon, Mar 13, 2017 at 01:46:47PM +0100, Richard Biener wrote: > > On Fri, 10 Mar 2017, Martin Jambor wrote: > > > > > Hi, > > > > > > PR 77333 is a i686-windows target bug, which however has its root in > > > our general mechanism of adjusting gimple statements when redirecting > > > call graph edge. Basically, these three things trigger it: > > > > > > 1) IPA-CP figures out that the this parameter of a C++ class method is > > >unused and because the class is in an anonymous namespace, it can > > >be removed and all calls adjusted. That effectively changes a > > >normal method into a static method and so internally, its type > > >changes from METHOD_TYPE to FUNCTION_TYPE. > > > > > > 2) Since the fix of PR 57330, we do not update gimple_call_fntype to > > >match the new type, in fact we explicitely set it to the old, now > > >invalid, type (see redirect_call_stmt_to_callee in cgraph.c). > > > > > > 3) Function ix86_get_callcvt which decides on call ABI, ends with the > > >following condition: > > > > > > if (ret != 0 > > > || is_stdarg > > > || TREE_CODE (type) != METHOD_TYPE > > > || ix86_function_type_abi (type) != MS_ABI) > > >return IX86_CALLCVT_CDECL | ret; > > > > > > return IX86_CALLCVT_THISCALL; > > > > > >...and since now the callee is no longer a METHOD_TYPE but callers > > >still think that they are, leading to calling convention mismatches > > >and subsequent crashes. It took me quite a lot of time to come up > > >with a small testcase (reproducible using wine) but eventually I > > >managed. > > > > > > The fix is not to do 2) above, but doing so without re-introducing PR > > > 57330, of course. ... > > > > In general I am sympathetic with not doing any IPA propagation > > across call stmt signature incompatibilties. Of course we may > > be still too strict in those compatibility check... > > > > > So the alternative would be to re-check when doing the gimple > > > statement adjustment and if the types match, then set the correct new > > > gimple_fntype and if they don't... then we can either leave it be or > > > just run the same type transformation on it as we did on the callee, > > > though they would be bogus either way. That is implemented in the > > > attached patch. > > ... > After talking to Honza today, we decided to probably go this route and > use the patch doing the type conversion at acall-sites when necessary. > Honza promised to review the patch soon (he wants to figure out why > former_clone_of can be NULL, something I decided not to bother about > since at that time I thought the other approach was going to be > preferable). > and this is a slightly adjusted patch that is a result of what we talked about. I know that it is potentially disruptive change, so I have tested it with: - bootstrap and testing and LTO-bootstrap and testing on x86_64-linux, - bootstrap and testing on i686-linux, ppc64le-linux and ia64-linux - bootstrap on aarch64-linux (no testing because there is no dejagnu installed on gcc117.fsffrance.org), - testing on i686-w64-mingw32 on Linux+wine, and - testing on powerpc-aix is underway. OK for trunk (and subsequently to backport to gcc 6 and 5)? Thanks, Martin 2017-03-24 Martin Jambor PR ipa/77333 * cgraph.h (cgraph_build_function_type_skip_args): Declare. * cgraph.c (redirect_call_stmt_to_callee): Set gimple fntype so that it reflects the signature changes performed at the callee side. * cgraphclones.c (build_function_type_skip_args): Make public, renamed to cgraph_build_function_type_skip_args. (build_function_decl_skip_args): Adjust call to the above function. testsuite/ * g++.dg/ipa/pr77333.C: New test. --- gcc/cgraph.c | 17 +- gcc/cgraph.h | 2 ++ gcc/cgraphclones.c | 9 +++--- gcc/testsuite/g++.dg/ipa/pr77333.C | 65 ++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr77333.C diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 839388496ee..92ae0910c60 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1424,8 +1424,23 @@ cgraph_edge::redirect_call_stmt_to_callee (void) if (skip_bounds) new_stmt = chkp_copy_call_skip_bounds (new_stmt); + tree old_fntype = gimple_call_fntype (e->call_stmt); gimple_call_set_fndecl (new_stmt, e->callee->decl); - gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt)); + cgraph_node *origin = e->callee; + while (origin->clone_of) + origin = origin->clone_of; + + if ((origin->former_clone_of + && old_fntype == TREE_TYPE (origin->former_clone_of)) + || old_fntype == TREE_TYPE (origin->decl)) + gimple_call_set_fnty
Re: [PATCH][RFC] Fix P1 PR77498
On Wed, Mar 29, 2017 at 11:05 AM, Richard Biener wrote: > > After quite some pondering over this and other related bugs I propose > the following for GCC 7 which tames down PRE a bit (back to levels > of GCC 6). Technically it's the wrong place to fix this, we do > have measures in place during elimination but they are not in effect > at -O2. For GCC 8 I'd like to be more aggressive there but that > would require to enable predictive commoning at -O2 (with some > limits to its unrolling) to not lose optimization opportunities. > > The other option is to ignore this issue and postpone the solution > to GCC 8. > > Bootstrapped / tested on x86_64-unknown-linux-gnu. > > Any preference? +1 enabling predcom at O2 for this kind of transformation, remember I once suggested that. Thanks, bin > > Thanks, > Richard. > > 2017-03-29 Richard Biener > > PR tree-optimization/77498 > * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications > to non-constants over backedges. > > * gfortran.dg/pr77498.f: New testcase. > > Index: gcc/tree-ssa-pre.c > === > *** gcc/tree-ssa-pre.c (revision 246026) > --- gcc/tree-ssa-pre.c (working copy) > *** phi_translate_1 (pre_expr expr, bitmap_s > *** 1468,1477 >leader for it. */ > if (constant->kind != CONSTANT) > { > ! unsigned value_id = get_expr_value_id (constant); > ! constant = find_leader_in_sets (value_id, set1, set2); > ! if (constant) > ! return constant; > } > else > return constant; > --- 1468,1487 >leader for it. */ > if (constant->kind != CONSTANT) > { > ! /* Do not allow simplifications to non-constants over > ! backedges as this will likely result in a loop PHI node > ! to be inserted and increased register pressure. > ! See PR77498 - this avoids doing predcoms work in > ! a less efficient way. */ > ! if (find_edge (pred, phiblock)->flags & EDGE_DFS_BACK) > ! ; > ! else > ! { > ! unsigned value_id = get_expr_value_id (constant); > ! constant = find_leader_in_sets (value_id, set1, set2); > ! if (constant) > ! return constant; > ! } > } > else > return constant; > Index: gcc/testsuite/gfortran.dg/pr77498.f > === > --- gcc/testsuite/gfortran.dg/pr77498.f (nonexistent) > +++ gcc/testsuite/gfortran.dg/pr77498.f (working copy) > @@ -0,0 +1,36 @@ > +! { dg-do compile } > +! { dg-options "-O2 -ffast-math -fdump-tree-pre" } > + > + subroutine foo(U,V,R,N,A) > + integer N > + real*8 U(N,N,N),V(N,N,N),R(N,N,N),A(0:3) > + integer I3, I2, I1 > +C > + do I3=2,N-1 > + do I2=2,N-1 > +do I1=2,N-1 > + R(I1,I2,I3)=V(I1,I2,I3) > + * -A(0)*( U(I1, I2, I3 ) ) > + * -A(1)*( U(I1-1,I2, I3 ) + U(I1+1,I2, I3 ) > + * + U(I1, I2-1,I3 ) + U(I1, I2+1,I3 ) > + * + U(I1, I2, I3-1) + U(I1, I2, I3+1) ) > + * -A(2)*( U(I1-1,I2-1,I3 ) + U(I1+1,I2-1,I3 ) > + * + U(I1-1,I2+1,I3 ) + U(I1+1,I2+1,I3 ) > + * + U(I1, I2-1,I3-1) + U(I1, I2+1,I3-1) > + * + U(I1, I2-1,I3+1) + U(I1, I2+1,I3+1) > + * + U(I1-1,I2, I3-1) + U(I1-1,I2, I3+1) > + * + U(I1+1,I2, I3-1) + U(I1+1,I2, I3+1) ) > + * -A(3)*( U(I1-1,I2-1,I3-1) + U(I1+1,I2-1,I3-1) > + * + U(I1-1,I2+1,I3-1) + U(I1+1,I2+1,I3-1) > + * + U(I1-1,I2-1,I3+1) + U(I1+1,I2-1,I3+1) > + * + U(I1-1,I2+1,I3+1) + U(I1+1,I2+1,I3+1) ) > +enddo > + enddo > + enddo > + return > + end > + > +! PRE shouldn't do predictive commonings job here (and in a bad way) > +! ??? It still does but not as bad as it could. Less prephitmps > +! would be better, pcom does it with 6. > +! { dg-final { scan-tree-dump-times "# prephitmp" 9 "pre" } }
C++ PATCH for c++/80095, ICE with this pointer in NSDMI
Here we have a reference initialization with NSDMI and *this. We are crashing because a PLACEHOLDER_EXPR crept into the gimplifier. This happens since r218653 where set_up_extended_ref_temp was changed to use split_nonconstant_init. As a consequence, cp_gimplify_init_expr might now be receiving D.2051.p = (void *) & instead of D.2051 = {.p = (void *) &} where the RHS was a CONSTRUCTOR. It no longer is, so replace_placeholders is not called anymore. It occurred to me that we should use the same check as in store_init_value (i.e. check that the object to be used in the substitution is a class), but given what split_nonconstant_init might produce, handle COMPONENT_REFs specially. Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? 2017-03-29 Marek Polacek PR c++/80095 - ICE with this pointer in NSDMI. * cp-gimplify.c (cp_gimplify_init_expr): Call replace_placeholders when TO is a class. * g++.dg/cpp1y/nsdmi-aggr8.C: New test. diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c index 354ae1a..e530daf 100644 --- gcc/cp/cp-gimplify.c +++ gcc/cp/cp-gimplify.c @@ -496,7 +496,16 @@ cp_gimplify_init_expr (tree *expr_p) TREE_TYPE (from) = void_type_node; } - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) + /* split_nonconstant_init might've produced something like +D.2051.p = (void *) & +in which case we want to substitute the placeholder with +D.2051. */ + tree op0 = to; + while (TREE_CODE (op0) == COMPONENT_REF) + op0 = TREE_OPERAND (op0, 0); + tree type = TREE_TYPE (op0); + + if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) /* Handle aggregate NSDMI. */ replace_placeholders (sub, to); diff --git gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C index e69de29..8c99ffb 100644 --- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C +++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr8.C @@ -0,0 +1,16 @@ +// PR c++/80095 +// { dg-do compile { target c++14 } } + +struct A +{ + void* p = this; +}; + +void +foo () +{ + const A& a = A{}; + A&& a2 = A{}; + const A& a3{}; + A&& a4{}; +} Marek
[PATCH] PR80101: Fix ICE in store_data_bypass_p
This problem reports an assertion error when certain rtl expressions which are not eligible as producers or consumers of a store bypass optimization are passed as arguments to the store_data_bypass_p function. The proposed patch returns false from store_data_bypass_p rather than terminating with an assertion error. False indicates that the passed arguments are not eligible for the store bypass scheduling optimization. The patch has been boostrapped without regressions on powerpc64le-unknown-linux-gnu. Is this ok for the trunk? gcc/ChangeLog: 2017-03-29 Kelvin Nilsen PR target/80101 * recog.c (store_data_bypass_p): Rather than terminate with assertion error, return false if either function argument is not a single_set or a PARALLEL with SETs inside. gcc/testsuite/ChangeLog: 2017-03-29 Kelvin Nilsen PR target/80101 * gcc.target/powerpc/pr80101-1.c: New test. Index: gcc/recog.c === --- gcc/recog.c (revision 246469) +++ gcc/recog.c (working copy) @@ -3663,9 +3663,12 @@ peephole2_optimize (void) /* Common predicates for use with define_bypass. */ -/* True if the dependency between OUT_INSN and IN_INSN is on the store - data not the address operand(s) of the store. IN_INSN and OUT_INSN - must be either a single_set or a PARALLEL with SETs inside. */ +/* Returns true if the dependency between OUT_INSN and IN_INSN is on + the stored data, false if there is no dependency. Note that a + consumer instruction that loads only the address (rather than the + value) stored by a producer instruction does not represent a + dependency. If IN_INSN or OUT_INSN are not a single_set or a + PARALLEL with SETs inside, this function returns false. */ int store_data_bypass_p (rtx_insn *out_insn, rtx_insn *in_insn) @@ -3701,7 +3704,8 @@ store_data_bypass_p (rtx_insn *out_insn, rtx_insn if (GET_CODE (out_exp) == CLOBBER) continue; -gcc_assert (GET_CODE (out_exp) == SET); + if (GET_CODE (out_exp) != SET) + return false; if (reg_mentioned_p (SET_DEST (out_exp), SET_DEST (in_set))) return false; @@ -3711,7 +3715,8 @@ store_data_bypass_p (rtx_insn *out_insn, rtx_insn else { in_pat = PATTERN (in_insn); - gcc_assert (GET_CODE (in_pat) == PARALLEL); + if (GET_CODE (in_pat) != PARALLEL) + return false; for (i = 0; i < XVECLEN (in_pat, 0); i++) { @@ -3720,7 +3725,8 @@ store_data_bypass_p (rtx_insn *out_insn, rtx_insn if (GET_CODE (in_exp) == CLOBBER) continue; - gcc_assert (GET_CODE (in_exp) == SET); + if (GET_CODE (in_exp) != SET) + return false; if (!MEM_P (SET_DEST (in_exp))) return false; @@ -3734,7 +3740,8 @@ store_data_bypass_p (rtx_insn *out_insn, rtx_insn else { out_pat = PATTERN (out_insn); - gcc_assert (GET_CODE (out_pat) == PARALLEL); + if (GET_CODE (out_pat) != PARALLEL) + return false; for (j = 0; j < XVECLEN (out_pat, 0); j++) { @@ -3743,7 +3750,8 @@ store_data_bypass_p (rtx_insn *out_insn, rtx_insn if (GET_CODE (out_exp) == CLOBBER) continue; - gcc_assert (GET_CODE (out_exp) == SET); + if (GET_CODE (out_exp) != SET) + return false; if (reg_mentioned_p (SET_DEST (out_exp), SET_DEST (in_exp))) return false; Index: gcc/testsuite/gcc.target/powerpc/pr80101-1.c === --- gcc/testsuite/gcc.target/powerpc/pr80101-1.c(revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr80101-1.c(working copy) @@ -0,0 +1,22 @@ +/* { dg-do compile { target { powerpc*-*-* } } } */ +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power6" } } */ +/* { dg-require-effective-target dfp_hw } */ +/* { dg-options "-mcpu=power6 -mno-sched-epilog -Ofast" } */ + +/* Prior to resolving PR 80101, this test case resulted in an internal + compiler error. The role of this test program is to assure that + dejagnu's "test for excess errors" does not find any. */ + +int b; + +void e (); + +int c () +{ + struct + { +int a[b]; + } d; + if (d.a[0]) +e (); +}
C PATCH for c/79730, ICE on invalid with DECL_HARD_REGISTER
DECL_HARD_REGISTER only expects a VAR_DECL, so check for that first (C++'s make_rtl_for_nonlocal_decl does that, too). Bootstrapped/regtested on x86_64-linux, ok for trunk? 2017-03-29 Marek Polacek PR c/79730 * c-decl.c (finish_decl): Check VAR_P. * gcc.dg/pr79730.c: New test. diff --git gcc/c/c-decl.c gcc/c/c-decl.c index a0dc5bc..53c390c 100644 --- gcc/c/c-decl.c +++ gcc/c/c-decl.c @@ -5066,7 +5066,7 @@ finish_decl (tree decl, location_t init_loc, tree init, when a tentative file-scope definition is seen. But at end of compilation, do output code for them. */ DECL_DEFER_OUTPUT (decl) = 1; - if (asmspec && C_DECL_REGISTER (decl)) + if (asmspec && VAR_P (decl) && C_DECL_REGISTER (decl)) DECL_HARD_REGISTER (decl) = 1; rest_of_decl_compilation (decl, true, 0); } diff --git gcc/testsuite/gcc.dg/pr79730.c gcc/testsuite/gcc.dg/pr79730.c index e69de29..497823a 100644 --- gcc/testsuite/gcc.dg/pr79730.c +++ gcc/testsuite/gcc.dg/pr79730.c @@ -0,0 +1,6 @@ +/* PR c/79730 */ +/* { dg-do compile } */ +/* { dg-options "-std=gnu11" } */ + +register int x() asm (""); /* { dg-error "invalid storage class" } */ +register float y() asm (""); /* { dg-error "invalid storage class" } */ Marek
[PATCH] combine: Fix PR80233
If combine has added an unconditional trap there will be a new basic block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK as the last_combined_insn, but then it tries to take the DF_INSN_LUID of that and that dereferences a NULL pointer (since such a note is not an INSN_P). This fixes it by not taking non-insns as last_combined_insn. Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Segher 2017-03-29 Segher Boessenkool PR rtl-optimization/80233 * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns as last_combined_insn. Do not test for BARRIER_P separately. gcc/testsuite/ PR rtl-optimization/80233 * gcc.c-torture/compile/pr80233.c: New testcase. --- gcc/combine.c | 4 ++-- gcc/testsuite/gcc.c-torture/compile/pr80233.c | 22 ++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr80233.c diff --git a/gcc/combine.c b/gcc/combine.c index 87e9670..88ac475 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -1250,10 +1250,10 @@ combine_instructions (rtx_insn *f, unsigned int nregs) continue; while (last_combined_insn -&& last_combined_insn->deleted ()) +&& (!NONDEBUG_INSN_P (last_combined_insn) +|| last_combined_insn->deleted ())) last_combined_insn = PREV_INSN (last_combined_insn); if (last_combined_insn == NULL_RTX - || BARRIER_P (last_combined_insn) || BLOCK_FOR_INSN (last_combined_insn) != this_basic_block || DF_INSN_LUID (last_combined_insn) <= DF_INSN_LUID (insn)) last_combined_insn = insn; diff --git a/gcc/testsuite/gcc.c-torture/compile/pr80233.c b/gcc/testsuite/gcc.c-torture/compile/pr80233.c new file mode 100644 index 000..eb9b4a4 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr80233.c @@ -0,0 +1,22 @@ +/* PR rtl-optimization/80233 */ + +int xg; + +void +t4 (int o9) +{ + int it; + + if (o9 == 0) +{ + int fx; + + xg *= it; + if (xg == 0) +it /= 0; + + fx = (it != 0) ? (xg < 0) : (xg / o9); + if (fx != 0) +xg = 0; +} +} -- 1.9.3
New French PO file for 'gcc' (version 7.1-b20170226)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the French team of translators. The file is available at: http://translationproject.org/latest/gcc/fr.po (This file, 'gcc-7.1-b20170226.fr.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: C++ PATCH for c++/80095, ICE with this pointer in NSDMI
On Wed, Mar 29, 2017 at 12:38 PM, Marek Polacek wrote: > Here we have a reference initialization with NSDMI and *this. We are crashing > because a PLACEHOLDER_EXPR crept into the gimplifier. > > This happens since r218653 where set_up_extended_ref_temp was changed to > use split_nonconstant_init. As a consequence, cp_gimplify_init_expr might > now be receiving > > D.2051.p = (void *) & > > instead of > > D.2051 = {.p = (void *) &} > > where the RHS was a CONSTRUCTOR. It no longer is, so replace_placeholders is > not called anymore. It occurred to me that we should use the same check as in > store_init_value (i.e. check that the object to be used in the substitution is > a class), but given what split_nonconstant_init might produce, handle > COMPONENT_REFs specially. > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? > > 2017-03-29 Marek Polacek > > PR c++/80095 - ICE with this pointer in NSDMI. > * cp-gimplify.c (cp_gimplify_init_expr): Call replace_placeholders > when TO is a class. > > * g++.dg/cpp1y/nsdmi-aggr8.C: New test. > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > index 354ae1a..e530daf 100644 > --- gcc/cp/cp-gimplify.c > +++ gcc/cp/cp-gimplify.c > @@ -496,7 +496,16 @@ cp_gimplify_init_expr (tree *expr_p) > TREE_TYPE (from) = void_type_node; > } > > - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) > + /* split_nonconstant_init might've produced something like > +D.2051.p = (void *) & > +in which case we want to substitute the placeholder with > +D.2051. */ > + tree op0 = to; > + while (TREE_CODE (op0) == COMPONENT_REF) > + op0 = TREE_OPERAND (op0, 0); > + tree type = TREE_TYPE (op0); > + > + if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) How about doing this checking in replace_placeholders, instead? That is, if the object isn't a (member of a) class, just return. Jason
Re: [PATCH][RFC] Fix P1 PR77498
On 03/29/2017 04:05 AM, Richard Biener wrote: After quite some pondering over this and other related bugs I propose the following for GCC 7 which tames down PRE a bit (back to levels of GCC 6). Technically it's the wrong place to fix this, we do have measures in place during elimination but they are not in effect at -O2. For GCC 8 I'd like to be more aggressive there but that would require to enable predictive commoning at -O2 (with some limits to its unrolling) to not lose optimization opportunities. The other option is to ignore this issue and postpone the solution to GCC 8. Bootstrapped / tested on x86_64-unknown-linux-gnu. Any preference? Thanks, Richard. 2017-03-29 Richard Biener PR tree-optimization/77498 * tree-ssa-pre.c (phi_translate_1): Do not allow simplifications to non-constants over backedges. * gfortran.dg/pr77498.f: New testcase. I've got a slight preference for this patch. If you had a good start on the real fix then I'd lean more towards postponing. jeff
Re: C PATCH for c/79730, ICE on invalid with DECL_HARD_REGISTER
On 03/29/2017 12:07 PM, Marek Polacek wrote: DECL_HARD_REGISTER only expects a VAR_DECL, so check for that first (C++'s make_rtl_for_nonlocal_decl does that, too). Bootstrapped/regtested on x86_64-linux, ok for trunk? 2017-03-29 Marek Polacek PR c/79730 * c-decl.c (finish_decl): Check VAR_P. * gcc.dg/pr79730.c: New test. OK. jeff
Re: [PATCH] For broken exception handling in GDB on AIX platform
On 03/26/2017 06:54 PM, David Edelsohn wrote: Nitish works for IBM and I can stand in as the contributor. The patch would be very helpful to work around an AIX exception handling problem. Because the patch is a generic option and not target-specific, it needs a build machinery maintainer or Global Reviewer to approve. Thanks. I totally missed it when it went by. Probably because it had GDB in the subject line :-) Found a copy of the thread and put it into my queue. jeff
[C++ PATCH] Disable VLA initialization? (PR sanitizer/79993)
Hi! GCC 4.8 and earlier didn't allow in C++ initialization of VLA and C doesn't allow it in any GCC release. This has changed when the support for C++1y VLA has been added, then reverted, but apparently only partially. The question is, do we want to support VLA initialization, if yes, with what exact semantics (in that case we need to fix up initialization from STRING_CST which is broken right now). The following patch is the variant that disables it (bootstrapped/regtested on x86_64-linux and i686-linux). Looking at what is emitted for initialization from non-STRING_CSTs, it seems that we consider UB if the VLA is shorter than the initializer array, and if it is longer, we somehow initialize the rest (dunno what exact C++ initialization kind, there are some testcases with ctors in the list below; for PODs zero initialization). So, if we want to keep it and just fix STRING_CST initialization, shall we emit memcpy from the array followed by whatever we emit right now for the excess elements (for initialization from STRING_CST, can it ever be anything but zero initialization? If not, then memset would do the job). 2017-03-29 Jakub Jelinek PR sanitizer/79993 * decl.c (check_array_initializer): Reject VLA initialization. * g++.dg/init/array24.C: Adjust for VLA initialization being forbidden. * g++.dg/opt/pr78201.C: Likewise. * g++.dg/ubsan/vla-1.C: Likewise. * g++.dg/ext/constexpr-vla1.C: Likewise. * g++.dg/ext/constexpr-vla2.C: Likewise. * g++.dg/ext/constexpr-vla3.C: Likewise. * g++.dg/ext/constexpr-vla4.C: Likewise. * g++.dg/cpp1y/pr63996.C: Likewise. * g++.dg/cpp1y/vla-initlist1.C: Likewise. * g++.dg/cpp1y/vla2.C: Likewise. * g++.dg/cpp1y/vla10.C: Likewise. --- gcc/cp/decl.c.jj2017-03-21 07:57:00.0 +0100 +++ gcc/cp/decl.c 2017-03-29 09:45:20.741094070 +0200 @@ -6148,12 +6148,16 @@ check_array_initializer (tree decl, tree error ("elements of array %q#T have incomplete type", type); return true; } - /* A compound literal can't have variable size. */ - if (init && !decl + /* It is not valid to initialize a VLA. */ + if (init && ((COMPLETE_TYPE_P (type) && !TREE_CONSTANT (TYPE_SIZE (type))) || !TREE_CONSTANT (TYPE_SIZE (element_type { - error ("variable-sized compound literal"); + if (decl) + error_at (DECL_SOURCE_LOCATION (decl), + "variable-sized object %qD may not be initialized", decl); + else + error ("variable-sized compound literal"); return true; } return false; --- gcc/testsuite/g++.dg/init/array24.C.jj 2016-04-14 21:17:01.0 +0200 +++ gcc/testsuite/g++.dg/init/array24.C 2017-03-29 15:20:03.593188437 +0200 @@ -3,5 +3,5 @@ void foo(int i) { - int x[][i] = { 0 }; -} + int x[][i] = { 0 }; // { dg-error "variable-sized object .x. may not be initialized" } +} // { dg-error "storage size of .x. isn.t known" "" { target *-*-* } .-1 } --- gcc/testsuite/g++.dg/opt/pr78201.C.jj 2016-11-17 18:08:39.0 +0100 +++ gcc/testsuite/g++.dg/opt/pr78201.C 2017-03-29 14:56:08.673071231 +0200 @@ -8,6 +8,6 @@ long e; void foo () { - char a[e] = ""; + char a[e] = ""; // { dg-error "variable-sized object 'a' may not be initialized" } c && c->d(); } --- gcc/testsuite/g++.dg/ubsan/vla-1.C.jj 2016-04-14 21:17:01.0 +0200 +++ gcc/testsuite/g++.dg/ubsan/vla-1.C 2017-03-29 14:59:31.477405607 +0200 @@ -1,9 +1,8 @@ -// { dg-do run } +// { dg-do compile } // { dg-options "-Wno-vla -fsanitize=undefined" } -// { dg-output "index 1 out of bounds" } void f(int i) { - int ar[i] = { 42, 24 }; + int ar[i] = { 42, 24 }; // { dg-error "variable-sized object .ar. may not be initialized" } } int main() --- gcc/testsuite/g++.dg/ext/constexpr-vla2.C.jj2016-04-04 12:28:40.0 +0200 +++ gcc/testsuite/g++.dg/ext/constexpr-vla2.C 2017-03-29 14:52:55.648624577 +0200 @@ -4,7 +4,7 @@ constexpr int fn_bad (int n) { - __extension__ int a [n] = { 0 }; + __extension__ int a [n] = { 0 }; // { dg-error "variable-sized object .a. may not be initialized" } int z = a [0] + (n ? fn_bad (n - 1) : 0); return z; } @@ -12,10 +12,10 @@ fn_bad (int n) constexpr int fn_ok (int n) { - __extension__ int a [n] = { 0 }; + __extension__ int a [n] = { 0 }; // { dg-error "variable-sized object .a. may not be initialized" } int z = a [0] + (n > 1 ? fn_ok (n - 1) : 0); return z; } -constexpr int i1 = fn_ok (3); -constexpr int i2 = fn_bad (3); // { dg-error "array subscript" } +constexpr int i1 = fn_ok (3); // { dg-error "accessing uninitialized array element" } +constexpr int i2 = fn_bad (3); // { dg-error "accessing uninitialized array element" } --- gcc/testsuite/g++.dg/ext/constexpr-vla3.C.jj2016-04-04 12:28:40.0 +0200 +++ gcc/testsuite/g
Re: [PATCH] For broken exception handling in GDB on AIX platform
On 03/26/2017 06:05 PM, Joel Brobecker wrote: Hello, I got some review comment from Bernhard Reutner-Fischer, and I have updated the patch accordingly. This patch is for bug opened here:https://sourceware.org/bugzilla/show_bug.cgi?id=21187 This patch has been identified as one of the desirable patches to have for the GDB 8.0 release, for which we are hoping to create the branch ASAP. Without this patch, it would be difficult for users on AIX to build a functional debugger. Would it be possible to help Nitish through the review and approval process? Here is a ChangeLog entry: * configure.ac: Add support for --disable-staticlib. * configure: Regenerate. Can someone review the patch, please? Isn't this just papering over the problem by just disabling static linking rather than digging into why EH is not working with static linking on AIX? It'd be different if there was some fundamental reason why EH could not work with static linking on AIX. Am I missing something? Jeff
[PATCH] Fix dwarf2out ICE with C++17 inline static data members with redundant redeclaration (PR debug/80234)
Hi! When a C++17 inline static data member has a redundant out-of-class deprecated redeclaration, we can end up with 2 DW_TAG_variable in DW_TAG_compile_unit, one DW_AT_declaration and one with DW_AT_specification pointing to it (the latter emitted for the redeclaration), before gen_member_die can do its job. In there we want to move the declaration DIE into the class and have a CU child DW_TAG_variable that has DW_AT_specification pointing to it. But in this case we've put the DIE with DW_AT_specification into the hash table and gen_member_die ICEs in splice_child_die. The following patch handles that case gracefully, by moving the DW_AT_declaration DIE into the class instead of trying to move the DW_AT_specification one, and by making sure we don't create yet another DIE with DW_AT_specification because we already have one. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2017-03-29 Jakub Jelinek PR debug/80234 * dwarf2out.c (gen_member_die): Handle C++17 inline static data members with redundant out-of-class redeclaration. * g++.dg/debug/dwarf2/pr80234-1.C: New test. * g++.dg/debug/dwarf2/pr80234-2.C: New test. --- gcc/dwarf2out.c.jj 2017-03-24 14:17:59.0 +0100 +++ gcc/dwarf2out.c 2017-03-29 17:19:08.513497649 +0200 @@ -24085,6 +24085,10 @@ gen_member_die (tree type, dw_die_ref co for (member = TYPE_FIELDS (type); member; member = DECL_CHAIN (member)) { struct vlr_context vlr_ctx = { type, NULL_TREE }; + bool static_inline_p + = (TREE_STATIC (member) + && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline) + != -1)); /* If we thought we were generating minimal debug info for TYPE and then changed our minds, some of the member declarations @@ -24096,9 +24100,33 @@ gen_member_die (tree type, dw_die_ref co { /* Handle inline static data members, which only have in-class declarations. */ + dw_die_ref ref = NULL; + if (child->die_tag == DW_TAG_variable + && child->die_parent == comp_unit_die ()) + { + ref = get_AT_ref (child, DW_AT_specification); + /* For C++17 inline static data members followed by redundant +out of class redeclaration, we might get here with +child being the DIE created for the out of class +redeclaration and with its DW_AT_specification being +the DIE created for in-class definition. We want to +reparent the latter, and don't want to create another +DIE with DW_AT_specification in that case, because +we already have one. */ + if (ref + && static_inline_p + && ref->die_tag == DW_TAG_variable + && ref->die_parent == comp_unit_die () + && get_AT (ref, DW_AT_specification) == NULL) + { + child = ref; + ref = NULL; + static_inline_p = false; + } + } if (child->die_tag == DW_TAG_variable && child->die_parent == comp_unit_die () - && get_AT (child, DW_AT_specification) == NULL) + && ref == NULL) { reparent_child (child, context_die); if (dwarf_version < 5) @@ -24126,9 +24154,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/DW_TAG_variable through DW_AT_specification. */ - if (TREE_STATIC (member) - && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline) - != -1)) + if (static_inline_p) { int old_extern = DECL_EXTERNAL (member); DECL_EXTERNAL (member) = 0; --- gcc/testsuite/g++.dg/debug/dwarf2/pr80234-1.C.jj2017-03-29 17:23:43.606901317 +0200 +++ gcc/testsuite/g++.dg/debug/dwarf2/pr80234-1.C 2017-03-29 17:22:51.0 +0200 @@ -0,0 +1,15 @@ +// PR debug/80234 +// { dg-do compile } +// { dg-options "-gdwarf-4 -std=c++17" } + +struct S +{ + static constexpr const char n = 'S'; + virtual ~S (); +}; + +constexpr const char S::n; + +S::~S() +{ +} --- gcc/testsuite/g++.dg/debug/dwarf2/pr80234-2.C.jj2017-03-29 17:23:54.663756769 +0200 +++ gcc/testsuite/g++.dg/debug/dwarf2/pr80234-2.C 2017-03-29 17:23:59.818689378 +0200 @@ -0,0 +1,15 @@ +// PR debug/80234 +// { dg-do compile } +// { dg-options "-gdwarf-5 -std=c++17" } + +struct S +{ + static constexpr const char n = 'S'; + virtual ~S (); +}; + +constexpr const char S::n; + +S::~S() +{ +} Jakub
Re: [PATCH] Real fix for AIX exception handling
On 03/27/2017 09:41 AM, David Edelsohn wrote: As far as I have discovered, the real problem with AIX exception handling is that the exception landing pads are symbols that must not (but still are) exported from shared libraries - even libstdc++. I'm wondering if attached libtool(!)-patch would fix even that GDB problem once applied to each(!) shared library creation procedure. However, each workaround still applies as long as there's a single shared library involved that has not stopped exporting these symbols yet. Thoughts? Maybe gcc's collect2 should apply this additional symbol filter itself calling (AIX) ld, rather than leaving this to each build system? * m4/libtool.m4 (_LT_LINKER_SHLIBS): On AIX, GNU g++ generates _GLOBAL__ symbols as, amongst others, landing pads for C++ exceptions. These symbols must not be exported from shared libraries, or exception handling may break for applications with runtime linking enabled. Hi, Michael Thanks for the analysis. The problem with EH for GDB involves static linking, not runtime linking. That seems to be my understanding as well. And there seems to be different behavior for GCC 4.8 and GCC 4.9. Could it perhaps be an IPA issue -- we know IPA can change symbol scope/linkage in some cases. Maybe it's mucking things up. Is there more detail in a thread elsewhere on this issue? The patch seems correct for the runtime linking case, but I don't believe that it solves all of the EH issues -- at least more explanation is needed. The current libtool.m4 in GCC looks out of date relative to what's in Michael's patch. So we'd either need a patch specific to the version in GCC right now or we'd need to update libtool.m4 then apply Michael's patch. I think that the problem for static linking needs to be addressed by collect2. Could be -- I just don't have enough background to know either way. jeff
Re: [PATCH] Real fix for AIX exception handling
On Wed, Mar 29, 2017 at 3:50 PM, Jeff Law wrote: > On 03/27/2017 09:41 AM, David Edelsohn wrote: >>> >>> As far as I have discovered, the real problem with AIX exception handling >>> is >>> that the exception landing pads are symbols that must not (but still are) >>> exported from shared libraries - even libstdc++. >>> >>> I'm wondering if attached libtool(!)-patch would fix even that GDB >>> problem >>> once applied to each(!) shared library creation procedure. >>> >>> However, each workaround still applies as long as there's a single shared >>> library involved that has not stopped exporting these symbols yet. >>> >>> Thoughts? >>> >>> Maybe gcc's collect2 should apply this additional symbol filter itself >>> calling (AIX) ld, rather than leaving this to each build system? >>> >>> * m4/libtool.m4 (_LT_LINKER_SHLIBS): On AIX, GNU g++ generates >>> _GLOBAL__ symbols as, amongst others, landing pads for C++ exceptions. >>> These symbols must not be exported from shared libraries, or exception >>> handling may break for applications with runtime linking enabled. >> >> >> Hi, Michael >> >> Thanks for the analysis. >> >> The problem with EH for GDB involves static linking, not runtime >> linking. > > That seems to be my understanding as well. > >> And there seems to be different behavior for GCC 4.8 and GCC >> 4.9. > > Could it perhaps be an IPA issue -- we know IPA can change symbol > scope/linkage in some cases. Maybe it's mucking things up. Is there more > detail in a thread elsewhere on this issue? The problem is GCC EH tables and static linking. libstdc++ and the main application are ending up with two separate copies of the tables to register EH frames. Static linking worked in GCC 4.8, but not in GCC 4.9. I have been trying to understand what changed and if GCC 4.8 worked by accident. Note that AIX does not install a separate libstdc++ static archive and instead statically links against the shared object. libstdc++ apparently uses the EH table referenced when it was bound by collect2 and the application uses the one created when the executable is created -- the libgcc_eh.a solution doesn't work. Again, the question is why this apparently functioned correctly for GCC.4.8. There was a change to constructor order around that time and that may have affected where EH frames were recorded. Thanks, David
[PING][PATCH][PR sanitizer/77631] Support separate debug info in libbacktrace
Hello everyone, can someone please review that patch https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01171.html Thanks.
[Patch, Fortran, F03] PR 80046: Explicit interface required: pointer argument
Hi all, here is a patch that enhances the diagnostics for procedure-pointer assignments, so that procedure-pointer components that need an explicit interface are correctly rejected. Regtests cleanly on x86_64-linux-gnu. Ok for trunk? Cheers, Janus 2017-03-29 Janus Weil PR fortran/80046 * expr.c (gfc_check_pointer_assign): Check if procedure pointer components in a pointer assignment need an explicit interface. 2017-03-29 Janus Weil PR fortran/80046 * gfortran.dg/proc_ptr_comp_48.f90: New test case. Index: gcc/fortran/expr.c === --- gcc/fortran/expr.c (revision 246573) +++ gcc/fortran/expr.c (working copy) @@ -3595,25 +3595,41 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_ex return false; } - if (s1 == s2 || !s1 || !s2) - return true; - /* F08:7.2.2.4 (4) */ - if (s1->attr.if_source == IFSRC_UNKNOWN - && gfc_explicit_interface_required (s2, err, sizeof(err))) + if (s2 && gfc_explicit_interface_required (s2, err, sizeof(err))) { - gfc_error ("Explicit interface required for %qs at %L: %s", -s1->name, &lvalue->where, err); - return false; + if (comp1 && !s1) + { + gfc_error ("Explicit interface required for component %qs at %L: %s", +comp1->name, &lvalue->where, err); + return false; + } + else if (s1->attr.if_source == IFSRC_UNKNOWN) + { + gfc_error ("Explicit interface required for %qs at %L: %s", +s1->name, &lvalue->where, err); + return false; + } } - if (s2->attr.if_source == IFSRC_UNKNOWN - && gfc_explicit_interface_required (s1, err, sizeof(err))) + if (s1 && gfc_explicit_interface_required (s1, err, sizeof(err))) { - gfc_error ("Explicit interface required for %qs at %L: %s", -s2->name, &rvalue->where, err); - return false; + if (comp2 && !s2) + { + gfc_error ("Explicit interface required for component %qs at %L: %s", +comp2->name, &rvalue->where, err); + return false; + } + else if (s2->attr.if_source == IFSRC_UNKNOWN) + { + gfc_error ("Explicit interface required for %qs at %L: %s", +s2->name, &rvalue->where, err); + return false; + } } + if (s1 == s2 || !s1 || !s2) + return true; + if (!gfc_compare_interfaces (s1, s2, name, 0, 1, err, sizeof(err), NULL, NULL)) { ! { dg-do compile } ! ! PR 80046: [F03] Explicit interface required: pointer argument ! ! Contributed by Joachim Herb program p implicit none type :: Node_t procedure(NodeCloner), nopass, pointer :: cloneProc => NULL() procedure(), nopass, pointer :: noIfc => NULL() end type interface subroutine NodeCloner( tgt, src ) import Node_t type(Node_t), pointer, intent(out) :: tgt type(Node_t), intent(in) :: src end subroutine end interface type(Node_t) :: node procedure(NodeCloner), pointer :: cloneNode procedure(), pointer :: noIfc cloneNode => node%noIfc ! { dg-error "Explicit interface required" } node%noIfc => cloneNode ! { dg-error "Explicit interface required" } noIfc => node%cloneProc ! { dg-error "Explicit interface required" } node%cloneProc => noIfc ! { dg-error "Explicit interface required" } node%cloneProc => node%noIfc ! { dg-error "Explicit interface required" } node%noIfc => node%cloneProc ! { dg-error "Explicit interface required" } ! the following cases are legal node%noIfc => node%noIfc node%cloneProc => node%cloneProc cloneNode => node%cloneProc node%cloneProc => cloneNode noIfc => node%noIfc node%noIfc => noIfc end
Re: [PATCH] combine: Fix PR80233
On 03/29/2017 12:23 PM, Segher Boessenkool wrote: If combine has added an unconditional trap there will be a new basic block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK as the last_combined_insn, but then it tries to take the DF_INSN_LUID of that and that dereferences a NULL pointer (since such a note is not an INSN_P). This fixes it by not taking non-insns as last_combined_insn. Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Segher 2017-03-29 Segher Boessenkool PR rtl-optimization/80233 * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns as last_combined_insn. Do not test for BARRIER_P separately. gcc/testsuite/ PR rtl-optimization/80233 * gcc.c-torture/compile/pr80233.c: New testcase. No strong opinions on this vs Jakub's patch. I guess yours may walk more objects on the chain, but in doing so is more likely to find a useful LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to have stopped on something useful. Your call Segher. jeff ps. Never in a million years would I have expected isolation of division by zero to have exposed as many latent issues as it has. Sigh.
Re: [PATCH] combine: Fix PR80233
On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote: > On 03/29/2017 12:23 PM, Segher Boessenkool wrote: > > If combine has added an unconditional trap there will be a new basic > > block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK > > as the last_combined_insn, but then it tries to take the DF_INSN_LUID > > of that and that dereferences a NULL pointer (since such a note is not > > an INSN_P). > > > > This fixes it by not taking non-insns as last_combined_insn. > > > > Bootstrapped and tested on powerpc64-linux {-m32,-m64}. > > > > > > Segher > > > > > > 2017-03-29 Segher Boessenkool > > > > PR rtl-optimization/80233 > > * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns > > as last_combined_insn. Do not test for BARRIER_P separately. > > > > gcc/testsuite/ > > PR rtl-optimization/80233 > > * gcc.c-torture/compile/pr80233.c: New testcase. > No strong opinions on this vs Jakub's patch. I guess yours may walk more > objects on the chain, but in doing so is more likely to find a useful > LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to have > stopped on something useful. > > Your call Segher. I like Segher's latest patch. But it is his call anyway ;) Jakub
Re: [PATCH] Real fix for AIX exception handling
On 03/29/2017 02:21 PM, David Edelsohn wrote: The problem is GCC EH tables and static linking. libstdc++ and the main application are ending up with two separate copies of the tables to register EH frames. Static linking worked in GCC 4.8, but not in GCC 4.9. I have been trying to understand what changed and if GCC 4.8 worked by accident. Note that AIX does not install a separate libstdc++ static archive and instead statically links against the shared object. libstdc++ apparently uses the EH table referenced when it was bound by collect2 and the application uses the one created when the executable is created -- the libgcc_eh.a solution doesn't work. Again, the question is why this apparently functioned correctly for GCC.4.8. There was a change to constructor order around that time and that may have affected where EH frames were recorded. Hmm, the act of statically linking against the shared libstdc++ certainly adds a wrinkle here. It's been a *long* time since I had to think about this stuff for a non-ELF system. Please correct me if I goof anything up. Since we build a shared libstdc++, doesn't that set up a library ctor which should register the EH tables for the library as a whole? When we link against that library statically, we have to arrange to run the library's ctor from the main program's global ctors. Right? ie, it should be OK to have separate copies -- we just have to get them all registered properly? Right? [ Again, just trying to start from the basics as it's been a horribly long time since I've pondered such stuff. ] Jeff
Re: [PATCH] combine: Fix PR80233
On 03/29/2017 02:44 PM, Jakub Jelinek wrote: On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote: On 03/29/2017 12:23 PM, Segher Boessenkool wrote: If combine has added an unconditional trap there will be a new basic block as well. It will then end up considering the NOTE_INSN_BASIC_BLOCK as the last_combined_insn, but then it tries to take the DF_INSN_LUID of that and that dereferences a NULL pointer (since such a note is not an INSN_P). This fixes it by not taking non-insns as last_combined_insn. Bootstrapped and tested on powerpc64-linux {-m32,-m64}. Segher 2017-03-29 Segher Boessenkool PR rtl-optimization/80233 * combine.c (combine_instructions): Only take NONDEBUG_INSN_P insns as last_combined_insn. Do not test for BARRIER_P separately. gcc/testsuite/ PR rtl-optimization/80233 * gcc.c-torture/compile/pr80233.c: New testcase. No strong opinions on this vs Jakub's patch. I guess yours may walk more objects on the chain, but in doing so is more likely to find a useful LAST_COMBINED_INSN. Jakub's stops earlier, but is less likely to have stopped on something useful. Your call Segher. I like Segher's latest patch. But it is his call anyway ;) In that case let's just go with Segher's patch :-) jeff
Re: [PATCH] combine: Fix PR80233
On Wed, Mar 29, 2017 at 02:35:32PM -0600, Jeff Law wrote: > ps. Never in a million years would I have expected isolation of > division by zero to have exposed as many latent issues as it has. Sigh. The trap insns weren't handled very well before, but we didn't generate many either. They still aren't handled very well, but hopefully we don't ICE so much on them anymore ;-) Segher
Re: [PATCH] Real fix for AIX exception handling
On Wed, Mar 29, 2017 at 4:48 PM, Jeff Law wrote: > On 03/29/2017 02:21 PM, David Edelsohn wrote: >> >> >> The problem is GCC EH tables and static linking. libstdc++ and the >> main application are ending up with two separate copies of the tables >> to register EH frames. >> >> Static linking worked in GCC 4.8, but not in GCC 4.9. I have been >> trying to understand what changed and if GCC 4.8 worked by accident. >> >> Note that AIX does not install a separate libstdc++ static archive and >> instead statically links against the shared object. libstdc++ >> apparently uses the EH table referenced when it was bound by collect2 >> and the application uses the one created when the executable is >> created -- the libgcc_eh.a solution doesn't work. Again, the question >> is why this apparently functioned correctly for GCC.4.8. >> >> There was a change to constructor order around that time and that may >> have affected where EH frames were recorded. > > Hmm, the act of statically linking against the shared libstdc++ certainly > adds a wrinkle here. > > It's been a *long* time since I had to think about this stuff for a non-ELF > system. Please correct me if I goof anything up. > > Since we build a shared libstdc++, doesn't that set up a library ctor which > should register the EH tables for the library as a whole? > > When we link against that library statically, we have to arrange to run the > library's ctor from the main program's global ctors. Right? ie, it should > be OK to have separate copies -- we just have to get them all registered > properly? Right? GCC has to get them all of the EH frames registered into one table, otherwise libgcc walks the frames to find a catcher for an exception and doesn't find the address of the program counter in the sorted table because that range was recorded in the other table. One can understand how this ended up broken, but I am trying to understand how it worked correctly in GCC 4.8. And, as we all know, GDB functionality on AIX is limited. Thanks, David
[PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
PR80246 shows a problem with the dxex, dxexq, diex and diexq insn patterns, that shows up when calling their associated builtins. The dxex and dxexq instructions are defined to return 64-bit signed integers (in a FP register), but the output mode of their patterns is _Decimal64/_Decimal128. This leads to corrupting of the returned value if we assign the return value to a integer type variable (ie, we do a cast). A similar problem exists with the diex and diexq patterns on their input operand that is supposed to hold a 64-bit signed integer. This patch fixes the patterns to use DImode (ie, a 64-bit integer). Google shows no use of the builtins anywhere except in the GCC source, so it should be safe to change these. If there were users, they would have noticed how broken the builtins were. This passed bootstrap and regtesting on powerpc64le-linux and powerpc64-linux (testsuite in both 32-bit and 64-bit). Ok for mainline? Ok for the open release branches too? Peter gcc/ PR target/80246 * config/rs6000/dfp.md (dfp_dxex_): Update mode of operand 0. (dfp_diex_): Update mode of operand 1. * doc/extend.texi (dxex, dxexq): Document change to return type. (diex, diexq): Document change to argument type. gcc/testsuite/ PR target/80246 * gcc.target/powerpc/dfp-builtin-1.c (dxex, dxexq): Update return types. (diex, diexq): Update argment types. * gcc.target/powerpc/pr80246.c: New test. Index: gcc/config/rs6000/dfp.md === --- gcc/config/rs6000/dfp.md(revision 246539) +++ gcc/config/rs6000/dfp.md(working copy) @@ -348,9 +348,9 @@ [(set_attr "type" "dfp")]) (define_insn "dfp_dxex_" - [(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d") - (unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d")] -UNSPEC_DXEX))] + [(set (match_operand:DI 0 "gpc_reg_operand" "=d") + (unspec:DI [(match_operand:D64_D128 1 "gpc_reg_operand" "d")] + UNSPEC_DXEX))] "TARGET_DFP" "dxex %0,%1" [(set_attr "type" "dfp")]) @@ -357,7 +357,7 @@ (define_insn "dfp_diex_" [(set (match_operand:D64_D128 0 "gpc_reg_operand" "=d") - (unspec:D64_D128 [(match_operand:D64_D128 1 "gpc_reg_operand" "d") + (unspec:D64_D128 [(match_operand:DI 1 "gpc_reg_operand" "d") (match_operand:D64_D128 2 "gpc_reg_operand" "d")] UNSPEC_DXEX))] "TARGET_DFP" Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 246539) +++ gcc/doc/extend.texi (working copy) @@ -15427,14 +15427,14 @@ of processors when hardware decimal floating point (@option{-mhard-dfp}) is available: @smallexample -_Decimal64 __builtin_dxex (_Decimal64); -_Decimal128 __builtin_dxexq (_Decimal128); +long long __builtin_dxex (_Decimal64); +long long __builtin_dxexq (_Decimal128); _Decimal64 __builtin_ddedpd (int, _Decimal64); _Decimal128 __builtin_ddedpdq (int, _Decimal128); _Decimal64 __builtin_denbcd (int, _Decimal64); _Decimal128 __builtin_denbcdq (int, _Decimal128); -_Decimal64 __builtin_diex (_Decimal64, _Decimal64); -_Decimal128 _builtin_diexq (_Decimal128, _Decimal128); +_Decimal64 __builtin_diex (long long, _Decimal64); +_Decimal128 _builtin_diexq (long long, _Decimal128); _Decimal64 __builtin_dscli (_Decimal64, int); _Decimal128 __builtin_dscliq (_Decimal128, int); _Decimal64 __builtin_dscri (_Decimal64, int); Index: gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c === --- gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c(revision 246539) +++ gcc/testsuite/gcc.target/powerpc/dfp-builtin-1.c(working copy) @@ -52,7 +52,7 @@ return __builtin_denbcd (1, a); } -_Decimal64 +long long do_xex (_Decimal64 a) { return __builtin_dxex (a); @@ -59,7 +59,7 @@ } _Decimal64 -do_iex (_Decimal64 a, _Decimal64 b) +do_iex (long long a, _Decimal64 b) { return __builtin_diex (a, b); } Index: gcc/testsuite/gcc.target/powerpc/pr80246.c === --- gcc/testsuite/gcc.target/powerpc/pr80246.c (nonexistent) +++ gcc/testsuite/gcc.target/powerpc/pr80246.c (working copy) @@ -0,0 +1,40 @@ +/* { dg-do compile { target { powerpc*-*-linux* } } } */ +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ +/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */ +/* { dg-require-effective-target dfp } */ +/* { dg-options "-O2" } */ +/* { dg-final { scan-assembler-times "dxex " 1 } } */ +/* { dg-final { scan-assembler-times "dxexq " 1 } } */ +/* { dg-final { scan-assembler-times "diex " 1 } } */ +/* { dg-final { scan-assembler-times "diexq " 1 } } */ +/* { dg-final { scan-assembler-not "bl __builtin" } } */ +/* { dg-final { scan-assembler-not "drintn\\." } } */ +/* { dg-final { scan-assembler-not
Re: [PATCH] Real fix for AIX exception handling
On 03/29/2017 02:56 PM, David Edelsohn wrote: On Wed, Mar 29, 2017 at 4:48 PM, Jeff Law wrote: On 03/29/2017 02:21 PM, David Edelsohn wrote: The problem is GCC EH tables and static linking. libstdc++ and the main application are ending up with two separate copies of the tables to register EH frames. Static linking worked in GCC 4.8, but not in GCC 4.9. I have been trying to understand what changed and if GCC 4.8 worked by accident. Note that AIX does not install a separate libstdc++ static archive and instead statically links against the shared object. libstdc++ apparently uses the EH table referenced when it was bound by collect2 and the application uses the one created when the executable is created -- the libgcc_eh.a solution doesn't work. Again, the question is why this apparently functioned correctly for GCC.4.8. There was a change to constructor order around that time and that may have affected where EH frames were recorded. Hmm, the act of statically linking against the shared libstdc++ certainly adds a wrinkle here. It's been a *long* time since I had to think about this stuff for a non-ELF system. Please correct me if I goof anything up. Since we build a shared libstdc++, doesn't that set up a library ctor which should register the EH tables for the library as a whole? When we link against that library statically, we have to arrange to run the library's ctor from the main program's global ctors. Right? ie, it should be OK to have separate copies -- we just have to get them all registered properly? Right? GCC has to get them all of the EH frames registered into one table, otherwise libgcc walks the frames to find a catcher for an exception and doesn't find the address of the program counter in the sorted table because that range was recorded in the other table. Right. We have to register all the frame_infos into a single table. I was mostly focused on the registration mechanism. My recollection is that we'll create a library-wide ctor for libstdc++ which would register the frame_infos for the library and that ctor should be run by the main program. The main program will collect its own frame_infos and register them as well. At least that's my fuzzy recollection. jeff
Re: [PATCH] Real fix for AIX exception handling
On Wed, Mar 29, 2017 at 5:20 PM, Jeff Law wrote: > On 03/29/2017 02:56 PM, David Edelsohn wrote: >> >> On Wed, Mar 29, 2017 at 4:48 PM, Jeff Law wrote: >>> >>> On 03/29/2017 02:21 PM, David Edelsohn wrote: The problem is GCC EH tables and static linking. libstdc++ and the main application are ending up with two separate copies of the tables to register EH frames. Static linking worked in GCC 4.8, but not in GCC 4.9. I have been trying to understand what changed and if GCC 4.8 worked by accident. Note that AIX does not install a separate libstdc++ static archive and instead statically links against the shared object. libstdc++ apparently uses the EH table referenced when it was bound by collect2 and the application uses the one created when the executable is created -- the libgcc_eh.a solution doesn't work. Again, the question is why this apparently functioned correctly for GCC.4.8. There was a change to constructor order around that time and that may have affected where EH frames were recorded. >>> >>> >>> Hmm, the act of statically linking against the shared libstdc++ certainly >>> adds a wrinkle here. >>> >>> It's been a *long* time since I had to think about this stuff for a >>> non-ELF >>> system. Please correct me if I goof anything up. >>> >>> Since we build a shared libstdc++, doesn't that set up a library ctor >>> which >>> should register the EH tables for the library as a whole? >>> >>> When we link against that library statically, we have to arrange to run >>> the >>> library's ctor from the main program's global ctors. Right? ie, it >>> should >>> be OK to have separate copies -- we just have to get them all registered >>> properly? Right? >> >> >> GCC has to get them all of the EH frames registered into one table, >> otherwise libgcc walks the frames to find a catcher for an exception >> and doesn't find the address of the program counter in the sorted >> table because that range was recorded in the other table. > > Right. We have to register all the frame_infos into a single table. I was > mostly focused on the registration mechanism. My recollection is that we'll > create a library-wide ctor for libstdc++ which would register the > frame_infos for the library and that ctor should be run by the main program. > The main program will collect its own frame_infos and register them as well. Yes, collect2 essentially creates a constructor that it schedules at a high priority to record the frames that it found during the scan. Thanks, David
[PATCH, rs6000] Document location of ELF V2 ABI specification
Hi, There is finally a permanent link to the OpenPOWER ELF V2 ABI Specification document. This patch adds a paragraph to the PowerPC Built-Ins section of the GCC documentation to describe its value and provide a link to it. I've generated and verified the PDF, and it looks ok. I am very open to changes in wording. Is this ok for trunk? Thanks, Bill 2017-03-29 Bill Schmidt * doc/extend.texi (6.60.22 PowerPC AltiVec Built-in Functions): Add reference to the OpenPOWER 64-Bit ELF V2 ABI Specification. Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 246566) +++ gcc/doc/extend.texi (working copy) @@ -15539,6 +15539,15 @@ Internally, GCC uses built-in functions to achieve the aforementioned header file, but they are not supported and are subject to change without notice. +GCC complies with the OpenPOWER 64-Bit ELF V2 ABI Specification, +which may be found at +@uref{http://openpowerfoundation.org/wp-content/uploads/resources/leabi-prd/content/index.html}. +Appendix A of this document lists the vector API interfaces that must be +provided by compliant compilers. Programmers should preferentially use +the interfaces described therein. However, historically GCC has provided +additional interfaces for access to vector instructions. These are +described briefly below. + The following interfaces are supported for the generic and specific AltiVec operations and the AltiVec predicates. In cases where there is a direct mapping between generic and specific operations, only the
Re: C++ PATCH for c++/80095, ICE with this pointer in NSDMI
On Wed, Mar 29, 2017 at 02:56:51PM -0400, Jason Merrill wrote: > On Wed, Mar 29, 2017 at 12:38 PM, Marek Polacek wrote: > > Here we have a reference initialization with NSDMI and *this. We are > > crashing > > because a PLACEHOLDER_EXPR crept into the gimplifier. > > > > This happens since r218653 where set_up_extended_ref_temp was changed to > > use split_nonconstant_init. As a consequence, cp_gimplify_init_expr might > > now be receiving > > > > D.2051.p = (void *) & > > > > instead of > > > > D.2051 = {.p = (void *) &} > > > > where the RHS was a CONSTRUCTOR. It no longer is, so replace_placeholders > > is > > not called anymore. It occurred to me that we should use the same check as > > in > > store_init_value (i.e. check that the object to be used in the substitution > > is > > a class), but given what split_nonconstant_init might produce, handle > > COMPONENT_REFs specially. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? > > > > 2017-03-29 Marek Polacek > > > > PR c++/80095 - ICE with this pointer in NSDMI. > > * cp-gimplify.c (cp_gimplify_init_expr): Call replace_placeholders > > when TO is a class. > > > > * g++.dg/cpp1y/nsdmi-aggr8.C: New test. > > > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > > index 354ae1a..e530daf 100644 > > --- gcc/cp/cp-gimplify.c > > +++ gcc/cp/cp-gimplify.c > > @@ -496,7 +496,16 @@ cp_gimplify_init_expr (tree *expr_p) > > TREE_TYPE (from) = void_type_node; > > } > > > > - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) > > + /* split_nonconstant_init might've produced something like > > +D.2051.p = (void *) & > > +in which case we want to substitute the placeholder with > > +D.2051. */ > > + tree op0 = to; > > + while (TREE_CODE (op0) == COMPONENT_REF) > > + op0 = TREE_OPERAND (op0, 0); > > + tree type = TREE_TYPE (op0); > > + > > + if (cxx_dialect >= cxx14 && CLASS_TYPE_P (strip_array_types (type))) > > How about doing this checking in replace_placeholders, instead? That > is, if the object isn't a (member of a) class, just return. Sure. I also moved the C++14 check into replace_placeholders, although maybe I shouldn't have. Bootstrapped/regtested on x86_64-linux, ok for trunk and 6? 2017-03-29 Marek Polacek PR c++/80095 * call.c (build_over_call): Don't check cxx_dialect. * cp-gimplify.c (cp_gimplify_init_expr): Don't check cxx_dialect nor whether SUB is a CONSTRUCTOR. * init.c (build_new_1): Don't check cxx_dialect. * tree.c (replace_placeholders): Add a function comment. Return if not in C++14, or if the object isn't a (member of a) class. * typeck2.c (store_init_value): Don't check cxx_dialect nor whether TYPE is CLASS_TYPE_P. * g++.dg/cpp1y/nsdmi-aggr8.C: New test. diff --git gcc/cp/call.c gcc/cp/call.c index 803fbd4..c15b8e4 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -8047,9 +8047,8 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) { arg = cp_build_indirect_ref (arg, RO_NULL, complain); val = build2 (MODIFY_EXPR, TREE_TYPE (to), to, arg); - if (cxx_dialect >= cxx14) - /* Handle NSDMI that refer to the object being initialized. */ - replace_placeholders (arg, to); + /* Handle NSDMI that refer to the object being initialized. */ + replace_placeholders (arg, to); } else { diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c index 354ae1a..3f91c35 100644 --- gcc/cp/cp-gimplify.c +++ gcc/cp/cp-gimplify.c @@ -496,9 +496,8 @@ cp_gimplify_init_expr (tree *expr_p) TREE_TYPE (from) = void_type_node; } - if (cxx_dialect >= cxx14 && TREE_CODE (sub) == CONSTRUCTOR) - /* Handle aggregate NSDMI. */ - replace_placeholders (sub, to); + /* Handle aggregate NSDMI. */ + replace_placeholders (sub, to); if (t == sub) break; diff --git gcc/cp/init.c gcc/cp/init.c index 7732795..6a70955 100644 --- gcc/cp/init.c +++ gcc/cp/init.c @@ -3373,8 +3373,7 @@ build_new_1 (vec **placement, tree type, tree nelts, object being initialized, replace them now and don't try to preevaluate. */ bool had_placeholder = false; - if (cxx_dialect >= cxx14 - && !processing_template_decl + if (!processing_template_decl && TREE_CODE (init_expr) == INIT_EXPR) TREE_OPERAND (init_expr, 1) = replace_placeholders (TREE_OPERAND (init_expr, 1), diff --git gcc/cp/tree.c gcc/cp/tree.c index 2757af6..9939135 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -2813,9 +2813,23 @@ replace_placeholders_r (tree* t, int* walk_subtrees, void* data_) return NULL_TREE; } +/* Replace PLACEHOLDER_EXPRs in EXP with object OBJ. SEEN_P is set if + a PLACEHOLDER_EXPR has
Re: [PATCH] Real fix for AIX exception handling
On 03/29/2017 03:22 PM, David Edelsohn wrote: On Wed, Mar 29, 2017 at 5:20 PM, Jeff Law wrote: On 03/29/2017 02:56 PM, David Edelsohn wrote: On Wed, Mar 29, 2017 at 4:48 PM, Jeff Law wrote: On 03/29/2017 02:21 PM, David Edelsohn wrote: The problem is GCC EH tables and static linking. libstdc++ and the main application are ending up with two separate copies of the tables to register EH frames. Static linking worked in GCC 4.8, but not in GCC 4.9. I have been trying to understand what changed and if GCC 4.8 worked by accident. Note that AIX does not install a separate libstdc++ static archive and instead statically links against the shared object. libstdc++ apparently uses the EH table referenced when it was bound by collect2 and the application uses the one created when the executable is created -- the libgcc_eh.a solution doesn't work. Again, the question is why this apparently functioned correctly for GCC.4.8. There was a change to constructor order around that time and that may have affected where EH frames were recorded. Hmm, the act of statically linking against the shared libstdc++ certainly adds a wrinkle here. It's been a *long* time since I had to think about this stuff for a non-ELF system. Please correct me if I goof anything up. Since we build a shared libstdc++, doesn't that set up a library ctor which should register the EH tables for the library as a whole? When we link against that library statically, we have to arrange to run the library's ctor from the main program's global ctors. Right? ie, it should be OK to have separate copies -- we just have to get them all registered properly? Right? GCC has to get them all of the EH frames registered into one table, otherwise libgcc walks the frames to find a catcher for an exception and doesn't find the address of the program counter in the sorted table because that range was recorded in the other table. Right. We have to register all the frame_infos into a single table. I was mostly focused on the registration mechanism. My recollection is that we'll create a library-wide ctor for libstdc++ which would register the frame_infos for the library and that ctor should be run by the main program. The main program will collect its own frame_infos and register them as well. Yes, collect2 essentially creates a constructor that it schedules at a high priority to record the frames that it found during the scan. OK. So I'd first verify there is a constructor for the libstdc++ library as a whole via nm/objdump. Then I'd verify that library constructor shows up in the ctor list for the main executable -- not sure the best way to do that. jeff
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
Hi! On Wed, Mar 29, 2017 at 04:11:19PM -0500, Peter Bergner wrote: > This passed bootstrap and regtesting on powerpc64le-linux and > powerpc64-linux (testsuite in both 32-bit and 64-bit). Ok for mainline? > Ok for the open release branches too? Okay. A few nits below. Thanks! > gcc/testsuite/ > PR target/80246 > * gcc.target/powerpc/dfp-builtin-1.c (dxex, dxexq): Update return types. > (diex, diexq): Update argment types. Typo ("argument"). > Index: gcc/testsuite/gcc.target/powerpc/pr80246.c > === > --- gcc/testsuite/gcc.target/powerpc/pr80246.c(nonexistent) > +++ gcc/testsuite/gcc.target/powerpc/pr80246.c(working copy) > @@ -0,0 +1,40 @@ > +/* { dg-do compile { target { powerpc*-*-linux* } } } */ > +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ > +/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */ > +/* { dg-require-effective-target dfp } */ You can leave off the default arguments "*" "". Do we really need to explicitly skip on Darwin and SPE if there is a test for DFP anyway? > +/* { dg-final { scan-assembler-times "dxex " 1 } } */ > +/* { dg-final { scan-assembler-times "dxexq " 1 } } */ > +/* { dg-final { scan-assembler-times "diex " 1 } } */ > +/* { dg-final { scan-assembler-times "diexq " 1 } } */ > +/* { dg-final { scan-assembler-not "bl __builtin" } } */ > +/* { dg-final { scan-assembler-not "drintn\\." } } */ > +/* { dg-final { scan-assembler-not "drintnq\\." } } */ > +/* { dg-final { scan-assembler-not "dctfix" } } */ > +/* { dg-final { scan-assembler-not "dctfixq" } } */ If there is no "dctfix" there surely is no "dctfixq" either (i.e., your regexen aren't very tight). Segher
[PATCH] Fix various avx512 extraction issues (PR target/80206)
Hi! As the testcase shows, we ICE with -mavx512f -ffloat-store, because at -O0 during expansion the destination is MEM, and the corresponding dup operand is some pseudo. There are *_mask patterns that have just register_operand / =v for the desination and vector_move_operand / 0C for the corresponding dup operand (but this doesn't apply when the destination is MEM), and then *_maskm patterns, that have memory_operand / =m and corresponding dup operand memory_operand / 0, but also requires rtx_equal_p between them in the condition, so that doesn't match either. The expanders have weirdo: if (MEM_P (operands[0]) && GET_CODE (operands[3]) == CONST_VECTOR) operands[0] = force_reg (mode, operands[0]); which can't really ever work, because the expander's caller expects the output to be stored in the original operands[0], but that is not where it stores it. Furthermore, force_reg makes no sense for the output operand. The following patch should fix that, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? There are still some remaining issues that can perhaps be resolved incrementally, e.g. some insns use: (define_insn "vec_extract_hi_" [(set (match_operand: 0 "" "=,vm") If , is register_operand, so having vm constraint for it is strange. Not really sure how well it can work with vector_move_operand and 0C constraint, what will LRA do with it if the input isn't in memory but dest is, or if both are memory, but not the same one. 2017-03-28 Jakub Jelinek PR target/80206 * config/i386/sse.md (_vextract_mask): Force dest into register whenever it is a MEM not rtx_equal_p to the corresponding dup operand, and when forcing into reg move the reg into the memory afterwards. (_vextract_mask): Likewise. Use instead of for the force_reg mode. (avx512vl_vextractf128): Force dest into register either always when a MEM, or when it is a MEM not rtx_equal_p to the corresponding dup operand, or even not when it is a CONST_VECTOR depending on the mode and lo vs. hi. (avx512dq_vextract64x2_1_maskm): Remove extraneous parens. (avx512f_vextract32x4_1_maskm): Likewise. (avx512dq_vextract64x2_1): Likewise. Require that operands[2] is even. (avx512f_vextract32x4_1): Remove extraneous parens. Require that operands[2] is a multiple of 4. (vec_extract_lo_): Don't bother testing if operands[0] is a MEM if , the predicates/constraints disallow memory then. * gcc.target/i386/pr80206.c: New test. --- gcc/config/i386/sse.md.jj 2017-03-07 09:10:56.946428168 +0100 +++ gcc/config/i386/sse.md 2017-03-29 19:22:37.394215557 +0200 @@ -7135,19 +7135,22 @@ (define_expand "_vextract< { int mask; mask = INTVAL (operands[2]); + rtx dest = operands[0]; - if (MEM_P (operands[0]) && GET_CODE (operands[3]) == CONST_VECTOR) -operands[0] = force_reg (mode, operands[0]); + if (MEM_P (operands[0]) && !rtx_equal_p (operands[0], operands[3])) +dest = force_reg (mode, dest); if (mode == V16SImode || mode == V16SFmode) -emit_insn (gen_avx512f_vextract32x4_1_mask (operands[0], +emit_insn (gen_avx512f_vextract32x4_1_mask (dest, operands[1], GEN_INT (mask * 4), GEN_INT (mask * 4 + 1), GEN_INT (mask * 4 + 2), GEN_INT (mask * 4 + 3), operands[3], operands[4])); else -emit_insn (gen_avx512dq_vextract64x2_1_mask (operands[0], +emit_insn (gen_avx512dq_vextract64x2_1_mask (dest, operands[1], GEN_INT (mask * 2), GEN_INT (mask * 2 + 1), operands[3], operands[4])); + if (dest != operands[0]) +emit_move_insn (operands[0], dest); DONE; }) @@ -7161,8 +7164,8 @@ (define_insn "avx512dq_vextract 4 "memory_operand" "0") (match_operand:QI 5 "register_operand" "Yk")))] "TARGET_AVX512DQ - && (INTVAL (operands[2]) % 2 == 0) - && (INTVAL (operands[2]) == INTVAL (operands[3]) - 1) + && INTVAL (operands[2]) % 2 == 0 + && INTVAL (operands[2]) == INTVAL (operands[3]) - 1 && rtx_equal_p (operands[4], operands[0])" { operands[2] = GEN_INT ((INTVAL (operands[2])) >> 1); @@ -7187,13 +7190,13 @@ (define_insn "avx512f_vextract 6 "memory_operand" "0") (match_operand:QI 7 "register_operand" "Yk")))] "TARGET_AVX512F - && ((INTVAL (operands[2]) % 4 == 0) - && INTVAL (operands[2]) == (INTVAL (operands[3]) - 1) - && INTVAL (operands[3]) == (INTVAL (operands[4]) - 1) - && INTVAL (operands[4]) == (INTVAL (operands[5]) - 1)) + && INTVAL (operands[2]) % 4 == 0 + && INTVAL (operands[2]) == INTVAL (operands[3]) - 1 + && INTVAL (operands[3]) == INTVAL (operands[4]) - 1 + && INTVAL (operands[4]) == INTVAL (operands[5]) - 1 && rtx_equal_p (operands[6], operands[0])" { - operands[2] = GEN_INT ((INTVAL (operands[2])) >> 2); + operands[2] = GEN_INT (INTVAL (operands[2]) >> 2); return "vextract32x4\
Re: [PATCH, rs6000] Document location of ELF V2 ABI specification
On Wed, Mar 29, 2017 at 04:27:28PM -0500, Bill Schmidt wrote: > There is finally a permanent link to the OpenPOWER ELF V2 ABI Specification > document. This patch adds a paragraph to the PowerPC Built-Ins section of > the GCC documentation to describe its value and provide a link to it. Woohoo! > I've generated and verified the PDF, and it looks ok. I am very open to > changes in wording. Is this ok for trunk? It's fine with me, but maybe Gerald (cc:ed) has some remarks? > 2017-03-29 Bill Schmidt > > * doc/extend.texi (6.60.22 PowerPC AltiVec Built-in Functions): Section numbers are useless: they are not in thre source document, and they also change all the time. Segher
Re: [PATCH] have chkp skip flexible member arrays (PR #79986)
On 03/21/2017 01:33 PM, Jason Merrill wrote: On Tue, Mar 21, 2017 at 11:08 AM, Martin Sebor wrote: On 03/20/2017 10:27 PM, Jason Merrill wrote: On Mon, Mar 20, 2017 at 7:58 PM, Martin Sebor wrote: On 03/20/2017 05:51 PM, Jason Merrill wrote: On Mon, Mar 20, 2017 at 7:04 PM, Martin Sebor wrote: Attached is a minimal patch to avoid an ICE in CHKP upon encountering one form of an initializer for a flexible array member, specifically the empty string: int f () { struct B { int n; char a[]; }; return ((struct B){ 1, "" }).a[0]; } Although GCC accepts (and doesn't ICE on) non-empty initializers for flexible array members, such as (struct B){ 1, "123" } How do you mean? When I compile this with the C front end, I get error: non-static initialization of a flexible array member I meant that G++ accepts it, doesn't ICE, but emits wrong code. (it's consistently rejected by the C front end). Sorry for not being clear about it. Ah, OK. It seems to me that the wrong code bug is worth addressing; why does rejecting the code seem risky to you? I have a few reasons: First, it's not a regression (I raised bug 69696 for it in early 2016) so strictly it's out of scope for this stage. Second, there are a number of bugs related to the initialization of flexible array members so the fixes are probably not going to be contained to a single function or file. Third, the flexible member array changes I made in the past were not trivial, took time to develop, and the two of us iterated over some of them for weeks. Despite your careful review and my extensive testing some of them introduced regressions that are still being patched up. Fourth, making a change to reject code this close to a release runs the risk of breaking code that has successfully compiled in mass rebuilds and others' tests with the new compiler. While that could be viewed as a good change for invalid code that's exercised at run time, it could also break programs where the bad code is never exercised. Fair enough. But I think the ICE is preferable to wrong code. I agree in general, although in this case it seems that ICE is more likely to penalize -fcheck-pointer-bounds users without benefiting anyone else. I decided to go ahead and implemented your suggestion just in case it was easier and safer than I thought. The attached patch is the result. I think the changes are actually not as risky as I had initially feared but they are still fairly extensive and I would hesitate to recommend they be made at this stage. Regardless of what happens to this patch, for GCC 8, I'd like to look into making the initialization work correctly in all contexts (i.e., including auto variables) . As I said in my response to Marek, I expect rejecting it will only lead users determined enough to make use of flexible array members for local variables to invent error-prone hacks. Martin PR c++/79986 - ICE in fold_convert_loc with a flexible array gcc/c-family/ChangeLog: PR c++/79986 * c.opt (-Wflexarray, -Wflexarray-init): New C++ options. gcc/cp/ChangeLog: PR c++/79986 * class.c (find_flexarrays): Add argument. (check_flexarrays): Same and handle it. Return tree instead of void. (find_flexarray): New extern function. (finish_struct_1): Adjust. * cp-tree.h (find_flexarray): Declare it. * decl.c (maybe_reject_flexarray_decl): New function. (check_array_initializer): Call it. (cp_finish_decl): Same. (grokdeclarator): Replace OPT_Wpedantic with OPT_Wflexarray. * pt.c (tsubst): Reject types with invalid flexible array members. * typeck2.c (init_ctor_info): New struct. (store_init_value): Use it. (process_init_constructor): Same. (maybe_diag_flexinit): New function. (digest_init_r, digest_init, digest_init_flags): Use init_ctor_info. (digest_init_flags): New overload. (digest_nsdmi_init): Use init_ctor_info. (massage_init_elt): Same. (process_init_constructor_array): Same. Call maybe_diag_flexinit. (process_init_constructor_record): Use init_ctor_info. (process_init_constructor_union): Same. gcc/ChangeLog: PR c++/79986 * doc/invoke.texi (Warning Options): Document -Wflexarray and -Wflexarray-init. gcc/testsuite/ChangeLog: PR c++/79986 * g++.dg/pr79986.C: New test. * g++.dg/ext/flexary24.C: New test. * g++.dg/ext/flexary25.C: New test. * g++.dg/cpp1z/has-unique-obj-representations1.C: Adjust. * g++.dg/ext/flexarray-mangle-2.C: Same. * g++.dg/ext/flexarray-mangle.C: Same. * g++.dg/ext/flexarray-subst.C: Same. * g++.dg/ext/flexary10.C: Same. * g++.dg/ext/flexary11.C: Same. * g++.dg/ext/flexary12.C: Same. * g++.dg/ext/flexary13.C: Same. * g++.dg/ext/flexary15.C: Same. * g++.dg/ext/flexary18.C: Same. * g++.dg/ext/flexary19.C: Same. * g++.dg/ext/flexary3.C: Same. * g++.dg/ext/flexary7.C: Same. * g++.dg/parse/pr43765.C: Same. * g++.dg/warn/Wplacement-new-size-1.C: Same. * g++.dg/warn/Wplacement-new-size-2.C: Same. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 78fea61..a0
Re: [PATCH, rs6000] Fix PR target/80246, DFP builtins using the wrong types
On 3/29/17 5:28 PM, Segher Boessenkool wrote: > Typo ("argument"). Oops! :-) >> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ >> +/* { dg-skip-if "" { powerpc*-*-*spe* } { "*" } { "" } } */ >> +/* { dg-require-effective-target dfp } */ > > You can leave off the default arguments "*" "". Do we really need to > explicitly skip on Darwin and SPE if there is a test for DFP anyway? Good question. I just cut pasted the above from another test, but I think you are correct we don't need those. I'll remove them. >> +/* { dg-final { scan-assembler-not "drintn\\." } } */ >> +/* { dg-final { scan-assembler-not "drintnq\\." } } */ >> +/* { dg-final { scan-assembler-not "dctfix" } } */ >> +/* { dg-final { scan-assembler-not "dctfixq" } } */ > > If there is no "dctfix" there surely is no "dctfixq" either (i.e., your > regexen aren't very tight). Ahh, true. I suppose I could also just look for "drintn" too, since that would catch both drintn. and drintnq., ok with that change? Peter