Re: [wwwdocs] GCC-7.3 changes: Add note about LEON3FT erratum workaround
On 2018-01-31 00:27, Eric Botcazou wrote: I would like to commit the following comment about LEON3-FT errata now available in GCC-7.3. Fine with me, thanks. Thanks, I have now committed it.
Re: [PATCH] Fix vectorizer ICEs with -ftrapv (PR tree-optimization/81661, PR tree-optimization/84117)
On Wed, 31 Jan 2018, Jakub Jelinek wrote:
> Hi!
>
> Some spots in the vectorizer create generic COND_EXPRs that in one of the
> branches compute some +/-/* arithmetics. When -ftrapv, the gimplifier
> ICEs on this as it may trap, we can't emit code with multiple basic blocks
> from the APIs and don't want to evaluate it if the guarding condition is
> false.
>
> This patch resolves it by making that arithmetic untrappable (i.e. in
> unsigned types).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Wow, yes - we _do_ have absv patterns. Looks like
rewrite_to_defined_overflow / arith_code_with_undefined_signed_overflow
miss handling of ABS_EXPR. This again reminds me of adding
ABSU_EXPR which would also work nicer for fixing the trapping case.
Anyway, we probably want to postpone this "cleanup" to GCC 9.
I wonder if you actually ran into the case of ABS_EXPR being
passed to rewrite_to_non_trapping_overflow?
Ok for trunk.
Thanks,
Richard.
> 2018-01-31 Jakub Jelinek
>
> PR tree-optimization/81661
> PR tree-optimization/84117
> * tree-eh.h (rewrite_to_non_trapping_overflow): Declare.
> * tree-eh.c: Include gimplify.h.
> (find_trapping_overflow, replace_trapping_overflow,
> rewrite_to_non_trapping_overflow): New functions.
> * tree-vect-loop.c: Include tree-eh.h.
> (vect_get_loop_niters): Use rewrite_to_non_trapping_overflow.
> * tree-data-ref.c: Include tree-eh.h.
> (get_segment_min_max): Use rewrite_to_non_trapping_overflow.
>
> * gcc.dg/pr81661.c: New test.
> * gfortran.dg/pr84117.f90: New test.
>
> --- gcc/tree-eh.h.jj 2018-01-03 10:19:54.561533862 +0100
> +++ gcc/tree-eh.h 2018-01-31 11:59:36.328182707 +0100
> @@ -37,6 +37,7 @@ extern bool operation_could_trap_helper_
> bool, tree, bool *);
> extern bool operation_could_trap_p (enum tree_code, bool, bool, tree);
> extern bool tree_could_trap_p (tree);
> +extern tree rewrite_to_non_trapping_overflow (tree);
> extern bool stmt_could_throw_p (gimple *);
> extern bool tree_could_throw_p (tree);
> extern bool stmt_can_throw_external (gimple *);
> --- gcc/tree-eh.c.jj 2018-01-03 10:19:56.103534109 +0100
> +++ gcc/tree-eh.c 2018-01-31 16:51:00.726119778 +0100
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "attribs.h"
> #include "asan.h"
> +#include "gimplify.h"
>
> /* In some instances a tree and a gimple need to be stored in a same table,
> i.e. in hash tables. This is a structure to do this. */
> @@ -2720,6 +2721,91 @@ tree_could_trap_p (tree expr)
> }
> }
>
> +/* Return non-NULL if there is an integer operation with trapping overflow
> + we can rewrite into non-trapping. Called via walk_tree from
> + rewrite_to_non_trapping_overflow. */
> +
> +static tree
> +find_trapping_overflow (tree *tp, int *walk_subtrees, void *data)
> +{
> + if (EXPR_P (*tp)
> + && !operation_no_trapping_overflow (TREE_TYPE (*tp), TREE_CODE (*tp)))
> +return *tp;
> + if (IS_TYPE_OR_DECL_P (*tp)
> + || (TREE_CODE (*tp) == SAVE_EXPR && data == NULL))
> +*walk_subtrees = 0;
> + return NULL_TREE;
> +}
> +
> +/* Rewrite selected operations into unsigned arithmetics, so that they
> + don't trap on overflow. */
> +
> +static tree
> +replace_trapping_overflow (tree *tp, int *walk_subtrees, void *data)
> +{
> + if (find_trapping_overflow (tp, walk_subtrees, data))
> +{
> + tree type = TREE_TYPE (*tp);
> + tree utype = unsigned_type_for (type);
> + *walk_subtrees = 0;
> + int len = TREE_OPERAND_LENGTH (*tp);
> + for (int i = 0; i < len; ++i)
> + walk_tree (&TREE_OPERAND (*tp, i), replace_trapping_overflow,
> +data, (hash_set *) data);
> +
> + if (TREE_CODE (*tp) == ABS_EXPR)
> + {
> + tree op = TREE_OPERAND (*tp, 0);
> + op = save_expr (op);
> + /* save_expr skips simple arithmetics, which is undesirable
> + here, if it might trap due to flag_trapv. We need to
> + force a SAVE_EXPR in the COND_EXPR condition, to evaluate
> + it before the comparison. */
> + if (EXPR_P (op)
> + && TREE_CODE (op) != SAVE_EXPR
> + && walk_tree (&op, find_trapping_overflow, NULL, NULL))
> + {
> + op = build1_loc (EXPR_LOCATION (op), SAVE_EXPR, type, op);
> + TREE_SIDE_EFFECTS (op) = 1;
> + }
> + /* Change abs (op) to op < 0 ? -op : op and handle the NEGATE_EXPR
> + like other signed integer trapping operations. */
> + tree cond = fold_build2 (LT_EXPR, boolean_type_node,
> +op, build_int_cst (type, 0));
> + tree neg = fold_build1 (NEGATE_EXPR, utype,
> + fold_convert (utype, op));
> + *tp = fold_build3 (COND_EXPR, type, cond,
> + fold_convert (type, neg), op);
> + }
>
[PATCH] PR 83975 Associate target with non-constant character length
When associating a variable of type character, if the length of the
target isn't known at compile time, generate an error. See PR 83344
for more details.
Regtested on x86_64-pc-linux-gnu, Ok for trunk?
gcc/fortran/ChangeLog:
2018-02-01 Janne Blomqvist
PR 83975
PR 83344
* resolve.c (resolve_assoc_var): Generate an error if
target length unknown.
---
gcc/fortran/resolve.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 9c8ba86..60b9bc3 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -8634,11 +8634,16 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
if (!sym->ts.u.cl)
sym->ts.u.cl = target->ts.u.cl;
- if (!sym->ts.u.cl->length && !sym->ts.deferred
- && target->expr_type == EXPR_CONSTANT)
- sym->ts.u.cl->length
- = gfc_get_int_expr (gfc_charlen_int_kind,
- NULL, target->value.character.length);
+ if (!sym->ts.u.cl->length && !sym->ts.deferred)
+ {
+ if (target->expr_type == EXPR_CONSTANT)
+ sym->ts.u.cl->length =
+ gfc_get_int_expr (gfc_charlen_int_kind, NULL,
+ target->value.character.length);
+ else
+ gfc_error ("Not Implemented: Associate target with type character"
+ " and non-constant length at %L", &target->where);
+ }
}
/* If the target is a good class object, so is the associate variable. */
--
2.7.4
Re: [PATCH] Fix vectorizer ICEs with -ftrapv (PR tree-optimization/81661, PR tree-optimization/84117)
On Thu, Feb 01, 2018 at 09:47:23AM +0100, Richard Biener wrote: > I wonder if you actually ran into the case of ABS_EXPR being > passed to rewrite_to_non_trapping_overflow? I haven't. In the first vrsion of the patch I wasn't using operation_no_trapping_overflow and so just ignored ABS_EXPR, with that helper either I'd need to explicitly punt on ABS_EXPR, or implement it and chose the latter. Yes, we have no testsuite coverage for it :(. > Ok for trunk. Thanks. Jakub
RE: [PATCH 0/6] [ARC] New baremetal features and fixes
> [ARC] Add JLI support. > [ARC] Add SJLI support. > [ARC] Add support for "register file 16" reduced register set > [ARC] Rework delegitimate_address hook > [ARC] Add 'uncached' attribute. > [ARC] Add 'aux' variable attribute. Thank you all for reviewing them. //Claudiu
Re: Fix PR rtl-optimization/84071
On 31/01/18 19:04, Richard Sandiford wrote:
> Eric Botcazou writes:
>>> Tested on SPARC64/Linux and ARM/EABI, applied on mainline and 7 branch.
>>
>> As discussed in the audit trail, this beefs up the internal documentation
>> about WORD_REGISTER_OPERATIONS.
>>
>> Tested with 'make doc', applied on mainline and 7 branch.
>>
>>
>> 2018-01-31 Eric Botcazou
>>
>> PR rtl-optimization/84071
>> * doc/tm.texi.in (WORD_REGISTER_OPERATIONS): Add explicit case.
>> * doc/tm.texi: Regenerate.
>>
>> Index: doc/tm.texi.in
>> ===
>> --- doc/tm.texi.in (revision 257227)
>> +++ doc/tm.texi.in (working copy)
>> @@ -7376,8 +7376,12 @@ is in effect.
>>
>> @defmac WORD_REGISTER_OPERATIONS
>> Define this macro to 1 if operations between registers with integral mode
>> -smaller than a word are always performed on the entire register.
>> -Most RISC machines have this property and most CISC machines do not.
>> +smaller than a word are always performed on the entire register. To be
>> +more explicit, if you start with a pair of @code{word_mode} registers with
>> +known values and you do a subword, for example @code{QImode}, addition on
>> +the low part of the registers, then the compiler may consider that the
>> +result has a known value in @code{word_mode} too if the macro is defined
>> +to 1. Most RISC machines have this property and most CISC machines do not.
>
> By QImode addition, do you mean:
>
>(set (subreg:QI (reg:SI X1) N)
> (plus:QI (subreg:QI (reg:SI X2) N)
>(subreg:QI (reg:SI X3) N)))
>
> ? I thought the point was instead that the target expected such ops
> to be done on word_mode, even if the values involved are naturally QImode:
>
>(set (subreg:SI (reg:QI Y1) 0)
> (plus:SI (subreg:SI (reg:QI Y2) 0)
>(subreg:SI (reg:QI Y3) 0)))
>
> Most RISC/WORD_REGISTER_OPERATIONS targets wouldn't provide QImode
> addition patterns, so the first insn seems unlikely.
>
> Thanks,
> Richard
>
That's always been my interpretation too. Seems like we may be changing
the meaning of this macro...
R.
Re: [PATCH] Fix gcc.target/aarch64/sve/peel_ind_1.c for -mcmodel=tiny
On Wed, Jan 24, 2018 at 04:28:13PM +, Szabolcs Nagy wrote:
> Fix test failures with -mcmodel=tiny when adr is generated instead of adrp.
>
> FAIL: gcc.target/aarch64/sve/peel_ind_1.c -march=armv8.2-a+sve scan-assembler
> \\tadrp\\tx[0-9]+, x\\n
> FAIL: gcc.target/aarch64/sve/peel_ind_2.c -march=armv8.2-a+sve scan-assembler
> \\tadrp\\tx[0-9]+, x\\n
> FAIL: gcc.target/aarch64/sve/peel_ind_3.c -march=armv8.2-a+sve scan-assembler
> \\tadrp\\tx[0-9]+, x\\n
I can't tell if this is a request for a review, or if you commited it
as obvious, but this is OK.
James
>
> gcc/testsuite/ChangeLog:
>
> 2018-01-24 Szabolcs Nagy
>
> * gcc.target/aarch64/sve/peel_ind_1.c: Match (adrp|adr) in
> scan-assembler.
> * gcc.target/aarch64/sve/peel_ind_2.c: Likewise.
> * gcc.target/aarch64/sve/peel_ind_3.c: Likewise.
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> index 864026499cd..a064c337b67 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_1.c
> @@ -21,7 +21,7 @@ foo (void)
> }
>
> /* We should operate on aligned vectors. */
> -/* { dg-final { scan-assembler {\tadrp\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> /* We should use an induction that starts at -5, with only the last
> 7 elements of the first iteration being active. */
> /* { dg-final { scan-assembler {\tindex\tz[0-9]+\.s, #-5, #5\n} } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> index 2bfc09a7602..f2113be90a7 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_2.c
> @@ -17,6 +17,6 @@ foo (void)
> }
>
> /* We should operate on aligned vectors. */
> -/* { dg-final { scan-assembler {\tadrp\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> /* We should unroll the loop three times. */
> /* { dg-final { scan-assembler-times "\tst1w\t" 3 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> index 8364dc6107a..441589eef60 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/peel_ind_3.c
> @@ -17,5 +17,5 @@ foo (int start)
> }
>
> /* We should operate on aligned vectors. */
> -/* { dg-final { scan-assembler {\tadrp\tx[0-9]+, x\n} } } */
> +/* { dg-final { scan-assembler {\t(adrp|adr)\tx[0-9]+, x\n} } } */
> /* { dg-final { scan-assembler {\tubfx\t} } } */
Re: [PATCH v2][AArch64] Remove remaining uses of * in patterns
On Tue, Jan 16, 2018 at 04:32:36PM +, Wilco Dijkstra wrote: > v2: Rebased after the big SVE commits > > Remove the remaining uses of '*' from aarch64.md. > Using '*' in alternatives is typically incorrect as it tells the register > allocator to ignore those alternatives. Also add a missing '?' so we > prefer a floating point register for same-size int<->fp conversions. > > Passes regress & bootstrap, OK for commit? Please queue for GCC 9. OK when trunk is back open for new code. Thanks, James > ChangeLog: > 2018-01-16 Wilco Dijkstra > > * config/aarch64/aarch64.md (mov): Remove '*' in alternatives. > (movsi_aarch64): Likewise. > (load_pairsi): Likewise. > (load_pairdi): Likewise. > (store_pairsi): Likewise. > (store_pairdi): Likewise. > (load_pairsf): Likewise. > (load_pairdf): Likewise. > (store_pairsf): Likewise. > (store_pairdf): Likewise. > (zero_extend): Likewise. > (fcvt_target): Add '?' to prefer w over r. > > gcc/testsuite/ > * gcc.target/aarch64/vfp-1.c: Update test. >
Re: [AArch64] Generalise aarch64_simd_valid_immediate for SVE
On Fri, Jan 26, 2018 at 01:47:48PM +, Richard Sandiford wrote:
> The current aarch64_simd_valid_immediate code predates the move
> to the new CONST_VECTOR representation, so for variable-length SVE
> it only handles duplicates of single elements, rather than duplicates
> of repeating patterns.
>
> This patch removes the restriction. It means that the validity
> of a duplicated constant depends only on the bit pattern, not on
> the mode used to represent it.
>
> The patch is needed by a later big-endian fix.
>
> Tested on aarch64_be-elf and aarch64-linux-gnu. OK to install?
OK.
Thanks,
James
> 2018-01-26 Richard Sandiford
>
> gcc/
> * config/aarch64/aarch64.c (aarch64_simd_valid_immediate): Handle
> all CONST_VECTOR_DUPLICATE_P vectors, not just those with a single
> duplicated element.
>
> Index: gcc/config/aarch64/aarch64.c
> ===
> --- gcc/config/aarch64/aarch64.c 2018-01-26 13:32:54.240529011 +
> +++ gcc/config/aarch64/aarch64.c 2018-01-26 13:46:00.955822193 +
> @@ -13164,10 +13164,11 @@ aarch64_simd_valid_immediate (rtx op, si
> return false;
>
>scalar_mode elt_mode = GET_MODE_INNER (mode);
> - rtx elt = NULL, base, step;
> + rtx base, step;
>unsigned int n_elts;
> - if (const_vec_duplicate_p (op, &elt))
> -n_elts = 1;
> + if (GET_CODE (op) == CONST_VECTOR
> + && CONST_VECTOR_DUPLICATE_P (op))
> +n_elts = CONST_VECTOR_NPATTERNS (op);
>else if ((vec_flags & VEC_SVE_DATA)
> && const_vec_series_p (op, &base, &step))
> {
> @@ -13192,14 +13193,17 @@ aarch64_simd_valid_immediate (rtx op, si
> || op == CONSTM1_RTX (mode));
>
>scalar_float_mode elt_float_mode;
> - if (elt
> - && is_a (elt_mode, &elt_float_mode)
> - && (aarch64_float_const_zero_rtx_p (elt)
> - || aarch64_float_const_representable_p (elt)))
> + if (n_elts == 1
> + && is_a (elt_mode, &elt_float_mode))
> {
> - if (info)
> - *info = simd_immediate_info (elt_float_mode, elt);
> - return true;
> + rtx elt = CONST_VECTOR_ENCODED_ELT (op, 0);
> + if (aarch64_float_const_zero_rtx_p (elt)
> + || aarch64_float_const_representable_p (elt))
> + {
> + if (info)
> + *info = simd_immediate_info (elt_float_mode, elt);
> + return true;
> + }
> }
>
>unsigned int elt_size = GET_MODE_SIZE (elt_mode);
> @@ -13214,11 +13218,11 @@ aarch64_simd_valid_immediate (rtx op, si
>bytes.reserve (n_elts * elt_size);
>for (unsigned int i = 0; i < n_elts; i++)
> {
> - if (!elt || n_elts != 1)
> - /* The vector is provided in gcc endian-neutral fashion.
> -For aarch64_be, it must be laid out in the vector register
> -in reverse order. */
> - elt = CONST_VECTOR_ELT (op, BYTES_BIG_ENDIAN ? (n_elts - 1 - i) : i);
> + /* The vector is provided in gcc endian-neutral fashion.
> + For aarch64_be Advanced SIMD, it must be laid out in the vector
> + register in reverse order. */
> + bool swap_p = ((vec_flags & VEC_ADVSIMD) != 0 && BYTES_BIG_ENDIAN);
> + rtx elt = CONST_VECTOR_ELT (op, swap_p ? (n_elts - 1 - i) : i);
>
>if (elt_mode != elt_int_mode)
> elt = gen_lowpart (elt_int_mode, elt);
Re: [AArch64] Tighten aarch64_secondary_reload condition (PR 83845)
On Fri, Jan 26, 2018 at 01:34:31PM +, Richard Sandiford wrote: > aarch64_secondary_reload enforced a secondary reload via > aarch64_sve_reload_be for memory and pseudo registers, but failed > to do the same for subregs of pseudo registers. To avoid this and > any similar problems, the patch instead tests for things that the move > patterns handle directly; if the operand isn't one of those, we should > use the reload pattern instead. > > The patch fixes an ICE in sve/mask_struct_store_3.c for aarch64_be, > where the bogus target description was (rightly) causing LRA to cycle. > > Tested on aarch64_be-elf and aarch64-linux-gnu. OK to install? OK. Thanks, James > > Richard > > > 2018-01-26 Richard Sandiford > > gcc/ > PR tearget/83845 > * config/aarch64/aarch64.c (aarch64_secondary_reload): Tighten > check for operands that need to go through aarch64_sve_reload_be. sri->icode = CODE_FOR_aarch64_sve_reload_be;
Re: [AArch64] Fix SVE testsuite failures for ILP32 (PR 83846)
On Fri, Jan 26, 2018 at 02:25:02PM +, Richard Sandiford wrote: > The SVE tests are split into code-quality compile tests and runtime > tests. A lot of the former are geared towards LP64. It would be > possible (but tedious!) to mark up every line that is expected to work > only for LP64, but I think it would be a constant source of problems. > > Since the code has not been tuned for ILP32 yet, I think the best > thing is to select only the runtime tests for that combination. > They all pass on aarch64-elf and aarch64_be-elf except vec-cond-[34].c, > which are unsupported due to the lack of fenv support. > > The patch also replaces uses of built-in types with stdint.h types > where possible. (This excludes tests that change the endianness, > since we can't assume that system header files work in that case.) > > Tested on aarch64_be-elf and aarch64-linux-gnu. OK to install? This is unfortunate, but I suppose this is the pragmatic fix. I hope we can turn them back on in GCC 9. Maybe this should have a bugzilla to track that? OK. James > 2018-01-17 Richard Sandiford > > gcc/testsuite/ > PR testsuite/83846 > * gcc.target/aarch64/sve/aarch64-sve.exp: Only do *_run tests > for ILP32. > * gcc.target/aarch64/sve/clastb_2_run.c (main): Use TYPE instead > of hard-coding the choice. > * gcc.target/aarch64/sve/clastb_4_run.c (main): Likewise. > * gcc.target/aarch64/sve/clastb_5_run.c (main): Likewise. > * gcc.target/aarch64/sve/clastb_3_run.c (main): Likewise. Generalize > memset call. > * gcc.target/aarch64/sve/const_pred_1.C: Include stdint.h and use > stdint.h types. > * gcc.target/aarch64/sve/const_pred_2.C: Likewise. > * gcc.target/aarch64/sve/const_pred_3.C: Likewise. > * gcc.target/aarch64/sve/const_pred_4.C: Likewise. > * gcc.target/aarch64/sve/load_const_offset_2.c: Likewise. > * gcc.target/aarch64/sve/logical_1.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_1.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_2.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_3.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_4.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_5.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_6.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_7.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_load_8.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_store_1.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_store_2.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_store_3.c: Likewise. > * gcc.target/aarch64/sve/mask_struct_store_4.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_1.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_2.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_2_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_3.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_3_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_4.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_4_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_7.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_8.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_8_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_9.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_9_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_10.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_10_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_11.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_11_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_12.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_12_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_13.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_13_run.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_14.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_18.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_19.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_20.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_21.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_22.c: Likewise. > * gcc.target/aarch64/sve/struct_vect_23.c: Likewise. > * gcc.target/aarch64/sve/popcount_1.c (popcount_64): Use > __builtin_popcountll rather than __builtin_popcountl.
Re: [PATCH/RFC] Fix ICE in find_taken_edge_computed_goto (PR 84136)
On Wed, Jan 31, 2018 at 4:39 PM, David Malcolm wrote:
> PR 84136 reports an ICE within sccvn_dom_walker when handling a
> C/C++ source file that overuses the labels-as-values extension.
> The code in question stores a jump label into a global, and then
> jumps to it from another function, which ICEs after inlining:
>
> void* a;
>
> void foo() {
> if ((a = &&l))
> return;
>
> l:;
> }
>
> int main() {
> foo();
> goto *a;
>
> return 0;
> }
>
> This appears to be far beyond what we claim to support in this
> extension - but we shouldn't ICE.
>
> What's happening is that, after inlining, we have usage of a *copy*
> of the label, which optimizes away the if-return logic, turning it
> into an infinite loop.
>
> On entry to the sccvn_dom_walker we have this gimple:
>
> main ()
> {
> void * a.0_1;
>
>[count: 0]:
> a = &l;
>
>[count: 0]:
> l:
> a.0_1 = a;
> goto a.0_1;
> }
>
> and:
> edge taken = find_taken_edge (bb, vn_valueize (val));
> reasonably valueizes the:
> goto a.0_1;
> after the:
> a = &l;
> a.0_1 = a;
> as if it were:
> goto *&l;
>
> find_taken_edge_computed_goto then has:
>
> 2380 dest = label_to_block (val);
> 2381 if (dest)
> 2382{
> 2383 e = find_edge (bb, dest);
> 2384 gcc_assert (e != NULL);
> 2385}
>
> which locates dest as a self-jump from block 3 back to itself.
>
> However, the find_edge call returns NULL - it has a predecessor edge
> from block 2, but no successor edges.
>
> Hence the assertion fails and we ICE.
>
> A successor edge from the computed goto could have been created by
> make_edges if the label stmt had been in the function, but make_edges
> only looks in the current function when handling computed gotos, and
> the label only appeared after inlining.
>
> The following patch removes the assertion, fixing the ICE.
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> If that's option (a), there could be some other approaches:
>
> (b) convert the assertion into a warning/error/sorry, on the
> assumption that if we don't detect such an edge then the code is
> presumably abusing the labels-as-values feature
> (c) have make_edges detect such a problematic computed goto (maybe
> converting make_edges_bb's return value to an enum and adding a 4th
> value - though it's not clear what to do then with it)
> (d) detect this case on inlining and handle it somehow (e.g. adding
> edges for labels that have appeared since make_edges originally
> ran, for computed gotos that have no out-edges)
> (e) do nothing, keeping the assertion, and accept that this is going
> to fail on a non-release build
> (f) something else?
>
> Of the above, (d) seems to me to be the most robust solution, but I
> don't know how far we want to go "down the rabbit hole" of handling
> such uses of labels-as-values (beyond not ICE-ing on them).
>
> Thoughts?
I think you can preserve the assert for ! DECL_NONLOCAL (val) thus
gcc_assert (e != NULL || DECL_NONLOCAL (val));
does the label in this case properly have DECL_NONLOCAL set? Probably
not given we shouldn't have duplicated it in this case. So the issue is really
that the FE doesn't set this bit for "escaped" labels... but I'm not sure how
to easily constrain the extension here.
The label should be FORCED_LABEL though so that's maybe a weaker
check.
Joseph?
Richard.
>
> gcc/testsuite/ChangeLog:
> PR tree-optimization/84136
> * gcc.c-torture/compile/pr84136.c: New test.
>
> gcc/ChangeLog:
> PR tree-optimization/84136
> * tree-cfg.c (find_taken_edge_computed_goto): Remove assertion
> that the result of find_edge is non-NULL.
> ---
> gcc/testsuite/gcc.c-torture/compile/pr84136.c | 15 +++
> gcc/tree-cfg.c| 5 +
> 2 files changed, 16 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr84136.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr84136.c
> b/gcc/testsuite/gcc.c-torture/compile/pr84136.c
> new file mode 100644
> index 000..0a70e4e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr84136.c
> @@ -0,0 +1,15 @@
> +void* a;
> +
> +void foo() {
> +if ((a = &&l))
> +return;
> +
> +l:;
> +}
> +
> +int main() {
> +foo();
> +goto *a;
> +
> +return 0;
> +}
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index c5318b9..6b89307 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -2379,10 +2379,7 @@ find_taken_edge_computed_goto (basic_block bb, tree
> val)
>
>dest = label_to_block (val);
>if (dest)
> -{
> - e = find_edge (bb, dest);
> - gcc_assert (e != NULL);
> -}
> +e = find_edge (bb, dest);
>
>return e;
> }
> --
> 1.8.5.3
>
Re: [Libgomp, Fortran] Fix canadian cross build
On 5 October 2017 at 13:38, Petr Ovtchenkov wrote: > On Thu, 5 Oct 2017 12:56:36 +0200 > Yvan Roux wrote: > >> On 5 September 2017 at 12:04, Jakub Jelinek wrote: >> > On Tue, Sep 05, 2017 at 10:58:22AM +0200, Yvan Roux wrote: >> >> ping >> >> >> >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01784.html >> > >> > This really needs to be reviewed by a build machinery maintainer. >> >> Thanks for the CC list update Jakub, ping again then... >> >> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01784.html ping > BTW, > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71212 > https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01332.html > https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01334.html > >> >> >> > config/ChangeLog >> >> > 2017-06-23 Yvan Roux >> >> > >> >> > * acx.m4 (NCN_STRICT_CHECK_TARGET_TOOLS): Renamed to ... >> >> > (NCN_STRICT_PATH_TARGET_TOOLS): ... this. It reflects the >> >> > replacement >> >> > of AC_CHECK_PROG by AC_PATH_PROG to get the absolute name >> >> > of the >> >> > program. >> >> > (ACX_CHECK_INSTALLED_TARGET_TOOL): Use renamed function. >> >> > >> >> > ChangeLog >> >> > 2017-06-23 Yvan Roux >> >> > >> >> > * configure.ac: Use NCN_STRICT_PATH_TARGET_TOOLS instead of >> >> > NCN_STRICT_CHECK_TARGET_TOOLS. >> >> > * configure: Regenerate. >> > >> > Jakub > > -- > >- ptr
[PATCH] Reduce number of pre_and_rev_post_order_compute calls via domwalk
This enhances the domwalk interface to allow disabling of RPO sorting
of dom children (after I've recently enhanced it to allow passing in
the RPO order). Then it attacks the most prominent and often-called
infrastructure helper - update_ssa - which doesn't really need any
special ordering of dom children.
This reduces the number of pre_and_rev_post_order_compute calls
significantly.
There are more low-hanging opportunities which could pass along
the RPO array between functions but this is the biggest and IMHO
only one worth tackling for GCC 8 via the usual compile-time-regression
argument.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
If you are curious, domwalk does sorting of dom children since GCC 4.9
only.
After this patch the most-calling pass is IRA which calls the function
5 times - mostly because it builds and tears down loop structures
three times ... for whatever reason (and init_alias_analysis calls
it which is called by IRA as well).
Richard.
2018-02-01 Richard Biener
* domwalk.h (dom_walker::dom_walker): Add additional constructor
for specifying RPO order and allow NULL for that.
* domwalk.c (dom_walker::dom_walker): Likewise.
(dom_walker::walk): Handle NULL RPO order.
* tree-into-ssa.c (rewrite_dom_walker): Do not walk dom children
in RPO order.
(rewrite_update_dom_walker): Likewise.
(mark_def_dom_walker): Likewise.
Index: gcc/domwalk.c
===
--- gcc/domwalk.c (revision 257233)
+++ gcc/domwalk.c (working copy)
@@ -191,13 +191,41 @@ dom_walker::dom_walker (cdi_direction di
int *bb_index_to_rpo)
: m_dom_direction (direction),
m_skip_unreachable_blocks (reachability != ALL_BLOCKS),
-m_user_bb_to_rpo (bb_index_to_rpo != NULL),
+m_user_bb_to_rpo (true),
m_unreachable_dom (NULL),
m_bb_to_rpo (bb_index_to_rpo)
{
- /* Compute the basic-block index to RPO mapping if not provided by
- the user. */
- if (! m_bb_to_rpo && direction == CDI_DOMINATORS)
+ /* Set up edge flags if need be. */
+ switch (reachability)
+{
+default:
+ gcc_unreachable ();
+case ALL_BLOCKS:
+ /* No need to touch edge flags. */
+ break;
+
+case REACHABLE_BLOCKS:
+ set_all_edges_as_executable (cfun);
+ break;
+
+case REACHABLE_BLOCKS_PRESERVING_FLAGS:
+ /* Preserve the edge flags. */
+ break;
+}
+}
+
+/* Constructor for a dom walker. */
+
+dom_walker::dom_walker (cdi_direction direction,
+ enum reachability reachability)
+ : m_dom_direction (direction),
+m_skip_unreachable_blocks (reachability != ALL_BLOCKS),
+m_user_bb_to_rpo (false),
+m_unreachable_dom (NULL),
+m_bb_to_rpo (NULL)
+{
+ /* Compute the basic-block index to RPO mapping. */
+ if (direction == CDI_DOMINATORS)
{
int *postorder = XNEWVEC (int, n_basic_blocks_for_fn (cfun));
int postorder_num = pre_and_rev_post_order_compute (NULL, postorder,
@@ -348,7 +376,10 @@ dom_walker::walk (basic_block bb)
for (dest = first_dom_son (m_dom_direction, bb);
dest; dest = next_dom_son (m_dom_direction, dest))
worklist[sp++] = dest;
- if (sp - saved_sp > 1 && m_dom_direction == CDI_DOMINATORS)
+ /* Sort worklist after RPO order if requested. */
+ if (sp - saved_sp > 1
+ && m_dom_direction == CDI_DOMINATORS
+ && m_bb_to_rpo)
sort_bbs_postorder (&worklist[saved_sp], sp - saved_sp);
}
}
Index: gcc/domwalk.h
===
--- gcc/domwalk.h (revision 257233)
+++ gcc/domwalk.h (working copy)
@@ -60,10 +60,13 @@ public:
REACHABLE_BLOCKS_PRESERVING_FLAGS
};
+ dom_walker (cdi_direction direction, enum reachability = ALL_BLOCKS);
+
/* You can provide a mapping of basic-block index to RPO if you
- have that readily available or you do multiple walks. */
- dom_walker (cdi_direction direction, enum reachability = ALL_BLOCKS,
- int *bb_index_to_rpo = NULL);
+ have that readily available or you do multiple walks. If you
+ specify NULL as BB_INDEX_TO_RPO dominator children will not be
+ walked in RPO order. */
+ dom_walker (cdi_direction direction, enum reachability, int
*bb_index_to_rpo);
~dom_walker ();
Index: gcc/tree-into-ssa.c
===
--- gcc/tree-into-ssa.c (revision 257233)
+++ gcc/tree-into-ssa.c (working copy)
@@ -1463,7 +1463,8 @@ rewrite_add_phi_arguments (basic_block b
class rewrite_dom_walker : public dom_walker
{
public:
- rewrite_dom_walker (cdi_direction direction) : dom_walker (direction) {}
+ rewrite_dom_walker (cdi_direction direction)
+: dom_walker (direction, ALL_BLOCKS, NULL) {}
virtual edge b
C++ PATCH to kill cxx_print_statistics
Does any of you use print_class_statistics?
It seems most of these variables were set-but-unused even in gcc-2.95 (!). So
I think we can safely 86 all of this. Or is the "n_vtables" info interesting
in any way?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-02-01 Marek Polacek
* class.c: Remove unused global variables.
(build_primary_vtable): Don't gather statistics.
(print_class_statistics): Remove.
* cp-tree.h (print_class_statistics): Remove.
* tree.c (cxx_print_statistics): Don't call print_class_statistics.
diff --git gcc/cp/class.c gcc/cp/class.c
index 4103630231a..7b2afc14dbe 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -215,16 +215,6 @@ static tree end_of_base (tree);
static tree get_vcall_index (tree, tree);
static bool type_maybe_constexpr_default_constructor (tree);
-/* Variables shared between class.c and call.c. */
-
-int n_vtables = 0;
-int n_vtable_entries = 0;
-int n_vtable_searches = 0;
-int n_vtable_elems = 0;
-int n_convert_harshness = 0;
-int n_compute_conversion_costs = 0;
-int n_inner_fields_searched = 0;
-
/* Return a COND_EXPR that executes TRUE_STMT if this execution of the
'structor is in charge of 'structing virtual bases, or FALSE_STMT
otherwise. */
@@ -892,12 +882,6 @@ build_primary_vtable (tree binfo, tree type)
virtuals = NULL_TREE;
}
- if (GATHER_STATISTICS)
-{
- n_vtables += 1;
- n_vtable_elems += list_length (virtuals);
-}
-
/* Initialize the association list for this type, based
on our first approximation. */
BINFO_VTABLE (TYPE_BINFO (type)) = decl;
@@ -8118,23 +8102,6 @@ get_vfield_name (tree type)
return get_identifier (buf);
}
-void
-print_class_statistics (void)
-{
- if (! GATHER_STATISTICS)
-return;
-
- fprintf (stderr, "convert_harshness = %d\n", n_convert_harshness);
- fprintf (stderr, "compute_conversion_costs = %d\n",
n_compute_conversion_costs);
- if (n_vtables)
-{
- fprintf (stderr, "vtables = %d; vtable searches = %d\n",
- n_vtables, n_vtable_searches);
- fprintf (stderr, "vtable entries = %d; vtable elems = %d\n",
- n_vtable_entries, n_vtable_elems);
-}
-}
-
/* Build a dummy reference to ourselves so Derived::Base (and A::A) works,
according to [class]:
The class-name is also inserted
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index 8983674881d..c7d42d3d76f 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -6036,7 +6036,6 @@ extern int current_lang_depth (void);
extern void push_lang_context (tree);
extern void pop_lang_context (void);
extern tree instantiate_type (tree, tree, tsubst_flags_t);
-extern void print_class_statistics (void);
extern void build_self_reference (void);
extern int same_signature_p(const_tree, const_tree);
extern void maybe_add_class_template_decl_list (tree, tree, int);
diff --git gcc/cp/tree.c gcc/cp/tree.c
index e87c59659a5..1ab10099780 100644
--- gcc/cp/tree.c
+++ gcc/cp/tree.c
@@ -2831,7 +2831,6 @@ extern int depth_reached;
void
cxx_print_statistics (void)
{
- print_class_statistics ();
print_template_statistics ();
if (GATHER_STATISTICS)
fprintf (stderr, "maximum template instantiation depth reached: %d\n",
Marek
build failure x86_64
A recent patch has broken RTL checking builds on x86_64 fails during combine RTL check: expected code 'reg' have 'lshirtrt' in rhs_regno at rtl.h:1891 builfing libgçc2.c in function __mulsc3
Re: [PATCH] have -Wformat-overflow handle -fexec-charset (PR 80503)
Hi Martin, On 01/02/18 00:40, Martin Sebor wrote: On 01/31/2018 10:36 AM, Renlin Li wrote: Hi there, I have a patch to fix to regressions we observed in armhf native environment. To effectively check out of range for format string, a target type should be used. And according to the standard, int type is used for "width" and "precision" field of format string. Here target_strtol10 is rename to target_strtoi10, and fixed to use target_int_max () which is target dependent. The value returned by target_strtol10 is (target_int_max () + 1) when it exceeds the range. This is used to indicate its value exceeds target INT_MAX for the later warning. Sorry for not responding to your original email. It's still in my inbox, just buried under a pile of stuff. Using LONG_MAX is not ideal but unless I missed something (it's been a while) I think using INT_MAX for the target would be even less optimal. Unless specified by the asterisk, width and precision can be arbitrarily large. I cannot find more document about this other than here: http://en.cppreference.com/w/c/io/fprintf (optional) integer value or * that specifies minimum field width. (optional) . followed by integer number or *, or neither that specifies precision of the conversion. It only mentions a integer value, with no type. I assume it could be of type int? It is indeed very unclear about the range of width and precision. constraining either to INT_MAX would trigger warnings on LP64 targets for safe calls like: // INT_MAX is 2147483647 sprintf (d, "%.2147483648s", ""); I think we want to use the maximum value of an unsigned type with the greatest precision on the host. It will still warn for directives with precisions in excess of HOST_WIDE_INT_MAX Is it here a target type should be used to test the range against? Instead of the host where the toolchain is built? Regards, Renlin but short of using something like offset_int it's probably as good as it's going to get. (It has been suggested that the whole pass would benefit from using offset_int to track large numbers so if/when it's enhanced to make this change it should also make the code behave more consistently across different hosts.) Martin Is it Okay to commit? Regards, Renlin gcc/ChangeLog: 2018-01-31 Renlin Li * gimple-ssa-sprintf.c (target_strtol10): Use target integer to check the range. Rename it into (target_strtoi10): This. (parse_directive): Compare with target int max instead of LONG_MAX. On 20/06/17 12:00, Renlin Li wrote: Hi Martin, I did a little investigation into this. Please correct me if I missed anything. I build a native arm-linux-gnueabihf toolchain in armhf hardware. It's ILP32. So in this situation: HOST_WIDE_INT is long, which is 32-bit. integer type 32-bit as well, so target_int_max () == LONG_MAX gimple-ssa-sprintf.c line 2887 /* Has the likely and maximum directive output exceeded INT_MAX? */ bool likelyximax = *dir.beg && res->range.likely > target_int_max (); likelyximax will be false as the latter expression is always false. res->range.likely is truncated to LONG_MAX (in target_strtol10 function) I have checked in cross build environment (host x86_64), this variable is true. Regards, Renlin On 13/06/17 09:16, Renlin Li wrote: Hi Martin, On 04/06/17 23:24, Martin Sebor wrote: On 06/02/2017 09:38 AM, Renlin Li wrote: Hi Martin, After r247444, I saw the following two regressions in arm-linux-gnueabihf environment: FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 119) PASS: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 121) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 121) The warning message related to those two lines are: testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:119:3: warning: '%9223372036854775808i' directive width out of range [-Wformat-overflow=] testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning: '%.9223372036854775808i' directive precision out of range [-Wformat-overflow=] testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning: '%.9223372036854775808i' directive precision out of range [-Wformat-overflow=] Did you notice similar things from your test environment, Christophe? Looks like you're missing a couple of warnings. I see the following output with both my arm-linux-gnueabihf cross compiler and my native x86_64 GCC, both in 32-bit and 64-bit modes, as expected by the test, so I don't see the same issue in my environment. Yes, it happens on arm-linux-gnueabihf native environment. the warnings with "INT_MAX" line are missing. I don't know if the host environment will cause the difference. Regards, Renlin
Re: [PATCH BACKPORT]Backport r254778 and test case in r244815 to GCC6
On Wed, Jan 31, 2018 at 10:55 AM, Richard Biener wrote: > On Tue, Dec 19, 2017 at 4:36 PM, Bin Cheng wrote: >> HI, >> This patch backports r254778 and test case in r244815 to GCC6. Bootstrap and >> test on x86_64. Is it OK? > > Ok. Retested and applied on GCC6 branch. Thanks, bin > > Richard. > >> Thanks, >> bin >> >> 2017-12-18 Bin Cheng >> >> Backport from mainline >> 2017-11-15 Bin Cheng >> >> PR tree-optimization/82726 >> PR tree-optimization/70754 >> * tree-predcom.c (order_drefs_by_pos): New function. >> (combine_chains): Move code setting has_max_use_after to... >> (try_combine_chains): ...here. New parameter. Sort combined chains >> according to position information. >> (tree_predictive_commoning_loop): Update call to above function. >> (update_pos_for_combined_chains, pcom_stmt_dominates_stmt_p): New. >> >> gcc/testsuite >> 2017-12-18 Bin Cheng >> >> Backport from mainline >> 2017-11-15 Bin Cheng >> >> PR tree-optimization/82726 >> * gcc.dg/tree-ssa/pr82726.c: New test. >> >> Backport from mainline >> 2017-01-23 Bin Cheng >> >> PR tree-optimization/70754 >> * gfortran.dg/pr70754.f90: New test.
Re: [PATCH v2][AArch64] Remove remaining uses of * in patterns
James Greenhalgh wrote: > Please queue for GCC 9. OK when trunk is back open for new code. This fixes the regressions introduced by the SVE merge conflicts and the failures of aarch64/pr62178.c, both of which are new regressions, so we should fix these now. Wilco
Re: [RFC][PR82479] missing popcount builtin detection
On Thu, Feb 1, 2018 at 5:07 AM, Kugan Vivekanandarajah wrote: > Hi Richard, > > On 31 January 2018 at 21:39, Richard Biener > wrote: >> On Wed, Jan 31, 2018 at 11:28 AM, Kugan Vivekanandarajah >> wrote: >>> Hi Richard, >>> >>> Thanks for the review. >>> On 25 January 2018 at 20:04, Richard Biener >>> wrote: On Wed, Jan 24, 2018 at 10:56 PM, Kugan Vivekanandarajah wrote: > Hi All, > > Here is a patch for popcount builtin detection similar to LLVM. I > would like to queue this for review for next stage 1. > > 1. This is done part of loop-distribution and effective for -O3 and above. > 2. This does not distribute loop to detect popcount (like > memcpy/memmove). I dont think that happens in practice. Please correct > me if I am wrong. But then it has no business inside loop distribution but instead is doing final value replacement, right? You are pattern-matching the whole loop after all. I think final value replacement would already do the correct thing if you teached number of iteration analysis that niter for [local count: 955630224]: # b_11 = PHI _1 = b_11 + -1; b_8 = _1 & b_11; if (b_8 != 0) goto ; [89.00%] else goto ; [11.00%] [local count: 850510900]: goto ; [100.00%] >>> >>> I am looking into this approach. What should be the scalar evolution >>> for b_8 (i.e. b & (b -1) in a loop) should be? This is not clear to me >>> and can this be represented with the scev? >> >> No, it's not affine and thus cannot be represented. You only need the >> scalar evolution of the counting IV which is already handled and >> the number of iteration analysis needs to handle the above IV - this >> is the missing part. > Thanks for the clarification. I am now matching this loop pattern in > number_of_iterations_exit when number_of_iterations_exit_assumptions > fails. If the pattern matches, I am inserting the _builtin_popcount in > the loop preheater and setting the loop niter with this. This will be > used by the final value replacement. Is this what you wanted? No, you shouldn't insert a popcount stmt but instead the niter GENERIC tree should be a CALL_EXPR to popcount with the appropriate argument. > In general, there is also a condition gating the loop like > > if (b_4 != 0) > goto loop; > else > end: > > This of course will not be removed by final value replacement. Since > popcount (0) is defined, this is redundant and could be removed but > not removed. Yeah, that's probably sth for another pass though. I suppose the end: case just uses zero in which case you'll have a PHI and you can optimize if (b != 0) return popcount (b); return 0; in phiopt. Richard. > Thanks, > Kugan > >> >> Richard. >> >>> Thanks, >>> Kugan is related to popcount (b_5). Richard. > Bootstrapped and regression tested on aarch64-linux-gnu with no new > regressions. > > Thanks, > Kugan > > gcc/ChangeLog: > > 2018-01-25 Kugan Vivekanandarajah > > PR middle-end/82479 > * tree-loop-distribution.c (handle_popcount): New. > (pass_loop_distribution::execute): Use handle_popcount. > > gcc/testsuite/ChangeLog: > > 2018-01-25 Kugan Vivekanandarajah > > PR middle-end/82479 > * gcc.dg/tree-ssa/popcount.c: New test.
Re: C++ PATCH to kill cxx_print_statistics
On 02/01/2018 06:51 AM, Marek Polacek wrote: Does any of you use print_class_statistics? It seems most of these variables were set-but-unused even in gcc-2.95 (!). So I think we can safely 86 all of this. Or is the "n_vtables" info interesting in any way? Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-02-01 Marek Polacek * class.c: Remove unused global variables. (build_primary_vtable): Don't gather statistics. (print_class_statistics): Remove. * cp-tree.h (print_class_statistics): Remove. * tree.c (cxx_print_statistics): Don't call print_class_statistics. I find no use for it. nathan -- Nathan Sidwell
[PATCH PR83789] Fix for Altivec builtin failure
Hi, Please find below a patch to fix PR83789. The altivec builtin '__builtin_altivec_lvx' fails for the 32-bit powerpc-linux target. This is observed in gcc-7 onwards and does not affect powerpc64-linux target. This is regression tested for powerpc-linux. Please let me know if this patch is OK. Regards, Kaushik M Phatak 2018-02-01 Kaushik Phatak PR target/83789 * config/rs6000/rs6000.c (altivec_expand_builtin): Provide support for 32-bit target for altivec builtin. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 256400) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -15799,8 +15799,10 @@ exp, target, false); case ALTIVEC_BUILTIN_LVX: case ALTIVEC_BUILTIN_LVX_V4SI: - return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v4si_2op, - exp, target, false); + return altivec_expand_lv_builtin (((TARGET_64BIT) +? CODE_FOR_altivec_lvx_v4si_2op +: CODE_FOR_altivec_lvx_v4si_internal), + exp, target, false); case ALTIVEC_BUILTIN_LVX_V8HI: return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v8hi_2op, exp, target, false);
Re: [PATCH v2][AArch64] Remove remaining uses of * in patterns
On Thu, Feb 01, 2018 at 12:09:09PM +, Wilco Dijkstra wrote: > James Greenhalgh wrote: > > > Please queue for GCC 9. OK when trunk is back open for new code. > > This fixes the regressions introduced by the SVE merge conflicts and > the failures of aarch64/pr62178.c, both of which are new regressions, > so we should fix these now. I don't see these mentioned in either the V1 or V2 of this patch submission. Do we have bug reports or anything you can point to for the regressions you are referring to? Is this a performance regression, or a correctness issue? gcc.target/aarch64/pr62178.c passes on trunk. If we don't have anything we can point to; please queue this for GCC 9. We need to stabilise for the GCC 8 release now. Thanks, James
Re: Use range info in split_constant_offset (PR 81635)
On Wed, Jan 31, 2018 at 4:06 PM, Richard Sandiford
wrote:
> This patch implements the original suggestion for fixing PR 81635:
> use range info in split_constant_offset to see whether a conversion
> of a wrapping type can be split. The range info problem described in:
>
> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01002.html
>
> seems to have been fixed.
>
> The patch is part 1. There needs to be a follow-on patch to handle:
>
> for (unsigned int i = 0; i < n; i += 4)
> {
> ...[i + 2]...
> ...[i + 3]...
>
> which the old SCEV test handles, but which the range check doesn't.
> At the moment we record that the low two bits of "i" are clear,
> but we still end up with a maximum range of 0x rather than
> 0xfffc.
>
> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
> Also tested by comparing the before and after testsuite assembly output
> for at least one target per CPU directory. Excluding a small number
> of register renamings on some targets, there were two differences:
>
> (1) In gcc.c-torture/compile/pr55350.c:
>
> void
> foo (__INTPTR_TYPE__ x, __INTPTR_TYPE__ y)
> {
> int i;
> void **a = (void *) (8UL * (x / 8UL));
> for (i = 0; i < x; i++)
> a[i] = (void *) y;
> }
>
> we previously kept base "a" and offset 0, but now use
> "(void **) _n" (where _n holds the multiplication result).
>
> This is because the old test had the side-effect of prohibiting
> casts from unsigned ints to pointers of the same size.
> What we do for code like this isn't going to help much though.
>
> (2) In gcc.c-torture/execute/2003016-1.c, we unrolled:
>
> void f (unsigned int *x)
> {
> unsigned char i;
> int j;
>
> i = 0x10;
> for (j = 0; j < 0x10; j++)
> {
> i += 0xe8;
> x[i] = 0;
> i -= 0xe7;
> }
> }
>
> and ended up with an unpropagated degenerate phi:
>
> # i_38 = PHI <16(3)>
> ...
> i_40 = i_38 + 1;
> ...
> i_48 = i_40 + 1;
> ... etc ...
> i_178 = i_168 + 1;
> i_16 = i_178 + 232;
> _17 = (long unsigned int) i_16;
> _18 = _17 * 4;
> _19 = &x + _18;
> *_19 = 0;
>
> Calling split_constant_offset on each (long unsigned int) operand
> gives i_38 + 0xe8, i_38 + 0xe9, ..., i_38 + 0xf7, with i_38
> still having the range [0x10, 0x20]. We can therefore tell
> that i_38 + 0xf0 has the range [0x00, 0x10], and similarly
> for +0xf1...+0xf7.
>
> We should really be folding to constants here though.
>
> OK to install?
>
>
> 2018-01-31 Richard Sandiford
>
> gcc/
> PR tree-optimization/81635
> * tree-data-ref.c (split_constant_offset_1): For types that
> wrap on overflow, try to use range info to prove that wrapping
> cannot occur.
>
> gcc/testsuite/
> PR tree-optimization/81635
> * gcc.dg/vect/bb-slp-pr81635-1.c: New test.
> * gcc.dg/vect/bb-slp-pr81635-2.c: Likewise.
>
> Index: gcc/tree-data-ref.c
> ===
> --- gcc/tree-data-ref.c 2018-01-13 18:02:00.946360352 +
> +++ gcc/tree-data-ref.c 2018-01-31 13:26:13.488630604 +
> @@ -704,11 +704,46 @@ split_constant_offset_1 (tree type, tree
>and the outer precision is at least as large as the inner. */
> tree itype = TREE_TYPE (op0);
> if ((POINTER_TYPE_P (itype)
> -|| (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_UNDEFINED (itype)))
> +|| (INTEGRAL_TYPE_P (itype) && !TYPE_OVERFLOW_TRAPS (itype)))
> && TYPE_PRECISION (type) >= TYPE_PRECISION (itype)
> && (POINTER_TYPE_P (type) || INTEGRAL_TYPE_P (type)))
> {
> - split_constant_offset (op0, &var0, off);
> + if (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_WRAPS (itype))
> + {
> + /* Split the unconverted operand and try to prove that
> + wrapping isn't a problem. */
> + tree tmp_var, tmp_off;
> + split_constant_offset (op0, &tmp_var, &tmp_off);
> +
> + /* See whether we have an SSA_NAME whose range is known
> + to be [A, B]. */
> + if (TREE_CODE (tmp_var) != SSA_NAME)
> + return false;
> + wide_int var_min, var_max;
> + if (get_range_info (tmp_var, &var_min, &var_max) != VR_RANGE)
> + return false;
> +
> + /* See whether the range of OP0 (i.e. TMP_VAR + TMP_OFF)
> + is known to be [A + TMP_OFF, B + TMP_OFF], with all
> + operations done in ITYPE. The addition must overflow
> + at both ends of the range or at neither. */
Hmm, if they overflow at both ends that's still an issue given we effectively
widen the addition? Consider char -> short and [255U, 255U] + 1 which
when widened
Re: Use range info in split_constant_offset (PR 81635)
Richard Biener writes:
> On Wed, Jan 31, 2018 at 4:06 PM, Richard Sandiford
> wrote:
>> This patch implements the original suggestion for fixing PR 81635:
>> use range info in split_constant_offset to see whether a conversion
>> of a wrapping type can be split. The range info problem described in:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01002.html
>>
>> seems to have been fixed.
>>
>> The patch is part 1. There needs to be a follow-on patch to handle:
>>
>> for (unsigned int i = 0; i < n; i += 4)
>> {
>> ...[i + 2]...
>> ...[i + 3]...
>>
>> which the old SCEV test handles, but which the range check doesn't.
>> At the moment we record that the low two bits of "i" are clear,
>> but we still end up with a maximum range of 0x rather than
>> 0xfffc.
>>
>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
>> Also tested by comparing the before and after testsuite assembly output
>> for at least one target per CPU directory. Excluding a small number
>> of register renamings on some targets, there were two differences:
>>
>> (1) In gcc.c-torture/compile/pr55350.c:
>>
>> void
>> foo (__INTPTR_TYPE__ x, __INTPTR_TYPE__ y)
>> {
>> int i;
>> void **a = (void *) (8UL * (x / 8UL));
>> for (i = 0; i < x; i++)
>> a[i] = (void *) y;
>> }
>>
>> we previously kept base "a" and offset 0, but now use
>> "(void **) _n" (where _n holds the multiplication result).
>>
>> This is because the old test had the side-effect of prohibiting
>> casts from unsigned ints to pointers of the same size.
>> What we do for code like this isn't going to help much though.
>>
>> (2) In gcc.c-torture/execute/2003016-1.c, we unrolled:
>>
>> void f (unsigned int *x)
>> {
>> unsigned char i;
>> int j;
>>
>> i = 0x10;
>> for (j = 0; j < 0x10; j++)
>> {
>> i += 0xe8;
>> x[i] = 0;
>> i -= 0xe7;
>> }
>> }
>>
>> and ended up with an unpropagated degenerate phi:
>>
>> # i_38 = PHI <16(3)>
>> ...
>> i_40 = i_38 + 1;
>> ...
>> i_48 = i_40 + 1;
>> ... etc ...
>> i_178 = i_168 + 1;
>> i_16 = i_178 + 232;
>> _17 = (long unsigned int) i_16;
>> _18 = _17 * 4;
>> _19 = &x + _18;
>> *_19 = 0;
>>
>> Calling split_constant_offset on each (long unsigned int) operand
>> gives i_38 + 0xe8, i_38 + 0xe9, ..., i_38 + 0xf7, with i_38
>> still having the range [0x10, 0x20]. We can therefore tell
>> that i_38 + 0xf0 has the range [0x00, 0x10], and similarly
>> for +0xf1...+0xf7.
>>
>> We should really be folding to constants here though.
>>
>> OK to install?
>>
>>
>> 2018-01-31 Richard Sandiford
>>
>> gcc/
>> PR tree-optimization/81635
>> * tree-data-ref.c (split_constant_offset_1): For types that
>> wrap on overflow, try to use range info to prove that wrapping
>> cannot occur.
>>
>> gcc/testsuite/
>> PR tree-optimization/81635
>> * gcc.dg/vect/bb-slp-pr81635-1.c: New test.
>> * gcc.dg/vect/bb-slp-pr81635-2.c: Likewise.
>>
>> Index: gcc/tree-data-ref.c
>> ===
>> --- gcc/tree-data-ref.c 2018-01-13 18:02:00.946360352 +
>> +++ gcc/tree-data-ref.c 2018-01-31 13:26:13.488630604 +
>> @@ -704,11 +704,46 @@ split_constant_offset_1 (tree type, tree
>>and the outer precision is at least as large as the inner. */
>> tree itype = TREE_TYPE (op0);
>> if ((POINTER_TYPE_P (itype)
>> -|| (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_UNDEFINED (itype)))
>> +|| (INTEGRAL_TYPE_P (itype) && !TYPE_OVERFLOW_TRAPS (itype)))
>> && TYPE_PRECISION (type) >= TYPE_PRECISION (itype)
>> && (POINTER_TYPE_P (type) || INTEGRAL_TYPE_P (type)))
>> {
>> - split_constant_offset (op0, &var0, off);
>> + if (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_WRAPS (itype))
>> + {
>> + /* Split the unconverted operand and try to prove that
>> + wrapping isn't a problem. */
>> + tree tmp_var, tmp_off;
>> + split_constant_offset (op0, &tmp_var, &tmp_off);
>> +
>> + /* See whether we have an SSA_NAME whose range is known
>> + to be [A, B]. */
>> + if (TREE_CODE (tmp_var) != SSA_NAME)
>> + return false;
>> + wide_int var_min, var_max;
>> + if (get_range_info (tmp_var, &var_min, &var_max) != VR_RANGE)
>> + return false;
>> +
>> + /* See whether the range of OP0 (i.e. TMP_VAR + TMP_OFF)
>> + is known to be [A + TMP_OFF, B + TMP_OFF], with all
>> + operations done in ITYPE. The addition must overflow
>> + at both ends of the range or at neither. */
>
>
Re: Use range info in split_constant_offset (PR 81635)
On Thu, Feb 1, 2018 at 2:18 PM, Richard Sandiford
wrote:
> Richard Biener writes:
>> On Wed, Jan 31, 2018 at 4:06 PM, Richard Sandiford
>> wrote:
>>> This patch implements the original suggestion for fixing PR 81635:
>>> use range info in split_constant_offset to see whether a conversion
>>> of a wrapping type can be split. The range info problem described in:
>>>
>>> https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01002.html
>>>
>>> seems to have been fixed.
>>>
>>> The patch is part 1. There needs to be a follow-on patch to handle:
>>>
>>> for (unsigned int i = 0; i < n; i += 4)
>>> {
>>> ...[i + 2]...
>>> ...[i + 3]...
>>>
>>> which the old SCEV test handles, but which the range check doesn't.
>>> At the moment we record that the low two bits of "i" are clear,
>>> but we still end up with a maximum range of 0x rather than
>>> 0xfffc.
>>>
>>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.
>>> Also tested by comparing the before and after testsuite assembly output
>>> for at least one target per CPU directory. Excluding a small number
>>> of register renamings on some targets, there were two differences:
>>>
>>> (1) In gcc.c-torture/compile/pr55350.c:
>>>
>>> void
>>> foo (__INTPTR_TYPE__ x, __INTPTR_TYPE__ y)
>>> {
>>> int i;
>>> void **a = (void *) (8UL * (x / 8UL));
>>> for (i = 0; i < x; i++)
>>> a[i] = (void *) y;
>>> }
>>>
>>> we previously kept base "a" and offset 0, but now use
>>> "(void **) _n" (where _n holds the multiplication result).
>>>
>>> This is because the old test had the side-effect of prohibiting
>>> casts from unsigned ints to pointers of the same size.
>>> What we do for code like this isn't going to help much though.
>>>
>>> (2) In gcc.c-torture/execute/2003016-1.c, we unrolled:
>>>
>>> void f (unsigned int *x)
>>> {
>>> unsigned char i;
>>> int j;
>>>
>>> i = 0x10;
>>> for (j = 0; j < 0x10; j++)
>>> {
>>> i += 0xe8;
>>> x[i] = 0;
>>> i -= 0xe7;
>>> }
>>> }
>>>
>>> and ended up with an unpropagated degenerate phi:
>>>
>>> # i_38 = PHI <16(3)>
>>> ...
>>> i_40 = i_38 + 1;
>>> ...
>>> i_48 = i_40 + 1;
>>> ... etc ...
>>> i_178 = i_168 + 1;
>>> i_16 = i_178 + 232;
>>> _17 = (long unsigned int) i_16;
>>> _18 = _17 * 4;
>>> _19 = &x + _18;
>>> *_19 = 0;
>>>
>>> Calling split_constant_offset on each (long unsigned int) operand
>>> gives i_38 + 0xe8, i_38 + 0xe9, ..., i_38 + 0xf7, with i_38
>>> still having the range [0x10, 0x20]. We can therefore tell
>>> that i_38 + 0xf0 has the range [0x00, 0x10], and similarly
>>> for +0xf1...+0xf7.
>>>
>>> We should really be folding to constants here though.
>>>
>>> OK to install?
>>>
>>>
>>> 2018-01-31 Richard Sandiford
>>>
>>> gcc/
>>> PR tree-optimization/81635
>>> * tree-data-ref.c (split_constant_offset_1): For types that
>>> wrap on overflow, try to use range info to prove that wrapping
>>> cannot occur.
>>>
>>> gcc/testsuite/
>>> PR tree-optimization/81635
>>> * gcc.dg/vect/bb-slp-pr81635-1.c: New test.
>>> * gcc.dg/vect/bb-slp-pr81635-2.c: Likewise.
>>>
>>> Index: gcc/tree-data-ref.c
>>> ===
>>> --- gcc/tree-data-ref.c 2018-01-13 18:02:00.946360352 +
>>> +++ gcc/tree-data-ref.c 2018-01-31 13:26:13.488630604 +
>>> @@ -704,11 +704,46 @@ split_constant_offset_1 (tree type, tree
>>>and the outer precision is at least as large as the inner. */
>>> tree itype = TREE_TYPE (op0);
>>> if ((POINTER_TYPE_P (itype)
>>> -|| (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_UNDEFINED
>>> (itype)))
>>> +|| (INTEGRAL_TYPE_P (itype) && !TYPE_OVERFLOW_TRAPS (itype)))
>>> && TYPE_PRECISION (type) >= TYPE_PRECISION (itype)
>>> && (POINTER_TYPE_P (type) || INTEGRAL_TYPE_P (type)))
>>> {
>>> - split_constant_offset (op0, &var0, off);
>>> + if (INTEGRAL_TYPE_P (itype) && TYPE_OVERFLOW_WRAPS (itype))
>>> + {
>>> + /* Split the unconverted operand and try to prove that
>>> + wrapping isn't a problem. */
>>> + tree tmp_var, tmp_off;
>>> + split_constant_offset (op0, &tmp_var, &tmp_off);
>>> +
>>> + /* See whether we have an SSA_NAME whose range is known
>>> + to be [A, B]. */
>>> + if (TREE_CODE (tmp_var) != SSA_NAME)
>>> + return false;
>>> + wide_int var_min, var_max;
>>> + if (get_range_info (tmp_var, &var_min, &var_max) !=
>>> VR_RANGE)
>>> + return false;
>>> +
>>> + /* See whether the range of OP0 (i.e. TMP_VAR + TMP_OFF)
>>> +
Re: build failure x86_64
On Thu, Feb 01, 2018 at 11:52:42AM +, graham stott wrote: > A recent patch has broken RTL checking builds on x86_64 fails during combine > RTL check: expected code 'reg' have 'lshirtrt' in rhs_regno at rtl.h:1891 > builfing libgçc2.c in function __mulsc3 This is PR84157. It has a patch. Segher
wwwdocs: Some release notes for powerpc for GCC 8
Hi! This is a first set of release notes for Power for GCC 8. Is this okay to commit? Segher Index: htdocs/gcc-8/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-8/changes.html,v retrieving revision 1.31 diff -u -3 -p -r1.31 changes.html --- htdocs/gcc-8/changes.html 23 Jan 2018 14:02:45 - 1.31 +++ htdocs/gcc-8/changes.html 1 Feb 2018 13:28:08 - @@ -374,6 +374,8 @@ a work-in-progress. + + PA-RISC @@ -387,13 +389,26 @@ a work-in-progress. - - PowerPC / PowerPC64 / RS6000 - + +The PowerPC SPE support is split off to a separate powerpcspe +port. See the separate entry for that new port. + + +The Paired Single support (as used on some PPC750 CPUs, +-mpaired, powerpc*-*-linux*paired*) +is deprecated and will be removed in a future release. + + +The Xilinx floating point support (-mxilinx-fpu, +powerpc-xilinx-eabi*) +is deprecated and will be removed in a future release. + +PowerPC SPE +
Re: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf
On Wed, Jan 31, 2018 at 11:42 PM, Uros Bizjak wrote:
> On Thu, Feb 1, 2018 at 1:16 AM, H.J. Lu wrote:
>>
>> We currently read and write beyond the builtin jmpbuf on ILP32 targets
>> where Pmode == DImode and ptr_mode == SImode. Since the builtin jmpbuf
>> is an array of 5 pointers, ptr_mode should be used to save and restore
>> frame and program pointers. Since x86 only saves stack pointer in
>> stack save area, STACK_SAVEAREA_MODE should be ptr_mode, not Pmode.
>>
>> Note: If ptr_mode != Pmode, builtin setjmp/longjmp tests may fail. When
>> it happens, please check if there are correct save_stack_nonlocal and
>> restore_stack_nonlocal expand patterns.
>>
>> Tested on i686 and x86-64. OK for trunk?
>>
>> H.J.
>>
>> gcc/
>>
>> PR middle-end/84150
>> * builtins.c (expand_builtin_setjmp_setup): Use ptr_mode to
>> save frame and program pointers.
>> (expand_builtin_longjmp): Use ptr_mode to restore frame and
>> program pointers.
>> * config/i386/i386.md (save_stack_nonlocal): New expand pattern.
>> (restore_stack_nonlocal): Likewise.
>> * config/i386/i386.h (STACK_SAVEAREA_MODE): New.
>> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>> index c08e4f55cff..c563a26cd60 100644
>> --- a/gcc/config/i386/i386.md
>> +++ b/gcc/config/i386/i386.md
>> @@ -18375,6 +18375,42 @@
>>"* return output_probe_stack_range (operands[0], operands[2]);"
>>[(set_attr "type" "multi")])
>>
>> +;; Non-local goto support.
>> +
>> +(define_expand "save_stack_nonlocal"
>> + [(use (match_operand 0 "memory_operand" ""))
>> + (use (match_operand 1 "register_operand" ""))]
>> + ""
>> +{
>> + /* Stack is saved in ptr_mode and stack register is in Pmode. */
>> + if (GET_MODE (operands[0]) != GET_MODE (operands[1]))
>> +{
>> + if (GET_MODE (operands[0]) != ptr_mode
>> + || GET_MODE (operands[1]) != Pmode)
>> + gcc_unreachable ();
>
> gcc_assert instead of if (...) gcc_unreachable.
>
>> + operands[1] = gen_rtx_SUBREG (ptr_mode, operands[1], 0);
>
> gen_lowpart
>
>> +}
>> + emit_move_insn (operands[0], operands[1]);
>> + DONE;
>> +})
>> +
>> +(define_expand "restore_stack_nonlocal"
>> + [(use (match_operand 0 "register_operand" ""))
>> + (use (match_operand 1 "memory_operand" ""))]
>> + ""
>> +{
>> + /* Stack is saved in ptr_mode and stack register is in Pmode. */
>> + if (GET_MODE (operands[0]) != GET_MODE (operands[1]))
>> +{
>> + if (GET_MODE (operands[0]) != Pmode
>> + || GET_MODE (operands[1]) != ptr_mode)
>> + gcc_unreachable ();
>
> Also here.
>
Here is the updated patch. OK for trunk?
BTW, the -maddress-mode=long test in Igor's patch for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84066
is expected to fail without this fix.
--
H.J.
From f8c86b9fc727dbd7e4de0691bd81d2d2bea8904b Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Wed, 31 Jan 2018 09:47:57 -0800
Subject: [PATCH] Use ptr_mode to save/restore pointers in builtin jmpbuf
We currently read and write beyond the builtin jmpbuf on ILP32 targets
where Pmode == DImode and ptr_mode == SImode. Since the builtin jmpbuf
is an array of 5 pointers, ptr_mode should be used to save and restore
frame and program pointers. Since x86 only saves stack pointer in
stack save area, STACK_SAVEAREA_MODE should be ptr_mode, not Pmode.
Note: If ptr_mode != Pmode, builtin setjmp/longjmp tests may fail. When
it happens, please check if there are correct save_stack_nonlocal and
restore_stack_nonlocal expand patterns.
gcc/
PR middle-end/84150
* builtins.c (expand_builtin_setjmp_setup): Use ptr_mode to
save frame and program pointers.
(expand_builtin_longjmp): Use ptr_mode to restore frame and
program pointers.
* config/i386/i386.md (save_stack_nonlocal): New expand pattern.
(restore_stack_nonlocal): Likewise.
* config/i386/i386.h (STACK_SAVEAREA_MODE): New.
gcc/testsuite/
PR middle-end/84150
* gcc.dg/pr84150.c: New test.
* gcc.target/i386/pr84150.c: Likewisde.
---
gcc/builtins.c | 36 +-
gcc/config/i386/i386.h | 4 +++
gcc/config/i386/i386.md | 34 +
gcc/testsuite/gcc.dg/pr84150.c | 45 +
gcc/testsuite/gcc.target/i386/pr84150.c | 44
5 files changed, 151 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr84150.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr84150.c
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 683c6ec6e5b..c3d45d5e3fa 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -840,6 +840,7 @@ expand_builtin_setjmp_setup (rtx buf_addr, rtx receiver_label)
machine_mode sa_mode = STACK_SAVEAREA_MODE (SAVE_NONLOCAL);
rtx stack_save;
rtx mem;
+ rtx tmp;
if (setjmp_alias_set == -1)
setjmp_alias_set = new_alias_set ();
@@ -852,20 +853,25 @@ expand_builtin_setjmp_setup (rtx buf_addr,
Re: [patch][x86] -march=icelake
On Thu, Feb 01, 2018 at 08:49:09AM +0100, Uros Bizjak wrote: > > gcc/c-family/ > > * c-common.h (omp_clause_mask): Move to wide_int_bitmask.h. > > > > gcc/ > > * config/i386/i386.c (ix86_option_override_internal): Change flags > > type to > > wide_int_bitmask. > > * wide-int-bitmask.h: New. > > > > Icelake patch changelog: > > > > gcc/ > > * config.gcc: Add -march=icelake. > > * config/i386/driver-i386.c (host_detect_local_cpu): Detect icelake. > > * config/i386/i386-c.c (ix86_target_macros_internal): Handle > > icelake. > > * config/i386/i386.c (processor_costs): Add m_ICELAKE. > > (PTA_ICELAKE, PTA_AVX512VNNI, PTA_GFNI, PTA_VAES, PTA_AVX512VBMI2, > > PTA_VPCLMULQDQ, PTA_RDPID, PTA_AVX512BITALG): New. > > (processor_target_table): Add icelake. > > (ix86_option_override_internal): Handle new PTAs. > > (get_builtin_code_for_version): Handle icelake. > > (M_INTEL_COREI7_ICELAKE): New. > > (fold_builtin_cpu): Handle icelake. > > * config/i386/i386.h (TARGET_ICELAKE, PROCESSOR_ICELAKE): New. > > * doc/invoke.texi: Add -march=icelake. > > gcc/testsuite/ > > * gcc.target/i386/funcspec-56.inc: Handle new march. > > * g++.dg/ext/mv16.C: Ditto. > > libgcc/ > > * config/i386/cpuinfo.h (processor_subtypes): Add > > INTEL_COREI7_ICELAKE. > > x86 parts are OK, generic parts need approval from global maintainer. The generic parts are ok as well. Jakub
Re: C++ PATCH to kill cxx_print_statistics
On Thu, Feb 1, 2018 at 7:25 AM, Nathan Sidwell wrote: > On 02/01/2018 06:51 AM, Marek Polacek wrote: >> >> Does any of you use print_class_statistics? >> >> It seems most of these variables were set-but-unused even in gcc-2.95 (!). >> So >> I think we can safely 86 all of this. Or is the "n_vtables" info interesting >> in any way? >> >> Bootstrapped/regtested on x86_64-linux, ok for trunk? >> >> 2018-02-01 Marek Polacek >> >> * class.c: Remove unused global variables. >> (build_primary_vtable): Don't gather statistics. >> (print_class_statistics): Remove. >> * cp-tree.h (print_class_statistics): Remove. >> * tree.c (cxx_print_statistics): Don't call >> print_class_statistics. > > I find no use for it. I've never used it. Jason
Re: [PATCH PR83789] Fix for Altivec builtin failure
Hi! On Thu, Feb 01, 2018 at 12:26:41PM +, Kaushik Phatak wrote: > --- gcc/config/rs6000/rs6000.c(revision 256400) > +++ gcc/config/rs6000/rs6000.c(working copy) > @@ -15799,8 +15799,10 @@ > exp, target, false); > case ALTIVEC_BUILTIN_LVX: > case ALTIVEC_BUILTIN_LVX_V4SI: > - return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v4si_2op, > - exp, target, false); > + return altivec_expand_lv_builtin (((TARGET_64BIT) > + ? CODE_FOR_altivec_lvx_v4si_2op > + : CODE_FOR_altivec_lvx_v4si_internal), > +exp, target, false); > case ALTIVEC_BUILTIN_LVX_V8HI: >return altivec_expand_lv_builtin (CODE_FOR_altivec_lvx_v8hi_2op, > exp, target, false); I think you should use altivec_lvx_v4si_2op_si instead? And the same needs to be done for v8hi, v16qi, v4sf, maybe more? Segher
Re: [PR83370][AARCH64]Use tighter register constraints for sibcall patterns.
Hi James,
Thanks for the review! I committed it on trunk.
Is it Okay to backport this patch to release branch 5, 6,7?
It applies cleanly without any logic changes.
Regards,
Renlin
On 31/01/18 17:56, James Greenhalgh wrote:
On Tue, Jan 30, 2018 at 03:45:17PM +, Renlin Li wrote:
Hi Richard,
Thanks for the review!
On 29/01/18 20:23, Richard Sandiford wrote:
The patch looks good to me FWIW. How about adding something like
the following testcase?
/* { dg-do run } */
/* { dg-options "-O2" } */
typedef void (*fun) (void);
void __attribute__ ((noipa))
f (fun x1)
{
register fun x2 asm ("x16");
int arr[5000];
int *volatile ptr = arr;
asm ("mov %0, %1" : "=r" (x2) : "r" (x1));
x2 ();
}
void g (void) {}
int
main (void)
{
f (g);
}
It was hard for me to construct an test case at that time.
Your example here exactly reflect the problem. The code-gen before the change
is:
f:
mov x16, 20016
sub sp, sp, x16
add x0, sp, 16
mov x16, 20016
str x0, [sp, 8]
add sp, sp, x16
br x16
After the change to the register constraint:
f:
mov x16, 20016
sub sp, sp, x16
// Start of user assembly
// 9 "indirect_tail_call_reg.c" 1
mov x16, x0
// 0 "" 2
// End of user assembly
add x0, sp, 16
str x0, [sp, 8]
mov x0, x16
mov x16, 20016
add sp, sp, x16
br x0
I updated the patch with the new test case,
the wording about the register constraint is also updated.
This patch is OK.
Thanks,
James
gcc/ChangeLog:
2018-01-30 Renlin Li
* config/aarch64/aarch64.c (aarch64_class_max_nregs): Handle
TAILCALL_ADDR_REGS.
(aarch64_register_move_cost): Likewise.
* config/aarch64/aarch64.h (reg_class): Rename CALLER_SAVE_REGS to
TAILCALL_ADDR_REGS.
(REG_CLASS_NAMES): Likewise.
(REG_CLASS_CONTENTS): Rename CALLER_SAVE_REGS to
TAILCALL_ADDR_REGS. Remove IP registers.
* config/aarch64/aarch64.md (Ucs): Update register constraint.
gcc/testsuite/ChangeLog:
2018-01-30 Richard Sandiford
* gcc.target/aarch64/indirect_tail_call_reg.c: New.
Re: [PR83370][AARCH64]Use tighter register constraints for sibcall patterns.
On Thu, Feb 01, 2018 at 02:43:17PM +, Renlin Li wrote: > Hi James, > > Thanks for the review! I committed it on trunk. > > Is it Okay to backport this patch to release branch 5, 6,7? > It applies cleanly without any logic changes. 6 and 7 yes, OK. 5 is no longer supported so may be a waste of your time :-) James
Re: [PATCH, rs6000] Fix PR56010 and PR83743, -mcpu=native use wrong names
On Wed, Jan 31, 2018 at 12:16:37PM -0600, Peter Bergner wrote: > On 1/31/18 11:39 AM, Segher Boessenkool wrote: > > On Mon, Jan 29, 2018 at 08:55:35PM -0600, Peter Bergner wrote: > >> > >> Either that, or I could still call candidates_list_and_hint() and just > >> throw the hint away, since it's meaningless. > > > > It's less code, simpler code, so that would be best I think yes. > > Ok, here's the updated patch that uses candidates_list_and hint() > and throws the hint away because we don't want it. I also removed > the handling of pa6t like you wanted. > > Ok for trunk now? Do we also want this on the release branches? Yes, okay for trunk. Thanks! I think we also want this for 7 (after a bit of burn in). I wouldn't bother with 6 though (the problem has existed since 4.7). Segher > PR target/56010 > PR target/83743 > * config/rs6000/driver-rs6000.c: #include "diagnostic.h". > #include "opts.h". > (rs6000_supported_cpu_names): New static variable. > (linux_cpu_translation_table): Likewise. > (elf_platform) : Define new static variable and use it. > Translate kernel AT_PLATFORM name to canonical name if needed. > Error if platform name is unknown.
Re: [C++ Patch/RFC] PR 83796 ("[6/7/8 Regression] Abstract classes allowed to be instantiated when initialised as default parameter to function or constructor")
On Wed, Jan 31, 2018 at 2:55 PM, Paolo Carlini wrote:
> Hi again,
>
> On 24/01/2018 16:58, Paolo Carlini wrote:
>>
>> Hi,
>>
>> I'm looking into this rather mild regression, which should be relatively
>> easy to fix. In short, Jason's fix for c++/54325 moved an
>> abstract_virtuals_error_sfinae check from build_aggr_init_expr to
>> build_cplus_new therefore the testcase in this new bug isn't rejected
>> anymore because a special conditional for value-initialization from { } in
>> convert_like_real simply calls build_value_init and quickly returns, thus
>> build_cplus_new isn't involved. Thus I'm working on the best way to add back
>> the check. The below, which also uses cp_unevaluated_operand, appears to
>> work. Likewise something similar inside build_value_init itself, which
>> however seems too generic to me (build_value_init is called in many other
>> cases). I'm also not sure about cp_unevaluated_operand, whether we need
>> something more precise.
>
> I'm gently "pinging" this message of mine... Definitely not an high priority
> regression (in any case it's only a P3) but I'm still wondering if we want
> to do something about the issue at this time. Lately I noticed that in terms
> of testsuite even something as basic as the below passes testing, not sure
> if we could consider it safe from a theoretical point of view, however.
This version is OK; unevaluated context shouldn't affect this, so that
SFINAE tricks can check for it.
Jason
libgo patch committed: Fix handling of getaddrinfo return type
This libgo patch declares the return type of the C getaddrinfo function, when called from Go, as int32 rather than int. That is important because getaddrinfo returns negative numbers on error, and so numbers like -2 in int32 were being treated as 0xfffe in int, which in Go is a 64 bit type. This caused comparisons against EAI_NONAME, etc., to fail. This fixes https://golang.org/issue/23645. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE === --- gcc/go/gofrontend/MERGE (revision 257280) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -71758f9ca1804743afe178f0e2fca489e0217474 +b833695618d1a5d9d531f5ba0f9c07c7e35e0073 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/net/cgo_unix.go === --- libgo/go/net/cgo_unix.go(revision 257217) +++ libgo/go/net/cgo_unix.go(working copy) @@ -23,7 +23,7 @@ import ( ) //extern getaddrinfo -func libc_getaddrinfo(node *byte, service *byte, hints *syscall.Addrinfo, res **syscall.Addrinfo) int +func libc_getaddrinfo(node *byte, service *byte, hints *syscall.Addrinfo, res **syscall.Addrinfo) int32 //extern freeaddrinfo func libc_freeaddrinfo(res *syscall.Addrinfo)
Go patch committed: Omit the field name for embedded fields in the reflect string
This patch to the Go frontend omits the field name for embedded fields
in the reflect string. This matches the gc compiler. The test case
was sent for the master repo as https://golang.org/cl/91138. This
fixes https://golang.org/issue/23620. Bootstrapped and ran Go
testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257299)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-b833695618d1a5d9d531f5ba0f9c07c7e35e0073
+023c3d4358d101c71ac1436065690eaec2ce138e
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/types.cc
===
--- gcc/go/gofrontend/types.cc (revision 257217)
+++ gcc/go/gofrontend/types.cc (working copy)
@@ -6417,11 +6417,11 @@ Struct_type::do_reflection(Gogo* gogo, s
if (p != this->fields_->begin())
ret->push_back(';');
ret->push_back(' ');
- if (p->is_anonymous())
- ret->push_back('?');
- else
- ret->append(Gogo::unpack_hidden_name(p->field_name()));
- ret->push_back(' ');
+ if (!p->is_anonymous())
+ {
+ ret->append(Gogo::unpack_hidden_name(p->field_name()));
+ ret->push_back(' ');
+ }
if (p->is_anonymous()
&& p->type()->named_type() != NULL
&& p->type()->named_type()->is_alias())
Index: libgo/go/reflect/all_test.go
===
--- libgo/go/reflect/all_test.go(revision 257217)
+++ libgo/go/reflect/all_test.go(working copy)
@@ -170,6 +170,14 @@ var typeTests = []pair{
}{},
"interface { reflect_test.a(func(func(int) int) func(func(int))
int); reflect_test.b() }",
},
+ {struct {
+ x struct {
+ int32
+ int64
+ }
+ }{},
+ "struct { int32; int64 }",
+ },
}
var valueTests = []pair{
Re: [C++ Patch/RFC] PR 83796 ("[6/7/8 Regression] Abstract classes allowed to be instantiated when initialised as default parameter to function or constructor")
Hi, On 01/02/2018 15:54, Jason Merrill wrote: I'm gently "pinging" this message of mine... Definitely not an high priority regression (in any case it's only a P3) but I'm still wondering if we want to do something about the issue at this time. Lately I noticed that in terms of testsuite even something as basic as the below passes testing, not sure if we could consider it safe from a theoretical point of view, however. This version is OK; unevaluated context shouldn't affect this, so that SFINAE tricks can check for it. You are of course totally right. For the specific testcase we got, not using templates, checking for unevaluated context was useful to cut some rather redundant diagnostic, that's what fooled me, at first. Anyway, I have now checked in the last version. Thanks again, Paolo.
[patch,avr,committed] Move disable of -fno-delete-null-pointer-checks to common.
Applied the following patch that moves disabling of
-fno-disable-null-pointer-checks to a different place so that it can be
overwritten on the command-line (as opposed to TARGET_OVERRIDE_OPTIONS).
This allows some more test cases to pass that set
-fdisable-null-pointer-checks.
gcc/
* config/avr/avr.c (avr_option_override): Move disabling of
-fdelete-null-pointer-checks to...
* common/config/avr/avr-common.c (avr_option_optimization_table):
...here.
gcc/testsuite/
* gcc.dg/tree-ssa/vrp111.c (dg-options): Add
-fdelete-null-pointer-checks.
Johann
Index: config/avr/avr.c
===
--- config/avr/avr.c (revision 257298)
+++ config/avr/avr.c (working copy)
@@ -730,14 +730,6 @@ && mcu->macro == NULL)
static void
avr_option_override (void)
{
- /* Disable -fdelete-null-pointer-checks option for AVR target.
- This option compiler assumes that dereferencing of a null pointer
- would halt the program. For AVR this assumption is not true and
- programs can safely dereference null pointers. Changes made by this
- option may not work properly for AVR. So disable this option. */
-
- flag_delete_null_pointer_checks = 0;
-
/* caller-save.c looks for call-clobbered hard registers that are assigned
to pseudos that cross calls and tries so save-restore them around calls
in order to reduce the number of stack slots needed.
Index: common/config/avr/avr-common.c
===
--- common/config/avr/avr-common.c (revision 257298)
+++ common/config/avr/avr-common.c (working copy)
@@ -27,6 +27,12 @@
/* Implement TARGET_OPTION_OPTIMIZATION_TABLE. */
static const struct default_options avr_option_optimization_table[] =
{
+// With -fdelete-null-pointer-checks option, the compiler assumes
+// that dereferencing of a null pointer would halt the program.
+// For AVR this assumption is not true and a program can safely
+// dereference null pointers. Changes made by this option may not
+// work properly for AVR. So disable this option.
+{ OPT_LEVELS_ALL, OPT_fdelete_null_pointer_checks, NULL, 0 },
// The only effect of -fcaller-saves might be that it triggers
// a frame without need when it tries to be smart around calls.
{ OPT_LEVELS_ALL, OPT_fcaller_saves, NULL, 0 },
Index: testsuite/gcc.dg/tree-ssa/vrp111.c
===
--- testsuite/gcc.dg/tree-ssa/vrp111.c (revision 257298)
+++ testsuite/gcc.dg/tree-ssa/vrp111.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-evrp" } */
+/* { dg-options "-O2 -fdump-tree-evrp -fdelete-null-pointer-checks" } */
void foo (void *p) __attribute__((nonnull(1)));
Re: [PATCH] PR84068: Fix sort order of SCHED_PRESSURE_MODEL
Richard Sandiford wrote: > But why wasn't the index 0 as expected for the insns outside of the block? Well it seems it checks for index 0 and sets the model_index as the current maximum model_index count. This means the target_bb check isn't strictly required - I build all of SPECINT2017 using the options from PR84068, and all 577k instances of instruction from another bb would sort correctly. So it looks we can remove the target_bb check. Wilco
[PATCH, combine]: Fix PR84157, ICE: RTL check: expected code 'reg', have 'lshiftrt'
2018-02-01 Uros Bizjak
PR rtl-optimization/84157
* combine.c (change_zero_ext): Use REG_P predicate in
front of HARD_REGISTER_P predicate.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN.
Uros.
diff --git a/gcc/combine.c b/gcc/combine.c
index 970dd26..a9929f2 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11483,7 +11483,7 @@ change_zero_ext (rtx pat)
if (mode != inner_mode)
{
- if (HARD_REGISTER_P (x)
+ if (REG_P (x) && HARD_REGISTER_P (x)
&& !can_change_dest_mode (x, 0, mode))
continue;
@@ -11501,7 +11501,7 @@ change_zero_ext (rtx pat)
x = SUBREG_REG (XEXP (x, 0));
if (GET_MODE (x) != mode)
{
- if (HARD_REGISTER_P (x)
+ if (REG_P (x) && HARD_REGISTER_P (x)
&& !can_change_dest_mode (x, 0, mode))
continue;
Re: wwwdocs: Some release notes for powerpc for GCC 8
On Thu, 1 Feb 2018, Segher Boessenkool wrote: > This is a first set of release notes for Power for GCC 8. > > Is this okay to commit? Yes, thank you! Just the move ofseems unclear to me? Can you omit that? Gerald
Re: build failure x86_64
On 02/01/2018 06:21 AM, Segher Boessenkool wrote: > On Thu, Feb 01, 2018 at 11:52:42AM +, graham stott wrote: >> A recent patch has broken RTL checking builds on x86_64 fails during combine >> RTL check: expected code 'reg' have 'lshirtrt' in rhs_regno at rtl.h:1891 >> builfing libgçc2.c in function __mulsc3 > > This is PR84157. It has a patch. And I think Uros just committed a fix for that :-) jeff
Re: Avoid assembler warnings from AArch64 constructor/destructor priorities
Hi Joseph, aarch64 maintainers, On 28/09/17 13:31, Joseph Myers wrote: Many GCC tests fail for AArch64 with current binutils because of assembler warnings of the form "Warning: ignoring incorrect section type for .init_array.00100". The same issue was fixed for ARM in r247015 by using SECTION_NOTYPE when creating those sections; this patch applies the same fix to AArch64. Tested with no regressions with cross to aarch64-linux-gnu. OK to commit? There is a user request to backport this patch to the GCC 7 and 6 branches: PR 84168. Would that be acceptable? If so, I'm happy to help with the testing on those branches. Thanks, Kyrill 2017-09-28 Joseph Myers * config/aarch64/aarch64.c (aarch64_elf_asm_constructor) (aarch64_elf_asm_destructor): Pass SECTION_NOTYPE to get_section when creating .init_array and .fini_array sections with priority specified. Index: gcc/config/aarch64/aarch64.c === --- gcc/config/aarch64/aarch64.c(revision 253248) +++ gcc/config/aarch64/aarch64.c(working copy) @@ -6095,7 +6095,7 @@ aarch64_elf_asm_constructor (rtx symbol, int prior -Wformat-truncation false positive, use a larger size. */ char buf[23]; snprintf (buf, sizeof (buf), ".init_array.%.5u", priority); - s = get_section (buf, SECTION_WRITE, NULL); + s = get_section (buf, SECTION_WRITE | SECTION_NOTYPE, NULL); switch_to_section (s); assemble_align (POINTER_SIZE); assemble_aligned_integer (POINTER_BYTES, symbol); @@ -6115,7 +6115,7 @@ aarch64_elf_asm_destructor (rtx symbol, int priori -Wformat-truncation false positive, use a larger size. */ char buf[23]; snprintf (buf, sizeof (buf), ".fini_array.%.5u", priority); - s = get_section (buf, SECTION_WRITE, NULL); + s = get_section (buf, SECTION_WRITE | SECTION_NOTYPE, NULL); switch_to_section (s); assemble_align (POINTER_SIZE); assemble_aligned_integer (POINTER_BYTES, symbol); -- Joseph S. Myers jos...@codesourcery.com
Re: wwwdocs: Some release notes for powerpc for GCC 8
On Fri, Feb 02, 2018 at 01:06:59AM +0900, Gerald Pfeifer wrote: > On Thu, 1 Feb 2018, Segher Boessenkool wrote: > > This is a first set of release notes for Power for GCC 8. > > > > Is this okay to commit? > > Yes, thank you! > > Just the move ofseems > unclear to me? Can you omit that? The entries are alphabetic, except that one. Should have put it in a separate patch, sorry. Segher
Re: patch for PR82444
Vladimir Makarov writes:
> Index: ira.c
> ===
> --- ira.c (revision 257157)
> +++ ira.c (working copy)
> @@ -1578,7 +1578,8 @@ ira_init_register_move_cost (machine_mod
>ira_assert (ira_register_move_cost[mode] == NULL
> && ira_may_move_in_cost[mode] == NULL
> && ira_may_move_out_cost[mode] == NULL);
> - ira_assert (have_regs_of_mode[mode]);
> + /* have_regs_of_mode[mode] might be false because it might be
> + E_ (see genmodes) of pseudo with . */
Not sure about the E_/genmodes reference here. Isn't it simply
"because it might be the mode a pseudo register"?
Is it OK to expand the explanation a bit, as below?
Richard
2018-02-01 Richard Sandiford
gcc/
* ira.c (ira_init_register_move_cost): Adjust comment.
Index: gcc/ira.c
===
--- gcc/ira.c 2018-02-01 17:09:45.998165072 +
+++ gcc/ira.c 2018-02-01 17:09:46.150158481 +
@@ -1578,8 +1578,10 @@ ira_init_register_move_cost (machine_mod
ira_assert (ira_register_move_cost[mode] == NULL
&& ira_may_move_in_cost[mode] == NULL
&& ira_may_move_out_cost[mode] == NULL);
- /* have_regs_of_mode[mode] might be false because it might be
- E_ (see genmodes) of pseudo with . */
+ /* Note that we might be asked about the move costs of modes that
+ cannot be stored in any hard register, for example if an inline
+ asm tries to create a register operand with an impossible mode.
+ We therefore can't assert have_regs_of_mode[mode] here. */
for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
{
Re: [PATCH] libgcc: xtensa: fix build with -mtext-section-literals
Hi Sterling, On Wed, Jan 31, 2018 at 11:17 AM, Max Filippov wrote: > On Wed, Jan 31, 2018 at 11:11 AM, augustine.sterl...@gmail.com > wrote: >> On Tue, Jan 30, 2018 at 8:02 PM, Max Filippov wrote: >>> >>> libgcc/ >>> 2018-01-31 Max Filippov >>> >>> * config/xtensa/ieee754-df.S (__adddf3_aux): Add >>> .literal_position directive. >>> * config/xtensa/ieee754-sf.S (__addsf3_aux): Likewise. >> >> This is fine, but when did it stop working, and why isn't there a test >> that would catch it? > > I broke it with the recent softfloat NaN fix. Haven't noticed that because > I usually build without any flags or with -mauto-litpools. > Let me add the test, thanks for the suggestion. I've taken a look at gcc testsuites and I don't see how to add a test that would check that libgcc builds with different compiler options. Any suggestions? -- Thanks. -- Max
Re: [PATCH] PR84068: Fix sort order of SCHED_PRESSURE_MODEL
Wilco Dijkstra writes: > Richard Sandiford wrote: > >> But why wasn't the index 0 as expected for the insns outside of the block? > > Well it seems it checks for index 0 and sets the model_index as the current > maximum model_index count. This means the target_bb check isn't > strictly required - I build all of SPECINT2017 using the options from PR84068, > and all 577k instances of instruction from another bb would sort correctly. > > So it looks we can remove the target_bb check. Thanks for testing that. Removing the check would be my preference, since model_index was supposed to cope with insns outside the model schedule, and I think other callers relied on that too. If we do end up finding cases in which INSN_MODEL_INDEX is nonzero for insns in other blocks, I think it would better to fix that, either by forcing it to zero at an appropriate place, or by checking the block in model_index itself. Richard
[PR target/84089] handle E_VOIDmode in PA's base14_operand
Before r196122, a VOIDmode would return the default value of false in base14_operand, but when S?mode and D?mode's were collapsed with the aforementioned patch, we started handling E_VOIDmode which has a size of 0. The zero is causes a division by zero in the PR's testcase. Dave has suggested returning false for E_VOIDmode. Having no access to a PA system, I haven't tested this other than visual inspection and making sure we don't ICE. However, since a VOID clearly causes a division by zero, we're not worse off than before :). Perhaps someone with access to a PA box can run further tests. In the meantime... committed to trunk. Pre-approved by Dave. Aldy PR target/84089 * config/pa/predicates.md (base14_operand): Handle E_VOIDmode. diff --git a/gcc/config/pa/predicates.md b/gcc/config/pa/predicates.md index 4600f988c87..cc2bffa0762 100644 --- a/gcc/config/pa/predicates.md +++ b/gcc/config/pa/predicates.md @@ -277,6 +277,9 @@ case E_HImode: return true; +case E_VOIDmode: + return false; + default: return (INTVAL (op) % GET_MODE_SIZE (mode)) == 0; }
Re: Avoid assembler warnings from AArch64 constructor/destructor priorities
On Thu, 1 Feb 2018, Kyrill Tkachov wrote: > Hi Joseph, aarch64 maintainers, > > On 28/09/17 13:31, Joseph Myers wrote: > > Many GCC tests fail for AArch64 with current binutils because of > > assembler warnings of the form "Warning: ignoring incorrect section > > type for .init_array.00100". The same issue was fixed for ARM in > > r247015 by using SECTION_NOTYPE when creating those sections; this > > patch applies the same fix to AArch64. > > > > Tested with no regressions with cross to aarch64-linux-gnu. OK to > > commit? > > There is a user request to backport this patch to the GCC 7 and 6 > branches: PR 84168. > > Would that be acceptable? I don't object, but I'm not aware of this being a regression. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] PR 83975 Associate target with non-constant character length
Hi Janne, When associating a variable of type character, if the length of the target isn't known at compile time, generate an error. See PR 83344 for more details. Regtested on x86_64-pc-linux-gnu, Ok for trunk? I think this is the most reasonable course of action, given the circumstances. OK for trunk. Thanks! Regards Thomas
Re: [PR tree-optimization/84047] missing -Warray-bounds on an out-of-bounds index
Since my patch isn't the easy one liner I wanted it to be, perhaps we
should concentrate on Martin's patch, which is more robust, and has
testcases to boot! His patch from last week also fixes a couple other
PRs.
Richard, would this be acceptable? That is, could you or Jakub review
Martin's all-encompassing patch? If so, I'll drop mine.
Also, could someone pontificate on whether we want to fix
-Warray-bounds regressions for this release cycle?
Thanks.
On Wed, Jan 31, 2018 at 6:05 AM, Richard Biener
wrote:
> On Tue, Jan 30, 2018 at 11:11 PM, Aldy Hernandez wrote:
>> Hi!
>>
>> [Note: Jakub has mentioned that missing -Warray-bounds regressions should be
>> punted to GCC 9. I think this particular one is easy pickings, but if this
>> and/or the rest of the -Warray-bounds regressions should be marked as GCC 9
>> material, please let me know so we can adjust all relevant PRs.]
>>
>> This is a -Warray-bounds regression that happens because the IL now has an
>> MEM_REF instead on ARRAY_REF.
>>
>> Previously we had an ARRAY_REF we could diagnose:
>>
>> D.2720_5 = "12345678"[1073741824];
>>
>> But now this is represented as:
>>
>> _1 = MEM[(const char *)"12345678" + 1073741824B];
>>
>> I think we can just allow check_array_bounds() to handle MEM_REF's and
>> everything should just work.
>>
>> The attached patch fixes both regressions mentioned in the PR.
>>
>> Tested on x86-64 Linux.
>>
>> OK?
>
> This doesn't look correct. You lump MEM_REF handling together with
> ADDR_EXPR handling but for the above case you want to diagnose
> _dereferences_ not address-taking.
>
> For the dereference case you need to amend the ARRAY_REF case, for example
> via
>
> Index: gcc/tree-vrp.c
> ===
> --- gcc/tree-vrp.c (revision 257181)
> +++ gcc/tree-vrp.c (working copy)
> @@ -5012,6 +5012,13 @@ check_array_bounds (tree *tp, int *walk_
>if (TREE_CODE (t) == ARRAY_REF)
> vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/);
>
> + else if (TREE_CODE (t) == MEM_REF
> + && TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR
> + && TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 0)) == STRING_CST)
> +{
> + call factored part of check_array_ref passing in STRING_CST and offset
> +}
> +
>else if (TREE_CODE (t) == ADDR_EXPR)
> {
>vrp_prop->search_for_addr_array (t, location);
>
> note your patch will fail to warn for "1"[1] because taking that
> address is valid but not
> dereferencing it.
>
> Richard.
Re: [PR target/84089] handle E_VOIDmode in PA's base14_operand
On 2018-02-01 12:20 PM, Aldy Hernandez wrote: Perhaps someone with access to a PA box can run further tests. I have a couple of tests running. Thanks, Dave -- John David Anglin dave.ang...@bell.net
Re: [PATCH] have -Wformat-overflow handle -fexec-charset (PR 80503)
On 02/01/2018 04:54 AM, Renlin Li wrote: Hi Martin, On 01/02/18 00:40, Martin Sebor wrote: On 01/31/2018 10:36 AM, Renlin Li wrote: Hi there, I have a patch to fix to regressions we observed in armhf native environment. To effectively check out of range for format string, a target type should be used. And according to the standard, int type is used for "width" and "precision" field of format string. Here target_strtol10 is rename to target_strtoi10, and fixed to use target_int_max () which is target dependent. The value returned by target_strtol10 is (target_int_max () + 1) when it exceeds the range. This is used to indicate its value exceeds target INT_MAX for the later warning. Sorry for not responding to your original email. It's still in my inbox, just buried under a pile of stuff. Using LONG_MAX is not ideal but unless I missed something (it's been a while) I think using INT_MAX for the target would be even less optimal. Unless specified by the asterisk, width and precision can be arbitrarily large. I cannot find more document about this other than here: http://en.cppreference.com/w/c/io/fprintf The best reference is the C standard. The most recent publicly available draft of C11 is here: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf The next best (and sometimes better) reference is POSIX: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html (optional) integer value or * that specifies minimum field width. (optional) . followed by integer number or *, or neither that specifies precision of the conversion. It only mentions a integer value, with no type. I assume it could be of type int? It is indeed very unclear about the range of width and precision. Because the spec doesn't say what precision the number is to be interpreted in, it must be taken to mean a number with an arbitrary precision. constraining either to INT_MAX would trigger warnings on LP64 targets for safe calls like: // INT_MAX is 2147483647 sprintf (d, "%.2147483648s", ""); I think we want to use the maximum value of an unsigned type with the greatest precision on the host. It will still warn for directives with precisions in excess of HOST_WIDE_INT_MAX Is it here a target type should be used to test the range against? Instead of the host where the toolchain is built? I think for the purposes of runtime evaluation by a conforming implementation, both the width and the precision need to be interpreted "as if" with infinite precision. But after sleeping on it, I suppose that for the purposes of the warning, it could be helpful to point out values that exceed even INT_MAX on the host (rather than target) because they aren't very meaningful and are most likely mistakes (e.g., hidden by macro expansion). It might still be useful to have the pass store the larger number (up to its limit, whatever that is on the host), and use it for its internal computation and subsequent diagnostics for accuracy. Martin Regards, Renlin but short of using something like offset_int it's probably as good as it's going to get. (It has been suggested that the whole pass would benefit from using offset_int to track large numbers so if/when it's enhanced to make this change it should also make the code behave more consistently across different hosts.) Martin Is it Okay to commit? Regards, Renlin gcc/ChangeLog: 2018-01-31 Renlin Li * gimple-ssa-sprintf.c (target_strtol10): Use target integer to check the range. Rename it into (target_strtoi10): This. (parse_directive): Compare with target int max instead of LONG_MAX. On 20/06/17 12:00, Renlin Li wrote: Hi Martin, I did a little investigation into this. Please correct me if I missed anything. I build a native arm-linux-gnueabihf toolchain in armhf hardware. It's ILP32. So in this situation: HOST_WIDE_INT is long, which is 32-bit. integer type 32-bit as well, so target_int_max () == LONG_MAX gimple-ssa-sprintf.c line 2887 /* Has the likely and maximum directive output exceeded INT_MAX? */ bool likelyximax = *dir.beg && res->range.likely > target_int_max (); likelyximax will be false as the latter expression is always false. res->range.likely is truncated to LONG_MAX (in target_strtol10 function) I have checked in cross build environment (host x86_64), this variable is true. Regards, Renlin On 13/06/17 09:16, Renlin Li wrote: Hi Martin, On 04/06/17 23:24, Martin Sebor wrote: On 06/02/2017 09:38 AM, Renlin Li wrote: Hi Martin, After r247444, I saw the following two regressions in arm-linux-gnueabihf environment: FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 119) PASS: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 121) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c (test for warnings, line 121) The warning message related to those two lines are: testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:119:3: warning: '%9223372036854
C++ PATCH for c++/84126, ICE with variadic generic lambda
In a partial instantiation of a pack expansion we can have a situation
where we have arguments for some of the packs but not all. We deal
with this by leaving the pattern uninstantiated and remembering the
arguments passed to the partial specialization so we can use them
later in doing a full instantiation. My patch for 84036 started to
use this mechanism when we don't have the expansion of a function
parameter pack, but it broke for this testcase because when we went to
look for the local specialization of the function parameter pack, we
were looking for a specialization of the original parameter pack, but
local_specializations only knows about the partially instantiated
parameter pack. Fixed by remembering in this situation what function
we are using parameter packs from, so we can set up the appropriate
mapping.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit ad4789988c130a58f6e3c885cfa10e02b8fa0c77
Author: Jason Merrill
Date: Wed Jan 31 17:35:59 2018 -0500
PR c++/84126 - ICE with variadic generic lambda
PR c++/84036
PR c++/82249
* pt.c (tsubst_pack_expansion): Handle function parameter_packs in
PACK_EXPANSION_EXTRA_ARGS.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8983674881d..5f14e514638 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3496,7 +3496,11 @@ extern void decl_shadowed_for_var_insert (tree, tree);
: &TYPE_MIN_VALUE_RAW (TYPE_PACK_EXPANSION_CHECK (NODE)))
/* Any additional template args to be applied when substituting into
- the pattern, set by tsubst_pack_expansion for partial instantiations. */
+ the pattern, set by tsubst_pack_expansion for partial instantiations.
+ If this is a TREE_LIST, the TREE_VALUE of the first element is the
+ usual template argument TREE_VEC, and the TREE_PURPOSE of later elements
+ are enclosing functions that provided function parameter packs we'll need
+ to map appropriately. */
#define PACK_EXPANSION_EXTRA_ARGS(NODE)\
*(TREE_CODE (NODE) == TYPE_PACK_EXPANSION\
? &TYPE_MAX_VALUE_RAW (NODE) \
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9516be893aa..ca73bb18d97 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11439,7 +11439,20 @@ tsubst_pack_expansion (tree t, tree args,
tsubst_flags_t complain,
pattern = PACK_EXPANSION_PATTERN (t);
/* Add in any args remembered from an earlier partial instantiation. */
- args = add_to_template_args (PACK_EXPANSION_EXTRA_ARGS (t), args);
+ tree extra = PACK_EXPANSION_EXTRA_ARGS (t);
+ if (extra && TREE_CODE (extra) == TREE_LIST)
+{
+ /* The partial instantiation involved function parameter packs; map
+ from the general template to our current context. */
+ for (tree fns = TREE_CHAIN (extra); fns; fns = TREE_CHAIN (fns))
+ {
+ tree fn = TREE_PURPOSE (fns);
+ tree inst = enclosing_instantiation_of (fn);
+ register_parameter_specializations (fn, inst);
+ }
+ extra = TREE_VALUE (extra);
+}
+ args = add_to_template_args (extra, args);
levels = TMPL_ARGS_DEPTH (args);
@@ -11610,7 +11623,28 @@ tsubst_pack_expansion (tree t, tree args,
tsubst_flags_t complain,
have values for all the packs. So remember these until then. */
t = make_pack_expansion (pattern, complain);
- PACK_EXPANSION_EXTRA_ARGS (t) = args;
+ tree extra = args;
+ if (unsubstituted_fn_pack)
+ {
+ /* For function parameter packs it's more complicated; we need to
+remember which enclosing function(s) provided them to this pack
+expansion so we can map their parameters to the parameters of a
+later full instantiation. */
+ tree fns = NULL_TREE;
+ for (tree p = packs; p; p = TREE_CHAIN (p))
+ {
+ tree parm = TREE_PURPOSE (p);
+ if (TREE_CODE (parm) != PARM_DECL)
+ continue;
+ parm = DECL_CONTEXT (parm);
+ if (purpose_member (parm, fns))
+ continue;
+ fns = tree_cons (parm, NULL_TREE, fns);
+ }
+ if (fns)
+ extra = tree_cons (NULL_TREE, extra, fns);
+ }
+ PACK_EXPANSION_EXTRA_ARGS (t) = extra;
return t;
}
else if (unsubstituted_packs)
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic9.C
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic9.C
new file mode 100644
index 000..1b3a6442efd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic9.C
@@ -0,0 +1,19 @@
+// PR c++/84126
+// { dg-do compile { target c++14 } }
+
+template
+void sink(Ts...);
+
+template
+int bar(T&); // ICE with reference, work with just T
+
+template
+void foo(T){
+ [](auto ... k){
+sink (bar(k) ...);
+ }(0);
+}
+
+int main() {
+ foo(0);
+}
Re: [PATCH, rs6000] Fix PR56010 and PR83743, -mcpu=native use wrong names
On 2/1/18 8:50 AM, Segher Boessenkool wrote: > I think we also want this for 7 (after a bit of burn in). I wouldn't > bother with 6 though (the problem has existed since 4.7). Ok, committed. I'll wait a few days before committing the FSF 7 back port. Thanks! Peter
Re: patch for PR82444
On 02/01/2018 12:10 PM, Richard Sandiford wrote:
Vladimir Makarov writes:
Not sure about the E_/genmodes reference here. Isn't it simply
"because it might be the mode a pseudo register"?
Is it OK to expand the explanation a bit, as below?
Yes, it is OK. It is a better explanation. Thank you, Richard.
2018-02-01 Richard Sandiford
gcc/
* ira.c (ira_init_register_move_cost): Adjust comment.
Index: gcc/ira.c
===
--- gcc/ira.c 2018-02-01 17:09:45.998165072 +
+++ gcc/ira.c 2018-02-01 17:09:46.150158481 +
@@ -1578,8 +1578,10 @@ ira_init_register_move_cost (machine_mod
ira_assert (ira_register_move_cost[mode] == NULL
&& ira_may_move_in_cost[mode] == NULL
&& ira_may_move_out_cost[mode] == NULL);
- /* have_regs_of_mode[mode] might be false because it might be
- E_ (see genmodes) of pseudo with . */
+ /* Note that we might be asked about the move costs of modes that
+ cannot be stored in any hard register, for example if an inline
+ asm tries to create a register operand with an impossible mode.
+ We therefore can't assert have_regs_of_mode[mode] here. */
for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
{
[PATCH] PowerPC PR target/84154, fix floating point to small integer conversion regression
This patch fixes the optimization regression that occurred on GCC 7 where conversions from the various floating point types to small integers would at times generate a store and a load. For example, converting from double to unsigned char generated the following code on GCC 6 for -mcpu=power8: fctiwuz 1,1 mfvsrd 3,1 rlwinm 3,3,0,0xff on GCC 7 and 8 it generates: fctiwuz 0,1 mfvsrwz 9,0 stw 9,-16(1) ori 2,2,0 lbz 3,-16(1) The insns before register allocation are: (insn 7 8 13 2 (set (subreg:SI (reg:QI 157) 0) (unsigned_fix:SI (reg:SF 33))) (insn 13 7 14 2 (set (reg/i:DI 3 3) (zero_extend:DI (reg:QI 157 After reload, the insns are: (insn 7 8 19 2 (set (reg:SI 32 0 [160]) (unsigned_fix:SI (reg:SF 33 (insn 19 7 18 2 (set (reg:SI 9 9 [160]) (reg:SI 32 0 [160]))) (insn 18 19 13 2 (set (mem/c:SI (plus:DI (reg/f:DI 1 1) (const_int -16)) (reg:SI 9 (insn 13 18 14 2 (set (reg/i:DI 3 3) (zero_extend:DI (mem/c:HI (plus:DI (reg/f:DI 1 1) (const_int -16)) ISA 3.0 (Power9) did not have this problem, because it already had a fixuns_truncdfqi2 pattern, since QI/HImode values are allowed in vector registers. Previous versions of the ISA did not allow QI/HImode into vector registers, because there wasn't load or store byte/half-word operations. I extended ISA 3.0 conversion patterns to handle ISA 2.07, using splitters to move the 32-bit int parts back to the GPR to do sign/zero extension or stores. I also moved the optimization to prevent the register allocator from doing a direct move on ISA 3.0 to do an offsettable store via the GPR register to a separate insn, like I had previously done for SImode. The rationale for this is to prevent some places where the register allocator decided to do change a store into a move (and then later store). I have tested this patch on a little endian power8 system (64-bit) and a big endian power8 system (both 32-bit and 64-bit executables). There were no regressions in the test suite and the compiler bootstrapped fine. I added some tests, and verified they ran in all 3 environments. Can I check this into the trunk? Given this is a regression in GCC 7 as well, can I check the patch if it applies cleanly into GCC 7 after a burn-in period. [gcc] 2018-02-01 Michael Meissner PR target/84154 * config/rs6000/rs6000.md (fix_trunc2): Convert from define_expand to be define_insn_and_split. Rework float/double/_Float128 conversions to QI/HI/SImode to work with both ISA 2.07 (power8) or ISA 3.0 (power9). Fix regression where conversions to QI/HImode types did a store and then a load to truncate the value. For conversions to VSX registers, don't split the insn, instead emit the code directly. (fixuns_trunc2): Likewise. (fix_trunc2): Likewise. (fix_trunc2_internal): Delete, no longer used. (fixuns_trunc2_internal): Likewise. (fix__mem): Likewise. (fix_trunc2_mem): On ISA 3.0, prevent the register allocator from doing a direct move to the GPRs to do a store, and instead use the ISA 3.0 store byte/half-word from vector register instruction. For IEEE 128-bit floating point, also optimize stores of 32-bit ints. (fixuns_trunc2_mem): Likewise. (fix_trunc2_mem): Likewise. (fixuns_trunc2_mem): Likewise. [gcc/testsuite] 2018-02-01 Michael Meissner PR target/84154 * gcc.target/powerpc/pr84154-1.c: New tests. * gcc.target/powerpc/pr84154-2.c: Likewise. * gcc.target/powerpc/pr84154-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.md === --- gcc/config/rs6000/rs6000.md (revision 257269) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -5700,43 +5700,48 @@ (define_insn "*fix_truncdi2_fctidz xscvdpsxds %x0,%x1" [(set_attr "type" "fp")]) -(define_expand "fix_trunc2" - [(parallel [(set (match_operand: 0 "nonimmediate_operand") - (fix:QHI (match_operand:SFDF 1 "gpc_reg_operand"))) - (clobber (match_scratch:DI 2))])] - "TARGET_P9_VECTOR && TARGET_DIRECT_MOVE_64BIT" +;; If have ISA 3.0, QI/HImode values can go in both VSX registers and GPR +;; registers. If we have ISA 2.07, we don't allow QI/HImode values in the +;; vector registers, so we need to do direct moves to the GPRs, but SImode +;; values can go in VSX registers. Ke
[PATCH, rs6000] Add builtin support for vec_insert4b, vec_extract4b
GCC maintainers: The following patch contains fixes for the builtins to add and extract a word from a char vector. The existing names for the builtins that do this are not consistent with the ABI. Additionally, the supported arguments are not all consistent with the ABI. This patch fixes the support for the insert and extract word builtins so they are consistent with the "64-Bit ELF V2 ABI Specification". The patch has been tested on: powerpc64le-unknown-linux-gnu (Power 8 LE) powerpc64le-unknown-linux-gnu (Power 9 LE) Let me know if the patch looks OK or not. Let me know if you want to include it in stage 4 or wait for the next release. Thanks. Carl Love -- gcc/ChangeLog: 2018-01-31 Carl Love * config/rs6000/altivec.h: Change vec_vextract4b to vec_extract4b and vec_vinsert4b to vec_insert4b. * config/rs6000/rs6000-builtin.def: Remove VEXTRACT4B, VINSERT4B and VINSERT4B_DI definitions. Add INSERT4B and EXTRACT4B definitions. * config/rs6000/rs6000-c.c: Remove P9V_BUILTIN_VEC_VEXTRACT4B and P9V_BUILTIN_VEC_VINSERT4B definitions. Add the definitions for P9V_BUILTIN_VEC_EXTRACT4B and P9V_BUILTIN_VEC_INSERT4B. * config/rs6000/rs6000.c (altivec_expand_builtin): Remove P9V_BUILTIN_VEXTRACT4B, P9V_BUILTIN_VEC_VEXTRACT4B, P9V_BUILTIN_VINSERT4B, P9V_BUILTIN_VINSERT4B_DI, P9V_BUILTIN_VEC_VINSERT4B case statements. Add P9V_BUILTIN_EXTRACT4B and P9V_BUILTIN_INSERT4B case statements. * config/rs6000/vsx.md: Remove define_expand vextract4b and define_insn_and_split *vextract4b_internal. Add define_insn extract4b definition. Rename define_expand vinsert4b to define_expand insert4b and define_insn *vinsert4b_internal to define_insn *insert4b_internal. Remove definitions for define_expand vinsert4b_di and define_insn *vinsert4b_di_internal. *doc/extend.texi: Remove documentation for the vec_vextract4b. Fix documentation for vec_insert4b. Add documentation for vec_extract4b. gcc/testsuite/ChangeLog: 2018-01-31 Carl Love * gcc.target/powerpc/builtins-7-p9-runnable.c: New runnable test file. * gcc.target/powerpc/p9_vinsert4b-1.c: Remove file. * gcc.target/powerpc/p9_vinsert4b-2.c: Remove file. --- gcc/config/rs6000/altivec.h| 4 +- gcc/config/rs6000/rs6000-builtin.def | 9 +- gcc/config/rs6000/rs6000-c.c | 31 +--- gcc/config/rs6000/rs6000.c | 7 +- gcc/config/rs6000/vsx.md | 65 ++-- gcc/doc/extend.texi| 11 +- .../gcc.target/powerpc/builtins-7-p9-runnable.c| 168 + gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-1.c | 39 - gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-2.c | 30 9 files changed, 197 insertions(+), 167 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/builtins-7-p9-runnable.c delete mode 100644 gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-1.c delete mode 100644 gcc/testsuite/gcc.target/powerpc/p9-vinsert4b-2.c diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h index 684cb1990..1e495e69c 100644 --- a/gcc/config/rs6000/altivec.h +++ b/gcc/config/rs6000/altivec.h @@ -433,8 +433,8 @@ #define vec_vctzd __builtin_vec_vctzd #define vec_vctzh __builtin_vec_vctzh #define vec_vctzw __builtin_vec_vctzw -#define vec_vextract4b __builtin_vec_vextract4b -#define vec_vinsert4b __builtin_vec_vinsert4b +#define vec_extract4b __builtin_vec_extract4b +#define vec_insert4b __builtin_vec_insert4b #define vec_vprtyb __builtin_vec_vprtyb #define vec_vprtybd __builtin_vec_vprtybd #define vec_vprtybw __builtin_vec_vprtybw diff --git a/gcc/config/rs6000/rs6000-builtin.def b/gcc/config/rs6000/rs6000-builtin.def index 86604da46..37595cc29 100644 --- a/gcc/config/rs6000/rs6000-builtin.def +++ b/gcc/config/rs6000/rs6000-builtin.def @@ -2226,9 +2226,8 @@ BU_P9V_AV_2 (VEXTUWLX, "vextuwlx",CONST, vextuwlx) BU_P9V_AV_2 (VEXTUWRX, "vextuwrx", CONST, vextuwrx) /* Insert/extract 4 byte word into a vector. */ -BU_P9V_VSX_2 (VEXTRACT4B, "vextract4b", CONST, vextract4b) -BU_P9V_VSX_3 (VINSERT4B,"vinsert4b", CONST, vinsert4b) -BU_P9V_VSX_3 (VINSERT4B_DI, "vinsert4b_di",CONST, vinsert4b_di) +BU_P9V_VSX_3 (INSERT4B,"insert4b", CONST, insert4b) +BU_P9V_VSX_2 (EXTRACT4B, "extract4b",CONST, extract4b) /* Hardware IEEE 128-bit floating point round to odd instrucitons added in ISA 3.0 (power9). */ @@ -2290,12 +2289,12 @@ BU_P9V_OVERLOAD_2 (LXVL,"lxvl") BU_P9V_OVERLOAD_2 (XL_LEN_R, "xl_len_r") BU_P9V_OVERLOAD_2 (VEXTULX,"vextulx") BU_P9V_OVERLOAD_2 (VEXTURX,"vexturx") -BU_P9V_OVERLOAD_2 (VEXTRACT4B, "vextr
Re: C++ PATCH to fix ICE in generic lambda with user-defined conversion (PR c++/84125)
On Tue, Jan 30, 2018 at 3:18 PM, Marek Polacek wrote:
> This testcase breaks since r256550 because we end up trying to build_address
> of
> a CONSTRUCTOR, but that doesn't work because we hit
> gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR);
>
> finish_static_assert gets {} as 'condition'.
Well, it gets X{}.
> In the testcase we have a
> user-defined conversion, so {} should be turned to false, via
> perform_implicit_conversion_flags -> ... -> build_user_type_conversion_1, but
> that crashes as explained above.
>
> This only happens while processing generic lambda because
> processing_template_decl is 1,
> so finish_compound_literal returns {} instead of a TARGET_EXPR.
So I think we should disable the above assert if
processing_template_decl; the CONSTRUCTOR is actually fine here.
> Part of r256550 was this change:
>
> @@ -8640,9 +8642,9 @@ finish_static_assert (tree condition, tree message,
> location_t location,
> }
>
>/* Fold the expression and convert it to a boolean value. */
> - condition = instantiate_non_dependent_expr (condition);
> - condition = cp_convert (boolean_type_node, condition, tf_warning_or_error);
> - condition = maybe_constant_value (condition);
> + condition = perform_implicit_conversion_flags (boolean_type_node,
> condition,
> +complain, LOOKUP_NORMAL);
> + condition = fold_non_dependent_expr (condition);
>
>if (TREE_CODE (condition) == INTEGER_CST && !integer_zerop (condition))
> /* Do nothing; the condition is satisfied. */
>
> where instantiate_non_dependent_expr turned {} into TARGET_EXPR ,
> which we can process just fine. So perhaps we need to put the call back.
The problem with that is that fold_non_dependent_expr does
instantiate_non_dependent_expr, so adding the call back would mean
instantiating twice in a row, which is likely to break.
Let's just adjust the assert.
Jason
[patch, fortran] Fix PR 68560
Hello world,
this patch fixes a regression by removing a KIND argument
(which is encoded into the function name anyway) from the
call to the library function. This extra argument led to
an argument mismatch between the front end and the library
and between different instances of the same function.
Regression-testing as I write this. If it passes
(which I expect), OK for trunk?
Regards
Thomas
2018-02-01 Thomas Koenig
PR fortran/68560
* trans-intrinsic.c (gfc_conv_intrinsic_shape): New function.
(gfc_conv_intrinsic_function): Call it.
2018-02-01 Thomas Koenig
PR fortran/68560
* gfortran.dg/shape_9.f90: New test.
Index: trans-intrinsic.c
===
--- trans-intrinsic.c (Revision 257131)
+++ trans-intrinsic.c (Arbeitskopie)
@@ -5593,6 +5593,25 @@ gfc_conv_intrinsic_ibits (gfc_se * se, gfc_expr *
}
static void
+gfc_conv_intrinsic_shape (gfc_se *se, gfc_expr *expr)
+{
+ gfc_actual_arglist *s, *k;
+ gfc_expr *e;
+
+ /* Remove the KIND argument, if present. */
+ s = expr->value.function.actual;
+ k = s->next;
+ if (k)
+{
+ e = k->expr;
+ gfc_free_expr (e);
+ k->expr = NULL;
+}
+
+ gfc_conv_intrinsic_funcall (se, expr);
+}
+
+static void
gfc_conv_intrinsic_shift (gfc_se * se, gfc_expr * expr, bool right_shift,
bool arithmetic)
{
@@ -8718,6 +8737,10 @@ gfc_conv_intrinsic_function (gfc_se * se, gfc_expr
gfc_conv_intrinsic_minmaxloc (se, expr, GT_EXPR);
break;
+ case GFC_ISYM_SHAPE:
+ gfc_conv_intrinsic_shape (se, expr);
+ break;
+
default:
gfc_conv_intrinsic_funcall (se, expr);
break;
! { dg-do run }
! { dg-require-effective-target lto }
! { dg-options "-flto" }
! Check that there are no warnings with LTO for a KIND argument.
!
program test
implicit none
real, allocatable :: x(:,:)
allocate(x(2,5))
if (any(shape(x) /= [ 2, 5 ])) call abort
if (any(shape(x,kind=1) /= [ 2, 5 ])) call abort
if (any(shape(x,kind=2) /= [ 2, 5 ])) call abort
if (any(shape(x,kind=4) /= [ 2, 5 ])) call abort
if (any(shape(x,kind=8) /= [ 2, 5 ])) call abort
end program test
Re: C++ PATCH to fix ICE in generic lambda with user-defined conversion (PR c++/84125)
On Thu, Feb 01, 2018 at 02:35:08PM -0500, Jason Merrill wrote:
> On Tue, Jan 30, 2018 at 3:18 PM, Marek Polacek wrote:
> > This testcase breaks since r256550 because we end up trying to
> > build_address of
> > a CONSTRUCTOR, but that doesn't work because we hit
> > gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR);
> >
> > finish_static_assert gets {} as 'condition'.
>
> Well, it gets X{}.
>
> > In the testcase we have a
> > user-defined conversion, so {} should be turned to false, via
> > perform_implicit_conversion_flags -> ... -> build_user_type_conversion_1,
> > but
> > that crashes as explained above.
> >
> > This only happens while processing generic lambda because
> > processing_template_decl is 1,
> > so finish_compound_literal returns {} instead of a TARGET_EXPR.
>
> So I think we should disable the above assert if
> processing_template_decl; the CONSTRUCTOR is actually fine here.
Thanks. So like this?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2018-02-01 Marek Polacek
PR c++/84125
* typeck.c (build_address): Relax the assert when
processing_template_decl.
* g++.dg/cpp1y/lambda-generic-84125.C:New test.
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 1102f677f15..83e76782998 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -5735,7 +5735,8 @@ build_address (tree t)
{
if (error_operand_p (t) || !cxx_mark_addressable (t))
return error_mark_node;
- gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR);
+ gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR
+ || processing_template_decl);
t = build_fold_addr_expr_loc (EXPR_LOCATION (t), t);
if (TREE_CODE (t) != ADDR_EXPR)
t = rvalue (t);
diff --git gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
index e69de29bb2d..8bf6a09652e 100644
--- gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
+++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
@@ -0,0 +1,10 @@
+// PR c++/84125
+// { dg-do compile { target c++14 } }
+
+struct X { constexpr operator bool() const { return true; } };
+
+int main(){
+[](auto) {
+static_assert(X{}, "");
+};
+}
Marek
Re: C++ PATCH to fix ICE in generic lambda with user-defined conversion (PR c++/84125)
OK.
On Thu, Feb 1, 2018 at 3:23 PM, Marek Polacek wrote:
> On Thu, Feb 01, 2018 at 02:35:08PM -0500, Jason Merrill wrote:
>> On Tue, Jan 30, 2018 at 3:18 PM, Marek Polacek wrote:
>> > This testcase breaks since r256550 because we end up trying to
>> > build_address of
>> > a CONSTRUCTOR, but that doesn't work because we hit
>> > gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR);
>> >
>> > finish_static_assert gets {} as 'condition'.
>>
>> Well, it gets X{}.
>>
>> > In the testcase we have a
>> > user-defined conversion, so {} should be turned to false, via
>> > perform_implicit_conversion_flags -> ... -> build_user_type_conversion_1,
>> > but
>> > that crashes as explained above.
>> >
>> > This only happens while processing generic lambda because
>> > processing_template_decl is 1,
>> > so finish_compound_literal returns {} instead of a TARGET_EXPR.
>>
>> So I think we should disable the above assert if
>> processing_template_decl; the CONSTRUCTOR is actually fine here.
>
> Thanks. So like this?
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-02-01 Marek Polacek
>
> PR c++/84125
> * typeck.c (build_address): Relax the assert when
> processing_template_decl.
>
> * g++.dg/cpp1y/lambda-generic-84125.C:New test.
>
> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> index 1102f677f15..83e76782998 100644
> --- gcc/cp/typeck.c
> +++ gcc/cp/typeck.c
> @@ -5735,7 +5735,8 @@ build_address (tree t)
> {
>if (error_operand_p (t) || !cxx_mark_addressable (t))
> return error_mark_node;
> - gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR);
> + gcc_checking_assert (TREE_CODE (t) != CONSTRUCTOR
> + || processing_template_decl);
>t = build_fold_addr_expr_loc (EXPR_LOCATION (t), t);
>if (TREE_CODE (t) != ADDR_EXPR)
> t = rvalue (t);
> diff --git gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
> gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
> index e69de29bb2d..8bf6a09652e 100644
> --- gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
> +++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-84125.C
> @@ -0,0 +1,10 @@
> +// PR c++/84125
> +// { dg-do compile { target c++14 } }
> +
> +struct X { constexpr operator bool() const { return true; } };
> +
> +int main(){
> +[](auto) {
> +static_assert(X{}, "");
> +};
> +}
>
> Marek
libgo patch committed: Adjust math flags, use them for testing
The libgo configury sets special flags to use when compiling the math
package, to get more efficient output. Unfortunately, it did not use
those flags when testing the math package. This patch fixes that.
Fixing that revealed that the flags weren't OK for the current math
package, so this patch stops using -funsafe-math-optimizations and
starts using -fno-trapping-math, and makes the options more consistent
across all targets. This patch also includes
https://golang.org/cl/91335, for the master repo, for better testing
in gccgo for now. This fixes https://golang.org/issue/23647.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed
to mainline.
Ian
Index: libgo/Makefile.am
===
--- libgo/Makefile.am (revision 257306)
+++ libgo/Makefile.am (working copy)
@@ -1138,6 +1138,7 @@
# Pass -ffp-contract=off, or 386-specific options, when building the
# math package. MATH_FLAG is defined in configure.ac.
math_lo_GOCFLAGS = $(MATH_FLAG)
+math_check_GOCFLAGS = $(MATH_FLAG)
# Add the generated file runtime_sysinfo.go to the runtime package.
extra_go_files_runtime = runtime_sysinfo.go sigtab.go
Index: libgo/configure.ac
===
--- libgo/configure.ac (revision 257306)
+++ libgo/configure.ac (working copy)
@@ -649,8 +649,7 @@
AC_SUBST(STRINGOPS_FLAG)
dnl For x86 we want to compile the math library with -mfancy-math-387
-dnl -funsafe-math-optimizations so that we can use the builtin
-dnl instructions directly.
+dnl so that we can use the builtin instructions directly.
AC_CACHE_CHECK([whether compiler supports -mfancy-math-387],
[libgo_cv_c_fancymath],
[CFLAGS_hold=$CFLAGS
@@ -661,10 +660,9 @@
CFLAGS=$CFLAGS_hold])
MATH_FLAG=
if test "$libgo_cv_c_fancymath" = yes; then
- MATH_FLAG="-mfancy-math-387 -funsafe-math-optimizations -fno-math-errno"
-else
- MATH_FLAG="-ffp-contract=off"
+ MATH_FLAG="-mfancy-math-387"
fi
+MATH_FLAG="${MATH_FLAG} -ffp-contract=off -fno-math-errno -fno-trapping-math"
AC_SUBST(MATH_FLAG)
CFLAGS_hold=$CFLAGS
Index: libgo/go/math/all_test.go
===
--- libgo/go/math/all_test.go (revision 257306)
+++ libgo/go/math/all_test.go (working copy)
@@ -128,7 +128,7 @@
var ceil = []float64{
5.e+00,
8.e+00,
- 0.e+00,
+ Copysign(0, -1),
-5.e+00,
1.e+01,
3.e+00,
@@ -644,7 +644,7 @@
var trunc = []float64{
4.e+00,
7.e+00,
- -0.e+00,
+ Copysign(0, -1),
-5.e+00,
9.e+00,
2.e+00,
@@ -2134,7 +2134,7 @@
func TestCeil(t *testing.T) {
for i := 0; i < len(vf); i++ {
- if f := Ceil(vf[i]); ceil[i] != f {
+ if f := Ceil(vf[i]); !alike(ceil[i], f) {
t.Errorf("Ceil(%g) = %g, want %g", vf[i], f, ceil[i])
}
}
@@ -2361,7 +2361,7 @@
func TestFloor(t *testing.T) {
for i := 0; i < len(vf); i++ {
- if f := Floor(vf[i]); floor[i] != f {
+ if f := Floor(vf[i]); !alike(floor[i], f) {
t.Errorf("Floor(%g) = %g, want %g", vf[i], f, floor[i])
}
}
@@ -2884,7 +2884,7 @@
func TestTrunc(t *testing.T) {
for i := 0; i < len(vf); i++ {
- if f := Trunc(vf[i]); trunc[i] != f {
+ if f := Trunc(vf[i]); !alike(trunc[i], f) {
t.Errorf("Trunc(%g) = %g, want %g", vf[i], f, trunc[i])
}
}
Extend aligned_membuf<> usage
Hi
As we just bump version namespace it might be interesting to do the
following change now. What do you think ?
François
diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index 56b3ac5..05abd43 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -116,7 +116,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
_Fwd_list_node() = default;
+#if _GLIBCXX_INLINE_VERSION
+ __gnu_cxx::__aligned_membuf<_Tp> _M_storage;
+#else
__gnu_cxx::__aligned_buffer<_Tp> _M_storage;
+#endif
_Tp*
_M_valptr() noexcept
diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index 3ff6b14..222a1b2 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -231,7 +231,11 @@ namespace __detail
{
typedef _Value value_type;
+#if _GLIBCXX_INLINE_VERSION
+ __gnu_cxx::__aligned_membuf<_Value> _M_storage;
+#else
__gnu_cxx::__aligned_buffer<_Value> _M_storage;
+#endif
_Value*
_M_valptr() noexcept
@@ -1516,7 +1520,11 @@ namespace __detail
template::value>
struct _Hash_code_storage
{
+#if _GLIBCXX_INLINE_VERSION
+ __gnu_cxx::__aligned_membuf<_Tp> _M_storage;
+#else
__gnu_cxx::__aligned_buffer<_Tp> _M_storage;
+#endif
_Tp*
_M_h() { return _M_storage._M_ptr(); }
diff --git a/libstdc++-v3/include/bits/shared_ptr_base.h b/libstdc++-v3/include/bits/shared_ptr_base.h
index b58273a..8e00f1b 100644
--- a/libstdc++-v3/include/bits/shared_ptr_base.h
+++ b/libstdc++-v3/include/bits/shared_ptr_base.h
@@ -534,7 +534,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Alloc& _M_alloc() noexcept { return _A_base::_S_get(*this); }
+#if _GLIBCXX_INLINE_VERSION
+ __gnu_cxx::__aligned_membuf<_Tp> _M_storage;
+#else
__gnu_cxx::__aligned_buffer<_Tp> _M_storage;
+#endif
};
public:
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
The previous patch didn't resolve all the false positives
in the Linux kernel. The attached is an update that fixes
the remaining one having to do with multidimensional array
members:
struct S { char a[2][4]; };
void f (struct S *p, int i)
{
strcpy (p->a[0], "012");
strcpy (p->a[i] + 1, p->a[0]); // false positive here
}
In the process of fixing this I also made a couple of minor
restructuring changes to the builtin_memref constructor to
in order to make the code easier to follow: I broke it out
into a couple of helper functions and called those.
As with the first revision of the patch, this one is also
meant to be applied on top of
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html
Sorry about the late churn. Even though I tested the original
implementation with the Linux kernel the bugs were only exposed
non-default configurations that I didn't build.
Jakub, you had concerns about the code in the constructor
and about interpreting the offsets in the diagnostics.
I tried to address those in the patch. Please review
the changes and let me know if you have any further comments.
Thanks
Martin
On 01/30/2018 04:19 PM, Martin Sebor wrote:
Testing GCC 8 with recent Linux kernel sources has uncovered
a bug in the handling of arrays of arrays by the -Wrestrict
checker where it fails to take references to different array
elements into consideration, issuing false positives.
The attached patch corrects this mistake.
In addition, to make warnings involving excessive offset bounds
more meaningful (less confusing), I've made a cosmetic change
to constrain them to the bounds of the accessed object. I've
done this in response to multiple comments indicating that
the warnings are hard to interpret. This change is meant to
be applied on top of the patch for bug 83698 (submitted mainly
to improve the readability of the offsets):
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html
Martin
PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within array
gcc/ChangeLog:
PR middle-end/84095
* gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New.
(builtin_memref::set_base_and_offset): Same. Handle inner references.
(builtin_memref::builtin_memref): Factor out parts into
set_base_and_offset and call it.
gcc/testsuite/ChangeLog:
PR middle-end/84095
* c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings.
* c-c++-common/Wrestrict.c: Same.
* gcc.dg/Wrestrict-6.c: Same.
* gcc.dg/Warray-bounds-27.c: New test.
* gcc.dg/Wrestrict-8.c: New test.
* gcc.dg/Wrestrict-9.c: New test.
* gcc.dg/pr84095.c: New test.
diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c
index 528eb5b..367e05f 100644
--- a/gcc/gimple-ssa-warn-restrict.c
+++ b/gcc/gimple-ssa-warn-restrict.c
@@ -158,6 +158,14 @@ struct builtin_memref
builtin_memref (tree, tree);
tree offset_out_of_bounds (int, offset_int[2]) const;
+
+private:
+
+ /* Ctor helper to set or extend OFFRANGE based on argument. */
+ void extend_offset_range (tree);
+
+ /* Ctor helper to determine BASE and OFFRANGE from argument. */
+ void set_base_and_offset (tree);
};
/* Description of a memory access by a raw memory or string built-in
@@ -214,6 +222,7 @@ class builtin_access
bool (builtin_access::*detect_overlap) ();
};
+
/* Initialize a memory reference representation from a pointer EXPR and
a size SIZE in bytes. If SIZE is NULL_TREE then the size is assumed
to be unknown. */
@@ -235,11 +244,120 @@ builtin_memref::builtin_memref (tree expr, tree size)
const offset_int maxobjsize = tree_to_shwi (max_object_size ());
+ /* Find the BASE object or pointer referenced by EXPR and set
+ the offset range OFFRANGE in the process. */
+ set_base_and_offset (expr);
+
+ if (size)
+{
+ tree range[2];
+ /* Determine the size range, allowing for the result to be [0, 0]
+ for SIZE in the anti-range ~[0, N] where N >= PTRDIFF_MAX. */
+ get_size_range (size, range, true);
+ sizrange[0] = wi::to_offset (range[0]);
+ sizrange[1] = wi::to_offset (range[1]);
+ /* get_size_range returns SIZE_MAX for the maximum size.
+ Constrain it to the real maximum of PTRDIFF_MAX. */
+ if (sizrange[1] > maxobjsize)
+ sizrange[1] = maxobjsize;
+}
+ else
+sizrange[1] = maxobjsize;
+
+ tree basetype = TREE_TYPE (base);
+ if (DECL_P (base) && TREE_CODE (basetype) == ARRAY_TYPE)
+{
+ /* If the offset could be in range of the referenced object
+ constrain its bounds so neither exceeds those of the object. */
+ if (offrange[0] < 0 && offrange[1] > 0)
+ offrange[0] = 0;
+
+ offset_int maxoff = maxobjsize;
+ if (ref && array_at_struct_end_p (ref))
+ ; /* Use the maximum possible offset for last member arrays. */
+ else if (tree basesize = TYPE_SIZE_UNIT (basetype))
+ maxoff = wi::to_offset (basesize);
+
+ if (offrange[0] >= 0)
+ {
+ if (offrange[1] < 0)
+ offrange
Go patch committed: Turn on escape analysis by default
This patch by Cherry Zhang turns on escape analysis by default in the
Go frontend. It's late for this change but I'd like this work to get
out. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.
Ian
2018-02-01 Ian Lance Taylor
* lang.opt (fgo-optimize): Remove RejectNegative.
* go-c.h (go_enable_optimize): Update declaration to take value
argument.
* go-lang.c (go_langhook_handle_option): Pass value to
go_enable_optimize.
* gccgo.texi (Invoking gccgo): Update -fgo-optimize-allocs doc.
Index: gcc/go/gccgo.texi
===
--- gcc/go/gccgo.texi (revision 257312)
+++ gcc/go/gccgo.texi (working copy)
@@ -229,10 +229,10 @@ may be used. Or the checks may be remov
by default, but in the future may be off by default on systems that do
not require it.
-@item -fgo-optimize-allocs
-@cindex @option{-fgo-optimize-allocs}
-Use escape analysis to allocate objects on the stack rather than the
-heap when possible. In the future this may be the default.
+@item -fno-go-optimize-allocs
+@cindex @option{-fno-go-optimize-allocs}
+Disable escape analysis, which tries to allocate objects on the stack
+rather than the heap.
@item -fgo-debug-escape@var{n}
@cindex @option{-fgo-debug-escape}
Index: gcc/go/go-c.h
===
--- gcc/go/go-c.h (revision 257312)
+++ gcc/go/go-c.h (working copy)
@@ -29,7 +29,7 @@ class Backend;
interface. */
extern int go_enable_dump (const char*);
-extern int go_enable_optimize (const char*);
+extern int go_enable_optimize (const char*, int);
extern void go_add_search_path (const char*);
Index: gcc/go/go-lang.c
===
--- gcc/go/go-lang.c(revision 257312)
+++ gcc/go/go-lang.c(working copy)
@@ -194,7 +194,7 @@ static bool
go_langhook_handle_option (
size_t scode,
const char *arg,
-int value ATTRIBUTE_UNUSED,
+int value,
int kind ATTRIBUTE_UNUSED,
location_t loc ATTRIBUTE_UNUSED,
const struct cl_option_handlers *handlers ATTRIBUTE_UNUSED)
@@ -251,7 +251,7 @@ go_langhook_handle_option (
break;
case OPT_fgo_optimize_:
- ret = go_enable_optimize (arg) ? true : false;
+ ret = go_enable_optimize (arg, value) ? true : false;
break;
case OPT_fgo_pkgpath_:
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257312)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-023c3d4358d101c71ac1436065690eaec2ce138e
+e148068360699f24118950b728f23a5c98e1f85e
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/escape.cc
===
--- gcc/go/gofrontend/escape.cc (revision 257312)
+++ gcc/go/gofrontend/escape.cc (working copy)
@@ -825,7 +825,7 @@ Escape_note::parse_tag(std::string* tag)
// The -fgo-optimize-alloc flag activates this escape analysis.
-Go_optimize optimize_allocation_flag("allocs");
+Go_optimize optimize_allocation_flag("allocs", true);
// A helper function to compute whether a function name has a
// matching hash value.
Index: gcc/go/gofrontend/go-optimize.cc
===
--- gcc/go/gofrontend/go-optimize.cc(revision 257312)
+++ gcc/go/gofrontend/go-optimize.cc(working copy)
@@ -19,8 +19,8 @@ Go_optimize* optimizations;
// Create a new optimization.
-Go_optimize::Go_optimize(const char* name)
- : next_(optimizations), name_(name), is_enabled_(false)
+Go_optimize::Go_optimize(const char* name, bool enabled)
+ : next_(optimizations), name_(name), is_enabled_(enabled)
{
optimizations = this;
}
@@ -28,7 +28,7 @@ Go_optimize::Go_optimize(const char* nam
// Enable an optimization by name.
bool
-Go_optimize::enable_by_name(const char* name)
+Go_optimize::enable_by_name(const char* name, bool value)
{
bool is_all = strcmp(name, "all") == 0;
bool found = false;
@@ -36,18 +36,18 @@ Go_optimize::enable_by_name(const char*
{
if (is_all || strcmp(name, p->name_) == 0)
{
- p->is_enabled_ = true;
+ p->is_enabled_ = value;
found = true;
}
}
return found;
}
-// Enable an optimization. Return 1 if this is a real name, 0 if not.
+// Enable/disable an optimization. Return 1 if this is a real name, 0 if not.
GO_EXTERN_C
int
-go_enable_optimize(const char* name)
+go_enable_optimize(const char* name, int value)
{
- return Go_optimize::enable_by_name(name) ? 1 : 0;
+ return Go_optimize::enable_by_name(name, (bool)value) ? 1 : 0;
}
Index: gcc/go/gofrontend/go-optimize.h
===
--- gcc/go/gofrontend/go-optimize.h (revision 257312)
+++ gcc
libgo patch committed: scan register backing store on ia64
This libgo patch by James Clarke scans the register backing store on
ia64 when doing a Go garbage collection. On ia64, a separate stack is
used for saving/restoring register frames, occupying the other end of
the stack mapping. This must also be scanned for pointers into the
heap. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257319)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-e148068360699f24118950b728f23a5c98e1f85e
+5e8a91bf239c253d7b5c84bd2c1dd3ecb18980e9
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/go/runtime/runtime2.go
===
--- libgo/go/runtime/runtime2.go(revision 257312)
+++ libgo/go/runtime/runtime2.go(working copy)
@@ -409,11 +409,15 @@ type g struct {
// gcnextsegment: unused
// gcnextsp: current SP while executing a syscall
// gcinitialsp: g0: top of stack; others: start of stack memory
+ // gcnextsp2: current secondary stack pointer (if present)
+ // gcinitialsp2: start of secondary stack (if present)
gcstack uintptr
gcstacksize uintptr
gcnextsegment uintptr
gcnextsp uintptr
gcinitialsp unsafe.Pointer
+ gcnextsp2 uintptr
+ gcinitialsp2 unsafe.Pointer
// gcregs holds the register values while executing a syscall.
// This is set by getcontext and scanned by the garbage collector.
Index: libgo/runtime/proc.c
===
--- libgo/runtime/proc.c(revision 257312)
+++ libgo/runtime/proc.c(working copy)
@@ -308,6 +308,7 @@ runtime_mcall(FuncVal *fv)
// Ensure that all registers are on the stack for the garbage
// collector.
__builtin_unwind_init();
+ flush_registers_to_secondary_stack();
gp = g;
mp = gp->m;
@@ -322,6 +323,7 @@ runtime_mcall(FuncVal *fv)
// We have to point to an address on the stack that is
// below the saved registers.
gp->gcnextsp = (uintptr)(&afterregs);
+ gp->gcnextsp2 = (uintptr)(secondary_stack_pointer());
#endif
gp->fromgogo = false;
getcontext(ucontext_arg(&gp->context[0]));
@@ -500,6 +502,8 @@ runtime_mstart(void *arg)
// is the top of the stack, not the bottom.
gp->gcstacksize = 0;
gp->gcnextsp = (uintptr)(&arg);
+ gp->gcinitialsp2 = secondary_stack_pointer();
+ gp->gcnextsp2 = (uintptr)(gp->gcinitialsp2);
#endif
// Save the currently active context. This will return
@@ -576,6 +580,8 @@ setGContext(void)
gp->gcstack = 0;
gp->gcstacksize = 0;
gp->gcnextsp = (uintptr)(&val);
+ gp->gcinitialsp2 = secondary_stack_pointer();
+ gp->gcnextsp2 = (uintptr)(gp->gcinitialsp2);
#endif
getcontext(ucontext_arg(&gp->context[0]));
@@ -654,6 +660,7 @@ doentersyscall(uintptr pc, uintptr sp)
void *v;
g->gcnextsp = (uintptr)(&v);
+ g->gcnextsp2 = (uintptr)(secondary_stack_pointer());
}
#endif
@@ -694,6 +701,7 @@ doentersyscallblock(uintptr pc, uintptr
void *v;
g->gcnextsp = (uintptr)(&v);
+ g->gcnextsp2 = (uintptr)(secondary_stack_pointer());
}
#endif
@@ -756,6 +764,7 @@ runtime_malg(bool allocatestack, bool si
*ret_stacksize = (uintptr)stacksize;
newg->gcinitialsp = *ret_stack;
newg->gcstacksize = (uintptr)stacksize;
+ newg->gcinitialsp2 =
initial_secondary_stack_pointer(*ret_stack);
#endif
}
return newg;
@@ -807,6 +816,7 @@ resetNewG(G *newg, void **sp, uintptr *s
if(*spsize == 0)
runtime_throw("bad spsize in resetNewG");
newg->gcnextsp = (uintptr)(*sp);
+ newg->gcnextsp2 = (uintptr)(newg->gcinitialsp2);
#endif
}
Index: libgo/runtime/runtime.h
===
--- libgo/runtime/runtime.h (revision 257312)
+++ libgo/runtime/runtime.h (working copy)
@@ -437,6 +437,23 @@ void runtime_check(void)
// the stacks are allocated by the splitstack library.
extern uintptr runtime_stacks_sys;
+/*
+ * ia64's register file is spilled to a separate stack, the register backing
+ * store, on window overflow, and must also be scanned. This occupies the other
+ * end of the normal stack allocation, growing upwards.
+ * We also need to ensure all register windows are flushed to the backing
+ * store, as unlike SPARC, __builtin_unwind_init doesn't do this on ia64.
+ */
+#ifdef __ia64__
+# define secondary_stack_pointer() __builtin_ia64_bsp()
+# de
libgo patch committed: Enable escape analysis tests in reflect package
This patch by Cherry Zhang enables some tests in the reflect package
that depend on escape analysis being enabled by default. Bootstrapped
and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 257323)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-5e8a91bf239c253d7b5c84bd2c1dd3ecb18980e9
+b332ba2f0d0302eeb01a228c217928296cec56f6
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: libgo/go/reflect/all_test.go
===
--- libgo/go/reflect/all_test.go(revision 257312)
+++ libgo/go/reflect/all_test.go(working copy)
@@ -6162,9 +6162,6 @@ func TestPtrToMethods(t *testing.T) {
}
func TestMapAlloc(t *testing.T) {
- if runtime.Compiler == "gccgo" {
- t.Skip("skipping on gccgo until we have escape analysis")
- }
m := ValueOf(make(map[int]int, 10))
k := ValueOf(5)
v := ValueOf(7)
@@ -6195,9 +6192,6 @@ func TestMapAlloc(t *testing.T) {
}
func TestChanAlloc(t *testing.T) {
- if runtime.Compiler == "gccgo" {
- t.Skip("skipping on gccgo until we have escape analysis")
- }
// Note: for a chan int, the return Value must be allocated, so we
// use a chan *int instead.
c := ValueOf(make(chan *int, 1))
C++ PATCH for c++/84160, ICE with nested variadic lambda capture
In dealing with variadic capture, we've historically not bothered
trying to track the capture proxies and just dealt with the
COMPONENT_REFs directly. This was breaking the new DECL_CAPTURED_VAR
code. The simplest solution for now is to make an exception for this
case; this doesn't actually cause any problems because the primary use
of DECL_CAPTURED_VAR is looking up the value of a constant variable,
and a function parameter pack is never a constant variable.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 35c8dba74b3dac15cc440e060e2e7f829202e85c
Author: Jason Merrill
Date: Thu Feb 1 17:34:42 2018 -0500
PR c++/84160 - ICE with nested variadic capture.
* lambda.c (is_capture_proxy_with_ref): New.
(insert_capture_proxy): Don't set DECL_CAPTURED_VARIABLE from a
COMPONENT_REF.
* expr.c (mark_use): Use is_capture_proxy_with_ref.
* constexpr.c (potential_constant_expression_1): Likewise.
* semantics.c (process_outer_var_ref): Likewise.
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 1390405c416..171c389515a 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5369,7 +5369,7 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
case VAR_DECL:
if (DECL_HAS_VALUE_EXPR_P (t))
{
- if (now && is_normal_capture_proxy (t))
+ if (now && is_capture_proxy_with_ref (t))
{
/* -- in a lambda-expression, a reference to this or to a
variable with automatic storage duration defined outside that
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5f14e514638..a53f4fd9c03 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6901,6 +6901,7 @@ extern void insert_capture_proxy (tree);
extern void insert_pending_capture_proxies (void);
extern bool is_capture_proxy (tree);
extern bool is_normal_capture_proxy (tree);
+extern bool is_capture_proxy_with_ref (tree);
extern void register_capture_members (tree);
extern tree lambda_expr_this_capture(tree, bool);
extern void maybe_generic_this_capture (tree, tree);
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 2e679868970..b2c8cfaf88c 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -111,7 +111,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
{
case VAR_DECL:
case PARM_DECL:
- if (rvalue_p && is_normal_capture_proxy (expr))
+ if (rvalue_p && is_capture_proxy_with_ref (expr))
{
/* Look through capture by copy. */
tree cap = DECL_CAPTURED_VARIABLE (expr);
@@ -154,7 +154,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
{
/* Try to look through the reference. */
tree ref = TREE_OPERAND (expr, 0);
- if (rvalue_p && is_normal_capture_proxy (ref))
+ if (rvalue_p && is_capture_proxy_with_ref (ref))
{
/* Look through capture by reference. */
tree cap = DECL_CAPTURED_VARIABLE (ref);
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index e1caaef6fe9..ff8236ad316 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -290,13 +290,24 @@ is_normal_capture_proxy (tree decl)
return DECL_NORMAL_CAPTURE_P (val);
}
+/* Returns true iff DECL is a capture proxy for which we can use
+ DECL_CAPTURED_VARIABLE. In effect, this is a normal proxy other than a
+ nested capture of a function parameter pack. */
+
+bool
+is_capture_proxy_with_ref (tree var)
+{
+ return (is_normal_capture_proxy (var) && DECL_LANG_SPECIFIC (var)
+ && DECL_CAPTURED_VARIABLE (var));
+}
+
/* VAR is a capture proxy created by build_capture_proxy; add it to the
current function, which is the operator() for the appropriate lambda. */
void
insert_capture_proxy (tree var)
{
- if (is_normal_capture_proxy (var))
+ if (is_capture_proxy_with_ref (var))
{
tree cap = DECL_CAPTURED_VARIABLE (var);
if (CHECKING_P)
@@ -443,11 +454,20 @@ build_capture_proxy (tree member, tree init)
init = TREE_OPERAND (init, 0);
STRIP_NOPS (init);
}
- gcc_assert (VAR_P (init) || TREE_CODE (init) == PARM_DECL);
- while (is_normal_capture_proxy (init))
- init = DECL_CAPTURED_VARIABLE (init);
- retrofit_lang_decl (var);
- DECL_CAPTURED_VARIABLE (var) = init;
+
+ if (TREE_CODE (init) == COMPONENT_REF)
+ /* We're capturing a capture of a function parameter pack, and have
+ lost track of the original variable. It's not important to have
+ DECL_CAPTURED_VARIABLE in this case, since a function parameter pack
+ isn't a constant variable, so don't bother trying to set it. */;
+ else
+ {
+ gcc_assert (VAR_P (init) || TREE_CODE (init) == PARM_DECL);
+ while (is_normal_capture_proxy (init))
+ init = DECL_CAPTURED_VARIABLE (init);
+
