Re: [PATCH] [i386] Fix _mm256_zeroupper to notify LRA that vzeroupper will kill sse registers. [PR target/82735]

2021-05-27 Thread Uros Bizjak via Gcc-patches
On Thu, May 27, 2021 at 7:03 AM Hongtao Liu  wrote:
>
> Hi:
>   This is an updated patch which implements vzeroupper as call_insn
> which has a special vzeroupper ABI, also in this patch i reverted
> r11-7684, r10-6451, r10-3677 which seems to fix the same issue but in
> a different way.
>   Bootstrapped and regtested on x86_64-linux-gnux{-m32,} and
> x86_64-linux-gnux{-m32 \-march=cascadelake,-march=cascadelake}.
>   Also test the patch on SPEC2017 and eembc, no performance impact as 
> expected.
>   Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/82735
> * config/i386/i386-expand.c (ix86_expand_builtin): Remove
> assignment of cfun->machine->has_explicit_vzeroupper.
> * config/i386/i386-features.c
> (ix86_add_reg_usage_to_vzerouppers): Delete.
> (ix86_add_reg_usage_to_vzeroupper): Ditto.
> (rest_of_handle_insert_vzeroupper): Remove
> ix86_add_reg_usage_to_vzerouppers, add df_analyze at the end
> of the function.
> (gate): Remove cfun->machine->has_explicit_vzeroupper.
> * config/i386/i386-protos.h (ix86_expand_avx_vzeroupper):
> Declared.
> * config/i386/i386.c (ix86_insn_callee_abi): New function.
> (ix86_initialize_callee_abi): Ditto.
> (ix86_expand_avx_vzeroupper): Ditto.
> (ix86_hard_regno_call_part_clobbered): Adjust for vzeroupper
> ABI.
> (TARGET_INSN_CALLEE_ABI): Define as ix86_insn_callee_abi.
> * config/i386/i386.h (enum i386_insn_callee_abi_index): New.
> (struct GTY(()) machine_function): Delete
> has_explicit_vzeroupper.
> * config/i386/i386.md (enum unspec): New member
> UNSPEC_CALLEE_ABI.
> * config/i386/predicates.md (vzeroupper_pattern): Adjust.
> * config/i386/sse.md (avx_vzeroupper): Call
> ix86_expand_avx_vzeroupper.
> (*avx_vzeroupper): Rename to ..
> (avx_vzeroupper_callee_abi): .. this, and adjust pattern as
> call_insn which has a special vzeroupper ABI.
> (*avx_vzeroupper_1): Deleted.
> * df-scan.c (df_get_call_refs): When call_insn is a fake call,
> it won't use stack pointer reg.
> * final.c (leaf_function_p): When call_insn is a fake call, it
> won't affect caller as a leaf function.
> * reg-stack.c (callee_clobbers_any_stack_reg): New.
> (subst_stack_regs): When call_insn doesn't clobber any stack
> reg, don't clear the arguments.
> * rtl.c (shallow_copy_rtx): Don't clear flag used when orig is
> a insn.
> * shrink-wrap.c (requires_stack_frame_p): No need for stack
> frame for a fake call.
>
> gcc/testsuite/ChangeLog:
>
> PR target/82735
> * gcc.target/i386/pr82735-1.c: New test.
> * gcc.target/i386/pr82735-2.c: New test.
> * gcc.target/i386/pr82735-3.c: New test.
> * gcc.target/i386/pr82735-4.c: New test.
> * gcc.target/i386/pr82735-5.c: New test.

Please split the patch to middle-end and target part. The middle-end
should be approved first.

 (define_expand "avx_vzeroupper"
-  [(parallel [(unspec_volatile [(const_int 0)] UNSPECV_VZEROUPPER)])]
-  "TARGET_AVX")
+  [(parallel [(call (mem:QI (unspec_volatile [(const_int 0)]
UNSPECV_VZEROUPPER))
+(const_int 0))
+ (unspec [(const_int 1)] UNSPEC_CALLEE_ABI)])]

The call insn doesn't look like a valid RTX. Why not just:

+  [(parallel [(call (mem:QI (const_int 0)
+(const_int 0))

for a fake call? Also, UNSPEC_VZEROUPPER can be removed this way since
the const_int 1 of UNSPEC_CALLEE_ABI is now used to detect vzeroupper.

Also, you don't need the avx_vzeroupper pattern to just call
ix86_expand_avx_vzeroupper. Just call the function directly from the
call site:

case AVX_U128:
  if (mode == AVX_U128_CLEAN)
emit_insn (gen_avx_vzeroupper ());
  break;

+ (unspec [(const_int 1)] UNSPEC_CALLEE_ABI)])]

Can this const_int 1 be somehow more descriptive? Perhaps use
define_constant to define I386_VZEROUPPER ABI and use it in .md as
well as .c files.

Uros.


Re: [PATCH] Extend is_cond_scalar_reduction to handle nop_expr after/before scalar reduction.[PR98365]

2021-05-27 Thread Hongtao Liu via Gcc-patches
On Wed, May 26, 2021 at 8:41 PM Richard Biener
 wrote:
>
> On Wed, May 26, 2021 at 7:06 AM Hongtao Liu  wrote:
> >
> > On Tue, May 25, 2021 at 6:24 PM Richard Biener
> >  wrote:
> > >
> > > On Mon, May 24, 2021 at 11:52 AM Hongtao Liu  wrote:
> > > >
> > > > Hi:
> > > >   Details described in PR.
> > > >   Bootstrapped and regtest on
> > > > x86_64-linux-gnu{-m32,}/x86_64-linux-gnu{-m32\
> > > > -march=cascadelake,-march=cascadelake}
> > > >   Ok for trunk?
> > >
> > > +static tree
> > > +strip_nop_cond_scalar_reduction (bool has_nop, tree op)
> > > +{
> > > +  if (!has_nop)
> > > +return op;
> > > +
> > > +  if (TREE_CODE (op) != SSA_NAME)
> > > +return NULL_TREE;
> > > +
> > > +  gimple* stmt = SSA_NAME_DEF_STMT (op);
> > > +  if (!stmt
> > > +  || gimple_code (stmt) != GIMPLE_ASSIGN
> > > +  || gimple_has_volatile_ops (stmt)
> > > +  || gimple_assign_rhs_code (stmt) != NOP_EXPR)
> > > +return NULL_TREE;
> > > +
> > > +  return gimple_assign_rhs1 (stmt);
> > >
> > > this allows arbitrary conversions where the comment suggests you
> > > only want to allow conversions to the same precision but different sign.
> > > Sth like
> > >
> > >   gassign *stmt = safe_dyn_cast  (SSA_NAME_DEF_STMT (op));
> > >   if (!stmt
> > >   || !CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt))
> > >   || !tree_nop_conversion_p (TREE_TYPE (op), TREE_TYPE
> > > (gimple_assign_rhs1 (stmt
> > > return NULL_TREE;
> > >
Changed.
> > > +  if (gimple_bb (stmt) != gimple_bb (*nop_reduc)
> > > + || gimple_code (stmt) != GIMPLE_ASSIGN
> > > + || gimple_has_volatile_ops (stmt))
> > > +   return false;
> > >
> > > !is_gimple_assign (stmt) instead of gimple_code (stmt) != GIMPLE_ASSIGN
> > >
Changed.
> > > the gimple_has_volatile_ops check is superfluous given you restrict
> > > the assign code.
> > >
Changed.
> > > +  /* Check that R_NOP1 is used in nop_stmt or in PHI only.  */
> > > +  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, r_nop1)
> > > +   {
> > > + gimple *use_stmt = USE_STMT (use_p);
> > > + if (is_gimple_debug (use_stmt))
> > > +   continue;
> > > + if (use_stmt == SSA_NAME_DEF_STMT (r_op1))
> > > +   continue;
> > > + if (gimple_code (use_stmt) != GIMPLE_PHI)
> > > +   return false;
> > >
> > > can the last check be use_stmt == phi since we should have the
> > > PHI readily available?
> > >
Yes, changed.
> > > @@ -1735,6 +1822,23 @@ convert_scalar_cond_reduction (gimple *reduc,
> > > gimple_stmt_iterator *gsi,
> > >rhs = fold_build2 (gimple_assign_rhs_code (reduc),
> > >  TREE_TYPE (rhs1), op0, tmp);
> > >
> > > +  if (has_nop)
> > > +{
> > > +  /* Create assignment nop_rhs = op0 +/- _ifc_.  */
> > > +  tree nop_rhs = make_temp_ssa_name (TREE_TYPE (rhs1), NULL, 
> > > "_nop_");
> > > +  gimple* new_assign2 = gimple_build_assign (nop_rhs, rhs);
> > > +  gsi_insert_before (gsi, new_assign2, GSI_SAME_STMT);
> > > +  /* Rebuild rhs for nop_expr.  */
> > > +  rhs = fold_build1 (NOP_EXPR,
> > > +TREE_TYPE (gimple_assign_lhs (nop_reduc)),
> > > +nop_rhs);
> > > +
> > > +  /* Delete nop_reduc.  */
> > > +  stmt_it = gsi_for_stmt (nop_reduc);
> > > +  gsi_remove (&stmt_it, true);
> > > +  release_defs (nop_reduc);
> > > +}
> > > +
> > >
> > > hmm, the whole function could be cleaned up with sth like
> > >
> > >  /* Build rhs for unconditional increment/decrement.  */
> > >  gimple_seq stmts = NULL;
> > >  rhs = gimple_build (&stmts, gimple_assing_rhs_code (reduc),
> > > TREE_TYPE (rhs1), op0, tmp);
> > >  if (has_nop)
> > >rhs = gimple_convert (&stmts, TREE_TYPE (gimple_assign_lhs
> > > (nop_reduc)), rhs);
> > >  gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
> > >
> > > plus in the caller moving the
> > >
> > >   new_stmt = gimple_build_assign (res, rhs);
> > >   gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
> > >
> > > to the else branch as well as the folding done on new_stmt (maybe return
> > > new_stmt instead of rhs from convert_scalar_cond_reduction.
> > Eventually, we needed to assign rhs to res, and with an extra mov stmt
> > from rhs to res, the vectorizer failed.
> > the only difference in 166t.ifcvt between successfully vectorization
> > and failed vectorization is below
> >char * _24;
> >char _25;
> >unsigned char _ifc__29;
> > +  unsigned char _30;
> >
> > [local count: 118111600]:
> >if (n_10(D) != 0)
> > @@ -70,7 +71,8 @@ char foo2 (char * a, char * c, int n)
> >_5 = c_14(D) + _1;
> >_6 = *_5;
> >_ifc__29 = _3 == _6 ? 1 : 0;
> > -  cnt_7 = cnt_18 + _ifc__29;
> > +  _30 = cnt_18 + _ifc__29;
> > +  cnt_7 = _30;
> >i_16 = i_20 + 1;
> >if (n_10(D) != i_16)
> >  goto ; [89.00%]
> > @@ -110,7 +112,7 @@ char foo2 (char * a, char * c, int n)
> >goto ; [100.00

Re: [PATCH] [i386] Fix _mm256_zeroupper to notify LRA that vzeroupper will kill sse registers. [PR target/82735]

2021-05-27 Thread Jakub Jelinek via Gcc-patches
On Thu, May 27, 2021 at 01:07:09PM +0800, Hongtao Liu via Gcc-patches wrote:
> +  /* Flag used for call_insn indicates it's a fake call.  */
> +  RTX_FLAG (insn, used) = 1;

> +  /* CALL_INSN use "used" flag to indicate it's a fake call.  */
> +  if (i == STACK_POINTER_REGNUM
> +   && !RTX_FLAG (insn_info->insn, used))

> -   && ! SIBLING_CALL_P (insn))
> +   && ! SIBLING_CALL_P (insn)
> +   && !RTX_FLAG (insn, used))

> -  /* For all other RTXes clear the used flag on the copy.  */
> -  RTX_FLAG (copy, used) = 0;
> +  /* For all other RTXes clear the used flag on the copy.
> +  CALL_INSN use "used" flag to indicate it's a fake call.  */
> +  if (!INSN_P (orig))
> + RTX_FLAG (copy, used) = 0;
>break;
>  }
>return copy;
> @@ -57,7 +57,8 @@ requires_stack_frame_p (rtx_insn *insn, HARD_REG_SET 
> prologue_used,
>HARD_REG_SET hardregs;
>unsigned regno;
>  
> -  if (CALL_P (insn))
> +  /* CALL_INSN use "used" flag to indicate it's a fake call.  */
> +  if (CALL_P (insn) && !RTX_FLAG (insn, used))
>  return !SIBLING_CALL_P (insn);

Please define a macro for this in rtl.h (and mention it above used;
member too in a comment, see all the other comments in there), like:
/* 1 if RTX is a call_insn for a fake call.  */
#define FAKE_CALL_P(RTX)\
  (RTL_FLAG_CHECK1 ("FAKE_CALL_P", (RTX), CALL_INSN)->used)
Though, I'm also not sure if used can be actually used for this,
because it is used e.g. in emit-rtl.c for verification of RTL sharing.
Though, it seems no other rtl flag is free for CALL_INSN.
Could this fake call flag sit on the CALL rtx instead?

Jakub



Re: [PATCH 2/5] Convert Walloca pass to RANGE_QUERY(cfun).

2021-05-27 Thread Christophe Lyon via Gcc-patches
Hi,

On Tue, 25 May 2021 at 18:17, Aldy Hernandez via Gcc-patches
 wrote:
>
> Adjustments per discussion.
>
> OK pending tests?
>

The xfail removal causes failures on 32 bits platforms (eg arm, or
aarch64 with -mabi=ilp32):
FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 86)
FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 94)


Christophe

> Aldy


[PATCH] i386: Add uavg_ceil patterns for 4-byte vectors [PR100637]

2021-05-27 Thread Uros Bizjak via Gcc-patches
2021-05-27  Uroš Bizjak  

gcc/
PR target/100637
* config/i386/mmx.md (uavgv4qi3_ceil): New insn pattern.
(uavgv2hi3_ceil): Ditto.

gcc/testsuite/

PR target/100637
* gcc.target/i386/pr100637-3b.c (avgu): New test.
* gcc.target/i386/pr100637-3w.c (avgu): Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Pushed to master.

Uros.
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 453e8ea406d..23d88a4c265 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -3270,6 +3270,47 @@ (define_expand "uavg3_ceil"
   ix86_fixup_binary_operands_no_copy (PLUS, mode, operands);
 })
 
+(define_insn "uavgv4qi3_ceil"
+  [(set (match_operand:V4QI 0 "register_operand" "=x,Yw")
+   (truncate:V4QI
+ (lshiftrt:V4HI
+   (plus:V4HI
+ (plus:V4HI
+   (zero_extend:V4HI
+ (match_operand:V4QI 1 "register_operand" "%0,Yw"))
+   (zero_extend:V4HI
+ (match_operand:V4QI 2 "register_operand" "x,Yw")))
+ (const_vector:V4HI [(const_int 1) (const_int 1)
+ (const_int 1) (const_int 1)]))
+   (const_int 1]
+  "TARGET_SSE2"
+  "@
+   pavgb\t{%2, %0|%0, %2}
+   vpavgb\t{%2, %1, %0|%0, %1, %2}"
+  [(set_attr "isa" "noavx,avx")
+   (set_attr "type" "sseiadd")
+   (set_attr "mode" "TI")])
+
+(define_insn "uavgv2hi3_ceil"
+  [(set (match_operand:V2HI 0 "register_operand" "=x,Yw")
+   (truncate:V2HI
+ (lshiftrt:V2SI
+   (plus:V2SI
+ (plus:V2SI
+   (zero_extend:V2SI
+ (match_operand:V2HI 1 "register_operand" "%0,Yw"))
+   (zero_extend:V2SI
+ (match_operand:V2HI 2 "register_operand" "x,Yw")))
+ (const_vector:V2SI [(const_int 1) (const_int 1)]))
+   (const_int 1]
+  "TARGET_SSE2"
+  "@
+   pavgw\t{%2, %0|%0, %2}
+   vpavgw\t{%2, %1, %0|%0, %1, %2}"
+  [(set_attr "isa" "noavx,avx")
+   (set_attr "type" "sseiadd")
+   (set_attr "mode" "TI")])
+
 (define_insn "mmx_psadbw"
   [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
 (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
diff --git a/gcc/testsuite/gcc.target/i386/pr100637-3b.c 
b/gcc/testsuite/gcc.target/i386/pr100637-3b.c
index 16df70059a9..b17f8b8c19b 100644
--- a/gcc/testsuite/gcc.target/i386/pr100637-3b.c
+++ b/gcc/testsuite/gcc.target/i386/pr100637-3b.c
@@ -54,3 +54,13 @@ void _abs (void)
 }
 
 /* { dg-final { scan-assembler "pabsb" } } */
+
+void avgu (void)
+{
+  int i;
+
+  for (i = 0; i < 4; i++)
+ur[i] = (ua[i] + ub[i] + 1) >> 1;
+}
+
+/* { dg-final { scan-assembler "pavgb" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr100637-3w.c 
b/gcc/testsuite/gcc.target/i386/pr100637-3w.c
index 7f1882e7a56..b951f30f571 100644
--- a/gcc/testsuite/gcc.target/i386/pr100637-3w.c
+++ b/gcc/testsuite/gcc.target/i386/pr100637-3w.c
@@ -84,3 +84,13 @@ void _abs (void)
 }
 
 /* { dg-final { scan-assembler "pabsw" } } */
+
+void avgu (void)
+{
+  int i;
+
+  for (i = 0; i < 2; i++)
+ur[i] = (ua[i] + ub[i] + 1) >> 1;
+}
+
+/* { dg-final { scan-assembler "pavgw" } } */


Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Jakub Jelinek via Gcc-patches
On Wed, May 26, 2021 at 10:15:28PM +0200, Tobias Burnus wrote:
> @@ -15508,6 +15511,57 @@ c_parser_omp_iterators (c_parser *parser)
>return ret ? ret : error_mark_node;
>  }
>  
> +/* OpenMP 5.0:
> +   affinity ( [aff-modifier :] variable-list )
> +   aff-modifier:
> + iterator ( iterators-definition )  */
> +
> +static tree
> +c_parser_omp_clause_affinity (c_parser *parser, tree list)
> +{
> +  location_t clause_loc = c_parser_peek_token (parser)->location;
> +  tree nl, iterators = NULL_TREE;
> +
> +  matching_parens parens;
> +  if (!parens.require_open (parser))
> +return list;
> +
> +  if (c_parser_next_token_is (parser, CPP_NAME))
> +{
> +  const char *p = IDENTIFIER_POINTER (c_parser_peek_token 
> (parser)->value);
> +  if (strcmp ("iterator", p) == 0)
> + {
> +   iterators = c_parser_omp_iterators (parser);
> +   if (!c_parser_require (parser, CPP_COLON, "expected %<:%>"))
> + {
> +   if (iterators)
> + pop_scope ();
> +   parens.skip_until_found_close (parser);
> +   return list;
> + }
> + }

One important thing I've missed.
Unlike depend clause where dependence-type : is required, in affinity clause
the aff-modifier is optional.
So
affinity (iterator)
should be valid (provided iterator is a variable), similarly
affinity (iterator(1,2,3)[0])
(e.g. if iterator is a function returning pointer), etc.
This is something that should be tested in the testsuite coverage, and
during parsing what we likely want to do is if we see iterator as the first
CPP_NAME, check if next token is ( and if yes, tentatively search for
corresponding balanced ) and check if it is followed by : and only in that
case c_parser_omp_iterators etc.
As Marcel found, in the C FE one can use c_parser_peek_nth_token_raw
for it and we even have c_parser_check_balanced_raw_token_sequence.

> +  if (cp_lexer_next_token_is (parser->lexer, CPP_NAME))
> +{
> +  tree id = cp_lexer_peek_token (parser->lexer)->u.value;
> +  const char *p = IDENTIFIER_POINTER (id);
> +  if (strcmp ("iterator", p) == 0)
> + {
> +   begin_scope (sk_omp, NULL);
> +   iterators = cp_parser_omp_iterators (parser);
> +   if (!cp_parser_require (parser, CPP_COLON, RT_COLON))

In the C++ FE we have two options, do tentative parsing (provided
that cp_parser_omp_iterators or anything it calls doesn't emit premature
errors), or can use cp_parser_skip_balanced_tokens to skip over the
( ... ) after iterator without consuming tokens to see if it is followed
by :.

> @@ -996,6 +999,107 @@ gfc_match_omp_map_clause (gfc_omp_namelist **list, 
> gfc_omp_map_op map_op,
>return false;
>  }
>  
> +static match
> +gfc_match_iterator (gfc_namespace **ns)
> +{
> +  if (gfc_match ("iterator ( ") != MATCH_YES)
> +return MATCH_NO;
> +
> +  gfc_typespec ts;
> +  gfc_symbol *last = NULL;
> +  gfc_expr *begin, *end, *step;
> +  *ns = gfc_build_block_ns (gfc_current_ns);

What happens with this block ns:

> +  char name[GFC_MAX_SYMBOL_LEN + 1];
> +  while (true)
> +{
> +  locus old_loc = gfc_current_locus;
> +  if (gfc_match_type_spec (&ts) == MATCH_YES
> +   && gfc_match (" :: ") == MATCH_YES)
> + {
> +   if (ts.type != BT_INTEGER)
> + {
> +   gfc_error ("Expected INTEGER type at %L", &old_loc);

In the error handling?
Especially if it is tentative and user code just contains
affinity (iterator(4))
for say
integer :: iterator(20)
?

Jakub



Re: [PATCH 2/5] Convert Walloca pass to RANGE_QUERY(cfun).

2021-05-27 Thread Aldy Hernandez via Gcc-patches
On x86-32 warn_ptrdiff_anti_range_add() and warn_int_anti_range()
degrade to the same function so ICF is folding the latter into a call
into the former.  This is causing no warnings to be emitted for
warn_int_anti_range.

Fixed by passing -fno-ipa-icf to the test.

Long term, we really should be finding a way to run these warning
passes earlier :-/.

Pushed.

On Thu, May 27, 2021 at 9:39 AM Christophe Lyon
 wrote:
>
> Hi,
>
> On Tue, 25 May 2021 at 18:17, Aldy Hernandez via Gcc-patches
>  wrote:
> >
> > Adjustments per discussion.
> >
> > OK pending tests?
> >
>
> The xfail removal causes failures on 32 bits platforms (eg arm, or
> aarch64 with -mabi=ilp32):
> FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 86)
> FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 94)
>
>
> Christophe
>
> > Aldy
>
From 95bef94c6c6c6cb7bf640068aea77c209bca7c65 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez 
Date: Thu, 27 May 2021 09:32:42 +0200
Subject: [PATCH] Tweak Wstringop-overflow-55.c test.

On x86-32 warn_ptrdiff_anti_range_add() and warn_int_anti_range()
degrade to the same function so ICF is folding the latter into a call
into the former.  This is causing no warnings to be emitted for
warn_int_anti_range.

Fixed by passing -fno-ipa-icf.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wstringop-overflow-55.c: Pass -fno-ipa-icf.
---
 gcc/testsuite/gcc.dg/Wstringop-overflow-55.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
index 8df5cb629ae..c3c2dbe06dd 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-55.c
@@ -1,6 +1,6 @@
 /* Verify that offsets in "anti-ranges" are handled correctly.
{ dg-do compile }
-   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0 -fno-ipa-icf" } */
 
 typedef __PTRDIFF_TYPE__ ptrdiff_t;
 typedef __SIZE_TYPE__size_t;
-- 
2.31.1



Re: [r12-1078 Regression] FAIL: gcc.dg/Wstringop-overflow-55.c pr????? (test for warnings, line 94) on Linux/x86_64

2021-05-27 Thread Aldy Hernandez via Gcc-patches
Fixed in trunk.

On Thu, May 27, 2021 at 2:32 AM sunil.k.pandey  wrote:
>
> On Linux/x86_64,
>
> fe9a499cb8775cfbcea356ab0cae5c365971cf86 is the first bad commit
> commit fe9a499cb8775cfbcea356ab0cae5c365971cf86
> Author: Aldy Hernandez 
> Date:   Wed May 19 18:27:47 2021 +0200
>
> Convert Walloca pass to get_range_query.
>
> caused
>
> FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 86)
> FAIL: gcc.dg/Wstringop-overflow-55.c pr? (test for warnings, line 94)
>
> with GCC configured with
>
> ../../gcc/configure 
> --prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r12-1078/usr
>  --enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
> --with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
> --enable-libmpx x86_64-linux --disable-bootstrap
>
> To reproduce:
>
> $ cd {build_dir}/gcc && make check 
> RUNTESTFLAGS="dg.exp=gcc.dg/Wstringop-overflow-55.c 
> --target_board='unix{-m32}'"
> $ cd {build_dir}/gcc && make check 
> RUNTESTFLAGS="dg.exp=gcc.dg/Wstringop-overflow-55.c 
> --target_board='unix{-m32\ -march=cascadelake}'"
>
> (Please do not reply to this email, for question about this report, contact 
> me at skpgkp2 at gmail dot com)
>



[PATCH] Move global range code to value-query.cc.

2021-05-27 Thread Aldy Hernandez via Gcc-patches
As discussed...

This patch moves all the global range code from gimple-range.cc into
value-query.cc.  It also moves get_range_info and get_ptr_nonnull from
tree-ssanames.c into their only uses, and removes external access to them.

No functional changes.

Pushed.

gcc/ChangeLog:

* gimple-range.cc (get_range_global): Move to value-query.cc.
(gimple_range_global): Same.
(get_global_range_query): Same.
(global_range_query::range_of_expr): Same.
* gimple-range.h (class global_range_query): Move to
value-query.h.
(gimple_range_global): Same.
* tree-ssanames.c (get_range_info): Move to value-query.cc.
(get_ptr_nonnull): Same.
* tree-ssanames.h (get_range_info): Remove.
(get_ptr_nonnull): Remove.
* value-query.cc (get_ssa_name_range_info): Move from
tree-ssanames.c.
(get_ssa_name_ptr_info_nonnull): Same.
(get_range_global): Move from gimple-range.cc.
(gimple_range_global): Same.
(get_global_range_query): Same.
(global_range_query::range_of_expr): Same.
* value-query.h (class global_range_query): Move from
gimple-range.h.
(gimple_range_global): Same.
---
 gcc/gimple-range.cc | 103 ---
 gcc/gimple-range.h  |  11 
 gcc/tree-ssanames.c |  44 -
 gcc/tree-ssanames.h |   3 -
 gcc/value-query.cc  | 147 
 gcc/value-query.h   |  11 
 6 files changed, 158 insertions(+), 161 deletions(-)

diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index e351a841583..b4dfaa92168 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -1441,109 +1441,6 @@ trace_ranger::range_of_expr (irange &r, tree name, 
gimple *s)
   return trailer (idx, "range_of_expr", res, name, r);
 }
 
-// Return the legacy global range for NAME if it has one, otherwise
-// return VARYING.
-
-static void
-get_range_global (irange &r, tree name)
-{
-  tree type = TREE_TYPE (name);
-
-  if (SSA_NAME_IS_DEFAULT_DEF (name))
-{
-  tree sym = SSA_NAME_VAR (name);
-  // Adapted from vr_values::get_lattice_entry().
-  // Use a range from an SSA_NAME's available range.
-  if (TREE_CODE (sym) == PARM_DECL)
-   {
- // Try to use the "nonnull" attribute to create ~[0, 0]
- // anti-ranges for pointers.  Note that this is only valid with
- // default definitions of PARM_DECLs.
- if (POINTER_TYPE_P (type)
- && ((cfun && nonnull_arg_p (sym)) || get_ptr_nonnull (name)))
-   r.set_nonzero (type);
- else if (INTEGRAL_TYPE_P (type))
-   {
- get_range_info (name, r);
- if (r.undefined_p ())
-   r.set_varying (type);
-   }
- else
-   r.set_varying (type);
-   }
-  // If this is a local automatic with no definition, use undefined.
-  else if (TREE_CODE (sym) != RESULT_DECL)
-   r.set_undefined ();
-  else
-   r.set_varying (type);
-   }
-  else if (!POINTER_TYPE_P (type) && SSA_NAME_RANGE_INFO (name))
-{
-  get_range_info (name, r);
-  if (r.undefined_p ())
-   r.set_varying (type);
-}
-  else if (POINTER_TYPE_P (type) && SSA_NAME_PTR_INFO (name))
-{
-  if (get_ptr_nonnull (name))
-   r.set_nonzero (type);
-  else
-   r.set_varying (type);
-}
-  else
-r.set_varying (type);
-}
-
-// ?? Like above, but only for default definitions of NAME.  This is
-// so VRP passes using ranger do not start with known ranges,
-// otherwise we'd eliminate builtin_unreachables too early because of
-// inlining.
-//
-// Without this restriction, the test in g++.dg/tree-ssa/pr61034.C has
-// all of its unreachable calls removed too early.  We should
-// investigate whether we should just adjust the test above.
-
-value_range
-gimple_range_global (tree name)
-{
-  gcc_checking_assert (gimple_range_ssa_p (name));
-  tree type = TREE_TYPE (name);
-
-  if (SSA_NAME_IS_DEFAULT_DEF (name))
-{
-  value_range vr;
-  get_range_global (vr, name);
-  return vr;
-}
-  return value_range (type);
-}
-
-// --
-// global_range_query implementation.
-
-global_range_query global_ranges;
-
-// Like get_range_query, but for accessing global ranges.
-
-range_query *
-get_global_range_query ()
-{
-  return &global_ranges;
-}
-
-bool
-global_range_query::range_of_expr (irange &r, tree expr, gimple *)
-{
-  tree type = TREE_TYPE (expr);
-
-  if (!irange::supports_type_p (type) || !gimple_range_ssa_p (expr))
-return get_tree_range (r, expr);
-
-  get_range_global (r, expr);
-
-  return true;
-}
-
 gimple_ranger *
 enable_ranger (struct function *fun)
 {
diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h
index 23734c6e226..ecd332a3c54 100644
--- a/gcc/gimple-range.h
+++ b/gcc/gimple-range.h
@@ -252,17 +252,6 @@ private:
 // Flag to enable debugging the various internal Caches

[PUSHED] Use get_range_query in simplify_conversion_using_ranges.

2021-05-27 Thread Aldy Hernandez via Gcc-patches
Before the fix to the ranger dependency chain yesterday (commit
7f0cfeb1) I thought an ICE I was seeing was due to my get_range_query
patchet.  This was not the case, but this small change crept in while I
was debugging the failure.

I'm reverting the change as was approved.

Tested on x86-64 Linux.

Pushed.

gcc/ChangeLog:

* vr-values.c (simplify_conversion_using_ranges): Use
get_range_query instead of get_global_range_query.
---
 gcc/vr-values.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index d283108b7c2..3d0be8edb3b 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -3837,7 +3837,7 @@ simplify_conversion_using_ranges (gimple_stmt_iterator 
*gsi, gimple *stmt)
   value_range vr;
   if (!INTEGRAL_TYPE_P (TREE_TYPE (innerop)))
 return false;
-  get_global_range_query ()->range_of_expr (vr, innerop, stmt);
+  get_range_query (cfun)->range_of_expr (vr, innerop, stmt);
   if (vr.undefined_p () || vr.varying_p ())
 return false;
   innermin = widest_int::from (vr.lower_bound (), TYPE_SIGN (TREE_TYPE 
(innerop)));
-- 
2.31.1



[PATCH 1/2] Implement generic expression evaluator for range_query.

2021-05-27 Thread Aldy Hernandez via Gcc-patches
Right now, range_of_expr only works with constants, SSA names, and
pointers.  Anything else gets returned as VARYING.  This patch adds the
capability to deal with arbitrary expressions, inasmuch as these
tree codes are implemented in range-ops.cc.

This will give us the ability to ask for the range of any tree expression,
not just constants and SSA names, with range_of_expr().

This is a more generic implementation of determine_value_range in VRP.
A follow-up patch will remove all uses of it in favor of the
range_query API.

Tested on x86-64 Linux.

OK?

gcc/ChangeLog:

* function-tests.c (test_ranges): Call gimple_range_tests.
* gimple-range-gori.cc (gori_compute::expr_range_at_stmt): Use
get_global_range_query instead of get_tree_range.
* gimple-range.cc (fur_source::get_operand): Add argument to
get_tree_range.
(get_arith_expr_range): New.
(get_tree_range): Add gimple and range_query arguments.
Call get_arith_expr_range.
(gimple_ranger::range_of_expr): Add argument to get_tree_range.
Include gimple-range-tests.cc.
* gimple-range.h (get_tree_range): Add argument.
* selftest.h (gimple_range_tests): New.
* value-query.cc (global_range_query::range_of_expr): Add
argument to get_tree_range.
* vr-values.c (vr_values::range_of_expr): Same.
* gimple-range-tests.cc: New file.
---
 gcc/function-tests.c  |  5 +++
 gcc/gimple-range-gori.cc  |  2 +-
 gcc/gimple-range-tests.cc | 72 +++
 gcc/gimple-range.cc   | 50 ---
 gcc/gimple-range.h|  2 +-
 gcc/selftest.h|  1 +
 gcc/value-query.cc|  4 +--
 gcc/vr-values.c   |  2 +-
 8 files changed, 129 insertions(+), 9 deletions(-)
 create mode 100644 gcc/gimple-range-tests.cc

diff --git a/gcc/function-tests.c b/gcc/function-tests.c
index 1b8f665cde1..0ac1d37f8ff 100644
--- a/gcc/function-tests.c
+++ b/gcc/function-tests.c
@@ -581,6 +581,11 @@ test_ranges ()
   push_cfun (fun);
   range_tests ();
   range_op_tests ();
+
+  build_cfg (fndecl);
+  convert_to_ssa (fndecl);
+  gimple_range_tests ();
+
   pop_cfun ();
 }
 
diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc
index c51e6ce0697..12178e2ed90 100644
--- a/gcc/gimple-range-gori.cc
+++ b/gcc/gimple-range-gori.cc
@@ -589,7 +589,7 @@ gori_compute::expr_range_at_stmt (irange &r, tree expr, 
gimple *s)
   if (gimple_range_ssa_p (expr))
 ssa_range_in_bb (r, expr, gimple_bb (s));
   else
-get_tree_range (r, expr);
+get_global_range_query ()->range_of_expr (r, expr);
 }
 
 // Calculate the range for NAME if the lhs of statement S has the
diff --git a/gcc/gimple-range-tests.cc b/gcc/gimple-range-tests.cc
new file mode 100644
index 000..9ee4c5aa6cd
--- /dev/null
+++ b/gcc/gimple-range-tests.cc
@@ -0,0 +1,72 @@
+/* Unit tests for GIMPLE range related routines.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#if CHECKING_P
+
+#include "selftest.h"
+
+namespace selftest {
+
+// Test ranges of tree expressions.
+class test_expr_eval : public gimple_ranger
+{
+public:
+  test_expr_eval ()
+  {
+type = integer_type_node;
+op0 = make_ssa_name (type);
+op1 = make_ssa_name (type);
+
+// [5,10] + [15,20] => [20, 30]
+tree expr = fold_build2 (PLUS_EXPR, type, op0, op1);
+int_range<2> expect (build_int_cst (type, 20), build_int_cst (type, 30));
+int_range_max r;
+
+ASSERT_TRUE (range_of_expr (r, expr));
+ASSERT_TRUE (r == expect);
+  }
+
+  virtual bool range_of_expr (irange &r, tree expr, gimple * = NULL) OVERRIDE
+  {
+if (expr == op0)
+  {
+   r.set (build_int_cst (type, 5), build_int_cst (type, 10));
+   return true;
+  }
+if (expr == op1)
+  {
+   r.set (build_int_cst (type, 15), build_int_cst (type, 20));
+   return true;
+  }
+return gimple_ranger::range_of_expr (r, expr);
+  }
+
+private:
+  tree op0, op1, type;
+};
+
+void
+gimple_range_tests ()
+{
+  test_expr_eval e;
+}
+
+} // namespace selftest
+
+#endif // CHECKING_P
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index b4dfaa92168..434b82b63b6 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -55,7 +55,7 @@ bool
 fur_source::get_operand (irange &r, tree expr)
 {
   if (!gimple_range_ss

[PATCH 2/2] Replace uses of determine_value_range with range_of_expr.

2021-05-27 Thread Aldy Hernandez via Gcc-patches
The expression evaluator changes to the range_query API provide
everything determine_value_range does.  This patch replaces all uses
with calls into the range_query API.

Tested on x86-64 Linux.

OK?

gcc/ChangeLog:

* calls.c (get_size_range): Use range_of_expr instead of
determine_value_range.
* tree-affine.c (expr_to_aff_combination): Same.
* tree-data-ref.c (split_constant_offset): Same.
* tree-vrp.c (determine_value_range_1): Remove.
(determine_value_range): Remove.
* tree-vrp.h (determine_value_range): Remove.
---
 gcc/calls.c | 21 +
 gcc/tree-affine.c   |  7 +-
 gcc/tree-data-ref.c | 12 +-
 gcc/tree-vrp.c  | 56 -
 gcc/tree-vrp.h  |  1 -
 5 files changed, 23 insertions(+), 74 deletions(-)

diff --git a/gcc/calls.c b/gcc/calls.c
index dd8ff2aa7cb..a7c78ed9c16 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1276,19 +1276,20 @@ get_size_range (range_query *query, tree exp, gimple 
*stmt, tree range[2],
   wide_int min, max;
   enum value_range_kind range_type;
 
+  if (!query)
+query = get_global_range_query ();
+
   if (integral)
 {
   value_range vr;
-  if (query && query->range_of_expr (vr, exp, stmt))
-   {
- if (vr.undefined_p ())
-   vr.set_varying (TREE_TYPE (exp));
- range_type = vr.kind ();
- min = wi::to_wide (vr.min ());
- max = wi::to_wide (vr.max ());
-   }
-  else
-   range_type = determine_value_range (exp, &min, &max);
+
+  query->range_of_expr (vr, exp, stmt);
+
+  if (vr.undefined_p ())
+   vr.set_varying (TREE_TYPE (exp));
+  range_type = vr.kind ();
+  min = wi::to_wide (vr.min ());
+  max = wi::to_wide (vr.max ());
 }
   else
 range_type = VR_VARYING;
diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index b273adb173d..a65719def23 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "dumpfile.h"
 #include "cfgexpand.h"
+#include "value-query.h"
 
 /* Extends CST as appropriate for the affine combinations COMB.  */
 
@@ -345,11 +346,15 @@ expr_to_aff_combination (aff_tree *comb, tree_code code, 
tree type,
   for below case:
 (T1)(X *+- CST) -> (T1)X *+- (T1)CST
   if X *+- CST doesn't overflow by range information.  */
+   value_range vr;
if (TYPE_UNSIGNED (itype)
&& TYPE_OVERFLOW_WRAPS (itype)
&& TREE_CODE (op1) == INTEGER_CST
-   && determine_value_range (op0, &minv, &maxv) == VR_RANGE)
+   && get_range_query (cfun)->range_of_expr (vr, op0)
+   && vr.kind () == VR_RANGE)
  {
+   wide_int minv = vr.lower_bound ();
+   wide_int maxv = vr.upper_bound ();
wi::overflow_type overflow = wi::OVF_NONE;
signop sign = UNSIGNED;
if (icode == PLUS_EXPR)
diff --git a/gcc/tree-data-ref.c b/gcc/tree-data-ref.c
index 09d46671565..b1f64684840 100644
--- a/gcc/tree-data-ref.c
+++ b/gcc/tree-data-ref.c
@@ -1069,12 +1069,12 @@ split_constant_offset (tree exp, tree *var, tree *off, 
value_range *exp_range,
   if (INTEGRAL_TYPE_P (type))
 *var = fold_convert (sizetype, *var);
   *off = ssize_int (0);
-  if (exp_range && code != SSA_NAME)
-{
-  wide_int var_min, var_max;
-  if (determine_value_range (exp, &var_min, &var_max) == VR_RANGE)
-   *exp_range = value_range (type, var_min, var_max);
-}
+
+  value_range r;
+  if (exp_range && code != SSA_NAME
+  && get_range_query (cfun)->range_of_expr (r, exp)
+  && !r.undefined_p ())
+*exp_range = r;
 }
 
 /* Expresses EXP as VAR + OFF, where OFF is a constant.  VAR has the same
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 450926d5f9b..b9c0e65bd98 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -4606,59 +4606,3 @@ make_pass_vrp (gcc::context *ctxt)
 {
   return new pass_vrp (ctxt);
 }
-
-
-/* Worker for determine_value_range.  */
-
-static void
-determine_value_range_1 (value_range *vr, tree expr)
-{
-  if (BINARY_CLASS_P (expr))
-{
-  value_range vr0, vr1;
-  determine_value_range_1 (&vr0, TREE_OPERAND (expr, 0));
-  determine_value_range_1 (&vr1, TREE_OPERAND (expr, 1));
-  range_fold_binary_expr (vr, TREE_CODE (expr), TREE_TYPE (expr),
- &vr0, &vr1);
-}
-  else if (UNARY_CLASS_P (expr))
-{
-  value_range vr0;
-  determine_value_range_1 (&vr0, TREE_OPERAND (expr, 0));
-  range_fold_unary_expr (vr, TREE_CODE (expr), TREE_TYPE (expr),
-&vr0, TREE_TYPE (TREE_OPERAND (expr, 0)));
-}
-  else if (TREE_CODE (expr) == INTEGER_CST)
-vr->set (expr);
-  else
-{
-  value_range r;
-  /* For SSA names try to extract range info computed by VRP.  Otherwise
- 

[committed] arm: Remove use of opts_set in arm_configure_build_target [PR100767]

2021-05-27 Thread Richard Earnshaw via Gcc-patches

The variable global_options_set is a reflection of which options have
been explicitly set from the command line in the structure
global_options.  But it doesn't describe the contents of a
cl_target_option.  cl_target_option is a set of options to apply and
once configured should represent a viable set of options without
needing to know which were explicitly set by the user.

Unfortunately arm_configure_build_target was incorrectly conflating
the two.  Fortunately, however, we do not really need to know this
since the various override_options functions should have sanitized the
target_options values before constructing a cl_target_option
structure.  It is safe, therefore, to simply drop this parameter to
arm_configure_build_target and rely on checking that various string
parameters are non-null before dereferencing them.

gcc:

PR target/100767
* config/arm/arm.c (arm_configure_build_target): Remove parameter
opts_set, directly check opts parameters for being non-null.
(arm_option_restore): Update call to arm_configure_build_target.
(arm_option_override): Likewise.
(arm_can_inline_p): Likewise.
(arm_valid_target_attribute_tree): Likewise.
* config/arm/arm-c.c (arm_pragma_target_parse): Likewise.
* config/arm/arm-protos.h (arm_configure_build_target): Adjust
prototype.
---
 gcc/config/arm/arm-c.c  |  3 +--
 gcc/config/arm/arm-protos.h |  3 +--
 gcc/config/arm/arm.c| 23 ++-
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c
index 7f97b843f92..ae2139c4bfa 100644
--- a/gcc/config/arm/arm-c.c
+++ b/gcc/config/arm/arm-c.c
@@ -408,8 +408,7 @@ arm_pragma_target_parse (tree args, tree pop_target)
target_option_current_node, but not handle_pragma_target.  */
   target_option_current_node = cur_tree;
   arm_configure_build_target (&arm_active_target,
-  TREE_TARGET_OPTION (cur_tree),
-  &global_options_set, false);
+  TREE_TARGET_OPTION (cur_tree), false);
 }
 
   /* Update macros if target_node changes. The global state will be restored
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index ffccaa77377..9b1f61394ad 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -243,8 +243,7 @@ extern bool arm_change_mode_p (tree);
 extern tree arm_valid_target_attribute_tree (tree, struct gcc_options *,
 	 struct gcc_options *);
 extern void arm_configure_build_target (struct arm_build_target *,
-	struct cl_target_option *,
-	struct gcc_options *, bool);
+	struct cl_target_option *, bool);
 extern void arm_option_reconfigure_globals (void);
 extern void arm_options_perform_arch_sanity_checks (void);
 extern void arm_pr_long_calls (struct cpp_reader *);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 9377aaef342..7b37e1b602c 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3054,9 +3054,10 @@ arm_override_options_after_change (void)
 /* Implement TARGET_OPTION_RESTORE.  */
 static void
 arm_option_restore (struct gcc_options */* opts */,
-		struct gcc_options *opts_set, struct cl_target_option *ptr)
+		struct gcc_options */* opts_set */,
+		struct cl_target_option *ptr)
 {
-  arm_configure_build_target (&arm_active_target, ptr, opts_set, false);
+  arm_configure_build_target (&arm_active_target, ptr, false);
 }
 
 /* Reset options between modes that the user has specified.  */
@@ -3179,7 +3180,6 @@ static sbitmap isa_quirkbits;
 void
 arm_configure_build_target (struct arm_build_target *target,
 			struct cl_target_option *opts,
-			struct gcc_options *opts_set,
 			bool warn_compatible)
 {
   const cpu_option *arm_selected_tune = NULL;
@@ -3194,7 +3194,7 @@ arm_configure_build_target (struct arm_build_target *target,
   target->core_name = NULL;
   target->arch_name = NULL;
 
-  if (opts_set->x_arm_arch_string)
+  if (opts->x_arm_arch_string)
 {
   arm_selected_arch = arm_parse_arch_option_name (all_architectures,
 		  "-march",
@@ -3202,7 +3202,7 @@ arm_configure_build_target (struct arm_build_target *target,
   arch_opts = strchr (opts->x_arm_arch_string, '+');
 }
 
-  if (opts_set->x_arm_cpu_string)
+  if (opts->x_arm_cpu_string)
 {
   arm_selected_cpu = arm_parse_cpu_option_name (all_cores, "-mcpu",
 		opts->x_arm_cpu_string);
@@ -3212,7 +3212,7 @@ arm_configure_build_target (struct arm_build_target *target,
 	 options for tuning.  */
 }
 
-  if (opts_set->x_arm_tune_string)
+  if (opts->x_arm_tune_string)
 {
   arm_selected_tune = arm_parse_cpu_option_name (all_cores, "-mtune",
 		 opts->x_arm_tune_string);
@@ -3476,8 +3476,7 @@ arm_option_override (void)
 }
 
   cl_target_option_save (&opts, &global_options, &global_options_set);
-  arm_configure_build_target (&arm_active_target, &opts, &global_options_set,
-			  true);
+  arm_conf

Re: [PATCH] [i386] Fix _mm256_zeroupper to notify LRA that vzeroupper will kill sse registers. [PR target/82735]

2021-05-27 Thread Richard Sandiford via Gcc-patches
Jakub Jelinek  writes:
> On Thu, May 27, 2021 at 01:07:09PM +0800, Hongtao Liu via Gcc-patches wrote:
>> +  /* Flag used for call_insn indicates it's a fake call.  */
>> +  RTX_FLAG (insn, used) = 1;
>
>> +  /* CALL_INSN use "used" flag to indicate it's a fake call.  */
>> +  if (i == STACK_POINTER_REGNUM
>> +  && !RTX_FLAG (insn_info->insn, used))
>
>> -  && ! SIBLING_CALL_P (insn))
>> +  && ! SIBLING_CALL_P (insn)
>> +  && !RTX_FLAG (insn, used))
>
>> -  /* For all other RTXes clear the used flag on the copy.  */
>> -  RTX_FLAG (copy, used) = 0;
>> +  /* For all other RTXes clear the used flag on the copy.
>> + CALL_INSN use "used" flag to indicate it's a fake call.  */
>> +  if (!INSN_P (orig))
>> +RTX_FLAG (copy, used) = 0;
>>break;
>>  }
>>return copy;
>> @@ -57,7 +57,8 @@ requires_stack_frame_p (rtx_insn *insn, HARD_REG_SET 
>> prologue_used,
>>HARD_REG_SET hardregs;
>>unsigned regno;
>>  
>> -  if (CALL_P (insn))
>> +  /* CALL_INSN use "used" flag to indicate it's a fake call.  */
>> +  if (CALL_P (insn) && !RTX_FLAG (insn, used))
>>  return !SIBLING_CALL_P (insn);
>
> Please define a macro for this in rtl.h (and mention it above used;
> member too in a comment, see all the other comments in there), like:
> /* 1 if RTX is a call_insn for a fake call.  */
> #define FAKE_CALL_P(RTX)  \
>   (RTL_FLAG_CHECK1 ("FAKE_CALL_P", (RTX), CALL_INSN)->used)
> Though, I'm also not sure if used can be actually used for this,
> because it is used e.g. in emit-rtl.c for verification of RTL sharing.

I thought it should be OK, since:

- copy_rtx_if_shared_1 and mark_used_flags do nothing for insns
- verify_rtx_sharing is only called for parts of an insn, rather than
  an insn itself

I guess an alternative would be to add a new rtx_code for fake call
insns and use CALL_P to test for both.  However, that would lose the
property that the default behaviour is conservatively correct
(even for direct checks of CALL_INSN), so the flag IMO seems better.

Thanks,
Richard

> Though, it seems no other rtl flag is free for CALL_INSN.
> Could this fake call flag sit on the CALL rtx instead?
>
>   Jakub


Re: [PATCH 0/11] warning control by group and location (PR 74765)

2021-05-27 Thread Richard Sandiford via Gcc-patches
Thanks for doing this.

Martin Sebor via Gcc-patches  writes:
> […]
> On 5/24/21 5:08 PM, David Malcolm wrote:
>> On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:
>>> Subsequent patches then replace invocations of the TREE_NO_WARNING()
>>> macro and the gimple_no_warning_p() and gimple_set_no_warning()
>>> functions throughout GCC with those and remove the legacy APIs to
>>> keep them from being accidentally reintroduced along with the
>>> problem.
>>> These are mostly mechanical changes, except that most of the new
>>> invocations also specify the option whose disposition to query for
>>> the expression or location, or which to enable or disable[2].
>>> The last function, copy_no_warning(), copies the disposition from
>>> one expression or location to another.
>>>
>>> A couple of design choices might be helpful to explain:
>>>
>>> First, introducing "warning groups" rather than controlling each
>>> individual warning is motivated by a) the fact that the latter
>>> would make avoiding redundant warnings for related problems
>>> cumbersome (e.g., after issuing a -Warray-bounds we want to
>>> suppress -Wstringop-overflow as well -Wstringop-overread for
>>> the same access and vice versa), and b) simplicity and efficiency
>>> of the implementation (mapping each option would require a more
>>> complex data structure like a bitmap).
>>>
>>> Second, using location_t to associate expressions/statements with
>>> the warning groups also turns out to be more useful in practice
>>> than a direct association between a tree or gimple*, and also
>>> simplifies managing the data structure.  Trees and gimple* come
>>> and go across passes, and maintaining a mapping for them that
>>> accounts for the possibility of them being garbage-collected
>>> and the same addresses reused is less than straightforward.
>> 
>> I find some of the terminology rather awkard due to it the negation
>> we're already using with TREE_NO_WARNING, in that we're turning on a
>> no_warning flag, and that this is a per-location/expr/stmt thing that's
>> different from the idea of enabling/disabling a specific warning
>> altogether (and the pragmas that control that).   Sometimes the patches
>> refer to enabling/disabling warnings and I think I want "enabling" and
>> "disabling" to mean the thing the user does with -Wfoo and -Wno-foo.
>> 
>> Would calling it a "warning suppression" or somesuch be more or less
>> klunky?
>
> I like warning suppression :)  But I'm not sure where you suggest
> I use the phrase.
>
> I don't particularly care for the "no" in the API names either
> (existing or new) and would prefer a positive form.  I considered
> enable_warning() and warning_enabled() but I chose the names I did
> because they're close to their established gimple namesakes.  I'm
> fine changing them to the alternatives, or if you or someone else
> has a preference for different names I'm open to suggestions.  Let
> me know.

Not my area, but FWIW, +1 for David's suggestion of
s/set_no_warning/suppress_warning/ or similar.  As well as the
problem with the double negative in negated conditions, I always have to
remember whether TREE_NO_WARNING means that hasn't been anything to warn
about yet or whether we shouldn't warn in future.

Richard


[PATCH] arm: Auto-vectorization for MVE: vabs

2021-05-27 Thread Christophe Lyon via Gcc-patches
This patch adds support for auto-vectorization of absolute value
computation using vabs.

We use a similar pattern to what is used in neon.md and extend the
existing neg2 expander to match both 'neg' and 'abs'.  This
implies renaming the existing abs2 define_insn in neon.md to
avoid a clash with the new expander with the same name.

2021-05-26  Christophe Lyon  

gcc/
* config/arm/mve.md (mve_vabsq_f): Use 'abs' instead of unspec.
(mve_vabsq_s): Likewise.
* config/arm/neon.md (abs2): Rename to neon_abs2.
* config/arm/unspecs.md (VABSQ_F, VABSQ_S): Delete.
* config/arm/vec-common.md (neg2): Rename to
2.

gcc/testsuite/
* gcc.target/arm/simd/mve-vabs.c: New test.
---
 gcc/config/arm/mve.md|  6 +--
 gcc/config/arm/neon.md   |  2 +-
 gcc/config/arm/unspecs.md|  2 -
 gcc/config/arm/vec-common.md |  4 +-
 gcc/testsuite/gcc.target/arm/simd/mve-vabs.c | 44 
 5 files changed, 49 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/simd/mve-vabs.c

diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
index 0a6ba80c99d..0bfa6a91d55 100644
--- a/gcc/config/arm/mve.md
+++ b/gcc/config/arm/mve.md
@@ -269,8 +269,7 @@ (define_insn "mve_vdupq_n_f"
 (define_insn "mve_vabsq_f"
   [
(set (match_operand:MVE_0 0 "s_register_operand" "=w")
-   (unspec:MVE_0 [(match_operand:MVE_0 1 "s_register_operand" "w")]
-VABSQ_F))
+   (abs:MVE_0 (match_operand:MVE_0 1 "s_register_operand" "w")))
   ]
   "TARGET_HAVE_MVE && TARGET_HAVE_MVE_FLOAT"
   "vabs.f%#  %q0, %q1"
@@ -481,8 +480,7 @@ (define_insn "@mve_vaddvq_"
 (define_insn "mve_vabsq_s"
   [
(set (match_operand:MVE_2 0 "s_register_operand" "=w")
-   (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" "w")]
-VABSQ_S))
+   (abs:MVE_2 (match_operand:MVE_2 1 "s_register_operand" "w")))
   ]
   "TARGET_HAVE_MVE"
   "vabs.s%#\t%q0, %q1"
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 6a6573317cf..077c62ffd20 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -739,7 +739,7 @@ (define_insn "one_cmpl2_neon"
   [(set_attr "type" "neon_move")]
 )
 
-(define_insn "abs2"
+(define_insn "neon_abs2"
   [(set (match_operand:VDQW 0 "s_register_operand" "=w")
(abs:VDQW (match_operand:VDQW 1 "s_register_operand" "w")))]
   "TARGET_NEON"
diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
index 0778db1bf4f..ed1bc293b78 100644
--- a/gcc/config/arm/unspecs.md
+++ b/gcc/config/arm/unspecs.md
@@ -538,7 +538,6 @@ (define_c_enum "unspec" [
   VRNDAQ_F
   VREV64Q_F
   VDUPQ_N_F
-  VABSQ_F
   VREV32Q_F
   VCVTTQ_F32_F16
   VCVTBQ_F32_F16
@@ -562,7 +561,6 @@ (define_c_enum "unspec" [
   VCLSQ_S
   VADDVQ_S
   VADDVQ_U
-  VABSQ_S
   VREV32Q_U
   VREV32Q_S
   VMOVLTQ_U
diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-common.md
index 8e35151da46..80b273229f5 100644
--- a/gcc/config/arm/vec-common.md
+++ b/gcc/config/arm/vec-common.md
@@ -208,9 +208,9 @@ (define_expand "one_cmpl2"
   "ARM_HAVE__ARITH && !TARGET_REALLY_IWMMXT"
 )
 
-(define_expand "neg2"
+(define_expand "2"
   [(set (match_operand:VDQWH 0 "s_register_operand" "")
-   (neg:VDQWH (match_operand:VDQWH 1 "s_register_operand" "")))]
+   (ABSNEG:VDQWH (match_operand:VDQWH 1 "s_register_operand" "")))]
   "ARM_HAVE__ARITH && !TARGET_REALLY_IWMMXT"
 )
 
diff --git a/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c 
b/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c
new file mode 100644
index 000..64cd1c2eb4a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c
@@ -0,0 +1,44 @@
+/* { dg-do assemble } */
+/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+/* { dg-additional-options "-O3 -funsafe-math-optimizations" } */
+
+#include 
+#include 
+
+#define ABS(a) ((a < 0) ? -a : a)
+
+#define FUNC(SIGN, TYPE, BITS, NB, NAME)   \
+  void test_ ## NAME ##_ ## SIGN ## BITS ## x ## NB (TYPE##BITS##_t * 
__restrict__ dest, TYPE##BITS##_t *a) { \
+int i; \
+for (i=0; i

Re: [PATCH] Modula-2 into the GCC tree on master

2021-05-27 Thread Matthias Klose
On 1/18/21 2:55 PM, Gaius Mulley via Gcc-patches wrote:
> Richard Biener  writes:
>> I've just done ./configure --enable-languages=m2; make -j24
>>
>> I would suggest to not rush this in now during stage4
>> but instead take the opportunity of this "quiet" phase
>> to prepare an integration branch with all the issues above
>> sorted out which we can merge at the beginning of stage1
>> for GCC 12 (or later during stage4 if everyone is happy
>> and/or backport for GCC 11.2 when it landed in trunk).
> 
> ok sure - this sounds a good plan

Gaius, now with the 1.1 relase out of the door, please could you clarify about
your plans getting this into trunk, and do you plan to get this into 11.2 as 
well?

Thanks, Matthias


Re: RFA: Fix match_scratch bug in define_subst

2021-05-27 Thread Richard Sandiford via Gcc-patches
Joern Rennecke  writes:
> Bootstrapped on x86_64-pc-linux-gnu.
>
> 2020-12-10  Joern Rennecke  
>
> Fix bug in the define_subst handling that made match_scratch unusable for
> multi-alternative patterns.

OK, and sorry for the slow response.

The changelog won't pass, but I'll leave you to negotiate that with the
commit hook. :-)

Thanks,
Richard

>
> diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> index e1ca06dbc1e..4022c661adb 100644
> --- a/gcc/gensupport.c
> +++ b/gcc/gensupport.c
> @@ -1291,6 +1291,9 @@ alter_constraints (rtx pattern, int n_dup, 
> constraints_handler_t alter)
>  case MATCH_OPERAND:
>XSTR (pattern, 2) = alter (XSTR (pattern, 2), n_dup);
>break;
> +case MATCH_SCRATCH:
> +  XSTR (pattern, 1) = alter (XSTR (pattern, 1), n_dup);
> +  break;
>  
>  default:
>break;


Re: [committed] libstdc++: Fix std::jthread assertion and re-enable skipped test

2021-05-27 Thread Bernd Edlinger
On 5/19/21 3:37 PM, Bernd Edlinger wrote:
> On 5/19/21 3:27 PM, Jonathan Wakely wrote:
>> On 18/05/21 13:58 +0200, Bernd Edlinger wrote:
>>> On 5/18/21 1:55 PM, Bernd Edlinger wrote:
 On 5/17/21 7:13 PM, Jonathan Wakely via Gcc-patches wrote:
> libstdc++-v3/ChangeLog:
>
> * include/std/thread (jthread::_S_create): Fix static assert
> message.
> * testsuite/30_threads/jthread/95989.cc: Re-enable test.
> * testsuite/30_threads/jthread/jthread.cc: Do not require
> pthread effective target.
> * testsuite/30_threads/jthread/2.cc: Moved to...
> * testsuite/30_threads/jthread/version.cc: ...here.
>
> Tested powerpc64le-linux. Committed to trunk.
>
> Let's see if this test is actually fixed, or if it still causes
> failures on some targets.
>
>

 Yes, indeed it is failing on x86_64-pc-linux-gnu.

>>>
>>> that means only this one:
>>>
>>> FAIL: 30_threads/jthread/95989.cc execution test
>>
>> What's your glibc version?
>>
>>
> 
> that is ubuntu 20.04 with latest patches:
> 
> $ ldd --version
> ldd (Ubuntu GLIBC 2.31-0ubuntu9.3) 2.31
> Copyright (C) 2020 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> Written by Roland McGrath and Ulrich Drepper.
> 

The glibc version is probably not the reason,
I've got the same effect with Ubuntu EGLIBC 2.19-0ubuntu6.15 as well.

But I use binutils-2.36.1.

I think the problem is here:

libstdc++-v3/src/c++11/thread.cc:  __e = __gthread_join(_M_id._M_thread, 0);

which calls

libgcc/gthr-posix.h:  return __gthrw_(pthread_join) (__threadid, __value_ptr);

but this is only a weak reference to pthread_join:

669   return __gthrw_(pthread_join) (__threadid, __value_ptr);
(gdb) disass
Dump of assembler code for function std::thread::join():
   0x00405e20 <+0>: endbr64 
   0x00405e24 <+4>: push   %rbx
   0x00405e25 <+5>: mov%rdi,%rbx
   0x00405e28 <+8>: mov(%rdi),%rdi
   0x00405e2b <+11>:test   %rdi,%rdi
   0x00405e2e <+14>:je 0x405e44 
=> 0x00405e30 <+16>:xor%esi,%esi
   0x00405e32 <+18>:callq  0x0
   0x00405e37 <+23>:test   %eax,%eax
   0x00405e39 <+25>:jne0x405e49 
   0x00405e3b <+27>:movq   $0x0,(%rbx)
   0x00405e42 <+34>:pop%rbx
   0x00405e43 <+35>:retq   
   0x00405e44 <+36>:mov$0x16,%eax
   0x00405e49 <+41>:mov%eax,%edi
   0x00405e4b <+43>:callq  0x4012a9 
End of assembler dump.

but the pthread_join symbol is not checked for potentially being null.

The test case passes if I change this:

diff --git a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc 
b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
index fb3f43bc722..61a3ff76aa1 100644
--- a/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
+++ b/libstdc++-v3/testsuite/30_threads/jthread/95989.cc
@@ -52,4 +52,5 @@ main()
   test01();
   test02();
   test03();
+  pthread_join(NULL, NULL);
 }


Thanks
Bernd.


Re: none

2021-05-27 Thread Richard Sandiford via Gcc-patches
Joern Rennecke  writes:
> At the moment, for a match_dup in a define_cond_exec, you'd have to
> give the number in the
> resulting pattern(s) rather than in the substitute pattern.  That's
> not only wrong, but can also
> be impossible when the pattern should apply to multiple patterns with
> different operand numbers.
>
> The attached patch fixes this.
>
> Bootstrapped on x86_64-pc-linux-gnu.
>
> 2020-12-12  Joern Rennecke  
>
>   Fix match_dup bug of define_cond_exec.
>   * gensupport.c (alter_predicate_for_insn): Handle MATCH_DUP.

The “Fix match_dup …” should come before the changelog in the commit message.

OK otherwise, thanks.

Richard

> diff --git a/gcc/gensupport.c b/gcc/gensupport.c
> index e1ca06dbc1e..92275358078 100644
> --- a/gcc/gensupport.c
> +++ b/gcc/gensupport.c
> @@ -1230,6 +1230,7 @@ alter_predicate_for_insn (rtx pattern, int alt, int 
> max_op,
>  case MATCH_OPERATOR:
>  case MATCH_SCRATCH:
>  case MATCH_PARALLEL:
> +case MATCH_DUP:
>XINT (pattern, 0) += max_op;
>break;
>  


Re: [PATCH] arm: Auto-vectorization for MVE: vabs

2021-05-27 Thread Christophe Lyon via Gcc-patches
At the moment I sent that patch I remembered that Victor is also
working on vabs, so sorry if this conflicts with his work.


On Thu, 27 May 2021 at 13:21, Christophe Lyon
 wrote:
>
> This patch adds support for auto-vectorization of absolute value
> computation using vabs.
>
> We use a similar pattern to what is used in neon.md and extend the
> existing neg2 expander to match both 'neg' and 'abs'.  This
> implies renaming the existing abs2 define_insn in neon.md to
> avoid a clash with the new expander with the same name.
>
> 2021-05-26  Christophe Lyon  
>
> gcc/
> * config/arm/mve.md (mve_vabsq_f): Use 'abs' instead of unspec.
> (mve_vabsq_s): Likewise.
> * config/arm/neon.md (abs2): Rename to neon_abs2.
> * config/arm/unspecs.md (VABSQ_F, VABSQ_S): Delete.
> * config/arm/vec-common.md (neg2): Rename to
> 2.
>
> gcc/testsuite/
> * gcc.target/arm/simd/mve-vabs.c: New test.
> ---
>  gcc/config/arm/mve.md|  6 +--
>  gcc/config/arm/neon.md   |  2 +-
>  gcc/config/arm/unspecs.md|  2 -
>  gcc/config/arm/vec-common.md |  4 +-
>  gcc/testsuite/gcc.target/arm/simd/mve-vabs.c | 44 
>  5 files changed, 49 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arm/simd/mve-vabs.c
>
> diff --git a/gcc/config/arm/mve.md b/gcc/config/arm/mve.md
> index 0a6ba80c99d..0bfa6a91d55 100644
> --- a/gcc/config/arm/mve.md
> +++ b/gcc/config/arm/mve.md
> @@ -269,8 +269,7 @@ (define_insn "mve_vdupq_n_f"
>  (define_insn "mve_vabsq_f"
>[
> (set (match_operand:MVE_0 0 "s_register_operand" "=w")
> -   (unspec:MVE_0 [(match_operand:MVE_0 1 "s_register_operand" "w")]
> -VABSQ_F))
> +   (abs:MVE_0 (match_operand:MVE_0 1 "s_register_operand" "w")))
>]
>"TARGET_HAVE_MVE && TARGET_HAVE_MVE_FLOAT"
>"vabs.f%#  %q0, %q1"
> @@ -481,8 +480,7 @@ (define_insn "@mve_vaddvq_"
>  (define_insn "mve_vabsq_s"
>[
> (set (match_operand:MVE_2 0 "s_register_operand" "=w")
> -   (unspec:MVE_2 [(match_operand:MVE_2 1 "s_register_operand" "w")]
> -VABSQ_S))
> +   (abs:MVE_2 (match_operand:MVE_2 1 "s_register_operand" "w")))
>]
>"TARGET_HAVE_MVE"
>"vabs.s%#\t%q0, %q1"
> diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
> index 6a6573317cf..077c62ffd20 100644
> --- a/gcc/config/arm/neon.md
> +++ b/gcc/config/arm/neon.md
> @@ -739,7 +739,7 @@ (define_insn "one_cmpl2_neon"
>[(set_attr "type" "neon_move")]
>  )
>
> -(define_insn "abs2"
> +(define_insn "neon_abs2"
>[(set (match_operand:VDQW 0 "s_register_operand" "=w")
> (abs:VDQW (match_operand:VDQW 1 "s_register_operand" "w")))]
>"TARGET_NEON"
> diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
> index 0778db1bf4f..ed1bc293b78 100644
> --- a/gcc/config/arm/unspecs.md
> +++ b/gcc/config/arm/unspecs.md
> @@ -538,7 +538,6 @@ (define_c_enum "unspec" [
>VRNDAQ_F
>VREV64Q_F
>VDUPQ_N_F
> -  VABSQ_F
>VREV32Q_F
>VCVTTQ_F32_F16
>VCVTBQ_F32_F16
> @@ -562,7 +561,6 @@ (define_c_enum "unspec" [
>VCLSQ_S
>VADDVQ_S
>VADDVQ_U
> -  VABSQ_S
>VREV32Q_U
>VREV32Q_S
>VMOVLTQ_U
> diff --git a/gcc/config/arm/vec-common.md b/gcc/config/arm/vec-common.md
> index 8e35151da46..80b273229f5 100644
> --- a/gcc/config/arm/vec-common.md
> +++ b/gcc/config/arm/vec-common.md
> @@ -208,9 +208,9 @@ (define_expand "one_cmpl2"
>"ARM_HAVE__ARITH && !TARGET_REALLY_IWMMXT"
>  )
>
> -(define_expand "neg2"
> +(define_expand "2"
>[(set (match_operand:VDQWH 0 "s_register_operand" "")
> -   (neg:VDQWH (match_operand:VDQWH 1 "s_register_operand" "")))]
> +   (ABSNEG:VDQWH (match_operand:VDQWH 1 "s_register_operand" "")))]
>"ARM_HAVE__ARITH && !TARGET_REALLY_IWMMXT"
>  )
>
> diff --git a/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c 
> b/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c
> new file mode 100644
> index 000..64cd1c2eb4a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/simd/mve-vabs.c
> @@ -0,0 +1,44 @@
> +/* { dg-do assemble } */
> +/* { dg-require-effective-target arm_v8_1m_mve_fp_ok } */
> +/* { dg-add-options arm_v8_1m_mve_fp } */
> +/* { dg-additional-options "-O3 -funsafe-math-optimizations" } */
> +
> +#include 
> +#include 
> +
> +#define ABS(a) ((a < 0) ? -a : a)
> +
> +#define FUNC(SIGN, TYPE, BITS, NB, NAME)   \
> +  void test_ ## NAME ##_ ## SIGN ## BITS ## x ## NB (TYPE##BITS##_t * 
> __restrict__ dest, TYPE##BITS##_t *a) { \
> +int i; \
> +for (i=0; i +  dest[i] = ABS(a[i]); \
> +}  \
> +}
> +
> +#define FUNC_FLOAT(SIGN, TYPE, BITS, NB, NAME) \
> +  void test_ ## NAME ##_ ## SIGN ## BITS ## x ## NB (TYPE * __restrict__ 
> 

[PATCH] i386: Add XOP comparisons for 4- and 8-byte vectors [PR100637]

2021-05-27 Thread Uros Bizjak via Gcc-patches
2021-05-27  Uroš Bizjak  

gcc/
PR target/100637
* config/i386/i386-expand.c (ix86_expand_int_sse_cmp):
For TARGET_XOP bypass SSE comparisons for all supported vector modes.
* config/i386/mmx.md (*xop_maskcmp3): New insn pattern.
(*xop_maskcmp3): Ditto.
(*xop_maskcmp_uns3): Ditto.
(*xop_maskcmp_uns3): Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Pushed to master.

Uros.
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index 931b3362144..4185f58eed5 100644
--- a/gcc/config/i386/i386-expand.c
+++ b/gcc/config/i386/i386-expand.c
@@ -4124,8 +4124,8 @@ ix86_expand_int_sse_cmp (rtx dest, enum rtx_code code, 
rtx cop0, rtx cop1,
 
   /* XOP supports all of the comparisons on all 128-bit vector int types.  */
   if (TARGET_XOP
-  && (mode == V16QImode || mode == V8HImode
- || mode == V4SImode || mode == V2DImode))
+  && GET_MODE_CLASS (mode) == MODE_VECTOR_INT
+  && GET_MODE_SIZE (mode) <= 16)
 ;
   /* AVX512F supports all of the comparsions
  on all 128/256/512-bit vector int types.  */
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 23d88a4c265..35e4123fa25 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -2121,6 +2121,62 @@ (define_insn "*gt3"
(set_attr "type" "ssecmp")
(set_attr "mode" "TI")])
 
+(define_insn "*xop_maskcmp3"
+  [(set (match_operand:MMXMODEI 0 "register_operand" "=x")
+   (match_operator:MMXMODEI 1 "ix86_comparison_int_operator"
+[(match_operand:MMXMODEI 2 "register_operand" "x")
+ (match_operand:MMXMODEI 3 "register_operand" "x")]))]
+  "TARGET_XOP"
+  "vpcom%Y1\t{%3, %2, %0|%0, %2, %3}"
+  [(set_attr "type" "sse4arg")
+   (set_attr "prefix_data16" "0")
+   (set_attr "prefix_rep" "0")
+   (set_attr "prefix_extra" "2")
+   (set_attr "length_immediate" "1")
+   (set_attr "mode" "TI")])
+
+(define_insn "*xop_maskcmp3"
+  [(set (match_operand:VI_32 0 "register_operand" "=x")
+   (match_operator:VI_32 1 "ix86_comparison_int_operator"
+[(match_operand:VI_32 2 "register_operand" "x")
+ (match_operand:VI_32 3 "register_operand" "x")]))]
+  "TARGET_XOP"
+  "vpcom%Y1\t{%3, %2, %0|%0, %2, %3}"
+  [(set_attr "type" "sse4arg")
+   (set_attr "prefix_data16" "0")
+   (set_attr "prefix_rep" "0")
+   (set_attr "prefix_extra" "2")
+   (set_attr "length_immediate" "1")
+   (set_attr "mode" "TI")])
+
+(define_insn "*xop_maskcmp_uns3"
+  [(set (match_operand:MMXMODEI 0 "register_operand" "=x")
+   (match_operator:MMXMODEI 1 "ix86_comparison_uns_operator"
+[(match_operand:MMXMODEI 2 "register_operand" "x")
+ (match_operand:MMXMODEI 3 "register_operand" "x")]))]
+  "TARGET_XOP"
+  "vpcom%Y1u\t{%3, %2, %0|%0, %2, %3}"
+  [(set_attr "type" "ssecmp")
+   (set_attr "prefix_data16" "0")
+   (set_attr "prefix_rep" "0")
+   (set_attr "prefix_extra" "2")
+   (set_attr "length_immediate" "1")
+   (set_attr "mode" "TI")])
+
+(define_insn "*xop_maskcmp_uns3"
+  [(set (match_operand:VI_32 0 "register_operand" "=x")
+   (match_operator:VI_32 1 "ix86_comparison_uns_operator"
+[(match_operand:VI_32 2 "register_operand" "x")
+ (match_operand:VI_32 3 "register_operand" "x")]))]
+  "TARGET_XOP"
+  "vpcom%Y1u\t{%3, %2, %0|%0, %2, %3}"
+  [(set_attr "type" "ssecmp")
+   (set_attr "prefix_data16" "0")
+   (set_attr "prefix_rep" "0")
+   (set_attr "prefix_extra" "2")
+   (set_attr "length_immediate" "1")
+   (set_attr "mode" "TI")])
+
 (define_expand "vec_cmp"
   [(set (match_operand:MMXMODEI 0 "register_operand")
(match_operator:MMXMODEI 1 ""


Re: [PATCH v2] forwprop: Support vec perm fed by CTOR and CTOR/CST [PR99398]

2021-05-27 Thread Richard Sandiford via Gcc-patches
Sorry for the slow reponse.

"Kewen.Lin"  writes:
> diff --git a/gcc/vec-perm-indices.c b/gcc/vec-perm-indices.c
> index ede590dc5c9..57dd11d723c 100644
> --- a/gcc/vec-perm-indices.c
> +++ b/gcc/vec-perm-indices.c
> @@ -101,6 +101,70 @@ vec_perm_indices::new_expanded_vector (const 
> vec_perm_indices &orig,
>m_encoding.finalize ();
>  }
>  
> +/* Check whether we can switch to a new permutation vector that
> +   selects the same input elements as ORIG, but with each element
> +   built up from FACTOR pieces.  Return true if yes, otherwise
> +   return false.  Every FACTOR permutation indexes should be
> +   continuous separately and the first one of each batch should
> +   be able to exactly modulo FACTOR.  For example, if ORIG is
> +   { 2, 3, 4, 5, 0, 1, 6, 7 } and FACTOR is 2, the new permutation
> +   is { 1, 2, 0, 3 }.  */
> +
> +bool
> +vec_perm_indices::new_shrunk_vector (const vec_perm_indices &orig,
> +  unsigned int factor)
> +{
> +  gcc_assert (factor > 0);
> +
> +  if (maybe_lt (orig.m_nelts_per_input, factor))
> +return false;
> +
> +  poly_uint64 nelts;
> +  /* Invalid if vector units number isn't multiple of factor.  */
> +  if (!multiple_p (orig.m_nelts_per_input, factor, &nelts))
> +return false;
> +
> +  /* Only handle the case that npatterns is multiple of factor.
> + FIXME: Try to see whether we can reshape it by factor npatterns.  */
> +  if (orig.m_encoding.npatterns () % factor != 0)
> +return false;
> +
> +  unsigned int encoded_nelts = orig.m_encoding.encoded_nelts ();
> +  auto_vec encodings (encoded_nelts);

auto_vec would avoid memory allocations in the
same cases that m_encoding can.  “encoding” might be better than
“encodings” since there's only really one encoding here.

> +  /* Separate all encoded elements into batches by size factor,
> + then ensure the first element of each batch is multiple of
> + factor and all elements in each batch is consecutive from
> + the first one.  */
> +  for (unsigned int i = 0; i < encoded_nelts; i += factor)
> +{
> +  element_type first = orig.m_encoding[i];
> +  element_type new_index;
> +  if (!multiple_p (first, factor, &new_index))
> + return false;
> +  for (unsigned int j = 1; j < factor; ++j)
> + {
> +   if (maybe_ne (first + j, orig.m_encoding[i + j]))
> + return false;
> + }

Formatting nit: unnecessary braces around if.

> +  encodings.quick_push (new_index);
> +}
> +
> +  m_ninputs = orig.m_ninputs;
> +  m_nelts_per_input = nelts;
> +  poly_uint64 full_nelts = exact_div (orig.m_encoding.full_nelts (), factor);
> +  unsigned int npatterns = orig.m_encoding.npatterns () / factor;
> +
> +  m_encoding.new_vector (full_nelts, npatterns,
> +  orig.m_encoding.nelts_per_pattern ());
> +
> +  for (unsigned int i = 0; i < encodings.length (); i++)
> +m_encoding.quick_push (encodings[i]);

I think this can be:

   m_encoding.splice (encodings);

OK with those changes, thanks.  Thanks also for doing it in a
variable-length-friendly way.

Richard

> +
> +  m_encoding.finalize ();
> +
> +  return true;
> +}
> +
>  /* Rotate the inputs of the permutation right by DELTA inputs.  This changes
> the values of the permutation vector but it doesn't change the way that
> the elements are encoded.  */
> diff --git a/gcc/vec-perm-indices.h b/gcc/vec-perm-indices.h
> index bc70ecd8a1d..98d27f0ec42 100644
> --- a/gcc/vec-perm-indices.h
> +++ b/gcc/vec-perm-indices.h
> @@ -57,6 +57,7 @@ public:
>  
>void new_vector (const vec_perm_builder &, unsigned int, poly_uint64);
>void new_expanded_vector (const vec_perm_indices &, unsigned int);
> +  bool new_shrunk_vector (const vec_perm_indices &, unsigned int);
>void rotate_inputs (int delta);
>  
>/* Return the underlying vector encoding.  */


[PATCH] OpenMP: Use build_indirect_ref for struct deferences in C FE

2021-05-27 Thread Julian Brown
This patch fixes an ICE in the C FE (discovered by Tobias) when a user
tries to use a dereference of a non-pointer-valued base (e.g. a plain
struct) in an OpenMP clause.

This patch has been tested on a tree that already has the following patches by
Chung-Lin applied:

  (a) https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570886.html
  "[PATCH, OpenMP 5.0] Improve OpenMP target support for C++ (includes 
PR92120 v3)"
  (b) https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570365.html
  "[PATCH, OpenMP 5.0] Implement relaxation of implicit map vs. existing 
device mappings (for mainline trunk)"
  (c) https://gcc.gnu.org/pipermail/gcc-patches/2021-May/571195.html
  "[PATCH, OpenMP 5.0] Remove array section base-pointer mapping semantics, 
and other front-end adjustments (mainline trunk)"

Without those patches, equivalent changes are needed in several places
in the C FE to handle INDIRECT_REFs instead of only MEM_REFs, as at
present. So this one should go in after those patches, assuming it's
otherwise OK.

The C++ FE already catches this case correctly (though the error message
is different for C/C++).

Tested with offloading to NVPTX (and bootstrapped).

Thanks,

Julian

2021-05-27  Julian Brown  

gcc/c/
* c-parser.c (c_parser_omp_variable_list); Call build_indirect_ref
instead of build_simple_mem_ref for struct dereferences.

gcc/testsuite/
* c-c++-common/gomp/target-indir-struct-1.c: New test.
---
 gcc/c/c-parser.c|  2 +-
 .../c-c++-common/gomp/target-indir-struct-1.c   | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/gomp/target-indir-struct-1.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 582f86b9b35..ee21be43ed8 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -13035,7 +13035,7 @@ c_parser_omp_variable_list (c_parser *parser,
{
  location_t op_loc = c_parser_peek_token (parser)->location;
  if (c_parser_next_token_is (parser, CPP_DEREF))
-   t = build_simple_mem_ref (t);
+   t = build_indirect_ref (op_loc, t, RO_ARROW);
  c_parser_consume_token (parser);
  if (!c_parser_next_token_is (parser, CPP_NAME))
{
diff --git a/gcc/testsuite/c-c++-common/gomp/target-indir-struct-1.c 
b/gcc/testsuite/c-c++-common/gomp/target-indir-struct-1.c
new file mode 100644
index 000..7e5ce2a61d5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/target-indir-struct-1.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+struct t { int *p; };
+
+void foo () {
+  struct t s;
+  #pragma omp target map(to: s->p) /* { dg-error "invalid type argument of 
'->' \\(have 'struct t'\\)" "" { target c } } */ 
+  /* { dg-error "base operand of '->' has non-pointer type 't'" "" { target 
c++ } .-1 } */
+  {
+  }
+}
+
+int main (int argc, char *argv[])
+{
+  foo ();
+  return 0;
+}
-- 
2.29.2



Re: [PATCH] c++: Relax rules for non-type arguments in partial specs [CWG1315]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/26/21 7:36 PM, Patrick Palka wrote:

This implements the wording changes of CWG 1315, which permits non-type
template arguments in a partial specialization to use template
parameters more freely.  Delightfully, it seems the only change needed
is to remove a few checks from process_partial_specialization.

But that change alone revealed a latent problem with
for_each_template_parm: it ends up looking into some non-deduced
contexts even when include_nondeduced_p is false.  This causes us to
silently accept some partial specializations within the testsuite that
contain non-deducible non-type template parameters (and that were
previously rejected due to the rule that CWG 1315 removed).  For now
this patch makes a minimal amount of changes to for_each_template_parm_r
so we continue to reject existing ill-formed partial specializations
within the testsuite.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  Patch generated with -w to hide noisy whitespace changes.


OK.


DR 1315
PR c++/96555
PR c++/100779

gcc/cp/ChangeLog:

* pt.c (process_partial_specialization): Don't error on a
non-simple non-type template argument that involves template
parameters.
(for_each_template_parm_r): Don't walk TRAIT_EXPR, PLUS_EXPR,
MULT_EXPR, or SCOPE_REF when include_nondeduced_p is unset.

gcc/testsuite/ChangeLog:

* g++.dg/template/partial16.C: New test.
* g++.dg/template/partial17.C: New test.
* g++.dg/template/partial18.C: New test.
* g++.dg/cpp0x/pr68724.C: Adjust expected diagnostic for
  ill-formed partial specialization.
* g++.dg/cpp0x/variadic38.C: Likewise.
* g++.dg/cpp1z/pr81016.C: Likewise.
* g++.dg/template/partial5.C: Likewise.
* g++.old-deja/g++.pt/spec21.C: Likewise.
---
  gcc/cp/pt.c| 28 --
  gcc/testsuite/g++.dg/cpp0x/pr68724.C   |  2 +-
  gcc/testsuite/g++.dg/cpp0x/variadic38.C|  3 ++-
  gcc/testsuite/g++.dg/cpp1z/pr81016.C   |  2 +-
  gcc/testsuite/g++.dg/template/partial16.C  |  8 +++
  gcc/testsuite/g++.dg/template/partial17.C  | 14 +++
  gcc/testsuite/g++.dg/template/partial18.C  | 19 +++
  gcc/testsuite/g++.dg/template/partial5.C   |  2 +-
  gcc/testsuite/g++.old-deja/g++.pt/spec21.C |  3 +--
  9 files changed, 57 insertions(+), 24 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/template/partial16.C
  create mode 100644 gcc/testsuite/g++.dg/template/partial17.C
  create mode 100644 gcc/testsuite/g++.dg/template/partial18.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f3fa9c192ad..6b0bc815404 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5157,11 +5157,7 @@ process_partial_specialization (tree decl)
maintmpl);
  }
  
-  /* [temp.class.spec]

-
- A partially specialized non-type argument expression shall not
- involve template parameters of the partial specialization except
- when the argument expression is a simple identifier.
+  /* [temp.spec.partial]
  
   The type of a template parameter corresponding to a specialized

   non-type argument shall not be dependent on a parameter of the
@@ -5221,13 +5217,6 @@ process_partial_specialization (tree decl)
  && !((REFERENCE_REF_P (arg)
|| TREE_CODE (arg) == VIEW_CONVERT_EXPR)
   && TREE_CODE (TREE_OPERAND (arg, 0)) == TEMPLATE_PARM_INDEX))
-{
-  if ((!packed_args && tpd.arg_uses_template_parms[i])
-  || (packed_args && uses_template_parms (arg)))
-   error_at (cp_expr_loc_or_input_loc (arg),
- "template argument %qE involves template "
- "parameter(s)", arg);
-  else
  {
  /* Look at the corresponding template parameter,
 marking which template parameters its type depends
@@ -5281,7 +5270,6 @@ process_partial_specialization (tree decl)
  }
  }
  }
-}
  
/* We should only get here once.  */

if (TREE_CODE (decl) == TYPE_DECL)
@@ -10502,6 +10490,15 @@ for_each_template_parm_r (tree *tp, int 
*walk_subtrees, void *d)
return error_mark_node;
break;
  
+case TRAIT_EXPR:

+case PLUS_EXPR:
+case MULT_EXPR:
+case SCOPE_REF:
+  /* These are always non-deduced contexts.  */
+  if (!pfd->include_nondeduced_p)
+   *walk_subtrees = 0;
+  break;
+
  case MODOP_EXPR:
  case CAST_EXPR:
  case IMPLICIT_CONV_EXPR:
@@ -10517,11 +10514,6 @@ for_each_template_parm_r (tree *tp, int 
*walk_subtrees, void *d)
return error_mark_node;
break;
  
-case SCOPE_REF:

-  if (pfd->include_nondeduced_p)
-   WALK_SUBTREE (TREE_OPERAND (t, 0));
-  break;
-
  case REQUIRES_EXPR:
{
if (!fn)
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr68724.C 
b/gcc/testsu

Re: [PATCH] libstdc++: Avoid hard error in ranges::unique_copy [PR100770]

2021-05-27 Thread Patrick Palka via Gcc-patches
On Wed, 26 May 2021, Tim Song wrote:

> On Wed, May 26, 2021 at 1:43 PM Patrick Palka  wrote:
> >
> > On Wed, 26 May 2021, Tim Song wrote:
> > >
> > > On Wed, May 26, 2021 at 12:00 PM Patrick Palka via Libstdc++
> > >  wrote:
> > > >
> > > > -   else if constexpr (input_iterator<_Out>
> > > > -  && same_as, 
> > > > iter_value_t<_Out>>)
> > > > +   else if constexpr (requires { requires (input_iterator<_Out>
> > > > +   && 
> > > > same_as,
> > > > +  
> > > > iter_value_t<_Out>>); })
> > >
> > > It's arguably cleaner to extract this into a concept which can then
> > > also be used in the constraint.
> >
> > Sounds good, though I'm not sure what name to give to this relatively
> > ad-hoc set of requirements.  Any suggestions? :)
> >
> 
> Something along the lines of "__can_reread_output", perhaps?

Works for me.  Here's v2, which factors out the condition into a concept
and defines the value_type, pointer and reference of output_iterator_wrapper
to void but leaves alone its difference_type.  Tested on x86_64-pc-linux-gnu.

-- >8 --

Subject: [PATCH] libstdc++: Avoid hard error in ranges::unique_copy [PR100770]

Here, in the constexpr if condition within unique_copy, when
input_iterator<_Out> isn't satisfied we must avoid substituting into
iter_value_t<_Out> because the latter isn't necessarily well-formed in
this case.  To that end, this patch factors out the condition into a
concept and uses it throughout.

This patch also makes the definition of our testsuite
output_iterator_wrapper more minimal by defining its value_type, pointer
and reference member types to void.  This means our existing tests for
unique_copy already exercise the fix for this bug, so we don't need
to add another test.  The only other fallout of this testsuite iterator
change appears in std/ranges/range.cc, where the use of range_value_t
on a test_output_range is now ill-formed.

libstdc++-v3/ChangeLog:

* include/bits/ranges_algo.h (__detail::__can_reread_output):
Factor out this concept from ...
(__unique_copy_fn::operator()): ... here.  Use the concept
throughout.
* testsuite/std/ranges/range.cc: Remove now ill-formed use
of range_value_t on an output_range.
* testsuite/util/testsuite_iterators.h (output_iterator_wrapper):
Define value_type, pointer and reference member types to void.
---
 libstdc++-v3/include/bits/ranges_algo.h  | 16 ++--
 libstdc++-v3/testsuite/std/ranges/range.cc   |  3 ---
 .../testsuite/util/testsuite_iterators.h |  2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/libstdc++-v3/include/bits/ranges_algo.h 
b/libstdc++-v3/include/bits/ranges_algo.h
index cda3042c11f..ecf1378742d 100644
--- a/libstdc++-v3/include/bits/ranges_algo.h
+++ b/libstdc++-v3/include/bits/ranges_algo.h
@@ -1396,6 +1396,13 @@ namespace ranges
 
   inline constexpr __unique_fn unique{};
 
+  namespace __detail
+  {
+template
+  concept __can_reread_output = input_iterator<_Out>
+   && same_as<_Tp, iter_value_t<_Out>>;
+  }
+
   template
 using unique_copy_result = in_out_result<_Iter, _Out>;
 
@@ -1407,8 +1414,7 @@ namespace ranges
   projected<_Iter, _Proj>> _Comp = ranges::equal_to>
   requires indirectly_copyable<_Iter, _Out>
&& (forward_iterator<_Iter>
-   || (input_iterator<_Out>
-   && same_as, iter_value_t<_Out>>)
+   || __detail::__can_reread_output<_Out, iter_value_t<_Iter>>
|| indirectly_copyable_storable<_Iter, _Out>)
   constexpr unique_copy_result<_Iter, _Out>
   operator()(_Iter __first, _Sent __last, _Out __result,
@@ -1432,8 +1438,7 @@ namespace ranges
}
return {__next, std::move(++__result)};
  }
-   else if constexpr (input_iterator<_Out>
-  && same_as, iter_value_t<_Out>>)
+   else if constexpr (__detail::__can_reread_output<_Out, 
iter_value_t<_Iter>>)
  {
*__result = *__first;
while (++__first != __last)
@@ -1467,8 +1472,7 @@ namespace ranges
   projected, _Proj>> _Comp = ranges::equal_to>
   requires indirectly_copyable, _Out>
&& (forward_iterator>
-   || (input_iterator<_Out>
-   && same_as, iter_value_t<_Out>>)
+   || __detail::__can_reread_output<_Out, range_value_t<_Range>>
|| indirectly_copyable_storable, _Out>)
   constexpr unique_copy_result, _Out>
   operator()(_Range&& __r, _Out __result,
diff --git a/libstdc++-v3/testsuite/std/ranges/range.cc 
b/libstdc++-v3/testsuite/std/ranges/range.cc
index 795aca472e5..aa29af471a4 100644
--- a/libstdc++-v3/testsuite/std/ranges/range.cc
+++ b/libstdc++-v3/testsuite/std/ranges/range.cc
@@ -75,9 +75,6 @@ static_assert( same_as,
 static_assert( same_as,
   s

Re: [PATCH] Modula-2 into the GCC tree on master

2021-05-27 Thread Gaius Mulley via Gcc-patches
Matthias Klose  writes:

> On 1/18/21 2:55 PM, Gaius Mulley via Gcc-patches wrote:
>> Richard Biener  writes:
>>> I've just done ./configure --enable-languages=m2; make -j24
>>>
>>> I would suggest to not rush this in now during stage4
>>> but instead take the opportunity of this "quiet" phase
>>> to prepare an integration branch with all the issues above
>>> sorted out which we can merge at the beginning of stage1
>>> for GCC 12 (or later during stage4 if everyone is happy
>>> and/or backport for GCC 11.2 when it landed in trunk).
>>
>> ok sure - this sounds a good plan
>
> Gaius, now with the 1.1 relase out of the door, please could you clarify about
> your plans getting this into trunk, and do you plan to get this into 11.2 as 
> well?
>
> Thanks, Matthias

Hi Matthias,

sure yes - I plan on getting it into trunk and then backporting it to
11.2.  If anyone with copyright assignment wants to expedite this - feel
free to forge ahead (and post patches).  I'm working though Richard's
review:

 https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563747.html

and should be able to post the latest patch set in say 2 weeks
time (for trunk) - would that time frame work?


regards,
Gaius


Re: [PUSHED] Skip out on processing __builtin_clz when varying.

2021-05-27 Thread Aldy Hernandez via Gcc-patches
PING

On Thu, May 13, 2021 at 8:02 PM Aldy Hernandez  wrote:
>
>
>
> On 5/12/21 5:08 PM, Jakub Jelinek wrote:
> > On Wed, May 12, 2021 at 05:01:00PM -0400, Aldy Hernandez via Gcc-patches 
> > wrote:
> >>
> >>  PR c/100521
> >>  * gimple-range.cc (range_of_builtin_call): Skip out on
> >>processing __builtin_clz when varying.
> >> ---
> >>   gcc/gimple-range.cc | 2 +-
> >>   gcc/testsuite/gcc.dg/pr100521.c | 8 
> >>   2 files changed, 9 insertions(+), 1 deletion(-)
> >>   create mode 100644 gcc/testsuite/gcc.dg/pr100521.c
> >>
> >> --- /dev/null
> >> +++ b/gcc/testsuite/gcc.dg/pr100521.c
> >> @@ -0,0 +1,8 @@
> >> +/* { dg-do compile } */
> >> +/* { dg-options "-O2" } */
> >> +
> >> +int
> >> +__builtin_clz (int a)
> >
> > Is this intentional?  People shouldn't be redefining builtins...
>
> Ughhh.  I don't think that's intentional.  For that matter, the current
> nor the old code is designed to deal with this, especially in this case
> when the builtin is being redefined with incompatible arguments.  That
> is, the above "builtin" has a signed integer as an argument, whereas the
> original builtin had an unsigned one.
>
> In looking at the original vr-values code, I think this could use a
> cleanup.  First, ranges from range_of_expr are always numeric so we
> should adjust.  Also, the checks for non-zero were assuming the argument
> was unsigned, which in the above redirect is clearly not.  I've cleaned
> this up, so that it works either way, though perhaps we should _also_
> bail on non-builtins. I don't know...this is before my time.
>
> BTW, I've removed the following annoying idiom:
>
> - int newmini = prec - 1 - wi::floor_log2 (r.upper_bound ());
> - if (newmini == prec)
>
> This is really a check for r.upper_bound() == 0, as floor_log2(0)
> returns -1.  It's confusing.
>
> How does this look?  For reference, the original code where this all
> came from is 82b6d25d289195.
>
> Thanks for pointing this out.
> Aldy



Re: RFA: save/restore target options in handle_optimize_attribute

2021-05-27 Thread Martin Liška

On 5/26/21 10:51 PM, Joseph Myers wrote:

This commit breaks the build of glibc for powerpc64le-linux-gnu.  Compile
the following code with -O2 -mlong-double-128 -mabi=ibmlongdouble and I
get the error

opts-bug.c:8:1: error: '-mabi=ibmlongdouble' requires '-mlong-double-128'
 8 | {
   | ^

which is clearly nonsensical, since -mlong-double-128 is passed.

extern unsigned long int x;
extern float f (float);
extern __typeof (f) f_power8;
extern __typeof (f) f_power9;
extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
f_ifunc (void)
{
   __typeof (f) *res = x ? f_power9 : f_power8;
   return res;
}



Sorry about the fallout, I'm working on it now..

Martin


[PATCH] c++: parameter pack inside static_assert [PR99893]

2021-05-27 Thread Patrick Palka via Gcc-patches
Here, we're not finding the parameter pack inside the static_assert because
STATIC_ASSERT trees are tcc_exceptional, and we weren't explicitly walking
them in cp_walk_subtrees.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

PR c++/99893
* tree.c (cp_walk_subtrees) : New case.

gcc/testsuite/ChangeLog:

PR c++/99893
* g++.dg/cpp0x/static_assert17.C: New test.
---
 gcc/cp/tree.c| 5 +
 gcc/testsuite/g++.dg/cpp0x/static_assert17.C | 9 +
 2 files changed, 14 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/static_assert17.C

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 372d89fa9ed..fec5afaa2be 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5446,6 +5446,11 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, 
walk_tree_fn func,
}
   break;
 
+case STATIC_ASSERT:
+  WALK_SUBTREE (STATIC_ASSERT_CONDITION (*tp));
+  WALK_SUBTREE (STATIC_ASSERT_MESSAGE (*tp));
+  break;
+
 default:
   return NULL_TREE;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/static_assert17.C 
b/gcc/testsuite/g++.dg/cpp0x/static_assert17.C
new file mode 100644
index 000..64843c60edd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/static_assert17.C
@@ -0,0 +1,9 @@
+// PR c++/99893
+// { dg-do compile { target c++11 } }
+
+void f(...);
+
+template 
+void g() {
+  f([] { static_assert(Ts::value, ""); }...);
+}
-- 
2.32.0.rc0



Re: [PATCH] c++: Output less irrelevant info for function template decl [PR100716]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/26/21 5:29 PM, Matthias Kretz wrote:

New revision which can also be compiled with GCC 4.8.

From: Matthias Kretz 

Ensure dump_template_decl for function templates never prints template
parameters after the function name (it did with -fno-pretty-templates)
and skip output of irrelevant & confusing "[with T = T]" in
dump_substitution.

gcc/cp/ChangeLog:

PR c++/100716
* error.c (dump_template_bindings): Include code to print
"[with" and ']', conditional on whether anything is printed at
all. This is tied to whether a semicolon is needed to separate
multiple template parameters. If the template argument repeats
the template parameter (T = T), then skip the parameter.


This description should really be in a comment in the code, rather than 
the ChangeLog.  OK either way.



(dump_substitution): Moved code to print "[with" and ']' to
dump_template_bindings.
(dump_function_decl): Partial revert of PR50828, which masked
TFF_TEMPLATE_NAME for all of dump_function_decl. Now
TFF_TEMPLATE_NAME is masked for the scope of the function and
only carries through to dump_function_name.
(dump_function_name): Avoid calling dump_template_parms if
TFF_TEMPLATE_NAME is set.

gcc/testsuite/ChangeLog:

PR c++/100716
* g++.dg/diagnostic/pr100716.C: New test.
* g++.dg/diagnostic/pr100716-1.C: Same test with
-fno-pretty-templates.
---
  gcc/cp/error.c   | 59 +++-
  gcc/testsuite/g++.dg/diagnostic/pr100716-1.C | 54 ++
  gcc/testsuite/g++.dg/diagnostic/pr100716.C   | 54 ++
  3 files changed, 152 insertions(+), 15 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716-1.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716.C


--
──
  Dr. Matthias Kretz   https://mattkretz.github.io
  GSI Helmholtz Centre for Heavy Ion Research   https://gsi.de
  std::experimental::simd  https://github.com/VcDevel/std-simd
──





Re: [PATCH] c++: Add missing scope in typedef diagnostic [PR100763]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/26/21 5:27 PM, Matthias Kretz wrote:



From: Matthias Kretz 

dump_type on 'const std::string' should not print 'const string' unless
TFF_UNQUALIFIED_NAME is requested.

gcc/cp/ChangeLog:

PR c++/100763
* error.c: Call dump_scope when printing a typedef.



+ if (! (flags & TFF_UNQUALIFIED_NAME))
+   dump_scope (pp, CP_DECL_CONTEXT (TYPE_NAME (t)), flags);


You can use "decl" instead of "TYPE_NAME (t)" here.

OK with that change.

Jason



Re: [PATCH] c++: Output less irrelevant info for function template decl [PR100716]

2021-05-27 Thread Matthias Kretz
On Thursday, 27 May 2021 17:07:40 CEST Jason Merrill wrote:
> On 5/26/21 5:29 PM, Matthias Kretz wrote:
> > New revision which can also be compiled with GCC 4.8.
> > 
> > From: Matthias Kretz 
> > 
> > Ensure dump_template_decl for function templates never prints template
> > parameters after the function name (it did with -fno-pretty-templates)
> > and skip output of irrelevant & confusing "[with T = T]" in
> > dump_substitution.
> > 
> > gcc/cp/ChangeLog:
> > PR c++/100716
> > * error.c (dump_template_bindings): Include code to print
> > "[with" and ']', conditional on whether anything is printed at
> > all. This is tied to whether a semicolon is needed to separate
> > multiple template parameters. If the template argument repeats
> > the template parameter (T = T), then skip the parameter.
> 
> This description should really be in a comment in the code, rather than
> the ChangeLog.  OK either way.

Added comments in the code. New patch below.


From: Matthias Kretz 

Ensure dump_template_decl for function templates never prints template
parameters after the function name (it did with -fno-pretty-templates)
and skip output of irrelevant & confusing "[with T = T]" in
dump_substitution.

gcc/cp/ChangeLog:

PR c++/100716
* error.c (dump_template_bindings): Include code to print
"[with" and ']', conditional on whether anything is printed at
all. This is tied to whether a semicolon is needed to separate
multiple template parameters. If the template argument repeats
the template parameter (T = T), then skip the parameter.
(dump_substitution): Moved code to print "[with" and ']' to
dump_template_bindings.
(dump_function_decl): Partial revert of PR50828, which masked
TFF_TEMPLATE_NAME for all of dump_function_decl. Now
TFF_TEMPLATE_NAME is masked for the scope of the function and
only carries through to dump_function_name.
(dump_function_name): Avoid calling dump_template_parms if
TFF_TEMPLATE_NAME is set.

gcc/testsuite/ChangeLog:

PR c++/100716
* g++.dg/diagnostic/pr100716.C: New test.
* g++.dg/diagnostic/pr100716-1.C: Same test with
-fno-pretty-templates.
---
 gcc/cp/error.c   | 63 +++-
 gcc/testsuite/g++.dg/diagnostic/pr100716-1.C | 54 +
 gcc/testsuite/g++.dg/diagnostic/pr100716.C   | 54 +
 3 files changed, 156 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716-1.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716.C


--
──
 Dr. Matthias Kretz   https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research   https://gsi.de
 std::experimental::simd  https://github.com/VcDevel/std-simd
──diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index a2f19d1a5c1..ade9b17e663 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -371,7 +371,35 @@ static void
 dump_template_bindings (cxx_pretty_printer *pp, tree parms, tree args,
 vec *typenames)
 {
-  bool need_semicolon = false;
+  /* Print "[with" and ']', conditional on whether anything is printed at all.
+ This is tied to whether a semicolon is needed to separate multiple template
+ parameters.  */
+  struct prepost_semicolon
+  {
+cxx_pretty_printer *pp;
+bool need_semicolon;
+
+void operator()()
+{
+  if (need_semicolon)
+	pp_separate_with_semicolon (pp);
+  else
+	{
+	  pp_cxx_whitespace (pp);
+	  pp_cxx_left_bracket (pp);
+	  pp->translate_string ("with");
+	  pp_cxx_whitespace (pp);
+	  need_semicolon = true;
+	}
+}
+
+~prepost_semicolon()
+{
+  if (need_semicolon)
+	pp_cxx_right_bracket (pp);
+}
+  } semicolon_or_introducer = {pp, false};
+
   int i;
   tree t;
 
@@ -395,10 +423,20 @@ dump_template_bindings (cxx_pretty_printer *pp, tree parms, tree args,
 	  if (lvl_args && NUM_TMPL_ARGS (lvl_args) > arg_idx)
 	arg = TREE_VEC_ELT (lvl_args, arg_idx);
 
-	  if (need_semicolon)
-	pp_separate_with_semicolon (pp);
-	  dump_template_parameter (pp, TREE_VEC_ELT (p, i),
-   TFF_PLAIN_IDENTIFIER);
+	  tree parm_i = TREE_VEC_ELT (p, i);
+	  /* If the template argument repeats the template parameter (T = T),
+	 skip the parameter.*/
+	  if (arg && TREE_CODE (arg) == TEMPLATE_TYPE_PARM
+		&& TREE_CODE (parm_i) == TREE_LIST
+		&& TREE_CODE (TREE_VALUE (parm_i)) == TYPE_DECL
+		&& TREE_CODE (TREE_TYPE (TREE_VALUE (parm_i)))
+		 == TEMPLATE_TYPE_PARM
+		&& DECL_NAME (TREE_VALUE (parm_i))
+		 == DECL_NAME (TREE_CHAIN (arg)))
+	continue;
+
+	  semicolon_or_introducer();
+	  dump_template_parameter (pp, parm_i, TFF_PLAIN_IDENTIFIER);
 	  pp_cxx_whitespace (pp);
 	

Re: [PATCH] c++: Add missing scope in typedef diagnostic [PR100763]

2021-05-27 Thread Dr. Matthias Kretz
On Thursday, 27 May 2021 17:18:58 CEST Jason Merrill wrote:
> On 5/26/21 5:27 PM, Matthias Kretz wrote:
> > From: Matthias Kretz 
> > 
> > dump_type on 'const std::string' should not print 'const string' unless
> > TFF_UNQUALIFIED_NAME is requested.
> > 
> > gcc/cp/ChangeLog:
> > PR c++/100763
> > * error.c: Call dump_scope when printing a typedef.
> > 
> > + if (! (flags & TFF_UNQUALIFIED_NAME))
> > +   dump_scope (pp, CP_DECL_CONTEXT (TYPE_NAME (t)), flags);
> 
> You can use "decl" instead of "TYPE_NAME (t)" here.
> 
> OK with that change.

Updated patch below.


From: Matthias Kretz 

dump_type on 'const std::string' should not print 'const string' unless
TFF_UNQUALIFIED_NAME is requested.

gcc/cp/ChangeLog:

PR c++/100763
* error.c: Call dump_scope when printing a typedef.
---
 gcc/cp/error.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 3d5eebd4bcd..ae78b10c7b2 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -501,6 +501,8 @@ dump_type (cxx_pretty_printer *pp, tree t, int flags)
   else
 	{
 	  pp_cxx_cv_qualifier_seq (pp, t);
+	  if (! (flags & TFF_UNQUALIFIED_NAME))
+	dump_scope (pp, CP_DECL_CONTEXT (decl), flags);
 	  pp_cxx_tree_identifier (pp, TYPE_IDENTIFIER (t));
 	  return;
 	}


[PATCH] handle erroneous types in attribute nonnull (PR 100783)

2021-05-27 Thread Martin Sebor via Gcc-patches

When attribute nonnull is applied to an argument of an erroneous
type the attribute positional argument validation function ICEs
while printing a warning that mentions the invalid type.

The attached patch changes the validation function to ignore
erroneous types on the assumption that they must have already
been diagnosed.  It also enhances the pretty-printer to detect
erroneous types and print "{erroneous}" instead of causing
an ICE.  There already is code in the pretty printer that tries
to be robust in thew presence of erroneous types without actually
printing them, and this extends the detection to also print them.
(With the first part of this patch I don't have a test case for
it but handling the condition gracefully feels preferable to
waiting for another bug report with invalid code triggering
an ICE).

Martin
PR c/100783 - ICE on -Wnonnull and erroneous type

gcc/c-family/ChangeLog:

	PR c/100783
	* c-attribs.c (positional_argument): Bail on erroneous types.

gcc/c/ChangeLog:

	PR c/100783
	* c-objc-common.c (print_type): Handle erroneous types.

gcc/testsuite/ChangeLog:

	PR c/100783
	* gcc.dg/nonnull-6.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 804374d5acc..756a91b1c0d 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -698,6 +698,9 @@ positional_argument (const_tree fntype, const_tree atname, tree pos,
 
   if (tree argtype = type_argument_type (fntype, ipos))
 {
+  if (argtype == error_mark_node)
+	return NULL_TREE;
+
   if (flags & POSARG_ELLIPSIS)
 	{
 	  if (argno < 1)
diff --git a/gcc/c/c-objc-common.c b/gcc/c/c-objc-common.c
index a68249d7011..b945de15ab8 100644
--- a/gcc/c/c-objc-common.c
+++ b/gcc/c/c-objc-common.c
@@ -185,6 +185,12 @@ get_aka_type (tree type)
 static void
 print_type (c_pretty_printer *cpp, tree t, bool *quoted)
 {
+  if (t == error_mark_node)
+{
+  pp_string (cpp, _("{erroneous}"));
+  return;
+}
+
   gcc_assert (TYPE_P (t));
   struct obstack *ob = pp_buffer (cpp)->obstack;
   char *p = (char *) obstack_base (ob);
diff --git a/gcc/testsuite/gcc.dg/nonnull-6.c b/gcc/testsuite/gcc.dg/nonnull-6.c
new file mode 100644
index 000..8f368702e0e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/nonnull-6.c
@@ -0,0 +1,15 @@
+/* PR c/100783 - ICE on attribute nonnull and erroneous type
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+__attribute__((nonnull (1))) void
+f1 (char[][n]); // { dg-error "undeclared" }
+
+__attribute__((nonnull (2))) void
+f2 (int n, char[n][m]); // { dg-error "undeclared" }
+
+__attribute__((nonnull (1))) void
+f3 (char[*][n]);// { dg-error "undeclared" }
+
+__attribute__((nonnull (1))) void
+f4 (char[f1]);  // { dg-error "size" }


Re: [PATCH 0/11] warning control by group and location (PR 74765)

2021-05-27 Thread Martin Sebor via Gcc-patches

On 5/27/21 5:19 AM, Richard Sandiford wrote:

Thanks for doing this.

Martin Sebor via Gcc-patches  writes:

[…]
On 5/24/21 5:08 PM, David Malcolm wrote:

On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:

Subsequent patches then replace invocations of the TREE_NO_WARNING()
macro and the gimple_no_warning_p() and gimple_set_no_warning()
functions throughout GCC with those and remove the legacy APIs to
keep them from being accidentally reintroduced along with the
problem.
These are mostly mechanical changes, except that most of the new
invocations also specify the option whose disposition to query for
the expression or location, or which to enable or disable[2].
The last function, copy_no_warning(), copies the disposition from
one expression or location to another.

A couple of design choices might be helpful to explain:

First, introducing "warning groups" rather than controlling each
individual warning is motivated by a) the fact that the latter
would make avoiding redundant warnings for related problems
cumbersome (e.g., after issuing a -Warray-bounds we want to
suppress -Wstringop-overflow as well -Wstringop-overread for
the same access and vice versa), and b) simplicity and efficiency
of the implementation (mapping each option would require a more
complex data structure like a bitmap).

Second, using location_t to associate expressions/statements with
the warning groups also turns out to be more useful in practice
than a direct association between a tree or gimple*, and also
simplifies managing the data structure.  Trees and gimple* come
and go across passes, and maintaining a mapping for them that
accounts for the possibility of them being garbage-collected
and the same addresses reused is less than straightforward.


I find some of the terminology rather awkard due to it the negation
we're already using with TREE_NO_WARNING, in that we're turning on a
no_warning flag, and that this is a per-location/expr/stmt thing that's
different from the idea of enabling/disabling a specific warning
altogether (and the pragmas that control that).   Sometimes the patches
refer to enabling/disabling warnings and I think I want "enabling" and
"disabling" to mean the thing the user does with -Wfoo and -Wno-foo.

Would calling it a "warning suppression" or somesuch be more or less
klunky?


I like warning suppression :)  But I'm not sure where you suggest
I use the phrase.

I don't particularly care for the "no" in the API names either
(existing or new) and would prefer a positive form.  I considered
enable_warning() and warning_enabled() but I chose the names I did
because they're close to their established gimple namesakes.  I'm
fine changing them to the alternatives, or if you or someone else
has a preference for different names I'm open to suggestions.  Let
me know.


Not my area, but FWIW, +1 for David's suggestion of
s/set_no_warning/suppress_warning/ or similar.  As well as the
problem with the double negative in negated conditions, I always have to
remember whether TREE_NO_WARNING means that hasn't been anything to warn
about yet or whether we shouldn't warn in future.


Sure.  Jason has a patch out for review to introduce

  warning_enabled_at (location, int option).

With that, how does the following API look? (I'd replace the int
argument above with opt_code myself):

  void enable_warning (location_t, enum opt_code = N_OPTS);
  void disable_warning (location_t, enum opt_code = N_OPTS);
  void copy_warning (location_t, location_t);
  bool warning_enabled_at (location_t, enum opt_code = N_OPTS).

  void enable_warning (tree, enum opt_code = N_OPTS);
  void disable_warning (tree, enum opt_code = N_OPTS);
  void copy_warning (tree, const_tree);
  bool warning_enabled_at (const_tree, enum opt_code = N_OPTS).

  void enable_warning (gimple *, enum opt_code = N_OPTS);
  void disable_warning (gimple *, enum opt_code = N_OPTS);
  void copy_warning (gimple *, const gimple *);
  bool warning_enabled_at (gimple *, enum opt_code = N_OPTS).

Martin



Richard





[PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light

2021-05-27 Thread François Dumont via Gcc-patches
We have been talking for a long time of a debug mode with less impact on 
performances.


I propose to simply use the existing _GLIBCXX_ASSERTIONS macro.

    libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks

    Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When 
defined it activates
    all _GLIBCXX_DEBUG checks but skipping those requiring to loop 
through the iterator

    range unless in case of constexpr.

    libstdc++-v3/ChangeLog:

    * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define debug 
macros non-empty.
    * include/debug/helper_functions.h: Cleanup comment about 
removed _Iter_base.
    * include/debug/functions.h (__skip_debug_runtime_check): 
New, returns false if

    _GLIBCXX_DEBUG is defined or if constant evaluated.
    (__check_sorted, __check_partitioned_lower, 
__check_partitioned_upper): Use latter.


Tested under Linux x64.

Ok to commit ?

François

diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index 116f2f023e2..2e6ce1c8a93 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -61,7 +61,7 @@ namespace __gnu_debug
 struct _Safe_iterator;
 }
 
-#ifndef _GLIBCXX_DEBUG
+#ifndef _GLIBCXX_ASSERTIONS
 
 # define __glibcxx_requires_cond(_Cond,_Msg)
 # define __glibcxx_requires_valid_range(_First,_Last)
diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h
index 6cac11f2abd..ee0eb877568 100644
--- a/libstdc++-v3/include/debug/functions.h
+++ b/libstdc++-v3/include/debug/functions.h
@@ -48,6 +48,25 @@ namespace __gnu_debug
   template
 struct _Is_contiguous_sequence : std::__false_type { };
 
+  _GLIBCXX20_CONSTEXPR
+  inline bool
+  __skip_debug_runtime_check()
+  {
+// We could be here while only _GLIBCXX_ASSERTIONS has been defined.
+// In this case we skip expensive runtime checks, constexpr will still
+// be checked.
+return
+#ifndef _GLIBCXX_DEBUG
+# if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED
+  !__builtin_is_constant_evaluated();
+# else
+  true;
+# endif
+#else
+  false;
+#endif
+  }
+
   /* Checks that [first, last) is a valid range, and then returns
* __first. This routine is useful when we can't use a separate
* assertion statement because, e.g., we are in a constructor.
@@ -260,8 +279,9 @@ namespace __gnu_debug
 inline bool
 __check_sorted(const _InputIterator& __first, const _InputIterator& __last)
 {
-  return __check_sorted_aux(__first, __last,
-std::__iterator_category(__first));
+  return __skip_debug_runtime_check()
+	|| __check_sorted_aux(__first, __last,
+			  std::__iterator_category(__first));
 }
 
   template
@@ -270,8 +290,9 @@ namespace __gnu_debug
 __check_sorted(const _InputIterator& __first, const _InputIterator& __last,
_Predicate __pred)
 {
-  return __check_sorted_aux(__first, __last, __pred,
-std::__iterator_category(__first));
+  return __skip_debug_runtime_check()
+	|| __check_sorted_aux(__first, __last, __pred,
+			  std::__iterator_category(__first));
 }
 
   template
@@ -351,6 +372,9 @@ namespace __gnu_debug
 __check_partitioned_lower(_ForwardIterator __first,
 			  _ForwardIterator __last, const _Tp& __value)
 {
+  if (__skip_debug_runtime_check())
+	return true;
+
   while (__first != __last && *__first < __value)
 	++__first;
   if (__first != __last)
@@ -368,6 +392,9 @@ namespace __gnu_debug
 __check_partitioned_upper(_ForwardIterator __first,
 			  _ForwardIterator __last, const _Tp& __value)
 {
+  if (__skip_debug_runtime_check())
+	return true;
+
   while (__first != __last && !(__value < *__first))
 	++__first;
   if (__first != __last)
@@ -387,6 +414,9 @@ namespace __gnu_debug
 			  _ForwardIterator __last, const _Tp& __value,
 			  _Pred __pred)
 {
+  if (__skip_debug_runtime_check())
+	return true;
+
   while (__first != __last && bool(__pred(*__first, __value)))
 	++__first;
   if (__first != __last)
@@ -405,6 +435,9 @@ namespace __gnu_debug
 			  _ForwardIterator __last, const _Tp& __value,
 			  _Pred __pred)
 {
+  if (__skip_debug_runtime_check())
+	return true;
+
   while (__first != __last && !bool(__pred(__value, *__first)))
 	++__first;
   if (__first != __last)
diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h
index c0144ced979..587eba2f3e5 100644
--- a/libstdc++-v3/include/debug/helper_functions.h
+++ b/libstdc++-v3/include/debug/helper_functions.h
@@ -30,8 +30,8 @@
 #define _GLIBCXX_DEBUG_HELPER_FUNCTIONS_H 1
 
 #include // for __addressof
-#include 	// for iterator_traits,
-		// categories and _Iter_base
+#include 	// for iterator_traits and
+		// categories
 #include 		// for __is_integer
 
 #include 			// for pair



Re: [PATCH] Add gnu::diagnose_as attribute

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/4/21 7:13 AM, Matthias Kretz wrote:

From: Matthias Kretz 

This attribute overrides the diagnostics output string for the entity it
appertains to. The motivation is to improve QoI for library TS
implementations, where diagnostics have a very bad signal-to-noise ratio
due to the long namespaces involved.



On Tuesday, 27 April 2021 11:46:48 CEST Jonathan Wakely wrote:

I think it's a great idea and would like to use it for all the TS
implementations where there is some inline namespace that the user
doesn't care about. std::experimental::fundamentals_v1:: would be much
better as just std::experimental::, or something like std::[LFTS]::.


Hmm, how much of the benefit could we get from a flag (probably on by 
default) to skip inline namespaces in diagnostics?



With the attribute, it is possible to solve PR89370 and make
std::__cxx11::basic_string<_CharT, _Traits, _Alloc> appear as
std::string in diagnostic output without extra hacks to recognize the
type.


That sounds wrong to me; std::string is the  instantiation, not 
the template.  Your patch doesn't make it possible to apply this 
attribute to class template instantiations, does it?


Jason



Re: [PATCH] handle erroneous types in attribute nonnull (PR 100783)

2021-05-27 Thread Jeff Law via Gcc-patches




On 5/27/2021 10:00 AM, Martin Sebor via Gcc-patches wrote:

When attribute nonnull is applied to an argument of an erroneous
type the attribute positional argument validation function ICEs
while printing a warning that mentions the invalid type.

The attached patch changes the validation function to ignore
erroneous types on the assumption that they must have already
been diagnosed.  It also enhances the pretty-printer to detect
erroneous types and print "{erroneous}" instead of causing
an ICE.  There already is code in the pretty printer that tries
to be robust in thew presence of erroneous types without actually
printing them, and this extends the detection to also print them.
(With the first part of this patch I don't have a test case for
it but handling the condition gracefully feels preferable to
waiting for another bug report with invalid code triggering
an ICE).

Martin

gcc-100783.diff

PR c/100783 - ICE on -Wnonnull and erroneous type

gcc/c-family/ChangeLog:

PR c/100783
* c-attribs.c (positional_argument): Bail on erroneous types.

gcc/c/ChangeLog:

PR c/100783
* c-objc-common.c (print_type): Handle erroneous types.

gcc/testsuite/ChangeLog:

PR c/100783
* gcc.dg/nonnull-6.c: New test.

OK
jeff



Re: [PATCH] c++: parameter pack inside static_assert [PR99893]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 11:05 AM, Patrick Palka wrote:

Here, we're not finding the parameter pack inside the static_assert because
STATIC_ASSERT trees are tcc_exceptional, and we weren't explicitly walking
them in cp_walk_subtrees.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?


OK.


gcc/cp/ChangeLog:

PR c++/99893
* tree.c (cp_walk_subtrees) : New case.

gcc/testsuite/ChangeLog:

PR c++/99893
* g++.dg/cpp0x/static_assert17.C: New test.
---
  gcc/cp/tree.c| 5 +
  gcc/testsuite/g++.dg/cpp0x/static_assert17.C | 9 +
  2 files changed, 14 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/static_assert17.C

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 372d89fa9ed..fec5afaa2be 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5446,6 +5446,11 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, 
walk_tree_fn func,
}
break;
  
+case STATIC_ASSERT:

+  WALK_SUBTREE (STATIC_ASSERT_CONDITION (*tp));
+  WALK_SUBTREE (STATIC_ASSERT_MESSAGE (*tp));
+  break;
+
  default:
return NULL_TREE;
  }
diff --git a/gcc/testsuite/g++.dg/cpp0x/static_assert17.C 
b/gcc/testsuite/g++.dg/cpp0x/static_assert17.C
new file mode 100644
index 000..64843c60edd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/static_assert17.C
@@ -0,0 +1,9 @@
+// PR c++/99893
+// { dg-do compile { target c++11 } }
+
+void f(...);
+
+template 
+void g() {
+  f([] { static_assert(Ts::value, ""); }...);
+}





[pushed] c++: argument pack with expansion [PR86355]

2021-05-27 Thread Jason Merrill via Gcc-patches
This testcase revealed that we were using PACK_EXPANSION_EXTRA_ARGS a lot
more than necessary; use_pack_expansion_extra_args_p meant to use it in the
case of corresponding arguments in different argument packs differing in
whether they are pack expansions, but it was mistakenly also returning true
for the case of a single argument pack containing both expansion and
non-expansion elements.

Surprisingly, just disabling that didn't lead to any regressions in the
testsuite; it seems other changes have prevented us getting to this point
for code that used to exercise it.  So this patch limits the check to
arguments in the same position in the packs, and asserts that we never
actually see a mismatch.

PR c++/86355

gcc/cp/ChangeLog:

* pt.c (use_pack_expansion_extra_args_p): Don't compare
args from the same argument pack.

gcc/testsuite/ChangeLog:

* g++.dg/cpp0x/alias-decl-variadic2.C: New test.
---
 gcc/cp/pt.c   |  7 +--
 gcc/testsuite/g++.dg/cpp0x/alias-decl-variadic2.C | 13 +
 2 files changed, 18 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-variadic2.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index e4950aa448a..bb22d685617 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12422,9 +12422,9 @@ use_pack_expansion_extra_args_p (tree parm_packs,
   return false;
 }
 
-  bool has_expansion_arg = false;
   for (int i = 0 ; i < arg_pack_len; ++i)
 {
+  bool has_expansion_arg = false;
   bool has_non_expansion_arg = false;
   for (tree parm_pack = parm_packs;
   parm_pack;
@@ -12444,7 +12444,10 @@ use_pack_expansion_extra_args_p (tree parm_packs,
}
 
   if (has_expansion_arg && has_non_expansion_arg)
-   return true;
+   {
+ gcc_checking_assert (false);
+ return true;
+   }
 }
   return false;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-variadic2.C 
b/gcc/testsuite/g++.dg/cpp0x/alias-decl-variadic2.C
new file mode 100644
index 000..4299c7e88dc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-variadic2.C
@@ -0,0 +1,13 @@
+// PR c++/86355
+// { dg-do compile { target c++11 } }
+
+template  struct integral_constant {
+  static const int value = 1;
+};
+template  using mp_all = integral_constant;
+template  using check2 = mp_all>>;
+check2<> x;
+
+template  struct assert_same;
+template  struct assert_same { };
+assert_same> a;

base-commit: 9b94785dedb08b006419bec1a402614d9241317a
-- 
2.27.0



[r12-1085 Regression] FAIL: gcc.target/i386/pr100637-3w.c scan-assembler pavgw on Linux/x86_64

2021-05-27 Thread sunil.k.pandey via Gcc-patches
On Linux/x86_64,

04ba00d4ed735242c5284d2c623a3a9d42d94742 is the first bad commit
commit 04ba00d4ed735242c5284d2c623a3a9d42d94742
Author: Uros Bizjak 
Date:   Thu May 27 09:22:01 2021 +0200

i386: Add uavg_ceil patterns for 4-byte vectors [PR100637]

caused

FAIL: gcc.target/i386/pr100637-3w.c scan-assembler pavgw
FAIL: gcc.target/i386/pr100637-3w.c scan-assembler pmulhrsw

with GCC configured with

../../gcc/configure 
--prefix=/local/skpandey/gccwork/toolwork/gcc-bisect-master/master/r12-1085/usr 
--enable-clocale=gnu --with-system-zlib --with-demangler-in-ld 
--with-fpmath=sse --enable-languages=c,c++,fortran --enable-cet --without-isl 
--enable-libmpx x86_64-linux --disable-bootstrap

To reproduce:

$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="i386.exp=gcc.target/i386/pr100637-3w.c --target_board='unix{-m32\ 
-march=cascadelake}'"
$ cd {build_dir}/gcc && make check 
RUNTESTFLAGS="i386.exp=gcc.target/i386/pr100637-3w.c --target_board='unix{-m64\ 
-march=cascadelake}'"

(Please do not reply to this email, for question about this report, contact me 
at skpgkp2 at gmail dot com)


Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Tobias Burnus

@Joseph: I CC'ed you in case you have comments regarding
c-parser.c's c_parser_check_balanced_raw_token_sequence (comment update)
and c_parser_check_tight_balanced_raw_token_sequence (new); the latter
is essentially cp_parser_skip_balanced_tokens with slight adaptions.

On 27.05.21 10:22, Jakub Jelinek via Gcc-patches wrote:

One important thing I've missed.
Unlike depend clause where dependence-type : is required, in affinity clause
the aff-modifier is optional.

Now handled for C, C++ and Fortran.

What happens with this block ns:

+  char name[GFC_MAX_SYMBOL_LEN + 1];
+  while (true)
+{
+  locus old_loc = gfc_current_locus;
+  if (gfc_match_type_spec (&ts) == MATCH_YES
+  && gfc_match (" :: ") == MATCH_YES)
+{
+  if (ts.type != BT_INTEGER)
+{
+  gfc_error ("Expected INTEGER type at %L", &old_loc);

In the error handling?


Well, in the old code nothing – until the parent namespace
(gfc_current_ns) is beeing freed. Otherwise, it is walked
at several places but not ns->symtree but only ns->code
or ns->entry or ...

As it now can be used in a non-error case, I decided to
free it inside openmp.c instead of leaving it there for
later cleanup.

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
OpenMP: Add iterator support to Fortran's depend; add affinity clause

gcc/c-family/ChangeLog:

	* c-pragma.h (enum pragma_omp_clause): Add PRAGMA_OMP_CLAUSE_AFFINITY.

gcc/c/ChangeLog:

	* c-parser.c (c_parser_check_balanced_raw_token_sequence): Update
	description.
	(c_parser_check_tight_balanced_raw_token_sequence,
	c_parser_omp_clause_affinity): New.
	(c_parser_omp_clause_name, c_parser_omp_variable_list,
	c_parser_omp_all_clauses, OMP_TASK_CLAUSE_MASK): Handle affinity clause.
	* c-typeck.c (handle_omp_array_sections_1, handle_omp_array_sections,
	c_finish_omp_clauses): Likewise.

gcc/cp/ChangeLog:

	* parser.c (cp_parser_omp_clause_affinity): New.
	(cp_parser_omp_clause_name, cp_parser_omp_var_list_no_open,
	cp_parser_omp_all_clauses, OMP_TASK_CLAUSE_MASK): Handle affinity
	clause.
	* semantics.c (handle_omp_array_sections_1, handle_omp_array_sections,
	finish_omp_clauses): Likewise.

gcc/fortran/ChangeLog:

	* dump-parse-tree.c (show_iterator): New.
	(show_omp_namelist): Handle iterators.
	(show_omp_clauses): Handle affinity.
	* gfortran.h (gfc_free_omp_namelist): New union with 'udr' and new 'ns'.
	* match.c (gfc_free_omp_namelist): Add are to choose union element.
	* openmp.c (gfc_free_omp_clauses, gfc_match_omp_detach): Update
	call to gfc_free_omp_namelist.
	(gfc_match_omp_variable_list): Likewise; permit preceeding whitespace.
	(enum omp_mask1):
	(gfc_match_iterator): New.
	(gfc_match_omp_clause_reduction):
	(gfc_match_omp_clauses):
	(gfc_match_omp_flush):
	(gfc_match_omp_taskwait): Match depend clause.
	(resolve_omp_clauses): Handle affinity; update for udr/union change.
	(gfc_resolve_omp_directive): Resolve clauses of taskwait.
	* st.c (gfc_free_statement): Update gfc_free_omp_namelist call.
	* trans-openmp.c (gfc_trans_omp_array_reduction_or_udr): Likewise
	(handle_iterator): New.
	(gfc_trans_omp_clauses): Handle iterators for depend/affinity clause.
	(gfc_trans_omp_taskwait): Handle depend clause.
	(gfc_trans_omp_directive): Update call.

gcc/ChangeLog:

	* (gimplify_scan_omp_clauses): Ignore affinity clause.
	* tree-core.h (enum omp_clause_code): Add OMP_CLAUSE_AFFINITY.
	* tree-pretty-print.c (dump_omp_clause): Handle OMP_CLAUSE_AFFINITY.
	* tree.c (omp_clause_num_ops, omp_clause_code_name): Add clause.
	(walk_tree_1): Handle OMP_CLAUSE_AFFINITY.

libgomp/ChangeLog:

	* testsuite/libgomp.fortran/depend-iterator-2.f90: New test.

gcc/testsuite/ChangeLog:

	* c-c++-common/gomp/affinity-1.c: New test.
	* c-c++-common/gomp/affinity-2.c: New test.
	* c-c++-common/gomp/affinity-3.c: New test.
	* c-c++-common/gomp/affinity-4.c: New test.
	* c-c++-common/gomp/affinity-5.c: New test.
	* c-c++-common/gomp/affinity-6.c: New test.
	* c-c++-common/gomp/affinity-7.c: New test.
	* gfortran.dg/gomp/affinity-clause-1.f90: New test.
	* gfortran.dg/gomp/affinity-clause-2.f90: New test.
	* gfortran.dg/gomp/affinity-clause-3.f90: New test.
	* gfortran.dg/gomp/affinity-clause-4.f90: New test.
	* gfortran.dg/gomp/affinity-clause-5.f90: New test.
	* gfortran.dg/gomp/affinity-clause-6.f90: New test.
	* gfortran.dg/gomp/depend-iterator-1.f90: New test.
	* gfortran.dg/gomp/depend-iterator-2.f90: New test.
	* gfortran.dg/gomp/depend-iterator-3.f90: New test.
	* gfortran.dg/gomp/taskwait.f90: New test.

 gcc/c-family/c-pragma.h|   1 +
 gcc/c/c-parser.c   | 124 -
 gcc/c/c-typeck.c   |  52 ++--
 gcc/cp/parser.c|  84 +-
 gcc/cp/semantics.c |  53 ++--
 gcc/fortran/dump-parse-tree.c 

Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Jakub Jelinek via Gcc-patches
On Thu, May 27, 2021 at 08:30:33PM +0200, Tobias Burnus wrote:
> +  if (c_parser_next_token_is (parser, CPP_NAME))
> +{
> +  const char *p = IDENTIFIER_POINTER (c_parser_peek_token 
> (parser)->value);
> +  bool parse_iter = (strcmp ("iterator", p) == 0);
> +  if (parse_iter)

I'd add here && c_parser_peek_2nd_token (parser)->type == CPP_OPEN_PAREN

> +  if (parse_iter)

And similarly here && cp_lexer_nth_token_is (parser->lexer, 2, CPP_OPEN_PAREN)

Otherwise LGTM, but please give Joseph some time to comment on the new
function.

Jakub



Re: [PATCH] Add gnu::diagnose_as attribute

2021-05-27 Thread Matthias Kretz
On Thursday, 27 May 2021 19:39:48 CEST Jason Merrill wrote:
> On 5/4/21 7:13 AM, Matthias Kretz wrote:
> > From: Matthias Kretz 
> > 
> > This attribute overrides the diagnostics output string for the entity it
> > appertains to. The motivation is to improve QoI for library TS
> > implementations, where diagnostics have a very bad signal-to-noise ratio
> > due to the long namespaces involved.
> > 
> > On Tuesday, 27 April 2021 11:46:48 CEST Jonathan Wakely wrote:
> >> I think it's a great idea and would like to use it for all the TS
> >> implementations where there is some inline namespace that the user
> >> doesn't care about. std::experimental::fundamentals_v1:: would be much
> >> better as just std::experimental::, or something like std::[LFTS]::.
> 
> Hmm, how much of the benefit could we get from a flag (probably on by
> default) to skip inline namespaces in diagnostics?

I'd say about 20% for the TS's. Even std::experimental::simd (i.e. without the 
'::parallelism_v2' part) is still rather noisy. I want stdₓ::simd, std-x::simd 
or std::[PTS2]::simd or whatever shortest shorthand Jonathan will allow. ;)

For PR89370, the benefit would be ~2%:

'template std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::_If_sv<_Tp, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&> 
std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::size_type, const _Tp&, std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::size_type, std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits = 
std::char_traits; _Alloc = std::allocator]'

would then only turn into:

'template std::basic_string<_CharT, _Traits, 
_Alloc>::_If_sv<_Tp, std::basic_string<_CharT, _Traits, _Alloc>&> 
std::basic_string<_CharT, _Traits, 
_Alloc>::insert(std::basic_string<_CharT, _Traits, 
_Alloc>::size_type, const _Tp&, std::basic_string<_CharT, _Traits, 
_Alloc>::size_type, std::basic_string<_CharT, _Traits, 
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits = 
std::char_traits; _Alloc = std::allocator]'

instead of:

'template std::string::_If_sv<_Tp, std::string&> 
std::string::insert<_Tp>(std::string::size_type, const _Tp&, 
std::string::size_type, std::string::size_type)'


Also hiding all inline namespace by default might make some error messages 
harder to understand:

namespace Vir {
  inline namespace foo {
struct A {};
  }
  struct A {};
}
using Vir::A;


:7:12: error: reference to 'A' is ambiguous
:3:12: note: candidates are: 'struct Vir::A'
:5:10: note: 'struct Vir::A'

> > With the attribute, it is possible to solve PR89370 and make
> > std::__cxx11::basic_string<_CharT, _Traits, _Alloc> appear as
> > std::string in diagnostic output without extra hacks to recognize the
> > type.
> 
> That sounds wrong to me; std::string is the  instantiation, not
> the template.  Your patch doesn't make it possible to apply this
> attribute to class template instantiations, does it?

Yes, it does.

Initially, when I tried to improve the TS experience, it didn't. When Jonathan 
showed PR89370 to me I tried to make [[gnu::diagnose_as]] more generic & 
useful. Since there's no obvious syntax to apply an attribute to a template 
instantiation, I had to be creative. This is from my pending std::string 
patch:

--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -299,7 +299,8 @@ namespace std
 #if _GLIBCXX_USE_CXX11_ABI
 namespace std
 {
-  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+  inline namespace __cxx11
+__attribute__((__abi_tag__ ("cxx11"), __diagnose_as__("std"))) { }
 }
 namespace __gnu_cxx
 {
--- a/libstdc++-v3/include/bits/stringfwd.h
+++ b/libstdc++-v3/include/bits/stringfwd.h
@@ -76,24 +76,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 _GLIBCXX_END_NAMESPACE_CXX11
 
   /// A string of @c char
-  typedef basic_stringstring;   
+  typedef basic_string string [[__gnu__::__diagnose_as__("string")]];
 
 #ifdef _GLIBCXX_USE_WCHAR_T
   /// A string of @c wchar_t
-  typedef basic_string wstring;   
+  typedef basic_string wstring
 [[__gnu__::__diagnose_as__("wstring")]];
 #endif
[...]

The part of my frontend patch that makes this work is in 
handle_diagnose_as_attribute:

+  if (TREE_CODE (*node) == TYPE_DECL)
+{
+  // Apply the attribute to the type alias itself.
+  decl = *node;
+  tree type = TREE_TYPE (*node);
+  if (CLASS_TYPE_P (type) && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
+   {
+ if (COMPLETE_OR_OPEN_TYPE_P (type))
+   warning (OPT_Wattributes,
+"ignoring %qE attribute applied to template %qT after "
+"instantiation", name, type);
+ else
+   {
+ type = strip_typedefs(type, nullptr, 0);
+ // And apply the attribute to the specialization on the RHS.
+ tree attributes = tree_cons (name, args, NULL_TREE);
+   

[PING 3][PATCH] define auto_vec copy ctor and assignment (PR 90904)

2021-05-27 Thread Martin Sebor via Gcc-patches

Jason, I wonder if you could review this patch that Richard has
apparently decided to defer to others.

https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568901.html

Thanks

On 5/11/21 2:02 PM, Martin Sebor wrote:

Ping 2:
https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568901.html


On 5/3/21 3:50 PM, Martin Sebor wrote:

Ping:

https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568901.html

On 4/27/21 9:52 AM, Martin Sebor wrote:

On 4/27/21 8:04 AM, Richard Biener wrote:

On Tue, Apr 27, 2021 at 3:59 PM Martin Sebor  wrote:


On 4/27/21 1:58 AM, Richard Biener wrote:

On Tue, Apr 27, 2021 at 2:46 AM Martin Sebor via Gcc-patches
 wrote:


PR 90904 notes that auto_vec is unsafe to copy and assign because
the class manages its own memory but doesn't define (or delete)
either special function.  Since I first ran into the problem,
auto_vec has grown a move ctor and move assignment from
a dynamically-allocated vec but still no copy ctor or copy
assignment operator.

The attached patch adds the two special functions to auto_vec along
with a few simple tests.  It makes auto_vec safe to use in 
containers
that expect copyable and assignable element types and passes 
bootstrap

and regression testing on x86_64-linux.


The question is whether we want such uses to appear since those
can be quite inefficient?  Thus the option is to delete those 
operators?


I would strongly prefer the generic vector class to have the 
properties

expected of any other generic container: copyable and assignable.  If
we also want another vector type with this restriction I suggest to 
add
another "noncopyable" type and make that property explicit in its 
name.

I can submit one in a followup patch if you think we need one.


I'm not sure (and not strictly against the copy and assign).  
Looking around

I see that vec<> does not do deep copying.  Making auto_vec<> do it
might be surprising (I added the move capability to match how vec<>
is used - as "reference" to a vector)


The vec base classes are special: they have no ctors at all (because
of their use in unions).  That's something we might have to live with
but it's not a model to follow in ordinary containers.

The auto_vec class was introduced to fill the need for a conventional
sequence container with a ctor and dtor.  The missing copy ctor and
assignment operators were an oversight, not a deliberate feature.
This change fixes that oversight.

The revised patch also adds a copy ctor/assignment to the auto_vec
primary template (that's also missing it).  In addition, it adds
a new class called auto_vec_ncopy that disables copying and
assignment as you prefer.  It also disables copying for
the auto_string_vec class.

Martin








Re: [PATCH][version 3]add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc

2021-05-27 Thread Qing Zhao via Gcc-patches
Hi, Richard,

Thanks a lot for your comments.


On May 26, 2021, at 6:18 AM, Richard Biener 
mailto:rguent...@suse.de>> wrote:

On Wed, 12 May 2021, Qing Zhao wrote:

Hi,

This is the 3rd version of the patch for the new security feature for GCC.

Please take look and let me know your comments and suggestions.

thanks.

Qing

**Compare with the 2nd version, the following are the major changes:

1. use "lookup_attribute ("uninitialized",) directly instead of adding
  one new field "uninitialized" into tree_decl_with_vis.
2. update documentation to mention that the new option will not confuse
  -Wuninitialized, GCC still consider an auto without explicit initializer
  as uninitialized.
3. change the name of "build_pattern_cst" to more specific name as
  "build_pattern_cst_for_auto_init".
4. handling of nested VLA;
  Adding new testing cases (auto-init-15/16.c) for this new handling.
5. Add  new verifications of calls to .DEFERRED_INIT in tree-cfg.c;
6. in tree-sra.c, update the handling of "grp_to_be_debug_replaced",
  bind the lhs variable to a call to .DEFERRED_INIT.
7. In tree-ssa-structalias.c, delete "find_func_aliases_for_deferred_init",
  return directly for a call to .DEFERRED_INIT in "find_func_aliases_for_call".
8. Add more detailed comments in tree-ssa-uninit.c and tree-ssa.c to explain
  the special handling on REALPART_EXPR/IMAGPRT_EXPR.
9. in build_pattern_cst_for_auto_init:
  BOOLEAN_TYPE will be set to zero always;
  INTEGER_TYPE (?and ENUMERAL_TYPE) use wi::from_buffer in order to
   correctly handle 128-bit integers.
  POINTER_TYPE will not assert on SIZE < 32.
  REAL_TYPE add fallback;
10. changed gcc_assert to gcc_unreachable in several places;
11. add more comments;
12. some style issue changes.

**Please see the version 2 at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567262.html


**The following 2 items are the ones I didn’t addressed in this version due 
to further study and might need more discussion:

1. Using __builtin_clear_padding  to replace type_has_padding.

My study shows: the call to __builtin_clear_padding is expanded during 
gimplification phase.
And there is no __bultin_clear_padding expanding during rtx expanding phase.
If so,  for -ftrivial-auto-var-init, padding initialization should be done both 
in gimplification phase and rtx expanding phase.
And since the __builtin_clear_padding might not be good for rtx expanding, 
reusing __builtin_clear_padding might not work.

2. Pattern init to NULLPTR_TYPE and ENUMERAL_TYPE: need more comments from 
Richard Biener on this.

**The change of the 3rd version compared to the 2nd version are:


+@item -ftrivial-auto-var-init=@var{choice}
+@opindex ftrivial-auto-var-init
+Initialize automatic variables with either a pattern or with zeroes to
increase
+the security and predictability of a program by preventing uninitialized
memory
+disclosure and use.

the docs do not state what "trivial" actually means?  Does it affect
C++ classes with ctors, thus is "trivial" equal to what C++ considers
a POD type?

Thank you for this question.

The name -ftrivial-auto-var-init is just for compatible with Clang. I really 
don’t know why
they added trivial.

As I checked a small example with C++ class with ctors, I see both Clang and my 
patch add
Initialization to this class:

=
#include 

using namespace std;

class Line {
   public:
  void setLength( double len );
  double getLength( void );
  Line();  // This is the constructor
   private:
  double length;
};

// Member functions definitions including constructor
Line::Line(void) {
   cout << "Object is being created" << endl;
}
void Line::setLength( double len ) {
  length = len;
}
double Line::getLength( void ) {
  return length;
}

// Main function for the program
int main() {
  Line line;

  // set line length
  line.setLength(6.0);
  cout << "Length of line : " << line.getLength() <
AUTO_INIT_UNINITIALIZED
+&& !TREE_STATIC (exp)
+&& type_has_padding (type)))

testing flag_trivial_auto_var_init tests the global options, if TUs
are compiled with different setting of flag_trivial_auto_var_init
and you use LTO or flag_trivial_auto_var_init is specified per
function via optimize attributes it's more appropriate to test
opt_for_fn (cfun->decl, flag_trivial_auto_var_init)

Okay.  Thanks for the info. I will update this.


You do not actually test whether TARGET is an auto-var in this place,
so I question this change

Will add checking for Auto on this.

- the documentation of ftrivial-auto-var-init
also doesn't mention initialization of padding

Will add initialization of padding on this.

Clang add the padding initialization with this option, I guess that we might 
need to
be compatible with it?

and the above doesn't
seem to honor -ftrivial-auto-var-init=pattern for this.

Richard Sandiford raised the same question in the previous review.
And this behavior also follows Clang’s behavior.

T

Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Joseph Myers
On Thu, 27 May 2021, Tobias Burnus wrote:

> @Joseph: I CC'ed you in case you have comments regarding
> c-parser.c's c_parser_check_balanced_raw_token_sequence (comment update)
> and c_parser_check_tight_balanced_raw_token_sequence (new); the latter
> is essentially cp_parser_skip_balanced_tokens with slight adaptions.

I don't understand why the name says either "tight" or "balanced".  As far 
as I can see, c_parser_check_tight_balanced_raw_token_sequence isn't 
checking for balanced token sequences (in the sense defined in C2x) at all 
and would accept e.g. }]{[ as being balanced.  Is that really what's 
supposed to be accepted there?  If it is, the comment on the function 
definition needs to explain the exact definition of what token sequences 
are accepted.

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


Re: [PATCH] Remove CC0

2021-05-27 Thread H.J. Lu via Gcc-patches
On Mon, May 03, 2021 at 10:55:36PM +, Segher Boessenkool wrote:
> This removes CC0 and all directly related infrastructure.
> 
> CC_STATUS, CC_STATUS_MDEP, CC_STATUS_MDEP_INIT, and NOTICE_UPDATE_CC
> are deleted and poisoned.  CC0 is only deleted (some targets use that
> name for something else).  HAVE_cc0 is automatically generated, and we
> no longer will do that after this patch.
> 
> CC_STATUS_INIT is suggested in final.c to also be useful for ports that
> are not CC0, and at least arm seems to use it for something.  So I am
> leaving that alone, but most targets that have it could remove it.
> 
> Is this okay for trunk?
> 
> 
> Segher
> 
> 
> 2021-05-03  Segher Boessenkool  
> 
>   * caller-save.c: Remove CC0.
>   * cfgcleanup.c: Remove CC0.
>   * cfgrtl.c: Remove CC0.
>   * combine.c: Remove CC0.
>   * compare-elim.c: Remove CC0.
>   * conditions.h: Remove CC0.
>   * config/h8300/h8300.h: Remove CC0.
>   * config/h8300/peepholes.md: Remove CC0.
>   * config/i386/x86-tune-sched.c: Remove CC0.
>   * config/m68k/m68k.c: Remove CC0.
>   * config/rl78/rl78.c: Remove CC0.
>   * config/sparc/sparc.c: Remove CC0.
>   * config/xtensa/xtensa.c: Remove CC0.
>   (gen_conditional_move):  Use pc_rtx instead of cc0_rtx in a piece of
>   RTL where that is used as a placeholder only.
>   * cprop.c: Remove CC0.
>   * cse.c: Remove CC0.
>   * cselib.c: Remove CC0.
>   * df-problems.c: Remove CC0.
>   * df-scan.c: Remove CC0.
>   * doc/md.texi: Remove CC0.  Adjust an example.
>   * doc/rtl.texi: Remove CC0.  Adjust an example.
>   * doc/tm.texi: Regenerate.
>   * doc/tm.texi.in: Remove CC0.
>   * emit-rtl.c: Remove CC0.
>   * final.c: Remove CC0.
>   * fwprop.c: Remove CC0.
>   * gcse-common.c: Remove CC0.
>   * gcse.c: Remove CC0.
>   * genattrtab.c: Remove CC0.
>   * genconfig.c: Remove CC0.
>   * genemit.c: Remove CC0.
>   * genextract.c: Remove CC0.
>   * gengenrtl.c: Remove CC0.
>   * genrecog.c: Remove CC0.
>   * haifa-sched.c: Remove CC0.
>   * ifcvt.c: Remove CC0.
>   * ira-costs.c: Remove CC0.
>   * ira.c: Remove CC0.
>   * jump.c: Remove CC0.
>   * loop-invariant.c: Remove CC0.
>   * lra-constraints.c: Remove CC0.
>   * lra-eliminations.c: Remove CC0.
>   * optabs.c: Remove CC0.
>   * postreload-gcse.c: Remove CC0.
>   * postreload.c: Remove CC0.
>   * print-rtl.c: Remove CC0.
>   * read-rtl-function.c: Remove CC0.
>   * reg-notes.def: Remove CC0.
>   * reg-stack.c: Remove CC0.
>   * reginfo.c: Remove CC0.
>   * regrename.c: Remove CC0.
>   * reload.c: Remove CC0.
>   * reload1.c: Remove CC0.
>   * reorg.c: Remove CC0.
>   * resource.c: Remove CC0.
>   * rtl.c: Remove CC0.
>   * rtl.def: Remove CC0.
>   * rtl.h: Remove CC0.
>   * rtlanal.c: Remove CC0.
>   * sched-deps.c: Remove CC0.
>   * sched-rgn.c: Remove CC0.
>   * shrink-wrap.c: Remove CC0.
>   * simplify-rtx.c: Remove CC0.
>   * system.h: Remove CC0.  Poison NOTICE_UPDATE_CC, CC_STATUS_MDEP_INIT,
>   CC_STATUS_MDEP, and CC_STATUS.
>   * target.def: Remove CC0.
>   * valtrack.c: Remove CC0.
>   * var-tracking.c: Remove CC0.
> 

You missed:

config/cr16/cr16.md:   (clobber (cc0))]
config/cr16/cr16.md:  [(parallel [(set (cc0)
config/cr16/cr16.md:  [(set (cc0)
config/cr16/cr16.md: [(cc0) (const_int 0)]))]
config/cr16/cr16.md:  [(set (cc0)
config/cr16/cr16.md: [(cc0) (const_int 0)]))]

Now, cr16-elf won't build.

H.J.


Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Jakub Jelinek via Gcc-patches
On Thu, May 27, 2021 at 07:58:03PM +, Joseph Myers wrote:
> On Thu, 27 May 2021, Tobias Burnus wrote:
> 
> > @Joseph: I CC'ed you in case you have comments regarding
> > c-parser.c's c_parser_check_balanced_raw_token_sequence (comment update)
> > and c_parser_check_tight_balanced_raw_token_sequence (new); the latter
> > is essentially cp_parser_skip_balanced_tokens with slight adaptions.
> 
> I don't understand why the name says either "tight" or "balanced".  As far 
> as I can see, c_parser_check_tight_balanced_raw_token_sequence isn't 
> checking for balanced token sequences (in the sense defined in C2x) at all 
> and would accept e.g. }]{[ as being balanced.  Is that really what's 
> supposed to be accepted there?  If it is, the comment on the function 
> definition needs to explain the exact definition of what token sequences 
> are accepted.

Not }]{[ but {[}], true.
I guess what Tobias posted comes from the C++ FE cp_parser_skip_balanced_tokens
which is apparently my fault.
The C++ grammar is:

balanced-token-seq:
balanced-token
balanced-token-seq balanced-token
balanced-token:
( balanced-token-seq opt )
[ balanced-token-seq opt ]
{ balanced-token-seq opt }
any token other than a parenthesis, a bracket, or a brace

so I bet we need something like the C c_parser_check_balanced_raw_token_sequence
instead in the C++ FE too.
For the particular OpenMP use case, I think it really doesn't matter which
one we use, if the opening and closing parens kinds/counts would match but
they wouldn't be balanced, it would be invalid in any case and we'd reported
it during parsing of the iterators.

Jakub



[PATCH v2] Add a target calls hook: TARGET_PUSH_ARGUMENT

2021-05-27 Thread H.J. Lu via Gcc-patches
1. Replace PUSH_ARGS with a target calls hook, TARGET_PUSH_ARGUMENT, which
takes an integer argument.  When it returns true, push instructions will
be used to pass outgoing arguments.  If the argument is nonzero, it is
the number of bytes to push and indicates the PUSH instruction usage is
optional so that the backend can decide if PUSH instructions should be
generated.  Otherwise, the argument is zero.
2. Implement x86 target hook which returns false when the number of bytes
to push is no less than 16 (8 for 32-bit targets) if vector load and store
can be used.
3. Remove target PUSH_ARGS definitions which return 0 as it is the same
as the default.
4. Define TARGET_PUSH_ARGUMENT of cr16 and m32c to always return true.

gcc/

PR target/100704
* calls.c (expand_call): Replace PUSH_ARGS with
targetm.calls.push_argument (0).
(emit_library_call_value_1): Likewise.
* defaults.h (PUSH_ARGS): Removed.
(PUSH_ARGS_REVERSED): Replace PUSH_ARGS with
targetm.calls.push_argument (0).
* expr.c (block_move_libcall_safe_for_call_parm): Likewise.
(emit_push_insn): Pass the number bytes to push to
targetm.calls.push_argument and pass 0 if ARGS_ADDR is 0.
* hooks.c (hook_bool_uint_true): New.
* hooks.h (hook_bool_uint_true): Likewise.
* rtlanal.c (nonzero_bits1): Replace PUSH_ARGS with
targetm.calls.push_argument (0).
* target.def (push_argument): Add a targetm.calls hook.
* targhooks.c (default_push_argument): New.
* targhooks.h (default_push_argument): Likewise.
* config/bpf/bpf.h (PUSH_ARGS): Removed.
* config/cr16/cr16.c (TARGET_PUSH_ARGUMENT): New.
* config/cr16/cr16.h (PUSH_ARGS): Removed.
* config/i386/i386.c (ix86_push_argument):
(TARGET_PUSH_ARGUMENT): Likewise.
* config/i386/i386.h (PUSH_ARGS): Removed.
* config/m32c/m32c.c (TARGET_PUSH_ARGUMENT): New.
* config/m32c/m32c.h (PUSH_ARGS): Removed.
* config/nios2/nios2.h (PUSH_ARGS): Likewise.
* config/pru/pru.h (PUSH_ARGS): Likewise.
* doc/tm.texi.in: Remove PUSH_ARGS documentation.  Add
TARGET_PUSH_ARGUMENT hook.
* doc/tm.texi: Regenerated.

gcc/testsuite/

PR target/100704
* gcc.target/i386/pr100704-1.c: New test.
* gcc.target/i386/pr100704-2.c: Likewise.
* gcc.target/i386/pr100704-3.c: Likewise.
---
 gcc/calls.c|  6 +++---
 gcc/config/bpf/bpf.h   |  3 ---
 gcc/config/cr16/cr16.c |  2 ++
 gcc/config/cr16/cr16.h |  2 --
 gcc/config/i386/i386.c | 14 +
 gcc/config/i386/i386.h |  7 +--
 gcc/config/m32c/m32c.c |  3 +++
 gcc/config/m32c/m32c.h |  1 -
 gcc/config/nios2/nios2.h   |  1 -
 gcc/config/pru/pru.h   |  1 -
 gcc/defaults.h | 11 +-
 gcc/doc/tm.texi| 19 +
 gcc/doc/tm.texi.in |  9 +---
 gcc/expr.c | 14 ++---
 gcc/hooks.c|  8 
 gcc/hooks.h|  1 +
 gcc/rtlanal.c  |  2 +-
 gcc/target.def | 14 +
 gcc/targhooks.c| 12 +++
 gcc/targhooks.h|  1 +
 gcc/testsuite/gcc.target/i386/pr100704-1.c | 24 ++
 gcc/testsuite/gcc.target/i386/pr100704-2.c | 23 +
 gcc/testsuite/gcc.target/i386/pr100704-3.c | 20 ++
 23 files changed, 151 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr100704-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr100704-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr100704-3.c

diff --git a/gcc/calls.c b/gcc/calls.c
index f3da1839dc5..336b05f01c4 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -3727,7 +3727,7 @@ expand_call (tree exp, rtx target, int ignore)
  So the entire argument block must then be preallocated (i.e., we
  ignore PUSH_ROUNDING in that case).  */
 
-  int must_preallocate = !PUSH_ARGS;
+  int must_preallocate = !targetm.calls.push_argument (0);
 
   /* Size of the stack reserved for parameter registers.  */
   int reg_parm_stack_space = 0;
@@ -3836,7 +3836,7 @@ expand_call (tree exp, rtx target, int ignore)
 #endif
 
   if (! OUTGOING_REG_PARM_STACK_SPACE ((!fndecl ? fntype : TREE_TYPE (fndecl)))
-  && reg_parm_stack_space > 0 && PUSH_ARGS)
+  && reg_parm_stack_space > 0 && targetm.calls.push_argument (0))
 must_preallocate = 1;
 
   /* Set up a place to return a structure.  */
@@ -5477,7 +5477,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx 
value,
 }
   else
 {
-  if (

[PATCH] PR fortran/99839 - [9/10/11/12 Regression] ICE in inline_matmul_assign, at fortran/frontend-passes.c:4234

2021-05-27 Thread Harald Anlauf via Gcc-patches
Dear Fortranners,

frontend optimization tries to inline matmul, but then it also needs
to take care of the assignment to the result array.  If that one is
not of canonical type, we currently get an ICE.  The straightforward
solution is to simply punt in those cases and avoid inlining.

Regtested on x86_64-pc-linux-gnu.

OK for mainline?  Backport to affected branches?

Thanks,
Harald


Fortran - ICE in inline_matmul_assign

Restrict inlining of matmul to those cases where assignment to the
result array does not need special treatment.

gcc/fortran/ChangeLog:

PR fortran/99839
* frontend-passes.c (inline_matmul_assign): Do not inline matmul
if the assignment to the resulting array if it is not of canonical
type (real/integer/complex/logical).

gcc/testsuite/ChangeLog:

PR fortran/99839
* gfortran.dg/inline_matmul_25.f90: New test.

diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c
index ffe2db4881d..8aa4cf0eca7 100644
--- a/gcc/fortran/frontend-passes.c
+++ b/gcc/fortran/frontend-passes.c
@@ -4193,6 +4193,19 @@ inline_matmul_assign (gfc_code **c, int *walk_subtrees,
   if (m_case == none)
 return 0;

+  /* We only handle assignment to numeric or logical variables.  */
+  switch(expr1->ts.type)
+{
+case BT_INTEGER:
+case BT_LOGICAL:
+case BT_REAL:
+case BT_COMPLEX:
+  break;
+
+default:
+  return 0;
+}
+
   ns = insert_block ();

   /* Assign the type of the zero expression for initializing the resulting
diff --git a/gcc/testsuite/gfortran.dg/inline_matmul_25.f90 b/gcc/testsuite/gfortran.dg/inline_matmul_25.f90
new file mode 100644
index 000..df8ad06c123
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/inline_matmul_25.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! { dg-options "-ffrontend-optimize" }
+! PR fortran/99839 - ICE in inline_matmul_assign
+
+program p
+  real :: x(3, 3) = 1.0
+  class(*), allocatable :: z(:, :)
+  z = matmul(x, x)
+end


Re: [PATCH] Pass the number of bytes to push to PUSH_ARGS

2021-05-27 Thread H.J. Lu via Gcc-patches
On Mon, May 24, 2021 at 1:23 PM Joseph Myers  wrote:
>
> On Sat, 22 May 2021, H.J. Lu via Gcc-patches wrote:
>
> > 1. Update PUSH_ARGS to accept an argument.  When the PUSH instruction
> > usage is optional, pass the number of bytes to push to PUSH_ARGS so that
> > the backend can decide if PUSH instructions should be generated.
> > 2. Change x86 PUSH_ARGS to return 0 when number of bytes to push is more
> > than a word to avoid generating non-existent PUSH instructions.
> > 3. Remove target PUSH_ARGS definitions which return 0 as it is the same
> > as the default.
>
> If you're changing all users and definitions of PUSH_ARGS anyway, it
> probably makes sense to change it to a target hook.

I sent out the v2 patch to add TARGET_PUSH_ARGUMENT.

-- 
H.J.


[PATCH] i386: Remove unneeded binary operand fixup from expanders.

2021-05-27 Thread Uros Bizjak via Gcc-patches
There is no need to call ix86_fixup_binary_operands when there are
only one or no memory operands allowed.

2021-05-27  Uroš Bizjak  

gcc/
* config/i386/mmx.md (addv2sf3): Do not call
ix86_fixup_binary_operands_no_copy.
(subv2sf3): Ditto.
(mulv2sf3): Ditto.
(v2sf3): Ditto.
(3): Ditto.
(3): Remove expander.
(3): Rename from
"*3".
(mulv4hi): Do not call ix86_fixup_binary_operands_no_copy.
(mulv2hi3): Remove expander.
(mulv2hi3): Rename from *mulv2hi3.
(mulv2hi3_highpart): Remove expander.
(mulv2hi3_highpart): Rename from *mulv2hi3_highpart.
(3): Rename from
"*mmx_3".
(3): Remove expander.
(SMAXMIN_MMXMODEI): Remove mode iterator.
(v4hi3): New expander.
(v4qi3): Rename from *v4qi3.
(v2hi3): Rename from *v2hi3.
(3): Remove expander.
(SMAXMIN_VI_32): Remove mode iterator.
(3): Rename from
"*mmx_3".
(3): Remove expander.
(UMAXMIN_MMXMODEI): Remove mode iterator.
(v8qi3): New expander.
(v4qi3): Rename from *v4qi3.
(v2hi3): Rename from *v2hi3.
(3): Remove expander.
(UMAXMIN_VI_32): Remove mode iterator.
(v2hi3): Remove expander.
(v2hi3): Rename from *v2hi3.
(3): Do not call
ix86_fixup_binary_operands_no_copy.
(3): Remove expander.
(3): Rename from
"*3".
(uavg3_ceil): Do not call ix86_fixup_binary_operands_no_copy.
* config/i386/sse.md (div3): Do not call
ix86_fixup_binary_operands_no_copy.
(div3): Ditto.
(3): Ditto.
(smulhrsv4hi3): Ditto.
(smulhrsv2hi3): Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Pushed to master.

Uros.
diff --git a/gcc/config/i386/mmx.md b/gcc/config/i386/mmx.md
index 35e4123fa25..f39e062ddfc 100644
--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -417,8 +417,7 @@ (define_expand "addv2sf3"
(plus:V2SF
  (match_operand:V2SF 1 "register_operand")
  (match_operand:V2SF 2 "register_operand")))]
-  "TARGET_MMX_WITH_SSE"
-  "ix86_fixup_binary_operands_no_copy (PLUS, V2SFmode, operands);")
+  "TARGET_MMX_WITH_SSE")
 
 (define_insn "*mmx_addv2sf3"
   [(set (match_operand:V2SF 0 "register_operand" "=y,x,v")
@@ -455,8 +454,7 @@ (define_expand "subv2sf3"
(minus:V2SF
  (match_operand:V2SF 1 "register_operand")
  (match_operand:V2SF 2 "register_operand")))]
-  "TARGET_MMX_WITH_SSE"
-  "ix86_fixup_binary_operands_no_copy (MINUS, V2SFmode, operands);")
+  "TARGET_MMX_WITH_SSE")
 
 (define_insn "*mmx_subv2sf3"
   [(set (match_operand:V2SF 0 "register_operand" "=y,y,x,v")
@@ -489,8 +487,7 @@ (define_expand "mulv2sf3"
(mult:V2SF
  (match_operand:V2SF 1 "register_operand")
  (match_operand:V2SF 2 "register_operand")))]
-  "TARGET_MMX_WITH_SSE"
-  "ix86_fixup_binary_operands_no_copy (MULT, V2SFmode, operands);")
+  "TARGET_MMX_WITH_SSE")
 
 (define_insn "*mmx_mulv2sf3"
   [(set (match_operand:V2SF 0 "register_operand" "=y,x,v")
@@ -542,8 +539,6 @@ (define_expand "v2sf3"
 (operands[0], operands[1], operands[2]));
   DONE;
 }
-  else
-ix86_fixup_binary_operands_no_copy (, V2SFmode, operands);
 })
 
 ;; These versions of the min/max patterns are intentionally ignorant of
@@ -709,7 +704,7 @@ (define_insn "*mmx_haddv2sf3_low"
  (vec_select:SF
(match_dup 1)
(parallel [(match_operand:SI 3 "const_0_to_1_operand")]]
-  "TARGET_MMX_WITH_SSE && TARGET_SSE3
+  "TARGET_SSE3 && TARGET_MMX_WITH_SSE
&& INTVAL (operands[2]) != INTVAL (operands[3])"
   "@
haddps\t{%0, %0|%0, %0}
@@ -747,7 +742,7 @@ (define_insn "*mmx_hsubv2sf3_low"
  (vec_select:SF
(match_dup 1)
(parallel [(const_int 1)]]
-  "TARGET_MMX_WITH_SSE && TARGET_SSE3"
+  "TARGET_SSE3 && TARGET_MMX_WITH_SSE"
   "@
hsubps\t{%0, %0|%0, %0}
vhsubps\t{%1, %1, %0|%0, %1, %1}"
@@ -1091,7 +1086,7 @@ (define_expand "fix_truncv2sfv2si2"
 (define_insn "fixuns_truncv2sfv2si2"
   [(set (match_operand:V2SI 0 "register_operand" "=v")
(unsigned_fix:V2SI (match_operand:V2SF 1 "register_operand" "v")))]
-  "TARGET_MMX_WITH_SSE && TARGET_AVX512VL"
+  "TARGET_AVX512VL && TARGET_MMX_WITH_SSE"
   "vcvttps2udq\t{%1, %0|%0, %1}"
   [(set_attr "type" "ssecvt")
(set_attr "prefix" "evex")
@@ -1119,7 +1114,7 @@ (define_expand "floatv2siv2sf2"
 (define_insn "floatunsv2siv2sf2"
   [(set (match_operand:V2SF 0 "register_operand" "=v")
(unsigned_float:V2SF (match_operand:V2SI 1 "register_operand" "v")))]
-  "TARGET_MMX_WITH_SSE && TARGET_AVX512VL"
+  "TARGET_AVX512VL && TARGET_MMX_WITH_SSE"
   "vcvtudq2ps\t{%1, %0|%0, %1}"
   [(set_attr "type" "ssecvt")
(set_attr "prefix" "evex")
@@ -1361,8 +1356,7 @@ (define_expand "3"
(plusminus:MMXMODEI
  (match_operand:MMXMODEI 1 "register_operand")
  (match_operand:MMXMODEI 2 "register_operand")))]
-  "TARGET_MMX_WITH_SSE"
-  "ix86_fixup_binary_operands_no_copy (, mode, operands);")
+  "TARGET_MMX_WITH_SSE")
 
 (d

Re: [PATCH 2/11] use xxx_no_warning APIs in Ada

2021-05-27 Thread Martin Sebor via Gcc-patches

On 5/25/21 2:59 AM, Eric Botcazou wrote:

[PATCH 2/11] use xxx_no_warning APIs in Ada.


Looks good to me, but remove the useless pair of parentheses in the 3rd hunk.



The hunk was actually incorrect, thanks for drawing my attention
to it!  The second argument to the function is the option.  To
suppress all warnings it should have been:

diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 1786fbf8186..f2a6f44da76 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -836,7 +836,7 @@ gnat_pushdecl (tree decl, Node_Id gnat_node)
   if (!deferred_decl_context)
 DECL_CONTEXT (decl) = context;

-  TREE_NO_WARNING (decl) = (No (gnat_node) || Warnings_Off (gnat_node));
+  set_no_warning (decl, -1, No (gnat_node) || Warnings_Off (gnat_node));

   /* Set the location of DECL and emit a declaration for it.  */
   if (Present (gnat_node) && !renaming_from_instantiation_p (gnat_node))

This mistake will be caught when I change the functions to take
enum opt_code instead of int as David suggested.

Martin


Re: [PATCH] define auto_vec copy ctor and assignment (PR 90904)

2021-05-27 Thread Jason Merrill via Gcc-patches

On 4/27/21 11:52 AM, Martin Sebor via Gcc-patches wrote:

On 4/27/21 8:04 AM, Richard Biener wrote:

On Tue, Apr 27, 2021 at 3:59 PM Martin Sebor  wrote:


On 4/27/21 1:58 AM, Richard Biener wrote:

On Tue, Apr 27, 2021 at 2:46 AM Martin Sebor via Gcc-patches
 wrote:


PR 90904 notes that auto_vec is unsafe to copy and assign because
the class manages its own memory but doesn't define (or delete)
either special function.  Since I first ran into the problem,
auto_vec has grown a move ctor and move assignment from
a dynamically-allocated vec but still no copy ctor or copy
assignment operator.

The attached patch adds the two special functions to auto_vec along
with a few simple tests.  It makes auto_vec safe to use in containers
that expect copyable and assignable element types and passes bootstrap
and regression testing on x86_64-linux.


The question is whether we want such uses to appear since those
can be quite inefficient?  Thus the option is to delete those 
operators?


I would strongly prefer the generic vector class to have the properties
expected of any other generic container: copyable and assignable.  If
we also want another vector type with this restriction I suggest to add
another "noncopyable" type and make that property explicit in its name.
I can submit one in a followup patch if you think we need one.


I'm not sure (and not strictly against the copy and assign).  Looking 
around

I see that vec<> does not do deep copying.  Making auto_vec<> do it
might be surprising (I added the move capability to match how vec<>
is used - as "reference" to a vector)


The vec base classes are special: they have no ctors at all (because
of their use in unions).  That's something we might have to live with
but it's not a model to follow in ordinary containers.


I don't think we have to live with it anymore, now that we're writing C++11.


The auto_vec class was introduced to fill the need for a conventional
sequence container with a ctor and dtor.  The missing copy ctor and
assignment operators were an oversight, not a deliberate feature.
This change fixes that oversight.

The revised patch also adds a copy ctor/assignment to the auto_vec
primary template (that's also missing it).  In addition, it adds
a new class called auto_vec_ncopy that disables copying and
assignment as you prefer.


Hmm, adding another class doesn't really help with the confusion richi 
mentions.  And many uses of auto_vec will pass them as vec, which will 
still do a shallow copy.  I think it's probably better to disable the 
copy special members for auto_vec until we fix vec<>.


Jason



Re: [PATCH] Add gnu::diagnose_as attribute

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 2:54 PM, Matthias Kretz wrote:

On Thursday, 27 May 2021 19:39:48 CEST Jason Merrill wrote:

On 5/4/21 7:13 AM, Matthias Kretz wrote:

From: Matthias Kretz 

This attribute overrides the diagnostics output string for the entity it
appertains to. The motivation is to improve QoI for library TS
implementations, where diagnostics have a very bad signal-to-noise ratio
due to the long namespaces involved.

On Tuesday, 27 April 2021 11:46:48 CEST Jonathan Wakely wrote:

I think it's a great idea and would like to use it for all the TS
implementations where there is some inline namespace that the user
doesn't care about. std::experimental::fundamentals_v1:: would be much
better as just std::experimental::, or something like std::[LFTS]::.


Hmm, how much of the benefit could we get from a flag (probably on by
default) to skip inline namespaces in diagnostics?


I'd say about 20% for the TS's. Even std::experimental::simd (i.e. without the
'::parallelism_v2' part) is still rather noisy. I want stdₓ::simd, std-x::simd
or std::[PTS2]::simd or whatever shortest shorthand Jonathan will allow. ;)

For PR89370, the benefit would be ~2%:

'template std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::_If_sv<_Tp, std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&>
std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::insert(std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type, const _Tp&, std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type, std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits =
std::char_traits; _Alloc = std::allocator]'

would then only turn into:

'template std::basic_string<_CharT, _Traits,
_Alloc>::_If_sv<_Tp, std::basic_string<_CharT, _Traits, _Alloc>&>
std::basic_string<_CharT, _Traits,
_Alloc>::insert(std::basic_string<_CharT, _Traits,
_Alloc>::size_type, const _Tp&, std::basic_string<_CharT, _Traits,
_Alloc>::size_type, std::basic_string<_CharT, _Traits,
_Alloc>::size_type) [with _Tp = _Tp; _CharT = char; _Traits =
std::char_traits; _Alloc = std::allocator]'

instead of:

'template std::string::_If_sv<_Tp, std::string&>
std::string::insert<_Tp>(std::string::size_type, const _Tp&,
std::string::size_type, std::string::size_type)'


Also hiding all inline namespace by default might make some error messages
harder to understand:

namespace Vir {
   inline namespace foo {
 struct A {};
   }
   struct A {};
}
using Vir::A;


:7:12: error: reference to 'A' is ambiguous
:3:12: note: candidates are: 'struct Vir::A'
:5:10: note: 'struct Vir::A'


That doesn't seem so bad.


With the attribute, it is possible to solve PR89370 and make
std::__cxx11::basic_string<_CharT, _Traits, _Alloc> appear as
std::string in diagnostic output without extra hacks to recognize the
type.


That sounds wrong to me; std::string is the  instantiation, not
the template.  Your patch doesn't make it possible to apply this
attribute to class template instantiations, does it?


Yes, it does.

Initially, when I tried to improve the TS experience, it didn't. When Jonathan
showed PR89370 to me I tried to make [[gnu::diagnose_as]] more generic &
useful. Since there's no obvious syntax to apply an attribute to a template
instantiation, I had to be creative. This is from my pending std::string
patch:

--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -299,7 +299,8 @@ namespace std
  #if _GLIBCXX_USE_CXX11_ABI
  namespace std
  {
-  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+  inline namespace __cxx11
+__attribute__((__abi_tag__ ("cxx11"), __diagnose_as__("std"))) { }


This seems to have the same benefits and drawbacks of my inline 
namespace suggestion.  And it seems like applying the attribute to a 
namespace means that enclosing namespaces are not printed, unlike the 
handling for types.



  }
  namespace __gnu_cxx
  {
--- a/libstdc++-v3/include/bits/stringfwd.h
+++ b/libstdc++-v3/include/bits/stringfwd.h
@@ -76,24 +76,24 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
  _GLIBCXX_END_NAMESPACE_CXX11
  
/// A string of @c char

-  typedef basic_stringstring;
+  typedef basic_string string [[__gnu__::__diagnose_as__("string")]];


Here it seems like you want to say "use this typedef as the true name of 
the type".  Is it useful to have to repeat the name?  Allowing people to 
use names that don't correspond to actual declarations seems unnecessary.



  #ifdef _GLIBCXX_USE_WCHAR_T
/// A string of @c wchar_t
-  typedef basic_string wstring;
+  typedef basic_string wstring
  [[__gnu__::__diagnose_as__("wstring")]];
  #endif
[...]

The part of my frontend patch that makes this work is in
handle_diagnose_as_attribute:

+  if (TREE_CODE (*node) == TYPE_DECL)
+{
+  // Apply the attribute to the type alias itself.
+  decl = *node;
+  tree type = TREE_TYPE (*node);
+  if (CLASS_TYPE_P (type) && CLASSTYPE_TEMPLATE_INSTANTIATION (type))
+   {
+ 

Re: [PATCH] c++: Add missing scope in typedef diagnostic [PR100763]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 11:30 AM, Dr. Matthias Kretz wrote:

On Thursday, 27 May 2021 17:18:58 CEST Jason Merrill wrote:

On 5/26/21 5:27 PM, Matthias Kretz wrote:

From: Matthias Kretz 

dump_type on 'const std::string' should not print 'const string' unless
TFF_UNQUALIFIED_NAME is requested.

gcc/cp/ChangeLog:
PR c++/100763
* error.c: Call dump_scope when printing a typedef.

+ if (! (flags & TFF_UNQUALIFIED_NAME))
+   dump_scope (pp, CP_DECL_CONTEXT (TYPE_NAME (t)), flags);


You can use "decl" instead of "TYPE_NAME (t)" here.

OK with that change.


Updated patch below.


Applied, thanks.


From: Matthias Kretz 

dump_type on 'const std::string' should not print 'const string' unless
TFF_UNQUALIFIED_NAME is requested.

gcc/cp/ChangeLog:

PR c++/100763
* error.c: Call dump_scope when printing a typedef.
---
  gcc/cp/error.c | 2 ++
  1 file changed, 2 insertions(+)





Re: [PATCH] c++: Output less irrelevant info for function template decl [PR100716]

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 11:25 AM, Matthias Kretz wrote:

On Thursday, 27 May 2021 17:07:40 CEST Jason Merrill wrote:

On 5/26/21 5:29 PM, Matthias Kretz wrote:

New revision which can also be compiled with GCC 4.8.

From: Matthias Kretz 

Ensure dump_template_decl for function templates never prints template
parameters after the function name (it did with -fno-pretty-templates)
and skip output of irrelevant & confusing "[with T = T]" in
dump_substitution.

gcc/cp/ChangeLog:
PR c++/100716
* error.c (dump_template_bindings): Include code to print
"[with" and ']', conditional on whether anything is printed at
all. This is tied to whether a semicolon is needed to separate
multiple template parameters. If the template argument repeats
the template parameter (T = T), then skip the parameter.


This description should really be in a comment in the code, rather than
the ChangeLog.  OK either way.


Added comments in the code. New patch below.


Applied with missing spaces before () added.


From: Matthias Kretz 

Ensure dump_template_decl for function templates never prints template
parameters after the function name (it did with -fno-pretty-templates)
and skip output of irrelevant & confusing "[with T = T]" in
dump_substitution.

gcc/cp/ChangeLog:

PR c++/100716
* error.c (dump_template_bindings): Include code to print
"[with" and ']', conditional on whether anything is printed at
all. This is tied to whether a semicolon is needed to separate
multiple template parameters. If the template argument repeats
the template parameter (T = T), then skip the parameter.
(dump_substitution): Moved code to print "[with" and ']' to
dump_template_bindings.
(dump_function_decl): Partial revert of PR50828, which masked
TFF_TEMPLATE_NAME for all of dump_function_decl. Now
TFF_TEMPLATE_NAME is masked for the scope of the function and
only carries through to dump_function_name.
(dump_function_name): Avoid calling dump_template_parms if
TFF_TEMPLATE_NAME is set.

gcc/testsuite/ChangeLog:

PR c++/100716
* g++.dg/diagnostic/pr100716.C: New test.
* g++.dg/diagnostic/pr100716-1.C: Same test with
-fno-pretty-templates.
---
  gcc/cp/error.c   | 63 +++-
  gcc/testsuite/g++.dg/diagnostic/pr100716-1.C | 54 +
  gcc/testsuite/g++.dg/diagnostic/pr100716.C   | 54 +
  3 files changed, 156 insertions(+), 15 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716-1.C
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/pr100716.C


--
──
  Dr. Matthias Kretz   https://mattkretz.github.io
  GSI Helmholtz Centre for Heavy Ion Research   https://gsi.de
  std::experimental::simd  https://github.com/VcDevel/std-simd
──





Re: [PATCH][version 3]add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc

2021-05-27 Thread Qing Zhao via Gcc-patches
(Resend, the previous version messed up your questions and my answers, 
hopefully this time it’s better)

Hi, Richard,

Thanks a lot for your comments.

On May 26, 2021, at 6:18 AM, Richard Biener 
mailto:rguent...@suse.de>> wrote:

On Wed, 12 May 2021, Qing Zhao wrote:

Hi,

This is the 3rd version of the patch for the new security feature for GCC.

Please take look and let me know your comments and suggestions.

thanks.

Qing

**Compare with the 2nd version, the following are the major changes:

1. use "lookup_attribute ("uninitialized",) directly instead of adding
  one new field "uninitialized" into tree_decl_with_vis.
2. update documentation to mention that the new option will not confuse
  -Wuninitialized, GCC still consider an auto without explicit initializer
  as uninitialized.
3. change the name of "build_pattern_cst" to more specific name as
  "build_pattern_cst_for_auto_init".
4. handling of nested VLA;
  Adding new testing cases (auto-init-15/16.c) for this new handling.
5. Add  new verifications of calls to .DEFERRED_INIT in tree-cfg.c;
6. in tree-sra.c, update the handling of "grp_to_be_debug_replaced",
  bind the lhs variable to a call to .DEFERRED_INIT.
7. In tree-ssa-structalias.c, delete "find_func_aliases_for_deferred_init",
  return directly for a call to .DEFERRED_INIT in "find_func_aliases_for_call".
8. Add more detailed comments in tree-ssa-uninit.c and tree-ssa.c to explain
  the special handling on REALPART_EXPR/IMAGPRT_EXPR.
9. in build_pattern_cst_for_auto_init:
  BOOLEAN_TYPE will be set to zero always;
  INTEGER_TYPE (?and ENUMERAL_TYPE) use wi::from_buffer in order to
   correctly handle 128-bit integers.
  POINTER_TYPE will not assert on SIZE < 32.
  REAL_TYPE add fallback;
10. changed gcc_assert to gcc_unreachable in several places;
11. add more comments;
12. some style issue changes.

**Please see the version 2 at:
https://gcc.gnu.org/pipermail/gcc-patches/2021-March/567262.html


**The following 2 items are the ones I didn’t addressed in this version due 
to further study and might need more discussion:

1. Using __builtin_clear_padding  to replace type_has_padding.

My study shows: the call to __builtin_clear_padding is expanded during 
gimplification phase.
And there is no __bultin_clear_padding expanding during rtx expanding phase.
If so,  for -ftrivial-auto-var-init, padding initialization should be done both 
in gimplification phase and rtx expanding phase.
And since the __builtin_clear_padding might not be good for rtx expanding, 
reusing __builtin_clear_padding might not work.

2. Pattern init to NULLPTR_TYPE and ENUMERAL_TYPE: need more comments from 
Richard Biener on this.

**The change of the 3rd version compared to the 2nd version are:


+@item -ftrivial-auto-var-init=@var{choice}
+@opindex ftrivial-auto-var-init
+Initialize automatic variables with either a pattern or with zeroes to
increase
+the security and predictability of a program by preventing uninitialized
memory
+disclosure and use.

the docs do not state what "trivial" actually means?  Does it affect
C++ classes with ctors, thus is "trivial" equal to what C++ considers
a POD type?


Thank you for this question.

The name -ftrivial-auto-var-init is just for compatible with Clang. I really 
don’t know why
they added trivial.

As I checked a small example with C++ class with ctors, I see both Clang and my 
patch add
Initialization to this class:

=
#include 

using namespace std;

class Line {
  public:
 void setLength( double len );
 double getLength( void );
 Line();  // This is the constructor
  private:
 double length;
};

// Member functions definitions including constructor
Line::Line(void) {
  cout << "Object is being created" << endl;
}
void Line::setLength( double len ) {
 length = len;
}
double Line::getLength( void ) {
 return length;
}

// Main function for the program
int main() {
 Line line;

 // set line length
 line.setLength(6.0);
 cout << "Length of line : " << line.getLength() <
AUTO_INIT_UNINITIALIZED
+&& !TREE_STATIC (exp)
+&& type_has_padding (type)))

testing flag_trivial_auto_var_init tests the global options, if TUs
are compiled with different setting of flag_trivial_auto_var_init
and you use LTO or flag_trivial_auto_var_init is specified per
function via optimize attributes it's more appropriate to test
opt_for_fn (cfun->decl, flag_trivial_auto_var_init)


Okay.  Thanks for the info. I will update this.


You do not actually test whether TARGET is an auto-var in this place,
so I question this change

Will add checking for Auto on this.

- the documentation of ftrivial-auto-var-init
also doesn't mention initialization of padding

Will add initialization of padding on this.
Clang add the padding initialization with this option, I guess that we might 
need to
be compatible with it?

and the above doesn't
seem to honor -ftrivial-auto-var-init=pattern for this.


Richard Sandiford raised

Re: [PATCH 0/11] warning control by group and location (PR 74765)

2021-05-27 Thread David Malcolm via Gcc-patches
On Thu, 2021-05-27 at 10:41 -0600, Martin Sebor wrote:
> On 5/27/21 5:19 AM, Richard Sandiford wrote:
> > Thanks for doing this.
> > 
> > Martin Sebor via Gcc-patches  writes:
> > > […]
> > > On 5/24/21 5:08 PM, David Malcolm wrote:
> > > > On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:
> > > > > Subsequent patches then replace invocations of the
> > > > > TREE_NO_WARNING()
> > > > > macro and the gimple_no_warning_p() and
> > > > > gimple_set_no_warning()
> > > > > functions throughout GCC with those and remove the legacy
> > > > > APIs to
> > > > > keep them from being accidentally reintroduced along with the
> > > > > problem.
> > > > > These are mostly mechanical changes, except that most of the
> > > > > new
> > > > > invocations also specify the option whose disposition to
> > > > > query for
> > > > > the expression or location, or which to enable or disable[2].
> > > > > The last function, copy_no_warning(), copies the disposition
> > > > > from
> > > > > one expression or location to another.
> > > > > 
> > > > > A couple of design choices might be helpful to explain:
> > > > > 
> > > > > First, introducing "warning groups" rather than controlling
> > > > > each
> > > > > individual warning is motivated by a) the fact that the
> > > > > latter
> > > > > would make avoiding redundant warnings for related problems
> > > > > cumbersome (e.g., after issuing a -Warray-bounds we want to
> > > > > suppress -Wstringop-overflow as well -Wstringop-overread for
> > > > > the same access and vice versa), and b) simplicity and
> > > > > efficiency
> > > > > of the implementation (mapping each option would require a
> > > > > more
> > > > > complex data structure like a bitmap).
> > > > > 
> > > > > Second, using location_t to associate expressions/statements
> > > > > with
> > > > > the warning groups also turns out to be more useful in
> > > > > practice
> > > > > than a direct association between a tree or gimple*, and also
> > > > > simplifies managing the data structure.  Trees and gimple*
> > > > > come
> > > > > and go across passes, and maintaining a mapping for them that
> > > > > accounts for the possibility of them being garbage-collected
> > > > > and the same addresses reused is less than straightforward.
> > > > 
> > > > I find some of the terminology rather awkard due to it the
> > > > negation
> > > > we're already using with TREE_NO_WARNING, in that we're turning
> > > > on a
> > > > no_warning flag, and that this is a per-location/expr/stmt
> > > > thing that's
> > > > different from the idea of enabling/disabling a specific
> > > > warning
> > > > altogether (and the pragmas that control that).   Sometimes the
> > > > patches
> > > > refer to enabling/disabling warnings and I think I want
> > > > "enabling" and
> > > > "disabling" to mean the thing the user does with -Wfoo and -
> > > > Wno-foo.
> > > > 
> > > > Would calling it a "warning suppression" or somesuch be more or
> > > > less
> > > > klunky?
> > > 
> > > I like warning suppression :)  But I'm not sure where you suggest
> > > I use the phrase.
> > > 
> > > I don't particularly care for the "no" in the API names either
> > > (existing or new) and would prefer a positive form.  I considered
> > > enable_warning() and warning_enabled() but I chose the names I
> > > did
> > > because they're close to their established gimple namesakes.  I'm
> > > fine changing them to the alternatives, or if you or someone else
> > > has a preference for different names I'm open to suggestions. 
> > > Let
> > > me know.
> > 
> > Not my area, but FWIW, +1 for David's suggestion of
> > s/set_no_warning/suppress_warning/ or similar.  As well as the
> > problem with the double negative in negated conditions, I always
> > have to
> > remember whether TREE_NO_WARNING means that hasn't been anything to
> > warn
> > about yet or whether we shouldn't warn in future.
> 
> Sure.  Jason has a patch out for review to introduce
> 
>    warning_enabled_at (location, int option).

CCing Jason.

BTW, do you have a URL for that patch?

> 
> With that, how does the following API look? (I'd replace the int
> argument above with opt_code myself):
> 
>    void enable_warning (location_t, enum opt_code = N_OPTS);
>    void disable_warning (location_t, enum opt_code = N_OPTS);
>    void copy_warning (location_t, location_t);
>    bool warning_enabled_at (location_t, enum opt_code = N_OPTS).
> 
>    void enable_warning (tree, enum opt_code = N_OPTS);
>    void disable_warning (tree, enum opt_code = N_OPTS);
>    void copy_warning (tree, const_tree);
>    bool warning_enabled_at (const_tree, enum opt_code = N_OPTS).
> 
>    void enable_warning (gimple *, enum opt_code = N_OPTS);
>    void disable_warning (gimple *, enum opt_code = N_OPTS);
>    void copy_warning (gimple *, const gimple *);
>    bool warning_enabled_at (gimple *, enum opt_code = N_OPTS).

I'm not so happy with "enable_warning" and "disable_warning" in the
above proposal; it seems to me that three things 

Re: [PATCH] Add gnu::diagnose_as attribute

2021-05-27 Thread Matthias Kretz
On Thursday, 27 May 2021 23:15:46 CEST Jason Merrill wrote:
> On 5/27/21 2:54 PM, Matthias Kretz wrote:
> > Also hiding all inline namespace by default might make some error messages
> > harder to understand:
> > 
> > namespace Vir {
> >inline namespace foo {
> >  struct A {};
> >}
> >struct A {};
> > }
> > using Vir::A;
> > 
> > :7:12: error: reference to 'A' is ambiguous
> > :3:12: note: candidates are: 'struct Vir::A'
> > :5:10: note: 'struct Vir::A'
> 
> That doesn't seem so bad.

As long as you ignore the line numbers, it's a puzzling diagnostic.

> > This is from my pending std::string patch:
> > 
> > --- a/libstdc++-v3/include/bits/c++config
> > +++ b/libstdc++-v3/include/bits/c++config
> > @@ -299,7 +299,8 @@ namespace std
> > 
> >   #if _GLIBCXX_USE_CXX11_ABI
> >   namespace std
> >   {
> > 
> > -  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
> > +  inline namespace __cxx11
> > +__attribute__((__abi_tag__ ("cxx11"), __diagnose_as__("std"))) { }
> 
> This seems to have the same benefits and drawbacks of my inline
> namespace suggestion.

True for std::string, not true for TS's where the extra '::experimental' still 
makes finding the relevant information in diagnostics harder than necessary.

> And it seems like applying the attribute to a
> namespace means that enclosing namespaces are not printed, unlike the
> handling for types.

Yes, that's also how I documented it. For nested namespaces I wanted to enable 
the removal of nesting (e.g. from std::experimental::parallelism_v2::simd to 
stdx::simd). However, for types and functions it would be a problem to drop 
the enclosing scope, because the scope can be class templates and thus the 
diagnose_as attribute would remove all template parms & args.

> > -  typedef basic_stringstring;
> > +  typedef basic_string string
> > [[__gnu__::__diagnose_as__("string")]];
>
> Here it seems like you want to say "use this typedef as the true name of
> the type".  Is it useful to have to repeat the name?  Allowing people to
> use names that don't correspond to actual declarations seems unnecessary.

Yes, but you could also use it to apply diagnose_as to a template 
instantiation without introducing a name for users. E.g.

  using __only_to_apply_the_attribute [[gnu::diagnose_as("intvector")]]
= std::vector;

Now all diagnostics of 'std::vector' would print 'intvector' instead. But 
in general, I tend to agree, for type aliases there's rarely a case where the 
names wouldn't match.

However, I didn't want to special-case the attribute parameters for type 
aliases (or introduce another attribute just for this case). The attribute 
works consistently and with the same interface independent of where it's used. 
I tried to build a generic, broad feature instead of a narrow one-problem 
solution.

FWIW, before you suggest to have one attribute for namespaces and one for type 
aliases (to cover the std::string case), I have another use case in stdx::simd 
(the spec requires simd_abi::scalar to be an alias):

  namespace std::experimental::parallelism_v2::simd_abi {
struct [[gnu::diagnose_as("scalar")]] _Scalar;
using scalar = _Scalar;
  }

If the attribute were on the type alias (using scalar [[gnu::diagnose_as]] = 
_Scalar;), then we'd have to apply the attribute to _Scalar after it was 
completed. That seemed like a bad idea to me.  

-- 
──
 Dr. Matthias Kretz   https://mattkretz.github.io
 GSI Helmholtz Centre for Heavy Ion Research   https://gsi.de
 std::experimental::simd  https://github.com/VcDevel/std-simd
──





Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-27 Thread David Malcolm via Gcc-patches
On Tue, 2021-05-25 at 20:19 -0400, Antoni Boucher wrote:
> @David: PING
> 
> As far as I know, the only remaining question is about using
> `ssize_t`
> for the return type of some functions.
> Here's why I use this type:
> 
> That seemed off to return NULL for the functions returning a 
> size_t to indicate an error. So I changed it to return -1 (and return
> type to ssize_t). Is that the proper way to indicate an error?
> 
> Once I know the answer for this error handling question, I'll fix the
> types.
> 
> Thanks!

I think the things that accept params for indexes should be size_t
(i.e. unsigned).

As for the error-handling, I've been going back and forth.

The various return of -1 in the patch all occur when NULL is passed in
as the gcc_jit_foo*, for the various foo. Unfortunately this means that
we can't get at a jit context to emit the error on, so jit_error is
called with a NULL context, and hence merely prints to stderr; it can't
set state on any jit context.

Presumably if the code is passing in NULL for one of these things, then
something's gone wrong already.

Option (A): returning a size_t (unsigned):
- No way to indicate error if the user passes in NULL, or a bogus
value, beyond output on stderr; we have to defensively return 0, which
the user's code is likely to gracefully handle

Option (B): returning a ssize_t:
- Can return -1 if the user passes in NULL or a bogus value.  But if
the user doesn't check for -1 and blithely casts to unsigned (e.g.
allocating a buffer), their code is likely to explode with a very large
allocation.

Consider e.g. gcc_jit_function_type_get_param_count.  If the user uses 
gcc_jit_type_is_function_ptr_type to dynamically cast some type to
function_type and forgets to check, and passes in NULL, then...

...with option (A), it returns 0, as if the function has no params;
eventually a type-checking error occurs.

...with option (B), it returns -1, and the user who didn't check before
has to check again

So I think I prefer option (A), but I'm willing to be convinced on the
alternative.

I also now think that the dynamic cast functions (e.g.
gcc_jit_type_is_function_ptr_type) should be "_dyncast_" rather than
"_is_" (e.g. gcc_jit_type_dyncast_function_ptr_type) to make it clear
that this is a dynamic cast that can fail, and to contrast them with
the "_as_" functions which are static casts.

Hope this makes sense and is constructive.
Dave


> 
> Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> > On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > > I was missing a check in gcc_jit_struct_get_field, I added it in
> > > this
> > > new patch.
> > > 
> > 
> > Sorry about the long delay in reviewing this patch.
> > 
> > The main high-level points are:
> > - currently the get_*_count functions return "ssize_t" - why?  Only
> > unsigned values are meaningful; shouldn't they return "size_t"
> > instead?
> > 
> > - the various "lookup by index" functions take "int" i.e. signed, but
> > only >= 0 is meaningful.  I think it makes sense to make them take
> > size_t instead.
> > 
> > Sorry if we covered that before in the review, it's been a while.
> > 
> > Various nitpicks inline below...
> > 
> > [...snip...]
> >  
> > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > b/gcc/jit/docs/topics/compatibility.rst
> > > index 6bfa101ed71..236e5c72d81 100644
> > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > @@ -226,3 +226,44 @@ entrypoints:
> > >  
> > >  ``LIBGCCJIT_ABI_14`` covers the addition of
> > >  :func:`gcc_jit_global_set_initializer`
> > > +
> > > +.. _LIBGCCJIT_ABI_15:
> > > +
> > > +``LIBGCCJIT_ABI_15``
> > > +
> > > +``LIBGCCJIT_ABI_15`` covers the addition of reflection functions
> > > via
> > > API
> > > +entrypoints:
> > 
> > This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.
> > 
> > [...snip...]
> > 
> > > diff --git a/gcc/jit/docs/topics/functions.rst
> > > b/gcc/jit/docs/topics/functions.rst
> > > index eb40d64010e..aa6de87282d 100644
> > > --- a/gcc/jit/docs/topics/functions.rst
> > > +++ b/gcc/jit/docs/topics/functions.rst
> > > @@ -171,6 +171,16 @@ Functions
> > >     underlying string, so it is valid to pass in a pointer to an
> > > on-
> > > stack
> > >     buffer.
> > >  
> > > +.. function::  ssize_t \
> > > +   gcc_jit_function_get_param_count (gcc_jit_function
> > > *func)
> > > +
> > > +   Get the number of parameters of the function.
> > > +
> > > +.. function::  gcc_jit_type \*
> > > +   gcc_jit_function_get_return_type (gcc_jit_function
> > > *func)
> > > +
> > > +   Get the return type of the function.
> > 
> > As noted before, this doesn't yet document all the new entrypoints; I
> > think you wanted to hold off until all the details were thrashed out,
> > but hopefully we're close.
> > 
> > The documentation for an entrypoint should specify which ABI it was
> > added in.
> > 
> > [...snip...]
>

Re: [PATCH] Remove CC0

2021-05-27 Thread Segher Boessenkool
Hi!

Whoops, I forgot to reply to this...

On Wed, May 05, 2021 at 06:25:49PM +, Koning, Paul wrote:
> > void g(void);
> > void h(void);
> > void i(void);
> > void f(long a, long b)
> > {
> >if (a < b)
> >g();
> >if (a == b)
> >h();
> >if (a > b)
> >i();
> > }
> 
> FWIW, that also works on pdp11, so it seems the general mechanism is in place 
> and working.  Well, with one oddity, an unnecessary third conditional branch:
> 
> _f:
>   mov 02(sp),r1
>   mov 04(sp),r0
>   cmp r1,r0
>   blt L_7
>   beq L_4
>   bgt L_5
>   rts pc
> L_5:
>   jsr pc,_i
>   rts pc
> L_4:
>   jsr pc,_h
>   rts pc
> L_7:
>   jsr pc,_g
>   rts pc

We have that on PowerPC as well (-m32 because it is shorter code with
fewer distractions):

f:
cmpw 0,3,4
blt 0,.L9
beq 0,.L3
blelr 0
b i
.p2align 4,,15
.L3:
b h
.p2align 4,,15
.L9:
b g

The "blelr" (branch to LR if less than or equal) can never be taken.
This is there in the Gimple already:

===
void f (long int a, long int b)
{
;;   basic block 2, loop depth 0
;;pred:   ENTRY
  if (a_4(D) < b_5(D))
goto ; [33.00%]
  else
goto ; [67.00%]
;;succ:   3
;;4

;;   basic block 3, loop depth 0
;;pred:   2
  g (); [tail call]
  goto ; [100.00%]
;;succ:   8

;;   basic block 4, loop depth 0
;;pred:   2
  if (a_4(D) == b_5(D))
goto ; [30.21%]
  else
goto ; [69.79%]
;;succ:   6
;;5

;;   basic block 5, loop depth 0
;;pred:   4
  if (a_4(D) > b_5(D))
goto ; [70.57%]
  else
goto ; [29.43%]
;;succ:   7
;;8

;;   basic block 6, loop depth 0
;;pred:   4
  h (); [tail call]
  goto ; [100.00%]
;;succ:   8

;;   basic block 7, loop depth 0
;;pred:   5
  i (); [tail call]
;;succ:   8

;;   basic block 8, loop depth 0
;;pred:   5
;;7
;;6
;;3
  return;
;;succ:   EXIT

}
===

So, it also exists in the RTL -- and it isn't optimised away later.
That would be quite hard to do in fact, and if it is handled in Gimple
all will be fine, and it should be much easier to do in Gimple.

Can you open a PR please?


Segher


Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498]

2021-05-27 Thread David Malcolm via Gcc-patches
On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> I updated the patch according to the comments by Tom Tromey.
> 
> There's one question left about your question regarding
> C_MAYBE_CONST_EXPR, David:
> 
> I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit, and
> it
> indeed seems like it's only created in c-family.
> However, we do use it in libgccjit here:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> 
> I tried removing the condition `if (TREE_CODE (t_ret) !=
> C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> 
> That code was copied from here:
> https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> and might not be needed in libgccjit.
> 
> Should I just remove the condition, then?

I think so.

Thanks
Dave

> 
> Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > Thanks for your answer.
> > > 
> > > See my answers below:
> > > 
> > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-
> > > > patches
> > > > wrote:
> > > > > Hi.
> > > > > Thanks for your feedback!
> > > > > 
> > > > 
> > > > Sorry about the delay in responding.
> > > > 
> > > > In the past I was hesitant about adding more cast support to
> > > > libgccjit
> > > > since I felt that the user could always just create a union to
> > > > do
> > > > the
> > > > cast.  Then I tried actually using the libgccjit API to do
> > > > this,
> > > > and
> > > > realized how much work it adds, so I now think we do want to
> > > > support
> > > > casting more types.
> > > > 
> > > > 
> > > > > See answers below:
> > > > > 
> > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > 
> > > > > > Antoni> gcc/jit/
> > > > > > Antoni> PR target/95498
> > > > > > Antoni> * jit-playback.c: Add support to handle
> > > > > > truncation
> > > > > > and extension
> > > > > > Antoni> in the convert function.
> > > > > > 
> > > > > > Antoni> +  switch (dst_code)
> > > > > > Antoni> +    {
> > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > Antoni> +  t_ret = convert_to_integer (dst_type, expr);
> > > > > > Antoni> +  goto maybe_fold;
> > > > > > Antoni> +
> > > > > > Antoni> +    default:
> > > > > > Antoni> +  gcc_assert (gcc::jit::active_playback_ctxt);
> > > > > > Antoni> +  gcc::jit::active_playback_ctxt->add_error
> > > > > > (NULL,
> > > > > > "unhandled conversion");
> > > > > > Antoni> +  fprintf (stderr, "input expression:\n");
> > > > > > Antoni> +  debug_tree (expr);
> > > > > > Antoni> +  fprintf (stderr, "requested type:\n");
> > > > > > Antoni> +  debug_tree (dst_type);
> > > > > > Antoni> +  return error_mark_node;
> > > > > > Antoni> +
> > > > > > Antoni> +    maybe_fold:
> > > > > > Antoni> +  if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
> > > > 
> > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree code
> > > > is
> > > > defined in c-family/c-common.def; how can nodes of that kind be
> > > > created
> > > > outside of the c-family?
> > > 
> > > I am not sure, but that seems like it's only created in c-family
> > > indeed.
> > > However, we do use it in libgccjit here:
> > > 
> > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > 
> > > > 
> > > > > > Antoni> +   t_ret = fold (t_ret);
> > > > > > Antoni> +  return t_ret;
> > > > > > 
> > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > especially
> > > > > > inside
> > > > > > a switch like this.
> > > > > > 
> > > > > > If you think the maybe_fold code won't be reused, then it
> > > > > > should
> > > > > > just
> > > > > > be
> > > > > > hoisted up and the 'goto' removed.
> > > > > 
> > > > > This actually depends on how the support for cast between
> > > > > integers
> > > > > and 
> > > > > pointers will be implemented (see below).
> > > > > If we will support truncating pointers (does that even make
> > > > > sense?
> > > > > and
> > > > > I 
> > > > > guess we cannot extend a pointer unless we add the support
> > > > > for 
> > > > > uint128_t), that label will be reused for that case.
> > > > > Otherwise, it might not be reused.
> > > > > 
> > > > > So, please tell me which option to choose and I'll update my
> > > > > patch.
> > > > 
> > > > FWIW I don't think we'll want to support truncating or
> > > > extending
> > > > pointers.
> > > 
> > > Ok, but do you think we'll want to support casts between integers
> > > and
> > > pointers?
> > 
> > Yes, though we probably want to reject truncating a pointer into a
> > smaller integer type.
> > 
> > > I opened an issue about this
> > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?i

Re: [PATCH] Remove CC0

2021-05-27 Thread Segher Boessenkool
On Thu, May 27, 2021 at 12:58:10PM -0700, H.J. Lu wrote:
> You missed:
> 
> config/cr16/cr16.md:   (clobber (cc0))]
> config/cr16/cr16.md:  [(parallel [(set (cc0)
> config/cr16/cr16.md:  [(set (cc0)
> config/cr16/cr16.md: [(cc0) (const_int 0)]))]
> config/cr16/cr16.md:  [(set (cc0)
> config/cr16/cr16.md: [(cc0) (const_int 0)]))]
> 
> Now, cr16-elf won't build.

I did not miss that.  I did not convert targets away from cc0 here.  We
deprecated cc0 long ago, and it is time to remove it now.  cr16 is the
one target that still uses cc0.  It should be relatively trivial to move
to the modern world: two expanders, two insns; and only cmp, cbranch,
cstore for that matter; anyone who knows cr16 at all can probably do it.
But that is not me, I do not know how to *test* cr16 at all.


Segher


Go patch committed: Don't quote ampersand in gccgo.texi

2021-05-27 Thread Ian Lance Taylor via Gcc-patches
This patch removes HTML quoting from the Texinfo file gccgo.html.
Committed to mainline.

Ian
diff --git a/gcc/go/gccgo.texi b/gcc/go/gccgo.texi
index ce6b518bb7b..fa0e4882403 100644
--- a/gcc/go/gccgo.texi
+++ b/gcc/go/gccgo.texi
@@ -495,7 +495,7 @@ like (after importing the @code{os} package):
 
 @smallexample
 var name = [4]byte@{'f', 'o', 'o', 0@};
-i := c_open(&name[0], os.O_RDONLY, 0);
+i := c_open(&name[0], os.O_RDONLY, 0);
 @end smallexample
 
 Note that this serves as an example only.  To open a file in Go please


Re: [Patch] OpenMP: Add iterator support to Fortran's depend; add affinity clause

2021-05-27 Thread Tobias Burnus

On 27.05.21 21:58, Joseph Myers wrote:


On Thu, 27 May 2021, Tobias Burnus wrote:

@Joseph: I CC'ed you in case you have comments regarding
c-parser.c's c_parser_check_balanced_raw_token_sequence


Pilot error on my side – doing three things in parallel
(fixing a patch, updating docs + attending a spec conference)
does not work well.
Unsurprisingly, when properly used, calling
c_parser_check_balanced_raw_token_sequence does work.

Hence, I removed my balanced function & only the OpenMP specific
omp_affinity function remains.

(Updated version attached.)


As far as I can see, ... isn't
checking for balanced token sequences (in the sense defined in C2x) at all
and would accept e.g. }]{[ as being balanced.

It looks as if one should/could update the gcc/cp/ function to use the same
algorithm as the gcc/c/ one – as the latter has also this issue. But that's
a separate issue and completely independent of this patch.

Thanks,

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
OpenMP: Add iterator support to Fortran's depend; add affinity clause

gcc/c-family/ChangeLog:

	* c-pragma.h (enum pragma_omp_clause): Add PRAGMA_OMP_CLAUSE_AFFINITY.

gcc/c/ChangeLog:

	* c-parser.c (c_parser_omp_clause_affinity): New.
	(c_parser_omp_clause_name, c_parser_omp_variable_list,
	c_parser_omp_all_clauses, OMP_TASK_CLAUSE_MASK): Handle affinity clause.
	* c-typeck.c (handle_omp_array_sections_1, handle_omp_array_sections,
	c_finish_omp_clauses): Likewise.

gcc/cp/ChangeLog:

	* parser.c (cp_parser_omp_clause_affinity): New.
	(cp_parser_omp_clause_name, cp_parser_omp_var_list_no_open,
	cp_parser_omp_all_clauses, OMP_TASK_CLAUSE_MASK): Handle affinity
	clause.
	* semantics.c (handle_omp_array_sections_1, handle_omp_array_sections,
	finish_omp_clauses): Likewise.

gcc/fortran/ChangeLog:

	* dump-parse-tree.c (show_iterator): New.
	(show_omp_namelist): Handle iterators.
	(show_omp_clauses): Handle affinity.
	* gfortran.h (gfc_free_omp_namelist): New union with 'udr' and new 'ns'.
	* match.c (gfc_free_omp_namelist): Add are to choose union element.
	* openmp.c (gfc_free_omp_clauses, gfc_match_omp_detach): Update
	call to gfc_free_omp_namelist.
	(gfc_match_omp_variable_list): Likewise; permit preceeding whitespace.
	(enum omp_mask1):
	(gfc_match_iterator): New.
	(gfc_match_omp_clause_reduction):
	(gfc_match_omp_clauses):
	(gfc_match_omp_flush):
	(gfc_match_omp_taskwait): Match depend clause.
	(resolve_omp_clauses): Handle affinity; update for udr/union change.
	(gfc_resolve_omp_directive): Resolve clauses of taskwait.
	* st.c (gfc_free_statement): Update gfc_free_omp_namelist call.
	* trans-openmp.c (gfc_trans_omp_array_reduction_or_udr): Likewise
	(handle_iterator): New.
	(gfc_trans_omp_clauses): Handle iterators for depend/affinity clause.
	(gfc_trans_omp_taskwait): Handle depend clause.
	(gfc_trans_omp_directive): Update call.

gcc/ChangeLog:

	* (gimplify_scan_omp_clauses): Ignore affinity clause.
	* tree-core.h (enum omp_clause_code): Add OMP_CLAUSE_AFFINITY.
	* tree-pretty-print.c (dump_omp_clause): Handle OMP_CLAUSE_AFFINITY.
	* tree.c (omp_clause_num_ops, omp_clause_code_name): Add clause.
	(walk_tree_1): Handle OMP_CLAUSE_AFFINITY.

libgomp/ChangeLog:

	* testsuite/libgomp.fortran/depend-iterator-2.f90: New test.

gcc/testsuite/ChangeLog:

	* c-c++-common/gomp/affinity-1.c: New test.
	* c-c++-common/gomp/affinity-2.c: New test.
	* c-c++-common/gomp/affinity-3.c: New test.
	* c-c++-common/gomp/affinity-4.c: New test.
	* c-c++-common/gomp/affinity-5.c: New test.
	* c-c++-common/gomp/affinity-6.c: New test.
	* c-c++-common/gomp/affinity-7.c: New test.
	* gfortran.dg/gomp/affinity-clause-1.f90: New test.
	* gfortran.dg/gomp/affinity-clause-2.f90: New test.
	* gfortran.dg/gomp/affinity-clause-3.f90: New test.
	* gfortran.dg/gomp/affinity-clause-4.f90: New test.
	* gfortran.dg/gomp/affinity-clause-5.f90: New test.
	* gfortran.dg/gomp/affinity-clause-6.f90: New test.
	* gfortran.dg/gomp/depend-iterator-1.f90: New test.
	* gfortran.dg/gomp/depend-iterator-2.f90: New test.
	* gfortran.dg/gomp/depend-iterator-3.f90: New test.
	* gfortran.dg/gomp/taskwait.f90: New test.

 gcc/c-family/c-pragma.h|   1 +
 gcc/c/c-parser.c   |  81 +-
 gcc/c/c-typeck.c   |  52 ++--
 gcc/cp/parser.c|  86 +-
 gcc/cp/semantics.c |  53 ++--
 gcc/fortran/dump-parse-tree.c  |  51 +++-
 gcc/fortran/gfortran.h |   9 +-
 gcc/fortran/match.c|  18 +-
 gcc/fortran/openmp.c   | 307 ++---
 gcc/fortran/st.c   |   2 +-
 gcc/fortran/trans-openmp.c | 198 ++---
 gcc/gimplify.c   

Re: [PATCH] rs6000: MMA test case ICEs using -O3 [PR99842]

2021-05-27 Thread Segher Boessenkool
Hi!

On Fri, May 21, 2021 at 02:45:18PM -0500, Peter Bergner wrote:
> Getting back to this now that trunk is open again...
> 
> On 3/31/21 2:17 PM, Segher Boessenkool wrote:
> > On Tue, Mar 30, 2021 at 06:49:29PM -0500, Peter Bergner wrote:
> >> The mma_assemble_input_operand predicate does not accept reg+reg indexed
> >> addresses which can lead to ICEs.  The problem is that the quad_address_p
> >> function only accepts reg+offset addresses that are valid for quad word
> >> accesses, but not reg+reg addresses which are also valid for quad word
> >> accesses when dealing with vector types.  The solution used here is to
> >> call memory_operand, which uses rs6000_legitimate_address_p to ensure
> >> the address is valid.  For reg+offset addresses, it uses quad_address_p 
> >> like
> >> before, but for reg+reg addresses, it calls legitimate_indexed_address_p
> >> addresses which fixes this specific ICE.
> > 
> > quad_address_p should really only be used for lq/stq (and their prefixed
> > forms).  Those insns have very different semantics: they are atomic,
> > while vector loads and stores are not; and the prefixed form has some
> > special semantics (it swaps halves on LE).
> 
> quad_address_p has since day one been used for both lq/stq as well as
> for vector loads/stores.

The was "quad_memory_operand" in 2013 already, three years earlier, and
it was exclusively for load/store quad insns -- which, as I said, have
very different semantics (they are atomic, they have different alignment
requirements, they have different addressing modes, they have different
semantics in LE -- is there anything the *same* as vector memory
operands even?)

So it would be much clearer and less error-prone if these different
concepts were not artificially put together.  And the name already
suggests it is only for load/store quad insns, not for vectors!

> I think the "quad" here is meant to describe
> that the address will be used for a dform quad word memory access
> (eg, lq/stq, lxv/stxv and lxvp/stxvp) which use DQ offsets.

No.  If you look at history, quad_memory_operand is for lq/stq only.

> I don't
> think quad_address_p cares whether the address is being used for an
> atomic access or not.  That restriction is done elsewhere it seems.

And that is a big part of the problem.

Please don't use "quad" things for vectors at all.  The code will become
simpler, and we might even have a chance of getting it correct!

> rs6000: MMA test case ICEs using -O3 [PR99842]
> 
> The mma_assemble_input_operand predicate does not accept reg+reg indexed
> addresses which can lead to ICEs.  The lxv and lxvp instructions have
> indexed forms (lxvx and lxvpx), so the simple solution is to just allow
> indexed addresses in the predicate.

> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index e21bc745f72..121cbf14810 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1172,7 +1172,8 @@ (define_special_predicate "mma_assemble_input_operand"
>(match_test "(mode == V16QImode
>   && (vsx_register_operand (op, mode)
>   || (MEM_P (op)
> - && quad_address_p (XEXP (op, 0), mode, false"))
> + && (indexed_or_indirect_address (XEXP (op, 0), mode)
> + || quad_address_p (XEXP (op, 0), mode, false)"))

This is okay for trunk and for backports.  But please fix it properly on
trunk after the backport: don't abuse quad* for vectors anymore!

Thanks,


Segher


Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498]

2021-05-27 Thread Antoni Boucher via Gcc-patches
Here's the patch with the condition removed.
I believe everything is now fixed.
Thanks!

Le jeudi 27 mai 2021 à 18:21 -0400, David Malcolm a écrit :
> On Tue, 2021-05-25 at 20:16 -0400, Antoni Boucher wrote:
> > I updated the patch according to the comments by Tom Tromey.
> > 
> > There's one question left about your question regarding
> > C_MAYBE_CONST_EXPR, David:
> > 
> > I am not sure if we can get a C_MAYBE_CONST_EXPR from libgccjit,
> > and
> > it
> > indeed seems like it's only created in c-family.
> > However, we do use it in libgccjit here:
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > 
> > I tried removing the condition `if (TREE_CODE (t_ret) !=
> > C_MAYBE_CONST_EXPR)` and all the tests of libgccjit still pass.
> > 
> > That code was copied from here:
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/c/c-convert.c#L175
> > and might not be needed in libgccjit.
> > 
> > Should I just remove the condition, then?
> 
> I think so.
> 
> Thanks
> Dave
> 
> > 
> > Le jeudi 13 mai 2021 à 19:58 -0400, David Malcolm a écrit :
> > > On Thu, 2021-05-13 at 19:31 -0400, Antoni Boucher wrote:
> > > > Thanks for your answer.
> > > > 
> > > > See my answers below:
> > > > 
> > > > Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit :
> > > > > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-
> > > > > patches
> > > > > wrote:
> > > > > > Hi.
> > > > > > Thanks for your feedback!
> > > > > > 
> > > > > 
> > > > > Sorry about the delay in responding.
> > > > > 
> > > > > In the past I was hesitant about adding more cast support to
> > > > > libgccjit
> > > > > since I felt that the user could always just create a union
> > > > > to
> > > > > do
> > > > > the
> > > > > cast.  Then I tried actually using the libgccjit API to do
> > > > > this,
> > > > > and
> > > > > realized how much work it adds, so I now think we do want to
> > > > > support
> > > > > casting more types.
> > > > > 
> > > > > 
> > > > > > See answers below:
> > > > > > 
> > > > > > On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
> > > > > > > > > > > > "Antoni" == Antoni Boucher via Gcc-patches <   
> > > > > > > > > > > > gcc-patches@gcc.gnu.org> writes:
> > > > > > > 
> > > > > > > Antoni> gcc/jit/
> > > > > > > Antoni> PR target/95498
> > > > > > > Antoni> * jit-playback.c: Add support to handle
> > > > > > > truncation
> > > > > > > and extension
> > > > > > > Antoni> in the convert function.
> > > > > > > 
> > > > > > > Antoni> +  switch (dst_code)
> > > > > > > Antoni> +    {
> > > > > > > Antoni> +    case INTEGER_TYPE:
> > > > > > > Antoni> +    case ENUMERAL_TYPE:
> > > > > > > Antoni> +  t_ret = convert_to_integer (dst_type,
> > > > > > > expr);
> > > > > > > Antoni> +  goto maybe_fold;
> > > > > > > Antoni> +
> > > > > > > Antoni> +    default:
> > > > > > > Antoni> +  gcc_assert
> > > > > > > (gcc::jit::active_playback_ctxt);
> > > > > > > Antoni> +  gcc::jit::active_playback_ctxt->add_error
> > > > > > > (NULL,
> > > > > > > "unhandled conversion");
> > > > > > > Antoni> +  fprintf (stderr, "input expression:\n");
> > > > > > > Antoni> +  debug_tree (expr);
> > > > > > > Antoni> +  fprintf (stderr, "requested type:\n");
> > > > > > > Antoni> +  debug_tree (dst_type);
> > > > > > > Antoni> +  return error_mark_node;
> > > > > > > Antoni> +
> > > > > > > Antoni> +    maybe_fold:
> > > > > > > Antoni> +  if (TREE_CODE (t_ret) !=
> > > > > > > C_MAYBE_CONST_EXPR)
> > > > > 
> > > > > Do we even get C_MAYBE_CONST_EXPR in libgccjit?  That tree
> > > > > code
> > > > > is
> > > > > defined in c-family/c-common.def; how can nodes of that kind
> > > > > be
> > > > > created
> > > > > outside of the c-family?
> > > > 
> > > > I am not sure, but that seems like it's only created in c-
> > > > family
> > > > indeed.
> > > > However, we do use it in libgccjit here:
> > > > 
> > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180
> > > > 
> > > > > 
> > > > > > > Antoni> +   t_ret = fold (t_ret);
> > > > > > > Antoni> +  return t_ret;
> > > > > > > 
> > > > > > > It seems weird to have a single 'goto' to maybe_fold,
> > > > > > > especially
> > > > > > > inside
> > > > > > > a switch like this.
> > > > > > > 
> > > > > > > If you think the maybe_fold code won't be reused, then it
> > > > > > > should
> > > > > > > just
> > > > > > > be
> > > > > > > hoisted up and the 'goto' removed.
> > > > > > 
> > > > > > This actually depends on how the support for cast between
> > > > > > integers
> > > > > > and 
> > > > > > pointers will be implemented (see below).
> > > > > > If we will support truncating pointers (does that even make
> > > > > > sense?
> > > > > > and
> > > > > > I 
> > > > > > guess we cannot extend a pointer unless we add the support
> > > > > > for 
> > > > > > uint128_t), that label will be reused for that case.
> > > > > > Otherwise, it might not be reused.
> > > > > > 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-27 Thread Antoni Boucher via Gcc-patches
I chose option A, so everything is a size_t, now.
I also renamed the dyncast functions.
Here's the new patch.

Le jeudi 27 mai 2021 à 18:19 -0400, David Malcolm a écrit :
> On Tue, 2021-05-25 at 20:19 -0400, Antoni Boucher wrote:
> > @David: PING
> > 
> > As far as I know, the only remaining question is about using
> > `ssize_t`
> > for the return type of some functions.
> > Here's why I use this type:
> > 
> > That seemed off to return NULL for the functions returning a 
> > size_t to indicate an error. So I changed it to return -1 (and
> > return
> > type to ssize_t). Is that the proper way to indicate an error?
> > 
> > Once I know the answer for this error handling question, I'll fix
> > the
> > types.
> > 
> > Thanks!
> 
> I think the things that accept params for indexes should be size_t
> (i.e. unsigned).
> 
> As for the error-handling, I've been going back and forth.
> 
> The various return of -1 in the patch all occur when NULL is passed
> in
> as the gcc_jit_foo*, for the various foo. Unfortunately this means
> that
> we can't get at a jit context to emit the error on, so jit_error is
> called with a NULL context, and hence merely prints to stderr; it
> can't
> set state on any jit context.
> 
> Presumably if the code is passing in NULL for one of these things,
> then
> something's gone wrong already.
> 
> Option (A): returning a size_t (unsigned):
> - No way to indicate error if the user passes in NULL, or a bogus
> value, beyond output on stderr; we have to defensively return 0,
> which
> the user's code is likely to gracefully handle
> 
> Option (B): returning a ssize_t:
> - Can return -1 if the user passes in NULL or a bogus value.  But if
> the user doesn't check for -1 and blithely casts to unsigned (e.g.
> allocating a buffer), their code is likely to explode with a very
> large
> allocation.
> 
> Consider e.g. gcc_jit_function_type_get_param_count.  If the user
> uses 
> gcc_jit_type_is_function_ptr_type to dynamically cast some type to
> function_type and forgets to check, and passes in NULL, then...
> 
> ...with option (A), it returns 0, as if the function has no params;
> eventually a type-checking error occurs.
> 
> ...with option (B), it returns -1, and the user who didn't check
> before
> has to check again
> 
> So I think I prefer option (A), but I'm willing to be convinced on
> the
> alternative.
> 
> I also now think that the dynamic cast functions (e.g.
> gcc_jit_type_is_function_ptr_type) should be "_dyncast_" rather than
> "_is_" (e.g. gcc_jit_type_dyncast_function_ptr_type) to make it clear
> that this is a dynamic cast that can fail, and to contrast them with
> the "_as_" functions which are static casts.
> 
> Hope this makes sense and is constructive.
> Dave
> 
> 
> > 
> > Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> > > On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > > > I was missing a check in gcc_jit_struct_get_field, I added it
> > > > in
> > > > this
> > > > new patch.
> > > > 
> > > 
> > > Sorry about the long delay in reviewing this patch.
> > > 
> > > The main high-level points are:
> > > - currently the get_*_count functions return "ssize_t" - why? 
> > > Only
> > > unsigned values are meaningful; shouldn't they return "size_t"
> > > instead?
> > > 
> > > - the various "lookup by index" functions take "int" i.e. signed,
> > > but
> > > only >= 0 is meaningful.  I think it makes sense to make them
> > > take
> > > size_t instead.
> > > 
> > > Sorry if we covered that before in the review, it's been a while.
> > > 
> > > Various nitpicks inline below...
> > > 
> > > [...snip...]
> > >  
> > > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > > b/gcc/jit/docs/topics/compatibility.rst
> > > > index 6bfa101ed71..236e5c72d81 100644
> > > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > > @@ -226,3 +226,44 @@ entrypoints:
> > > >  
> > > >  ``LIBGCCJIT_ABI_14`` covers the addition of
> > > >  :func:`gcc_jit_global_set_initializer`
> > > > +
> > > > +.. _LIBGCCJIT_ABI_15:
> > > > +
> > > > +``LIBGCCJIT_ABI_15``
> > > > +
> > > > +``LIBGCCJIT_ABI_15`` covers the addition of reflection
> > > > functions
> > > > via
> > > > API
> > > > +entrypoints:
> > > 
> > > This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.
> > > 
> > > [...snip...]
> > > 
> > > > diff --git a/gcc/jit/docs/topics/functions.rst
> > > > b/gcc/jit/docs/topics/functions.rst
> > > > index eb40d64010e..aa6de87282d 100644
> > > > --- a/gcc/jit/docs/topics/functions.rst
> > > > +++ b/gcc/jit/docs/topics/functions.rst
> > > > @@ -171,6 +171,16 @@ Functions
> > > >     underlying string, so it is valid to pass in a pointer to
> > > > an
> > > > on-
> > > > stack
> > > >     buffer.
> > > >  
> > > > +.. function::  ssize_t \
> > > > +   gcc_jit_function_get_param_count
> > > > (gcc_jit_function
> > > > *func)
> > > > +
> > > > +   Get the number of paramete

Re: [PATCH] Add gnu::diagnose_as attribute

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 6:07 PM, Matthias Kretz wrote:

On Thursday, 27 May 2021 23:15:46 CEST Jason Merrill wrote:

On 5/27/21 2:54 PM, Matthias Kretz wrote:

Also hiding all inline namespace by default might make some error messages
harder to understand:

namespace Vir {
inline namespace foo {
  struct A {};
}
struct A {};
}
using Vir::A;

:7:12: error: reference to 'A' is ambiguous
:3:12: note: candidates are: 'struct Vir::A'
:5:10: note: 'struct Vir::A'


That doesn't seem so bad.


As long as you ignore the line numbers, it's a puzzling diagnostic.


Only briefly puzzling, I think; Vir::A is a valid way of referring to 
both types.



This is from my pending std::string patch:

--- a/libstdc++-v3/include/bits/c++config
+++ b/libstdc++-v3/include/bits/c++config
@@ -299,7 +299,8 @@ namespace std

   #if _GLIBCXX_USE_CXX11_ABI
   namespace std
   {

-  inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+  inline namespace __cxx11
+__attribute__((__abi_tag__ ("cxx11"), __diagnose_as__("std"))) { }


This seems to have the same benefits and drawbacks of my inline
namespace suggestion.


True for std::string, not true for TS's where the extra '::experimental' still
makes finding the relevant information in diagnostics harder than necessary.


And it seems like applying the attribute to a
namespace means that enclosing namespaces are not printed, unlike the
handling for types.


Yes, that's also how I documented it. For nested namespaces I wanted to enable
the removal of nesting (e.g. from std::experimental::parallelism_v2::simd to
stdx::simd). However, for types and functions it would be a problem to drop
the enclosing scope, because the scope can be class templates and thus the
diagnose_as attribute would remove all template parms & args.


I'd think you could get the same effect from a hypothetical

namespace [[gnu::diagnose_as]] stdx = std::experimental;

though we'll need to add support for attributes on namespace aliases to 
the grammar.



-  typedef basic_stringstring;
+  typedef basic_string string
[[__gnu__::__diagnose_as__("string")]];


Here it seems like you want to say "use this typedef as the true name of
the type".  Is it useful to have to repeat the name?  Allowing people to
use names that don't correspond to actual declarations seems unnecessary.


Yes, but you could also use it to apply diagnose_as to a template
instantiation without introducing a name for users. E.g.

   using __only_to_apply_the_attribute [[gnu::diagnose_as("intvector")]]
 = std::vector;

Now all diagnostics of 'std::vector' would print 'intvector' instead.


Yes, but why would you want to?  Making diagnostics print names that the 
user can't use in their own code seems obfuscatory, and requiring users 
to write the same names in two places seems like extra work.



But in general, I tend to agree, for type aliases there's rarely a case where 
the
names wouldn't match.

However, I didn't want to special-case the attribute parameters for type
aliases (or introduce another attribute just for this case). The attribute
works consistently and with the same interface independent of where it's used.
I tried to build a generic, broad feature instead of a narrow one-problem
solution.


"Treat this declaration as the name of the type/namespace it refers to 
in diagnostics" also seems consistent to me.



FWIW, before you suggest to have one attribute for namespaces and one for type
aliases (to cover the std::string case), I have another use case in stdx::simd
(the spec requires simd_abi::scalar to be an alias):

   namespace std::experimental::parallelism_v2::simd_abi {
 struct [[gnu::diagnose_as("scalar")]] _Scalar;
 using scalar = _Scalar;
   }

If the attribute were on the type alias (using scalar [[gnu::diagnose_as]] =
_Scalar;), then we'd have to apply the attribute to _Scalar after it was
completed. That seemed like a bad idea to me.


Well, in your sample code, _Scalar isn't complete at the point of the 
alias-declaration.  But even if it were, since the attribute doesn't 
affect code generation, it doesn't strike me as a big problem.  Still, 
perhaps it would be better to store these aliases in a separate hash 
table instead of *_ATTRIBUTES.


Jason



Re: [PATCH] Remove CC0

2021-05-27 Thread Jeff Law via Gcc-patches




On 5/27/2021 4:28 PM, Segher Boessenkool wrote:

On Thu, May 27, 2021 at 12:58:10PM -0700, H.J. Lu wrote:

You missed:

config/cr16/cr16.md:   (clobber (cc0))]
config/cr16/cr16.md:  [(parallel [(set (cc0)
config/cr16/cr16.md:  [(set (cc0)
config/cr16/cr16.md: [(cc0) (const_int 0)]))]
config/cr16/cr16.md:  [(set (cc0)
config/cr16/cr16.md: [(cc0) (const_int 0)]))]

Now, cr16-elf won't build.

I did not miss that.  I did not convert targets away from cc0 here.  We
deprecated cc0 long ago, and it is time to remove it now.  cr16 is the
one target that still uses cc0.  It should be relatively trivial to move
to the modern world: two expanders, two insns; and only cmp, cbranch,
cstore for that matter; anyone who knows cr16 at all can probably do it.
But that is not me, I do not know how to *test* cr16 at all.
Similarly, I'm aware of it (it's failing in the tester), but I have 
neither the time nor the inclination to fix it.


Jeff


Re: [PATCH 0/11] warning control by group and location (PR 74765)

2021-05-27 Thread Jason Merrill via Gcc-patches

On 5/27/21 5:55 PM, David Malcolm wrote:

On Thu, 2021-05-27 at 10:41 -0600, Martin Sebor wrote:

On 5/27/21 5:19 AM, Richard Sandiford wrote:

Thanks for doing this.

Martin Sebor via Gcc-patches  writes:

[…]
On 5/24/21 5:08 PM, David Malcolm wrote:

On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:

Subsequent patches then replace invocations of the
TREE_NO_WARNING()
macro and the gimple_no_warning_p() and
gimple_set_no_warning()
functions throughout GCC with those and remove the legacy
APIs to
keep them from being accidentally reintroduced along with the
problem.
These are mostly mechanical changes, except that most of the
new
invocations also specify the option whose disposition to
query for
the expression or location, or which to enable or disable[2].
The last function, copy_no_warning(), copies the disposition
from
one expression or location to another.

A couple of design choices might be helpful to explain:

First, introducing "warning groups" rather than controlling
each
individual warning is motivated by a) the fact that the
latter
would make avoiding redundant warnings for related problems
cumbersome (e.g., after issuing a -Warray-bounds we want to
suppress -Wstringop-overflow as well -Wstringop-overread for
the same access and vice versa), and b) simplicity and
efficiency
of the implementation (mapping each option would require a
more
complex data structure like a bitmap).

Second, using location_t to associate expressions/statements
with
the warning groups also turns out to be more useful in
practice
than a direct association between a tree or gimple*, and also
simplifies managing the data structure.  Trees and gimple*
come
and go across passes, and maintaining a mapping for them that
accounts for the possibility of them being garbage-collected
and the same addresses reused is less than straightforward.


I find some of the terminology rather awkard due to it the
negation
we're already using with TREE_NO_WARNING, in that we're turning
on a
no_warning flag, and that this is a per-location/expr/stmt
thing that's
different from the idea of enabling/disabling a specific
warning
altogether (and the pragmas that control that).   Sometimes the
patches
refer to enabling/disabling warnings and I think I want
"enabling" and
"disabling" to mean the thing the user does with -Wfoo and -
Wno-foo.

Would calling it a "warning suppression" or somesuch be more or
less
klunky?


I like warning suppression :)  But I'm not sure where you suggest
I use the phrase.

I don't particularly care for the "no" in the API names either
(existing or new) and would prefer a positive form.  I considered
enable_warning() and warning_enabled() but I chose the names I
did
because they're close to their established gimple namesakes.  I'm
fine changing them to the alternatives, or if you or someone else
has a preference for different names I'm open to suggestions.
Let
me know.


Not my area, but FWIW, +1 for David's suggestion of
s/set_no_warning/suppress_warning/ or similar.  As well as the
problem with the double negative in negated conditions, I always
have to
remember whether TREE_NO_WARNING means that hasn't been anything to
warn
about yet or whether we shouldn't warn in future.


Sure.  Jason has a patch out for review to introduce

    warning_enabled_at (location, int option).


CCing Jason.

BTW, do you have a URL for that patch?


https://gcc.gnu.org/pipermail/gcc-patches/2021-May/571307.html

is the most recent version.

Jason



[PATCH] Generate gimple-match.c and generic-match.c earlier

2021-05-27 Thread Bernd Edlinger
Hi,

I was wondering, why gimple-match.c and generic-match.c
are not built early but always last, which slows down parallel
makes significantly.

The reason seems to be that generated_files does not
mention gimple-match.c and generic-match.c.

This comment in Makefile.in says it all:

$(ALL_HOST_OBJS) : | $(generated_files)

So this patch adds gimple-match.c generic-match.c to generated_files.


Tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.


2021-05-28  Bernd Edlinger  

* Makefile.in (generated_files): Add gimple-match.c and
generic-match.c
From 5c14c3f5852ddcc1d3b76e7c53260b0187604cd7 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger 
Date: Fri, 28 May 2021 06:27:27 +0200
Subject: [PATCH] Generate gimple-match.c and generic-match.c earlier

I was wondering, why gimple-match.c and generic-match.c
are not built early but always last, which slows down parallel
makes significantly.

The reason seems to be that generated_files does not
mention gimple-match.c and generic-match.c.

This comment in Makefile.in says it all:

$(ALL_HOST_OBJS) : | $(generated_files)

So this patch adds gimple-match.c generic-match.c to generated_files.

2021-05-28  Bernd Edlinger  

	* Makefile.in (generated_files): Add gimple-match.c and
	generic-match.c
---
 gcc/Makefile.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index da2ef24..4cb2966 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2753,6 +2753,7 @@ generated_files = config.h tm.h $(TM_P_H) $(TM_D_H) $(TM_H) multilib.h \
$(ALL_GTFILES_H) gtype-desc.c gtype-desc.h version.h \
options.h target-hooks-def.h insn-opinit.h \
common/common-target-hooks-def.h pass-instances.def \
+   gimple-match.c generic-match.c \
c-family/c-target-hooks-def.h d/d-target-hooks-def.h \
case-cfn-macros.h \
cfn-operators.pd omp-device-properties.h
-- 
1.9.1



Re: [PATCH] avoid invalid sprintf argument checking and folding (PR 100732)

2021-05-27 Thread Jeff Law via Gcc-patches




On 5/26/2021 5:18 PM, Martin Sebor via Gcc-patches wrote:

While checking objects whose addresses are passed to functions
declared to take const pointers and making sure they're initialized
the GCC 11 -Wmaybe-uninitialized enhancement assumes that the actual
argument is a pointer.

That's normally a safe assumption because for nonpointer arguments
the front ends add an explicit cast to the expected pointer type.
This doesn't happen for arguments passed through the ellipsis in
calls to variadic functions but those arguments aren't checked by
the enhancement (yet).

But when an invalid call to a variadic function like sprintf is
folded into one to strcpy() such as in the test case below this
assumption breaks and causes an ICE in the uninit pass.

  void f (char *d, int i)
  {
    __builtin_sprintf (d, "%s", i);   // uninit sees strcpy (d, i)
  }

The attached patch solves this by a) avoiding the unsafe assumption
in the uninit pass, and b) avoiding folding sprintf calls with invalid
arguments of nonpointer types to strcpy.

Tested on x86_64-linux.

Martin

gcc-100732.diff

PR middle-end/100732 - ICE on sprintf %s with integer argument

gcc/ChangeLog:

PR middle-end/100732
* gimple-fold.c (gimple_fold_builtin_sprintf): Avoid folding calls
with either source or destination argument of invalid type.
* tree-ssa-uninit.c (maybe_warn_pass_by_reference): Avoid checking
calls with arguments of invalid type.

gcc/testsuite/ChangeLog:

PR middle-end/100732
* gcc.dg/tree-ssa/builtin-snprintf-11.c: New test.
* gcc.dg/tree-ssa/builtin-snprintf-12.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-28.c: New test.
* gcc.dg/tree-ssa/builtin-sprintf-29.c: New test.
* gcc.dg/uninit-pr100732.c: New test.

OK
jeff



Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

2021-05-27 Thread Bernd Edlinger


On 5/28/21 6:42 AM, Bernd Edlinger wrote:
> Hi,
> 
> I was wondering, why gimple-match.c and generic-match.c
> are not built early but always last, which slows down parallel
> makes significantly.
> 
> The reason seems to be that generated_files does not
> mention gimple-match.c and generic-match.c.
> 
> This comment in Makefile.in says it all:
> 

Oh, dear, git commit did eliminate the comments
starting with "#"
the mentined comment is

# Dependency information.

# In order for parallel make to really start compiling the expensive
# objects from $(OBJS) as early as possible, build all their
# prerequisites strictly before all objects.

> $(ALL_HOST_OBJS) : | $(generated_files)
> 
> So this patch adds gimple-match.c generic-match.c to generated_files.
> 
> 
> Tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 
> 
> 2021-05-28  Bernd Edlinger  
> 
>   * Makefile.in (generated_files): Add gimple-match.c and
>   generic-match.c
> 
From 99eab77ebfaa02ee22263d89eb3ca812cf65263b Mon Sep 17 00:00:00 2001
From: Bernd Edlinger 
Date: Fri, 28 May 2021 06:27:27 +0200
Subject: [PATCH] Generate gimple-match.c and generic-match.c earlier

I was wondering, why gimple-match.c and generic-match.c
are not built early but always last, which slows down parallel
makes significantly.

The reason seems to be that generated_files does not
mention gimple-match.c and generic-match.c.

This comment in Makefile.in says it all:

"In order for parallel make to really start compiling the expensive
objects from $(OBJS) as early as possible, build all their
prerequisites strictly before all objects."

So this patch adds gimple-match.c generic-match.c to generated_files.

2021-05-28  Bernd Edlinger  

	* Makefile.in (generated_files): Add gimple-match.c and
	generic-match.c
---
 gcc/Makefile.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index da2ef24..4cb2966 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -2753,6 +2753,7 @@ generated_files = config.h tm.h $(TM_P_H) $(TM_D_H) $(TM_H) multilib.h \
$(ALL_GTFILES_H) gtype-desc.c gtype-desc.h version.h \
options.h target-hooks-def.h insn-opinit.h \
common/common-target-hooks-def.h pass-instances.def \
+   gimple-match.c generic-match.c \
c-family/c-target-hooks-def.h d/d-target-hooks-def.h \
case-cfn-macros.h \
cfn-operators.pd omp-device-properties.h
-- 
1.9.1



Re: [PATCHv2] Add a couple of A?CST1:CST2 match and simplify optimizations

2021-05-27 Thread Jeff Law via Gcc-patches




On 5/26/2021 11:29 AM, Andrew MacLeod via Gcc-patches wrote:

On 5/26/21 7:07 AM, Andrew Pinski wrote:

On Wed, May 26, 2021 at 2:01 AM Andrew Pinski  wrote:

On Wed, May 26, 2021 at 1:43 AM Bernd Edlinger
 wrote:

On 5/25/21 4:22 PM, Richard Biener via Gcc-patches wrote:

On Sun, May 23, 2021 at 12:03 PM apinski--- via Gcc-patches
 wrote:

From: Andrew Pinski 

Instead of some of the more manual optimizations inside phi-opt,
it would be good idea to do a lot of the heavy lifting inside match
and simplify instead. In the process, this moves the three simple
A?CST1:CST2 (where CST1 or CST2 is zero) simplifications.

OK? Boostrapped and tested on x86_64-linux-gnu with no regressions.

Differences from V1:
* Use bit_xor 1 instead of bit_not to fix the problem with 
boolean types

which are not 1 bit precision.

OK.

Thanks,
Richard.


Hmm, sorry, no luck.

I think this caused:

If anything it is a bad interaction with changes between r12-1046 and
r12-1053; I am suspecting a bug in those changes rather than my
changes causing the bug.  Debugging it right now.

(gdb) p debug_tree(name)
  
 unit-size 
 align:8 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x76b45b28 precision:1 min  max >

 def_stmt _19 = ~_8;
 version:19>

So what is happening is evrp converted:
ct_12 = ct_5 + -1;
Into
ct_12 = ct_5 == 1 ? 0 : 1;
(this was done before my patch)
And then it gets simplified to:
   _8 = ct_5 == 1;
   _19 = ~_8;
   ct_12 = (int) _19;
(after my match.pd patch)
Yup.  I've chased this kind of thing down repeatedly through the years.  
It's rare, but some transformations from match.pd create new SSA_NAMEs 
and the various passes need to be prepared to handle that.


Jeff



Re: [PATCH v2] forwprop: Support vec perm fed by CTOR and CTOR/CST [PR99398]

2021-05-27 Thread Kewen.Lin via Gcc-patches
on 2021/5/27 下午8:55, Richard Sandiford wrote:
> Sorry for the slow reponse.
> 
> "Kewen.Lin"  writes:
>> diff --git a/gcc/vec-perm-indices.c b/gcc/vec-perm-indices.c
>> index ede590dc5c9..57dd11d723c 100644
>> --- a/gcc/vec-perm-indices.c
>> +++ b/gcc/vec-perm-indices.c
>> @@ -101,6 +101,70 @@ vec_perm_indices::new_expanded_vector (const 
>> vec_perm_indices &orig,
>>m_encoding.finalize ();
>>  }
>>  
>> +/* Check whether we can switch to a new permutation vector that
>> +   selects the same input elements as ORIG, but with each element
>> +   built up from FACTOR pieces.  Return true if yes, otherwise
>> +   return false.  Every FACTOR permutation indexes should be
>> +   continuous separately and the first one of each batch should
>> +   be able to exactly modulo FACTOR.  For example, if ORIG is
>> +   { 2, 3, 4, 5, 0, 1, 6, 7 } and FACTOR is 2, the new permutation
>> +   is { 1, 2, 0, 3 }.  */
>> +
>> +bool
>> +vec_perm_indices::new_shrunk_vector (const vec_perm_indices &orig,
>> + unsigned int factor)
>> +{
>> +  gcc_assert (factor > 0);
>> +
>> +  if (maybe_lt (orig.m_nelts_per_input, factor))
>> +return false;
>> +
>> +  poly_uint64 nelts;
>> +  /* Invalid if vector units number isn't multiple of factor.  */
>> +  if (!multiple_p (orig.m_nelts_per_input, factor, &nelts))
>> +return false;
>> +
>> +  /* Only handle the case that npatterns is multiple of factor.
>> + FIXME: Try to see whether we can reshape it by factor npatterns.  */
>> +  if (orig.m_encoding.npatterns () % factor != 0)
>> +return false;
>> +
>> +  unsigned int encoded_nelts = orig.m_encoding.encoded_nelts ();
>> +  auto_vec encodings (encoded_nelts);
> 
> auto_vec would avoid memory allocations in the
> same cases that m_encoding can.  “encoding” might be better than
> “encodings” since there's only really one encoding here.
> 
>> +  /* Separate all encoded elements into batches by size factor,
>> + then ensure the first element of each batch is multiple of
>> + factor and all elements in each batch is consecutive from
>> + the first one.  */
>> +  for (unsigned int i = 0; i < encoded_nelts; i += factor)
>> +{
>> +  element_type first = orig.m_encoding[i];
>> +  element_type new_index;
>> +  if (!multiple_p (first, factor, &new_index))
>> +return false;
>> +  for (unsigned int j = 1; j < factor; ++j)
>> +{
>> +  if (maybe_ne (first + j, orig.m_encoding[i + j]))
>> +return false;
>> +}
> 
> Formatting nit: unnecessary braces around if.
> 
>> +  encodings.quick_push (new_index);
>> +}
>> +
>> +  m_ninputs = orig.m_ninputs;
>> +  m_nelts_per_input = nelts;
>> +  poly_uint64 full_nelts = exact_div (orig.m_encoding.full_nelts (), 
>> factor);
>> +  unsigned int npatterns = orig.m_encoding.npatterns () / factor;
>> +
>> +  m_encoding.new_vector (full_nelts, npatterns,
>> + orig.m_encoding.nelts_per_pattern ());
>> +
>> +  for (unsigned int i = 0; i < encodings.length (); i++)
>> +m_encoding.quick_push (encodings[i]);
> 
> I think this can be:
> 
>m_encoding.splice (encodings);
> 
> OK with those changes, thanks.  Thanks also for doing it in a
> variable-length-friendly way.
> 


Thanks for the comments, Richard!  The patch was updated as them,
re-tested and committed in r12-1103.

BR,
Kewen


Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

2021-05-27 Thread Richard Biener
On Fri, 28 May 2021, Bernd Edlinger wrote:

> 
> 
> On 5/28/21 6:42 AM, Bernd Edlinger wrote:
> > Hi,
> > 
> > I was wondering, why gimple-match.c and generic-match.c
> > are not built early but always last, which slows down parallel
> > makes significantly.
> > 
> > The reason seems to be that generated_files does not
> > mention gimple-match.c and generic-match.c.
> > 
> > This comment in Makefile.in says it all:
> > 
> 
> Oh, dear, git commit did eliminate the comments
> starting with "#"
> the mentined comment is
> 
> # Dependency information.
> 
> # In order for parallel make to really start compiling the expensive
> # objects from $(OBJS) as early as possible, build all their
> # prerequisites strictly before all objects.
> 
> > $(ALL_HOST_OBJS) : | $(generated_files)
> > 
> > So this patch adds gimple-match.c generic-match.c to generated_files.
> > 
> > 
> > Tested on x86_64-pc-linux-gnu.
> > Is it OK for trunk?

OK.  Does it really help though?

Thanks,
Richard.

> > 
> > 
> > Thanks
> > Bernd.
> > 
> > 
> > 2021-05-28  Bernd Edlinger  
> > 
> > * Makefile.in (generated_files): Add gimple-match.c and
> > generic-match.c
> > 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)


Re: [PATCH] Generate gimple-match.c and generic-match.c earlier

2021-05-27 Thread Bernd Edlinger
On 5/28/21 8:41 AM, Richard Biener wrote:
> On Fri, 28 May 2021, Bernd Edlinger wrote:
> 
>>
>>
>> On 5/28/21 6:42 AM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> I was wondering, why gimple-match.c and generic-match.c
>>> are not built early but always last, which slows down parallel
>>> makes significantly.
>>>
>>> The reason seems to be that generated_files does not
>>> mention gimple-match.c and generic-match.c.
>>>
>>> This comment in Makefile.in says it all:
>>>
>>
>> Oh, dear, git commit did eliminate the comments
>> starting with "#"
>> the mentined comment is
>>
>> # Dependency information.
>>
>> # In order for parallel make to really start compiling the expensive
>> # objects from $(OBJS) as early as possible, build all their
>> # prerequisites strictly before all objects.
>>
>>> $(ALL_HOST_OBJS) : | $(generated_files)
>>>
>>> So this patch adds gimple-match.c generic-match.c to generated_files.
>>>
>>>
>>> Tested on x86_64-pc-linux-gnu.
>>> Is it OK for trunk?
> 
> OK.  Does it really help though?
> 

Yes, I guess so, at least a little bit.

Prior to this patch the whole build stage was completed for everything
then those two big files got generated,
and then there are only two large files compiled in parallel for
several minutes at least.

So a make -j8 utilizes only 25 % cpu power
and make -j16 only 12.5 % utilization.
That can certainly be a bit annoying.


Bernd.


> Thanks,
> Richard.
> 
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>>
>>> 2021-05-28  Bernd Edlinger  
>>>
>>> * Makefile.in (generated_files): Add gimple-match.c and
>>> generic-match.c
>>>
>>
> 


Re: [PATCH RFA] tree-iterator: C++11 range-for and tree_stmt_iterator

2021-05-27 Thread Richard Biener via Gcc-patches
On Wed, May 26, 2021 at 3:56 PM Jason Merrill  wrote:
>
> Ping.

The non-C++ parts are OK.

Richard.

> On 5/17/21 1:58 PM, Jason Merrill wrote:
> > On 5/17/21 3:56 AM, Richard Biener wrote:
> >> On Fri, May 14, 2021 at 2:23 AM Martin Sebor via Gcc-patches
> >>  wrote:
> >>>
> >>> On 5/13/21 1:26 PM, Jason Merrill via Gcc-patches wrote:
>  Ping.
> 
>  On 5/1/21 12:29 PM, Jason Merrill wrote:
> > Like my recent patch to add ovl_range and lkp_range in the C++
> > front end,
> > this patch adds the tsi_range adaptor for using C++11 range-based
> > 'for' with
> > a STATEMENT_LIST, e.g.
> >
> > for (tree stmt : tsi_range (stmt_list)) { ... }
> >
> > This also involves adding some operators to tree_stmt_iterator that
> > are
> > needed for range-for iterators, and should also be useful in code that
> > uses
> > the iterators directly.
> >
> > The patch updates the suitable loops in the C++ front end, but does
> > not
> > touch any loops elsewhere in the compiler.
> >>>
> >>> I like the modernization of the loops.
> >>
> >> The only worry I have (and why I stopped looking at range-for) is that
> >> this adds another style of looping over stmts without opening the
> >> possibility to remove another or even unify all of them.  That's because
> >> range-for isn't powerful enough w/o jumping through hoops and/or
> >> we cannot use what appearantly ranges<> was intended for (fix
> >> this limitation).
> >
> > The range-for enabled by my patch simplifies the common case of simple
> > iteration over elements; that seems worth doing to me even if it doesn't
> > replace all loops.  Just as FOR_EACH_VEC_ELT isn't suitable for all
> > loops over a vector.
> >
> >> That said, if some C++ literate could see if for example
> >> what gimple-iterator.h provides can be completely modernized
> >> then that would be great of course.
> >>
> >> There's stuff like reverse iteration
> >
> > This is typically done with the reverse_iterator<> adaptor, which we
> > could get from  or duplicate.  I didn't see enough reverse
> > iterations to make it seem worth the bother.
> >
> >> iteration skipping debug stmts,
> >
> > There you can move the condition into the loop:
> >
> > if (gimple_code (stmt) == GIMPLE_DEBUG)
> >   continue;
> >
> >> compares of iterators like gsi_one_before_end_p, etc.
> >
> > Certainly anything where you want to mess with the iterators directly
> > doesn't really translate to range-for.
> >
> >> Given my failed tries (but I'm a C++ illiterate) my TODO list now
> >> only contains turning the iterators into STL style ones, thus
> >> gsi_stmt (it) -> *it, gsi_next (&it) -> ++it, etc. - but even
> >> it != end_p looks a bit awkward there.
> >
> > Well, it < end_val is pretty typical for loops involving integer
> > iterators.  But you don't have to use that style if you'd rather not.
> > You could continue to use gsi_end_p, or just *it, since we know that *it
> > is NULL at the end of the sequence.
> >
> >>> I can't find anything terribly wrong with the iterator but let me
> >>> at least pick on some nits ;)
> >>>
> >
> > gcc/ChangeLog:
> >
> >  * tree-iterator.h (struct tree_stmt_iterator): Add operator++,
> >  operator--, operator*, operator==, and operator!=.
> >  (class tsi_range): New.
> >
> > gcc/cp/ChangeLog:
> >
> >  * constexpr.c (build_data_member_initialization): Use tsi_range.
> >  (build_constexpr_constructor_member_initializers): Likewise.
> >  (constexpr_fn_retval, cxx_eval_statement_list): Likewise.
> >  (potential_constant_expression_1): Likewise.
> >  * coroutines.cc (await_statement_expander): Likewise.
> >  (await_statement_walker): Likewise.
> >  * module.cc (trees_out::core_vals): Likewise.
> >  * pt.c (tsubst_expr): Likewise.
> >  * semantics.c (set_cleanup_locs): Likewise.
> > ---
> >gcc/tree-iterator.h  | 28 +++-
> >gcc/cp/constexpr.c   | 42
> > ++
> >gcc/cp/coroutines.cc | 10 --
> >gcc/cp/module.cc |  5 ++---
> >gcc/cp/pt.c  |  5 ++---
> >gcc/cp/semantics.c   |  5 ++---
> >6 files changed, 47 insertions(+), 48 deletions(-)
> >
> > diff --git a/gcc/tree-iterator.h b/gcc/tree-iterator.h
> > index 076fff8644c..f57456bb473 100644
> > --- a/gcc/tree-iterator.h
> > +++ b/gcc/tree-iterator.h
> > @@ -1,4 +1,4 @@
> > -/* Iterator routines for manipulating GENERIC tree statement list.
> > +/* Iterator routines for manipulating GENERIC tree statement list.
> > -*- C++ -*-
> >   Copyright (C) 2003-2021 Free Software Foundation, Inc.
> >   Contributed by Andrew MacLeod  
> > @@ -32,6 +32,13 @@ along with GCC; see the file COPYING3.  If not see
> >struct tree_stmt_iterator {
> >  struct tree_s