[PATCH] PR target/86814
Xtensa architecture is not affected by speculation. gcc/ 2018-12-30 Max Filippov * config/xtensa/xtensa.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): Define to speculation_safe_value_not_needed. --- gcc/config/xtensa/xtensa.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/config/xtensa/xtensa.c b/gcc/config/xtensa/xtensa.c index 080bb4ad765d..34e85dcc164a 100644 --- a/gcc/config/xtensa/xtensa.c +++ b/gcc/config/xtensa/xtensa.c @@ -331,6 +331,9 @@ static unsigned HOST_WIDE_INT xtensa_asan_shadow_offset (void); #undef TARGET_ASAN_SHADOW_OFFSET #define TARGET_ASAN_SHADOW_OFFSET xtensa_asan_shadow_offset +#undef TARGET_HAVE_SPECULATION_SAFE_VALUE +#define TARGET_HAVE_SPECULATION_SAFE_VALUE speculation_safe_value_not_needed + struct gcc_target targetm = TARGET_INITIALIZER; -- 2.11.0
[PATCH] Check requested alignment in SET_{DECL,TYPE}_ALIGN is pow2_or_zerop before aligning on targets with partial int modes
There have been some ICEs in the past for msp430-elf with the large memory model (20-bit pointers), caused by SET_{DECL,TYPE}_ALIGN being called with an argument which resolves to 20 i.e. POINTER_SIZE, or the default value of TARGET_VTABLE_ENTRY_ALIGN (which is POINTER_SIZE). The attached patch adds an assertion that SET_{DECL,TYPE}_ALIGN is called with a value which is a power of 2, or 0, for targets which support a partial int mode. This should catch issues in the future with non-power of 2 alignments being set, which could propagate into serious problems later in the compilation process. If the filtering to only perform this check for targets supporting a partial int mode is unnecessary, I can remove that so CHECK_POW2_OR_ZEROP always expands to check_pow2_or_zerop. Successfully bootstrapped and regtested on x86_64-pc-linux-gnu and msp430-elf/-mlarge. Ok for trunk, or does this have to wait for GCC10 Stage 1? >From b44723988dfb0bb9e8c647dd86aeba762ebdf84d Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz Date: Tue, 18 Dec 2018 11:14:35 + Subject: [PATCH] Check requested alignment in SET_{DECL,TYPE}_ALIGN is pow2_or_zerop before aligning on targets with partial int modes 2018-12-30 Jozef Lawrynowicz gcc/ChangeLog: * tree.h (check_pow2_or_zerop): New. (CHECK_POW2_OR_ZEROP): New. (SET_TYPE_ALIGN): Call CHECK_POW2_OR_ZEROP before setting alignment. (SET_DECL_ALIGN): Call CHECK_POW2_OR_ZEROP before setting alignment. --- gcc/tree.h | 26 -- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/gcc/tree.h b/gcc/tree.h index ed37e54..e1188b7 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -74,6 +74,27 @@ as_internal_fn (combined_fn code) return internal_fn (int (code) - int (END_BUILTINS)); } +/* Historically there have been attempts to call SET_DECL/TYPE_ALIGN with + POINTER_SIZE as the alignment. Alignment is expected to always be a power + of 2, so aligning to POINTER_SIZE on targets that use a partial integer + mode for pointers will cause problems. + So for targets that support a partial integer mode, check the requested + alignment is 0 or a power of 2 before aligning. */ +#if defined(HAVE_PQImode) || defined(HAVE_PHImode) \ + || defined(HAVE_PSImode) || defined(HAVE_PDImode) +inline HOST_WIDE_INT +check_pow2_or_zerop (HOST_WIDE_INT x) +{ + gcc_assert (pow2_or_zerop (x)); + return x; +} + +#define CHECK_POW2_OR_ZEROP(X) check_pow2_or_zerop (X) + +#else /* !defined(HAVE_P{Q,H,S,D}Imode) */ +#define CHECK_POW2_OR_ZEROP(X) X +#endif + /* Macros for initializing `tree_contains_struct'. */ #define MARK_TS_BASE(C) \ (tree_contains_struct[C][TS_BASE] = true) @@ -1993,7 +2014,7 @@ extern machine_mode vector_type_mode (const_tree); /* Specify that TYPE_ALIGN(NODE) is X. */ #define SET_TYPE_ALIGN(NODE, X) \ -(TYPE_CHECK (NODE)->type_common.align = ffs_hwi (X)) +(TYPE_CHECK (NODE)->type_common.align = ffs_hwi (CHECK_POW2_OR_ZEROP (X))) /* 1 if the alignment for this type was requested by "aligned" attribute, 0 if it is the default for this type. */ @@ -2444,7 +2465,8 @@ extern machine_mode vector_type_mode (const_tree); ? ((unsigned)1) << ((NODE)->decl_common.align - 1) : 0) /* Specify that DECL_ALIGN(NODE) is X. */ #define SET_DECL_ALIGN(NODE, X) \ -(DECL_COMMON_CHECK (NODE)->decl_common.align = ffs_hwi (X)) +(DECL_COMMON_CHECK (NODE)->decl_common.align \ + = ffs_hwi (CHECK_POW2_OR_ZEROP (X))) /* The minimum alignment necessary for the datum, in bits, without warning. */ -- 2.7.4
Re: [PATCH] fortran/88342 -- interaction of -ffpe-trap and IEEE_VALUE
On Fri, Dec 28, 2018 at 4:38 PM Jerry DeLisle wrote: > > On 12/28/18 10:43 AM, Steve Kargl wrote: > > Ping. > > > > On Mon, Dec 24, 2018 at 11:59:50AM -0800, Steve Kargl wrote: > >> All, > >> > >> The IEEE modules and -ffpe-trap are to some extent orthogonal > >> features of gfortran. Unfortunately, some users have the > >> expectation of using -ffpe-trap for debugging while also using only > >> some of the mechanisms provided by the IEEE modules. For example, > >> > >> % t.f90 > >> program test > >>use, intrinsic :: ieee_arithmetic > >>real :: inf > >>inf = ieee_value(inf, ieee_positive_inf) > >> end program test > >> % gfc8 -o z -ffpe-trap=overflow t.f90 && ./z > >> Floating exception (core dumped) > >> > >> The correct use of the module would be along the lines of > >> > >> program test > >>use, intrinsic :: ieee_arithmetic > >>real :: inf > >>logical h > >>call ieee_get_halting_mode(ieee_overflow, h) ! store halting > >> mode > >>call ieee_set_halting_mode(ieee_overflow, .false.) ! no halting > >>inf = ieee_value(inf, ieee_positive_inf) > >>call ieee_set_halting_mode(ieee_overflow, h) ! restore > >> halting mode > >> end program test > >> > >> Technically (as I have done in the patch), the user should also > >> use 'ieee_support_halting(ieee_overflow)', but that's just a detail. > >> > >> Now, IEEE_VALUE() is specifically included in the Fortran standard > >> to allow it to provide qNaN, sNaN, +inf, and -inf (among a few other > >> questionable constants). The attached patch allows gfortran to > >> generate an executable that does not abort with SIGFPE. > >> > >> 2018-12-24 Steven G. Kargl > >> > >> PR fortran/88342 > >> * ieee/ieee_arithmetic.F90: Prevent exceptions in IEEE_VALUE if > >> -ffpe-trap=invalid or -ffpe-trap=overflow is used. > >> > >> 2018-12-24 Steven G. Kargl > >> > >> PR fortran/88342 > >> * gfortran.dg/ieee/ieee_10.f90: New test. > >> > >> Regression tested on i586-*-freebsd and x86_64-*-freebsd. OK to commit? > >> > > OK Steve, thanks. The test fails on Linux/x86: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88639 -- H.J.
C++ PATCH for c++/88631, CTAD failing for value-initialization
This PR points out that while we are able to deduce the template arguments for A{}, we fail for A(). For A{}, the deduction happens in finish_compound_literal: 2789 if (tree anode = type_uses_auto (type)) 2790 if (CLASS_PLACEHOLDER_TEMPLATE (anode)) 2791 { 2792 type = do_auto_deduction (type, compound_literal, anode, complain, 2793 adc_variable_type) and for A() in build_functional_cast, but there we always give error if there are no parameters. That is wrong because even though there are no arguments to deduce from, we might still be able to deduce from default template arguments as in the test. Fixed thus; I'm passing tf_none if there are no params because we don't want to give redundant diagnostics if the deduction fails. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-12-30 Marek Polacek PR c++/88631 - CTAD failing for value-initialization. * typeck2.c (build_functional_cast): Try deducing the template arguments even if there are no arguments to deduce from. * g++.dg/cpp1z/class-deduction59.C: New test. diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index cc9bf02439b..206a87b94a1 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -2147,9 +2147,18 @@ build_functional_cast (tree exp, tree parms, tsubst_flags_t complain) } else if (!parms) { - if (complain & tf_error) - error ("cannot deduce template arguments for %qT from ()", anode); - return error_mark_node; + /* Even if there are no parameters, we might be able to deduce from +default template arguments. Pass TF_NONE so that we don't +generate redundant diagnostics. */ + type = do_auto_deduction (type, parms, anode, tf_none, + adc_variable_type); + if (type == error_mark_node) + { + if (complain & tf_error) + error ("cannot deduce template arguments for %qT from ()", + anode); + return error_mark_node; + } } else type = do_auto_deduction (type, parms, anode, complain, diff --git gcc/testsuite/g++.dg/cpp1z/class-deduction59.C gcc/testsuite/g++.dg/cpp1z/class-deduction59.C new file mode 100644 index 000..129d723d07c --- /dev/null +++ gcc/testsuite/g++.dg/cpp1z/class-deduction59.C @@ -0,0 +1,12 @@ +// PR c++/88631 +// { dg-do compile { target c++17 } } + +template +class A { }; + +int main() +{ + auto x = A(); + auto x2 = A{}; + A y; +}
Re: V2 [PATCH] i386: Add pass_remove_partial_avx_dependency
On Wed, Nov 28, 2018 at 12:17 PM Jeff Law wrote: > > On 11/28/18 12:48 PM, H.J. Lu wrote: > > On Mon, Nov 5, 2018 at 7:29 AM Jan Hubicka wrote: > >> > >>> On 11/5/18 7:21 AM, Jan Hubicka wrote: > > > > Did you mean "the nearest common dominator"? > > If the nearest common dominator appears in the loop while all uses are > out of loops, this will result in suboptimal xor placement. > In this case you want to split edges out of the loop. > > In general this is what the LCM framework will do for you if the problem > is modelled siimlar way as in mode_swtiching. At entry function mode is > "no zero register needed" and all conversions need mode "zero register > needed". Mode switching should then do the correct placement decisions > (reaching minimal number of executions of xor). > > Jeff, whan is your optinion on the approach taken by the patch? > It seems like a special case of more general issue, but I do not see > very elegant way to solve it at least in the GCC 9 horisont, so if > the placement is correct we can probalby go either with new pass or > making this part of mode swithcing (which is anyway run by x86 backend) > >>> So I haven't followed this discussion at all, but did touch on this > >>> issue with some patch a month or two ago with a target patch that was > >>> trying to avoid the partial stalls. > >>> > >>> My assumption is that we're trying to find one or more places to > >>> initialize the upper half of an avx register so as to avoid partial > >>> register stall at existing sites that set the upper half. > >>> > >>> This sounds like a classic PRE/LCM style problem (of which mode > >>> switching is just another variant). A common-dominator approach is > >>> closer to a classic GCSE and is going to result is more initializations > >>> at sub-optimal points than a PRE/LCM style. > >> > >> yes, it is usual code placement problem. It is special case because the > >> zero register is not modified by the conversion (just we need to have > >> zero somewhere). So basically we do not have kills to the zero except > >> for entry block. > >> > > > > Do you have testcase to show thatf the nearest common dominator > > in the loop, while all uses areout of loops, leads to suboptimal xor > > placement? > I don't have a testcase, but it's all but certain nearest common > dominator is going to be a suboptimal placement. That's going to create > paths where you're going to emit the xor when it's not used. > > The whole point of the LCM algorithms is they are optimal in terms of > expression evaluations. We tried LCM and it didn't work well for this case. LCM places a single VXOR close to the location where it is needed, which can be inside a loop. There is nothing wrong with the LCM algorithms. But this doesn't solve https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87007 where VXOR is executed multiple times inside of a function, instead of just once. We are investigating to generate a single VXOR at entry of the nearest dominator for basic blocks with SF/DF conversions, which is in the the fake loop that contains the whole function: bb = nearest_common_dominator_for_set (CDI_DOMINATORS, convert_bbs); while (bb->loop_father->latch != EXIT_BLOCK_PTR_FOR_FN (cfun)) bb = get_immediate_dominator (CDI_DOMINATORS, bb->loop_father->header); insn = BB_HEAD (bb); if (!NONDEBUG_INSN_P (insn)) insn = next_nonnote_nondebug_insn (insn); set = gen_rtx_SET (v4sf_const0, CONST0_RTX (V4SFmode)); set_insn = emit_insn_before (set, insn); -- H.J.
Re: V2 [PATCH] i386: Add pass_remove_partial_avx_dependency
On Wed, Nov 28, 2018 at 12:21 PM Jan Hubicka wrote: > > > On 11/28/18 12:48 PM, H.J. Lu wrote: > > > On Mon, Nov 5, 2018 at 7:29 AM Jan Hubicka wrote: > > >> > > >>> On 11/5/18 7:21 AM, Jan Hubicka wrote: > > >>>>> > > >>>>> Did you mean "the nearest common dominator"? > > >>>> > > >>>> If the nearest common dominator appears in the loop while all uses are > > >>>> out of loops, this will result in suboptimal xor placement. > > >>>> In this case you want to split edges out of the loop. > > >>>> > > >>>> In general this is what the LCM framework will do for you if the > > >>>> problem > > >>>> is modelled siimlar way as in mode_swtiching. At entry function mode > > >>>> is > > >>>> "no zero register needed" and all conversions need mode "zero register > > >>>> needed". Mode switching should then do the correct placement decisions > > >>>> (reaching minimal number of executions of xor). > > >>>> > > >>>> Jeff, whan is your optinion on the approach taken by the patch? > > >>>> It seems like a special case of more general issue, but I do not see > > >>>> very elegant way to solve it at least in the GCC 9 horisont, so if > > >>>> the placement is correct we can probalby go either with new pass or > > >>>> making this part of mode swithcing (which is anyway run by x86 backend) > > >>> So I haven't followed this discussion at all, but did touch on this > > >>> issue with some patch a month or two ago with a target patch that was > > >>> trying to avoid the partial stalls. > > >>> > > >>> My assumption is that we're trying to find one or more places to > > >>> initialize the upper half of an avx register so as to avoid partial > > >>> register stall at existing sites that set the upper half. > > >>> > > >>> This sounds like a classic PRE/LCM style problem (of which mode > > >>> switching is just another variant). A common-dominator approach is > > >>> closer to a classic GCSE and is going to result is more initializations > > >>> at sub-optimal points than a PRE/LCM style. > > >> > > >> yes, it is usual code placement problem. It is special case because the > > >> zero register is not modified by the conversion (just we need to have > > >> zero somewhere). So basically we do not have kills to the zero except > > >> for entry block. > > >> > > > > > > Do you have testcase to show thatf the nearest common dominator > > > in the loop, while all uses areout of loops, leads to suboptimal xor > > > placement? > > I don't have a testcase, but it's all but certain nearest common > > dominator is going to be a suboptimal placement. That's going to create > > paths where you're going to emit the xor when it's not used. > > > > The whole point of the LCM algorithms is they are optimal in terms of > > expression evaluations. > > i think testcase should be something like > > test() > { > while (true) > { > if (cond1) >{ > do_one_conversion; > return; >} > if (cond2) >{ > do_other_conversion; > return; >} > } > } We got [hjl@gnu-cfl-1 pr87007]$ cat test2.i extern float f1[],f2[]; extern int i1[],i2[]; float foo (int k, int n[]) { if (k == 1) return 1; if (k == 4) return 5; for(int i = 0; i != k; i++){ if(n[i] > 100) f1[i] = i1[i]; else f2[i] = i2[i]; } return k; } [hjl@gnu-cfl-1 pr87007]$ make test2.s /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -O2 -mavx -S test2.i [hjl@gnu-cfl-1 pr87007]$ cat test2.s .file "test2.i" .text .p2align 4 .globl foo .type foo, @function foo: .LFB0: .cfi_startproc vmovss .LC0(%rip), %xmm0 cmpl $1, %edi je .L15 vmovss .LC1(%rip), %xmm0 cmpl $4, %edi je .L15 vxorps %xmm0, %xmm0, %xmm0 testl %edi, %edi je .L3 leal -1(%rdi), %ecx xorl %eax, %eax jmp .L6 .p2align 4,,10 .p2align 3 .L17: vcvtsi2ss i1(,%rax,4), %xmm0, %xmm1 leaq 1(%rax), %rdx vmovss %xmm1, f1(,%rax,4) cmpq %rcx, %rax je .L3 .L9: movq %rdx, %rax .L6: cmpl $100, (%rsi,%rax,4) jg .L17 vcvtsi2ss i2(,%rax,4), %xmm0, %xmm1 leaq 1(%rax), %rdx vmovss %xmm1, f2(,%rax,4) cmpq %rcx, %rax jne .L9 .L3: vcvtsi2ss %edi, %xmm0, %xmm0 .L15: ret .cfi_endproc .LFE0: .size foo, .-foo .section .rodata.cst4,"aM",@progbits,4 .align 4 .LC0: .long 1065353216 .align 4 .LC1: .long 1084227584 .ident "GCC: (GNU) 9.0.0 20181230 (experimental)" .section .note.GNU-stack,"",@progbits [hjl@gnu-cfl-1 pr87007]$ The placement is optimal. -- H.J.
Re: [PATCH] fortran/88342 -- interaction of -ffpe-trap and IEEE_VALUE
On Sun, Dec 30, 2018 at 08:06:40AM -0800, H.J. Lu wrote: > > > > OK Steve, thanks. > > The test fails on Linux/x86: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88639 > The test works on both i586-*-freebsd and x86_64-*-freebsd. What does -- Steve
Re: [PATCH] fortran/88342 -- interaction of -ffpe-trap and IEEE_VALUE
On Sun, Dec 30, 2018 at 9:09 AM Steve Kargl wrote: > > On Sun, Dec 30, 2018 at 08:06:40AM -0800, H.J. Lu wrote: > > > > > > OK Steve, thanks. > > > > The test fails on Linux/x86: > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88639 > > > > The test works on both i586-*-freebsd and x86_64-*-freebsd. > What does > diff --git a/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 b/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 index 9eb4620f0f9..c3cb24d 100644 --- a/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 +++ b/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 @@ -1,5 +1,8 @@ ! { dg-do run } -! { dg-options "-ffpe-trap=overflow,invalid" } +! { dg-additional-options "-ffpe-trap=overflow,invalid" } +! +! Use dg-additional-options rather than dg-options to avoid overwriting the +! default IEEE options which are passed by ieee.exp and necessary. I am checking in this patch as an obvious fix. -- H.J.
Re: [patch, fortran] Handle missing optional MASK for intrinsics
On Sun, Dec 30, 2018 at 06:10:14PM +0100, Thomas Koenig wrote: > Index: gcc/fortran/trans-expr.c > === > --- gcc/fortran/trans-expr.c (Revision 267347) > +++ gcc/fortran/trans-expr.c (Arbeitskopie) > @@ -5760,17 +5760,21 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * >array-descriptor actual to array-descriptor dummy, see >PR 41911 for why a check has to be inserted. >fsym == NULL is checked as intrinsics required the descriptor > - but do not always set fsym. */ > + but do not always set fsym. > + Also, it is necessary to pass a NULL pointer to library routines > + which usually ignoer optional arguments, so they can handle s/ignoer/ignore > + these themselves. */ > if (e->expr_type == EXPR_VARIABLE > && e->symtree->n.sym->attr.optional > - && ((e->rank != 0 && elemental_proc) > - || e->representation.length || e->ts.type == BT_CHARACTER > - || (e->rank != 0 > - && (fsym == NULL > - || (fsym-> as > - && (fsym->as->type == AS_ASSUMED_SHAPE > - || fsym->as->type == AS_ASSUMED_RANK > - || fsym->as->type == AS_DEFERRED)) > + && (((e->rank != 0 && elemental_proc) > +|| e->representation.length || e->ts.type == BT_CHARACTER > +|| (e->rank != 0 > +&& (fsym == NULL > +|| (fsym-> as Remove space in 'fsym-> as' > +&& (fsym->as->type == AS_ASSUMED_SHAPE > +|| fsym->as->type == AS_ASSUMED_RANK > +|| fsym->as->type == AS_DEFERRED) > + || se->ignore_optional)) > gfc_conv_missing_dummy (&parmse, e, fsym ? fsym->ts : e->ts, > e->representation.length); > } (snip) > + { > + tree present; > + tree type; > + > + type = TREE_TYPE (maskse.expr); > + present = gfc_conv_expr_present (maskexpr->symtree->n.sym); > + present = convert (type, present); > + present = fold_build1_loc (input_location, TRUTH_NOT_EXPR, type, > + present); > + ifmask = fold_build2_loc (input_location, TRUTH_ORIF_EXPR, > + type, present, maskse.expr); > + } > + else > + ifmask = maskse.expr; > + This block of code appears multiple time in the patch. I wonder if you should split it out into its own function. static tree generate_mask (gfc_expr *mask) /* Choose whatever name you like. { } You could then just to ifmask = generate_mask (maskse.expr); Other than that, the patch looks ok to me. -- steve
Re: [PATCH] fortran/88342 -- interaction of -ffpe-trap and IEEE_VALUE
On Sun, Dec 30, 2018 at 09:32:43AM -0800, H.J. Lu wrote: > On Sun, Dec 30, 2018 at 9:09 AM Steve Kargl > wrote: > > > > On Sun, Dec 30, 2018 at 08:06:40AM -0800, H.J. Lu wrote: > > > > > > > > OK Steve, thanks. > > > > > > The test fails on Linux/x86: > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88639 > > > > > > > The test works on both i586-*-freebsd and x86_64-*-freebsd. > > What does > > > > diff --git a/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 > b/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 > index 9eb4620f0f9..c3cb24d 100644 > --- a/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 > +++ b/gcc/testsuite/gfortran.dg/ieee/ieee_10.f90 > @@ -1,5 +1,8 @@ > ! { dg-do run } > -! { dg-options "-ffpe-trap=overflow,invalid" } > +! { dg-additional-options "-ffpe-trap=overflow,invalid" } > +! > +! Use dg-additional-options rather than dg-options to avoid overwriting the > +! default IEEE options which are passed by ieee.exp and necessary. > > I am checking in this patch as an obvious fix. > Thanks for the fix and sorry about the breakage. -- Steve
Re: [patch, fortran] Handle missing optional MASK for intrinsics
On Sun, Dec 30, 2018 at 09:37:20AM -0800, Steve Kargl wrote: > On Sun, Dec 30, 2018 at 06:10:14PM +0100, Thomas Koenig wrote: > > > + { > > + tree present; > > + tree type; > > + > > + type = TREE_TYPE (maskse.expr); > > + present = gfc_conv_expr_present (maskexpr->symtree->n.sym); > > + present = convert (type, present); > > + present = fold_build1_loc (input_location, TRUTH_NOT_EXPR, type, > > +present); > > + ifmask = fold_build2_loc (input_location, TRUTH_ORIF_EXPR, > > + type, present, maskse.expr); > > + } > > + else > > + ifmask = maskse.expr; > > + > > This block of code appears multiple time in the patch. > I wonder if you should split it out into its own function. > > static tree > generate_mask (gfc_expr *mask) /* Choose whatever name you like. I suppose that should be s/gfc_expr/tree. -- Steve
[committed] Replace type/fuzzy arguments with a single enum
This is another chunk of Martin's work that I've extracted. It removes the "type" and "fuzzy" parameters we pass to get_range_strlen and replaces them with a strlen_range_kind enum. Mostly this cleans up the API and avoids the possibility of bogus combinations of "type" and "fuzzy". It has one enum value not in Martin's changes. It's temporary until the rest of Martin's work goes in. Essentially it's what used to be fuzzy2 and will be handled by recording two ranges in the later hunks from Martin. By introducing it now we can drop in a significant chunk of the API changes without affecting behavior which makes the final behavior changing patch easier to manage. Bootstrapped and regression tested on x86_64-linux-gnu. Installing on the trunk. My testers will churn on this over the holiday on the other targets. Jeff commit 6454350352aa678b61caa3bc65e463bd1695e7fa Author: Jeff Law Date: Fri Dec 21 14:23:49 2018 -0500 * gimple-fold.c (strlen_range_kind): New enum. (get_range_strlen): Update signature to use strlen_range_kind instead of type+fuzzy. (get_range_strlen_tree): Update signature to use strlen_range_kind instead of type+fuzzy. Pass rkind down to get_range_strlen. Check the rkind enum value instead of type+fuzzy. (get_range_strlen): Similarly. (get_maxval_strlen): Update signature to make NONSTR argument have a default value and make static. Add asserts to ensure sanity of arguments. Update calls to get_range_strlen. (gimple_fold_builtin_strcpy): Update calls to get_maxval_strlen. (gimple_fold_builtin_strcat, gimple_fold_builtin_fputs): Similarly. (gimple_fold_builtin_memory_chk): Similarly. (gimple_fold_builtin_stxcpy_chk): Similarly. (gimple_fold_builtin_snprintf_chk): Similarly. (gimple_fold_builtin_sprintf, gimple_fold_builtin_snprintf): Similarly. * gimple-fold.h (get_maxval_strlen): Delete prototype. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index bb02095e42f..1ee865e1de8 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,25 @@ +2018-12-30 Martin Sebor + Jeff Law + + * gimple-fold.c (strlen_range_kind): New enum. + (get_range_strlen): Update signature to use strlen_range_kind + instead of type+fuzzy. + (get_range_strlen_tree): Update signature to use + strlen_range_kind instead of type+fuzzy. Pass rkind down to + get_range_strlen. Check the rkind enum value instead of + type+fuzzy. + (get_range_strlen): Similarly. + (get_maxval_strlen): Update signature to make NONSTR argument have + a default value and make static. Add asserts to ensure sanity of + arguments. Update calls to get_range_strlen. + (gimple_fold_builtin_strcpy): Update calls to get_maxval_strlen. + (gimple_fold_builtin_strcat, gimple_fold_builtin_fputs): Similarly. + (gimple_fold_builtin_memory_chk): Similarly. + (gimple_fold_builtin_stxcpy_chk): Similarly. + (gimple_fold_builtin_snprintf_chk): Similarly. + (gimple_fold_builtin_sprintf, gimple_fold_builtin_snprintf): Similarly. + * gimple-fold.h (get_maxval_strlen): Delete prototype. + 2018-12-29 Jozef Lawrynowicz * config/msp430/msp430.c (msp430_attr): Warn when the critical and diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 4f84f0c0cc3..76fa328703c 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -66,8 +66,25 @@ along with GCC; see the file COPYING3. If not see #include "tree-vector-builder.h" #include "tree-ssa-strlen.h" -static bool get_range_strlen (tree, tree[2], bitmap *, int, - int, bool *, unsigned, tree *); +enum strlen_range_kind { + /* Compute the exact constant string length. */ + SRK_STRLEN, + /* Compute the maximum constant string length. */ + SRK_STRLENMAX, + /* Compute a range of string lengths bounded by object sizes. When + the length of a string cannot be determined, consider as the upper + bound the size of the enclosing object the string may be a member + or element of. Also determine the size of the largest character + array the string may refer to. */ + SRK_LENRANGE, + /* Temporary until the rest of Martin's strlen range work is integrated. */ + SRK_LENRANGE_2, + /* Determine the integer value of the argument (not string length). */ + SRK_INT_VALUE +}; + +static bool get_range_strlen (tree, tree[2], bitmap *, strlen_range_kind, + bool *, unsigned, tree *); /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. @@ -1264,8 +1281,9 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *gsi, tree c, tree len) /* Helper of get_range_strlen for ARG that is not an SSA_NAME. */ static bool -get_range_strlen_tr
[committed] Minor logic cleanup in get_range_strlen
Just a trivial cleanup from Martin's work. if (x == CONST) do something; else return value; Turns into if (x != CONST) return value; do something; Again no behavior changes, but makes it easier to find the nuggets in the rest of Martin's work. Bootstrapped, regression tested on x86_64 and installed on the trunk. jeff ps. We're just about done with this stuff :-) The holidays are both a blessing and a curse when it comes to wrapping up something like this... commit 0d90d6a22287cecf0c509336005cfd6c27560c67 Author: Jeff Law Date: Fri Dec 21 14:23:49 2018 -0500 * gimple-fold.c (get_range_strlen): Minor logic cleanup. Add comments on code's intent. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 1ee865e1de8..54585381cbe 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,6 +1,9 @@ 2018-12-30 Martin Sebor Jeff Law + * gimple-fold.c (get_range_strlen): Minor logic cleanup. Add comments + on code's intent. + * gimple-fold.c (strlen_range_kind): New enum. (get_range_strlen): Update signature to use strlen_range_kind instead of type+fuzzy. diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 76fa328703c..fb43552bc35 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -1548,10 +1548,17 @@ get_range_strlen (tree arg, tree length[2], bitmap *visited, if (!get_range_strlen (ops[i], length, visited, rkind, flexp, eltsize, nonstr)) { - if (rkind == SRK_LENRANGE_2) - *maxlen = build_all_ones_cst (size_type_node); - else + if (rkind != SRK_LENRANGE_2) return false; + /* Set the upper bound to the maximum to prevent +it from being adjusted in the next iteration but +leave MINLEN and the more conservative MAXBOUND +determined so far alone (or leave them null if +they haven't been set yet). That the MINLEN is +in fact zero can be determined from MAXLEN being +unbounded but the discovered minimum is used for +diagnostics. */ + *maxlen = build_all_ones_cst (size_type_node); } return true; } @@ -1576,10 +1583,17 @@ get_range_strlen (tree arg, tree length[2], bitmap *visited, if (!get_range_strlen (arg, length, visited, rkind, flexp, eltsize, nonstr)) { - if (rkind == SRK_LENRANGE_2) - *maxlen = build_all_ones_cst (size_type_node); - else + if (rkind != SRK_LENRANGE_2) return false; + /* Set the upper bound to the maximum to prevent + it from being adjusted in the next iteration but + leave MINLEN and the more conservative MAXBOUND + determined so far alone (or leave them null if + they haven't been set yet). That the MINLEN is + in fact zero can be determined from MAXLEN being + unbounded but the discovered minimum is used for + diagnostics. */ + *maxlen = build_all_ones_cst (size_type_node); } } return true;
Re: [PATCH] PR target/86814
On Sun, Dec 30, 2018 at 1:06 AM Max Filippov wrote: > > Xtensa architecture is not affected by speculation. > > gcc/ > 2018-12-30 Max Filippov > > * config/xtensa/xtensa.c (TARGET_HAVE_SPECULATION_SAFE_VALUE): > Define to speculation_safe_value_not_needed. Approved.
[C++ PATCH] [PR87768] do not suppress location wrappers when tsubsting
Concepts-checking and other kinds of early tsubsting may often take place while location wrappers are suppressed, e.g. because we've triggered template instantiation within template parameter lists. With that, exprs that are usually wrapped by VIEW_CONVERT_EXPRs location wrappers may end up wrapped by NON_LVALUE_EXPRs that are not marked as location wrappers. If such NON_LVALUE_EXPRs tsubsted exprs undergo another round of tsubsting, say for constraint checking, or even for another round of specialization, they will be rejected by tsubst_copy_and_build. This patch introduces a sentinel to reset suppress_location_wrappers to zero, and uses it for template class and decl instantiation, including constraint checking. Regstrapped on x86_64- and i686-linux-gnu. Ok to install? for gcc/ChangeLog PR c++/87768 * tree.h (auto_restore_location_wrappers): New sentinel class. for gcc/cp/ChangeLog PR c++/87768 * pt.c (instantiate_class_template_1, instantiate_decl): Do not suppress location wrappers. for gcc/testsuite/ChangeLog PR c++/87768 * g++.dg/concepts/pr87768.C: New. --- gcc/cp/pt.c |8 gcc/testsuite/g++.dg/concepts/pr87768.C | 14 ++ gcc/tree.h | 17 + 3 files changed, 39 insertions(+) create mode 100644 gcc/testsuite/g++.dg/concepts/pr87768.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 18b093e7d2d2..e4d07c5103b6 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -10895,6 +10895,10 @@ instantiate_class_template_1 (tree type) it now. */ deferring_access_check_sentinel acs (dk_no_deferred); + /* Do not drop location wrappers just because we're tsubsting e.g. + constraints while parsing e.g. template parameters. */ + auto_restore_location_wrappers sentinel; + /* Determine what specialization of the original template to instantiate. */ t = most_specialized_partial_spec (type, tf_warning_or_error); @@ -24172,6 +24176,10 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p) timevar_push (TV_TEMPLATE_INST); + /* Do not drop location wrappers just because we're tsubsting e.g. + constraints while parsing e.g. template parameters. */ + auto_restore_location_wrappers sentinel; + /* Set TD to the template whose DECL_TEMPLATE_RESULT is the pattern for the instantiation. */ td = template_for_substitution (d); diff --git a/gcc/testsuite/g++.dg/concepts/pr87768.C b/gcc/testsuite/g++.dg/concepts/pr87768.C new file mode 100644 index ..de436e5d9edd --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr87768.C @@ -0,0 +1,14 @@ +// { dg-do compile { target c++17 } } +// { dg-options "-fconcepts" } + +struct a {}; +template using b = a; + +template struct c; +template + requires requires(d e) { e[0]; } +struct c { + static constexpr bool f = [] { return false; }(); +}; + +struct g : b::f> {}; diff --git a/gcc/tree.h b/gcc/tree.h index ed37e5455743..5bab3b284074 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -1194,6 +1194,23 @@ class auto_suppress_location_wrappers ~auto_suppress_location_wrappers () { --suppress_location_wrappers; } }; +/* A class for restoring the creation of location wrappers. */ + +class auto_restore_location_wrappers +{ + const int saved; +public: + auto_restore_location_wrappers () +: saved (suppress_location_wrappers) + { +suppress_location_wrappers = 0; + } + ~auto_restore_location_wrappers () + { +suppress_location_wrappers = saved; + } +}; + /* In a TARGET_EXPR node. */ #define TARGET_EXPR_SLOT(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 0) #define TARGET_EXPR_INITIAL(NODE) TREE_OPERAND_CHECK_CODE (NODE, TARGET_EXPR, 1) -- Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain EngineerFree Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe