Re: [PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi
Hi Sudi, On 22 March 2018 at 18:26, Sudakshina Das wrote: > Hi > > > On 22/03/18 16:52, Kyrill Tkachov wrote: >> >> >> On 22/03/18 16:20, Sudakshina Das wrote: >>> >>> Hi Kyrill >>> >>> On 22/03/18 16:08, Kyrill Tkachov wrote: Hi Sudi, On 21/03/18 17:44, Sudakshina Das wrote: > > Hi > > The ICE in the bug report was happening because the macro > USE_RETURN_INSN (FALSE) was returning different values at different > points in the compilation. This was internally occurring because the > function arm_compute_static_chain_stack_bytes () which was dependent on > arm_r3_live_at_start_p () was giving a different value after the > cond_exec instructions were created in ce3 causing the liveness of r3 > to escape up to the start block. > > The function arm_compute_static_chain_stack_bytes () should really > only compute the value once during epilogue/prologue stage. This pass > introduces a new member 'static_chain_stack_bytes' to the target > definition of the struct machine_function which gets calculated in > expand_prologue and is the value that is returned by > arm_compute_static_chain_stack_bytes () beyond that. > > Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf > and added the reported test to the testsuite. > > Is this ok for trunk? > Thanks for working on this. I agree with the approach, I have a couple of comments inline. > Sudi > > > ChangeLog entries: > > *** gcc/ChangeLog *** > > 2018-03-21 Sudakshina Das > > PR target/84826 > * config/arm/arm.h (machine_function): Add > static_chain_stack_bytes. > * config/arm/arm.c (arm_compute_static_chain_stack_bytes): > Avoid re-computing once computed. > (arm_expand_prologue): Compute > machine->static_chain_stack_bytes. > (arm_init_machine_status): Initialize > machine->static_chain_stack_bytes. > > *** gcc/testsuite/ChangeLog *** > > 2018-03-21 Sudakshina Das > > PR target/84826 > * gcc.target/arm/pr84826.c: New test The new test fails on arm-none-linux-gnueabi --with-mode thumb --with-cpu cortex-a9 --with-fpu default Dejagnu flags: -march=armv5t Because: /gcc/testsuite/gcc.target/arm/pr84826.c: In function 'a': /gcc/testsuite/gcc.target/arm/pr84826.c:15:1: sorry, unimplemented: -fstack-check=specific for Thumb-1 compiler exited with status 1 FAIL: gcc.target/arm/pr84826.c (test for excess errors) You probably have to add a require-effective-target to skip the test in such cases. Thanks, Christophe diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index bbf3937..2809112 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function machine_mode thumb1_cc_mode; /* Set to 1 after arm_reorg has started. */ int after_arm_reorg; + /* The number of bytes used to store the static chain register on the + stack, above the stack frame. */ + int static_chain_stack_bytes; } machine_function; #endif diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index cb6ab81..bc31810 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void) static int arm_compute_static_chain_stack_bytes (void) { + /* Once the value is updated from the init value of -1, do not + re-compute. */ + if (cfun->machine->static_chain_stack_bytes != -1) +return cfun->machine->static_chain_stack_bytes; + My concern is that this approach caches the first value that is computed for static_chain_stack_bytes. I believe the layout frame code is called multiple times during register allocation as it goes through the motions and I think we want the last value it computes during reload How about we do something like: if (cfun->machine->static_chain_stack_bytes != -1 &&epilogue_completed) return cfun->machine->static_chain_stack_bytes; ?... /* See the defining assertion in arm_expand_prologue. */ if (IS_NESTED (arm_current_func_type ()) && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) @@ -21699,6 +21704,11 @@ arm_expand_prologue (void) emit_insn (gen_movsi (stack_pointer_rtx, r1)); } + /* Let's compute the static_chain_stack_bytes required and store it. Right + now the value must the -1 as stored by arm_init_machine_status (). */ ... this comment would need to be tweaked as cfun->machine->static_chain_stack_bytes may hold an intermediate value computed in reload or some ot
Re: [PATCH] Fix sanopt -fsanitize=pointer-overflow optimization (PR sanitizer/85029)
On Thu, 22 Mar 2018, Jakub Jelinek wrote: > Hi! > > As the testcase shows, we can hit the assertion here (with code that is > rejected only later on during expansion). Instead of the assertion, this > patch just doesn't try to optimize those, maybe_optimize_ubsan_ptr_ifn > is a pure optimization. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Richard. > 2018-03-22 Jakub Jelinek > > PR sanitizer/85029 > * sanopt.c (maybe_optimize_ubsan_ptr_ifn): If DECL_REGISTER (base), > just don't try to optimize it rather than assert it never happens. > > * g++.dg/ubsan/pr85029.C: New test. > > --- gcc/sanopt.c.jj 2018-03-14 09:49:45.462028237 +0100 > +++ gcc/sanopt.c 2018-03-22 08:20:30.639183003 +0100 > @@ -488,9 +488,9 @@ maybe_optimize_ubsan_ptr_ifn (sanopt_ctx > &unsignedp, &reversep, &volatilep); >if ((offset == NULL_TREE || TREE_CODE (offset) == INTEGER_CST) > && DECL_P (base) > + && !DECL_REGISTER (base) > && pbitpos.is_constant (&bitpos)) > { > - gcc_assert (!DECL_REGISTER (base)); > offset_int expr_offset; > if (offset) > expr_offset = wi::to_offset (offset) + bitpos / BITS_PER_UNIT; > --- gcc/testsuite/g++.dg/ubsan/pr85029.C.jj 2018-03-22 08:22:34.952174863 > +0100 > +++ gcc/testsuite/g++.dg/ubsan/pr85029.C 2018-03-22 08:31:49.554138533 > +0100 > @@ -0,0 +1,15 @@ > +// PR sanitizer/85029 > +// { dg-do compile } > +// { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } > +// { dg-options "-fsanitize=undefined" } > + > +struct B { > + virtual B bar (); > + int e; > +} register a;// { dg-error "register name not specified for 'a'" } > + > +int > +foo (...) > +{ > + return foo (a); > +} > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Fix alias.c ICE on inline-asm "=m" incomplete operand (PR inline-asm/85022)
On Thu, 22 Mar 2018, Jakub Jelinek wrote: > Hi! > > Something I wasn't really aware, apparently we allow extern vars with > incomplete types in "m" and "=m" constrained asm. The problem is that > incomplete vars have VOIDmode mode and so their MEMs do as well, apparently > everything works with it except a 2013-ish assert in alias.c. > > The following patch just makes sure x_mode is not VOIDmode if x doesn't have > VOIDmode, if x has VOIDmode, then it is fine for x_mode to be VOIDmode too. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. The question now is whether we handle dependences correctly for VOIDmode MEMs? Do they have !MEM_SIZE_KNOWN_P? Thanks, Richard. > 2018-03-22 Jakub Jelinek > > PR inline-asm/85022 > * alias.c (write_dependence_p): Don't require for x_canonicalized > non-VOIDmode if x has VOIDmode. > > * c-c++-common/torture/pr85022.c: New test. > > --- gcc/alias.c.jj2018-03-01 11:29:06.725104754 +0100 > +++ gcc/alias.c 2018-03-22 10:35:15.649546027 +0100 > @@ -2999,7 +2999,8 @@ write_dependence_p (const_rtx mem, >int ret; > >gcc_checking_assert (x_canonicalized > -? (x_addr != NULL_RTX && x_mode != VOIDmode) > +? (x_addr != NULL_RTX > + && (x_mode != VOIDmode || GET_MODE (x) == VOIDmode)) > : (x_addr == NULL_RTX && x_mode == VOIDmode)); > >if (MEM_VOLATILE_P (x) && MEM_VOLATILE_P (mem)) > --- gcc/testsuite/c-c++-common/torture/pr85022.c.jj 2018-03-22 > 10:44:40.520679173 +0100 > +++ gcc/testsuite/c-c++-common/torture/pr85022.c 2018-03-22 > 10:44:29.550675460 +0100 > @@ -0,0 +1,9 @@ > +/* PR inline-asm/85022 */ > + > +extern struct B b; > + > +void > +foo () > +{ > + __asm ("" : "+m" (b)); > +} > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/22/2018 08:04 PM, Cesar Philippidis wrote: I'm going to retest the variable vector length changes without it and see if it's still necessary. On one hand, maxntid should be fairly innocuous, but I don't like how it can mask other PTX JIT bugs. At this point, I'm leaning towards dropping it if does not impact the libgomp regression test suite anymore. What do you want to do? If there is no observable difference in tests passing/failing, then we should drop it. Thanks, - Tom
Re: [PATCH] Fix another match_asm_constraints_1 issue (PR PR inline-asm/85034)
On Thu, 22 Mar 2018, Jakub Jelinek wrote: > Hi! > > match_asm_constraints_1 verifies that the mode of input and output > is the same, with one exception - when input is VOIDmode. > It isn't correct to allow that blindly VOIDmode CONST_INT/CONST_DOUBLE > is only ok if the output mode is scalar integer mode, on the testcase below > output is (reg:SF), while input is (const_int 0) and we emit a move > from this input to output, which isn't valid RTL. > Mode mismatch isn't the only problem that can come up, another is e.g. > if input is some large integer constant, but output is only small mode, > e.g. unsigned short s; asm volatile ("" : "=r" (s) : "0" > (0x12345678abcdef0ULL)); > So, in addition to mode we should also check that for CONST_INT > trunc_int_for_mode gives the INTVAL. general_operand predicate checks > already all that and other requirements; if input is a REG (and not hard > reg), which is the most common case, general_operand is always true for it > if the reg has the right mode. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Richard. > 2018-03-22 Jakub Jelinek > > PR inline-asm/85034 > * function.c (match_asm_constraints_1): Don't optimize if input > doesn't satisfy general_operand predicate for output's mode. > > * gcc.target/i386/pr85034.c: New test. > > --- gcc/function.c.jj 2018-03-22 13:31:13.393054131 +0100 > +++ gcc/function.c2018-03-22 18:44:37.864083638 +0100 > @@ -6661,10 +6661,9 @@ match_asm_constraints_1 (rtx_insn *insn, >/* Only do the transformation for pseudos. */ >if (! REG_P (output) > || rtx_equal_p (output, input) > - || (GET_MODE (input) != VOIDmode > - && GET_MODE (input) != GET_MODE (output)) > || !(REG_P (input) || SUBREG_P (input) > -|| MEM_P (input) || CONSTANT_P (input))) > +|| MEM_P (input) || CONSTANT_P (input)) > + || !general_operand (input, GET_MODE (output))) > continue; > >/* We can't do anything if the output is also used as input, > --- gcc/testsuite/gcc.target/i386/pr85034.c.jj2018-03-22 > 18:47:32.794168043 +0100 > +++ gcc/testsuite/gcc.target/i386/pr85034.c 2018-03-22 18:47:09.235156562 > +0100 > @@ -0,0 +1,11 @@ > +/* PR inline-asm/85034 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +void > +foo (void) > +{ > + volatile float a; > + struct S { char a; } b = { 0 }; > + asm volatile ("" : "=r" (a) : "0ir" (b)); > +} > > Jakub > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Fix alias.c ICE on inline-asm "=m" incomplete operand (PR inline-asm/85022)
On Fri, 23 Mar 2018, Jakub Jelinek wrote: > On Fri, Mar 23, 2018 at 09:53:53AM +0100, Richard Biener wrote: > > > Something I wasn't really aware, apparently we allow extern vars with > > > incomplete types in "m" and "=m" constrained asm. The problem is that > > > incomplete vars have VOIDmode mode and so their MEMs do as well, > > > apparently > > > everything works with it except a 2013-ish assert in alias.c. > > > > > > The following patch just makes sure x_mode is not VOIDmode if x doesn't > > > have > > > VOIDmode, if x has VOIDmode, then it is fine for x_mode to be VOIDmode > > > too. > > > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > OK. > > > > The question now is whether we handle dependences correctly for > > VOIDmode MEMs? Do they have !MEM_SIZE_KNOWN_P? > > They have MEM_SIZE_KNOWN_P true unfortunately. > set_mem_attributes_minus_bitpos starts with: > defattrs = mode_mem_attrs[(int) GET_MODE (ref)]; > gcc_assert (!defattrs->expr); > gcc_assert (!defattrs->offset_known_p); > > /* Respect mode size. */ > attrs.size_known_p = defattrs->size_known_p; > attrs.size = defattrs->size; > and because both TYPE_SIZE_UNIT and DECL_SIZE_UNIT are NULL, nothing updates > the size_known_p. > Wonder if we just shouldn't make mode_mem_attrs[VOIDmode]->size_known_p > false and mode_mem_attrs[VOIDmode]->size NULL, like we do for BLKmode? > >for (i = 0; i < (int) MAX_MACHINE_MODE; i++) > { >mode = (machine_mode) i; >attrs = ggc_cleared_alloc (); >attrs->align = BITS_PER_UNIT; >attrs->addrspace = ADDR_SPACE_GENERIC; > - if (mode != BLKmode) > + if (mode != BLKmode && mode != VOIDmode) > { >attrs->size_known_p = true; >attrs->size = GET_MODE_SIZE (mode); >if (STRICT_ALIGNMENT) > attrs->align = GET_MODE_ALIGNMENT (mode); > } > > ? Yes, that sounds like a very good idea. Pre-approved if it passes testing. Richard.
Re: [PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi
On 23/03/18 08:47, Christophe Lyon wrote: Hi Sudi, On 22 March 2018 at 18:26, Sudakshina Das wrote: Hi On 22/03/18 16:52, Kyrill Tkachov wrote: On 22/03/18 16:20, Sudakshina Das wrote: Hi Kyrill On 22/03/18 16:08, Kyrill Tkachov wrote: Hi Sudi, On 21/03/18 17:44, Sudakshina Das wrote: Hi The ICE in the bug report was happening because the macro USE_RETURN_INSN (FALSE) was returning different values at different points in the compilation. This was internally occurring because the function arm_compute_static_chain_stack_bytes () which was dependent on arm_r3_live_at_start_p () was giving a different value after the cond_exec instructions were created in ce3 causing the liveness of r3 to escape up to the start block. The function arm_compute_static_chain_stack_bytes () should really only compute the value once during epilogue/prologue stage. This pass introduces a new member 'static_chain_stack_bytes' to the target definition of the struct machine_function which gets calculated in expand_prologue and is the value that is returned by arm_compute_static_chain_stack_bytes () beyond that. Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf and added the reported test to the testsuite. Is this ok for trunk? Thanks for working on this. I agree with the approach, I have a couple of comments inline. Sudi ChangeLog entries: *** gcc/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * config/arm/arm.h (machine_function): Add static_chain_stack_bytes. * config/arm/arm.c (arm_compute_static_chain_stack_bytes): Avoid re-computing once computed. (arm_expand_prologue): Compute machine->static_chain_stack_bytes. (arm_init_machine_status): Initialize machine->static_chain_stack_bytes. *** gcc/testsuite/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: New test The new test fails on arm-none-linux-gnueabi --with-mode thumb --with-cpu cortex-a9 --with-fpu default Dejagnu flags: -march=armv5t Because: /gcc/testsuite/gcc.target/arm/pr84826.c: In function 'a': /gcc/testsuite/gcc.target/arm/pr84826.c:15:1: sorry, unimplemented: -fstack-check=specific for Thumb-1 compiler exited with status 1 FAIL: gcc.target/arm/pr84826.c (test for excess errors) You probably have to add a require-effective-target to skip the test in such cases. Yeah, these tests need a { dg-require-effective-target supports_stack_clash_protection } A patch to add that is pre-approved. Sorry for missing it in review. Kyrill Thanks, Christophe diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index bbf3937..2809112 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function machine_mode thumb1_cc_mode; /* Set to 1 after arm_reorg has started. */ int after_arm_reorg; + /* The number of bytes used to store the static chain register on the + stack, above the stack frame. */ + int static_chain_stack_bytes; } machine_function; #endif diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index cb6ab81..bc31810 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void) static int arm_compute_static_chain_stack_bytes (void) { + /* Once the value is updated from the init value of -1, do not + re-compute. */ + if (cfun->machine->static_chain_stack_bytes != -1) +return cfun->machine->static_chain_stack_bytes; + My concern is that this approach caches the first value that is computed for static_chain_stack_bytes. I believe the layout frame code is called multiple times during register allocation as it goes through the motions and I think we want the last value it computes during reload How about we do something like: if (cfun->machine->static_chain_stack_bytes != -1 &&epilogue_completed) return cfun->machine->static_chain_stack_bytes; ?... /* See the defining assertion in arm_expand_prologue. */ if (IS_NESTED (arm_current_func_type ()) && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) @@ -21699,6 +21704,11 @@ arm_expand_prologue (void) emit_insn (gen_movsi (stack_pointer_rtx, r1)); } + /* Let's compute the static_chain_stack_bytes required and store it. Right + now the value must the -1 as stored by arm_init_machine_status (). */ ... this comment would need to be tweaked as cfun->machine->static_chain_stack_bytes may hold an intermediate value computed in reload or some other point before epilogue_completed. + cfun->machine->static_chain_stack_bytes += arm_compute_static_chain_stack_bytes (); + Maybe I did not understand this completely, but my idea was that I am initializing the value of cfun->machine->static_chain_stack_bytes to be -1 in arm_init_machine_status () and then it stays as -1 all throughout reload and hence the function
Re: [PATCH] Fix alias.c ICE on inline-asm "=m" incomplete operand (PR inline-asm/85022)
On Fri, Mar 23, 2018 at 09:53:53AM +0100, Richard Biener wrote: > > Something I wasn't really aware, apparently we allow extern vars with > > incomplete types in "m" and "=m" constrained asm. The problem is that > > incomplete vars have VOIDmode mode and so their MEMs do as well, apparently > > everything works with it except a 2013-ish assert in alias.c. > > > > The following patch just makes sure x_mode is not VOIDmode if x doesn't have > > VOIDmode, if x has VOIDmode, then it is fine for x_mode to be VOIDmode too. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > OK. > > The question now is whether we handle dependences correctly for > VOIDmode MEMs? Do they have !MEM_SIZE_KNOWN_P? They have MEM_SIZE_KNOWN_P true unfortunately. set_mem_attributes_minus_bitpos starts with: defattrs = mode_mem_attrs[(int) GET_MODE (ref)]; gcc_assert (!defattrs->expr); gcc_assert (!defattrs->offset_known_p); /* Respect mode size. */ attrs.size_known_p = defattrs->size_known_p; attrs.size = defattrs->size; and because both TYPE_SIZE_UNIT and DECL_SIZE_UNIT are NULL, nothing updates the size_known_p. Wonder if we just shouldn't make mode_mem_attrs[VOIDmode]->size_known_p false and mode_mem_attrs[VOIDmode]->size NULL, like we do for BLKmode? for (i = 0; i < (int) MAX_MACHINE_MODE; i++) { mode = (machine_mode) i; attrs = ggc_cleared_alloc (); attrs->align = BITS_PER_UNIT; attrs->addrspace = ADDR_SPACE_GENERIC; - if (mode != BLKmode) + if (mode != BLKmode && mode != VOIDmode) { attrs->size_known_p = true; attrs->size = GET_MODE_SIZE (mode); if (STRICT_ALIGNMENT) attrs->align = GET_MODE_ALIGNMENT (mode); } ? Jakub
Re: [PATCH] Fix PR85020
On Thu, 22 Mar 2018, Jakub Jelinek wrote: > On Thu, Mar 22, 2018 at 03:16:22PM +0100, Richard Biener wrote: > > the upper bound decl is global as well). Jakub, do you remember > > a reason for having any constraints here? I've reworked the > > I don't, but it has been added in the > https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html > patch. It matches what we were using before: > - if (loc == NULL > - && early_dwarf > - && current_function_decl > - && DECL_CONTEXT (rszdecl) == current_function_decl) > when emitting the DW_OP_call4 stuff. > > But more importantly, it seems that resolve_variable_value_in_expr > will not change anything if > tree decl = loc->dw_loc_oprnd1.v.val_decl_ref; > if (DECL_CONTEXT (decl) != current_function_decl) > continue; > Which means resolve_variable_values would never process those > DW_OP_GNU_variable_value you've added, Ah, ok. I guess this check can be simply removed. > and I believe later on > note_variable_value_in_expr will just ICE on it: > if (loc->dw_loc_opc == DW_OP_GNU_variable_value > && loc->dw_loc_oprnd1.val_class == dw_val_class_decl_ref) > ... > if (! ref && (flag_generate_lto || flag_generate_offload)) > { > /* ??? This is somewhat a hack because we do not create DIEs >for variables not in BLOCK trees early but when generating >early LTO output we need the dw_val_class_decl_ref to be >fully resolved. For fat LTO objects we'd also like to >undo this after LTO dwarf output. */ > gcc_assert (DECL_CONTEXT (decl)); > Because DECL_CONTEXT (decl) is NULL, right? It's TRANSLATION_UNIT_DECL for the Ada case (we do have FEs where decls may have NULL context which then means file-scope). > Or is DECL_CONTEXT never NULL in LTO, just TRANSLATION_UNIT_DECL? > If so, perhaps note_variable_value_in_expr will handle those fine, > but for these we really don't want to force any DIEs, but rather just give > up if lookup_decl fails by the time we've processed the whole TU. Yeah, I guess the whole ! ref case should go and instead drop the location expression. Is there some convenient "NULL" OP we can use for or do we really have to prune the attribute refering to it? As the comment says for the FAT part we'd like to keep the decl and defer for later fixup so if there's a way to emit DWARF for a val_decl_ref we should maybe simply fix it up at emission time? I see there's DW_OP_nop but I guess a nop in random places of a DWARF location expression isn't a good way to say for that part. That said, the second hunk of my patch fixes the bootstrap issue with Ada (will double-check), the first hunk is only to try preserving some debug info for the Ada case where the variable parts are not necessarily function-local. Let's delay that for now. I suppose the 2nd hunk is ok. For reference that was Index: gcc/dwarf2out.c === --- gcc/dwarf2out.c (revision 258684) +++ gcc/dwarf2out.c (working copy) @@ -19878,6 +19878,7 @@ rtl_for_decl_location (tree decl) in the current CU, resolve_addr will remove the expression referencing it. */ if (rtl == NULL_RTX + && !(early_dwarf && (flag_generate_lto || flag_generate_offload)) && VAR_P (decl) && !DECL_EXTERNAL (decl) && TREE_STATIC (decl) Richard.
Re: [C++ Patch] PR 84632 ("[8 Regression] internal compiler error: tree check: expected record_type or union_type or qual_union_type, have array_type in reduced_constant_expression_p...")
Hi, On 22/03/2018 23:26, Jason Merrill wrote: On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini wrote: ... with patch ;) If you are curious where the heck that INDIRECT_REF is coming from, is coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr. Hmm, maybe build_vec_init should call itself directly rather than via build_aggr_init in the case of multidimensional arrays. Yes, arranging things like that seems doable. However, yesterday, while fiddling with the idea and instrumenting the code with some gcc_asserts, I noticed that we have yet another tree code to handle, TARGET_EXPR, as in lines #41, #47, #56 of ext/complit12.C, and in that case build_aggr_init is simply called by check_initializer via build_aggr_init_full_exprs, the "normal" path. Well, unless we want to adjust/reject complit12.C too, which clang rejects, in fact with errors on lines #19 and #29 too. The below passes testing. Thanks, Paolo. / Index: cp/init.c === --- cp/init.c (revision 258758) +++ cp/init.c (working copy) @@ -1688,14 +1688,6 @@ build_aggr_init (tree exp, tree init, int flags, t } else { - /* An array may not be initialized use the parenthesized -initialization form -- unless the initializer is "()". */ - if (init && TREE_CODE (init) == TREE_LIST) - { - if (complain & tf_error) - error ("bad array initializer"); - return error_mark_node; - } /* Must arrange to initialize each element of EXP from elements of INIT. */ if (cv_qualified_p (type)) @@ -1705,14 +1697,16 @@ build_aggr_init (tree exp, tree init, int flags, t from_array = (itype && same_type_p (TREE_TYPE (init), TREE_TYPE (exp))); - if (init && !from_array - && !BRACE_ENCLOSED_INITIALIZER_P (init)) + if (init && !BRACE_ENCLOSED_INITIALIZER_P (init) + && (!from_array + || (TREE_CODE (init) != CONSTRUCTOR + && TREE_CODE (init) != INDIRECT_REF + && TREE_CODE (init) != TARGET_EXPR))) { if (complain & tf_error) - permerror (init_loc, "array must be initialized " - "with a brace-enclosed initializer"); - else - return error_mark_node; + error_at (init_loc, "array must be initialized " + "with a brace-enclosed initializer"); + return error_mark_node; } } Index: testsuite/g++.dg/init/array49.C === --- testsuite/g++.dg/init/array49.C (nonexistent) +++ testsuite/g++.dg/init/array49.C (working copy) @@ -0,0 +1,6 @@ +// PR c++/84632 +// { dg-additional-options "-w" } + +class { + &a; // { dg-error "forbids declaration" } +} b[2] = b; // { dg-error "initialized" } Index: testsuite/g++.dg/torture/pr70499.C === --- testsuite/g++.dg/torture/pr70499.C (revision 258758) +++ testsuite/g++.dg/torture/pr70499.C (working copy) @@ -1,5 +1,5 @@ // { dg-do compile } -// { dg-additional-options "-w -fpermissive -Wno-psabi" } +// { dg-additional-options "-w -Wno-psabi" } // { dg-additional-options "-mavx" { target x86_64-*-* i?86-*-* } } typedef double __m256d __attribute__ ((__vector_size__ (32), __may_alias__)); @@ -30,7 +30,7 @@ struct Foo { template __attribute__((__always_inline__)) inline void inlineFunc(Tx hx[]) { Tx x = hx[0], y = hx[1]; -Tx lam[1] = (x*y); +Tx lam[1] = {(x*y)}; } void FooBarFunc () {
C++ PATCH for c++/85033, ICE with enumerator in __builtin_offsetof
In this test: struct S { enum { X }; }; int foo (struct S s) { return s.X; } unlike in C, lookup_member in C++ is able to find X in S. So in the following testcase finish_offsetof happily passes the CONST_DECL X down to fold_offsetof, which then crashes because fold_offsetof_1 doesn't handle CONST_DECLs. I think let's simply disallow 'em in finish_offsetof; we can't take their address anyway. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-23 Marek Polacek PR c++/85033 * semantics.c (finish_offsetof): Don't allow CONST_DECLs. * g++.dg/ext/builtin-offsetof2.C: New test. diff --git gcc/cp/semantics.c gcc/cp/semantics.c index 97fa57ae94e..035e3951574 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -4072,6 +4072,11 @@ finish_offsetof (tree object_ptr, tree expr, location_t loc) } return error_mark_node; } + if (TREE_CODE (expr) == CONST_DECL) +{ + error ("cannot apply % to an enumerator %qD", expr); + return error_mark_node; +} if (REFERENCE_REF_P (expr)) expr = TREE_OPERAND (expr, 0); if (!complete_type_or_else (TREE_TYPE (TREE_TYPE (object_ptr)), object_ptr)) diff --git gcc/testsuite/g++.dg/ext/builtin-offsetof2.C gcc/testsuite/g++.dg/ext/builtin-offsetof2.C index e69de29bb2d..07cc55a65c1 100644 --- gcc/testsuite/g++.dg/ext/builtin-offsetof2.C +++ gcc/testsuite/g++.dg/ext/builtin-offsetof2.C @@ -0,0 +1,7 @@ +// PR c++/85033 + +struct S { + enum { E }; +}; + +int b = __builtin_offsetof(S, E); // { dg-error "cannot apply .offsetof. to an enumerator" } Marek
C++ PATCH for c++/85045, ICE when printing RDIV_EXPR
cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so when we tried to print a RDIV_EXPR, we printed it as a pm-expression. That led to printing it as a cast-expression. That led to printing it as a primary-expression. That led to printing it as a multiplicative expression. That led to printing it as a pm-expression. That led to printing it as a cast-expression. That led to printing it as a primary-expression. That led to... a crash. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-23 Marek Polacek PR c++/85045 * cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression): Handle RDIV_EXPR. * g++.dg/cpp0x/Wnarrowing5.C: New test. diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c index ca7f5e1..8527a326c57 100644 --- gcc/cp/cxx-pretty-print.c +++ gcc/cp/cxx-pretty-print.c @@ -899,11 +899,12 @@ cxx_pretty_printer::multiplicative_expression (tree e) case MULT_EXPR: case TRUNC_DIV_EXPR: case TRUNC_MOD_EXPR: +case RDIV_EXPR: multiplicative_expression (TREE_OPERAND (e, 0)); pp_space (this); if (code == MULT_EXPR) pp_star (this); - else if (code == TRUNC_DIV_EXPR) + else if (code == TRUNC_DIV_EXPR || code == RDIV_EXPR) pp_slash (this); else pp_modulo (this); diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C index e69de29bb2d..9acdc6b0c91 100644 --- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C @@ -0,0 +1,11 @@ +// PR c++/85045 +// { dg-do compile { target c++11 } } + +typedef struct tt { + unsigned short h; +} tt; + +void mainScreen(float a) +{ + tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" } +} Marek
Re: [PATCH] Fix PR85020
On Fri, 23 Mar 2018, Eric Botcazou wrote: > > This should then trigger the use of DW_OP_GNU_variable_value but > > for the Ada ACATS testcase I was looking at to debug the issue > > (c330001, resp. c330001_0-c330001_1.ads) this doesn't work > > because the constraints are not met (we're in global context and > > the upper bound decl is global as well). Jakub, do you remember > > a reason for having any constraints here? I've reworked the > > constraints to allow the globals (eventually using decl_function_context > > on this may also allow other not-quite-local cases? But it already > > allowed local statics). Maybe we can simply drop the restriction > > alltogether? > > Thanks for looking into this. > > > Eric, can you generate a gnat.dg LTO testcase from (reduced) c330001? > > Attached. > > > * gnat.dg/lto22.adb: New test. > * gnat.dg/lto22_pkg1.ad[sb]: New helper. > * gnat.dg/lto22_pkg2.ads: Likewise. Thanks, committed. Richard.
Re: [PATCH] Fix PR85020
On Fri, Mar 23, 2018 at 10:58:15AM +0100, Richard Biener wrote: > On Thu, 22 Mar 2018, Jakub Jelinek wrote: > > > On Thu, Mar 22, 2018 at 03:16:22PM +0100, Richard Biener wrote: > > > the upper bound decl is global as well). Jakub, do you remember > > > a reason for having any constraints here? I've reworked the > > > > I don't, but it has been added in the > > https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01499.html > > patch. It matches what we were using before: > > - if (loc == NULL > > - && early_dwarf > > - && current_function_decl > > - && DECL_CONTEXT (rszdecl) == current_function_decl) > > when emitting the DW_OP_call4 stuff. > > > > But more importantly, it seems that resolve_variable_value_in_expr > > will not change anything if > > tree decl = loc->dw_loc_oprnd1.v.val_decl_ref; > > if (DECL_CONTEXT (decl) != current_function_decl) > > continue; > > Which means resolve_variable_values would never process those > > DW_OP_GNU_variable_value you've added, > > Ah, ok. I guess this check can be simply removed. Actually it should be kept. resolve_variable_values works on expressions that were added to the hash table by the note_variable_value_in_expr function, and there a hash table entry for each containing function and we want to process it only when doing resolve_variable_values for the particular function e.g. if some expression happens to be referenced from multiple hash tables. > > and I believe later on > > note_variable_value_in_expr will just ICE on it: > > if (loc->dw_loc_opc == DW_OP_GNU_variable_value > > && loc->dw_loc_oprnd1.val_class == dw_val_class_decl_ref) > > ... > > if (! ref && (flag_generate_lto || flag_generate_offload)) > > { > > /* ??? This is somewhat a hack because we do not create DIEs > >for variables not in BLOCK trees early but when generating > >early LTO output we need the dw_val_class_decl_ref to be > >fully resolved. For fat LTO objects we'd also like to > >undo this after LTO dwarf output. */ > > gcc_assert (DECL_CONTEXT (decl)); > > Because DECL_CONTEXT (decl) is NULL, right? > > It's TRANSLATION_UNIT_DECL for the Ada case (we do have FEs where > decls may have NULL context which then means file-scope). Anyway, the above code in note_variable_value_in_expr for -flto will ensure nothing is ever added into variable_value_hash and thus resolve_variable_values isn't reall invoked in that case. > Yeah, I guess the whole ! ref case should go and instead drop the > location expression. Is there some convenient "NULL" OP we can There is none, except newly DW_OP_GNU_variable_value referencing a DIE (say DW_TAG_dwarf_procedure with no DW_AT_location/DW_AT_const_value) - that is something that never has a usable value, so it is always optimized out. That said, resolve_addr_in_expr should handle the removal of expressions containing DW_OP_GNU_variable_value with dw_val_class_decl_ref if lookup_decl_die fails. > I suppose the 2nd hunk is ok. For reference that was > > Index: gcc/dwarf2out.c > === > --- gcc/dwarf2out.c (revision 258684) > +++ gcc/dwarf2out.c (working copy) > @@ -19878,6 +19878,7 @@ rtl_for_decl_location (tree decl) > in the current CU, resolve_addr will remove the expression > referencing > it. */ >if (rtl == NULL_RTX > + && !(early_dwarf && (flag_generate_lto || flag_generate_offload)) >&& VAR_P (decl) >&& !DECL_EXTERNAL (decl) >&& TREE_STATIC (decl) Sure, this is obviously ok. Jakub
Re: C++ PATCH for c++/85045, ICE when printing RDIV_EXPR
On Fri, Mar 23, 2018 at 11:49:00AM +0100, Marek Polacek wrote: > cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so when > we tried to print a RDIV_EXPR, we printed it as a pm-expression. That led to > printing it as a cast-expression. That led to printing it as a > primary-expression. That led to printing it as a multiplicative expression. > That led to printing it as a pm-expression. That led to printing it as a > cast-expression. That led to printing it as a primary-expression. That led > to... a crash. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-23 Marek Polacek > > PR c++/85045 > * cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression): > Handle RDIV_EXPR. > > * g++.dg/cpp0x/Wnarrowing5.C: New test. I think that is not enough. This is closely related to c-pretty-print.c, which has: c_pretty_printer::multiplicative_expression case MULT_EXPR: case TRUNC_DIV_EXPR: case TRUNC_MOD_EXPR: case EXACT_DIV_EXPR: case RDIV_EXPR: and is broken: else if (code == TRUNC_DIV_EXPR) pp_slash (this); else pp_modulo (this); That code == TRUNC_DIV_EXPR really should have been code != TRUNC_MOD_EXPR, because all the above expressions are / except MULT_EXPR handled earlier and TRUNC_MOD_EXPR. c_pretty_printer::expression then has: case MULT_EXPR: case TRUNC_MOD_EXPR: case TRUNC_DIV_EXPR: case EXACT_DIV_EXPR: case RDIV_EXPR: multiplicative_expression (e); break; So, I think you want: 1) in cxx_pretty_printer::multiplicative_expression add EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash condition to code != TRUNC_MOD_EXPR 2) in cxx_pretty_printer::expression above the multiplicative_expression call add case EXACT_DIV_EXPR: and case RDIV_EXPR 3) in c_pretty_printer::multiplicative_expression change the code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR. 4) see if you can reproduce the c-pretty-print.c bug in some C testcase, whether you get really % instead of / printed for floating point division. > > diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c > index ca7f5e1..8527a326c57 100644 > --- gcc/cp/cxx-pretty-print.c > +++ gcc/cp/cxx-pretty-print.c > @@ -899,11 +899,12 @@ cxx_pretty_printer::multiplicative_expression (tree e) > case MULT_EXPR: > case TRUNC_DIV_EXPR: > case TRUNC_MOD_EXPR: > +case RDIV_EXPR: >multiplicative_expression (TREE_OPERAND (e, 0)); >pp_space (this); >if (code == MULT_EXPR) > pp_star (this); > - else if (code == TRUNC_DIV_EXPR) > + else if (code == TRUNC_DIV_EXPR || code == RDIV_EXPR) > pp_slash (this); >else > pp_modulo (this); > diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C > gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C > index e69de29bb2d..9acdc6b0c91 100644 > --- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C > +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C > @@ -0,0 +1,11 @@ > +// PR c++/85045 > +// { dg-do compile { target c++11 } } > + > +typedef struct tt { > + unsigned short h; > +} tt; > + > +void mainScreen(float a) > +{ > + tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" } > +} Jakub
Re: [PATCH, PR84660] Fix combine bug with SHIFT_COUNT_TRUNCATED.
On Thu, Mar 22, 2018 at 6:58 PM, Jim Wilson wrote: > On Wed, Mar 21, 2018 at 2:45 AM, Richard Biener > wrote: >> On Tue, Mar 20, 2018 at 11:10 PM, Jim Wilson wrote: >>> This fixes a wrong-code issue on RISC-V, but in theory could be a problem >>> for >>> any SHIFT_COUNT_TRUNCATED target. > >> IMHO the real issue is that SHIFT_COUNT_TRUNCATED is used for >> optimizing away the and early. We then rely on the compiler not >> assuming anything on the value. Like if you'd do that on GIMPLE >> VRP would come along and second-guess you because the shift >> value may not be too large. This combine issue sounds very similar. >> >> So I'm suggesting again to instead provide patterns that >> match (shft A (and B cst)) > > I found a message from 2013 that talks about this. > https://gcc.gnu.org/ml/gcc-patches/2013-11/msg00169.html > I can try looking at this. > > I'd question the timing though. I don't think that trying to rewrite > shift patterns in the RISC-V port at this point in the release process > is a good idea. I think we should fix the combine bug now with a > simple patch, and try reworking the shift support after the gcc-8 > release. I'm leaving the "simple" combiner patch to review by others but for RISC-V you could simply #define SHIFT_COUNT_TRUNCATED to zero to fix the issue. Then add patterns if it turns out to be required to avoid regressions. For example x86 has ;; Avoid useless masking of count operand. (define_insn_and_split "*3_mask" [(set (match_operand:SWI48 0 "nonimmediate_operand") (any_shiftrt:SWI48 (match_operand:SWI48 1 "nonimmediate_operand") (subreg:QI (and:SI (match_operand:SI 2 "register_operand" "c,r") (match_operand:SI 3 "const_int_operand")) 0))) (clobber (reg:CC FLAGS_REG))] "ix86_binary_operator_ok (, mode, operands) && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (mode)-1)) == GET_MODE_BITSIZE (mode)-1 && can_create_pseudo_p ()" "#" "&& 1" [(parallel [(set (match_dup 0) (any_shiftrt:SWI48 (match_dup 1) (match_dup 2))) (clobber (reg:CC FLAGS_REG))])] "operands[2] = gen_lowpart (QImode, operands[2]);" [(set_attr "isa" "*,bmi2")]) not sure why it uses define_insn_and_split here. The splitter exactly re-introduces the issue you hit with SHIFT_COUNT_TRUNCATED simplification... I suppose shift expanders of the x86 backends ensure QImode shift counts here to more reliably match the above. Richard. > Jim
Re: C++ PATCH for c++/85033, ICE with enumerator in __builtin_offsetof
OK. On Fri, Mar 23, 2018 at 6:48 AM, Marek Polacek wrote: > In this test: > > struct S { > enum { X }; > }; > > int > foo (struct S s) > { > return s.X; > } > > unlike in C, lookup_member in C++ is able to find X in S. So in the following > testcase finish_offsetof happily passes the CONST_DECL X down to > fold_offsetof, > which then crashes because fold_offsetof_1 doesn't handle CONST_DECLs. I > think > let's simply disallow 'em in finish_offsetof; we can't take their address > anyway. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-23 Marek Polacek > > PR c++/85033 > * semantics.c (finish_offsetof): Don't allow CONST_DECLs. > > * g++.dg/ext/builtin-offsetof2.C: New test. > > diff --git gcc/cp/semantics.c gcc/cp/semantics.c > index 97fa57ae94e..035e3951574 100644 > --- gcc/cp/semantics.c > +++ gcc/cp/semantics.c > @@ -4072,6 +4072,11 @@ finish_offsetof (tree object_ptr, tree expr, > location_t loc) > } >return error_mark_node; > } > + if (TREE_CODE (expr) == CONST_DECL) > +{ > + error ("cannot apply % to an enumerator %qD", expr); > + return error_mark_node; > +} >if (REFERENCE_REF_P (expr)) > expr = TREE_OPERAND (expr, 0); >if (!complete_type_or_else (TREE_TYPE (TREE_TYPE (object_ptr)), > object_ptr)) > diff --git gcc/testsuite/g++.dg/ext/builtin-offsetof2.C > gcc/testsuite/g++.dg/ext/builtin-offsetof2.C > index e69de29bb2d..07cc55a65c1 100644 > --- gcc/testsuite/g++.dg/ext/builtin-offsetof2.C > +++ gcc/testsuite/g++.dg/ext/builtin-offsetof2.C > @@ -0,0 +1,7 @@ > +// PR c++/85033 > + > +struct S { > + enum { E }; > +}; > + > +int b = __builtin_offsetof(S, E); // { dg-error "cannot apply .offsetof. to > an enumerator" } > > Marek
Re: [C++ Patch] PR 84632 ("[8 Regression] internal compiler error: tree check: expected record_type or union_type or qual_union_type, have array_type in reduced_constant_expression_p...")
On Fri, Mar 23, 2018 at 6:13 AM, Paolo Carlini wrote: > On 22/03/2018 23:26, Jason Merrill wrote: >> >> On Thu, Mar 22, 2018 at 5:39 PM, Paolo Carlini >> wrote: >>> >>> ... with patch ;) >>> >>> If you are curious where the heck that INDIRECT_REF is coming from, is >>> coming from the gimplifier, cp_gimpliify_expr, via build_vec_init. Grrr. >> >> Hmm, maybe build_vec_init should call itself directly rather than via >> build_aggr_init in the case of multidimensional arrays. > > Yes, arranging things like that seems doable. However, yesterday, while > fiddling with the idea and instrumenting the code with some gcc_asserts, I > noticed that we have yet another tree code to handle, TARGET_EXPR, as in > lines #41, #47, #56 of ext/complit12.C, and in that case build_aggr_init is > simply called by check_initializer via build_aggr_init_full_exprs, the > "normal" path. Well, unless we want to adjust/reject complit12.C too, which > clang rejects, in fact with errors on lines #19 and #29 too. The below > passes testing. I think I'd like to allow TARGET_EXPR here, with a comment about compound literals, but avoid INDIRECT_REF with that build_vec_init change. Jason
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva wrote: > fn[0]() ICEs because we end up with addr_expr of a decl, and that > should only happen for artificial or otherwise special internal > functions. For anything else, we should find the decl earlier, but we > don't because we build an indirect_ref or an addr_expr and don't > cancel them out. That's deliberate; we recently changed the C++ front end to defer most folding until genericization time. Jason
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/02/2018 05:55 PM, Cesar Philippidis wrote: @@ -4115,13 +4225,23 @@ nvptx_single (unsigned mask, basic_block from, basic_block to) pred = gen_reg_rtx (BImode); cfun->machine->axis_predicate[mode - GOMP_DIM_WORKER] = pred; } - + It's fine to clean up whitespace, but please do that in separate patches. Committed. Thanks, - Tom [nvptx] Fix whitespace in nvptx_single 2018-03-23 Tom de Vries * config/nvptx/nvptx.c (nvptx_single): Fix whitespace. --- gcc/config/nvptx/nvptx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index b7e3f59..50d7319 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -4100,7 +4100,7 @@ nvptx_single (unsigned mask, basic_block from, basic_block to) pred = gen_reg_rtx (BImode); cfun->machine->axis_predicate[mode - GOMP_DIM_WORKER] = pred; } - + rtx br; if (mode == GOMP_DIM_VECTOR) br = gen_br_true (pred, label);
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/02/2018 05:55 PM, Cesar Philippidis wrote: +/* Loop structure of the function. The entire function is described as + a NULL loop. */ + struct parallel { /* Parent parallel. */ You dropped this comment in "vector_length extension part 1: generalize function and variable names". It's good to add it back, but that needs to be a separate patch. Committed. Thanks, - Tom [nvptx] Re-add removed struct parallel comment 2018-03-23 Tom de Vries * config/nvptx/nvptx.c (struct parallel): Re-add comment. --- gcc/config/nvptx/nvptx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 50d7319..9873449 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -2831,6 +2831,9 @@ struct offload_attrs int max_workers; }; +/* Loop structure of the function. The entire function is described as + a NULL loop. */ + struct parallel { /* Parent parallel. */
Re: C++ PATCH for c++/85045, ICE when printing RDIV_EXPR
On Fri, Mar 23, 2018 at 11:59:04AM +0100, Jakub Jelinek wrote: > On Fri, Mar 23, 2018 at 11:49:00AM +0100, Marek Polacek wrote: > > cxx_pretty_printer::multiplicative_expression didn't handle RDIV_EXPR, so > > when > > we tried to print a RDIV_EXPR, we printed it as a pm-expression. That led > > to > > printing it as a cast-expression. That led to printing it as a > > primary-expression. That led to printing it as a multiplicative expression. > > That led to printing it as a pm-expression. That led to printing it as a > > cast-expression. That led to printing it as a primary-expression. That led > > to... a crash. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > 2018-03-23 Marek Polacek > > > > PR c++/85045 > > * cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression): > > Handle RDIV_EXPR. > > > > * g++.dg/cpp0x/Wnarrowing5.C: New test. > > I think that is not enough. This is closely related to c-pretty-print.c, > which has: > c_pretty_printer::multiplicative_expression > case MULT_EXPR: > case TRUNC_DIV_EXPR: > case TRUNC_MOD_EXPR: > case EXACT_DIV_EXPR: > case RDIV_EXPR: > and is broken: > else if (code == TRUNC_DIV_EXPR) > pp_slash (this); > else > pp_modulo (this); > That code == TRUNC_DIV_EXPR really should have been code != TRUNC_MOD_EXPR, > because all the above expressions are / except MULT_EXPR handled earlier and > TRUNC_MOD_EXPR. > c_pretty_printer::expression > then has: > case MULT_EXPR: > case TRUNC_MOD_EXPR: > case TRUNC_DIV_EXPR: > case EXACT_DIV_EXPR: > case RDIV_EXPR: > multiplicative_expression (e); > break; You're right, sorry for being sloppy. > So, I think you want: > 1) in cxx_pretty_printer::multiplicative_expression add > EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash > condition to code != TRUNC_MOD_EXPR > 2) in cxx_pretty_printer::expression above the multiplicative_expression > call add case EXACT_DIV_EXPR: and case RDIV_EXPR > 3) in c_pretty_printer::multiplicative_expression change the > code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR. Done. > 4) see if you can reproduce the c-pretty-print.c bug in some C testcase, > whether you get really % instead of / printed for floating point division. I've found one, though it's quite bizzare. But it showed that we were printing "a % b" despite the user code having "a / b". Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-23 Marek Polacek PR c++/85045 * c-pretty-print.c (c_pretty_printer::multiplicative_expression) : Tweak condition. * cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression): Handle EXACT_DIV_EXPR and RDIV_EXPR. Tweak condition. (cxx_pretty_printer::expression): Handle EXACT_DIV_EXPR and RDIV_EXPR. * g++.dg/cpp0x/Wnarrowing5.C: New test. * gcc.dg/pr85045.c: New test. diff --git gcc/c-family/c-pretty-print.c gcc/c-family/c-pretty-print.c index c9dd8aefff9..dc76c9957d3 100644 --- gcc/c-family/c-pretty-print.c +++ gcc/c-family/c-pretty-print.c @@ -1841,7 +1841,7 @@ c_pretty_printer::multiplicative_expression (tree e) pp_c_whitespace (this); if (code == MULT_EXPR) pp_c_star (this); - else if (code == TRUNC_DIV_EXPR) + else if (code != TRUNC_MOD_EXPR) pp_slash (this); else pp_modulo (this); diff --git gcc/cp/cxx-pretty-print.c gcc/cp/cxx-pretty-print.c index ca7f5e1..9a5545cb5cc 100644 --- gcc/cp/cxx-pretty-print.c +++ gcc/cp/cxx-pretty-print.c @@ -899,11 +899,13 @@ cxx_pretty_printer::multiplicative_expression (tree e) case MULT_EXPR: case TRUNC_DIV_EXPR: case TRUNC_MOD_EXPR: +case EXACT_DIV_EXPR: +case RDIV_EXPR: multiplicative_expression (TREE_OPERAND (e, 0)); pp_space (this); if (code == MULT_EXPR) pp_star (this); - else if (code == TRUNC_DIV_EXPR) + else if (code != TRUNC_MOD_EXPR) pp_slash (this); else pp_modulo (this); @@ -1113,6 +1115,8 @@ cxx_pretty_printer::expression (tree t) case MULT_EXPR: case TRUNC_DIV_EXPR: case TRUNC_MOD_EXPR: +case EXACT_DIV_EXPR: +case RDIV_EXPR: multiplicative_expression (t); break; diff --git gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C index e69de29bb2d..d2abf03313e 100644 --- gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C +++ gcc/testsuite/g++.dg/cpp0x/Wnarrowing5.C @@ -0,0 +1,11 @@ +// PR c++/85045 +// { dg-do compile { target c++11 } } + +typedef struct tt { + unsigned short h; +} tt; + +void mainScreen(float a) +{ + tt numlrect = {int(100/a)}; // { dg-error "narrowing conversion" } +} diff --git gcc/testsuite/gcc.dg/pr85045.c gcc/testsuite/gcc.dg/pr85045.c index e69de29bb2d..8c4a7aa36a4 100644 --- gcc/testsuite/gcc.dg/pr85045.c +++ gcc/testsuite/gcc.dg/pr85045.c @@ -0,0 +1,9 @@ +/* PR c/85045 *
Re: [PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi
On 23/03/18 09:12, Kyrill Tkachov wrote: On 23/03/18 08:47, Christophe Lyon wrote: Hi Sudi, On 22 March 2018 at 18:26, Sudakshina Das wrote: Hi On 22/03/18 16:52, Kyrill Tkachov wrote: On 22/03/18 16:20, Sudakshina Das wrote: Hi Kyrill On 22/03/18 16:08, Kyrill Tkachov wrote: Hi Sudi, On 21/03/18 17:44, Sudakshina Das wrote: Hi The ICE in the bug report was happening because the macro USE_RETURN_INSN (FALSE) was returning different values at different points in the compilation. This was internally occurring because the function arm_compute_static_chain_stack_bytes () which was dependent on arm_r3_live_at_start_p () was giving a different value after the cond_exec instructions were created in ce3 causing the liveness of r3 to escape up to the start block. The function arm_compute_static_chain_stack_bytes () should really only compute the value once during epilogue/prologue stage. This pass introduces a new member 'static_chain_stack_bytes' to the target definition of the struct machine_function which gets calculated in expand_prologue and is the value that is returned by arm_compute_static_chain_stack_bytes () beyond that. Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf and added the reported test to the testsuite. Is this ok for trunk? Thanks for working on this. I agree with the approach, I have a couple of comments inline. Sudi ChangeLog entries: *** gcc/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * config/arm/arm.h (machine_function): Add static_chain_stack_bytes. * config/arm/arm.c (arm_compute_static_chain_stack_bytes): Avoid re-computing once computed. (arm_expand_prologue): Compute machine->static_chain_stack_bytes. (arm_init_machine_status): Initialize machine->static_chain_stack_bytes. *** gcc/testsuite/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: New test The new test fails on arm-none-linux-gnueabi --with-mode thumb --with-cpu cortex-a9 --with-fpu default Dejagnu flags: -march=armv5t Because: /gcc/testsuite/gcc.target/arm/pr84826.c: In function 'a': /gcc/testsuite/gcc.target/arm/pr84826.c:15:1: sorry, unimplemented: -fstack-check=specific for Thumb-1 compiler exited with status 1 FAIL: gcc.target/arm/pr84826.c (test for excess errors) You probably have to add a require-effective-target to skip the test in such cases. Yeah, these tests need a { dg-require-effective-target supports_stack_clash_protection } A patch to add that is pre-approved. Sorry for missing it in review. Kyrill Hi Christophe and Kyrill How about the attached patch? { dg-require-effective-target supports_stack_clash_protection } is not enabled for any of ARM targets, so this is my work around for that. There is a comment in target-supports.exp which makes me a little hesitant in tinkering with the require effective target code. proc check_effective_target_supports_stack_clash_protection { } { # Temporary until the target bits are fully ACK'd. # if { [istarget aarch*-*-*] } { # return 1 # } if { [istarget x86_64-*-*] || [istarget i?86-*-*] || [istarget powerpc*-*-*] || [istarget rs6000*-*-*] || [istarget s390*-*-*] } { return 1 } return 0 } I have opened a new PR 85005 which mentions this that is meant for GCC 9 as part for a bigger cleanup and redesign of the stack clash protection code on ARM backend. *** gcc/testsuite/ChangeLog *** 2018-03-23 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: Add dg directive. Thanks Sudi Thanks, Christophe diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index bbf3937..2809112 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function machine_mode thumb1_cc_mode; /* Set to 1 after arm_reorg has started. */ int after_arm_reorg; + /* The number of bytes used to store the static chain register on the + stack, above the stack frame. */ + int static_chain_stack_bytes; } machine_function; #endif diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index cb6ab81..bc31810 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void) static int arm_compute_static_chain_stack_bytes (void) { + /* Once the value is updated from the init value of -1, do not + re-compute. */ + if (cfun->machine->static_chain_stack_bytes != -1) + return cfun->machine->static_chain_stack_bytes; + My concern is that this approach caches the first value that is computed for static_chain_stack_bytes. I believe the layout frame code is called multiple times during register allocation as it goes through the motions and I think we want the last value it computes during reload How about we do something like: if (cfun->machine->static_chai
Re: [C++ PATCH] Fix error-recovery in compute_array_index_type (PR c++/85015)
On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek wrote: > We ICE during error-recovery on the following testcase, > compute_array_index_type > checks size for error_operand_p early (and it is not error operand; it is > > where c is of reference type with > error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns > error_mark_node), but we let it go > through maybe_constant_size etc. and because error_mark_node is > !TREE_CONSTANT, set it back to the original size. Hmm, maybe we need to also update osize with the result of mark_rvalue_use. Or flip the logic of the constant value block to use a local variable and only change size if the result is constant. Jason
[PATCH] Add a testcase for already fixed PR c/80778
Hi! This testcase has been already fixed on the trunk with r257620 aka PR84305 fix. I've tested and committed as obvious following testcase so that PR80778 can be closed. 2018-03-23 Jakub Jelinek PR c/80778 * gcc.dg/lto/pr80778_0.c: New test. --- gcc/testsuite/gcc.dg/lto/pr80778_0.c.jj 2018-03-23 11:42:03.210732409 +0100 +++ gcc/testsuite/gcc.dg/lto/pr80778_0.c2018-03-23 11:41:58.087734143 +0100 @@ -0,0 +1,5 @@ +/* PR c/80778 */ +/* { dg-lto-do link } */ +/* { dg-require-effective-target alloca } */ + +#include "../auto-type-1.c" Jakub
Re: [PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi
On 23/03/18 13:31, Sudakshina Das wrote: On 23/03/18 09:12, Kyrill Tkachov wrote: On 23/03/18 08:47, Christophe Lyon wrote: Hi Sudi, On 22 March 2018 at 18:26, Sudakshina Das wrote: Hi On 22/03/18 16:52, Kyrill Tkachov wrote: On 22/03/18 16:20, Sudakshina Das wrote: Hi Kyrill On 22/03/18 16:08, Kyrill Tkachov wrote: Hi Sudi, On 21/03/18 17:44, Sudakshina Das wrote: Hi The ICE in the bug report was happening because the macro USE_RETURN_INSN (FALSE) was returning different values at different points in the compilation. This was internally occurring because the function arm_compute_static_chain_stack_bytes () which was dependent on arm_r3_live_at_start_p () was giving a different value after the cond_exec instructions were created in ce3 causing the liveness of r3 to escape up to the start block. The function arm_compute_static_chain_stack_bytes () should really only compute the value once during epilogue/prologue stage. This pass introduces a new member 'static_chain_stack_bytes' to the target definition of the struct machine_function which gets calculated in expand_prologue and is the value that is returned by arm_compute_static_chain_stack_bytes () beyond that. Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf and added the reported test to the testsuite. Is this ok for trunk? Thanks for working on this. I agree with the approach, I have a couple of comments inline. Sudi ChangeLog entries: *** gcc/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * config/arm/arm.h (machine_function): Add static_chain_stack_bytes. * config/arm/arm.c (arm_compute_static_chain_stack_bytes): Avoid re-computing once computed. (arm_expand_prologue): Compute machine->static_chain_stack_bytes. (arm_init_machine_status): Initialize machine->static_chain_stack_bytes. *** gcc/testsuite/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: New test The new test fails on arm-none-linux-gnueabi --with-mode thumb --with-cpu cortex-a9 --with-fpu default Dejagnu flags: -march=armv5t Because: /gcc/testsuite/gcc.target/arm/pr84826.c: In function 'a': /gcc/testsuite/gcc.target/arm/pr84826.c:15:1: sorry, unimplemented: -fstack-check=specific for Thumb-1 compiler exited with status 1 FAIL: gcc.target/arm/pr84826.c (test for excess errors) You probably have to add a require-effective-target to skip the test in such cases. Yeah, these tests need a { dg-require-effective-target supports_stack_clash_protection } A patch to add that is pre-approved. Sorry for missing it in review. Kyrill Hi Christophe and Kyrill How about the attached patch? { dg-require-effective-target supports_stack_clash_protection } is not enabled for any of ARM targets, so this is my work around for that. There is a comment in target-supports.exp which makes me a little hesitant in tinkering with the require effective target code. proc check_effective_target_supports_stack_clash_protection { } { # Temporary until the target bits are fully ACK'd. # if { [istarget aarch*-*-*] } { # return 1 # } if { [istarget x86_64-*-*] || [istarget i?86-*-*] || [istarget powerpc*-*-*] || [istarget rs6000*-*-*] || [istarget s390*-*-*] } { return 1 } return 0 } I have opened a new PR 85005 which mentions this that is meant for GCC 9 as part for a bigger cleanup and redesign of the stack clash protection code on ARM backend. Ok. Thanks for doing this. Kyrill *** gcc/testsuite/ChangeLog *** 2018-03-23 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: Add dg directive. Thanks Sudi Thanks, Christophe diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index bbf3937..2809112 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function machine_mode thumb1_cc_mode; /* Set to 1 after arm_reorg has started. */ int after_arm_reorg; + /* The number of bytes used to store the static chain register on the + stack, above the stack frame. */ + int static_chain_stack_bytes; } machine_function; #endif diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index cb6ab81..bc31810 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void) static int arm_compute_static_chain_stack_bytes (void) { + /* Once the value is updated from the init value of -1, do not + re-compute. */ + if (cfun->machine->static_chain_stack_bytes != -1) +return cfun->machine->static_chain_stack_bytes; + My concern is that this approach caches the first value that is computed for static_chain_stack_bytes. I believe the layout frame code is called multiple times during register allocation as it goes through the motions and I think we want the last value it computes during reload How abo
Re: [PATCH][ARM][PR target/84826] Fix ICE in extract_insn, at recog.c:2304 on arm-linux-gnueabi
On 23/03/18 13:50, Kyrill Tkachov wrote: On 23/03/18 13:31, Sudakshina Das wrote: On 23/03/18 09:12, Kyrill Tkachov wrote: On 23/03/18 08:47, Christophe Lyon wrote: Hi Sudi, On 22 March 2018 at 18:26, Sudakshina Das wrote: Hi On 22/03/18 16:52, Kyrill Tkachov wrote: On 22/03/18 16:20, Sudakshina Das wrote: Hi Kyrill On 22/03/18 16:08, Kyrill Tkachov wrote: Hi Sudi, On 21/03/18 17:44, Sudakshina Das wrote: Hi The ICE in the bug report was happening because the macro USE_RETURN_INSN (FALSE) was returning different values at different points in the compilation. This was internally occurring because the function arm_compute_static_chain_stack_bytes () which was dependent on arm_r3_live_at_start_p () was giving a different value after the cond_exec instructions were created in ce3 causing the liveness of r3 to escape up to the start block. The function arm_compute_static_chain_stack_bytes () should really only compute the value once during epilogue/prologue stage. This pass introduces a new member 'static_chain_stack_bytes' to the target definition of the struct machine_function which gets calculated in expand_prologue and is the value that is returned by arm_compute_static_chain_stack_bytes () beyond that. Testing done: Bootstrapped and regtested on arm-none-linux-gnueabihf and added the reported test to the testsuite. Is this ok for trunk? Thanks for working on this. I agree with the approach, I have a couple of comments inline. Sudi ChangeLog entries: *** gcc/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * config/arm/arm.h (machine_function): Add static_chain_stack_bytes. * config/arm/arm.c (arm_compute_static_chain_stack_bytes): Avoid re-computing once computed. (arm_expand_prologue): Compute machine->static_chain_stack_bytes. (arm_init_machine_status): Initialize machine->static_chain_stack_bytes. *** gcc/testsuite/ChangeLog *** 2018-03-21 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: New test The new test fails on arm-none-linux-gnueabi --with-mode thumb --with-cpu cortex-a9 --with-fpu default Dejagnu flags: -march=armv5t Because: /gcc/testsuite/gcc.target/arm/pr84826.c: In function 'a': /gcc/testsuite/gcc.target/arm/pr84826.c:15:1: sorry, unimplemented: -fstack-check=specific for Thumb-1 compiler exited with status 1 FAIL: gcc.target/arm/pr84826.c (test for excess errors) You probably have to add a require-effective-target to skip the test in such cases. Yeah, these tests need a { dg-require-effective-target supports_stack_clash_protection } A patch to add that is pre-approved. Sorry for missing it in review. Kyrill Hi Christophe and Kyrill How about the attached patch? { dg-require-effective-target supports_stack_clash_protection } is not enabled for any of ARM targets, so this is my work around for that. There is a comment in target-supports.exp which makes me a little hesitant in tinkering with the require effective target code. proc check_effective_target_supports_stack_clash_protection { } { # Temporary until the target bits are fully ACK'd. # if { [istarget aarch*-*-*] } { # return 1 # } if { [istarget x86_64-*-*] || [istarget i?86-*-*] || [istarget powerpc*-*-*] || [istarget rs6000*-*-*] || [istarget s390*-*-*] } { return 1 } return 0 } I have opened a new PR 85005 which mentions this that is meant for GCC 9 as part for a bigger cleanup and redesign of the stack clash protection code on ARM backend. Ok. Thanks for doing this. Thanks and sorry about this! Committed as r258805. Sudi Kyrill *** gcc/testsuite/ChangeLog *** 2018-03-23 Sudakshina Das PR target/84826 * gcc.target/arm/pr84826.c: Add dg directive. Thanks Sudi Thanks, Christophe diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index bbf3937..2809112 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -1384,6 +1384,9 @@ typedef struct GTY(()) machine_function machine_mode thumb1_cc_mode; /* Set to 1 after arm_reorg has started. */ int after_arm_reorg; + /* The number of bytes used to store the static chain register on the + stack, above the stack frame. */ + int static_chain_stack_bytes; } machine_function; #endif diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index cb6ab81..bc31810 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19392,6 +19392,11 @@ arm_r3_live_at_start_p (void) static int arm_compute_static_chain_stack_bytes (void) { + /* Once the value is updated from the init value of -1, do not + re-compute. */ + if (cfun->machine->static_chain_stack_bytes != -1) + return cfun->machine->static_chain_stack_bytes; + My concern is that this approach caches the first value that is computed for static_chain_stack_bytes. I believe the layout frame code is called multiple times during
Re: [C++ PATCH] Fix error-recovery in compute_array_index_type (PR c++/85015)
On Fri, Mar 23, 2018 at 09:31:53AM -0400, Jason Merrill wrote: > On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek wrote: > > We ICE during error-recovery on the following testcase, > > compute_array_index_type > > checks size for error_operand_p early (and it is not error operand; it is > > > where c is of reference type with > > error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns > > error_mark_node), but we let it go > > through maybe_constant_size etc. and because error_mark_node is > > !TREE_CONSTANT, set it back to the original size. > > Hmm, maybe we need to also update osize with the result of > mark_rvalue_use. Or flip the logic of the constant value block to use > a local variable and only change size if the result is constant. Like this? Passes check-c++-all... 2018-03-23 Jakub Jelinek PR c++/85015 * decl.c (compute_array_index_type): Return error_mark_node if mark_rvalue_use or maybe_constant_value returns error_operand_p. Set osize to mark_rvalue_use result. * g++.dg/cpp0x/pr85015.C: New test. --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 +++ gcc/cp/decl.c 2018-03-23 14:52:07.274893996 +0100 @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre if (!type_dependent_expression_p (size)) { - size = mark_rvalue_use (size); + osize = size = mark_rvalue_use (size); + + if (error_operand_p (size)) + return error_mark_node; if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR && TREE_SIDE_EFFECTS (size)) @@ -9534,11 +9537,10 @@ compute_array_index_type (tree name, tre if (!TREE_CONSTANT (size)) size = osize; + else if (error_operand_p (size)) + return error_mark_node; } - if (error_operand_p (size)) - return error_mark_node; - /* The array bound must be an integer type. */ tree type = TREE_TYPE (size); if (!INTEGRAL_OR_UNSCOPED_ENUMERATION_TYPE_P (type)) --- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj 2018-03-23 14:51:19.207892516 +0100 +++ gcc/testsuite/g++.dg/cpp0x/pr85015.C2018-03-23 14:51:19.207892516 +0100 @@ -0,0 +1,12 @@ +// PR c++/85015 +// { dg-do compile { target c++11 } } +// { dg-options "" } + +void +foo () +{ + int &&c = v + 1; // { dg-error "was not declared in this scope" } + struct S { // { dg-message "declared here" "" { target *-*-* } .-1 } +void bar () { int a[c]; } // { dg-error "use of local variable with automatic storage from containing function" } + } e; +} Jakub
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/02/2018 05:55 PM, Cesar Philippidis wrote: diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md index 28ae263c867..ac2731233dd 100644 --- a/gcc/config/nvptx/nvptx.md +++ b/gcc/config/nvptx/nvptx.md @@ -1418,10 +1418,16 @@ [(set_attr "atomic" "true")]) (define_insn "nvptx_barsync" - [(unspec_volatile [(match_operand:SI 0 "const_int_operand" "")] + [(unspec_volatile [(match_operand:SI 0 "nvptx_nonmemory_operand" "Ri") +(match_operand:SI 1 "const_int_operand")] UNSPECV_BARSYNC)] "" - "\\tbar.sync\\t%0;" + { +if (!REG_P (operands[0])) + return "\\tbar.sync\\t%0;"; +else + return "\\tbar.sync\\t%0, %1;"; + } [(set_attr "predicable" "false")]) This is wrong. The first operand can be a register or a constant, and the second operand is independent. Whether or not we print the second operand is independent of whether the first is a register. In this patch I've reserved INTVAL (operands[1]) == 0 for the "no second operand" case. Committed. Thanks, - Tom [nvptx] Add thread count parm to bar.sync 2018-03-23 Tom de Vries * config/nvptx/nvptx.md (nvptx_barsync): Add and handle operand. * config/nvptx/nvptx.c (nvptx_cta_sync): Change arguments to take in a lock and thread count. Update call to gen_nvptx_barsync. (nvptx_single, nvptx_process_pars): Update calls to nvptx_cta_sync. --- gcc/config/nvptx/nvptx.c | 22 ++ gcc/config/nvptx/nvptx.md | 10 -- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 12441cb..32f2efb 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -3939,13 +3939,14 @@ nvptx_shared_propagate (bool pre_p, bool is_call, basic_block block, return empty; } -/* Emit a CTA-level synchronization barrier. We use different - markers for before and after synchronizations. */ +/* Emit a CTA-level synchronization barrier (bar.sync). LOCK is the + barrier number, which is an integer or a register. THREADS is the + number of threads controlled by the barrier. */ static rtx -nvptx_cta_sync (bool after) +nvptx_cta_sync (rtx lock, int threads) { - return gen_nvptx_barsync (GEN_INT (after)); + return gen_nvptx_barsync (lock, GEN_INT (threads)); } #if WORKAROUND_PTXJIT_BUG @@ -4195,6 +4196,8 @@ nvptx_single (unsigned mask, basic_block from, basic_block to) /* Includes worker mode, do spill & fill. By construction we should never have worker mode only. */ broadcast_data_t data; + rtx barrier = GEN_INT (0); + int threads = 0; data.base = oacc_bcast_sym; data.ptr = 0; @@ -4207,14 +4210,14 @@ nvptx_single (unsigned mask, basic_block from, basic_block to) false), before); /* Barrier so other workers can see the write. */ - emit_insn_before (nvptx_cta_sync (false), tail); + emit_insn_before (nvptx_cta_sync (barrier, threads), tail); data.offset = 0; emit_insn_before (nvptx_gen_shared_bcast (pvar, PM_write, 0, &data, false), tail); /* This barrier is needed to avoid worker zero clobbering the broadcast buffer before all the other workers have had a chance to read this instance of it. */ - emit_insn_before (nvptx_cta_sync (false), tail); + emit_insn_before (nvptx_cta_sync (barrier, threads), tail); } extract_insn (tail); @@ -4331,12 +4334,15 @@ nvptx_process_pars (parallel *par) bool empty = nvptx_shared_propagate (true, is_call, par->forked_block, par->fork_insn, false); + rtx barrier = GEN_INT (0); + int threads = 0; if (!empty || !is_call) { /* Insert begin and end synchronizations. */ - emit_insn_before (nvptx_cta_sync (false), par->forked_insn); - emit_insn_before (nvptx_cta_sync (false), par->join_insn); + emit_insn_before (nvptx_cta_sync (barrier, threads), + par->forked_insn); + emit_insn_before (nvptx_cta_sync (barrier, threads), par->join_insn); } } else if (par->mask & GOMP_DIM_MASK (GOMP_DIM_VECTOR)) diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md index 2b4bcb3a..2609222 100644 --- a/gcc/config/nvptx/nvptx.md +++ b/gcc/config/nvptx/nvptx.md @@ -1421,10 +1421,16 @@ [(set_attr "atomic" "true")]) (define_insn "nvptx_barsync" - [(unspec_volatile [(match_operand:SI 0 "const_int_operand" "")] + [(unspec_volatile [(match_operand:SI 0 "nvptx_nonmemory_operand" "Ri") + (match_operand:SI 1 "const_int_operand")] UNSPECV_BARSYNC)] "" - "\\tbar.sync\\t%0;" + { +if (INTVAL (operands[1]) == 0) + return "\\tbar.sync\\t%0;"; +else + return "\\tbar.sync\\t%0, %1;"; + } [(set_attr "predicable" "false")]) (define_insn "nvptx_nounroll"
Re: [C++ PATCH] Fix FIX_TRUNC_EXPR instantiation (PR c++/84942)
On Thu, Mar 22, 2018 at 02:02:01PM -0400, Jason Merrill wrote: > He hadn't yet checked in the relevant change, "Disable > auto_is_implicit_function_template_parm_p while parsing attributes". > That should happen soon. > > > but with > > the following patch we don't ICE, because args is NULL and > > tsubst_copy starts with: > > 14945 if (t == NULL_TREE || t == error_mark_node || args == NULL_TREE) > > 14946 return t; > > So, do you still want the FIX_TRUNC_EXPR handling removed or fixed (as done > > in the first patch)? > > Hmm, let's make it gcc_unreachable then. So like this? Passes make check-c++-all on current trunk, where the above patch from Alex is already in. 2018-03-23 Jakub Jelinek PR c++/84942 * pt.c (tsubst_copy_and_build) : Replace cp_build_unary_op call with gcc_unreachable (). * g++.dg/cpp1y/pr84942.C: New test. --- gcc/cp/pt.c.jj 2018-03-23 09:49:38.063519286 +0100 +++ gcc/cp/pt.c 2018-03-23 09:51:24.232501064 +0100 @@ -17514,8 +17514,7 @@ tsubst_copy_and_build (tree t, complain|decltype_flag)); case FIX_TRUNC_EXPR: - RETURN (cp_build_unary_op (FIX_TRUNC_EXPR, RECUR (TREE_OPERAND (t, 0)), -false, complain)); + gcc_unreachable (); case ADDR_EXPR: op1 = TREE_OPERAND (t, 0); --- gcc/testsuite/g++.dg/cpp1y/pr84942.C.jj 2018-03-23 09:50:16.320512716 +0100 +++ gcc/testsuite/g++.dg/cpp1y/pr84942.C2018-03-23 09:52:33.408489183 +0100 @@ -0,0 +1,6 @@ +// PR c++/84942 +// { dg-do compile { target c++14 } } +// { dg-options "-w" } + +int a(__attribute__((b((int)__builtin_inf() * 1ULL / auto; +// { dg-error "expected primary-expression before" "" { target *-*-* } .-1 } Jakub
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/22/2018 06:24 PM, Cesar Philippidis wrote: On 03/22/2018 09:18 AM, Tom de Vries wrote: That's obviously not good enough. When I compile this test-case: ... int main (void) { int a[10]; #pragma acc parallel num_workers (16) #pragma acc loop worker for (int i = 0; i < 10; i++) a[i] = i; return 0; } ... I get: ... .maxntid 32, 16, 1 ... That's the change you need to isolate. I attached an updated patch which incorporates the cfun->machine->axis_dim changes. It now generates more precise arguments for maxntid. Even with maxntid dropped, axis_dim is still used elsewhere in the patch series, so we can split off the introduction of axis_dim and helper functions in a separate patch. Committed. Thanks, - Tom Cesar 0001-emit-.maxntid-hint.patch From 11035dc92884146dc4d974156adcb260568db785 Mon Sep 17 00:00:00 2001 From: Cesar Philippidis Date: Thu, 22 Mar 2018 08:05:53 -0700 Subject: [PATCH] emit .maxntid hint --- gcc/config/nvptx/nvptx.c | 19 +++ gcc/config/nvptx/nvptx.h | 2 ++ 2 files changed, 21 insertions(+) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index eff87732c4b..3958f71e995 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -76,6 +76,7 @@ #include "target-def.h" #define WORKAROUND_PTXJIT_BUG 1 +#define WORKAROUND_PTXJIT_BUG_3 1 /* Define dimension sizes for known hardware. */ #define PTX_VECTOR_LENGTH 32 @@ -1219,6 +1220,16 @@ nvptx_declare_function_name (FILE *file, const char *name, const_tree decl) stream, in order to share the prototype writing code. */ std::stringstream s; write_fn_proto (s, true, name, decl); + +#if WORKAROUND_PTXJIT_BUG_3 + /* Emitting a .maxntid seems to have the effect of encouraging the + PTX JIT emit SYNC branches. */ + if (lookup_attribute ("omp target entrypoint", DECL_ATTRIBUTES (decl)) + && lookup_attribute ("oacc function", DECL_ATTRIBUTES (decl))) + s << ".maxntid " << cfun->machine->axis_dim[0] << ", " + << cfun->machine->axis_dim[1] << ", 1\n"; +#endif + s << "{\n"; bool return_in_mem = write_return_type (s, false, result_type); @@ -2831,6 +2842,11 @@ struct offload_attrs int max_workers; }; +/* Define entries for cfun->machine->axis_dim. */ + +#define MACH_VECTOR_LENGTH 0 +#define MACH_MAX_WORKERS 1 + struct parallel { /* Parent parallel. */ @@ -4525,6 +4541,9 @@ nvptx_reorg (void) populate_offload_attrs (&oa); + cfun->machine->axis_dim[MACH_VECTOR_LENGTH] = oa.vector_length; + cfun->machine->axis_dim[MACH_MAX_WORKERS] = oa.max_workers; + /* If there is worker neutering, there must be vector neutering. Otherwise the hardware will fail. */ gcc_assert (!(oa.mask & GOMP_DIM_MASK (GOMP_DIM_WORKER)) diff --git a/gcc/config/nvptx/nvptx.h b/gcc/config/nvptx/nvptx.h index 8a14507c88a..958516da604 100644 --- a/gcc/config/nvptx/nvptx.h +++ b/gcc/config/nvptx/nvptx.h @@ -226,6 +226,8 @@ struct GTY(()) machine_function int return_mode; /* Return mode of current fn. (machine_mode not defined yet.) */ rtx axis_predicate[2]; /* Neutering predicates. */ + int axis_dim[2]; /* Maximum number of threads on each axis, dim[0] is + vector_length, dim[1] is num_workers. */ rtx unisimt_master; /* 'Master lane index' for -muniform-simt. */ rtx unisimt_predicate; /* Predicate for -muniform-simt. */ rtx unisimt_location; /* Mask location for -muniform-simt. */ [nvptx] Add axis_dim 2018-03-23 Tom de Vries * config/nvptx/nvptx.c (MACH_VECTOR_LENGTH, MACH_MAX_WORKERS): Define. (nvptx_mach_max_workers, nvptx_mach_vector_length): New function. (nvptx_reorg): Set function-specific axis_dim's. * config/nvptx/nvptx.h (struct machine_function): Add axis_dims. --- gcc/config/nvptx/nvptx.c | 20 gcc/config/nvptx/nvptx.h | 2 ++ 3 files changed, 29 insertions(+) diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c index 32f2efb..3cb33ae 100644 --- a/gcc/config/nvptx/nvptx.c +++ b/gcc/config/nvptx/nvptx.c @@ -2831,6 +2831,23 @@ struct offload_attrs int max_workers; }; +/* Define entries for cfun->machine->axis_dim. */ + +#define MACH_VECTOR_LENGTH 0 +#define MACH_MAX_WORKERS 1 + +static int ATTRIBUTE_UNUSED +nvptx_mach_max_workers () +{ + return cfun->machine->axis_dim[MACH_MAX_WORKERS]; +} + +static int ATTRIBUTE_UNUSED +nvptx_mach_vector_length () +{ + return cfun->machine->axis_dim[MACH_VECTOR_LENGTH]; +} + /* Loop structure of the function. The entire function is described as a NULL loop. */ @@ -4534,6 +4551,9 @@ nvptx_reorg (void) populate_offload_attrs (&oa); + cfun->machine->axis_dim[MACH_VECTOR_LENGTH] = oa.vector_length; + cfun->machine->axis_dim[MACH_MAX_WORKERS] = oa.max_workers; + /* If there is worker neutering, there must be vector neutering. Otherwise the hardware will fail.
Re: C++ PATCH for c++/85045, ICE when printing RDIV_EXPR
On Fri, Mar 23, 2018 at 02:16:32PM +0100, Marek Polacek wrote: > > So, I think you want: > > 1) in cxx_pretty_printer::multiplicative_expression add > > EXACT_DIV_EXPR and (like you did) RDIV_EXPR, and change the pp_slash > > condition to code != TRUNC_MOD_EXPR > > 2) in cxx_pretty_printer::expression above the multiplicative_expression > > call add case EXACT_DIV_EXPR: and case RDIV_EXPR > > 3) in c_pretty_printer::multiplicative_expression change the > > code == TRUNC_DIV_EXPR condition to code != TRUNC_MOD_EXPR. > > Done. > > > 4) see if you can reproduce the c-pretty-print.c bug in some C testcase, > > whether you get really % instead of / printed for floating point division. > > I've found one, though it's quite bizzare. But it showed that we were > printing > "a % b" despite the user code having "a / b". > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-23 Marek Polacek > > PR c++/85045 > * c-pretty-print.c (c_pretty_printer::multiplicative_expression) > : Tweak condition. > > * cxx-pretty-print.c (cxx_pretty_printer::multiplicative_expression): > Handle EXACT_DIV_EXPR and RDIV_EXPR. Tweak condition. > (cxx_pretty_printer::expression): Handle EXACT_DIV_EXPR and RDIV_EXPR. > > * g++.dg/cpp0x/Wnarrowing5.C: New test. > * gcc.dg/pr85045.c: New test. Ok, thanks. Jakub
Re: [PR c++/84789] do not resolve typename into template-independent
On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: > OK, thanks. Note the testcase FAILs with -fconcepts when I do make check-c++-all, FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12) FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors) Excess errors: /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'A::I' {aka 'int'} is not a class type /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' in 'A::I' {aka 'int'} does not name a type In all other standard modes it is fine. Jakub
Re: [RFC Patch], PowerPC memory support pre-gcc9, Version 2, Patch #11
This is the last of the infrastructure patches that I have currently done. This adds a new reg_addr flag to note whether the d-form address is a ds-form (bottom 2 bits must be 0). At present, nothing uses this, but I have plans for it in the future. 2018-03-22 Michael Meissner * config/rs6000/rs6000.c (addr_mask_type): Grow mask to 16 bits. (RELOAD_REG_DS_OFFSET): New mask for DS-form addresses. (mode_supports_ds_form): New helper function to return if a reload register class uses DS-form addresses. (rs6000_debug_addr_mask): Print if we have DS-form addresses. (rs6000_setup_reg_addr_masks): Note which reload register classes use DS-form addresses. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797 Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 258782) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -503,16 +503,17 @@ static const struct reload_reg_map_type /* Mask bits for each register class, indexed per mode. Historically the compiler has been more restrictive which types can do PRE_MODIFY instead of PRE_INC and PRE_DEC, so keep track of sepaate bits for these two. */ -typedef unsigned char addr_mask_type; +typedef unsigned short addr_mask_type; -#define RELOAD_REG_VALID 0x01/* Mode valid in register.. */ -#define RELOAD_REG_MULTIPLE0x02/* Mode takes multiple registers. */ -#define RELOAD_REG_INDEXED 0x04/* Reg+reg addressing. */ -#define RELOAD_REG_OFFSET 0x08/* Reg+offset addressing. */ -#define RELOAD_REG_PRE_INCDEC 0x10/* PRE_INC/PRE_DEC valid. */ -#define RELOAD_REG_PRE_MODIFY 0x20/* PRE_MODIFY valid. */ -#define RELOAD_REG_AND_M16 0x40/* AND -16 addressing. */ -#define RELOAD_REG_QUAD_OFFSET 0x80/* quad offset is limited. */ +#define RELOAD_REG_VALID 0x001 /* Mode valid in register.. */ +#define RELOAD_REG_MULTIPLE0x002 /* Mode takes multiple registers. */ +#define RELOAD_REG_INDEXED 0x004 /* Reg+reg addressing. */ +#define RELOAD_REG_OFFSET 0x008 /* Reg+offset addressing. */ +#define RELOAD_REG_PRE_INCDEC 0x010 /* PRE_INC/PRE_DEC valid. */ +#define RELOAD_REG_PRE_MODIFY 0x020 /* PRE_MODIFY valid. */ +#define RELOAD_REG_AND_M16 0x040 /* AND -16 addressing. */ +#define RELOAD_REG_QUAD_OFFSET 0x080 /* quad offset is limited. */ +#define RELOAD_REG_DS_OFFSET 0x100 /* DS-form (bottom 2 bits 0). */ /* Register type masks based on the type, of valid addressing modes. */ struct rs6000_reg_addr { @@ -565,6 +566,16 @@ mode_supports_d_form (machine_mode mode, return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_OFFSET) != 0); } +/* Return true if we have DS-form addressing in a given reload register class + or if some reload register class supports it. DS-form addressing must have + the bottom 2 bits set to 0. */ +static inline bool +mode_supports_ds_form (machine_mode mode, + enum rs6000_reload_reg_type rt = RELOAD_REG_ANY) +{ + return ((reg_addr[mode].addr_mask[rt] & RELOAD_REG_DS_OFFSET) != 0); +} + /* Return true if we have DQ-form addressing in a given reload register class or if some reload register class supports it. DQ-form addressing must have the bottom 4 bits set to 0. */ @@ -2349,6 +2360,8 @@ rs6000_debug_addr_mask (addr_mask_type m if ((mask & RELOAD_REG_QUAD_OFFSET) != 0) *p++ = 'O'; + else if ((mask & RELOAD_REG_DS_OFFSET) != 0) +*p++ = 'O'; else if ((mask & RELOAD_REG_OFFSET) != 0) *p++ = 'o'; else if (keep_spaces) @@ -3035,27 +3048,40 @@ rs6000_setup_reg_addr_masks (void) /* GPR and FPR registers can do REG+OFFSET addressing, except possibly for SDmode. ISA 3.0 (i.e. power9) adds D-form addressing -for 64-bit scalars and 32-bit SFmode to altivec registers. */ - if ((addr_mask != 0) && !indexed_only_p - && msize <= 8 - && (rc == RELOAD_REG_GPR - || ((msize == 8 || m2 == SFmode) - && (rc == RELOAD_REG_FPR - || (rc == RELOAD_REG_VMX && TARGET_P9_VECTOR) - addr_mask |= RELOAD_REG_OFFSET; - - /* VSX registers can do REG+OFFSET addresssing if ISA 3.0 -instructions are enabled. The offset for 128-bit VSX registers is -only 12-bits. While GPRs can handle the full offset range, VSX -registers can only handle the restricted range. */ - else if ((addr_mask != 0) && !indexed_only_p - && msize == 16 && TARGET_P9_VECTOR - && (ALTIVEC_OR_VSX_VECTOR_MODE (m2) - || (m2 == TImode && TARGET_VSX))) - { - addr_mask |= RELOAD_REG_OFFSET; - if (rc == RELOAD_REG_FPR || rc ==
New Spanish PO file for 'gcc' (version 8.1-b20180128)
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 Spanish team of translators. The file is available at: http://translationproject.org/latest/gcc/es.po (This file, 'gcc-8.1-b20180128.es.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.
[PATCH] [PR c++/84973] don't defer output of uninstantiated templates
When an anon struct gets a name through a typedef, we reset its linkage and that of its members. Member functions may get vague linkage, which schedules them for deferred output, but we don't want to add them to the queue if they're uninstantiated templates, e.g. because the enclosing function is a template. They will be added as needed when the enclosing template is instantiated. Regstrapped on i686- and x86_64-linux-gnu. Ok to install? for gcc/cp/ChangeLog PR c++/84973 * decl2.c (note_vague_linkage_fn): Don't defer uninstantiated templates. for gcc/testsuite/ChangeLog PR c++/84973 * g++.dg/template/pr84973.C: New. * g++.dg/template/pr84973-2.C: New. * g++.dg/template/pr84973-3.C: New. --- gcc/cp/decl2.c|3 +++ gcc/testsuite/g++.dg/template/pr84973-2.C | 13 + gcc/testsuite/g++.dg/template/pr84973-3.C | 13 + gcc/testsuite/g++.dg/template/pr84973.C |8 4 files changed, 37 insertions(+) create mode 100644 gcc/testsuite/g++.dg/template/pr84973-2.C create mode 100644 gcc/testsuite/g++.dg/template/pr84973-3.C create mode 100644 gcc/testsuite/g++.dg/template/pr84973.C diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index e522b9ebe55a..fa753749e1a6 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -739,6 +739,9 @@ check_classfn (tree ctype, tree function, tree template_parms) void note_vague_linkage_fn (tree decl) { + if (processing_template_decl) +return; + DECL_DEFER_OUTPUT (decl) = 1; vec_safe_push (deferred_fns, decl); } diff --git a/gcc/testsuite/g++.dg/template/pr84973-2.C b/gcc/testsuite/g++.dg/template/pr84973-2.C new file mode 100644 index ..41c205ad5243 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84973-2.C @@ -0,0 +1,13 @@ +// { dg-do compile } + +template void a() { + typedef struct { +void b() try { b; } catch (short) { // { dg-error "invalid use" } +} + } c; +} + +int +main() { + a<0>(); +} diff --git a/gcc/testsuite/g++.dg/template/pr84973-3.C b/gcc/testsuite/g++.dg/template/pr84973-3.C new file mode 100644 index ..eeac214f2e1b --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84973-3.C @@ -0,0 +1,13 @@ +// { dg-do link } + +template void a() { + typedef struct { +void b() try { b(); } catch (short) { +} + } c; +} + +int +main() { + a<0>(); +} diff --git a/gcc/testsuite/g++.dg/template/pr84973.C b/gcc/testsuite/g++.dg/template/pr84973.C new file mode 100644 index ..b3f7170bc0dc --- /dev/null +++ b/gcc/testsuite/g++.dg/template/pr84973.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +template void a() { + typedef struct { +void b() try { b; } catch (short) { +} + } c; +} -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
[PATCH] [PR c++/84968] reject stmt-exprs in noexcept constexprs
We reject extended statement-expressions in template parameters, so we might as well reject them in constant expressions used in noexcept specifications. Regstrapped on i686- and x86_64-linux-gnu. Ok to install? for gcc/cp/ChangeLog PR c++/84968 * tree.c (strip_typedefs_expr): Reject STATEMENT_LISTs. for gcc/testsuite/ChangeLog PR c++/84968 * g++.dg/eh/pr84968.C: New. --- gcc/cp/tree.c |4 gcc/testsuite/g++.dg/eh/pr84968.C | 15 +++ 2 files changed, 19 insertions(+) create mode 100644 gcc/testsuite/g++.dg/eh/pr84968.C diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index f1a90bdec0fc..070bd11a0591 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -1785,6 +1785,10 @@ strip_typedefs_expr (tree t, bool *remove_attributes) error ("lambda-expression in a constant expression"); return error_mark_node; +case STATEMENT_LIST: + error ("statement-expression in a constant expression"); + return error_mark_node; + default: break; } diff --git a/gcc/testsuite/g++.dg/eh/pr84968.C b/gcc/testsuite/g++.dg/eh/pr84968.C new file mode 100644 index ..23c49f477a88 --- /dev/null +++ b/gcc/testsuite/g++.dg/eh/pr84968.C @@ -0,0 +1,15 @@ +// { dg-do compile { target c++11 } } + +// { dg-options "" } + +union b; + +struct S { + template + void a() +try { +} catch (int () +noexcept (({ union b a; true; }))) // { dg-error "constant" } + { + } +}; -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
[PATCH]Correct comment for ADDR_EXPR tree code.
Hi all, This is a simple patch to correct the comment for ADDR_EXPR tree code. The resulting expression of ADDR_EXPR is a tree with POINTER_TYPE. So the result mode should ptr_mode instead of Pmode. As far as I understand, Pmode is the addressing mode. But not the mode to represent a pointer (or address?). Okay to commit? Regards, Renlin gcc/ChangeLog: 2018-03-23 Renlin Li * tree.def (ADDR_EXPR): Correct the commnet. diff --git a/gcc/tree.def b/gcc/tree.def index 31de6c0994de43c175b924d4ba578a131fb4d524..1e5aca811f801c54be9215a9d86028f50a4ec608 100644 --- a/gcc/tree.def +++ b/gcc/tree.def @@ -870,7 +870,7 @@ DEFTREECODE (COMPOUND_LITERAL_EXPR, "compound_literal_expr", tcc_expression, 1) DEFTREECODE (SAVE_EXPR, "save_expr", tcc_expression, 1) /* & in C. Value is the address at which the operand's value resides. - Operand may have any mode. Result mode is Pmode. */ + Operand may have any mode. Result mode is ptr_mode. */ DEFTREECODE (ADDR_EXPR, "addr_expr", tcc_expression, 1) /* Operand0 is a function constant; result is part N of a function
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Mar 23, 2018, Jason Merrill wrote: > On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva wrote: >> fn[0]() ICEs because we end up with addr_expr of a decl, and that >> should only happen for artificial or otherwise special internal >> functions. For anything else, we should find the decl earlier, but we >> don't because we build an indirect_ref or an addr_expr and don't >> cancel them out. > That's deliberate; we recently changed the C++ front end to defer most > folding until genericization time. Ok, I can see a number of possibilities as to why this is done, which would lead to different choices in the implemnetation of a fix for this PR: a. to mirror the structure of the program as closely as possible b. to sort-of mirror the structure of the program for the benefit of operator overloading during template expansion c. to allow such constructs as *&x to hide symbolic information about x, so that stuff that isn't part of the type system proper (attributes? concepts?) can be "hidden" by such artifacts Since build_addr_func does discard/fold an INDIRECT_REF and expose the inner ADDR_EXPR and that does the inner FUNCTION_DECL, I'm jumping to the conclusion that the goal was b, and so I put in a loop to cancel out INDIRECT_REF and ADDR_EXPR when they are not template-dependent. If we find a FUNCTION_DECL, we use the info in there, even concepts. If we don't, we retain the function expression we got, except for allowing the simplification of the outermost INDIRECT_REF by build_call_addr. If the goal was a or c, I suppose we should drop the loop altogether, and stop build_addr_call from simplifying a possibly-meaningful INDIRECT_REF. If it was a, we'd probably have to distinguish more clearly between e.g. function-to-pointer decay and explicit unary &, and between dereference and array index. Anyway... Hoping it's b or something else close enough, how's this? Regstrapping on i686- and x86_64-linux-gnu. Ok to install? [PR c++/84943] cancel-out indirect_ref and addr_expr in call fn[0]() ICEs because we end up with addr_expr of a decl, and that should only happen for artificial or otherwise special internal functions. For anything else, we should find the decl earlier, but we don't because we build an indirect_ref of an addr_expr for the array indexing, and we don't want to fold them right away. When building the call, however, we would fold the INDIRECT_REF while building the address for the call, and then we'd find the decl within the remaining ADDR_EXPR, and complain it's a decl but not one of the artificial or special functions. Thus, when building the call, I've introduced code to skip any pairs of cancelling-out INDIRECT_REFs of ADDR_EXPRs, so that we stand a better chance of finding the original fndecl and constructing the call using the fndecl information. If we fail to find the decl, we go back to the original function expression; we'll still simplify the outermost INDIRECT_REF when taking its address, but then we won't find an ADDR_EXPR of a FUNCTION_DECL in there any more. for gcc/cp/ChangeLog PR c++/84943 * typeck.c (cp_build_function_call_vec): Cancel-out pairs of INDIRECT_REFs and ADDR_EXPRs to find a function decl. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. --- gcc/cp/typeck.c| 29 - gcc/testsuite/g++.dg/pr84943.C |8 2 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index d3183b5321d3..f4aa300c4ddb 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3670,7 +3670,19 @@ cp_build_function_call_vec (tree function, vec **params, && TREE_TYPE (function) == TREE_TYPE (TREE_OPERAND (function, 0))) function = TREE_OPERAND (function, 0); - if (TREE_CODE (function) == FUNCTION_DECL) + /* Short-circuit cancelling-out INDIRECT_REFs of ADDR_EXPRs. */ + fndecl = function; + while (TREE_CODE (fndecl) == INDIRECT_REF +&& TREE_TYPE (fndecl) +&& TREE_CODE (TREE_OPERAND (fndecl, 0)) == ADDR_EXPR +&& TREE_TYPE (TREE_OPERAND (fndecl, 0)) +&& (TREE_TYPE (fndecl) +== TREE_TYPE (TREE_TYPE (TREE_OPERAND (fndecl, 0 +&& (TREE_TYPE (fndecl) +== TREE_TYPE (TREE_OPERAND (TREE_OPERAND (fndecl, 0), 0 +fndecl = TREE_OPERAND (TREE_OPERAND (fndecl, 0), 0); + + if (TREE_CODE (fndecl) == FUNCTION_DECL) { /* If the function is a non-template member function or a non-template friend, then we need to check the @@ -3683,20 +3695,19 @@ cp_build_function_call_vec (tree function, vec **params, add_function_candidate. */ if (flag_concepts && (complain & tf_error) - && !constraints_satisfied_p (function)) + && !constraints_satisfied_p (fndecl)) { - error ("cannot call function %qD", function); - locati
RE: [PATCH 0/4] ASAN for MIPS (o32)
Hans-Peter Nilsson writes: > All patches are together run through the gcc and g++ test-suites > to check ASAN results for mipsisa32r2el-linux-gnu. As of > r258635 those results are on par with those for > arm-linux-gnueabihf, so without delay I present the current > state. Maybe it's non-intrusive enough to be ok for gcc-8? > MIPS maintainers (and interested party) CC:ed. From a MIPS backend perspective all 4 patches are OK. I've done very little to support upstream MIPS over this release so these improvements are fantastic. I don't know the detail of asan support so am going with the idea that your investigation has got to the bottom of the problems; thanks for the detailed explanations. I'm not sure I should really approve this for GCC-8 but rather ask a global maintainer or Jakub/Richard as release managers given I can't commit to do much to support the release and I won't want to risk burdening others with a late change. > For use with -fsanitize=address, you'll need a non-ancient glibc > or equivalent (2002-ish), one that iterates on ELF headers for > the EH info at exception time (rather, doesn't call > __register_frame_info or __register_frame_info_bases at startup, > ending up calling malloc/free) or else Asan will try to > instrument the call to free and hang on a lock for eternity (or > dies on a signal). But that's no different than for other > ports, AFAIU. > > So, ok to commit? As above, if a global maintainer is happy then yes. Matthew > > brgds, H-P
Re: [og7] vector_length extension part 2: Generalize state propagation and synchronization
On 03/02/2018 05:55 PM, Cesar Philippidis wrote: + if (cfun->machine->sync_bar) +fprintf (file, "\t\tadd.u32\t\t%%r%d, %%tidy, 1; " +"// vector synchronization barrier\n", +REGNO (cfun->machine->sync_bar)); I realize that atm we don't support large vector length when nesting a vector loop inside a worker loop, but ... if we did support that, and used a vector_length of 64, then with the "Maximum number of threads per block" of 1024 we have a possible 16 workers. And when using the maximum number of workers, we'll end up using logical barrier 16 (while we only have 0..15). It would be good to have at least an assert detecting this situation. Thanks, - Tom
Re: [PATCH] [PR c++/84968] reject stmt-exprs in noexcept constexprs
OK. On Fri, Mar 23, 2018 at 11:27 AM, Alexandre Oliva wrote: > We reject extended statement-expressions in template parameters, so we > might as well reject them in constant expressions used in noexcept > specifications. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? > > for gcc/cp/ChangeLog > > PR c++/84968 > * tree.c (strip_typedefs_expr): Reject STATEMENT_LISTs. > > for gcc/testsuite/ChangeLog > > PR c++/84968 > * g++.dg/eh/pr84968.C: New. > --- > gcc/cp/tree.c |4 > gcc/testsuite/g++.dg/eh/pr84968.C | 15 +++ > 2 files changed, 19 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/eh/pr84968.C > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index f1a90bdec0fc..070bd11a0591 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -1785,6 +1785,10 @@ strip_typedefs_expr (tree t, bool *remove_attributes) >error ("lambda-expression in a constant expression"); >return error_mark_node; > > +case STATEMENT_LIST: > + error ("statement-expression in a constant expression"); > + return error_mark_node; > + > default: >break; > } > diff --git a/gcc/testsuite/g++.dg/eh/pr84968.C > b/gcc/testsuite/g++.dg/eh/pr84968.C > new file mode 100644 > index ..23c49f477a88 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/eh/pr84968.C > @@ -0,0 +1,15 @@ > +// { dg-do compile { target c++11 } } > + > +// { dg-options "" } > + > +union b; > + > +struct S { > + template > + void a() > +try { > +} catch (int () > +noexcept (({ union b a; true; }))) // { dg-error "constant" } > + { > + } > +}; > > > -- > Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] [PR c++/84973] don't defer output of uninstantiated templates
OK. On Fri, Mar 23, 2018 at 11:28 AM, Alexandre Oliva wrote: > When an anon struct gets a name through a typedef, we reset its > linkage and that of its members. Member functions may get vague > linkage, which schedules them for deferred output, but we don't want > to add them to the queue if they're uninstantiated templates, > e.g. because the enclosing function is a template. They will be added > as needed when the enclosing template is instantiated. > > Regstrapped on i686- and x86_64-linux-gnu. Ok to install? > > for gcc/cp/ChangeLog > > PR c++/84973 > * decl2.c (note_vague_linkage_fn): Don't defer uninstantiated > templates. > > for gcc/testsuite/ChangeLog > > PR c++/84973 > * g++.dg/template/pr84973.C: New. > * g++.dg/template/pr84973-2.C: New. > * g++.dg/template/pr84973-3.C: New. > --- > gcc/cp/decl2.c|3 +++ > gcc/testsuite/g++.dg/template/pr84973-2.C | 13 + > gcc/testsuite/g++.dg/template/pr84973-3.C | 13 + > gcc/testsuite/g++.dg/template/pr84973.C |8 > 4 files changed, 37 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/template/pr84973-2.C > create mode 100644 gcc/testsuite/g++.dg/template/pr84973-3.C > create mode 100644 gcc/testsuite/g++.dg/template/pr84973.C > > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c > index e522b9ebe55a..fa753749e1a6 100644 > --- a/gcc/cp/decl2.c > +++ b/gcc/cp/decl2.c > @@ -739,6 +739,9 @@ check_classfn (tree ctype, tree function, tree > template_parms) > void > note_vague_linkage_fn (tree decl) > { > + if (processing_template_decl) > +return; > + >DECL_DEFER_OUTPUT (decl) = 1; >vec_safe_push (deferred_fns, decl); > } > diff --git a/gcc/testsuite/g++.dg/template/pr84973-2.C > b/gcc/testsuite/g++.dg/template/pr84973-2.C > new file mode 100644 > index ..41c205ad5243 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/pr84973-2.C > @@ -0,0 +1,13 @@ > +// { dg-do compile } > + > +template void a() { > + typedef struct { > +void b() try { b; } catch (short) { // { dg-error "invalid use" } > +} > + } c; > +} > + > +int > +main() { > + a<0>(); > +} > diff --git a/gcc/testsuite/g++.dg/template/pr84973-3.C > b/gcc/testsuite/g++.dg/template/pr84973-3.C > new file mode 100644 > index ..eeac214f2e1b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/pr84973-3.C > @@ -0,0 +1,13 @@ > +// { dg-do link } > + > +template void a() { > + typedef struct { > +void b() try { b(); } catch (short) { > +} > + } c; > +} > + > +int > +main() { > + a<0>(); > +} > diff --git a/gcc/testsuite/g++.dg/template/pr84973.C > b/gcc/testsuite/g++.dg/template/pr84973.C > new file mode 100644 > index ..b3f7170bc0dc > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/pr84973.C > @@ -0,0 +1,8 @@ > +// { dg-do compile } > + > +template void a() { > + typedef struct { > +void b() try { b; } catch (short) { > +} > + } c; > +} > > -- > Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [C++ PATCH] Fix error-recovery in compute_array_index_type (PR c++/85015)
On Fri, Mar 23, 2018 at 10:17 AM, Jakub Jelinek wrote: > On Fri, Mar 23, 2018 at 09:31:53AM -0400, Jason Merrill wrote: >> On Thu, Mar 22, 2018 at 5:27 PM, Jakub Jelinek wrote: >> > We ICE during error-recovery on the following testcase, >> > compute_array_index_type >> > checks size for error_operand_p early (and it is not error operand; it is >> > > where c is of reference type with >> > error_mark_node DECL_INITIAL), then calls mark_rvalue_use (which returns >> > error_mark_node), but we let it go >> > through maybe_constant_size etc. and because error_mark_node is >> > !TREE_CONSTANT, set it back to the original size. >> >> Hmm, maybe we need to also update osize with the result of >> mark_rvalue_use. Or flip the logic of the constant value block to use >> a local variable and only change size if the result is constant. > > Like this? Passes check-c++-all... > > 2018-03-23 Jakub Jelinek > > PR c++/85015 > * decl.c (compute_array_index_type): Return error_mark_node if > mark_rvalue_use or maybe_constant_value returns error_operand_p. > Set osize to mark_rvalue_use result. > > * g++.dg/cpp0x/pr85015.C: New test. > > --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 > +++ gcc/cp/decl.c 2018-03-23 14:52:07.274893996 +0100 > @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre > >if (!type_dependent_expression_p (size)) > { > - size = mark_rvalue_use (size); > + osize = size = mark_rvalue_use (size); With this, I think we shouldn't need the other changes. Jason
Re: [C++ PATCH] Fix FIX_TRUNC_EXPR instantiation (PR c++/84942)
OK. On Fri, Mar 23, 2018 at 10:18 AM, Jakub Jelinek wrote: > On Thu, Mar 22, 2018 at 02:02:01PM -0400, Jason Merrill wrote: >> He hadn't yet checked in the relevant change, "Disable >> auto_is_implicit_function_template_parm_p while parsing attributes". >> That should happen soon. >> >> > but with >> > the following patch we don't ICE, because args is NULL and >> > tsubst_copy starts with: >> > 14945 if (t == NULL_TREE || t == error_mark_node || args == NULL_TREE) >> > 14946 return t; >> > So, do you still want the FIX_TRUNC_EXPR handling removed or fixed (as done >> > in the first patch)? >> >> Hmm, let's make it gcc_unreachable then. > > So like this? Passes make check-c++-all on current trunk, where the above > patch from Alex is already in. > > 2018-03-23 Jakub Jelinek > > PR c++/84942 > * pt.c (tsubst_copy_and_build) : Replace > cp_build_unary_op call with gcc_unreachable (). > > * g++.dg/cpp1y/pr84942.C: New test. > > --- gcc/cp/pt.c.jj 2018-03-23 09:49:38.063519286 +0100 > +++ gcc/cp/pt.c 2018-03-23 09:51:24.232501064 +0100 > @@ -17514,8 +17514,7 @@ tsubst_copy_and_build (tree t, > complain|decltype_flag)); > > case FIX_TRUNC_EXPR: > - RETURN (cp_build_unary_op (FIX_TRUNC_EXPR, RECUR (TREE_OPERAND (t, 0)), > -false, complain)); > + gcc_unreachable (); > > case ADDR_EXPR: >op1 = TREE_OPERAND (t, 0); > --- gcc/testsuite/g++.dg/cpp1y/pr84942.C.jj 2018-03-23 09:50:16.320512716 > +0100 > +++ gcc/testsuite/g++.dg/cpp1y/pr84942.C2018-03-23 09:52:33.408489183 > +0100 > @@ -0,0 +1,6 @@ > +// PR c++/84942 > +// { dg-do compile { target c++14 } } > +// { dg-options "-w" } > + > +int a(__attribute__((b((int)__builtin_inf() * 1ULL / auto; > +// { dg-error "expected primary-expression before" "" { target *-*-* } .-1 } > > > Jakub
[PATCH][arm] PR target/85026: Fix ldrsh length estimate in Thumb state
Hi all, This bug has been reported against GCC 7.3.0 but it is latent in all release branches and on trunk. We underestimate the length of the LRSH instruction in Thumb state. Unlike other load instructions LDRSH can be encoded in 16 bits only when using a register offset. In the testcase we have "ldrsh r2, [r4]" being assigned a length of 2, which is wrong. So we don't calculate branch ranges properly and cause the assembler error. The fix is to make the unaligned_loadhis insn similar to the *arm_extendqihi_insn insn that outputs an LDRSB. Just remove the wrong 2-byte alternative. I don't think this is worth inventing a new "register-offset-only" constraint. This also makes the patch safer for backporting. Bootstrapped and tested on arm-none-linux-gnueabihf. Committing to trunk and the branches after some soaking time. Thanks, Kyrill 2018-03-23 Kyrylo Tkachov PR target/85026 * config/arm/arm.md (unaligned_loadhis): Remove first alternative. Clean up attributes. 2018-03-23 Kyrylo Tkachov PR target/85026 * g++.dg/pr85026.C: New test. diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index f9365cd..ad5f387 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -4498,16 +4498,13 @@ (define_insn "unaligned_loadsi" (set_attr "type" "load1")]) (define_insn "unaligned_loadhis" - [(set (match_operand:SI 0 "s_register_operand" "=l,r") + [(set (match_operand:SI 0 "s_register_operand" "=r") (sign_extend:SI - (unspec:HI [(match_operand:HI 1 "memory_operand" "Uw,Uh")] + (unspec:HI [(match_operand:HI 1 "memory_operand" "Uh")] UNSPEC_UNALIGNED_LOAD)))] "unaligned_access" "ldrsh%?\t%0, %1\t@ unaligned" - [(set_attr "arch" "t2,any") - (set_attr "length" "2,4") - (set_attr "predicable" "yes") - (set_attr "predicable_short_it" "yes,no") + [(set_attr "predicable" "yes") (set_attr "type" "load_byte")]) (define_insn "unaligned_loadhiu" diff --git a/gcc/testsuite/g++.dg/pr85026.C b/gcc/testsuite/g++.dg/pr85026.C new file mode 100644 index 000..e1e3ccd --- /dev/null +++ b/gcc/testsuite/g++.dg/pr85026.C @@ -0,0 +1,61 @@ +/* PR target/85026. */ +/* { dg-do assemble } */ +/* { dg-options "-O2 -std=gnu++11" } */ + +template class a; +class b; +struct c { + typedef a &g; +}; +template struct e { typedef typename d::f iter; }; +class h { +public: + void __attribute__((noreturn)) i(); +} ab; +template class a { +public: + typedef b *f; + b &operator[](unsigned m) { +if (ac) + ab.i(); +return ad[m]; + } + f n() { return ad; } + f m_fn3(); + b *ad; + unsigned ac; +}; +class b { +public: + short j; + short k; + signed l; +} __attribute__((__packed__)); +void o(a &m, b &p2, b &p) { + p2 = p = m[0]; + if (bool at = false) +; + else +for (c::g au(m);; at = true) + if (bool av = false) +; + else +for (e>::iter aw = au.n(), ax = au.m_fn3(); ax; + av ? (void)0 : (void)0) + if (bool ay = 0) +; + else +for (b az = *aw; !ay; ay = true) { + if (p2.j) +p2.j = az.j; + else if (p.j) +p.j = az.j; + if (p2.k) +p2.k = az.k; + else if (az.k > p.k) +p.k = az.k; + if (az.l < p2.l) +if (az.l > p.l) + p.l = az.l; +} +}
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Fri, Mar 23, 2018 at 12:17 PM, Alexandre Oliva wrote: > On Mar 23, 2018, Jason Merrill wrote: > >> On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva wrote: >>> fn[0]() ICEs because we end up with addr_expr of a decl, and that >>> should only happen for artificial or otherwise special internal >>> functions. For anything else, we should find the decl earlier, but we >>> don't because we build an indirect_ref or an addr_expr and don't >>> cancel them out. > >> That's deliberate; we recently changed the C++ front end to defer most >> folding until genericization time. > > Ok, I can see a number of possibilities as to why this is done, which > would lead to different choices in the implemnetation of a fix for this > PR: > > a. to mirror the structure of the program as closely as possible > b. to sort-of mirror the structure of the program for the benefit of > operator overloading during template expansion > > c. to allow such constructs as *&x to hide symbolic information about x, > so that stuff that isn't part of the type system proper (attributes? > concepts?) can be "hidden" by such artifacts Mostly c, as the difference is significant for some language rules. But in some cases, mainly when we're dealing with internally generated trees, we do fold. Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. Jason
Re: [PR c++/84789] do not resolve typename into template-independent
On Mar 23, 2018, Jakub Jelinek wrote: > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >> OK, thanks. > Note the testcase FAILs with -fconcepts when I do make check-c++-all, Hmm, I don't get that with check or check-g++. Should we expand the default std_list in g++-dg.exp, or should I manually test these additional variants? Anyway, thanks for the report, I'll try to figure out why this fails with concepts. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PR c++/84789] do not resolve typename into template-independent
On Fri, Mar 23, 2018 at 01:45:01PM -0300, Alexandre Oliva wrote: > On Mar 23, 2018, Jakub Jelinek wrote: > > > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: > >> OK, thanks. > > > Note the testcase FAILs with -fconcepts when I do make check-c++-all, > > Hmm, I don't get that with check or check-g++. Should we expand the > default std_list in g++-dg.exp, or should I manually test these > additional variants? I think we should add the -std=c++17 mode to the list of standards cycled by default, now that it is a released standard. -fconcepts is experimental and so is -std=c++2a, so perhaps those can stay out of the default and be just checked occassionally. The full set of check-c++-all FAILs I'm getting that don't show up with check-g++ is: FAIL: g++.dg/spellcheck-macro-ordering-2.C -std=c++17 -fconcepts (test for errors, line 10) FAIL: g++.dg/spellcheck-macro-ordering-2.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for errors, line 6) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for warnings, line 6) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for warnings, line 15) FAIL: g++.dg/spellcheck-macro-ordering.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, line 12) FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++17 (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++2a (test for excess errors) FAIL: g++.dg/warn/string1.C -std=gnu++17 -fconcepts (test for excess errors) Jakub
[PATCH. rs6000] Fix PR84912: ICE using -m32 on __builtin_divde*, patch #1
This is the first patch to fix PR84912, which is an ICE when calling some extended divide builtin functions. In discussing this offline, we decided that all div*o builtin functions make no sense because we don't model the OV bit in GCC. This patch simply removes all div*o builtins and their associated documentation. The next patch will cure the remaining ICEs. This passed bootstrap and regtesting on powerpc64-linux with no regressions. Ok for mainline? Do we want this backported to the open release branches too? Peter gcc/ PR target/84912 * config/rs6000/rs6000-builtin.def (DIVWEO): Delete macro expansion. (DIVWEUO): Likewise. (DIVDEO): Likewise. (DIVDEUO): Likewise. * config/rs6000/rs6000.c (builtin_function_type): Remove support for DIVWEUO and DIVDEUO. * config/rs6000/rs6000.md (UNSPEC_DIVEO, UNSPEC_DIVEUO): Delete unspecs. (UNSPEC_DIV_EXTEND): Remove deleted unspecs. (div_extend): Likewise. * doc/extend.texi (__builtin_divweo): Remove documention for deleted builtin function. (__builtin_divweuo): Likewise. (__builtin_divdeo): Likewise. (__builtin_divdeuo): Likewise. gcc/testsuite/ PR target/84912 * gcc.target/powerpc/extend-divide-1.c (div_weo): Remove test for deleted builtin function. (div_weuo): Likewise. * gcc.target/powerpc/extend-divide-2.c (div_deo): Likewise. (div_deuo): Likewise. Index: gcc/config/rs6000/rs6000-builtin.def === --- gcc/config/rs6000/rs6000-builtin.def(revision 258802) +++ gcc/config/rs6000/rs6000-builtin.def(working copy) @@ -2310,13 +2310,9 @@ BU_P9V_OVERLOAD_1 (VCTZLSBB, "vctzlsbb") /* 2 argument extended divide functions added in ISA 2.06. */ BU_P7_MISC_2 (DIVWE, "divwe",CONST, dive_si) -BU_P7_MISC_2 (DIVWEO, "divweo", CONST, diveo_si) BU_P7_MISC_2 (DIVWEU, "divweu", CONST, diveu_si) -BU_P7_MISC_2 (DIVWEUO, "divweuo", CONST, diveuo_si) BU_P7_MISC_2 (DIVDE, "divde",CONST, dive_di) -BU_P7_MISC_2 (DIVDEO, "divdeo", CONST, diveo_di) BU_P7_MISC_2 (DIVDEU, "divdeu", CONST, diveu_di) -BU_P7_MISC_2 (DIVDEUO, "divdeuo", CONST, diveuo_di) /* 1 argument DFP (decimal floating point) functions added in ISA 2.05. */ BU_DFP_MISC_1 (DXEX, "dxex", CONST, dfp_dxex_dd) Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 258802) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -18053,9 +18053,7 @@ builtin_function_type (machine_mode mode case CRYPTO_BUILTIN_VPMSUM: case MISC_BUILTIN_ADDG6S: case MISC_BUILTIN_DIVWEU: -case MISC_BUILTIN_DIVWEUO: case MISC_BUILTIN_DIVDEU: -case MISC_BUILTIN_DIVDEUO: case VSX_BUILTIN_UDIV_V2DI: case ALTIVEC_BUILTIN_VMAXUB: case ALTIVEC_BUILTIN_VMINUB: Index: gcc/config/rs6000/rs6000.md === --- gcc/config/rs6000/rs6000.md (revision 258802) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -130,9 +130,7 @@ (define_c_enum "unspec" UNSPEC_CDTBCD UNSPEC_CBCDTD UNSPEC_DIVE - UNSPEC_DIVEO UNSPEC_DIVEU - UNSPEC_DIVEUO UNSPEC_UNPACK_128BIT UNSPEC_PACK_128BIT UNSPEC_LSQ @@ -13863,14 +13861,10 @@ (define_insn "cbcdtd" (set_attr "length" "4")]) (define_int_iterator UNSPEC_DIV_EXTEND [UNSPEC_DIVE - UNSPEC_DIVEO - UNSPEC_DIVEU - UNSPEC_DIVEUO]) + UNSPEC_DIVEU]) (define_int_attr div_extend [(UNSPEC_DIVE "e") -(UNSPEC_DIVEO "eo") -(UNSPEC_DIVEU "eu") -(UNSPEC_DIVEUO "euo")]) +(UNSPEC_DIVEU "eu")]) (define_insn "div_" [(set (match_operand:GPR 0 "register_operand" "=r") Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 258802) +++ gcc/doc/extend.texi (working copy) @@ -15820,22 +15820,17 @@ or @option{-mpopcntd}): @smallexample long __builtin_bpermd (long, long); int __builtin_divwe (int, int); -int __builtin_divweo (int, int); unsigned int __builtin_divweu (unsigned int, unsigned int); -unsigned int __builtin_divweuo (unsigned int, unsigned int); long __builtin_divde (long, long); -long __builtin_divdeo (long, long); unsigned long __builtin_divdeu (unsigned long, unsigned long); -unsigned long __builtin_divdeuo (unsigned long, unsigned long); unsigned int cdtbcd (unsigned int); unsigned int cbcdtd (unsigned int); unsigned int addg6s (unsigned int, unsign
[PATCH. rs6000] Fix PR84912: ICE using -m32 on __builtin_divde*, patch #2
This is the second patch to fix PR84912, which is an ICE when calling some extended divide builtin functions. This patch is relative to the first patch. This fixes the ICE by adding a new mask to the builtin functions that are ICEing and then enforcing it is set. I have also added a helpful error message in the case it is not set. This passed bootstrap and regtesting on powerpc64-linux with no regressions. Ok for mainline? Do we also want this backported to the open release branches too? Peter gcc/ PR target/84912 * config/rs6000/rs6000.h (RS6000_BTM_POWERPC64): New define. (RS6000_BTM_COMMON): Add RS6000_BTM_POWERPC64. * config/rs6000/rs6000.c (rs6000_builtin_mask_calculate): Add support for RS6000_BTM_POWERPC64. (rs6000_invalid_builtin): Add handling for RS6000_BTM_POWERPC64 (rs6000_builtin_mask_names): Add RS6000_BTM_POWERPC64. * config/rs6000/rs6000-builtin.def (BU_P7_POWERPC64_MISC_2): New macro definition. (DIVDE): Use it. (DIVDEU): Likewise. diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000.h gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000.h --- gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000.h 2018-03-19 19:59:55.911285043 -0500 +++ gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000.h 2018-03-22 19:57:21.051923201 -0500 @@ -2506,6 +2506,7 @@ extern int frame_pointer_needed; #define RS6000_BTM_HARD_FLOAT MASK_SOFT_FLOAT /* Hardware floating point. */ #define RS6000_BTM_LDBL128 MASK_MULTIPLE /* 128-bit long double. */ #define RS6000_BTM_64BIT MASK_64BIT /* 64-bit addressing. */ +#define RS6000_BTM_POWERPC64 MASK_POWERPC64 /* 64-bit registers. */ #define RS6000_BTM_FLOAT128MASK_FLOAT128_KEYWORD /* IEEE 128-bit float. */ #define RS6000_BTM_FLOAT128_HW MASK_FLOAT128_HW /* IEEE 128-bit float h/w. */ @@ -2526,6 +2527,7 @@ extern int frame_pointer_needed; | RS6000_BTM_DFP \ | RS6000_BTM_HARD_FLOAT\ | RS6000_BTM_LDBL128 \ +| RS6000_BTM_POWERPC64 \ | RS6000_BTM_FLOAT128 \ | RS6000_BTM_FLOAT128_HW) diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000.c gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000.c --- gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000.c 2018-03-23 08:03:55.257469347 -0500 +++ gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000.c 2018-03-23 08:03:42.707253487 -0500 @@ -3916,6 +3916,7 @@ rs6000_builtin_mask_calculate (void) | ((TARGET_P9_MISC) ? RS6000_BTM_P9_MISC : 0) | ((TARGET_MODULO)? RS6000_BTM_MODULO: 0) | ((TARGET_64BIT) ? RS6000_BTM_64BIT : 0) + | ((TARGET_POWERPC64) ? RS6000_BTM_POWERPC64 : 0) | ((TARGET_CRYPTO)? RS6000_BTM_CRYPTO: 0) | ((TARGET_HTM) ? RS6000_BTM_HTM : 0) | ((TARGET_DFP) ? RS6000_BTM_DFP : 0) @@ -15952,6 +15953,10 @@ rs6000_invalid_builtin (enum rs6000_buil name); else if ((fnmask & RS6000_BTM_FLOAT128) != 0) error ("builtin function %qs requires the %qs option", name, "-mfloat128"); + else if ((fnmask & (RS6000_BTM_POPCNTD | RS6000_BTM_POWERPC64)) + == (RS6000_BTM_POPCNTD | RS6000_BTM_POWERPC64)) +error ("builtin function %qs requires the %qs and %qs options", + name, "-mcpu=power7 (or newer)", "-m64 or -mpowerpc64"); else error ("builtin function %qs is not supported with the current options", name); @@ -36612,6 +36617,7 @@ static struct rs6000_opt_mask const rs60 { "hard-dfp", RS6000_BTM_DFP,false, false }, { "hard-float", RS6000_BTM_HARD_FLOAT, false, false }, { "long-double-128", RS6000_BTM_LDBL128,false, false }, + { "powerpc64",RS6000_BTM_POWERPC64, false, false }, { "float128", RS6000_BTM_FLOAT128, false, false }, { "float128-hw", RS6000_BTM_FLOAT128_HW,false, false }, }; diff -urpN -X /home/bergner/cvs/dontdiff gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000-builtin.def gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000-builtin.def --- gcc-fsf-mainline-pr84912-2/gcc/config/rs6000/rs6000-builtin.def 2018-03-23 08:03:55.257469347 -0500 +++ gcc-fsf-mainline-pr84912/gcc/config/rs6000/rs6000-builtin.def 2018-03-23 08:03:42.707253487 -0500 @@ -646,6 +646,15 @@ | RS6000_BTC_BINARY), \ CODE_FOR_ ## ICODE) /* ICODE */ +#define BU_P7_POWERPC64_MISC_2(ENUM, NAME, ATTR, ICODE) \
Re: [C++ PATCH] Fix error-recovery in compute_array_index_type (PR c++/85015)
OK. On Fri, Mar 23, 2018 at 1:26 PM, Jakub Jelinek wrote: > On Fri, Mar 23, 2018 at 12:32:20PM -0400, Jason Merrill wrote: >> > 2018-03-23 Jakub Jelinek >> > >> > PR c++/85015 >> > * decl.c (compute_array_index_type): Return error_mark_node if >> > mark_rvalue_use or maybe_constant_value returns error_operand_p. >> > Set osize to mark_rvalue_use result. >> > >> > * g++.dg/cpp0x/pr85015.C: New test. >> > >> > --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 >> > +++ gcc/cp/decl.c 2018-03-23 14:52:07.274893996 +0100 >> > @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre >> > >> >if (!type_dependent_expression_p (size)) >> > { >> > - size = mark_rvalue_use (size); >> > + osize = size = mark_rvalue_use (size); >> >> With this, I think we shouldn't need the other changes. > > Yes, at least check-c++-all doesn't regress with this, will do now full > bootstrap/regtest. Ok if it passes? > > 2018-03-23 Jakub Jelinek > > PR c++/85015 > * decl.c (compute_array_index_type): Set osize to mark_rvalue_use > result. > > * g++.dg/cpp0x/pr85015.C: New test. > > --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 > +++ gcc/cp/decl.c 2018-03-23 17:33:52.412071232 +0100 > @@ -9520,7 +9520,7 @@ compute_array_index_type (tree name, tre > >if (!type_dependent_expression_p (size)) > { > - size = mark_rvalue_use (size); > + osize = size = mark_rvalue_use (size); > >if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR > && TREE_SIDE_EFFECTS (size)) > --- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj 2018-03-23 14:51:19.207892516 > +0100 > +++ gcc/testsuite/g++.dg/cpp0x/pr85015.C2018-03-23 14:51:19.207892516 > +0100 > @@ -0,0 +1,12 @@ > +// PR c++/85015 > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +void > +foo () > +{ > + int &&c = v + 1; // { dg-error "was not declared in this > scope" } > + struct S { // { dg-message "declared here" "" { target > *-*-* } .-1 } > +void bar () { int a[c]; } // { dg-error "use of local variable with > automatic storage from containing function" } > + } e; > +} > > > Jakub
Re: [PATCH, rs6000] Fix PR83789: __builtin_altivec_lvx fails for powerpc for altivec-4.c
On 3/22/18 6:03 PM, Segher Boessenkool wrote: > On Wed, Mar 21, 2018 at 09:10:38PM -0500, Peter Bergner wrote: >> If you're asking about whether we also need to backport to GCC 6, I >> believe Kaushik said in the bugzilla that he only encountered the >> ICE on GCC 7 and trunk, so I don't think we need a GCC 6 backport. > > Uh yes, that is what I meant. Okay, since we have no way to check if > it fixes anything, let's not backport it to 6. Ok, backport committed to the GCC 7 release branch. Thanks. Peter
RFC: Disable asan tests under ulimit -v
asan doesn't work under ulimit -v, which I want to use on shared hosts to avoid causing trouble with runaway processes. There doesn't seem to be a way within expect to access getrlimit/setrlimit, so in this patch I call out to the shell to test the current limit, and give up if I get back a number (rather than "unlimited" or an error message). Thoughts? commit e11e83293d6be03105eae16e92b00de591f4850e Author: Jason Merrill Date: Fri Mar 23 11:14:50 2018 -0400 * gcc.dg/asan/asan.exp: Don't run tests if ulimit -v is set. * g++.dg/asan/asan.exp: Likewise. diff --git a/gcc/testsuite/g++.dg/asan/asan.exp b/gcc/testsuite/g++.dg/asan/asan.exp index 4ee8dd98697..a22d2ac5e20 100644 --- a/gcc/testsuite/g++.dg/asan/asan.exp +++ b/gcc/testsuite/g++.dg/asan/asan.exp @@ -24,6 +24,13 @@ load_lib asan-dg.exp dg-init asan_init +# asan doesn't work if there's a ulimit on virtual memory. +if ![is_remote target] { +if [regexp {^[0-9]+$} "[exec ulimit -v]"] { + return +} +} + # Main loop. if [check_effective_target_fsanitize_address] { gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C $srcdir/c-c++-common/asan/*.c]] "" "" diff --git a/gcc/testsuite/gcc.dg/asan/asan.exp b/gcc/testsuite/gcc.dg/asan/asan.exp index 7b669056a97..bfe36e524dd 100644 --- a/gcc/testsuite/gcc.dg/asan/asan.exp +++ b/gcc/testsuite/gcc.dg/asan/asan.exp @@ -26,6 +26,13 @@ load_lib asan-dg.exp dg-init asan_init +# asan doesn't work if there's a ulimit on virtual memory. +if ![is_remote target] { +if [regexp {^[0-9]+$} "[exec ulimit -v]"] { + return +} +} + # Main loop. if [check_effective_target_fsanitize_address] { gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.c $srcdir/c-c++-common/asan/*.c]] "" ""
Re: [PR c++/84789] do not resolve typename into template-independent
On Mar 23, 2018, Jakub Jelinek wrote: > On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >> OK, thanks. > Note the testcase FAILs with -fconcepts when I do make check-c++-all, > FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, > line 12) > FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess > errors) > Excess errors: > /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: > 'A::I' {aka 'int'} is not a class type > /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' > in 'A::I' {aka 'int'} does not name a type Is this ok to install? [PR c++/84789] adjust testcase for -fconcepts When compiling with -fconcepts, cp_parser_template_declaration_after_export calls cp_parser_template_introduction and that preparses qualified-ids not preceded by typename in such a way that, when we get to cp_parser_parse_and_diagnose_invalid_type_name and then cp_parser_diagnose_invalid_type_name, the nested name specifier no longer carries the previous template-dependent context, so we don't stand a chance to suggest the use of 'typename' any more. Thus, tolerate in the testcase the poorer error messages we get. for gcc/testsuite/ChangeLog PR c++/84789 * g++.dg/template/pr84789.C: Adjust for testing with -fconcepts too. --- gcc/testsuite/g++.dg/template/pr84789.C |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/g++.dg/template/pr84789.C b/gcc/testsuite/g++.dg/template/pr84789.C index bc1567f3fe77..63b9832fecf8 100644 --- a/gcc/testsuite/g++.dg/template/pr84789.C +++ b/gcc/testsuite/g++.dg/template/pr84789.C @@ -9,5 +9,5 @@ template struct B : A {}; template struct C : B { - B::A::I::I i; // { dg-error "typename" } + B::A::I::I i; // { dg-error "not a class type|does not name a type|typename" } }; -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH], PR target/84914, Fix complex long double multiply/divide on PowerPC -mabi=ieeelongdouble
On Thu, Mar 22, 2018 at 05:52:56PM -0500, Segher Boessenkool wrote: > On Wed, Mar 21, 2018 at 11:42:01AM -0400, Michael Meissner wrote: > > +/* Create a decl for either complex long double multiply or complex long > > double > > + divide when long double is IEEE 128-bit floating point. We can't use > > + __multc3 and __divtc3 because the original long double using IBM > > extended > > + double used those names. The complex multiply/divide functions are > > encoded > > + as builtin functions with a complex result and 4 scalar inputs. */ > > The function is much more generic than that (other than the name and the > debug printf). Not really. All of the other built-ins go through more standard paths of specifying insns with pre-defined names. There is no way to create the complex multiply and divide builtins without doing the low level build of the function type and assigning the function into the builtin table. You can use the function to create other builtins, but it would be better to do those in the typical fashion. From the comments, the problem is the library support does not support returning complex types, so they hacked in the complex multiply/divide as a builtin. Even so, the function should have two complex arguments, instead of 4 scalars. > > +static void > > +create_complex_muldiv (const char *name, built_in_function fncode, tree > > fntype) > > +{ > > + tree fndecl = add_builtin_function (name, fntype, fncode, > > BUILT_IN_NORMAL, > > + name, NULL_TREE); > > + > > + set_builtin_decl (fncode, fndecl, true); > > + > > + if (TARGET_DEBUG_BUILTIN) > > +fprintf (stderr, "create complex %s, fncode: %d, fndecl: 0x%lx\n", > > name, > > +(int) fncode, (unsigned long)fndecl); > > Space after cast. Don't cast pointers to integer, just use %p. Is this > address useful to print at all? When I was doing the debugging, it was useful (making sure the right function got called). I eliminated the debug_tree I was also doing, I'll eliminate the > > @@ -18689,6 +18710,24 @@ init_float128_ieee (machine_mode mode) > > { > >if (FLOAT128_VECTOR_P (mode)) > > { > > + /* Set up to call __mulkc3 and __divkc3 under -mabi=ieeelongdouble. > > */ > > + if (mode == TFmode && TARGET_IEEEQUAD) > > + { > > +built_in_function fncode_mul = > > + (built_in_function)(BUILT_IN_COMPLEX_MUL_MIN + TCmode - > > MIN_MODE_COMPLEX_FLOAT); > > +built_in_function fncode_div = > > + (built_in_function)(BUILT_IN_COMPLEX_DIV_MIN + TCmode - > > MIN_MODE_COMPLEX_FLOAT); > > Space after cast, lines too long. Put TCmode - MIN_MODE_COMPLEX_FLOAT in > a temp, maybe? Yeah, I reorganized things before the submission, and I didn't notice the line being too long. > > --- gcc/testsuite/gcc.target/powerpc/mulkc3-3.c (revision 0) > > +++ gcc/testsuite/gcc.target/powerpc/mulkc3-3.c (revision 0) > > @@ -0,0 +1,16 @@ > > +/* { dg-do compile { target { powerpc64*-*-* } } } */ > > Does powerpc*-*-* not work? > > > +/* { dg-require-effective-target powerpc_p9vector_ok } */ > > +/* { dg-options "-O2 -mpower8-vector -mabi=ibmlongdouble -Wno-psabi" } */ > > powerpc_p9vector_ok but -mpower8-vector, that is strange. Both of these are cut+paste errors, where I took another test as the basis of this test (that other test was testing the float128 h/w), and wasn't careful about changing all of the conditions. Assuming this patch passes the bootstrap and regression testing, is it ok to check into trunk? [gcc] 2018-03-23 Michael Meissner PR target/84914 * config/rs6000/rs6000.c (create_complex_muldiv): New helper function to create the function decl for complex long double multiply and divide for -mabi=ieeelongdouble. (init_float128_ieee): Call it. [gcc/testsuite] 2018-03-23 Michael Meissner PR target/84914 * gcc.target/powerpc/mulkc-2.c: New tests to make sure complex long double multiply/divide uses the correct function. * gcc.target/powerpc/mulkc-3.c: Likewise. * gcc.target/powerpc/divkc-2.c: Likewise. * gcc.target/powerpc/divkc-3.c: Likewise. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797 Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 258601) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -18678,6 +18678,26 @@ init_float128_ibm (machine_mode mode) } } +/* Create a decl for either complex long double multiply or complex long double + divide when long double is IEEE 128-bit floating point. We can't use + __multc3 and __divtc3 because the original long double using IBM extended + double used those names. The complex multiply/divide functions are encoded + as builtin functions with a complex result and 4 scalar inputs. *
patch to fix PR85030
The following patch solves https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85030 The patch was successfully bootstrapped on x86 and x86-64 and tested on x86-64. Committed as rev. 258820. Index: ChangeLog === --- ChangeLog (revision 258819) +++ ChangeLog (working copy) @@ -1,3 +1,9 @@ +2018-03-23 Vladimir Makarov + + PR inline-asm/85030 + * lra-constraints.c (process_alt_operands): Don't match BLKmode + and non BLKmode operands. + 2018-03-23 Kyrylo Tkachov PR target/85026 Index: testsuite/ChangeLog === --- testsuite/ChangeLog (revision 258819) +++ testsuite/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2018-03-23 Vladimir Makarov + + PR inline-asm/85030 + * testsuite/gcc.target/i386/pr85030.c: New. + 2018-03-23 Kyrylo Tkachov PR target/85026 Index: lra-constraints.c === --- lra-constraints.c (revision 258691) +++ lra-constraints.c (working copy) @@ -2118,6 +2118,14 @@ process_alt_operands (int only_alternati GET_MODE_SIZE (biggest_mode[nop]))) break; + /* Don't match wrong asm insn operands for proper + diagnostic later. */ + if (INSN_CODE (curr_insn) < 0 + && (curr_operand_mode[m] == BLKmode + || curr_operand_mode[nop] == BLKmode) + && curr_operand_mode[m] != curr_operand_mode[nop]) + break; + this_alternative_matches = m; m_hregno = get_hard_regno (*curr_id->operand_loc[m], false); /* We are supposed to match a previous operand. Index: testsuite/gcc.target/i386/pr85030.c === --- testsuite/gcc.target/i386/pr85030.c (nonexistent) +++ testsuite/gcc.target/i386/pr85030.c (working copy) @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O1" } */ +struct S { int c, *b, *e; }; + +void +foo () +{ + struct S a; + asm volatile ("" : "=rm" (a) : "0" (1)); /* { dg-error "inconsistent operand constraints in an 'asm'" } */ +}
Re: [C++ PATCH] Fix error-recovery in compute_array_index_type (PR c++/85015)
On Fri, Mar 23, 2018 at 12:32:20PM -0400, Jason Merrill wrote: > > 2018-03-23 Jakub Jelinek > > > > PR c++/85015 > > * decl.c (compute_array_index_type): Return error_mark_node if > > mark_rvalue_use or maybe_constant_value returns error_operand_p. > > Set osize to mark_rvalue_use result. > > > > * g++.dg/cpp0x/pr85015.C: New test. > > > > --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 > > +++ gcc/cp/decl.c 2018-03-23 14:52:07.274893996 +0100 > > @@ -9520,7 +9520,10 @@ compute_array_index_type (tree name, tre > > > >if (!type_dependent_expression_p (size)) > > { > > - size = mark_rvalue_use (size); > > + osize = size = mark_rvalue_use (size); > > With this, I think we shouldn't need the other changes. Yes, at least check-c++-all doesn't regress with this, will do now full bootstrap/regtest. Ok if it passes? 2018-03-23 Jakub Jelinek PR c++/85015 * decl.c (compute_array_index_type): Set osize to mark_rvalue_use result. * g++.dg/cpp0x/pr85015.C: New test. --- gcc/cp/decl.c.jj2018-03-22 22:23:10.120015621 +0100 +++ gcc/cp/decl.c 2018-03-23 17:33:52.412071232 +0100 @@ -9520,7 +9520,7 @@ compute_array_index_type (tree name, tre if (!type_dependent_expression_p (size)) { - size = mark_rvalue_use (size); + osize = size = mark_rvalue_use (size); if (cxx_dialect < cxx11 && TREE_CODE (size) == NOP_EXPR && TREE_SIDE_EFFECTS (size)) --- gcc/testsuite/g++.dg/cpp0x/pr85015.C.jj 2018-03-23 14:51:19.207892516 +0100 +++ gcc/testsuite/g++.dg/cpp0x/pr85015.C2018-03-23 14:51:19.207892516 +0100 @@ -0,0 +1,12 @@ +// PR c++/85015 +// { dg-do compile { target c++11 } } +// { dg-options "" } + +void +foo () +{ + int &&c = v + 1; // { dg-error "was not declared in this scope" } + struct S { // { dg-message "declared here" "" { target *-*-* } .-1 } +void bar () { int a[c]; } // { dg-error "use of local variable with automatic storage from containing function" } + } e; +} Jakub
Re: [PR c++/84789] do not resolve typename into template-independent
OK. On Fri, Mar 23, 2018 at 3:17 PM, Alexandre Oliva wrote: > On Mar 23, 2018, Jakub Jelinek wrote: > >> On Wed, Mar 21, 2018 at 11:52:33PM -0400, Jason Merrill wrote: >>> OK, thanks. > >> Note the testcase FAILs with -fconcepts when I do make check-c++-all, >> FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for errors, >> line 12) >> FAIL: g++.dg/template/pr84789.C -std=c++17 -fconcepts (test for excess >> errors) >> Excess errors: >> /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: >> 'A::I' {aka 'int'} is not a class type >> /usr/src/gcc/gcc/testsuite/g++.dg/template/pr84789.C:12:15: error: 'I' >> in 'A::I' {aka 'int'} does not name a type > > Is this ok to install? > > [PR c++/84789] adjust testcase for -fconcepts > > When compiling with -fconcepts, > cp_parser_template_declaration_after_export calls > cp_parser_template_introduction and that preparses qualified-ids not > preceded by typename in such a way that, when we get to > cp_parser_parse_and_diagnose_invalid_type_name and then > cp_parser_diagnose_invalid_type_name, the nested name specifier no > longer carries the previous template-dependent context, so we don't > stand a chance to suggest the use of 'typename' any more. Thus, > tolerate in the testcase the poorer error messages we get. > > for gcc/testsuite/ChangeLog > > PR c++/84789 > * g++.dg/template/pr84789.C: Adjust for testing with > -fconcepts too. > --- > gcc/testsuite/g++.dg/template/pr84789.C |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/g++.dg/template/pr84789.C > b/gcc/testsuite/g++.dg/template/pr84789.C > index bc1567f3fe77..63b9832fecf8 100644 > --- a/gcc/testsuite/g++.dg/template/pr84789.C > +++ b/gcc/testsuite/g++.dg/template/pr84789.C > @@ -9,5 +9,5 @@ template struct B : A {}; > > template struct C : B > { > - B::A::I::I i; // { dg-error "typename" } > + B::A::I::I i; // { dg-error "not a class type|does not name a > type|typename" } > }; > > > -- > Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
[PATCH] [PR c++/84979] improve auto handling in explicit tmpl args for concepts
We fail to detect unresolved explicitly-passed auto template args. The first hunk in pt.c, that changes fn_type_unification, arranges for us to regard the template arg list as incomplete, although that's not necessary for any of the testcases I tried. I thought it might be relevant in case the args become part of the type of the function, and for us to continue overload resolution with processing_template_decl nonzero. The second hunk, that changes type_unification_real, is the actual fix for the testcase in the PR. The other hunks, that change unify, arrange for us to accept and properly update the template arglist, at least when auto stands by itself as a template argument. Without this hunk, the slightly modified testcase pr84979-2.C would fail, although there's no good reason for the deduction to be regarded as a mismatch. We still need some work, however: pr84979-3.C and pr84979-4.C should be accepted, if we are to support non-toplevel auto in explicit parameters (-3), or auto in parms whose index doesn't match the auto's own index. This is probably not good enough for inclusion, although I'm pretty sure it passes regstrap. The problem is the condition in unify() that intends to allow proper uses of auto by do_auto_deduction, and the AFAICT accidentally-working cases of auto deduction in explicit template args: the condition that covers those cases is growing incredibly hairier and fragile, and I'm growing convinced I took a wrong turn somewhere that led me there. I'm almost ready to conclude that I'm either missing something, or that we shouldn't be supporting auto in explicit template args at all, because I can't see that the implementation was ever meant to support that use. Is that so? I could use some advice there. Thanks in advance, for gcc/cp/ChangeLog PR c++/84979 * pt.c (fn_type_unification): Set incomplete if explicit args use auto. (type_unification_real): Do not skip deduction if explicit arg uses auto. (unify): Reject unsupported cases of auto in args. Regard auto as a match, but proceed to update targ. for gcc/testsuite/ChangeLog PR c++/84979 * g++.dg/concepts/pr84979.C: New. * g++.dg/concepts/pr84979-2.C: New. * g++.dg/concepts/pr84979-3.C: New. * g++.dg/concepts/pr84979-4.C: New. --- gcc/cp/pt.c | 56 +++-- gcc/testsuite/g++.dg/concepts/pr84979-2.C |9 + gcc/testsuite/g++.dg/concepts/pr84979-3.C |9 + gcc/testsuite/g++.dg/concepts/pr84979-4.C |9 + gcc/testsuite/g++.dg/concepts/pr84979.C |9 + 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-2.C create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-3.C create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979-4.C create mode 100644 gcc/testsuite/g++.dg/concepts/pr84979.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 5293c2b5491b..2b9e5f1ada45 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -19109,7 +19109,10 @@ fn_type_unification (tree fn, parameter_pack = TEMPLATE_PARM_PARAMETER_PACK (parm); } - if (!parameter_pack && targ == NULL_TREE) + if (!parameter_pack + && (targ == NULL_TREE + || (flag_concepts + && type_uses_auto (targ /* No explicit argument for this template parameter. */ incomplete = true; @@ -19891,7 +19894,12 @@ type_unification_real (tree tparms, ARGUMENT_PACK_EXPLICIT_ARGS (targ) = NULL_TREE; } - if (targ || tparm == error_mark_node) + if ((targ + && (!flag_concepts + /* We have to unify explicitly-passed template args + involving auto types. */ + || !type_uses_auto (targ))) + || tparm == error_mark_node) continue; tparm = TREE_VALUE (tparm); @@ -20952,6 +20960,37 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict, if (parm == any_targ_node || arg == any_targ_node) return unify_success (explain_p); + /* Concepts allows 'auto' in template arguments, even multiple + 'auto's in a single argument. We don't know how to deal with + them here: see how do_auto_deduction uses extract_autos to + construct a separate template args vec, that such autos index. + Since we don't do any of that, we can only substitute toplevel + auto parameters, and only when their index happens to match their + position in the template argument list. Catch deviations instead + of corrupting targs. */ + if (flag_concepts && is_auto (parm) + && ((TEMPLATE_TYPE_IDX (parm) + >= TREE_VEC_LENGTH (INNERMOST_TEMPLATE_ARGS (targs))) + || (TREE_CODE (TREE_VALUE (TREE_VEC_ELT +(tparms, TEMPLATE_TYPE_IDX (parm))
Re: [PATCH] [PR c++/84979] improve auto handling in explicit tmpl args for concepts
On Fri, Mar 23, 2018 at 3:38 PM, Alexandre Oliva wrote: > + /* Concepts allows 'auto' in template arguments, even multiple > + 'auto's in a single argument. I think that's only intended for class templates; we should reject 'auto' as a template argument for function or variable templates. Jason
Re: [PATCH, wwwdocs] Update cxx-status.html
On Fri, Mar 23, 2018 at 3:42 PM, Jakub Jelinek wrote: > The following patch adds C++2a stuff to cxx-status.html (list of papers from > clang table, filled in what has notes in cp/ChangeLog-2017 + __VA_OPT__). > For C++17 features I've just added a couple of / markings and > added the second deduction guides paper (P0512R0), plus mentioned that the > standard has been published already. > > Ok to commit? OK, thanks. > C++17 support is still marked experimental, do we want to keep it that way > for GCC 8, or change that? Keep it that way. > Are there any known feature macros for the C++2a features, or is that still > WIP? None yet in https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations Jason
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Mar 23, 2018, Jason Merrill wrote: > On Thu, Mar 22, 2018 at 7:00 PM, Alexandre Oliva wrote: >> fn[0]() ICEs because we end up with addr_expr of a decl, and that >> should only happen for artificial or otherwise special internal >> functions. For anything else, we should find the decl earlier, but we >> don't because we build an indirect_ref or an addr_expr and don't >> cancel them out. > That's deliberate; we recently changed the C++ front end to defer most > folding until genericization time. How about this? (not significantly tested yet) [PR c++/84943] keep fndecl hidden from call fn[0]() ICEd because we we would fold the INDIRECT_REF used for the array indexing while building the address for the call, after not finding the decl hiding there at first. But the decl would be exposed by the folding, and then lower layers would complain we had the decl, after all, but it wasn't one of the artificial or special functions that could be called that way. In order to preserve the program structure and properties that depend on it, we shouldn't cancel out the INDIRECT_REF with the ADDR_EXPR, nor should we bypass them to find the decl. So, make build_addr_func not drop an INDIRECT_REF: when it would do so, wrap its original INDIRECT_REF value in an ADDR_EXPR, then we won't find the decl hiding in there unless the decl was visible to begin with. for gcc/cp/ChangeLog PR c++/84943 * call.c (build_addr_func): If we'd drop an INDIRECT_REF, wrap it in a new ADDR_EXPR instead. for gcc/testsuite/ChangeLog PR c++/84943 * g++.dg/pr84943.C: New. --- gcc/cp/call.c | 10 +- gcc/testsuite/g++.dg/pr84943.C |8 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/pr84943.C diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 9351918b23af..5a0d09b1db4e 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -283,7 +283,15 @@ build_addr_func (tree function, tsubst_flags_t complain) function = build_address (function); } else -function = decay_conversion (function, complain, /*reject_builtin=*/false); +{ + tree orig = function; + function = decay_conversion (function, complain, /*reject_builtin=*/false); + + /* Do not cancel out an INDIRECT_REF. */ + if (TREE_CODE (orig) == INDIRECT_REF + && TREE_OPERAND (orig, 0) == function) + function = build1 (ADDR_EXPR, TREE_TYPE (function), orig); +} return function; } diff --git a/gcc/testsuite/g++.dg/pr84943.C b/gcc/testsuite/g++.dg/pr84943.C new file mode 100644 index ..36f75a164119 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr84943.C @@ -0,0 +1,8 @@ +// { dg-do compile } + +// Avoid -pedantic-error default +// { dg-options "" } + +void a() { + a[0](); // { dg-warning "arithmetic" } +} -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill wrote: > Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. Did you try this? That should avoid it being ADDR_EXPR of a decl. Jason
Re: [PATCH] [PR c++/84943] allow folding of array indexing indirect_ref
On Fri, Mar 23, 2018 at 4:55 PM, Jason Merrill wrote: > On Fri, Mar 23, 2018 at 12:44 PM, Jason Merrill wrote: >> Seems like cp_fold should update CALL_EXPR_FN with "callee" if non-null. > > Did you try this? That should avoid it being ADDR_EXPR of a decl. Oh, I was assuming the ICE was in the middle-end, but it's in build_call_a. And it looks like the problem isn't that it's an ADDR_EXPR of a decl, but that the function isn't marked TREE_USED. Jason
Re: C++ PATCH for c++/84489, dependent default template argument
On Tue, Feb 27, 2018 at 12:26 PM, Jason Merrill wrote: > The logic in type_unification_real for handling template parms that > depend on earlier template parms is a bit complicated. It already > recognizes when the type of the parm depends on something not > available yet, and it dealt with the case where substituting partial > args left some template parm uses behind, but it didn't handle the > case where substituting partial args just failed. And that was still wrong, leading to the regression on the later testcase in 78489. When substitution fails, that still needs to cause deduction to fail immediately. The important part of the 84489 patch was setting processing_template_decl so unresolved template parms don't cause substitution failure. And to handle the first testcase in 78489, we also need to substitute into the type of non-type parameters even if we aren't going to use their default arguments yet. Tested x86_64-pc-linux-gnu, applying to trunk. commit 02864b90c69f3af56054a627e834dd66a0aa0c80 Author: Jason Merrill Date: Fri Mar 23 11:15:05 2018 -0400 PR c++/78489 - wrong SFINAE behavior. PR c++/84489 * pt.c (type_unification_real): Don't defer substitution failure. diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 5293c2b5491..8af29cef8d3 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -19996,41 +19996,49 @@ type_unification_real (tree tparms, if (targ || tparm == error_mark_node) continue; tree parm = TREE_VALUE (tparm); - - tsubst_flags_t fcomplain = complain; - if (saw_undeduced == 1) - { - /* When saw_undeduced == 1, substitution into parm and arg might - fail or not replace all template parameters, and that's - fine. */ - fcomplain = tf_none; - if (TREE_CODE (parm) == PARM_DECL - && uses_template_parms (TREE_TYPE (parm))) - continue; - } - tree arg = TREE_PURPOSE (tparm); reopen_deferring_access_checks (*checks); location_t save_loc = input_location; if (DECL_P (parm)) input_location = DECL_SOURCE_LOCATION (parm); - if (saw_undeduced == 1) ++processing_template_decl; - arg = tsubst_template_arg (arg, full_targs, fcomplain, NULL_TREE); + + if (saw_undeduced == 1 + && TREE_CODE (parm) == PARM_DECL + && uses_template_parms (TREE_TYPE (parm))) + { + /* The type of this non-type parameter depends on undeduced + parameters. Don't try to use its default argument yet, + but do check whether the arguments we already have cause + substitution failure, so that that happens before we try + later default arguments (78489). */ + tree type = tsubst (TREE_TYPE (parm), full_targs, complain, + NULL_TREE); + if (type == error_mark_node) + arg = error_mark_node; + else + arg = NULL_TREE; + } + else + { + arg = tsubst_template_arg (arg, full_targs, complain, NULL_TREE); + + if (!uses_template_parms (arg)) + arg = convert_template_argument (parm, arg, full_targs, + complain, i, NULL_TREE); + else if (saw_undeduced == 1) + arg = NULL_TREE; + else + arg = error_mark_node; + } + if (saw_undeduced == 1) --processing_template_decl; - - if (arg != error_mark_node && !uses_template_parms (arg)) - arg = convert_template_argument (parm, arg, full_targs, complain, - i, NULL_TREE); - else if (saw_undeduced == 1) - arg = NULL_TREE; - else - arg = error_mark_node; input_location = save_loc; *checks = get_deferred_access_checks (); pop_deferring_access_checks (); + if (arg == error_mark_node) return 1; else if (arg) diff --git a/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C b/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C new file mode 100644 index 000..023d9afdf70 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/fntmpdefarg4a.C @@ -0,0 +1,6 @@ +// PR c++/55724 +// { dg-do compile { target c++11 } } + +template struct S {}; +template void f(S) {} +int main() { S<1> s; f(s); } diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae60.C b/gcc/testsuite/g++.dg/cpp0x/sfinae60.C new file mode 100644 index 000..cfb4dc0b9a7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/sfinae60.C @@ -0,0 +1,25 @@ +// PR c++/78489 +// { dg-do compile { target c++11 } } + +template struct enable_if { using type = T; }; +template struct enable_if {}; + +template struct use_type { using type = int; }; + +template +struct get_type { +static_assert(Pred, ""); +using type = int; +}; + +template ::type, // Evaluation/Substitution should end here + class ValT = typename get_type::type, // This should not be instantiated + typename use_type::type = 0 // This NTTP causes ValT to be required +> +constexpr bool test(int) { return false; } + +template +constexpr bool test(long) { return true; } + +static_assert(test(0), ""); // should call test(long) diff --git a/gcc/testsuite/g++.dg/cpp0x/sfinae61.C b/gcc/test
[PATCH, rs6000, committed] Fix test case builtins-1-le.c
Bill pointed out the following test case has been failing since January. It ends up it needs the same fixup that the builtins-1-be.c test case got. Segher approved this offline. Committed. Peter * gcc.target/powerpc/builtins-1-le.c: Filter out gimple folding disabled message. Fix scan-assembler patterns. Index: gcc/testsuite/gcc.target/powerpc/builtins-1-le.c === --- gcc/testsuite/gcc.target/powerpc/builtins-1-le.c(revision 258824) +++ gcc/testsuite/gcc.target/powerpc/builtins-1-le.c(revision 258825) @@ -1,6 +1,7 @@ /* { dg-do compile { target { powerpc64le-*-* } } } */ /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */ -/* { dg-options "-mcpu=power8 -O0 -mno-fold-gimple" } */ +/* { dg-options "-mcpu=power8 -O0 -mno-fold-gimple -dp" } */ +/* { dg-prune-output "gimple folding of rs6000 builtins has been disabled." } */ /* Test that a number of newly added builtin overloads are accepted by the compiler. */ @@ -36,34 +37,35 @@ vec_rsqrt xvrsqrtesp vec_rsqrte xvrsqrtesp */ -/* { dg-final { scan-assembler-times "vcmpequd." 4 } } */ -/* { dg-final { scan-assembler-times "vcmpgtud." 8 } } */ -/* { dg-final { scan-assembler-times "xxland" 29 } } */ -/* { dg-final { scan-assembler-times "vclzb" 2 } } */ -/* { dg-final { scan-assembler-times "vclzb" 2 } } */ -/* { dg-final { scan-assembler-times "vclzw" 2 } } */ -/* { dg-final { scan-assembler-times "vclzh" 2 } } */ -/* { dg-final { scan-assembler-times "xvcpsgnsp" 1 } } */ -/* { dg-final { scan-assembler-times "xvmuldp" 6 } } */ -/* { dg-final { scan-assembler-times "xvcvdpsxds" 1 } } */ -/* { dg-final { scan-assembler-times "vctsxs" 1 } } */ -/* { dg-final { scan-assembler-times "xvcvdpuxds" 1 } } */ -/* { dg-final { scan-assembler-times "vctuxs" 1 } } */ -/* { dg-final { scan-assembler-times "divd" 4 } } */ -/* { dg-final { scan-assembler-times "divdu" 2 } } */ -/* { dg-final { scan-assembler-times "vmrghb" 3 } } */ -/* { dg-final { scan-assembler-times "vmrghh" 4 } } */ -/* { dg-final { scan-assembler-times "xxmrghw" 4 } } */ -/* { dg-final { scan-assembler-times "xxmrglw" 1 } } */ -/* { dg-final { scan-assembler-times "vmrglh" 3 } } */ -/* { dg-final { scan-assembler-times "mulld" 4 } } */ -/* { dg-final { scan-assembler-times "xxlnor" 19 } } */ -/* { dg-final { scan-assembler-times "xxlor" 14 } } */ -/* { dg-final { scan-assembler-times "vpksdus" 1 } } */ -/* { dg-final { scan-assembler-times "vperm" 2 } } */ -/* { dg-final { scan-assembler-times "xvrdpi" 1 } } */ -/* { dg-final { scan-assembler-times "xxsel" 6 } } */ -/* { dg-final { scan-assembler-times "xxlxor" 6 } } */ +/* { dg-final { scan-assembler-times {\mvcmpequd\M\.} 4 } } */ +/* { dg-final { scan-assembler-times {\mvcmpgtud\M\.} 8 } } */ +/* { dg-final { scan-assembler-times {\mxxland\M} 16 } } */ +/* { dg-final { scan-assembler-times {\mxxlandc\M} 13 } } */ +/* { dg-final { scan-assembler-times {\mvclzb\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mvclzb\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mvclzw\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mvclzh\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mxvcpsgnsp\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mxvmuldp\M} 6 } } */ +/* { dg-final { scan-assembler-times {\mxvcvdpsxds\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mvctsxs\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mxvcvdpuxds\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mvctuxs\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mdivd\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mdivdu\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mvmrghb\M} 3 } } */ +/* { dg-final { scan-assembler-times {\mvmrghh\M} 4 } } */ +/* { dg-final { scan-assembler-times {\mxxmrghw\M} 4 } } */ +/* { dg-final { scan-assembler-times {\mxxmrglw\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mvmrglh\M} 3 } } */ +/* { dg-final { scan-assembler-times {\mmulld\M} 4 } } */ +/* { dg-final { scan-assembler-times {(?n)\mxxlnor\M.*\mboolccv4si3_internal1\M} 6 } } */ +/* { dg-final { scan-assembler-times {(?n)\mxxlor\M.*\mboolv4si3_internal\M} 6 } } */ +/* { dg-final { scan-assembler-times {\mvpksdus\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mvperm\M} 2 } } */ +/* { dg-final { scan-assembler-times {\mxvrdpi\M} 1 } } */ +/* { dg-final { scan-assembler-times {\mxxsel\M} 6 } } */ +/* { dg-final { scan-assembler-times {\mxxlxor\M} 6 } } */ /* The test code is in builtins -1.h. */ #include "builtins-1.h"
[Ada] Fix PR ada/85036
This is a fallout of my fix for PR ada/83016, which allowed the --LINK option of gnatlink to contain switches in addition to the linker itself. There is a conflict with the switches passed with the --GCC option so this patch arranges for the former switches to take precedence over the latter switches when it comes to the final linking step. Tested on x86-64/Linux, applied on the mainline. 2018-03-23 Eric Botcazou PR ada/85036 * gnatlink.adb (Process_Args): Drop existing link switches if multiple switches are passed for --LINK. -- Eric BotcazouIndex: gnatlink.adb === --- gnatlink.adb (revision 258629) +++ gnatlink.adb (working copy) @@ -544,6 +544,11 @@ procedure Gnatlink is end if; -- The other arguments are passed as-is to the linker + -- and override those coming from --GCC= if any. + + if L_Args.all'Last >= 2 then +Gcc_Linker_Options.Set_Last (0); + end if; for J in 2 .. L_Args.all'Last loop Gcc_Linker_Options.Increment_Last;
Re: [PATCH]Correct comment for ADDR_EXPR tree code.
On 03/23/2018 09:44 AM, Renlin Li wrote: > Hi all, > > This is a simple patch to correct the comment for ADDR_EXPR tree code. > > The resulting expression of ADDR_EXPR is a tree with POINTER_TYPE. > So the result mode should ptr_mode instead of Pmode. > > As far as I understand, Pmode is the addressing mode. But not the mode > to represent a pointer (or address?). > > Okay to commit? > > Regards, > Renlin > > gcc/ChangeLog: > > 2018-03-23 Renlin Li > > * tree.def (ADDR_EXPR): Correct the commnet.I'm not sure this is strictly > correct. More importantly, I'm not sure why we care :-0 Modes are more of a target/RTL issue. Why a tree node needs to specify a mode in this case vs a type seems to be the more important question. jeff
[Ada] Fix PR ada/85007
The switch has been broken for a long time so its removal is long overdue. Tested on x86-64/Linux, applied on the mainline. 2018-03-23 Eric Botcazou PR ada/85007 * gnatlink.adb (Gnatlink): Remove handling of -b switch. * doc/gnat_ugn/building_executable_programs_with_gnat.rst (gnatlink): Remove documentation of -b switch. -- Eric BotcazouIndex: doc/gnat_ugn/building_executable_programs_with_gnat.rst === --- doc/gnat_ugn/building_executable_programs_with_gnat.rst (revision 258629) +++ doc/gnat_ugn/building_executable_programs_with_gnat.rst (working copy) @@ -7270,14 +7270,6 @@ The following switches are available wit an executable called :file:`try`. -.. index:: -b (gnatlink) - -:switch:`-b {target}` - Compile your program to run on ``target``, which is the name of a - system configuration. You must have a GNAT cross-compiler built if - ``target`` is not the same as your host system. - - .. index:: -B (gnatlink) :switch:`-B{dir}` Index: gnatlink.adb === --- gnatlink.adb (revision 258827) +++ gnatlink.adb (working copy) @@ -425,36 +425,6 @@ procedure Gnatlink is elsif Arg'Length = 2 then case Arg (2) is - when 'b' => -Linker_Options.Increment_Last; -Linker_Options.Table (Linker_Options.Last) := - new String'(Arg); - -Binder_Options.Increment_Last; -Binder_Options.Table (Binder_Options.Last) := - Linker_Options.Table (Linker_Options.Last); - -Next_Arg := Next_Arg + 1; - -if Next_Arg > Argument_Count then - Exit_With_Error ("Missing argument for -b"); -end if; - -Get_Machine_Name : declare - Name_Arg : constant String_Access := -new String'(Argument (Next_Arg)); - -begin - Linker_Options.Increment_Last; - Linker_Options.Table (Linker_Options.Last) := - Name_Arg; - - Binder_Options.Increment_Last; - Binder_Options.Table (Binder_Options.Last) := - Name_Arg; - -end Get_Machine_Name; - when 'f' => if Object_List_File_Supported then Object_List_File_Required := True; @@ -1430,7 +1400,6 @@ procedure Gnatlink is Write_Line (" -v -v Very verbose mode"); Write_Eol; Write_Line (" -o nam Use 'nam' as the name of the executable"); - Write_Line (" -b target Compile the binder source to run on target"); Write_Line (" -Bdir Load compiler executables from dir"); if Is_Supported (Map_File) then
Re: RFC: Disable asan tests under ulimit -v
On Mar 23, 2018, at 11:52 AM, Jason Merrill wrote: > > asan doesn't work under ulimit -v, which I want to use on shared hosts > to avoid causing trouble with runaway processes. There doesn't seem > to be a way within expect to access getrlimit/setrlimit, so in this > patch I call out to the shell to test the current limit, and give up > if I get back a number (rather than "unlimited" or an error message). > > Thoughts? I'd punt to the asan folks. Seems reasonable if it doesn't work.
[PATCH,Testsuite,MIPS] Fixing fix-r4000-n.c failure started with r255348
Hi: The fix-r4000-n.c test fails after r255348, cause the r255348 does not print "[length = NN]" but "[c=NN l=NN]". The asm for fix-r4000-1.c. before r255348: ... mult$4,$5# 10 mulsi3_r4000[length = 8] mflo$2 ... after r255348: ... mult$4,$5# 10 [c=40 l=8] mulsi3_r4000 mflo$2 ... So changes those tests for match new print styles. The changes in patch for fix-r4000-1.c -/* { dg-final { scan-assembler-times "[concat {\tmult\t\$[45],\$[45][^\n]+mulsi3_r4000[^\n]+\n\tmflo\t\$2\n}]" 2 } } */ +/* { dg-final { scan-assembler-times "[concat {\tmult\t\$[45],\$[45][^\n]+mulsi3_r4000\n\tmflo\t\$2\n}]" 2 } } */ And also changes dg-final scan-assembler "mulditi3_r4000" instead of "mulditi3" in fix-r4000-7.c. changes dg-final scan-assembler "umulditi3_r4000" instead of "umulditi3" in fix-r4000-8.c. Thanks. Paul Hua. ChangeLog entries: gcc/testsuite/ChangeLog 2018-03-24 Chenghua Xu * gcc.target/mips/fix-r4000-1.c: Delete "[^\n]" in dg-final. * gcc.target/mips/fix-r4000-2.c: Likewise. * gcc.target/mips/fix-r4000-3.c: Likewise. * gcc.target/mips/fix-r4000-4.c: Likewise. * gcc.target/mips/fix-r4000-5.c: Likewise. * gcc.target/mips/fix-r4000-6.c: Likewise. * gcc.target/mips/fix-r4000-7.c: Likewise. * gcc.target/mips/fix-r4000-8.c: Likewise. * gcc.target/mips/fix-r4000-9.c: Likewise. * gcc.target/mips/fix-r4000-10.c: Likewise. * gcc.target/mips/fix-r4000-7.c: Change dg-final "mulditi3_r4000" instead of "mulditi3". * gcc.target/mips/fix-r4000-8.c: Change dg-final "umulditi3_r4000" instead of "umulditi3". diff --git a/gcc/testsuite/gcc.target/mips/fix-r4000-1.c b/gcc/testsuite/gcc.target/mips/fix-r4000-1.c index 5c812f25600..36062b0f6da 100644 --- a/gcc/testsuite/gcc.target/mips/fix-r4000-1.c +++ b/gcc/testsuite/gcc.target/mips/fix-r4000-1.c @@ -4,4 +4,4 @@ typedef int int32_t; typedef int uint32_t; NOMIPS16 int32_t foo (int32_t x, int32_t y) { return x * y; } NOMIPS16 uint32_t bar (uint32_t x, uint32_t y) { return x * y; } -/* { dg-final { scan-assembler-times "[concat {\tmult\t\$[45],\$[45][^\n]+mulsi3_r4000[^\n]+\n\tmflo\t\$2\n}]" 2 } } */ +/* { dg-final { scan-assembler-times "[concat {\tmult\t\$[45],\$[45][^\n]+mulsi3_r4000\n\tmflo\t\$2\n}]" 2 } } */ diff --git a/gcc/testsuite/gcc.target/mips/fix-r4000-10.c b/gcc/testsuite/gcc.target/mips/fix-r4000-10.c index 7227bc8c092..7345eb511be 100644 --- a/gcc/testsuite/gcc.target/mips/fix-r4000-10.c +++ b/gcc/testsuite/gcc.target/mips/fix-r4000-10.c @@ -6,4 +6,4 @@ typedef unsigned long long uint64_t; typedef unsigned int uint128_t __attribute__((mode(TI))); NOMIPS16 uint128_t foo (uint64_t x, uint64_t y) { return (uint128_t) x * y; } -/* { dg-final { scan-assembler "[concat {\tdmultu\t\$[45],\$[45][^\n]+umulditi3_r4000[^\n]+\n\tmflo\t\$2\n\tmfhi\t\$3\n}]" } } */ +/* { dg-final { scan-assembler "[concat {\tdmultu\t\$[45],\$[45][^\n]+umulditi3_r4000\n\tmflo\t\$2\n\tmfhi\t\$3\n}]" } } */ diff --git a/gcc/testsuite/gcc.target/mips/fix-r4000-2.c b/gcc/testsuite/gcc.target/mips/fix-r4000-2.c index 0261b16b1c8..4290d5f7fab 100644 --- a/gcc/testsuite/gcc.target/mips/fix-r4000-2.c +++ b/gcc/testsuite/gcc.target/mips/fix-r4000-2.c @@ -6,4 +6,4 @@ typedef long long int64_t; NOMIPS16 int32_t foo (int32_t x, int32_t y) { return ((int64_t) x * y) >> 32; } /* ??? A highpart pattern would be a better choice, but we currently don't use them. */ -/* { dg-final { scan-assembler "[concat {\tmult\t\$[45],\$[45][^\n]+mulsidi3_32bit_r4000[^\n]+\n\tmflo\t\$3\n\tmfhi\t\$2\n}]" } } */ +/* { dg-final { scan-assembler "[concat {\tmult\t\$[45],\$[45][^\n]+mulsidi3_32bit_r4000\n\tmflo\t\$3\n\tmfhi\t\$2\n}]" } } */ diff --git a/gcc/testsuite/gcc.target/mips/fix-r4000-3.c b/gcc/testsuite/gcc.target/mips/fix-r4000-3.c index 195a9d10ced..5bc8fc8ddd4 100644 --- a/gcc/testsuite/gcc.target/mips/fix-r4000-3.c +++ b/gcc/testsuite/gcc.target/mips/fix-r4000-3.c @@ -5,4 +5,4 @@ typedef unsigned long long uint64_t; NOMIPS16 uint32_t foo (uint32_t x, uint32_t y) { return ((uint64_t) x * y) >> 32; } /* ??? A highpart pattern would be a better choice, but we currently don't use them. */ -/* { dg-final { scan-assembler "[concat {\tmultu\t\$[45],\$[45][^\n]+umulsidi3_32bit_r4000[^\n]+\n\tmflo\t\$3\n\tmfhi\t\$2\n}]" } } */ +/* { dg-final { scan-assembler "[concat {\tmultu\t\$[45],\$[45][^\n]+umulsidi3_32bit_r4000\n\tmflo\t\$3\n\tmfhi\t\$2\n}]" } } */ diff --git a/gcc/testsuite/gcc.target/mips/fix-r4000-4.c b/gcc/testsuite/gcc.target/mips/fix-r4000-4.c index 7a66182f524..4b655b5edf6 100644 --- a/gcc/testsuite/gcc.target/mips/fix-r4000-4.c +++ b/gcc/testsuite/gcc.target/mips/fix-r4000-4.c @@ -7,4 +7,4 @@ typedef int int32_t; typedef long long int64_t; NOMIPS16 int64_t foo (int32_t x, int32_t y) { return (int64_t) x * y; } -/* { dg-final { scan-assembler "[concat {\tmult\t\$[45],\$[45][^\n]+mulsidi3_32bit_r4000[^\n]+\n\tmflo\t\$2\n\tmfhi\t\$3\n}]" } } */ +/* { dg-fina
[PATCH,Testsuite,MIPS] Fixing umips-stroe16-2.c failure started with r255348
Hi: The umips-stroe16-2.c test fails after r255348, cause the r255348 does not print "[length = NN]" but "[c=NN l=NN]". The asm for umips-stroe16-2.c. before r255348: ... sb $0,0($4) # 9*movqi_internal/6 [length = 2] ... after r255348: ... sb $0,0($4) # 9[c=4 l=2] *movqi_internal/5 ... The patch changs: -/* { dg-final { scan-assembler "\tsb\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*length = 2" } } */ +/* { dg-final { scan-assembler "\tsb\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*l=2" } } */ Thanks. Paul Hua. ChangeLog entries: gcc/testsuite/ChangeLog 2018-03-24 Chenghua Xu * gcc.target/mips/umips-stroe16-2.c: Change "length = 2" to "l=2" in dg-final. diff --git a/gcc/testsuite/gcc.target/mips/umips-store16-2.c b/gcc/testsuite/gcc.target/mips/umips-store16-2.c index 0748edb5692..7fbd5e57305 100644 --- a/gcc/testsuite/gcc.target/mips/umips-store16-2.c +++ b/gcc/testsuite/gcc.target/mips/umips-store16-2.c @@ -17,6 +17,6 @@ f3 (unsigned int *ptr) { *ptr = 0; } -/* { dg-final { scan-assembler "\tsb\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*length = 2" } } */ -/* { dg-final { scan-assembler "\tsh\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*length = 2" } } */ -/* { dg-final { scan-assembler "\tsw\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*length = 2" } } */ +/* { dg-final { scan-assembler "\tsb\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*l=2" } } */ +/* { dg-final { scan-assembler "\tsh\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*l=2" } } */ +/* { dg-final { scan-assembler "\tsw\t\\\$0,0\\(\\\$\[0-9\]+\\)\[^\n\]*l=2" } } */