Re: [PATCH 23/52] mmix: Remove macros {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE

[Hans-Peter Nilsson]

On Sun, 2 Jun 2024, Kewen Lin wrote:

> This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE
> defines in mmix port.

This is fine once prerequisites are in place.

If I may add a nit: In these target change commit messages, add 
a hint as to which defaulted hook or macro the removed macro now 
corresponds to, like "these now correspond to the default values 
of the new target hook mode_for_floating_type".  Else when doing 
port archaeology, from the commit message it looks like the 
context has not changed, you just removed some definitions.

> 
> gcc/ChangeLog:
> 
>   * config/mmix/mmix.h (FLOAT_TYPE_SIZE): Remove.
>   (DOUBLE_TYPE_SIZE): Likewise.
>   (LONG_DOUBLE_TYPE_SIZE): Likewise.
> ---
>  gcc/config/mmix/mmix.h | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/gcc/config/mmix/mmix.h b/gcc/config/mmix/mmix.h
> index c3c5a2a69c9..e20bca1d363 100644
> --- a/gcc/config/mmix/mmix.h
> +++ b/gcc/config/mmix/mmix.h
> @@ -195,10 +195,6 @@ struct GTY(()) machine_function
>  #define SHORT_TYPE_SIZE 16
>  #define LONG_LONG_TYPE_SIZE 64
>  
> -#define FLOAT_TYPE_SIZE 32
> -#define DOUBLE_TYPE_SIZE 64
> -#define LONG_DOUBLE_TYPE_SIZE 64
> -
>  #define DEFAULT_SIGNED_CHAR 1
>  
>  
> -- 
> 2.43.0
> 
> 

Re: Reverted recent patches to resource.cc

[Hans-Peter Nilsson]

> Date: Wed, 29 May 2024 21:23:58 -0600
> Cc: gcc-patches@gcc.gnu.org

> I don't bother with qemu.exp at all.  I've set up binfmt handlers so 
> that I can execute foreign binaries.
> 
> So given a root filesystem, I can chroot into it and do whatever I need. 
>   As far as dejagnu is concerned it looks like the native system.

Interesting.  In a Docker setup (or similar container)?
If so, care to share the Dockerfile (or equivalent)?

A quick web search shows similar attempts, but I found
nothing "turn-key ready" that enables bootstrapping.

I'll try to cook up something, likely Debian-based...later
this decade, aiming for suitability in contrib/.

brgds, H-P
ps. my hyperbolic time guesstimates somehow still expire too soon!

Re: Reverted recent patches to resource.cc

[Hans-Peter Nilsson]

> Date: Wed, 29 May 2024 20:07:22 -0600
> From: Jeff Law 

> > There appears to be only a single supported SPARC machine in
> > cfarm: cfarm216, and I currently can't reach it due to what
> > appears to be issues at my end.  I guess I'll either fix
> > that or breathe life into sparc-elf+sim.
> Or if you've got a reasonable server to use, QEMU might save you :-)

I believe so. :)

> I do bootstraps and regression testsuite runs on a variety of systems 
> via qemu (alpha, m68k, aarch64, s390, ppc64, etc).  It ain't fast, but 
> it does work if QEMU is in pretty good shape and you can find a root 
> filesystem to use.

That might certainly fit the bill.  I guess you mean with a
filesystem image for e.g. sparc-linux?

I keep postponing looking into getting a working setup
(mostly the baseboard file) for qemu-anything + newlib.
Last I looked, qemu.exp had a serious typo...but I see that
was just for arm-eabi and arm-pi4, so yes, that might be a
viable path, thanks for the reminder.

You (or anyone) don't happen to know if sparc-elf + qemu.exp
is in good shape, or some other specific sparc+qemu
configuration?

That "sparc=*" in qemu.exp entry (at dejagnu
ca371cf9c48186716d) looks suspicious though, so I guess it'd
be a tuple matching "sparc32plus-*" or "sparc64-*".

brgds, H-P

Reverted recent patches to resource.cc

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Mon, 27 May 2024 19:51:47 +0200

> 2: Does not depend on 1, but corrects an incidentally found wart:
> find_basic_block calls fails too often.  Replace it with "modern"
> insn-to-basic-block cross-referencing.
> 
> 3: Just an addendum to 2: removes an "if", where the condition is now
> always-true, dominated by a gcc_assert, and where the change in
> indentation was too ugly.
> 
> 4: Corrects another incidentally found wart: for the last 15 years the
> code in resource.cc has only been called from within reorg.cc (and
> reorg.c), specifically not possibly before calling init_resource_info
> or after free_resource_info, so we can discard the code that tests
> certain allocated arrays for NULL.  I didn't even bother with a
> gcc_assert; besides some gen*-generated files, only reorg.cc includes
> resource.h (not to be confused with the system sys/resource.h).
> A grep says the #include resource.h can be removed from those gen*
> files and presumably from RESOURCE_H(!) as well.  Some Other Time.
> Also, removed a redundant "if (tinfo != NULL)" and moved the then-code
> into the previous then-clause.
> 
>   resource.cc: Replace calls to find_basic_block with cfgrtl
> BLOCK_FOR_INSN
>   resource.cc (mark_target_live_regs): Remove check for bb not found
>   resource.cc: Remove redundant conditionals

I had to revert those last three patches due to PR
bootstrap/115284.  I hope to revisit once I have a means to
reproduce (and fix) the underlying bug.  It doesn't have to
be a bug with those changes per-se: IMHO the "improved"
lifetimes could just as well have uncovered a bug elsewhere
in reorg.  It's still on me to resolve that situation; done.
I'm just glad the cause was the incidental improvements and
not the original bug I wanted to fix.

There appears to be only a single supported SPARC machine in
cfarm: cfarm216, and I currently can't reach it due to what
appears to be issues at my end.  I guess I'll either fix
that or breathe life into sparc-elf+sim.

brgds, H-P

[gcc r15-916] Revert "resource.cc: Replace calls to find_basic_block with cfgrtl BLOCK_FOR_INSN"

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:c68bd7e8023f65d1dc23237f5a04a863344b1264

commit r15-916-gc68bd7e8023f65d1dc23237f5a04a863344b1264
Author: Hans-Peter Nilsson 
Date:   Thu May 30 01:57:39 2024 +0200

Revert "resource.cc: Replace calls to find_basic_block with cfgrtl 
BLOCK_FOR_INSN"

This reverts commit 933ab59c59bdc1ac9e3ca3a56527836564e1821b.

Diff:
---
 gcc/resource.cc | 66 -
 1 file changed, 56 insertions(+), 10 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 0d8cde93570..06fcfd3e44c 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -28,7 +28,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm_p.h"
 #include "regs.h"
 #include "emit-rtl.h"
-#include "cfgrtl.h"
 #include "resource.h"
 #include "insn-attr.h"
 #include "function-abi.h"
@@ -76,6 +75,7 @@ static HARD_REG_SET current_live_regs;
 static HARD_REG_SET pending_dead_regs;
 
 static void update_live_status (rtx, const_rtx, void *);
+static int find_basic_block (rtx_insn *, int);
 static rtx_insn *next_insn_no_annul (rtx_insn *);
 
 /* Utility function called from mark_target_live_regs via note_stores.
@@ -113,6 +113,46 @@ update_live_status (rtx dest, const_rtx x, void *data 
ATTRIBUTE_UNUSED)
CLEAR_HARD_REG_BIT (pending_dead_regs, i);
   }
 }
+
+/* Find the number of the basic block with correct live register
+   information that starts closest to INSN.  Return -1 if we couldn't
+   find such a basic block or the beginning is more than
+   SEARCH_LIMIT instructions before INSN.  Use SEARCH_LIMIT = -1 for
+   an unlimited search.
+
+   The delay slot filling code destroys the control-flow graph so,
+   instead of finding the basic block containing INSN, we search
+   backwards toward a BARRIER where the live register information is
+   correct.  */
+
+static int
+find_basic_block (rtx_insn *insn, int search_limit)
+{
+  /* Scan backwards to the previous BARRIER.  Then see if we can find a
+ label that starts a basic block.  Return the basic block number.  */
+  for (insn = prev_nonnote_insn (insn);
+   insn && !BARRIER_P (insn) && search_limit != 0;
+   insn = prev_nonnote_insn (insn), --search_limit)
+;
+
+  /* The closest BARRIER is too far away.  */
+  if (search_limit == 0)
+return -1;
+
+  /* The start of the function.  */
+  else if (insn == 0)
+return ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index;
+
+  /* See if any of the upcoming CODE_LABELs start a basic block.  If we reach
+ anything other than a CODE_LABEL or note, we can't find this code.  */
+  for (insn = next_nonnote_insn (insn);
+   insn && LABEL_P (insn);
+   insn = next_nonnote_insn (insn))
+if (BLOCK_FOR_INSN (insn))
+  return BLOCK_FOR_INSN (insn)->index;
+
+  return -1;
+}
 
 /* Similar to next_insn, but ignores insns in the delay slots of
an annulled branch.  */
@@ -674,8 +714,7 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 }
 
   if (b == -1)
-b = BLOCK_FOR_INSN (target)->index;
-  gcc_assert (b != -1);
+b = find_basic_block (target, param_max_delay_slot_live_search);
 
   if (target_hash_table != NULL)
 {
@@ -683,7 +722,7 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
{
  /* If the information is up-to-date, use it.  Otherwise, we will
 update it below.  */
- if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
+ if (b == tinfo->block && b != -1 && tinfo->bb_tick == bb_ticks[b])
{
  res->regs = tinfo->live_regs;
  return;
@@ -866,6 +905,7 @@ void
 init_resource_info (rtx_insn *epilogue_insn)
 {
   int i;
+  basic_block bb;
 
   /* Indicate what resources are required to be valid at the end of the current
  function.  The condition code never is and memory always is.
@@ -935,8 +975,10 @@ init_resource_info (rtx_insn *epilogue_insn)
   target_hash_table = XCNEWVEC (struct target_info *, TARGET_HASH_PRIME);
   bb_ticks = XCNEWVEC (int, last_basic_block_for_fn (cfun));
 
-  /* Set the BLOCK_FOR_INSN for each insn.  */
-  compute_bb_for_insn ();
+  /* Set the BLOCK_FOR_INSN of each label that starts a basic block.  */
+  FOR_EACH_BB_FN (bb, cfun)
+if (LABEL_P (BB_HEAD (bb)))
+  BLOCK_FOR_INSN (BB_HEAD (bb)) = bb;
 }
 
 /* Free up the resources allocated to mark_target_live_regs ().  This
@@ -945,6 +987,8 @@ init_resource_info (rtx_insn *epilogue_insn)
 void
 free_resource_info (void)
 {
+  basic_block bb;
+
   if (target_hash_table != NULL)
 {
   int i;
@@ -971,7 +1015,9 @@ free_resource_info (void)
   bb_ticks = NULL;
 }
 
-  free_bb_for_insn ();
+  FOR_EACH_BB_FN (bb, cfun)
+if (LABEL_P (BB_HEAD (bb)))
+  BLOCK_FOR_INSN (BB_HEAD (bb)) = NULL

[gcc r15-915] Revert "resource.cc (mark_target_live_regs): Remove check for bb not found"

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:afe48a45b8baa310c8373499b1e5b5407a3e2b94

commit r15-915-gafe48a45b8baa310c8373499b1e5b5407a3e2b94
Author: Hans-Peter Nilsson 
Date:   Thu May 30 01:57:29 2024 +0200

Revert "resource.cc (mark_target_live_regs): Remove check for bb not found"

This reverts commit e1abce5b6ad8f5aee86ec7729b516d81014db09e.

Diff:
---
 gcc/resource.cc | 270 +---
 1 file changed, 138 insertions(+), 132 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 62bd46f786e..0d8cde93570 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -704,150 +704,156 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
 
-  /* Get the live registers from the basic block and update them with
- anything set or killed between its start and the insn before
- TARGET; this custom life analysis is really about registers so we
- need to use the LR problem.  Otherwise, we must assume everything
- is live.  */
-  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
-  rtx_insn *start_insn, *stop_insn;
-  df_ref def;
-
-  /* Compute hard regs live at start of block.  */
-  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
-  FOR_EACH_ARTIFICIAL_DEF (def, b)
-if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
-  SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
-
-  /* Get starting and ending insn, handling the case where each might
- be a SEQUENCE.  */
-  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
-   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
-  stop_insn = target;
-
-  if (NONJUMP_INSN_P (start_insn)
-  && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
-start_insn = as_a  (PATTERN (start_insn))->insn (0);
-
-  if (NONJUMP_INSN_P (stop_insn)
-  && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
-stop_insn = next_insn (PREV_INSN (stop_insn));
-
-  for (insn = start_insn; insn != stop_insn;
-   insn = next_insn_no_annul (insn))
+  /* If we found a basic block, get the live registers from it and update
+ them with anything set or killed between its start and the insn before
+ TARGET; this custom life analysis is really about registers so we need
+ to use the LR problem.  Otherwise, we must assume everything is live.  */
+  if (b != -1)
 {
-  rtx link;
-  rtx_insn *real_insn = insn;
-  enum rtx_code code = GET_CODE (insn);
-
-  if (DEBUG_INSN_P (insn))
-   continue;
-
-  /* If this insn is from the target of a branch, it isn't going to
-be used in the sequel.  If it is used in both cases, this
-test will not be true.  */
-  if ((code == INSN || code == JUMP_INSN || code == CALL_INSN)
- && INSN_FROM_TARGET_P (insn))
-   continue;
-
-  /* If this insn is a USE made by update_block, we care about the
-underlying insn.  */
-  if (code == INSN
- && GET_CODE (PATTERN (insn)) == USE
- && INSN_P (XEXP (PATTERN (insn), 0)))
-   real_insn = as_a  (XEXP (PATTERN (insn), 0));
-
-  if (CALL_P (real_insn))
+  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
+  rtx_insn *start_insn, *stop_insn;
+  df_ref def;
+
+  /* Compute hard regs live at start of block.  */
+  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
+  FOR_EACH_ARTIFICIAL_DEF (def, b)
+   if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
+ SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
+
+  /* Get starting and ending insn, handling the case where each might
+be a SEQUENCE.  */
+  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
+   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
+  stop_insn = target;
+
+  if (NONJUMP_INSN_P (start_insn)
+ && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
+   start_insn = as_a  (PATTERN (start_insn))->insn (0);
+
+  if (NONJUMP_INSN_P (stop_insn)
+ && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
+   stop_insn = next_insn (PREV_INSN (stop_insn));
+
+  for (insn = start_insn; insn != stop_insn;
+  insn = next_insn_no_annul (insn))
{
- /* Values in call-clobbered registers survive a COND_EXEC CALL
-if that is not executed; this matters for resoure use because
-they may be used by a complementarily (or more strictly)
-predicated instruction, or if the CALL is NORETURN.  */
- if (GET_CODE (PATTERN (real_insn)) != COND_EXEC)
+ rtx link;
+ rtx_insn *real_insn = insn;
+ enum rtx_code code = GET_CODE (insn);
+
+ if (DEBUG_INSN_P (insn))
+   continue;
+
+ /* If this insn is from the target of a branch, it isn't going to
+be used in the sequel.  I

[gcc r15-914] Revert "resource.cc: Remove redundant conditionals"

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:c31a9d3152d6119aab83c403308ddb933fe905c5

commit r15-914-gc31a9d3152d6119aab83c403308ddb933fe905c5
Author: Hans-Peter Nilsson 
Date:   Thu May 30 01:57:16 2024 +0200

Revert "resource.cc: Remove redundant conditionals"

This reverts commit 802a98d128f9b0eea2432f6511328d14e0bd721b.

Diff:
---
 gcc/resource.cc | 123 
 1 file changed, 71 insertions(+), 52 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 7c1de886432..62bd46f786e 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -658,41 +658,48 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
   res->cc = 0;
 
   /* See if we have computed this value already.  */
-  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
-   tinfo; tinfo = tinfo->next)
-if (tinfo->uid == INSN_UID (target))
-  break;
-
-  /* Start by getting the basic block number.  If we have saved
- information, we can get it from there unless the insn at the
- start of the basic block has been deleted.  */
-  if (tinfo && tinfo->block != -1
-  && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
-b = tinfo->block;
+  if (target_hash_table != NULL)
+{
+  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+  tinfo; tinfo = tinfo->next)
+   if (tinfo->uid == INSN_UID (target))
+ break;
+
+  /* Start by getting the basic block number.  If we have saved
+information, we can get it from there unless the insn at the
+start of the basic block has been deleted.  */
+  if (tinfo && tinfo->block != -1
+ && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
+   b = tinfo->block;
+}
 
   if (b == -1)
 b = BLOCK_FOR_INSN (target)->index;
   gcc_assert (b != -1);
 
-  if (tinfo)
+  if (target_hash_table != NULL)
 {
-  /* If the information is up-to-date, use it.  Otherwise, we will
-update it below.  */
-  if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
+  if (tinfo)
{
- res->regs = tinfo->live_regs;
- return;
+ /* If the information is up-to-date, use it.  Otherwise, we will
+update it below.  */
+ if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
+   {
+ res->regs = tinfo->live_regs;
+ return;
+   }
+   }
+  else
+   {
+ /* Allocate a place to put our results and chain it into the
+hash table.  */
+ tinfo = XNEW (struct target_info);
+ tinfo->uid = INSN_UID (target);
+ tinfo->block = b;
+ tinfo->next
+   = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+ target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
}
-}
-  else
-{
-  /* Allocate a place to put our results and chain it into the hash
-table.  */
-  tinfo = XNEW (struct target_info);
-  tinfo->uid = INSN_UID (target);
-  tinfo->block = b;
-  tinfo->next = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
-  target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
 }
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
@@ -818,12 +825,13 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 to be live here still are.  The fallthrough edge may have
 left a live register uninitialized.  */
  bb = BLOCK_FOR_INSN (real_insn);
- gcc_assert (bb);
-
- HARD_REG_SET extra_live;
+ if (bb)
+   {
+ HARD_REG_SET extra_live;
 
- REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
- current_live_regs |= extra_live;
+ REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
+ current_live_regs |= extra_live;
+   }
}
 
   /* The beginning of the epilogue corresponds to the end of the
@@ -839,8 +847,10 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 {
   tinfo->block = b;
   tinfo->bb_tick = bb_ticks[b];
-  tinfo->live_regs = res->regs;
 }
+
+  if (tinfo != NULL)
+tinfo->live_regs = res->regs;
 }
 
 /* Initialize the resources required by mark_target_live_regs ().
@@ -929,25 +939,31 @@ init_resource_info (rtx_insn *epilogue_insn)
 void
 free_resource_info (void)
 {
-  int i;
-
-  for (i = 0; i < TARGET_HASH_PRIME; ++i)
+  if (target_hash_table != NULL)
 {
-  struct target_info *ti = target_hash_table[i];
+  int i;
 
-  while (ti)
+  for (i = 0; i < TARGET_HASH_PRIME; ++i)
{
- struct target_info *next = ti->next;
- free (ti);
- ti = 

[gcc r15-880] resource.cc: Remove redundant conditionals

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:802a98d128f9b0eea2432f6511328d14e0bd721b

commit r15-880-g802a98d128f9b0eea2432f6511328d14e0bd721b
Author: Hans-Peter Nilsson 
Date:   Tue May 28 23:18:14 2024 +0200

resource.cc: Remove redundant conditionals

No functional change.

- We always have a target_hash_table and bb_ticks because
init_resource_info is always called.  These conditionals are
an ancient artifact: it's been quite a while since
resource.cc was used elsewhere than exclusively from reorg.cc

- In mark_target_live_regs, get rid of a now-redundant "if
(tinfo != NULL)" conditional and replace an "if (bb)" with a
gcc_assert.

A "git diff -wb" (ignore whitespace diff) is better at
showing the actual changes.

* resource.cc (free_resource_info, clear_hashed_info_for_insn): 
Don't
check for non-null target_hash_table and bb_ticks.
(mark_target_live_regs): Ditto.  Replace check for non-NULL result 
from
BLOCK_FOR_INSN with a call to gcc_assert.  Fold code conditioned on
tinfo != NULL.

Diff:
---
 gcc/resource.cc | 123 
 1 file changed, 52 insertions(+), 71 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 62bd46f786e..7c1de886432 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -658,49 +658,42 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
   res->cc = 0;
 
   /* See if we have computed this value already.  */
-  if (target_hash_table != NULL)
-{
-  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
-  tinfo; tinfo = tinfo->next)
-   if (tinfo->uid == INSN_UID (target))
- break;
-
-  /* Start by getting the basic block number.  If we have saved
-information, we can get it from there unless the insn at the
-start of the basic block has been deleted.  */
-  if (tinfo && tinfo->block != -1
- && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
-   b = tinfo->block;
-}
+  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+   tinfo; tinfo = tinfo->next)
+if (tinfo->uid == INSN_UID (target))
+  break;
+
+  /* Start by getting the basic block number.  If we have saved
+ information, we can get it from there unless the insn at the
+ start of the basic block has been deleted.  */
+  if (tinfo && tinfo->block != -1
+  && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
+b = tinfo->block;
 
   if (b == -1)
 b = BLOCK_FOR_INSN (target)->index;
   gcc_assert (b != -1);
 
-  if (target_hash_table != NULL)
+  if (tinfo)
 {
-  if (tinfo)
+  /* If the information is up-to-date, use it.  Otherwise, we will
+update it below.  */
+  if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
{
- /* If the information is up-to-date, use it.  Otherwise, we will
-update it below.  */
- if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
-   {
- res->regs = tinfo->live_regs;
- return;
-   }
-   }
-  else
-   {
- /* Allocate a place to put our results and chain it into the
-hash table.  */
- tinfo = XNEW (struct target_info);
- tinfo->uid = INSN_UID (target);
- tinfo->block = b;
- tinfo->next
-   = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
- target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
+ res->regs = tinfo->live_regs;
+ return;
}
 }
+  else
+{
+  /* Allocate a place to put our results and chain it into the hash
+table.  */
+  tinfo = XNEW (struct target_info);
+  tinfo->uid = INSN_UID (target);
+  tinfo->block = b;
+  tinfo->next = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+  target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
+}
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
 
@@ -825,13 +818,12 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 to be live here still are.  The fallthrough edge may have
 left a live register uninitialized.  */
  bb = BLOCK_FOR_INSN (real_insn);
- if (bb)
-   {
- HARD_REG_SET extra_live;
+ gcc_assert (bb);
 
- REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
- current_live_regs |= extra_live;
-   }
+ HARD_REG_SET extra_live;
+
+ REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
+ current_live_regs |= extra_live;
}
 
   /* The beginning of the epilogue corresponds to 

[gcc r15-879] resource.cc (mark_target_live_regs): Remove check for bb not found

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:e1abce5b6ad8f5aee86ec7729b516d81014db09e

commit r15-879-ge1abce5b6ad8f5aee86ec7729b516d81014db09e
Author: Hans-Peter Nilsson 
Date:   Tue May 28 23:17:31 2024 +0200

resource.cc (mark_target_live_regs): Remove check for bb not found

No functional change.

A "git diff -wb" (ignore whitespace diff) shows that this
commit just removes a "if (b != -1)" after a "gcc_assert (b
!= -1)" and also removes the subsequent "else" clause.

* resource.cc (mark_target_live_regs): Remove redundant check for b
being -1, after gcc_assert.

Diff:
---
 gcc/resource.cc | 270 +++-
 1 file changed, 132 insertions(+), 138 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 0d8cde93570..62bd46f786e 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -704,156 +704,150 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
 
-  /* If we found a basic block, get the live registers from it and update
- them with anything set or killed between its start and the insn before
- TARGET; this custom life analysis is really about registers so we need
- to use the LR problem.  Otherwise, we must assume everything is live.  */
-  if (b != -1)
+  /* Get the live registers from the basic block and update them with
+ anything set or killed between its start and the insn before
+ TARGET; this custom life analysis is really about registers so we
+ need to use the LR problem.  Otherwise, we must assume everything
+ is live.  */
+  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
+  rtx_insn *start_insn, *stop_insn;
+  df_ref def;
+
+  /* Compute hard regs live at start of block.  */
+  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
+  FOR_EACH_ARTIFICIAL_DEF (def, b)
+if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
+  SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
+
+  /* Get starting and ending insn, handling the case where each might
+ be a SEQUENCE.  */
+  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
+   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
+  stop_insn = target;
+
+  if (NONJUMP_INSN_P (start_insn)
+  && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
+start_insn = as_a  (PATTERN (start_insn))->insn (0);
+
+  if (NONJUMP_INSN_P (stop_insn)
+  && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
+stop_insn = next_insn (PREV_INSN (stop_insn));
+
+  for (insn = start_insn; insn != stop_insn;
+   insn = next_insn_no_annul (insn))
 {
-  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
-  rtx_insn *start_insn, *stop_insn;
-  df_ref def;
-
-  /* Compute hard regs live at start of block.  */
-  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
-  FOR_EACH_ARTIFICIAL_DEF (def, b)
-   if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
- SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
-
-  /* Get starting and ending insn, handling the case where each might
-be a SEQUENCE.  */
-  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
-   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
-  stop_insn = target;
-
-  if (NONJUMP_INSN_P (start_insn)
- && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
-   start_insn = as_a  (PATTERN (start_insn))->insn (0);
-
-  if (NONJUMP_INSN_P (stop_insn)
- && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
-   stop_insn = next_insn (PREV_INSN (stop_insn));
-
-  for (insn = start_insn; insn != stop_insn;
-  insn = next_insn_no_annul (insn))
+  rtx link;
+  rtx_insn *real_insn = insn;
+  enum rtx_code code = GET_CODE (insn);
+
+  if (DEBUG_INSN_P (insn))
+   continue;
+
+  /* If this insn is from the target of a branch, it isn't going to
+be used in the sequel.  If it is used in both cases, this
+test will not be true.  */
+  if ((code == INSN || code == JUMP_INSN || code == CALL_INSN)
+ && INSN_FROM_TARGET_P (insn))
+   continue;
+
+  /* If this insn is a USE made by update_block, we care about the
+underlying insn.  */
+  if (code == INSN
+ && GET_CODE (PATTERN (insn)) == USE
+ && INSN_P (XEXP (PATTERN (insn), 0)))
+   real_insn = as_a  (XEXP (PATTERN (insn), 0));
+
+  if (CALL_P (real_insn))
{
- rtx link;
- rtx_insn *real_insn = insn;
- enum rtx_code code = GET_CODE (insn);
-
- if (DEBUG_INSN_P (insn))
-   continue;
-
- /* If this insn is from the target of a branch, it isn't going to
-be used in the sequel.  If it is used in both cases, this
-test will n

[gcc r15-878] resource.cc: Replace calls to find_basic_block with cfgrtl BLOCK_FOR_INSN

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:933ab59c59bdc1ac9e3ca3a56527836564e1821b

commit r15-878-g933ab59c59bdc1ac9e3ca3a56527836564e1821b
Author: Hans-Peter Nilsson 
Date:   Tue May 28 23:16:48 2024 +0200

resource.cc: Replace calls to find_basic_block with cfgrtl BLOCK_FOR_INSN

...and call compute_bb_for_insn in init_resource_info and
free_bb_for_insn in free_resource_info.

I put a gcc_unreachable in that else-clause for a failing
find_basic_block in mark_target_live_regs after the comment that says:

/* We didn't find the start of a basic block.  Assume everything
   in use.  This should happen only extremely rarely.  */
SET_HARD_REG_SET (res->regs);

and found that it fails not extremely rarely but extremely early in
the build (compiling libgcc).

That kind of pessimization leads to suboptimal delay-slot-filling.
Instead, do like many machine_dependent_reorg passes and call
compute_bb_for_insn as part of resource.cc initialization.

After this patch, there's a whole "if (b != -1)" conditional that's
dominated by a gcc_assert (b != -1).  I separated that, as it's a NFC
whitespace patch that hampers patch readability.

Altogether this improved coremark performance for CRIS at -O2
-march=v10 by 0.36%.

* resource.cc: Include cfgrtl.h.  Use BLOCK_FOR_INSN (insn)->index
instead of calling find_basic_block (insn).  Assert for not -1.
(find_basic_block): Remove function.
(init_resource_info): Call compute_bb_for_insn.
(free_resource_info): Call free_bb_for_insn.

Diff:
---
 gcc/resource.cc | 66 +
 1 file changed, 10 insertions(+), 56 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 06fcfd3e44c..0d8cde93570 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm_p.h"
 #include "regs.h"
 #include "emit-rtl.h"
+#include "cfgrtl.h"
 #include "resource.h"
 #include "insn-attr.h"
 #include "function-abi.h"
@@ -75,7 +76,6 @@ static HARD_REG_SET current_live_regs;
 static HARD_REG_SET pending_dead_regs;
 
 static void update_live_status (rtx, const_rtx, void *);
-static int find_basic_block (rtx_insn *, int);
 static rtx_insn *next_insn_no_annul (rtx_insn *);
 
 /* Utility function called from mark_target_live_regs via note_stores.
@@ -113,46 +113,6 @@ update_live_status (rtx dest, const_rtx x, void *data 
ATTRIBUTE_UNUSED)
CLEAR_HARD_REG_BIT (pending_dead_regs, i);
   }
 }
-
-/* Find the number of the basic block with correct live register
-   information that starts closest to INSN.  Return -1 if we couldn't
-   find such a basic block or the beginning is more than
-   SEARCH_LIMIT instructions before INSN.  Use SEARCH_LIMIT = -1 for
-   an unlimited search.
-
-   The delay slot filling code destroys the control-flow graph so,
-   instead of finding the basic block containing INSN, we search
-   backwards toward a BARRIER where the live register information is
-   correct.  */
-
-static int
-find_basic_block (rtx_insn *insn, int search_limit)
-{
-  /* Scan backwards to the previous BARRIER.  Then see if we can find a
- label that starts a basic block.  Return the basic block number.  */
-  for (insn = prev_nonnote_insn (insn);
-   insn && !BARRIER_P (insn) && search_limit != 0;
-   insn = prev_nonnote_insn (insn), --search_limit)
-;
-
-  /* The closest BARRIER is too far away.  */
-  if (search_limit == 0)
-return -1;
-
-  /* The start of the function.  */
-  else if (insn == 0)
-return ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index;
-
-  /* See if any of the upcoming CODE_LABELs start a basic block.  If we reach
- anything other than a CODE_LABEL or note, we can't find this code.  */
-  for (insn = next_nonnote_insn (insn);
-   insn && LABEL_P (insn);
-   insn = next_nonnote_insn (insn))
-if (BLOCK_FOR_INSN (insn))
-  return BLOCK_FOR_INSN (insn)->index;
-
-  return -1;
-}
 
 /* Similar to next_insn, but ignores insns in the delay slots of
an annulled branch.  */
@@ -714,7 +674,8 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 }
 
   if (b == -1)
-b = find_basic_block (target, param_max_delay_slot_live_search);
+b = BLOCK_FOR_INSN (target)->index;
+  gcc_assert (b != -1);
 
   if (target_hash_table != NULL)
 {
@@ -722,7 +683,7 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
{
  /* If the information is up-to-date, use it.  Otherwise, we will
 update it below.  */
- if (b == tinfo->block && b != -1 && tinfo->bb_tick == bb_ticks[b])
+ if (b == tinfo->block && 

[gcc r15-877] resource.cc (mark_target_live_regs): Don't look past target insn, PR115182

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:84b4ed45ea81ed5c4fb656a17846b26071c23e7d

commit r15-877-g84b4ed45ea81ed5c4fb656a17846b26071c23e7d
Author: Hans-Peter Nilsson 
Date:   Tue May 28 23:15:57 2024 +0200

resource.cc (mark_target_live_regs): Don't look past target insn, PR115182

The PR115182 regression is that a delay-slot for a conditional branch,
is no longer filled with an insn that has been "sunk" because of
r15-518-g99b1daae18c095, for cris-elf w. -O2 -march=v10.

There are still sufficient "nearby" dependency-less insns that the
delay-slot shouldn't be empty.  In particular there's one candidate in
the loop, right after an off-ramp branch, off the loop: a move from
$r9 to $r3.

beq .L2
nop
move.d $r9,$r3

But, the resource.cc data-flow-analysis incorrectly says it collides
with registers "live" at that .L2 off-ramp.  The off-ramp insns
(inlined from simple_rand) look like this (left-to-right direction):

.L2:
move.d $r12,[_seed.0]
move.d $r13,[_seed.0+4]
ret
movem [$sp+],$r8

So, a store of a long long to _seed, a return instruction and a
restoring multi-register-load of r0..r8 (all callee-saved registers)
in the delay-slot of the return insn.  The return-value is kept in
$r10,$r11 so in total $r10..$r13 live plus the stack-pointer and
return-address registers.  But, mark_target_live_regs says that
$r0..$r8 are also live because it *includes the registers live for the
return instruction*!  While they "come alive" after the movem, they
certainly aren't live at the "off-ramp" .L2 label.

The problem is in mark_target_live_regs: it consults a hash-table
indexed by insn uid, where it tracks the currently live registers with
a "generation" count to handle when it moves around insn, filling
delay-slots.  As a fall-back, it starts with registers live at the
start of each basic block, calculated by the comparatively modern df
machinery (except that it can fail finding out which basic block an
insn belongs to, at which times it includes all registers film at 11),
and tracks the semantics of insns up to each insn.

You'd think that's all that should be done, but then for some reason
it *also* looks at insns *after the target insn* up to a few branches,
and includes that in the set of live registers!  This is the code in
mark_target_live_regs that starts with the call to
find_dead_or_set_registers.  I couldn't make sense of it, so I looked
at its history, and I think I found the cause; it's a thinko or
possibly two thinkos.  The original implementation, gcc-git-described
as r0-97-g9c7e297806a27f, later moved from reorg.c to resource.c in
r0-20470-gca545bb569b756.

I believe the "extra" lookup was intended to counter flaws in the
reorg.c/resource.c register liveness analysis; to inspect insns along
the execution paths to exclude registers that, when looking at
subsequent insns, weren't live.  That guess is backed by a sentence in
the updated (i.e. deleted) part of the function head comment for
mark_target_live_regs: "Next, scan forward from TARGET looking for
things set or clobbered before they are used.  These are not live."
To me that sounds like flawed register-liveness data.

An epilogue expanded as RTX (i.e. not just assembly code emitted as
text) is introduced in basepoints/gcc-0-1334-gbdac5f5848fb, so before
that time, nobody would notice that saved registers were included as
live registers in delay-slots in "next-to-last" basic blocks.

Then in r0-24783-g96e9c98d59cc40, the intersection ("and") was changed
to a union ("or"), i.e. it added to the set of live registers instead
of thinning it out.  In the gcc-patches archives, I see the patch
submission doesn't offer a C test-case and only has RTX snippets
(apparently for SPARC).  The message does admit that the change goes
"against what the comments in the code say":
https://gcc.gnu.org/pipermail/gcc-patches/1999-November/021836.html
It looks like this was related to a bug with register liveness info
messed up when moving a "delay-slotted" insn from one slot to another.
But, I can't help but thinking it's just papering over a register
liveness bug elsewhere.

I think, with a reliable "DF_LR_IN", the whole thing *after* tracking
from start-of-bb up to the target insn should be removed; thus.

This patch also removes the now-unused find_dead_or_set_registers
function.

At r15-518, it fixes the issue for CRIS and improves coremark scores
at -O2 -march=v10 a tiny bit (about 0.05%).

PR rtl-optimiza

Re: [PATCH 2/4] resource.cc: Replace calls to find_basic_block with cfgrtl BLOCK_FOR_INSN

[Hans-Peter Nilsson]

> Date: Mon, 27 May 2024 12:57:53 -0600
> From: Jeff Law 

> > * resource.cc: Include cfgrtl.h.  Use BLOCK_FOR_INSN (insn)->index
> > instead of calling find_basic_block (insn).  Assert for not -1.
> > (find_basic_block): Remove function.
> > (init_resource_info): Call compute_bb_for_insn.
> > (free_resource_info): Call free_bb_for_insn.
> I'm pretty sure that code as part of the overall problem -- namely that 
> we didn't have good basic block info so we resorted to insn scanning.
> 
> Presumably we set BLOCK_FOR_INSN when we generate a wrapper SEQUENCE 
> insns for a filled delay slot?

Yes - one way or the other: most insn chain changes from
reorg are through calls to add_insn_after, which always sets
the bb of the added insn according to the reference insn
(except when either insn is a barrier, then it never sets a
bb); see for example emit_delay_sequence.  Others by
emit_insn_before and emit_copy_of_insn_after.

(Not-so-)fun fact: add_insn_after takes a bb parameter which
reorg.cc always passes as NULL.  But - the argument is
*always ignored* and the bb in the "after" insn is used.
I traced that ignored parameter as far as
r0-81421-g6fb5fa3cbc0d78 "Merge dataflow branch into
mainline" when is was added.  I *guess* it's an artifact
left over from some idea explored on that branch.  Ripe for
obvious cleanup by removal everywhere.

>  Assuming we do create the right mapping 
> for those new insns, then this is OK.

Thanks for the quick review of the whole set!

brgds, H-P

[PATCH 4/4] resource.cc: Remove redundant conditionals

[Hans-Peter Nilsson]

Regtested cris-elf.  Ok to commit?

-- >8 --
No functional change.

- We always have a target_hash_table and bb_ticks because
init_resource_info is always called.  These conditionals are
an ancient artifact: it's been quite a while since
resource.cc was used elsewhere than exclusively from reorg.cc

- In mark_target_live_regs, get rid of a now-redundant "if
(tinfo != NULL)" conditional and replace an "if (bb)" with a
gcc_assert.

A "git diff -wb" (ignore whitespace diff) is better at
showing the actual changes.

* resource.cc (free_resource_info, clear_hashed_info_for_insn): Don't
check for non-null target_hash_table and bb_ticks.
(mark_target_live_regs): Ditto.  Replace check for non-NULL result from
BLOCK_FOR_INSN with a call to gcc_assert.  Fold code conditioned on
tinfo != NULL.
---
 gcc/resource.cc | 123 
 1 file changed, 52 insertions(+), 71 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 62bd46f786eb..7c1de8864327 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -658,49 +658,42 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
   res->cc = 0;
 
   /* See if we have computed this value already.  */
-  if (target_hash_table != NULL)
-{
-  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
-  tinfo; tinfo = tinfo->next)
-   if (tinfo->uid == INSN_UID (target))
- break;
-
-  /* Start by getting the basic block number.  If we have saved
-information, we can get it from there unless the insn at the
-start of the basic block has been deleted.  */
-  if (tinfo && tinfo->block != -1
- && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
-   b = tinfo->block;
-}
+  for (tinfo = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+   tinfo; tinfo = tinfo->next)
+if (tinfo->uid == INSN_UID (target))
+  break;
+
+  /* Start by getting the basic block number.  If we have saved
+ information, we can get it from there unless the insn at the
+ start of the basic block has been deleted.  */
+  if (tinfo && tinfo->block != -1
+  && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
+b = tinfo->block;
 
   if (b == -1)
 b = BLOCK_FOR_INSN (target)->index;
   gcc_assert (b != -1);
 
-  if (target_hash_table != NULL)
+  if (tinfo)
 {
-  if (tinfo)
+  /* If the information is up-to-date, use it.  Otherwise, we will
+update it below.  */
+  if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
{
- /* If the information is up-to-date, use it.  Otherwise, we will
-update it below.  */
- if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
-   {
- res->regs = tinfo->live_regs;
- return;
-   }
-   }
-  else
-   {
- /* Allocate a place to put our results and chain it into the
-hash table.  */
- tinfo = XNEW (struct target_info);
- tinfo->uid = INSN_UID (target);
- tinfo->block = b;
- tinfo->next
-   = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
- target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
+ res->regs = tinfo->live_regs;
+ return;
}
 }
+  else
+{
+  /* Allocate a place to put our results and chain it into the hash
+table.  */
+  tinfo = XNEW (struct target_info);
+  tinfo->uid = INSN_UID (target);
+  tinfo->block = b;
+  tinfo->next = target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME];
+  target_hash_table[INSN_UID (target) % TARGET_HASH_PRIME] = tinfo;
+}
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
 
@@ -825,13 +818,12 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 to be live here still are.  The fallthrough edge may have
 left a live register uninitialized.  */
  bb = BLOCK_FOR_INSN (real_insn);
- if (bb)
-   {
- HARD_REG_SET extra_live;
+ gcc_assert (bb);
 
- REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
- current_live_regs |= extra_live;
-   }
+ HARD_REG_SET extra_live;
+
+ REG_SET_TO_HARD_REG_SET (extra_live, DF_LR_IN (bb));
+ current_live_regs |= extra_live;
}
 
   /* The beginning of the epilogue corresponds to the end of the
@@ -847,10 +839,8 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 {
   tinfo->block = b;
   tinfo->bb_tick = bb_ticks[b];
+  tinfo->live_regs = res->regs;
 }
-
-  if (tinfo != NULL)
-tinfo->live_regs = res->regs;
 }
 
 /* Initialize the resources required by mark_target_live_regs ().
@@ -939,31 +929,25 @@ init_resource_info (rtx_insn *epilogue_insn)
 void
 

[PATCH 3/4] resource.cc (mark_target_live_regs): Remove check for bb not found

[Hans-Peter Nilsson]

Regtested cris-elf.  Ok to commit?

-- >8 --
No functional change.

A "git diff -wb" (ignore whitespace diff) shows that this
commit just removes a "if (b != -1)" after a "gcc_assert (b
!= -1)" and also removes the subsequent "else" clause.

* resource.cc (mark_target_live_regs): Remove redundant check for b
being -1, after gcc_assert.
---
 gcc/resource.cc | 270 +++-
 1 file changed, 132 insertions(+), 138 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 0d8cde93570e..62bd46f786eb 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -704,156 +704,150 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 
   CLEAR_HARD_REG_SET (pending_dead_regs);
 
-  /* If we found a basic block, get the live registers from it and update
- them with anything set or killed between its start and the insn before
- TARGET; this custom life analysis is really about registers so we need
- to use the LR problem.  Otherwise, we must assume everything is live.  */
-  if (b != -1)
+  /* Get the live registers from the basic block and update them with
+ anything set or killed between its start and the insn before
+ TARGET; this custom life analysis is really about registers so we
+ need to use the LR problem.  Otherwise, we must assume everything
+ is live.  */
+  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
+  rtx_insn *start_insn, *stop_insn;
+  df_ref def;
+
+  /* Compute hard regs live at start of block.  */
+  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
+  FOR_EACH_ARTIFICIAL_DEF (def, b)
+if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
+  SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
+
+  /* Get starting and ending insn, handling the case where each might
+ be a SEQUENCE.  */
+  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
+   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
+  stop_insn = target;
+
+  if (NONJUMP_INSN_P (start_insn)
+  && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
+start_insn = as_a  (PATTERN (start_insn))->insn (0);
+
+  if (NONJUMP_INSN_P (stop_insn)
+  && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
+stop_insn = next_insn (PREV_INSN (stop_insn));
+
+  for (insn = start_insn; insn != stop_insn;
+   insn = next_insn_no_annul (insn))
 {
-  regset regs_live = DF_LR_IN (BASIC_BLOCK_FOR_FN (cfun, b));
-  rtx_insn *start_insn, *stop_insn;
-  df_ref def;
-
-  /* Compute hard regs live at start of block.  */
-  REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
-  FOR_EACH_ARTIFICIAL_DEF (def, b)
-   if (DF_REF_FLAGS (def) & DF_REF_AT_TOP)
- SET_HARD_REG_BIT (current_live_regs, DF_REF_REGNO (def));
-
-  /* Get starting and ending insn, handling the case where each might
-be a SEQUENCE.  */
-  start_insn = (b == ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index ?
-   insns : BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, b)));
-  stop_insn = target;
-
-  if (NONJUMP_INSN_P (start_insn)
- && GET_CODE (PATTERN (start_insn)) == SEQUENCE)
-   start_insn = as_a  (PATTERN (start_insn))->insn (0);
-
-  if (NONJUMP_INSN_P (stop_insn)
- && GET_CODE (PATTERN (stop_insn)) == SEQUENCE)
-   stop_insn = next_insn (PREV_INSN (stop_insn));
-
-  for (insn = start_insn; insn != stop_insn;
-  insn = next_insn_no_annul (insn))
+  rtx link;
+  rtx_insn *real_insn = insn;
+  enum rtx_code code = GET_CODE (insn);
+
+  if (DEBUG_INSN_P (insn))
+   continue;
+
+  /* If this insn is from the target of a branch, it isn't going to
+be used in the sequel.  If it is used in both cases, this
+test will not be true.  */
+  if ((code == INSN || code == JUMP_INSN || code == CALL_INSN)
+ && INSN_FROM_TARGET_P (insn))
+   continue;
+
+  /* If this insn is a USE made by update_block, we care about the
+underlying insn.  */
+  if (code == INSN
+ && GET_CODE (PATTERN (insn)) == USE
+ && INSN_P (XEXP (PATTERN (insn), 0)))
+   real_insn = as_a  (XEXP (PATTERN (insn), 0));
+
+  if (CALL_P (real_insn))
{
- rtx link;
- rtx_insn *real_insn = insn;
- enum rtx_code code = GET_CODE (insn);
-
- if (DEBUG_INSN_P (insn))
-   continue;
-
- /* If this insn is from the target of a branch, it isn't going to
-be used in the sequel.  If it is used in both cases, this
-test will not be true.  */
- if ((code == INSN || code == JUMP_INSN || code == CALL_INSN)
- && INSN_FROM_TARGET_P (insn))
-   continue;
-
- /* If this insn is a USE made by update_block, we care about the
-underlying insn.  */
- if (code == INSN
- && GET_CODE (PATTERN (insn)) == USE
- && INSN_P (XEXP (PATTERN (insn), 

[PATCH 2/4] resource.cc: Replace calls to find_basic_block with cfgrtl BLOCK_FOR_INSN

[Hans-Peter Nilsson]

Regtested cris-elf.  Ok to commit?

-- >8 --
...and call compute_bb_for_insn in init_resource_info and
free_bb_for_insn in free_resource_info.

I put a gcc_unreachable in that else-clause for a failing
find_basic_block in mark_target_live_regs after the comment that says:

/* We didn't find the start of a basic block.  Assume everything
   in use.  This should happen only extremely rarely.  */
SET_HARD_REG_SET (res->regs);

and found that it fails not extremely rarely but extremely early in
the build (compiling libgcc).

That kind of pessimization leads to suboptimal delay-slot-filling.
Instead, do like many machine_dependent_reorg passes and call
compute_bb_for_insn as part of resource.cc initialization.

After this patch, there's a whole "if (b != -1)" conditional that's
dominated by a gcc_assert (b != -1).  I separated that, as it's a NFC
whitespace patch that hampers patch readability.

Altogether this improved coremark performance for CRIS at -O2
-march=v10 by 0.36%.

* resource.cc: Include cfgrtl.h.  Use BLOCK_FOR_INSN (insn)->index
instead of calling find_basic_block (insn).  Assert for not -1.
(find_basic_block): Remove function.
(init_resource_info): Call compute_bb_for_insn.
(free_resource_info): Call free_bb_for_insn.
---
 gcc/resource.cc | 66 -
 1 file changed, 10 insertions(+), 56 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index 06fcfd3e44c7..0d8cde93570e 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -28,6 +28,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tm_p.h"
 #include "regs.h"
 #include "emit-rtl.h"
+#include "cfgrtl.h"
 #include "resource.h"
 #include "insn-attr.h"
 #include "function-abi.h"
@@ -75,7 +76,6 @@ static HARD_REG_SET current_live_regs;
 static HARD_REG_SET pending_dead_regs;
 
 static void update_live_status (rtx, const_rtx, void *);
-static int find_basic_block (rtx_insn *, int);
 static rtx_insn *next_insn_no_annul (rtx_insn *);
 
 /* Utility function called from mark_target_live_regs via note_stores.
@@ -113,46 +113,6 @@ update_live_status (rtx dest, const_rtx x, void *data 
ATTRIBUTE_UNUSED)
CLEAR_HARD_REG_BIT (pending_dead_regs, i);
   }
 }
-
-/* Find the number of the basic block with correct live register
-   information that starts closest to INSN.  Return -1 if we couldn't
-   find such a basic block or the beginning is more than
-   SEARCH_LIMIT instructions before INSN.  Use SEARCH_LIMIT = -1 for
-   an unlimited search.
-
-   The delay slot filling code destroys the control-flow graph so,
-   instead of finding the basic block containing INSN, we search
-   backwards toward a BARRIER where the live register information is
-   correct.  */
-
-static int
-find_basic_block (rtx_insn *insn, int search_limit)
-{
-  /* Scan backwards to the previous BARRIER.  Then see if we can find a
- label that starts a basic block.  Return the basic block number.  */
-  for (insn = prev_nonnote_insn (insn);
-   insn && !BARRIER_P (insn) && search_limit != 0;
-   insn = prev_nonnote_insn (insn), --search_limit)
-;
-
-  /* The closest BARRIER is too far away.  */
-  if (search_limit == 0)
-return -1;
-
-  /* The start of the function.  */
-  else if (insn == 0)
-return ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->index;
-
-  /* See if any of the upcoming CODE_LABELs start a basic block.  If we reach
- anything other than a CODE_LABEL or note, we can't find this code.  */
-  for (insn = next_nonnote_insn (insn);
-   insn && LABEL_P (insn);
-   insn = next_nonnote_insn (insn))
-if (BLOCK_FOR_INSN (insn))
-  return BLOCK_FOR_INSN (insn)->index;
-
-  return -1;
-}
 
 /* Similar to next_insn, but ignores insns in the delay slots of
an annulled branch.  */
@@ -714,7 +674,8 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
 }
 
   if (b == -1)
-b = find_basic_block (target, param_max_delay_slot_live_search);
+b = BLOCK_FOR_INSN (target)->index;
+  gcc_assert (b != -1);
 
   if (target_hash_table != NULL)
 {
@@ -722,7 +683,7 @@ mark_target_live_regs (rtx_insn *insns, rtx 
target_maybe_return, struct resource
{
  /* If the information is up-to-date, use it.  Otherwise, we will
 update it below.  */
- if (b == tinfo->block && b != -1 && tinfo->bb_tick == bb_ticks[b])
+ if (b == tinfo->block && tinfo->bb_tick == bb_ticks[b])
{
  res->regs = tinfo->live_regs;
  return;
@@ -905,7 +866,6 @@ void
 init_resource_info (rtx_insn *epilogue_insn)
 {
   int i;
-  basic_block bb;
 
   /* Indicate what resources are required to be valid at the end of the current
  function.  The condition code never is and memory always is.
@@ -975,10 +935,8 @@ init_resource_info (rtx_insn *epilogue_insn)
   target_hash_table = XCNEWVEC (struct target_info *, TARGET_HASH_PRIME);
   

[PATCH 1/4] resource.cc (mark_target_live_regs): Don't look past target insn, PR115182

[Hans-Peter Nilsson]

Regtested cris-elf.  Ok to commit?

-- >8 --
The PR115182 regression is that a delay-slot for a conditional branch,
is no longer filled with an insn that has been "sunk" because of
r15-518-g99b1daae18c095, for cris-elf w. -O2 -march=v10.

There are still sufficient "nearby" dependency-less insns that the
delay-slot shouldn't be empty.  In particular there's one candidate in
the loop, right after an off-ramp branch, off the loop: a move from
$r9 to $r3.

beq .L2
nop
move.d $r9,$r3

But, the resource.cc data-flow-analysis incorrectly says it collides
with registers "live" at that .L2 off-ramp.  The off-ramp insns
(inlined from simple_rand) look like this (left-to-right direction):

.L2:
move.d $r12,[_seed.0]
move.d $r13,[_seed.0+4]
ret
movem [$sp+],$r8

So, a store of a long long to _seed, a return instruction and a
restoring multi-register-load of r0..r8 (all callee-saved registers)
in the delay-slot of the return insn.  The return-value is kept in
$r10,$r11 so in total $r10..$r13 live plus the stack-pointer and
return-address registers.  But, mark_target_live_regs says that
$r0..$r8 are also live because it *includes the registers live for the
return instruction*!  While they "come alive" after the movem, they
certainly aren't live at the "off-ramp" .L2 label.

The problem is in mark_target_live_regs: it consults a hash-table
indexed by insn uid, where it tracks the currently live registers with
a "generation" count to handle when it moves around insn, filling
delay-slots.  As a fall-back, it starts with registers live at the
start of each basic block, calculated by the comparatively modern df
machinery (except that it can fail finding out which basic block an
insn belongs to, at which times it includes all registers film at 11),
and tracks the semantics of insns up to each insn.

You'd think that's all that should be done, but then for some reason
it *also* looks at insns *after the target insn* up to a few branches,
and includes that in the set of live registers!  This is the code in
mark_target_live_regs that starts with the call to
find_dead_or_set_registers.  I couldn't make sense of it, so I looked
at its history, and I think I found the cause; it's a thinko or
possibly two thinkos.  The original implementation, gcc-git-described
as r0-97-g9c7e297806a27f, later moved from reorg.c to resource.c in
r0-20470-gca545bb569b756.

I believe the "extra" lookup was intended to counter flaws in the
reorg.c/resource.c register liveness analysis; to inspect insns along
the execution paths to exclude registers that, when looking at
subsequent insns, weren't live.  That guess is backed by a sentence in
the updated (i.e. deleted) part of the function head comment for
mark_target_live_regs: "Next, scan forward from TARGET looking for
things set or clobbered before they are used.  These are not live."
To me that sounds like flawed register-liveness data.

An epilogue expanded as RTX (i.e. not just assembly code emitted as
text) is introduced in basepoints/gcc-0-1334-gbdac5f5848fb, so before
that time, nobody would notice that saved registers were included as
live registers in delay-slots in "next-to-last" basic blocks.

Then in r0-24783-g96e9c98d59cc40, the intersection ("and") was changed
to a union ("or"), i.e. it added to the set of live registers instead
of thinning it out.  In the gcc-patches archives, I see the patch
submission doesn't offer a C test-case and only has RTX snippets
(apparently for SPARC).  The message does admit that the change goes
"against what the comments in the code say":
https://gcc.gnu.org/pipermail/gcc-patches/1999-November/021836.html
It looks like this was related to a bug with register liveness info
messed up when moving a "delay-slotted" insn from one slot to another.
But, I can't help but thinking it's just papering over a register
liveness bug elsewhere.

I think, with a reliable "DF_LR_IN", the whole thing *after* tracking
from start-of-bb up to the target insn should be removed; thus.

This patch also removes the now-unused find_dead_or_set_registers
function.

At r15-518, it fixes the issue for CRIS and improves coremark scores
at -O2 -march=v10 a tiny bit (about 0.05%).

PR rtl-optimization/115182
* resource.cc (mark_target_live_regs): Don't look for
unconditional branches after the target to improve on the
register liveness.
(find_dead_or_set_registers): Remove unused function.
---
 gcc/resource.cc | 235 
 1 file changed, 235 deletions(-)

diff --git a/gcc/resource.cc b/gcc/resource.cc
index bb196fb06db6..06fcfd3e44c7 100644
--- a/gcc/resource.cc
+++ b/gcc/resource.cc
@@ -77,9 +77,6 @@ static HARD_REG_SET pending_dead_regs;
 static void update_live_status (rtx, const_rtx, void *);
 static int find_basic_block (rtx_insn *, int);
 static rtx_insn *next_insn_no_annul (rtx_insn *);
-static rtx_insn *find_dead_or_set_registers (rtx_insn *, 

Re: [PATCH V3] report message for operator %a on unaddressible operand

[Hans-Peter Nilsson]

On Mon, 20 May 2024, Jiufu Guo wrote:

> Hi,
> 
> For PR96866, when printing asm code for modifier "%a", an addressable
> operand is required.  While the constraint "X" allow any kind of
> operand even which is hard to get the address directly. e.g. extern
> symbol whose address is in TOC.
> An error message would be reported to indicate the invalid asm operand.
> 
> Compare with previous version, code comments and message are updated.
> 
> Bootstrap pass on ppc64{,le}.
> Is this ok for trunk?
> 
> BR,
> Jeff(Jiufu Guo)
> 
>   PR target/96866
> 
> gcc/ChangeLog:
> 
>   * config/rs6000/rs6000.cc (print_operand_address):
> 
> gcc/testsuite/ChangeLog:
> 
>   * gcc.target/powerpc/pr96866-1.c: New test.
>   * gcc.target/powerpc/pr96866-2.c: New test.

The gcc/ChangeLog entry needs some text after that ":".

brgds, H-P

Re: [PATCH] tree-optimization/114589 - remove profile based sink heuristics

[Hans-Peter Nilsson]

> Date: Wed, 15 May 2024 11:38:58 +0200 (CEST)
> From: Richard Biener 

> The following removes the profile based heuristic limiting sinking
> and instead uses post-dominators to avoid sinking to places that
> are executed under the same conditions as the earlier location which
> the profile based heuristic should have guaranteed as well.
> 
> To avoid regressing this moves the empty-latch check to cover all
> sink cases.
> 
> It also stream-lines the resulting select_best_block a bit but avoids
> adjusting heuristics more with this change.  gfortran.dg/streamio_9.f90
> starts execute failing with this on x86_64 with -m32 because the
> (float)i * 9....e-7 compute is sunk across a STOP causing it
> to be no longer spilled and thus the compare failing due to excess
> precision.  The patch adds -ffloat-store to avoid this, following
> other similar testcases.
> 
> This change doesn't fix the testcase in the PR on itself.

It may come as no surprise that this patch (commit
r15-518-g99b1daae18c095) caused a regression for some codes,
for some targets.

I entered PR115144 for the one that came to my attention.
TL;DR: not sure what to do about it, if anything; it
corresponds to the random_bitstring function in
gcc.c-torture/execute/arith-rand-ll.c compiled for cris-elf
with -O2 -march=v10 but there's no regression for coremark.
The random_bitstring code does intense operations on "long
long", i.e. lots of int64_t two-register operations and
arithmetic library calls.

I'd be grateful if you had a quick glance at
random_bitstring in gcc.c-torture/execute/arith-rand-ll.c in
case that code rings a bell related to the r15-518 change;
maybe there's a trivial improvement you see.

brgds, H-P

[COMMITTED] Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement"" combine improvement

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 11 Apr 2024 01:16:32 +0200

I committed this revert of a revert, as r15-311, as the
prerequisite was also revert-reverted, in r15-268.

-- >8 --
This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7.
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 912069c018d5..2ef6471a990b 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,19 +1,20 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
-/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
-/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
+/* { dg-final { scan-assembler-not "\tnot" } } */
+/* { dg-final { scan-assembler-not "\tlsr" } } */
+/* We should get just one move, storing the result into *d.  */
+/* { dg-final { scan-assembler-times "\tmove" 1 } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* Whoops!  We get a cmp.d with the original operands here. */
+  /* We used to get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* Whoops!  While we don't get a test.d for the result here for cc0,
- we get a sequence of insns: a move, a "not" and a shift of the
- subtraction-result, where a simple "spl" would have done. */
+  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
+ (a.k.a "spl") re-using flags from the subtraction. */
   return c >= 0;
 }
-- 
2.30.2

brgds, H-P

[gcc r15-311] Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement""

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:f6ce85502eb2e4e7bbd9b3c6c1c065a004f8f531

commit r15-311-gf6ce85502eb2e4e7bbd9b3c6c1c065a004f8f531
Author: Hans-Peter Nilsson 
Date:   Wed May 8 04:11:20 2024 +0200

Revert "Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from 
combine improvement""

This reverts commit 39f81924d88e3cc197fc3df74204c9b5e01e12f7.

Diff:
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 912069c018d5..2ef6471a990b 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,19 +1,20 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
-/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
-/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
+/* { dg-final { scan-assembler-not "\tnot" } } */
+/* { dg-final { scan-assembler-not "\tlsr" } } */
+/* We should get just one move, storing the result into *d.  */
+/* { dg-final { scan-assembler-times "\tmove" 1 } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* Whoops!  We get a cmp.d with the original operands here. */
+  /* We used to get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* Whoops!  While we don't get a test.d for the result here for cc0,
- we get a sequence of insns: a move, a "not" and a shift of the
- subtraction-result, where a simple "spl" would have done. */
+  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
+ (a.k.a "spl") re-using flags from the subtraction. */
   return c >= 0;
 }

Re: [PATCH v4 2/3] [RFC] ifcvt: Allow more operations in multiple set if conversion

[Hans-Peter Nilsson]

On Tue, 23 Apr 2024, Manolis Tsamis wrote:
> diff --git a/gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_sets_arithm.c 
> b/gcc/testsuite/gcc.target/aarch64/ifcvt_multiple_sets_arithm.c
...
> +/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through 
> noce_convert_multiple_sets" 6 "ce1" } } */
> \ No newline at end of file

This and the other new test-case both miss a newline at the end 
of the file.

brgds, H-P

Re: enable sqrt insns for cdce3.c

[Hans-Peter Nilsson]

On Mon, 22 Apr 2024, Alexandre Oliva wrote:
> [Revamped version of this patch, combined with others, to follow]
> 
> On Mar 10, 2021, Hans-Peter Nilsson  wrote:

Time flies...

> > On Wed, 10 Mar 2021, Alexandre Oliva wrote:

> Is mmix a sqrt_insn effective target?  proc
> check_effective_target_sqrt_insn in
> gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm
> surprised it would still try to run the test despite the added
> /* { dg-require-effective-target sqrt_insn } */ directive.

The effective-target sqrt_insn predicate says "supports hardware 
square root instructions" and doesn't make a difference between 
sqrtdf2 (double) and sqrtsf3 (float).  I'm extrapolating that 
the "divine meaning" of the comment is that such an instruction 
must be present for all supported floating-point modes for the 
predicate to yield true (when the predicate is correctly 
implemented).

(We could also fix the predicate description to actually say 
"for all floating-point modes" and/or split the predicate into 
mode-specific variants, etc. ;-)

MMIX has sqrtdf2 but not sqrtsf2, and the latter is what's used 
in cdce3.c.

> cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like
> (x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root
> instruction.

...for 32-bit single floats.

> Since we don't know why it still runs for you, I'm keeping the mmix
> explicit skip in the new version of the patch.

Thanks, that does seem like TRT.

brgds, H-P

[REVERTED] testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement

[Hans-Peter Nilsson]

> Date: Tue, 9 Apr 2024 15:18:10 -0500
> From: Segher Boessenkool 

> All (target-specific) new testsuite failures are just like that: bad
> testcases!

With a touch of bad assumptions by port-specific code, no
doubt.  Maybe also rtx costs including my pet peeve, the
default implementation of insn_costs (the one that doesn't
look at the destination of setters and which when you try
fixing it, pulls you down a rabbit-hole of cost-related
regressions that even Bernd S. backed away from).

> So no, no reversion.

(...)

> > That's the only test that's improved to the point of
> > affecting test-patterns.  E.g. pr93372-5.c (which references
> > pr93372-2.c) is also improved, though it retains a redundant
> > compare insn.  (PR 93372 was about regressions from the cc0
> > representation; not further improvement like here, thus it's
> > not tagged.  Though, I did not double-check whether this
> > actually *was* a regression from cc0.)
> 
> Interesting that this improved tests for you.  Huh.  Do you have an
> explanation how this happened?

Just a hunch: less combine churn (more straightforward code)
made cmpelim's job easier, same thing you wrote in order
words:

>  I suspect that as uaual it is just a
> side effect of random factors: combine is opportunistic, always does the
> first change it thinks good, not considering what this then does for
> other possible combinations; it is greedy.  It would be nice to see
> written out what happens in this example though :-)

Yes it would, but I have other things on my plate.  Besides,
it's your patch, can't rob you of the fun.

I committed the revert below, but hope to re-apply
(re-revert) it in stage 1, when as per Richard B's message
the combine improvement will reappear.

brgds, H-P

-- >8 --
From: Hans-Peter Nilsson 
Date: Wed, 10 Apr 2024 17:24:10 +0200
Subject: [PATCH] Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass
 from combine improvement"

This reverts commit 4c8b3600c4856f7915281ae3ff4d97271c83a540.
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 2ef6471a990b..912069c018d5 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,20 +1,19 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
-/* { dg-final { scan-assembler-not "\tnot" } } */
-/* { dg-final { scan-assembler-not "\tlsr" } } */
-/* We should get just one move, storing the result into *d.  */
-/* { dg-final { scan-assembler-times "\tmove" 1 } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* We used to get a cmp.d with the original operands here. */
+  /* Whoops!  We get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
- (a.k.a "spl") re-using flags from the subtraction. */
+  /* Whoops!  While we don't get a test.d for the result here for cc0,
+ we get a sequence of insns: a move, a "not" and a shift of the
+ subtraction-result, where a simple "spl" would have done. */
   return c >= 0;
 }
-- 
2.30.2



brgds, H-P

[gcc r14-9904] Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement"

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:39f81924d88e3cc197fc3df74204c9b5e01e12f7

commit r14-9904-g39f81924d88e3cc197fc3df74204c9b5e01e12f7
Author: Hans-Peter Nilsson 
Date:   Wed Apr 10 17:24:10 2024 +0200

Revert "testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine 
improvement"

This reverts commit 4c8b3600c4856f7915281ae3ff4d97271c83a540.

Diff:
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 2ef6471a990..912069c018d 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,20 +1,19 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
-/* { dg-final { scan-assembler-not "\tnot" } } */
-/* { dg-final { scan-assembler-not "\tlsr" } } */
-/* We should get just one move, storing the result into *d.  */
-/* { dg-final { scan-assembler-times "\tmove" 1 } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* We used to get a cmp.d with the original operands here. */
+  /* Whoops!  We get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
- (a.k.a "spl") re-using flags from the subtraction. */
+  /* Whoops!  While we don't get a test.d for the result here for cc0,
+ we get a sequence of insns: a move, a "not" and a shift of the
+ subtraction-result, where a simple "spl" would have done. */
   return c >= 0;
 }

Re: [PATCH 2/9] wwwdocs: gcc-14: add URLs to some options

[Hans-Peter Nilsson]

On Thu, 4 Apr 2024, David Malcolm wrote:

> Signed-off-by: David Malcolm 
> ---
>  htdocs/gcc-14/changes.html | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
> index 5cc729c5..397458d5 100644
> --- a/htdocs/gcc-14/changes.html
> +++ b/htdocs/gcc-14/changes.html
> @@ -149,26 +149,33 @@ a work-in-progress.
>  to enable additional hardening.
>
>
> -New option -fhardened, an umbrella option that enables a set
> -of hardening flags.  The options it enables can be displayed using the
> +New option
> + href="https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fhardened;>-fhardened,

Shouldn't those URLs better point to a specific version, lest 
they might break with any newer release?

The question is "a bit" rhetorical, since there appears to be 
nothing at onlinedocs/gcc-14.0.0/ (and "nearby numbers").

Still, maybe there ought to be a copy of onlinedocs/gcc/ that is 
frozen at time of release.

brgds, H-P

[COMMITTED] testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement

[Hans-Peter Nilsson]

The xpassing change in generated code was as follows, at
r14-9788-gb7bd2ec73d66f7 (where I locally applied a revert
to verify that this suspect was the cause).  That was so
much of an improvement that I had to share it!  Worth the
testsuite churn anyway. :)

Segher, if you end up reverting r14-9692-g839bc42772ba7a (as
unfortunately seems not unlikely), then please also revert this
commit: r14-9799-g4c8b3600c4856f7915281ae3ff4d97271c83a540.

--- pr93372-2.s.pre 2024-04-05 01:49:47.985685902 +0200
+++ pr93372-2.s.post2024-04-05 01:42:02.296489730 +0200
@@ -5,12 +5,9 @@
.global _f
.type   _f, @function
 _f:
-   move.d $r10,$r9
-   sub.d $r11,$r9
-   cmp.d $r11,$r10
-   seq $r10
-   move.d $r10,[$r12]
-   cmpq 0,$r9
+   sub.d $r11,$r10
+   seq $r9
+   move.d $r9,[$r12]
ret
sge $r10
 

-- >8 --
After r14-9692-g839bc42772ba7a, a sequence that actually
looks optimal is now emitted, observed at
r14-9788-gb7bd2ec73d66f7.  This caused an XPASS for this
test.  While adjusting the test, better also guard it
against regressions by checking that there are no redundant
move insns.

That's the only test that's improved to the point of
affecting test-patterns.  E.g. pr93372-5.c (which references
pr93372-2.c) is also improved, though it retains a redundant
compare insn.  (PR 93372 was about regressions from the cc0
representation; not further improvement like here, thus it's
not tagged.  Though, I did not double-check whether this
actually *was* a regression from cc0.)

* gcc.target/cris/pr93372-2.c: Tweak scan-assembler
checks to cover recent combine improvement.
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 912069c018d5..2ef6471a990b 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,19 +1,20 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
-/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
-/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
+/* { dg-final { scan-assembler-not "\tnot" } } */
+/* { dg-final { scan-assembler-not "\tlsr" } } */
+/* We should get just one move, storing the result into *d.  */
+/* { dg-final { scan-assembler-times "\tmove" 1 } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* Whoops!  We get a cmp.d with the original operands here. */
+  /* We used to get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* Whoops!  While we don't get a test.d for the result here for cc0,
- we get a sequence of insns: a move, a "not" and a shift of the
- subtraction-result, where a simple "spl" would have done. */
+  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
+ (a.k.a "spl") re-using flags from the subtraction. */
   return c >= 0;
 }
-- 
2.30.2

[COMMITTED] testsuite/gcc.dg/debug/btf/btf-datasec-1.c: Handle leading-underscore

[Hans-Peter Nilsson]

Committed as obvious.

-- >8 --
I noticed my autotester for cris-elf flagging this as a regression.

* gcc.dg/debug/btf/btf-datasec-1.c: Adjust pattern for targets with
symbols having a leading underscore.
---
 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c 
b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
index 782216d3cb12..c8ebe5d07ca0 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
@@ -20,7 +20,7 @@
 /* { dg-final { scan-assembler-times "0xf01\[\t \]+\[^\n\]*btt_info" 1 } } 
*/
 
 /* The offset entry for each variable in a DATSEC should contain a label.  */
-/* { dg-final { scan-assembler-times 
"(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t
 \]|\\.vbyte\t4,\[\t \]?)\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */
+/* { dg-final { scan-assembler-times 
"(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t
 \]|\\.vbyte\t4,\[\t \]?)_?\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */
 /* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 
} } */
 /* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 } } 
*/
 
-- 
2.30.2

[gcc r14-9799] testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:4c8b3600c4856f7915281ae3ff4d97271c83a540

commit r14-9799-g4c8b3600c4856f7915281ae3ff4d97271c83a540
Author: Hans-Peter Nilsson 
Date:   Fri Apr 5 02:50:16 2024 +0200

testsuite/gcc.target/cris/pr93372-2.c: Handle xpass from combine improvement

After r14-9692-g839bc42772ba7a, a sequence that actually
looks optimal is now emitted, observed at
r14-9788-gb7bd2ec73d66f7.  This caused an XPASS for this
test.  While adjusting the test, better also guard it
against regressions by checking that there are no redundant
move insns.

That's the only test that's improved to the point of
affecting test-patterns.  E.g. pr93372-5.c (which references
pr93372-2.c) is also improved, though it retains a redundant
compare insn.  (PR 93372 was about regressions from the cc0
representation; not further improvement like here, thus it's
not tagged.  Though, I did not double-check whether this
actually *was* a regression from cc0.)

* gcc.target/cris/pr93372-2.c: Tweak scan-assembler
checks to cover recent combine improvement.

Diff:
---
 gcc/testsuite/gcc.target/cris/pr93372-2.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/gcc/testsuite/gcc.target/cris/pr93372-2.c 
b/gcc/testsuite/gcc.target/cris/pr93372-2.c
index 912069c018d..2ef6471a990 100644
--- a/gcc/testsuite/gcc.target/cris/pr93372-2.c
+++ b/gcc/testsuite/gcc.target/cris/pr93372-2.c
@@ -1,19 +1,20 @@
 /* Check that eliminable compare-instructions are eliminated. */
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
-/* { dg-final { scan-assembler-not "\tcmp|\ttest" { xfail *-*-* } } } */
-/* { dg-final { scan-assembler-not "\tnot" { xfail cc0 } } } */
-/* { dg-final { scan-assembler-not "\tlsr" { xfail cc0 } } } */
+/* { dg-final { scan-assembler-not "\tcmp|\ttest" } } */
+/* { dg-final { scan-assembler-not "\tnot" } } */
+/* { dg-final { scan-assembler-not "\tlsr" } } */
+/* We should get just one move, storing the result into *d.  */
+/* { dg-final { scan-assembler-times "\tmove" 1 } } */
 
 int f(int a, int b, int *d)
 {
   int c = a - b;
 
-  /* Whoops!  We get a cmp.d with the original operands here. */
+  /* We used to get a cmp.d with the original operands here. */
   *d = (c == 0);
 
-  /* Whoops!  While we don't get a test.d for the result here for cc0,
- we get a sequence of insns: a move, a "not" and a shift of the
- subtraction-result, where a simple "spl" would have done. */
+  /* We used to get a suboptimal sequence, but now we get the optimal "sge"
+ (a.k.a "spl") re-using flags from the subtraction. */
   return c >= 0;
 }

[gcc r14-9798] testsuite/gcc.dg/debug/btf/btf-datasec-1.c: Handle leading-underscore

[Hans-Peter Nilsson via Gcc-cvs]

https://gcc.gnu.org/g:3b36e86d6af3b305207c1aa6d56c2b350fefba65

commit r14-9798-g3b36e86d6af3b305207c1aa6d56c2b350fefba65
Author: Hans-Peter Nilsson 
Date:   Fri Apr 5 01:36:54 2024 +0200

testsuite/gcc.dg/debug/btf/btf-datasec-1.c: Handle leading-underscore

I noticed my autotester for cris-elf flagging this as a regression.

* gcc.dg/debug/btf/btf-datasec-1.c: Adjust pattern for targets with
symbols having a leading underscore.

Diff:
---
 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c 
b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
index 782216d3cb1..c8ebe5d07ca 100644
--- a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
+++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-1.c
@@ -20,7 +20,7 @@
 /* { dg-final { scan-assembler-times "0xf01\[\t \]+\[^\n\]*btt_info" 1 } } 
*/
 
 /* The offset entry for each variable in a DATSEC should contain a label.  */
-/* { dg-final { scan-assembler-times 
"(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t
 \]|\\.vbyte\t4,\[\t \]?)\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */
+/* { dg-final { scan-assembler-times 
"(?:(?:\\.4byte|\\.long|data4\\.ua|\\.ualong|\\.uaword|\\.dword|long|dc\\.l|\\.word)\[\t
 \]|\\.vbyte\t4,\[\t \]?)_?\[a-e\]\[\t \]+\[^\n\]*bts_offset" 5 } } */
 /* { dg-final { scan-assembler-times "my_cstruct\[\t \]+\[^\n\]*bts_offset" 1 
} } */
 /* { dg-final { scan-assembler-times "bigarr\[\t \]+\[^\n\]*bts_offset" 1 } } 
*/

Re: [PATCH] testsuite: Fix up lra effective target

[Hans-Peter Nilsson]

> Date: Fri, 16 Feb 2024 11:16:22 +0100
> From: Jakub Jelinek 

> Given the recent discussions on IRC started with Andrew P. mentioning that
> an asm goto outputs test should have { target lra } and the lra effective
> target in GCC 11/12 only returning 0 for PA and in 13/14 for PA/AVR, while
> we clearly have 14 other targets which don't support LRA and a couple of
> further ones which have an -mlra/-mno-lra switch (whatever default they
> have), seems to me the effective target is quite broken.

Definitely, good riddance to that list.

I suggested a little over a year ago to generalize
check_effective_target_lra to get rid of that flawed target
list but was effectively shut down with a review request
that'd *keep* the faulty non-lra target list. :-(
"https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611531.html;

TL;DR: I based LRA-ness on EBB being scanned in LRA but not
for reload (same empty foo), i.e. matching the string "EBB 2
3".  I don't know which method more stable, but that didn't
require -O2 nor -fdump-rtl-reload-details.

Having said that, I'm glad there's now a generic, working
(non-target-list-dependent) effective_target lra.

brgds, H-P

Re: [PATCH v4]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Oops, I managed to send a version that only added a comment,
but still had a dg-do run.  Anyway, here's v4: actually
change the "dg-do run", not just adding a comment.  Sending
as a self-contained fresh patch for the benefit of
aforementioned CI.  See v2 and v3 for more.  Sorry!

Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, A dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C | 56 +++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..51feb2e558e7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is 
resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2


> From: Hans-Peter Nilsson 
> CC: , 
> Content-Type: text/plain; charset="iso-8859-1"
> Date: Fri, 9 Feb 2024 16:30:43 +0100
> 
> Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
> due to this patch.  Running it "as usual" didn't show
> anything suspicious.  Sure, there were "# of unresolved
> testcases 3" in the summary (see v2), but no error or other
> special message from dejagnu.  Perhaps there could be a way
> to have dg-ice automatically downgrade a dg-do run that
> failed due to the ICE to a dg-do compile (or just not emit
> the UNRESOLVED

[PATCH v3]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Bah.  Linaro's CI didn't like that there were UNRESOLVEDs
due to this patch.  Running it "as usual" didn't show
anything suspicious.  Sure, there were "# of unresolved
testcases 3" in the summary (see v2), but no error or other
special message from dejagnu.  Perhaps there could be a way
to have dg-ice automatically downgrade a dg-do run that
failed due to the ICE to a dg-do compile (or just not emit
the UNRESOLVED note), but pragmatically, this is a rare
enough case to not bother.  It looks like these were the
only UNRESOLVEDs in that CI run, so I'm just going to make a
mental note and move on.

For more comments, please see v2 of this patch.

v3: Change dg-do run to dg-do compile to avoid an UNRESOLVED.
Tested as with v2.  Ok to commit?

-- >8 --

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

Regarding the request in the comment, a dg-do run when there's an ICE
will cause some CI's to signal an error for the run being "UNRESOLVED"
(compilation failed to produce executable).  Note that dejagnu (1.6.3)
itself doesn't consider this an error.

gcc/testsuite:
PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C | 56 +++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
 2 files changed, 110 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..6c396bff72b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,56 @@
+// PR c++/113545
+// { dg-do run { target c++14 } }
+// Please change the above "dg-do compile" to "dg-do run" when the ICE is 
resolved.
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2

[PATCH v2]: testcases for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

> Date: Wed, 7 Feb 2024 16:32:57 -0500
> From: Jason Merrill 

> Incidentally, these testcases seem to require C++14; you can't have a 
> switch in a constexpr function in C++11.

Update, v2 (from v1 that had a few requests from Marek
resolved from v0 that was posted together with my patch^Whack):
Move from cpp0x to cpp1y.  Qualify with c++14 instead of
c++11.  Add a one-liner commit-message.

I checked that these DTRT.  Currently:
Using "make check-gcc-c++ RUNTESTFLAGS=dg.exp=constexpr-reinterpret\*":

Running /x/gcc/gcc/testsuite/g++.dg/dg.exp ...

=== g++ Summary ===

# of expected passes33
# of expected failures  3
# of unresolved testcases   3
# of unsupported tests  4

...and that there's an XPASS when a ICE-resolving patch is
applied, testing each of my hack and Jason's, both yield:

Using /x/gcc/gcc/testsuite/config/default.exp as tool-and-target-specific 
interface file.
Running /x/gcc/gcc/testsuite/g++.dg/dg.exp ...
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++14 (internal compiler 
error)
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++17 (internal compiler 
error)
XPASS: g++.dg/cpp1y/constexpr-reinterpret3.C  -std=c++20 (internal compiler 
error)

And (since it appears the benefit isn't obvious) why commit
a test-cases before the fix?  Well, I'm not dead sure the
fix both gets there soon, and that it then stays there.
Even if it does so within decent time, as an exception
(IIUC) because we're in stage 4 and the bug is not a
regression, it could easily be reverted if it'd uncover some
other wart that's not sufficiently easily resolved.

Also, the fix seems to me sufficiently remote to the
gcc_assert, that the execution path leading to it could be
diverted; hidden or resolved while fixing something else,
and then it'd be nice to know that it fixed *this* bug too.

To wit: More dg-ice test-cases!  Don't fear adding xfails or
dg-ices!  Marek, you write that dg-ice support.  Thank you!

Ok to commit?

-- >8 --

gcc/testsuite:

Test-cases, with constexpr-reinterpret3.C dg-ice:ing the PR c++/113545 bug.

PR c++/113545
* g++.dg/cpp1y/constexpr-reinterpret3.C,
g++.dg/cpp1y/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp1y/constexpr-reinterpret3.C | 55 +++
 .../g++.dg/cpp1y/constexpr-reinterpret4.C | 54 ++
 2 files changed, 109 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
new file mode 100644
index ..ed914383f78b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret3.C
@@ -0,0 +1,55 @@
+// PR c++/113545
+// { dg-do run { target c++14 } }
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
new file mode 100644
index ..9aaa6e463bc6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++14 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void 

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Thu, 8 Feb 2024 11:22:47 -0500
> From: Marek Polacek 

> I'm confused; are you planning to use the dg-ice directive I invented
> some years ago?

Please, let's keep the discussion about the test-cases in
that thread.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Thu, 8 Feb 2024 10:44:31 -0500
> From: Marek Polacek 
> Cc: ja...@redhat.com, gcc-patches@gcc.gnu.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Thu, Feb 08, 2024 at 04:40:40PM +0100, Hans-Peter Nilsson wrote:
> > > Date: Wed, 7 Feb 2024 21:11:59 -0500
> > > From: Marek Polacek 
> > 
> > > On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > > > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > > > From: Marek Polacek 
> > > > > 
> > > > > > On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:
> > > > > > > I don't really know whether this is the right way to treat
> > > > > > > CONVERT_EXPR as below, but...  Regtested native
> > > > > > > x86_64-linux-gnu.  Ok to commit?
> > > > > > 
> > > > > > Thanks for taking a look at this problem.
> > > > > 
> > > > > Thanks for the initial review.
> > 
> > > > Incidentally, these testcases seem to require C++14; you can't have a 
> > > > switch
> > > > in a constexpr function in C++11.
> > > > 
> > > > Jason
> > > 
> > > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > > > index 2ebb1470dd5..fa346fe01c9 100644
> > > > --- a/gcc/cp/constexpr.cc
> > > > +++ b/gcc/cp/constexpr.cc
> > > > @@ -7106,6 +7106,16 @@ cxx_eval_switch_expr (const constexpr_ctx *ctx, 
> > > > tree t,
> > > >cond = cxx_eval_constant_expression (ctx, cond, vc_prvalue,
> > > >non_constant_p, overflow_p);
> > > >VERIFY_CONSTANT (cond);
> > > > +  if (TREE_CODE (cond) != INTEGER_CST)
> > > > +{
> > > > +  /* If the condition doesn't reduce to an INTEGER_CST it isn't a 
> > > > usable
> > > > +switch condition even if it's constant enough for other things
> > > > +(c++/113545).  */
> > > > +  gcc_checking_assert (ctx->quiet);
> > > > +  *non_constant_p = true;
> > > > +  return t;
> > > > +}
> > > > +
> > > >*jump_target = cond;
> > > >  
> > > >tree body
> > > 
> > > The patch makes sense to me, although I'm afraid that losing the
> > > REINTERPRET_CAST_P flag will cause other issues.
> > > 
> > > HP, sorry that I never got back to you.  I would be more than happy to
> > > take the patch above, add some tests and test/bootstrap it, unless you
> > > want to do that yourself.
> > > 
> > > Thanks & sorry again,
> > 
> > No worries, feel very welcome to deal with handling the
> > actual fix.  Also, you're better prepared than me, when it
> > comes to dealing with any possible fallout. :)
> > 
> > I'll send an updated version of the test-cases, moving them
> > to the C++14 test directory (cpp1y, right?) and qualify them
> > as c++14 instead, as Jason pointed out.
> 
> Right, cpp1y is c++14.  Note that we wouldn't push the tests separately
> to the patch itself, unless it's something that already works.  Thanks,
> 
> Marek

But, the tests work.  They passes as-is, as they track the
ICE, but will XPASS (that part) once a fix is committed (at
which time: "I checked that these behave as expected (xfail
as ICE properly) without the previosly posted patch to
cp/constexpr.cc and XPASS with it applied."

Once the fix works, the xfail for the ICE should be removed.
(Hm, better comment on the patches in a reply to that message. :)

The point is that for this type of bug it's useful to have a
test-case tracking it, before a fix is committed.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Wed, 7 Feb 2024 21:11:59 -0500
> From: Marek Polacek 

> On Wed, Feb 07, 2024 at 04:32:57PM -0500, Jason Merrill wrote:
> > On 2/6/24 19:23, Hans-Peter Nilsson wrote:
> > > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > > From: Marek Polacek 
> > > 
> > > > On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:
> > > > > I don't really know whether this is the right way to treat
> > > > > CONVERT_EXPR as below, but...  Regtested native
> > > > > x86_64-linux-gnu.  Ok to commit?
> > > > 
> > > > Thanks for taking a look at this problem.
> > > 
> > > Thanks for the initial review.

> > Incidentally, these testcases seem to require C++14; you can't have a switch
> > in a constexpr function in C++11.
> > 
> > Jason
> 
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 2ebb1470dd5..fa346fe01c9 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -7106,6 +7106,16 @@ cxx_eval_switch_expr (const constexpr_ctx *ctx, tree 
> > t,
> >cond = cxx_eval_constant_expression (ctx, cond, vc_prvalue,
> >non_constant_p, overflow_p);
> >VERIFY_CONSTANT (cond);
> > +  if (TREE_CODE (cond) != INTEGER_CST)
> > +{
> > +  /* If the condition doesn't reduce to an INTEGER_CST it isn't a 
> > usable
> > +switch condition even if it's constant enough for other things
> > +(c++/113545).  */
> > +  gcc_checking_assert (ctx->quiet);
> > +  *non_constant_p = true;
> > +  return t;
> > +}
> > +
> >*jump_target = cond;
> >  
> >tree body
> 
> The patch makes sense to me, although I'm afraid that losing the
> REINTERPRET_CAST_P flag will cause other issues.
> 
> HP, sorry that I never got back to you.  I would be more than happy to
> take the patch above, add some tests and test/bootstrap it, unless you
> want to do that yourself.
> 
> Thanks & sorry again,

No worries, feel very welcome to deal with handling the
actual fix.  Also, you're better prepared than me, when it
comes to dealing with any possible fallout. :)

I'll send an updated version of the test-cases, moving them
to the C++14 test directory (cpp1y, right?) and qualify them
as c++14 instead, as Jason pointed out.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek 

> On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:
> > I don't really know whether this is the right way to treat
> > CONVERT_EXPR as below, but...  Regtested native
> > x86_64-linux-gnu.  Ok to commit?
> 
> Thanks for taking a look at this problem.

Thanks for the initial review.

>  
> > brgds, H-P
> > 
> > -- >8 --
> > That gcc_unreachable at the default-label seems to be over
> > the top.  It seems more correct to just say "that's not
> > constant" to whatever's not known (to be constant), when
> > looking for matches in switch-statements.
> 
> Unfortunately this doesn't seem correct to me; I don't think we
> should have gotten that far.  It appears that we lose track of
> the reinterpret_cast, which is not allowed in a constant expression:
> <http://eel.is/c++draft/expr.const#5.15>.
> 
> cp_convert -> ... -> convert_to_integer_1 gives us a CONVERT_EXPR
> but we only set REINTERPRET_CAST_P on NOP_EXPRs:
> 
>   expr = cp_convert (type, expr, complain);
>   if (TREE_CODE (expr) == NOP_EXPR)
> /* Mark any nop_expr that created as a reintepret_cast.  */
> REINTERPRET_CAST_P (expr) = true;
> 
> so when evaluating baz we get (long unsigned int) , which
> passes verify_constant.
>  
> I don't have a good suggestion yet, sorry.

But, with this patch, we're letting the non-constant case
take the same path and failing for the same reason, albeit
much later than desired, for the switch code as for the
if-chain code.  Isn't that better than the current ICE?

I mean, if there's a risk of accepts-invalid (like, some
non-constant case incorrectly "constexpr'd"), then that risk
is as already there, for the if-chain case.

Anyway, this is a bit too late in the release season and
isn't a regression, thus I can't argue for it being a
suitable stop-gap measure...

I'm unassigning myself from the PR as I have no clue how to
fix the actual non-constexpr-operand-seen-too-late bug.

Though, I'm asking again; any clue regarding:

"I briefly considered one of the cpp[0-9a-z]* subdirectories
but found no rule.

Isn't constexpr c++11 and therefor cpp0x isn't a good match
(contrary to the many constexpr tests therein)?

What *is* the actual rule for putting a test in
g++.dg/cpp0x, cpp1x and cpp1y (et al)?
(I STFW but found nothing.)"


> > With this patch, the code generated for the (inlined) call to
> > ifbar equals that to swbar, except for the comparisons being
> > in another order.
> > 
> > gcc/cp:
> > PR c++/113545
> > * constexpr.cc (label_matches): Replace call to_unreachable with
> 
> "to gcc_unreachable"
> 
> > return false.
> 
> More like with "break" but that's not important.
>  
> > gcc/testsuite:

(Deleted -- see separate patch)

> > ---
> >  gcc/cp/constexpr.cc |  3 +-
> >  gcc/testsuite/g++.dg/expr/pr113545.C | 49 +
> >  2 files changed, 51 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/g++.dg/expr/pr113545.C
> > 
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 6350fe154085..30caf3322fff 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -6922,7 +6922,8 @@ label_matches (const constexpr_ctx *ctx, tree 
> > *jump_target, tree stmt)
> >break;
> >  
> >  default:
> > -  gcc_unreachable ();
> > +  /* Something else, like CONVERT_EXPR.  Unknown whether it matches.  
> > */
> > +  break;
> >  }
> >return false;
> >  }
> > diff --git a/gcc/testsuite/g++.dg/expr/pr113545.C 
> > b/gcc/testsuite/g++.dg/expr/pr113545.C
> > new file mode 100644
> > index ..914ffdeb8e16

brgds, H-P

Ping*2 PATCH: testcase for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Tue, 30 Jan 2024 06:18:45 +0100

> Ping for the xfailed testsuite patch below the review
> (actual constexpr.cc patch to be handled separately):

Ping*2.  Again, this is for the xfailed test-case only.

> 
> > From: Hans-Peter Nilsson 
> > Date: Tue, 23 Jan 2024 05:55:00 +0100
> > 
> > > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > > From: Marek Polacek 
> > 
> > > The problem seems to be more about conversion so 
> > > g++.dg/conversion/reinterpret5.C
> > > or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate.
> > > 
> > > > @@ -0,0 +1,49 @@
> > > 
> > > Please add
> > > 
> > > PR c++/113545
> > 
> > > > +  unsigned const char c = 
> > > > swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > > > +  xyzzy(c);
> > > > +  unsigned const char d = 
> > > > ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > > 
> > > I suppose we should also test a C-style cast (which leads to a 
> > > reinterpret_cast
> > > in this case).
> > > 
> > > Maybe check we get an error when c/d are constexpr (that used to ICE).
> > 
> > Like this?  Not sure about the value of that variant, but here goes.
> > 
> > I checked that these behave as expected (xfail as ICE properly) without the
> > previosly posted patch to cp/constexpr.cc and XPASS with it applied.
> > 
> > Ok to commit?
> > 
> > -- >8 --
> > Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
> >  passing non-constexpr parameter)
> > 
> > gcc/testsuite:
> > PR c++/113545
> > * g++.dg/cpp0x/constexpr-reinterpret3.C,
> > g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
> > ---
> >  .../g++.dg/cpp0x/constexpr-reinterpret3.C | 55 +++
> >  .../g++.dg/cpp0x/constexpr-reinterpret4.C | 54 ++
> >  2 files changed, 109 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> >  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > 
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C 
> > b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> > new file mode 100644
> > index ..319cc5e8bee9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> > @@ -0,0 +1,55 @@
> > +// PR c++/113545
> > +// { dg-do run { target c++11 } }
> > +// { dg-ice "PR112545 - constexpr function with switch called for 
> > reinterpret_cast" }
> > +
> > +char foo;
> > +
> > +// This one caught a call to gcc_unreachable in
> > +// cp/constexpr.cc:label_matches, when passed a convert_expr from the
> > +// cast in the call.
> > +constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
> > +{
> > +  switch (baz)
> > +{
> > +case 13:
> > +  return 11;
> > +case 14:
> > +  return 78;
> > +case 2048:
> > +  return 13;
> > +default:
> > +  return 42;
> > +}
> > +}
> > +
> > +// For reference, the equivalent* if-statements.
> > +constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
> > +{
> > +  if (baz == 13)
> > +return 11;
> > +  else if (baz == 14)
> > +return 78;
> > +  else if (baz == 2048)
> > +return 13;
> > +  else
> > +return 42;
> > +}
> > +
> > +__attribute__ ((__noipa__))
> > +void xyzzy(int x)
> > +{
> > +  if (x != 42)
> > +__builtin_abort ();
> > +}
> > +
> > +int main()
> > +{
> > +  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > +  xyzzy(c);
> > +  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > +  xyzzy(d);
> > +  unsigned const char e = swbar((__UINTPTR_TYPE__) );
> > +  xyzzy(e);
> > +  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
> > +  xyzzy(f);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C 
> > b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > new file mode 100644
> > index ..4d0fdf2c0a78
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> > @@ -0,0 +1,54 @@
> > +// PR c++/113545
> > +// { dg-do compile { target c++11 } }
> > +
> > +char foo;
> > +
> > +// This one caught a call to gcc_unreachable in
&

Re: [PATCH 1/2] libstdc++: Replace padding bits with a bit-field in __format::_Spec

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Date: Thu, 1 Feb 2024 19:24:49 +

> I think I'd prefer to keep the reserved bits together, but a simpler
> way to avoid 'unsigned long' making a difference for
> PCC_BITFIELD_TYPE_MATTERS targets would be to use no more than 16 bits
> but do:
> 
>unsigned _M_reserved : 1;
>unsigned _M_reserved2 : 16;
> 

Hah! Genious! :-)

brgds, H-P

Re: [PATCH 1/2] libstdc++: Replace padding bits with a bit-field in __format::_Spec

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 1 Feb 2024 17:16:47 +0100

> Not speaking for other platforms with default-packed layout
> or where ABI structure layout alignment implies a change due
> to PCC_BITFIELD_TYPE_MATTERS and the "unsigned long"
> bitfield type.
> 
> That last one may matter though.

> > diff --git a/libstdc++-v3/include/std/format 
> > b/libstdc++-v3/include/std/format
> > index 0eca8b58bfa..6c958bc11a5 100644
> > --- a/libstdc++-v3/include/std/format
> > +++ b/libstdc++-v3/include/std/format
> > @@ -406,6 +406,7 @@ namespace __format
> >_WidthPrec _M_width_kind : 2;
> >_WidthPrec _M_prec_kind : 2;
> >_Pres_type _M_type : 4;
> > +  unsigned long _M_reserved : 17;

FAOD (no doubt you got this already, but...)

I'd suggest making that "unsigned long" only for 16-bitters (i.e. where
you'd need a type larger than "unsigned int" to cover 17 bits) and
"unsigned" elsewhere, so at least you have no impact from
PCC_BITFIELD_TYPE_MATTERS.

brgds, H-P

Re: [PATCH 1/2] libstdc++: Replace padding bits with a bit-field in __format::_Spec

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Cc: Hans-Peter Nilsson 
> Date: Thu,  1 Feb 2024 15:36:50 +

> I plan to push this to trunk soon.
> 
> CC HP for visibility of the change affecting cris-elf. In practice it
> shouldn't make any difference to any sensible code. It only affects
> C++20 mode (and later), and only changes the size of std::formatter
> objects which are typically only created by the library headers
> themselves, and only on the stack (when using std::format and other new
> C++20 APIs related to it).
> 
> -- >8 --
> 
> This ensures that the unused bits will be zero-initialized reliably, and
> so can be used later by assigning them values in formatter
> specializations. For example, formatters for std::chrono will need to
> use an extra bit for a boolean to optimize the conversions between
> locale encodings and UTF-8.
> 
> This will result in an ABI change for targets that use 1-byte alignment
> for all integral types, e.g. cris-elf.

Thanks for the heads-up, but don't worry about ABI changes
for cris-elf.

Not speaking for other platforms with default-packed layout
or where ABI structure layout alignment implies a change due
to PCC_BITFIELD_TYPE_MATTERS and the "unsigned long"
bitfield type.

That last one may matter though.

> We can't do that once C++20
> support is non-experimental and ABI stable, so do it now before GCC 14
> is released.
> 
> libstdc++-v3/ChangeLog:
> 
>   * include/std/format (__format::_Spec::_M_reserved): Define a
>   new bit-field member in place of padding bits.
> ---
>  libstdc++-v3/include/std/format | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libstdc++-v3/include/std/format b/libstdc++-v3/include/std/format
> index 0eca8b58bfa..6c958bc11a5 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -406,6 +406,7 @@ namespace __format
>_WidthPrec _M_width_kind : 2;
>_WidthPrec _M_prec_kind : 2;
>_Pres_type _M_type : 4;
> +  unsigned long _M_reserved : 17;
>unsigned short _M_width;
>unsigned short _M_prec;
>char32_t _M_fill = ' ';
> -- 
> 2.43.0
> 

Ping PATCH: testcase for "ICE for unknown parameter to constexpr'd switch-statement, PR113545"

[Hans-Peter Nilsson]

Ping for the xfailed testsuite patch below the review
(actual constexpr.cc patch to be handled separately):

> From: Hans-Peter Nilsson 
> Date: Tue, 23 Jan 2024 05:55:00 +0100
> 
> > Date: Mon, 22 Jan 2024 14:33:59 -0500
> > From: Marek Polacek 
> 
> > The problem seems to be more about conversion so 
> > g++.dg/conversion/reinterpret5.C
> > or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate.
> > 
> > > @@ -0,0 +1,49 @@
> > 
> > Please add
> > 
> > PR c++/113545
> 
> > > +  unsigned const char c = 
> > > swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > > +  xyzzy(c);
> > > +  unsigned const char d = 
> > > ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > 
> > I suppose we should also test a C-style cast (which leads to a 
> > reinterpret_cast
> > in this case).
> > 
> > Maybe check we get an error when c/d are constexpr (that used to ICE).
> 
> Like this?  Not sure about the value of that variant, but here goes.
> 
> I checked that these behave as expected (xfail as ICE properly) without the
> previosly posted patch to cp/constexpr.cc and XPASS with it applied.
> 
> Ok to commit?
> 
> -- >8 --
> Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
>  passing non-constexpr parameter)
> 
> gcc/testsuite:
>   PR c++/113545
>   * g++.dg/cpp0x/constexpr-reinterpret3.C,
>   g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
> ---
>  .../g++.dg/cpp0x/constexpr-reinterpret3.C | 55 +++
>  .../g++.dg/cpp0x/constexpr-reinterpret4.C | 54 ++
>  2 files changed, 109 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> 
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C 
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> new file mode 100644
> index ..319cc5e8bee9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
> @@ -0,0 +1,55 @@
> +// PR c++/113545
> +// { dg-do run { target c++11 } }
> +// { dg-ice "PR112545 - constexpr function with switch called for 
> reinterpret_cast" }
> +
> +char foo;
> +
> +// This one caught a call to gcc_unreachable in
> +// cp/constexpr.cc:label_matches, when passed a convert_expr from the
> +// cast in the call.
> +constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
> +{
> +  switch (baz)
> +{
> +case 13:
> +  return 11;
> +case 14:
> +  return 78;
> +case 2048:
> +  return 13;
> +default:
> +  return 42;
> +}
> +}
> +
> +// For reference, the equivalent* if-statements.
> +constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
> +{
> +  if (baz == 13)
> +return 11;
> +  else if (baz == 14)
> +return 78;
> +  else if (baz == 2048)
> +return 13;
> +  else
> +return 42;
> +}
> +
> +__attribute__ ((__noipa__))
> +void xyzzy(int x)
> +{
> +  if (x != 42)
> +__builtin_abort ();
> +}
> +
> +int main()
> +{
> +  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> +  xyzzy(c);
> +  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> +  xyzzy(d);
> +  unsigned const char e = swbar((__UINTPTR_TYPE__) );
> +  xyzzy(e);
> +  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
> +  xyzzy(f);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C 
> b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> new file mode 100644
> index ..4d0fdf2c0a78
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
> @@ -0,0 +1,54 @@
> +// PR c++/113545
> +// { dg-do compile { target c++11 } }
> +
> +char foo;
> +
> +// This one caught a call to gcc_unreachable in
> +// cp/constexpr.cc:label_matches, when passed a convert_expr from the
> +// cast in the call.
> +constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
> +{
> +  switch (baz)
> +{
> +case 13:
> +  return 11;
> +case 14:
> +  return 78;
> +case 2048:
> +  return 13;
> +default:
> +  return 42;
> +}
> +}
> +
> +// For reference, the equivalent* if-statements.
> +constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
> +{
> +  if (baz == 13)
> +return 11;
> +  else if (baz == 14)
> +return 78;
> +  else if (baz == 2048)
> +return 13;
> +  else
> +return 42;
> +}
> +
> +__attribute__ ((__noipa__))
> +void xyzzy(int x)
> +{

Re: RFC: Formalization of the Intel assembly syntax (PR53929)

[Hans-Peter Nilsson]

On Fri, 19 Jan 2024, LIU Hao wrote:

> ? 2024-01-18 20:54, Jan Beulich ??:
> > I'm sorry, but most of your proposal may even be considered for being
> > acceptable only if you would gain buy-off from the MASM guys. Anything
> > MASM treats as valid ought to be permitted by gas as well (within the
> > scope of certain divergence that cannot be changed in gas without
> > risking to break people's code). It could probably be considered to
> > introduce a "strict" mode of Intel syntax, following some / most of
> > what you propose; making this the default cannot be an option.
> 
> Thanks for your reply.
> 
> I have attached the Markdown source for that page, modified a few hours ago. I
> am planning to make some updates according to your advice tomorrow.
> 
> And yes, I am proposing a 'strict' mode, however not for humans, only for
> compilers.
> 
> My first message references a GCC bug report, where the problematic symbol
> `bx` comes from C source. I have been aware of the `/APP` and `/NO_APP`

It's #APP #NO_APP, not /APP /NO_APP, for x86_64-linux, even for 
-masm=intel.

> markers in generated assembly, so I suspect that GAS should be able to tell
> which parts are generated from a compiler and which parts are composed by
> hand.

Since a very long time, none but a very few gcc targets (not 
including i686/x64_64-linux) emit the initial #NO_APP, which 
have to be the very first characters of the generated assembly 
file, without which subsequent #APP/#NO_APP pairs are just for 
show.

That said, I guess you're going to modify gas too.  But please 
don't change the #APP/#NO_APP semantics for non-intel targets.

brgds, H-P

[PATCH v2] c/c++: Tweak warning for 'always_inline function might not be inlinable'

[Hans-Peter Nilsson]

Change from v1: The message is changed as per the review.
The powerpc test-case is dropped from the changes as the
part quoted in a comment now does not change and so cannot
cause further confusion.  The commit message is tweaked.
It now also mentions clang.  I intend to commit this on
Thursday 2024-01-25, as per Richard Biener's approval.

-- >8 --
When you're not regularly exposed to this warning, it is
easy to be misled by its wording, believing that there's
something else in the function that stops it from being
inlined, something other than the lack of also being
*declared* inline.  Also, clang does not warn.

It's just a warning: without the inline directive, there has
to be a secondary reason for the function to be inlined,
other than the always_inline attribute, a reason that may be
in effect despite the warning.

Whenever the text is quoted in inline-related bugzilla
entries, there seems to often have been an initial step of
confusion that has to be cleared, for example in PR55830.
A file in the powerpc-specific parts of the test-suite,
gcc.target/powerpc/vec-extract-v16qiu-v2.h, has a comment
and seems to be another example, and I testify as the
first-hand third "experience".  The wording has been the
same since the warning was added.

Let's just tweak the wording, adding the cause, so that the
reason for the warning is clearer.  This hopefully stops the
user from immediately asking "'Might'?  Because why?"  and
then going off looking at the function body - or grepping
the gcc source or documentation, or enter a bug-report
subsequently closed as resolved/invalid.

Since the message is only appended with additional
information, no test-case actually required adjustment.
I still changed them, so the message is covered.

gcc:
* cgraphunit.cc (process_function_and_variable_attributes): Tweak
the warning for an attribute-always_inline without inline declaration.

gcc/testsuite:
* g++.dg/Wattributes-3.C: Adjust expected warning.
* gcc.dg/fail_always_inline.c: Ditto.
---
 gcc/cgraphunit.cc | 3 ++-
 gcc/testsuite/g++.dg/Wattributes-3.C  | 4 ++--
 gcc/testsuite/gcc.dg/fail_always_inline.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
index 38052674aaa5..5c405258ec30 100644
--- a/gcc/cgraphunit.cc
+++ b/gcc/cgraphunit.cc
@@ -918,7 +918,8 @@ process_function_and_variable_attributes (cgraph_node 
*first,
  /* redefining extern inline function makes it DECL_UNINLINABLE.  */
  && !DECL_UNINLINABLE (decl))
warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
-   "% function might not be inlinable");
+   "% function might not be inlinable"
+   " unless also declared %");
 
   process_common_attributes (node, decl);
 }
diff --git a/gcc/testsuite/g++.dg/Wattributes-3.C 
b/gcc/testsuite/g++.dg/Wattributes-3.C
index 208ec6696551..dd9c2244900c 100644
--- a/gcc/testsuite/g++.dg/Wattributes-3.C
+++ b/gcc/testsuite/g++.dg/Wattributes-3.C
@@ -26,7 +26,7 @@ B::operator char () const { return 0; }
 
 ATTR ((__noinline__))
 B::operator int () const  // { dg-warning "ignoring attribute .noinline. 
because it conflicts with attribute .always_inline." }
-// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
+// { dg-warning "function might not be inlinable unless also declared 
.inline." "" { target *-*-* } .-1 }
 
 {
   return 0;
@@ -45,7 +45,7 @@ C::operator char () { return 0; }
 
 ATTR ((__noinline__))
 C::operator short ()   // { dg-warning "ignoring attribute .noinline. 
because it conflicts with attribute .always_inline." }
-// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
+// { dg-warning "function might not be inlinable unless also declared 
.inline." "" { target *-*-* } .-1 }
 { return 0; }
 
 inline ATTR ((__noinline__))
diff --git a/gcc/testsuite/gcc.dg/fail_always_inline.c 
b/gcc/testsuite/gcc.dg/fail_always_inline.c
index 86645b850de8..16a549ca0935 100644
--- a/gcc/testsuite/gcc.dg/fail_always_inline.c
+++ b/gcc/testsuite/gcc.dg/fail_always_inline.c
@@ -2,7 +2,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 extern __attribute__ ((always_inline)) void
- bar() { } /* { dg-warning "function might not be inlinable" } */
+ bar() { } /* { dg-warning "function might not be inlinable unless also 
declared .inline." } */
 
 void
 f()
-- 
2.30.2

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek 

> The problem seems to be more about conversion so 
> g++.dg/conversion/reinterpret5.C
> or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate.
> 
> > @@ -0,0 +1,49 @@
> 
> Please add
> 
> PR c++/113545

> > +  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > +  xyzzy(c);
> > +  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> 
> I suppose we should also test a C-style cast (which leads to a 
> reinterpret_cast
> in this case).
> 
> Maybe check we get an error when c/d are constexpr (that used to ICE).

Like this?  Not sure about the value of that variant, but here goes.

I checked that these behave as expected (xfail as ICE properly) without the
previosly posted patch to cp/constexpr.cc and XPASS with it applied.

Ok to commit?

-- >8 --
Subject: [PATCH] c++: testcases for PR113545 (constexpr with switch and
 passing non-constexpr parameter)

gcc/testsuite:
PR c++/113545
* g++.dg/cpp0x/constexpr-reinterpret3.C,
g++.dg/cpp0x/constexpr-reinterpret4.C: New tests.
---
 .../g++.dg/cpp0x/constexpr-reinterpret3.C | 55 +++
 .../g++.dg/cpp0x/constexpr-reinterpret4.C | 54 ++
 2 files changed, 109 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C

diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
new file mode 100644
index ..319cc5e8bee9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret3.C
@@ -0,0 +1,55 @@
+// PR c++/113545
+// { dg-do run { target c++11 } }
+// { dg-ice "PR112545 - constexpr function with switch called for 
reinterpret_cast" }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+  unsigned const char e = swbar((__UINTPTR_TYPE__) );
+  xyzzy(e);
+  unsigned const char f = ifbar((__UINTPTR_TYPE__) );
+  xyzzy(f);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C 
b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
new file mode 100644
index ..4d0fdf2c0a78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-reinterpret4.C
@@ -0,0 +1,54 @@
+// PR c++/113545
+// { dg-do compile { target c++11 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned constexpr char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(c);
+  unsigned constexpr char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>()); 
// { dg-error "conversion from pointer type" }
+  xyzzy(d);
+  unsigned constexpr char e = swbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(e);
+  unsigned constexpr char f = ifbar((__UINTPTR_TYPE__) ); // { dg-error 
"conversion from pointer type" }
+  xyzzy(f);
+}
-- 
2.30.2

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek 

Oh, there was more...  Also, I think I misinterpreted your
reply as meaning that the test-case is ice-on-invalid and I
wrongly replied in agreement to that misinterpretation. :)

(For others at a comparable level of (lack of) C++ knowledge
to me: my reading of
https://en.cppreference.com/w/cpp/language/constexpr is that
a constexpr function can be validly called with an
expression that isn't "constexpr" (compile-time computable,
immediately computable, core constant expressions or
whatever the term), and the test-case is a valid example (of
two such invocations).  So, an expression calling such a
function is only truly "constexpr" with the "right"
parameters.  I know this isn't C++ 102, but many of us
hacking gcc aren't originally c++ hackers; that was just
happenstance.  I was about to write "aren't C++ hackers" but
then again, C++ happened to gcc, and c++11 at that.)

> On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:

> The problem seems to be more about conversion so 
> g++.dg/conversion/reinterpret5.C
> or g++.dg/cpp0x/constexpr-reinterpret3.C seems more appropriate.

I briefly considered one of the cpp[0-9a-z]* subdirectories
but found no rule.

Isn't constexpr c++11 and therefor cpp0x isn't a good match
(contrary to the many constexpr tests therein)?

What *is* the actual rule for putting a test in
g++.dg/cpp0x, cpp1x and cpp1y (et al)?
(I STFW but found nothing.)

> > +++ b/gcc/testsuite/g++.dg/expr/pr113545.C
> > @@ -0,0 +1,49 @@
> 
> Please add
> 
> PR c++/113545

Certainly if the test is to change name and even if not
("git grep" wouldn't catch the file name).  Will do.

> > +  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
> > +  xyzzy(c);
> > +  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
> 
> I suppose we should also test a C-style cast (which leads to a 
> reinterpret_cast
> in this case).
> 
> Maybe check we get an error when c/d are constexpr (that used to ICE).

But the expressions aren't declared constexpr, just const
(as in "here 'const' means run-time evaluation due to the
weirdness that is C++").

...oh, I see what you mean, these are valid, but you suggest
adding tests declared constexpr to check that they get a
matching error (not ICE :) !

Thanks again for the review, I think I'll at least re-work
the test-case into two separate ones.

brgds, H-P

Re: [PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

> Date: Mon, 22 Jan 2024 14:33:59 -0500
> From: Marek Polacek 

> On Mon, Jan 22, 2024 at 06:02:32PM +0100, Hans-Peter Nilsson wrote:
> > I don't really know whether this is the right way to treat
> > CONVERT_EXPR as below, but...  Regtested native
> > x86_64-linux-gnu.  Ok to commit?
> 
> Thanks for taking a look at this problem.

Honestly, it's more like posting a patch is more effective
than just opening a PR. ;)  And I was curious about that
constexpr thing that usually works, but blew up here.

> > brgds, H-P
> > 
> > -- >8 --
> > That gcc_unreachable at the default-label seems to be over
> > the top.  It seems more correct to just say "that's not
> > constant" to whatever's not known (to be constant), when
> > looking for matches in switch-statements.
> 
> Unfortunately this doesn't seem correct to me; I don't think we
> should have gotten that far.

The gcc_unreachable was indeed a clue in that direction.

>  It appears that we lose track of
> the reinterpret_cast, which is not allowed in a constant expression:
> <http://eel.is/c++draft/expr.const#5.15>.

B...b..but clang allows it... (and I have no clue about the
finer --or admittedly even coarser-- details of C++) ...and
not-my-code, just "porting" it.

Seriously though, thanks for the reference.  Also, maybe
something to consider for -fpermissive, if this changes to a
more graceful error path.

> cp_convert -> ... -> convert_to_integer_1 gives us a CONVERT_EXPR
> but we only set REINTERPRET_CAST_P on NOP_EXPRs:
> 
>   expr = cp_convert (type, expr, complain);
>   if (TREE_CODE (expr) == NOP_EXPR)
> /* Mark any nop_expr that created as a reintepret_cast.  */
> REINTERPRET_CAST_P (expr) = true;
> 
> so when evaluating baz we get (long unsigned int) , which
> passes verify_constant.
>  
> I don't have a good suggestion yet, sorry.

Thanks for the review!

> > With this patch, the code generated for the (inlined) call to
> > ifbar equals that to swbar, except for the comparisons being
> > in another order.
> > 
> > gcc/cp:
> > PR c++/113545
> > * constexpr.cc (label_matches): Replace call to_unreachable with
> 
> "to gcc_unreachable"

Oops!

> > return false.
> 
> More like with "break" but that's not important.

(Well, *effectively* return false.  I'd change to something
like "Replace call to gcc_unreachable with arrangement to
return false" if this were to go anywhere.)

> > gcc/testsuite:
> > * g++.dg/expr/pr113545.C: New text.
> 
> "test"

Gosh, horrible typos, thanks.

brgds, H-P

[PATCH] c++: Don't ICE for unknown parameter to constexpr'd switch-statement, PR113545

[Hans-Peter Nilsson]

I don't really know whether this is the right way to treat
CONVERT_EXPR as below, but...  Regtested native
x86_64-linux-gnu.  Ok to commit?

brgds, H-P

-- >8 --
That gcc_unreachable at the default-label seems to be over
the top.  It seems more correct to just say "that's not
constant" to whatever's not known (to be constant), when
looking for matches in switch-statements.

With this patch, the code generated for the (inlined) call to
ifbar equals that to swbar, except for the comparisons being
in another order.

gcc/cp:
PR c++/113545
* constexpr.cc (label_matches): Replace call to_unreachable with
return false.

gcc/testsuite:
* g++.dg/expr/pr113545.C: New text.
---
 gcc/cp/constexpr.cc |  3 +-
 gcc/testsuite/g++.dg/expr/pr113545.C | 49 +
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/expr/pr113545.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 6350fe154085..30caf3322fff 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -6922,7 +6922,8 @@ label_matches (const constexpr_ctx *ctx, tree 
*jump_target, tree stmt)
   break;
 
 default:
-  gcc_unreachable ();
+  /* Something else, like CONVERT_EXPR.  Unknown whether it matches.  */
+  break;
 }
   return false;
 }
diff --git a/gcc/testsuite/g++.dg/expr/pr113545.C 
b/gcc/testsuite/g++.dg/expr/pr113545.C
new file mode 100644
index ..914ffdeb8e16
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/pr113545.C
@@ -0,0 +1,49 @@
+// { dg-do run { target c++11 } }
+
+char foo;
+
+// This one caught a call to gcc_unreachable in
+// cp/constexpr.cc:label_matches, when passed a convert_expr from the
+// cast in the call.
+constexpr unsigned char swbar(__UINTPTR_TYPE__ baz)
+{
+  switch (baz)
+{
+case 13:
+  return 11;
+case 14:
+  return 78;
+case 2048:
+  return 13;
+default:
+  return 42;
+}
+}
+
+// For reference, the equivalent* if-statements.
+constexpr unsigned char ifbar(__UINTPTR_TYPE__ baz)
+{
+  if (baz == 13)
+return 11;
+  else if (baz == 14)
+return 78;
+  else if (baz == 2048)
+return 13;
+  else
+return 42;
+}
+
+__attribute__ ((__noipa__))
+void xyzzy(int x)
+{
+  if (x != 42)
+__builtin_abort ();
+}
+
+int main()
+{
+  unsigned const char c = swbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(c);
+  unsigned const char d = ifbar(reinterpret_cast<__UINTPTR_TYPE__>());
+  xyzzy(d);
+}
-- 
2.30.2

Re: [PATCH] c/c++: Tweak warning for 'always_inline function might not be inlinable'

[Hans-Peter Nilsson]

> From: Richard Biener 
> Date: Mon, 22 Jan 2024 08:33:47 +0100

> > -   "% function might not be inlinable");
> > +   "% function is not always inlined"
> > +   " unless also declared %");
> 
> I don't like the "is not always inlined", maybe simply reword to
> 
>   "% function might not be inlinable"
>   " unless also declared %"
> 
> ?

Sure.  Though it's a small nuance to which I don't actually
agree, I'll go along with almost anything as long as the
"...declared inline" augmentation is there :-)

Also, I can see that keeping closer to the original wording
as you suggest can be preferable to some.

I assume by your reply that the patch is ok with that change
but will wait another 72 hours for "native speakers" to have
a say.

Thanks!

brgds, H-P

[PATCH] c/c++: Tweak warning for 'always_inline function might not be inlinable'

[Hans-Peter Nilsson]

Tested x86_64-linux-gnu.  Ok to commit?

Or, does the message need more tweaking?
(If so, suggestions from native speakers?)
FWIW, I found no PR for just the message being bad.

-- >8 --
When you're not regularly exposed to this warning, it is
easy to be misled by its wording, believing that there's
something else in the function that stops it from being
inlined, than the lack of also being *declared* inline.

It's just a warning: without the inline directive, there has
to be a secondary reasons for the function to be inlined,
than the always_inline attribute, reasons that may be in
effect despite the warning.

Whenever the text is quoted in inline-related bugzilla
entries, there seems to often have been an initial step of
confusion that has to be cleared, for example in PR55830.
The powerpc test-case in this patch where a comment is
adjusted, seems to be another example, and I testify as the
first-hand third "experience".  The wording has been the
same since the warning was added.

Let's just tweak the wording, adding the cause, so that the
reason for the warning is clearer.  This hopefully stops the
user from immediately asking "'Might'?  Because why?"  and
then going off looking at the function body - or grepping
the gcc source or documentation, or enter a bug-report
subsequently closed as resolved/invalid.

gcc:
* cgraphunit.cc (process_function_and_variable_attributes): Tweak
the warning for an attribute-always_inline without inline declaration.

gcc/testsuite:
* g++.dg/Wattributes-3.C: Adjust expected warning.
* gcc.dg/fail_always_inline.c: Ditto.
* gcc.target/powerpc/vec-extract-v16qiu-v2.h: Adjust
quoted warning in comment.
---
 gcc/cgraphunit.cc| 3 ++-
 gcc/testsuite/g++.dg/Wattributes-3.C | 4 ++--
 gcc/testsuite/gcc.dg/fail_always_inline.c| 2 +-
 gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
index 38052674aaa5..89dc1af522a4 100644
--- a/gcc/cgraphunit.cc
+++ b/gcc/cgraphunit.cc
@@ -918,7 +918,8 @@ process_function_and_variable_attributes (cgraph_node 
*first,
  /* redefining extern inline function makes it DECL_UNINLINABLE.  */
  && !DECL_UNINLINABLE (decl))
warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
-   "% function might not be inlinable");
+   "% function is not always inlined"
+   " unless also declared %");
 
   process_common_attributes (node, decl);
 }
diff --git a/gcc/testsuite/g++.dg/Wattributes-3.C 
b/gcc/testsuite/g++.dg/Wattributes-3.C
index 208ec6696551..4adf0944cd4f 100644
--- a/gcc/testsuite/g++.dg/Wattributes-3.C
+++ b/gcc/testsuite/g++.dg/Wattributes-3.C
@@ -26,7 +26,7 @@ B::operator char () const { return 0; }
 
 ATTR ((__noinline__))
 B::operator int () const  // { dg-warning "ignoring attribute .noinline. 
because it conflicts with attribute .always_inline." }
-// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
+// { dg-warning "function is not always inlined unless also declared .inline." 
"" { target *-*-* } .-1 }
 
 {
   return 0;
@@ -45,7 +45,7 @@ C::operator char () { return 0; }
 
 ATTR ((__noinline__))
 C::operator short ()   // { dg-warning "ignoring attribute .noinline. 
because it conflicts with attribute .always_inline." }
-// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
+// { dg-warning "function is not always inlined unless also declared .inline." 
"" { target *-*-* } .-1 }
 { return 0; }
 
 inline ATTR ((__noinline__))
diff --git a/gcc/testsuite/gcc.dg/fail_always_inline.c 
b/gcc/testsuite/gcc.dg/fail_always_inline.c
index 86645b850de8..2f48d7f5c6be 100644
--- a/gcc/testsuite/gcc.dg/fail_always_inline.c
+++ b/gcc/testsuite/gcc.dg/fail_always_inline.c
@@ -2,7 +2,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 extern __attribute__ ((always_inline)) void
- bar() { } /* { dg-warning "function might not be inlinable" } */
+ bar() { } /* { dg-warning "function is not always inlined unless also 
declared .inline." } */
 
 void
 f()
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h 
b/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
index d1157599ee7c..b9800e1c950d 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
+++ b/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
@@ -179,7 +179,7 @@ get_auto_15 (vector TYPE a)
 #ifdef DISABLE_INLINE_OF_GET_AUTO_N
 __attribute__ ((__noinline__))
 #else
-/* gcc issues warning: always_inline function might not be inlinable
+/* gcc issues warning: always_inline function is not always inlined unless 
also declared inline
 
__attribute__ ((__always_inline__))
 */
-- 
2.30.2

Re: lambda coding style

[Hans-Peter Nilsson]

On Wed, 10 Jan 2024, Jason Merrill via Gcc wrote:
> On 1/10/24 15:59, Marek Polacek wrote:
> > On Wed, Jan 10, 2024 at 02:58:03PM -0500, Jason Merrill via Gcc wrote:
> > > What formatting style do we want for non-trivial lambdas in GCC sources?
> > > I'm thinking the most consistent choice would be
> > > 
> > > auto l = [&] (parms) // space between ] (
> > >{  // brace on new line, indented two spaces
> > >  return stuff;
> > >};
> > 
> > Sure, why not.  Consistency is what matters.  Thus far we seem
> > to have been very inconsistent.  ;)
> >   
> > > By default, recent emacs lines up the { with the previous line, like an
> > > in-class function definition; I talked it into the above indentation with
> > > 
> > > (defun lambda-offset (elem)
> > >(if (assq 'inline-open c-syntactic-context) '+ 0))
> > > (add-to-hook 'c++-mode-hook '(c-set-offset 'inlambda 'lambda-offset))
> > > 
> > > I think we probably want the same formatting for lambdas in function
> > > argument lists, e.g.
> > > 
> > > algorithm ([] (parms)
> > >{
> > >  return foo;
> > >});
> > 
> > And what about lambdas in conditions:
> > 
> > if (foo ()
> >  && [&] (params) mutable
> > {
> >  return 42;
> > } ())
> > 
> > should the { go just below [?

Also, what about trailing-type and mutable (above) when needing 
a line-break?

(FTR: in technical terms, trailing-type is known as the 
pointy-arrow-declaring-return-type thing :) the optional "-> 
type" between "(parms)" and "{ body }")

I suggest the obvious (to me): line up stuff after (params) with 
the opening brace for body, when needing a line-break before 
that item, and line-break *before* "->" .

brgds, H-P

Ping [PATCH] testsuite: Reduce gcc.dg/torture/inline-mem-cpy-1.c by 11 for simulators

[Hans-Peter Nilsson]

Ping.  (Don't miss the gcc.dg/torture/inline-mem-cpy-1.c part.)

On Mon, 1 Jan 2024, Hans-Peter Nilsson wrote:

> Tested mmix-knuth-mmixware (where all torture-variants of
> gcc.dg/torture/inline-mem-cpy-1.c now pass) and native
> x86_64-pc-linux-gnu.  Also stepped through the test for native,
> w/wo. RUN_FRACTION defined to see that it worked as intended.
> 
> You may wonder what about the "sibling" tests inline-mem-cmp-1.c and
> inline-mem-cpy-cmp-1.c.  Well, they FAIL, but not because of
> timeouts(!)  To be continued
> 
> Ok to commit?
> 
> Or, other suggestions?
> 
> -- >8 --
> The test inline-mem-cpy-1.c takes 16 minutes at -O0 for the mmix
> simulator on a 3.5 year old laptop and thus always times out, despite
> the x 2 timeout (i.e. 10 minutes), and times out at all optimization
> levels.  For the included file (when run as gcc.dg/memcmp-1.c), the
> execution time on the same host is 9 minutes 54 seconds, so just
> within 10 minutes timeout limit.  Seems pragmatically best to reduce
> the torture-test by a factor of about 10, but there's no obvious small
> set of entities to scale down to get the intended effect, and
> splitting up the test into several tests seem a bit too intrusive.
> 
> Instead, introduce pseudo-random machinery to skip all but each
> RUN_FRACTION:th iteration, defaulting to no change when RUN_FRACTION
> isn't defined.  Use 11 for RUN_FRACTION, assuming this prime will lead
> to even distribution within nested iterations with loops looking like
> (0, 1) : (0, 1).  Do this only for the main loop in
> test_driver_memcmp; the "outermost" two levels of iterations.
> 
> With this, execution time for -O0 as above is down to 1 minute 32
> seconds.
> 
>   * gcc.dg/torture/inline-mem-cpy-1.c: Pass -DRUN_FRACTION=11
>   when testing in a simulator.
>   * gcc.dg/memcmp-1.c [RUN_FRACTION]: Add machinery to run only
>   for each RUN_FRACTION:th iteration.
>   (main): Call initialize_skip_iteration_count.
>   (test_driver_memcmp): Check SKIP_ITERATION for each iteration.
> ---
>  gcc/testsuite/gcc.dg/memcmp-1.c   | 35 +++
>  .../gcc.dg/torture/inline-mem-cpy-1.c |  1 +
>  2 files changed, 36 insertions(+)
> 
> diff --git a/gcc/testsuite/gcc.dg/memcmp-1.c b/gcc/testsuite/gcc.dg/memcmp-1.c
> index ea837ca0f577..13ef5b3380d0 100644
> --- a/gcc/testsuite/gcc.dg/memcmp-1.c
> +++ b/gcc/testsuite/gcc.dg/memcmp-1.c
> @@ -34,6 +34,36 @@ int lib_strncmp(const char *a, const char *b, size_t n)
>  
>  #define MAX_SZ 600
>  
> +/* A means to run only a fraction of the tests, beginning at a random
> +   count.  */
> +#ifdef RUN_FRACTION
> +
> +#define SKIP_ITERATION skip_iteration ()
> +static unsigned int iteration_count;
> +
> +static _Bool
> +skip_iteration (void)
> +{
> +  _Bool run = ++iteration_count == RUN_FRACTION;
> +
> +  if (run)
> +iteration_count = 0;
> +
> +  return !run;
> +}
> +
> +static void
> +initialize_skip_iteration_count ()
> +{
> +  srand (2024);
> +  iteration_count = (unsigned int) (rand ()) % RUN_FRACTION;
> +}
> +
> +#else
> +#define SKIP_ITERATION 0
> +#define initialize_skip_iteration_count()
> +#endif
> +
>  #define DEF_RS(ALIGN)  \
>  static void test_memcmp_runtime_size_ ## ALIGN (const char *str1,   \
>   const char *str2,  \
> @@ -110,6 +140,8 @@ static void test_driver_memcmp (void (test_memcmp)(const 
> char *, const char *, i
>int i,j,l;
>for(l=0;l  for(i=0;i +  if (SKIP_ITERATION)
> + continue;
>for(j=0;j   buf1[j] = rand() & 0xff;
>   buf2[j] = buf1[j];
> @@ -128,6 +160,8 @@ static void test_driver_memcmp (void (test_memcmp)(const 
> char *, const char *, i
>for(diff_pos = ((test_sz>TZONE)?(test_sz-TZONE):0); diff_pos < 
> test_sz+TZONE; diff_pos++)
>  for(zero_pos = ((test_sz>TZONE)?(test_sz-TZONE):0); zero_pos < 
> test_sz+TZONE; zero_pos++)
>{
> + if (SKIP_ITERATION)
> +   continue;
>   memset(buf1, 'A', 2*test_sz);
>   memset(buf2, 'A', 2*test_sz);
>   buf2[diff_pos] = 'B';
> @@ -490,6 +524,7 @@ DEF_TEST(49,1)
>  int
>  main(int argc, char **argv)
>  {
> +  initialize_skip_iteration_count ();
>  #ifdef TEST_ALL
>  RUN_TEST(1,1)
>  RUN_TEST(1,2)
> diff --git a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c 
> b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
> index f4952554dd01..f0752349571b 100644
> --- a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
> +++ b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
> @@ -1,5 +1,6 @@
>  /* { dg-do run } */
>  /* { dg-options "-finline-stringops=memcpy -save-temps -g0 -fno-lto" } */
> +/* { dg-additional-options "-DRUN_FRACTION=11" { target simulator } } */
>  /* { dg-timeout-factor 2 } */
>  
>  #include "../memcmp-1.c"
> -- 
> 2.30.2
> 
> 

Re: breakage with: [committed] libstdc++: Implement P2909R4 ("Dude, where's my char?") for C++20

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Mon, 8 Jan 2024 17:24:35 +0100

> For some reason, this (r14-6990-g74a0dab18292be) breaks a
> build of (newlib targets) at least cris-elf and arm-eabi:

...aaand, just now fixed in r14-7007-geb846114ed7c49.
(Thanks!)

brgds, H-P

breakage with: [committed] libstdc++: Implement P2909R4 ("Dude, where's my char?") for C++20

[Hans-Peter Nilsson]

(Sorry, never a bringer of good news...)

> From: Jonathan Wakely 
> Date: Mon,  8 Jan 2024 01:15:50 +

> Tested x86_64-linux and aarch64-linux. Pushed to trunk.
> 
> -- >8 --
> 
> This change ensures that char and wchar_t arguments are formatted
> consistently when using integer presentation types. This avoids
> non-portable std::format output that depends on whether char and wchar_t
> happen to be signed or unsigned on the target. Formatting '\xff' as an
> integer will now always format 255 and not sometimes -1. This was
> approved in Kona 2023 as a DR for C++20 so the change is implemented
> unconditionally.
> 
> Also make character formatters check for _Pres_c explicitly and call
> _M_format_character directly. This avoid the overhead of calling format
> and _S_to_character and then calling _M_format_character anyway.
> 
> libstdc++-v3/ChangeLog:
> 
>   * include/bits/version.def (format_uchar): Define.
>   * include/bits/version.h: Regenerate.
>   * include/std/format (formatter::format): Check for
>   _Pres_c and call _M_format_character directly. Cast C to its
>   unsigned equivalent for formatting as an integer.
>   (formatter::format): Likewise.
>   (basic_format_arg(T&)): Store char arguments as unsigned char
>   for formatting to a wide string.
>   * testsuite/std/format/functions/format.cc: Adjust test. Check
>   formatting of

For some reason, this (r14-6990-g74a0dab18292be) breaks a
build of (newlib targets) at least cris-elf and arm-eabi:

libtool: compile:  /obj/./gcc/xgcc -shared-libgcc -B/obj/./gcc -nostdinc++ 
-L/obj/cris-elf/libstdc++-v3/src -L/obj/cris-elf/libstdc++-v3/src/.libs 
-L/obj/cris-elf/libstdc++-v3/libsupc++/.libs -nostdinc -B/obj/cris-elf/newlib/ 
-isystem /obj/cris-elf/newlib/targ-include -isystem /x/gcc/newlib/libc/include 
-B/obj/cris-elf/libgloss/cris -L/obj/cris-elf/libgloss/libnosys 
-L/x/gcc/libgloss/cris -B/x/cris-elf/pre/cris-elf/bin/ 
-B/x/cris-elf/pre/cris-elf/lib/ -isystem /x/cris-elf/pre/cris-elf/include 
-isystem /x/cris-elf/pre/cris-elf/sys-include -I/x/gcc/libstdc++-v3/../libgcc 
-I/obj/cris-elf/libstdc++-v3/include/cris-elf 
-I/obj/cris-elf/libstdc++-v3/include -I/x/gcc/libstdc++-v3/libsupc++ 
-std=gnu++20 -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual 
-Wabi=2 -fdiagnostics-show-location=once -ffunction-sections -fdata-sections 
-frandom-seed=tzdb.lo -fimplicit-templates -g -O2 -I. -c 
/x/gcc/libstdc++-v3/src/c++20/tzdb.cc -o tzdb.o
In file included from /x/gcc/newlib/libc/include/time.h:11,
 from /obj/cris-elf/libstdc++-v3/include/ctime:42,
 from /obj/cris-elf/libstdc++-v3/include/bits/chrono.h:40,
 from /obj/cris-elf/libstdc++-v3/include/chrono:41,
 from /x/gcc/libstdc++-v3/src/c++20/tzdb.cc:31:
/obj/cris-elf/libstdc++-v3/include/bits/unicode.h:86:37: error: declaration 
does not declare anything [-fpermissive]
   86 |   inline constexpr _Null_sentinel_t __null_sentinel;
  | ^~~
make[5]: *** [Makefile:754: tzdb.lo] Error 1

I don't see anything immediately related to that line in the
patch, though, so the actual cause and fix isn't obvious, at
least to me.

brgds, H-P

Re: Problems with strub tests

[Hans-Peter Nilsson]

On Tue, 19 Dec 2023, Jeff Law wrote:
> 
> So the strub tests in c-c++-common are problematical.  They get run twice,
> once for C, once for C++.  Yet the name of the test is the same in both runs.
> (by the name, I mean the name emitted into the dejagnu summary and log files).
> 
> Thus if you have a test in there which passes in one context, but fails in the
> other, comparison tools like contrib/compare_tests may erroneously report the
> tests as both a test which now fails, but passed before and a test which now
> passes but failed before.
> 
> It looks like some of the strub tests are currently known to fail with C++ and
> are triggering this problem
> 
> 
> Ideally we'd include the c or c++ in the test name depending on which context
> its being run within.  That would be sufficient to resolve these problems and
> avoid them in the future.  It would also be sufficient to get all the tests to
> the point where their behavior is the same for both languages.
> 
> Not sure if the latter is reasonably in the cards or not.  If it's not likely
> to land soon, any change you could look at the framework for c-c++-common and
> get the names unique across the two times they're run?
> 
> A third option would be to change the compare_tests tool to somehow
> distinguish between the C and C++ tests.  Not sure how feasible that is.

How about including the name of the .sum file in the key?

(They're gcc.sum and g++.sum thus different.  This is what 
contrib/regression/btest-gcc.sh does.  On the other hand, that 
prunes the name of the test at the first space.  Don't copy 
that bit. :)

Also not sure how feasible that is.

brgds, H-P

Re: Generalizing DejaGnu timeout scaling (was: Re: [PATCH DejaGNU/GCC 0/1] Support per-test execution timeout factor)

[Hans-Peter Nilsson]

On Wed, 3 Jan 2024, Jacob Bachmeyer wrote:
> Comments before I start on an implementation?

I'd suggest to await the conclusion of the debate: I *think* 
I've proved that dg-timeout-factor is already active as intended 
(all parts of a test), specifically when the compilation result 
is executed (for the applicable tests).  Notably, modulo bugs in 
the test-suites.

Of course, it may be useful to separate different timeouts of 
separable parts of a test - compilation and execution being the 
topic at hand.  But IMHO, YAGNI.  Having said that, don't let 
that stand in the way of a fun hack!

brgds, H-P

Re: [PATCH DejaGNU/GCC 0/1] Support per-test execution timeout factor

[Hans-Peter Nilsson]

On Wed, 3 Jan 2024, Maciej W. Rozycki wrote:

> On Wed, 3 Jan 2024, Hans-Peter Nilsson wrote:
> 
> > >  The test execution timeout is different from the tool execution timeout 
> > > where it is GCC execution that is being guarded against taking excessive 
> > > amount of time on the test host rather than the resulting test case 
> > > executable run on the target afterwards, as concerned here.  GCC already 
> > > has a `dg-timeout-factor' setting for the tool execution timeout, but has 
> > > no means to increase the test execution timeout.  The GCC side of these 
> > > changes adds a corresponding `dg-test-timeout-factor' setting.
> > 
> > Hmm.  I think it would be more correct to emphasize that the 
> > existing dg-timeout-factor affects both the tool execution *and* 
> > the test execution, whereas your new dg-test-timeout-factor only 
> > affects the test execution.  (And still measured on the host.)
> 
>  Not really, `dg-timeout-factor' is only applied to tool execution and it 
> doesn't affect test execution.

Let's stop here: that statement is just incorrect.

There might be a use for separating timeouts, but the premise of 
dg-timeout-factor not affecting the execution of an (executable) 
test is plain wrong.  Something is off here.  Are we using the 
same terminology?

Please revisit the setup where the patch made a difference and 
report it for others to repeat, something like the following:
(Beware of typos, I didn't copy-paste like I usually do.)

A recent observation is me testing MMIX, where gcc+newlib is 
build and with --prefix and $PATH pointing at pre-installed 
binutils and simulator.  I test it all with "make -k check 
--target_board=mmixware-sim".  (This should all be familiar; 
pick another target and baseboard or use a native+unix.exp if 
you prefer.  Also JFTR I usually do it by means of 
contrib/regression/btest-gcc.sh - except of course when 
inspecting and manual testing.)

Anyway a repeatable case where dg-timeout-factor then makes a 
difference for the timeout is for libstdc++-v3 test-case 
20_util/hash/quality.cc.  I recently committed a patch adding 
dg-timeout-factor 3 for that test (26fe2808d8).  Let's consider 
the situation *before* that commit.

For the mmix simulator and the particular host where I ran that 
test, the test normally executes in very close to 6 minutes, and 
as the default timeout of 360 seconds, it sometimes times out 
when the machine is busy.  To make *sure* it times out for case 
of proof here, I edit the -DNTESTS=1 simulator setting to 
-DNTESTS=2.  I execute just this test by for example "make 
check-target-libstdc++-v3 
RUNTESTFLAGS=--target_board=mmixware-sim\ 
conformance.exp=quality.cc" (beware of quoting issues - which 
should be familiar to you).

That NTESTS=2 makes the execution time go up to 13 minutes 
elapsed time and the test gets a "WARNING: program timed out" 
and a failure for that test.  I also see a (timeout = 360) in 
the libstdc++.log - admittedly for the compilation line, but 
the timeout is consistent with being applied to the execution 
as well.

Then, apply the commit, which adds a line with dg-timeout-factor 3
(bah, I had to do it manually because of that edited -DNTESTS=2 line).

After, when I un the same command-line, the test *does not time 
out, it passes* and I see a (timeout = 1080) next to the 
compilation line in the .log - but it's apparently applied to 
the test run as well.

That, as well as numerous previous commits, is consistent with 
dg-timeout-factor affecting the execution time, not just the 
compilation time.

Of course, there may be some sub-test-suite that has a bug. I'm 
*guessing* you misinterpret observations that lead up to this 
patch-set, perhaps a bug in some sub-test.exp.

>  Timeout value reporting used to be limited 
> in DejaGNU, but you can enable it easily now by adding the DejaGNU patch 
> series referred in the cover letter and see that `dg-timeout-factor' is 
> ignored for test execution.

Please state a case where I can observe it being ignored.

> > Usually the compilation time is close to 0, so is this based on 
> > an actual need more than an itchy "wart"?
> > 
> > Or did I miss something?
> 
>  Compilation is usually quite fast, but this is not always the case.  If 
> you look at the tests that do use `dg-timeout-factor' in GCC, and some 
> commits that added the setting, then you ought to find actual use cases.  

I've not only looked at such commits, I've done quite a few 
myself.  I'd say most such commits are for test execution, some 
are for compilation.  Did you miss the ones where the commit log 
mentions "slow simulator" or "slow board"?

> I saw at least one such a test that takes an awful lot of time here on a 
> reasonably fast host machine and still 

Re: [PATCH] libstdc++: testsuite: reduce max_size_type.cc exec time [PR113175]

[Hans-Peter Nilsson]

> From: Patrick Palka 
> Date: Tue,  2 Jan 2024 12:48:26 -0500

> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and release
> branches (r14-205 was backported everywhere)?
> 
> -- >8 --
> 
> The adjustment to max_size_type.cc in r14-205-g83470a5cd4c3d2
> inadvertently increased the execution time of the test by over 5x due to
> enabling the two main loops to actually run in the signed_p case instead
> of being dead code.  This suggests that the current range of the loop is
> far too big and the test too time consuming, especially when run on
> simulators.
> 
> So this patch cuts the loop range by 10x as proposed in the PR.  This
> shouldn't significantly weaken the test since the same important edge
> cases are still checked in the new range.  On my x86_64 machine this
> reduces the test execution time by 10x, and 1.6x less time than before
> r14-205.
> 
>   PR testsuite/113175
> 
> libstdc++-v3/ChangeLog:
> 
>   * testsuite/std/ranges/iota/max_size_type.cc (test02): Reduce
>   'limit' to 100 from 1000 and adjust 'log2_limit' accordingly.
>   (test03): Likewise.


Oh the irony...  This now fails for cris-elf.  For CRIS,
it's not a timeout but an actual failure.

See PR113226; suddenly 1*-100 == 4294967196.  You changed
the type of "limit" to unsigned, but that doesn't appear to
matter.  So why would *narrowing* the tested range yield an
error?

I can't tell if this is target-specific: not enough
32-bitters results for r14-6888-ga138b99646a555 and later on
gcc-testresults@ for libstdc++ yet.  (No, I don't count the
32-bit multilibs of x86_64 and s390x.)

brgds, H-P

Re: [PATCH DejaGNU/GCC 0/1] Support per-test execution timeout factor

[Hans-Peter Nilsson]

On Tue, 12 Dec 2023, Maciej W. Rozycki wrote:

> Hi,
> 
>  This patch quasi-series makes it possible for individual test cases 
> identified as being slow to request more time via the GCC test harness by 
> providing a test execution timeout factor, applied to the tool execution 
> timeout set globally for all the test cases.  This is to avoid excessive 
> testsuite run times where other test cases do hang as it would be the 
> case if the timeout set globally was to be increased.
> 
>  The test execution timeout is different from the tool execution timeout 
> where it is GCC execution that is being guarded against taking excessive 
> amount of time on the test host rather than the resulting test case 
> executable run on the target afterwards, as concerned here.  GCC already 
> has a `dg-timeout-factor' setting for the tool execution timeout, but has 
> no means to increase the test execution timeout.  The GCC side of these 
> changes adds a corresponding `dg-test-timeout-factor' setting.

Hmm.  I think it would be more correct to emphasize that the 
existing dg-timeout-factor affects both the tool execution *and* 
the test execution, whereas your new dg-test-timeout-factor only 
affects the test execution.  (And still measured on the host.)

Usually the compilation time is close to 0, so is this based on 
an actual need more than an itchy "wart"?

Or did I miss something?

brgds, H-P

Re: [PATCH] testsuite: Reduce gcc.dg/torture/inline-mem-cpy-1.c by 11 for simulators

[Hans-Peter Nilsson]

On Tue, 2 Jan 2024, Jeff Law wrote:
> 
> On 1/1/24 20:22, Hans-Peter Nilsson wrote:
> > Tested mmix-knuth-mmixware (where all torture-variants of
> > gcc.dg/torture/inline-mem-cpy-1.c now pass) and native
> > x86_64-pc-linux-gnu.  Also stepped through the test for native,
> > w/wo. RUN_FRACTION defined to see that it worked as intended.
> > 
> > You may wonder what about the "sibling" tests inline-mem-cmp-1.c and
> > inline-mem-cpy-cmp-1.c.  Well, they FAIL, but not because of
> > timeouts(!)  To be continued
> > 
> > Ok to commit?
> > 
> > Or, other suggestions?
> I'm pretty sure there's already a target selector for "simulator"  So you
> might be able to do this automagically with somethign like
> 
> dg-additional-options "-DRUN_FRACTION=11" { target { simulator } }"
> 
> Or something close to that.

Hm...  But that's exactly what the one-line patch to 
gcc.dg/torture/inline-mem-cpy-1.c looked like, last in the 
submitted commit.  I had to double-check my sent-mail folder 
that I didn't miss that part. :)

I'm mostly worried about the patch to gcc.dg/memcpy-1.c.
Does that mean all-ok?

brgds, H-P

[PATCH] testsuite: Reduce gcc.dg/torture/inline-mem-cpy-1.c by 11 for simulators

[Hans-Peter Nilsson]

Tested mmix-knuth-mmixware (where all torture-variants of
gcc.dg/torture/inline-mem-cpy-1.c now pass) and native
x86_64-pc-linux-gnu.  Also stepped through the test for native,
w/wo. RUN_FRACTION defined to see that it worked as intended.

You may wonder what about the "sibling" tests inline-mem-cmp-1.c and
inline-mem-cpy-cmp-1.c.  Well, they FAIL, but not because of
timeouts(!)  To be continued

Ok to commit?

Or, other suggestions?

-- >8 --
The test inline-mem-cpy-1.c takes 16 minutes at -O0 for the mmix
simulator on a 3.5 year old laptop and thus always times out, despite
the x 2 timeout (i.e. 10 minutes), and times out at all optimization
levels.  For the included file (when run as gcc.dg/memcmp-1.c), the
execution time on the same host is 9 minutes 54 seconds, so just
within 10 minutes timeout limit.  Seems pragmatically best to reduce
the torture-test by a factor of about 10, but there's no obvious small
set of entities to scale down to get the intended effect, and
splitting up the test into several tests seem a bit too intrusive.

Instead, introduce pseudo-random machinery to skip all but each
RUN_FRACTION:th iteration, defaulting to no change when RUN_FRACTION
isn't defined.  Use 11 for RUN_FRACTION, assuming this prime will lead
to even distribution within nested iterations with loops looking like
(0, 1) : (0, 1).  Do this only for the main loop in
test_driver_memcmp; the "outermost" two levels of iterations.

With this, execution time for -O0 as above is down to 1 minute 32
seconds.

* gcc.dg/torture/inline-mem-cpy-1.c: Pass -DRUN_FRACTION=11
when testing in a simulator.
* gcc.dg/memcmp-1.c [RUN_FRACTION]: Add machinery to run only
for each RUN_FRACTION:th iteration.
(main): Call initialize_skip_iteration_count.
(test_driver_memcmp): Check SKIP_ITERATION for each iteration.
---
 gcc/testsuite/gcc.dg/memcmp-1.c   | 35 +++
 .../gcc.dg/torture/inline-mem-cpy-1.c |  1 +
 2 files changed, 36 insertions(+)

diff --git a/gcc/testsuite/gcc.dg/memcmp-1.c b/gcc/testsuite/gcc.dg/memcmp-1.c
index ea837ca0f577..13ef5b3380d0 100644
--- a/gcc/testsuite/gcc.dg/memcmp-1.c
+++ b/gcc/testsuite/gcc.dg/memcmp-1.c
@@ -34,6 +34,36 @@ int lib_strncmp(const char *a, const char *b, size_t n)
 
 #define MAX_SZ 600
 
+/* A means to run only a fraction of the tests, beginning at a random
+   count.  */
+#ifdef RUN_FRACTION
+
+#define SKIP_ITERATION skip_iteration ()
+static unsigned int iteration_count;
+
+static _Bool
+skip_iteration (void)
+{
+  _Bool run = ++iteration_count == RUN_FRACTION;
+
+  if (run)
+iteration_count = 0;
+
+  return !run;
+}
+
+static void
+initialize_skip_iteration_count ()
+{
+  srand (2024);
+  iteration_count = (unsigned int) (rand ()) % RUN_FRACTION;
+}
+
+#else
+#define SKIP_ITERATION 0
+#define initialize_skip_iteration_count()
+#endif
+
 #define DEF_RS(ALIGN)  \
 static void test_memcmp_runtime_size_ ## ALIGN (const char *str1, \
const char *str2,  \
@@ -110,6 +140,8 @@ static void test_driver_memcmp (void (test_memcmp)(const 
char *, const char *, i
   int i,j,l;
   for(l=0;lTZONE)?(test_sz-TZONE):0); diff_pos < 
test_sz+TZONE; diff_pos++)
 for(zero_pos = ((test_sz>TZONE)?(test_sz-TZONE):0); zero_pos < 
test_sz+TZONE; zero_pos++)
   {
+   if (SKIP_ITERATION)
+ continue;
memset(buf1, 'A', 2*test_sz);
memset(buf2, 'A', 2*test_sz);
buf2[diff_pos] = 'B';
@@ -490,6 +524,7 @@ DEF_TEST(49,1)
 int
 main(int argc, char **argv)
 {
+  initialize_skip_iteration_count ();
 #ifdef TEST_ALL
 RUN_TEST(1,1)
 RUN_TEST(1,2)
diff --git a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c 
b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
index f4952554dd01..f0752349571b 100644
--- a/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
+++ b/gcc/testsuite/gcc.dg/torture/inline-mem-cpy-1.c
@@ -1,5 +1,6 @@
 /* { dg-do run } */
 /* { dg-options "-finline-stringops=memcpy -save-temps -g0 -fno-lto" } */
+/* { dg-additional-options "-DRUN_FRACTION=11" { target simulator } } */
 /* { dg-timeout-factor 2 } */
 
 #include "../memcmp-1.c"
-- 
2.30.2

Re: [PATCH] libstdc++ testsuite/std/ranges/iota/max_size_type.cc: Reduce /10 for simulators

[Hans-Peter Nilsson]

On Sat, 30 Dec 2023, Hans-Peter Nilsson wrote:

> On Sat, 30 Dec 2023, Jonathan Wakely wrote:
> 
> > On Sat, 30 Dec 2023, 01:41 Hans-Peter Nilsson,  wrote:
> > > Or perhaps the cause is known?
> > 
> > Not to me. It probably is a target codegen bug, since all this test really
> > does is emulate a wide integer type using masks and shifts.
> 
> If so, a generic code-generator bug.  I've repeated the 5x 
> performance regression observation for a native build and 
> updated PR113175 (.32 vs 1.73 seconds).  I'll see if I can 
> quickly find out whether it's codegen or libstdc++.  I set it 
> the PR to the latter for the moment.

Now changed to /testsuite, because the apparent cause for the 
magnitude increase (.32 -> 1.63 seconds) is the testsuite part 
of r14-205.  See the PR for details.

HNY!

> 
> > > With this, the test successfully completes in ~34 seconds.
> > >
> > > Ok to commit?
> > >
> > 
> > Looks OK to me, but Patrick wrote this test so please wait for him to
> > confirm. I think this just reduces the number of cases tested, but doesn't
> > miss any important edge cases that should be checked.
> 
> Understood: holding, but will ping after the usual week.  
> Thanks for the review!
> 
> brgds, H-P
> 

Re: [PATCH] libstdc++ testsuite/std/ranges/iota/max_size_type.cc: Reduce /10 for simulators

[Hans-Peter Nilsson]

On Sat, 30 Dec 2023, Jonathan Wakely wrote:

> On Sat, 30 Dec 2023, 01:41 Hans-Peter Nilsson,  wrote:
> > Or perhaps the cause is known?
> 
> Not to me. It probably is a target codegen bug, since all this test really
> does is emulate a wide integer type using masks and shifts.

If so, a generic code-generator bug.  I've repeated the 5x 
performance regression observation for a native build and 
updated PR113175 (.32 vs 1.73 seconds).  I'll see if I can 
quickly find out whether it's codegen or libstdc++.  I set it 
the PR to the latter for the moment.

> > With this, the test successfully completes in ~34 seconds.
> >
> > Ok to commit?
> >
> 
> Looks OK to me, but Patrick wrote this test so please wait for him to
> confirm. I think this just reduces the number of cases tested, but doesn't
> miss any important edge cases that should be checked.

Understood: holding, but will ping after the usual week.  
Thanks for the review!

brgds, H-P

[PATCH] libstdc++ testsuite/std/ranges/iota/max_size_type.cc: Reduce /10 for simulators

[Hans-Peter Nilsson]

I'm not completely sure I got the intent of the "log2_limit",
or whether "limit" is sane to decrease like this; it just
looked like an obvious and safe reduction.  Also, I verified
the 10+ minute runtime, on this same host (clocked at 11:43.61
elapsed time) for a r12-2797-g307e0d40367996 build that I
happened to have kept around; likely the build that led up
to that commit.  Now it's 58:45.78 elapsed time for a
successful run.  Looks like a 5x performance regression.
Worrisome; PR mentioned below.

Incidentally, a parallel build and a serial test-run takes 9
hours on that laptop, so that's almost 2 hours just for one 
test, if just updating the timeout to fit.  IOW, currently 48
minutes out of 9 hours for one test that just times out.

(That was just mentioned for comparison purposed: when suitable,
I test with `nprocs`-1 in parallel.)

I'll put it on the back-burner to investigate.  I think I'll
try to graft that version of libstdc++-v3 to this version
and see if I can shift the blame away from MMIX code
generation onto libstdc++-v3.  ;)
Or perhaps the cause is known?

With this, the test successfully completes in ~34 seconds.

Ok to commit?

-- >8 --
Looks like the MMIX port code quality and/or libstdc++
performance of this test has regressed since
r12-2799-ge9b639c4b53221 by a factor 5.  Anyway what was 11+
minutes runtime then, is now at r14-6859-gd1eacedc6d9ba9
close to 60 minutes.  Better prune the test, not just
increase timeouts.  Also of course, investigate the
performance regression, logged as PR113175.

* testsuite/std/ranges/iota/max_size_type.cc: Adjust
limits from -1000..1000 to -100..100 for simulators.
---
 .../std/ranges/iota/max_size_type.cc  | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc 
b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc
index a1fbc3241dca..38fa6323d47e 100644
--- a/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc
+++ b/libstdc++-v3/testsuite/std/ranges/iota/max_size_type.cc
@@ -16,6 +16,7 @@
 // .
 
 // { dg-do run { target c++20 } }
+// { dg-additional-options "-DSIMULATOR_TEST" { target simulator } }
 // { dg-timeout-factor 4 }
 
 #include 
@@ -31,6 +32,14 @@ using signed_rep_t = __int128;
 using signed_rep_t = long long;
 #endif
 
+#ifdef SIMULATOR_TEST
+#define LIMIT 100
+#define LOG2_CEIL_LIMIT 7
+#else
+#define LIMIT 1000
+#define LOG2_CEIL_LIMIT 10
+#endif
+
 static_assert(sizeof(max_size_t) == sizeof(max_diff_t));
 static_assert(sizeof(rep_t) == sizeof(signed_rep_t));
 
@@ -199,8 +208,8 @@ test02()
   using max_type = std::conditional_t;
   using shorten_type = std::conditional_t;
   const int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__;
-  const int limit = 1000;
-  const int log2_limit = 10;
+  const int limit = LIMIT;
+  const int log2_limit = LOG2_CEIL_LIMIT;
   static_assert((1 << log2_limit) >= limit);
   const int min = (signed_p ? -limit : 0);
   const int max = limit;
@@ -257,8 +266,8 @@ test03()
   using max_type = std::conditional_t;
   using base_type = std::conditional_t;
   constexpr int hw_type_bit_size = sizeof(hw_type) * __CHAR_BIT__;
-  constexpr int limit = 1000;
-  constexpr int log2_limit = 10;
+  constexpr int limit = LIMIT;
+  constexpr int log2_limit = LOG2_CEIL_LIMIT;
   static_assert((1 << log2_limit) >= limit);
   const int min = (signed_p ? -limit : 0);
   const int max = limit;
@@ -312,7 +321,7 @@ test03()
 void
 test04()
 {
-  constexpr int limit = 1000;
+  constexpr int limit = LIMIT;
   for (int i = -limit; i <= limit; i++)
 {
   VERIFY( -max_size_t(-i) == i );
-- 
2.30.2

[PATCH] libstdc++ testsuite/20_util/hash/quality.cc: Increase timeout 3x

[Hans-Peter Nilsson]

Tested for mmix and observing the increased timeout in the .log 
file - and the test passing.

Ok to commit?  Or better suggestions?

-- >8 --
Testing for mmix (a 64-bit target using Knuth's simulator).  The test
is largely pruned for simulators, but still needs 5m57s on my laptop
from 3.5 years ago to run to successful completion.  Perhaps slow
hosted targets could also have problems so increasing the timeout
limit, not just for simulators but for everyone, and by more than a
factor 2.

* testsuite/20_util/hash/quality.cc: Increase timeout by a factor 3.
---
 libstdc++-v3/testsuite/20_util/hash/quality.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libstdc++-v3/testsuite/20_util/hash/quality.cc 
b/libstdc++-v3/testsuite/20_util/hash/quality.cc
index 7d4208ed6d21..80efc026 100644
--- a/libstdc++-v3/testsuite/20_util/hash/quality.cc
+++ b/libstdc++-v3/testsuite/20_util/hash/quality.cc
@@ -1,5 +1,6 @@
 // { dg-options "-DNTESTS=1 -DNSTRINGS=100 -DSTRSIZE=21" { target simulator } }
 // { dg-do run { target c++11 } }
+// { dg-timeout-factor 3 }
 
 // Copyright (C) 2010-2023 Free Software Foundation, Inc.
 //
-- 
2.30.2

[committed] CRIS: Fix PR middle-end/113109; "throw" failing

[Hans-Peter Nilsson]

No test-case, but the regress-367 from r14-6674-g4759383245ac97 is
"back" to regress-10 for cris-elf+cris-sim with this patch applied
to gcc from that revision.

Also, I wonder why none of those other targets with a MEM for
EH_RETURN_HANDLER_RTX with an address relative to
frame_pointer_rtx (as opposed to hard_frame_pointer_rtx or
virtual_incoming_args_rtx) don't see the same problem.

Oh well.  Merry Xmas.

brgds, H-P

-- >8 --
TL;DR: the "dse1" pass removed the eh-return-address store.  The
PA also marks its EH_RETURN_HANDLER_RTX as volatile, for the same
reason, as does visum.  See PR32769 - it's the same thing on PA.

Conceptually, it's logical that stores to incoming args are
optimized out on the return path or if no loads are seen -
at least before epilogue expansion, when the subsequent load
isn't seen in the RTL, as is the case for the "dse1" pass.

I haven't looked into why this problem, that appeared for the PA
already in 2007, was seen for CRIS only recently (with
r14-6674-g4759383245ac97).

PR middle-end/113109
* config/cris/cris.cc (cris_eh_return_handler_rtx): New function.
* config/cris/cris-protos.h (cris_eh_return_handler_rtx): Prototype.
* config/cris/cris.h (EH_RETURN_HANDLER_RTX): Redefine to call
cris_eh_return_handler_rtx.
---
 gcc/config/cris/cris-protos.h |  1 +
 gcc/config/cris/cris.cc   | 16 
 gcc/config/cris/cris.h|  3 +--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gcc/config/cris/cris-protos.h b/gcc/config/cris/cris-protos.h
index 666e04f9eeec..06678c723b56 100644
--- a/gcc/config/cris/cris-protos.h
+++ b/gcc/config/cris/cris-protos.h
@@ -28,6 +28,7 @@ extern bool cris_reload_address_legitimized (rtx, 
machine_mode, int, int, int);
 extern int cris_side_effect_mode_ok (enum rtx_code, rtx *, int, int,
  int, int, int);
 extern rtx cris_return_addr_rtx (int, rtx);
+extern rtx cris_eh_return_handler_rtx ();
 extern rtx cris_split_movdx (rtx *);
 extern bool cris_base_p (const_rtx, bool);
 extern bool cris_base_or_autoincr_p (const_rtx, bool);
diff --git a/gcc/config/cris/cris.cc b/gcc/config/cris/cris.cc
index 7705c25ed6c0..38a4dd29114d 100644
--- a/gcc/config/cris/cris.cc
+++ b/gcc/config/cris/cris.cc
@@ -1382,6 +1382,22 @@ cris_return_addr_rtx (int count, rtx frameaddr 
ATTRIBUTE_UNUSED)
 : NULL_RTX;
 }
 
+/* Setting the EH return return address is done by a *store* to a memory
+   address expressed as relative to "*incoming* args".  That store will
+   be optimized away, unless the MEM is marked as volatile.  N.B.: no
+   optimization opportunities are expected to be lost due to this hack;
+   __builtin_eh_return isn't called from elsewhere than the EH machinery
+   in libgcc.  */
+
+rtx
+cris_eh_return_handler_rtx ()
+{
+  rtx ret = cris_return_addr_rtx (0, NULL_RTX);
+  gcc_assert (MEM_P (ret));
+  MEM_VOLATILE_P (ret) = true;
+  return ret;
+}
+
 /* Accessor used in cris.md:return because cfun->machine isn't available
there.  */
 
diff --git a/gcc/config/cris/cris.h b/gcc/config/cris/cris.h
index 087b226ee475..ced356088302 100644
--- a/gcc/config/cris/cris.h
+++ b/gcc/config/cris/cris.h
@@ -551,8 +551,7 @@ enum reg_class
 #define CRIS_STACKADJ_REG CRIS_STRUCT_VALUE_REGNUM
 #define EH_RETURN_STACKADJ_RTX gen_rtx_REG (SImode, CRIS_STACKADJ_REG)
 
-#define EH_RETURN_HANDLER_RTX \
-  cris_return_addr_rtx (0, NULL)
+#define EH_RETURN_HANDLER_RTX cris_eh_return_handler_rtx ()
 
 #define INIT_EXPANDERS cris_init_expanders ()
 
-- 
2.30.2

Re: [PATCH] treat argp-based mem as frame related in dse

[Hans-Peter Nilsson]

> From: Jiufu Guo 
> Date: Wed,  6 Dec 2023 17:27:58 +0800

> Hi,
> 
> The issue mentioned in PR112525 would be able to be handled by
>  
> updating dse.cc to treat arg_pointer_rtx similarly with frame_pointer_rtx.
>  
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30271#c10 also mentioned 
>  
> this idea.   
> 
> One thing, arpg area may be used to pass argument to callee. So, it would 
>
> be needed to check if call insns are using that mem.
> 
> Bootstrap  pass on ppc64{,le} and x86_64.
> Is this ok for trunk?
> 
> BR,
> Jeff (Jiufu Guo)
> 
> 
>   PR rtl-optimization/112525
> 
> gcc/ChangeLog:
> 
>   * dse.cc (get_group_info): Add arg_pointer_rtx as frame_related.
>   (check_mem_read_rtx): Add parameter to indicate if it is checking mem
>   for call insn.
>   (scan_insn): Add mem checking on call usage.

This, when committed as r14-6674-g4759383245ac97, caused all
or most test that "throw" to fail for cris-elf at execution
time, but I don't see other targets failing (cf. gcc-testresults
archives).  I opened PR113109 and will dig a little deeper.

brgds, H-P

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> Date: Mon, 4 Dec 2023 12:58:03 +0100 (CET)
> From: Richard Biener 

> On Sat, 2 Dec 2023, Hans-Peter Nilsson wrote:
> > > Date: Fri, 1 Dec 2023 08:07:14 +0100 (CET)
> > > From: Richard Biener 
> > > I read from your messages that the testcases pass on arm*-*-*?
> > Yes: they pass (currently XPASS) on arm-eabi and
> > arm-unknown-linux-gnueabi, default configurations.  But,
> > scev-3 and -5 fail with for example -mcpu=cortex-r5
> 
> I see.  As said, the testcases test for "cost" things, so that we
> "regressed" might mean we really "regressed" here.  Even the x86 -m32
> result is questionable.
> 
> Of course whether using a single IV makes sense for all archs is
> unknown.
> 
> Btw, if we turn the testcases into ones that are (sub-)target
> specific then we want to again use C code as input.
> 
> I think at this point we've lost track and I'm juggling between
> removing the testcases or moving them to a place they succeed
> (with some specific -mcpu=?)
> 
> Richard.

So to not drop the ball(s) on this, here's a patch with your
first alternative: remove them.

Ok?

-- >8 --
Subject: [PATCH] testsuite: Remove gcc.dg/tree-ssa/scev-3.c -4.c and 5.c

These tests were recently xfailed on ilp32 targets though
passing on almost all ilp32 targets (known exceptions: ia32
and some arm subtargets).  They've been changed around too
much to remain useful.

PR testsuite/112786
* gcc.dg/tree-ssa/scev-3.c, gcc.dg/tree-ssa/scev-4.c,
gcc.dg/tree-ssa/scev-5.c: Remove.
---
 gcc/testsuite/gcc.dg/tree-ssa/scev-3.c | 44 ---
 gcc/testsuite/gcc.dg/tree-ssa/scev-4.c | 49 --
 gcc/testsuite/gcc.dg/tree-ssa/scev-5.c | 44 ---
 3 files changed, 137 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
 delete mode 100644 gcc/testsuite/gcc.dg/tree-ssa/scev-5.c

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
deleted file mode 100644
index beea9aed9fe9..
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
+++ /dev/null
@@ -1,44 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -fgimple -fdump-tree-ivopts" } */
-
-int *a_p;
-int a[1000];
-
-void __GIMPLE (ssa,startwith ("loop"))
-f (int k)
-{
-  int i;
-  int * _1;
-
-__BB(2):
-  i_5 = k_4(D);
-  if (i_5 <= 999)
-goto __BB4;
-  else
-goto __BB3;
-
-__BB(3):
-  return;
-
-__BB(4):
-  goto __BB5;
-
-__BB(5):
-  i_12 = __PHI (__BB6: i_9, __BB4: i_5);
-  _1 = [i_12];
-  a_p = _1;
-  __MEM  ((int *))[i_12] = 100;
-  i_9 = i_5 + i_12;
-  if (i_9 <= 999)
-goto __BB6;
-  else
-goto __BB3;
-
-__BB(6):
-  ;
-  goto __BB5;
-
-}
-
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
deleted file mode 100644
index a97f75f81f65..
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -fgimple -fdump-tree-ivopts" } */
-
-typedef struct {
-int x;
-int y;
-} S;
-
-int *a_p;
-S a[1000];
-
-void __GIMPLE (ssa, startwith ("loop"))
-f (int k)
-{
-  int i;
-  int * _1;
-
-__BB(2):
-  i_5 = k_4(D);
-  if (i_5 <= 999)
-goto __BB4;
-  else
-goto __BB3;
-
-__BB(3):
-  return;
-
-__BB(4):
-  goto __BB5;
-
-__BB(5):
-  i_12 = __PHI (__BB6: i_9, __BB4: i_5);
-  _1 = [i_12].y;
-  a_p = _1;
-  __MEM  ((int *))[i_12].y = 100;
-  i_9 = i_5 + i_12;
-  if (i_9 <= 999)
-goto __BB6;
-  else
-goto __BB3;
-
-__BB(6):
-  ;
-  goto __BB5;
-
-}
-
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
deleted file mode 100644
index 08f4260403c4..
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
+++ /dev/null
@@ -1,44 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -fgimple -fdump-tree-ivopts" } */
-
-int *a_p;
-int a[1000];
-
-void __GIMPLE (ssa,startwith ("loop"))
-f (int k)
-{
-  long long int i;
-  int * _1;
-
-__BB(2):
-  i_5 = (long long int) k_4(D);
-  if (i_5 <= 999ll)
-goto __BB4;
-  else
-goto __BB3;
-
-__BB(3):
-  return;
-
-__BB(4):
-  goto __BB5;
-
-__BB(5):
-  i_12 = __PHI (__BB6: i_9, __BB4: i_5);
-  _1 = [i_12];
-  a_p = _1;
-  __MEM  ((int *))[i_12] = 100;
-  i_9 = i_5 + i_12;
-  if (i_9 <= 999ll)
-goto __BB6;
-  else
-goto __BB3;
-
-__BB(6):
-  ;
-  goto __BB5;
-
-}
-
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
-- 
2.30.2

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> Date: Fri, 1 Dec 2023 08:07:14 +0100 (CET)
> From: Richard Biener 

> On Fri, 1 Dec 2023, Hans-Peter Nilsson wrote:
> 
> > > From: Hans-Peter Nilsson 
> > > Date: Thu, 30 Nov 2023 18:09:10 +0100
> > 
> > Richard B.:
> > > > > In the end we might need to move/duplicate the test to some
> > > > > gcc.target/* dir and restrict it to a specific tuning.
> > > 
> > > I intend to post two alternative patches to get this
> > > resolved:
> > > 1: Move the tests to gcc.target/i386/scev-[3-5].c
> > 
> > Subject: [PATCH 1/2] testsuite: Fix XPASS for gcc.dg/tree-ssa/scev-3.c, 
> > -4.c and -5.c [PR112786]
> > 
> > This is the first alternative, perhaps the more appropriate one.
> > 
> > Tested cris-elf, arm-eabi (default), x86_64-linux, ditto -m32,
> > h8300-elf and shle-linux; xpassing, skipped and passing as
> > applicable and intended.
> > 
> > Ok to commit?
> 
> Digging in history reveals the testcases were added by
> Jiangning Liu , not for any
> particular bugreport but "The problem is originally from a real benchmark,
> and the test case only tries to detect the GIMPLE level changes."
> 
> I'm not sure we can infer the testcase should be moved to
> gcc.target/arm/ because of that, but it does seem plausible.

It's been so long and so many changes since these tests were
regression guards, that the original target has lost
importance.  Heck, it was even xfail lp64 at one time!
According to my git dig, it's been adjusted for pass
changes, including reordering and dump output changes.  But
you know that; you've been instrumental in many of those
changes. :)

I'd say gcc.target/arm/ is the one target that's *not*
plausible, as according to Alex result differs between
subtargets.

> I read from your messages that the testcases pass on arm*-*-*?

Yes: they pass (currently XPASS) on arm-eabi and
arm-unknown-linux-gnueabi, default configurations.  But,
scev-3 and -5 fail with for example -mcpu=cortex-r5

brgds, H-P
PS. I'm open to just reverting the patch.  The s/ia32/ilp32/ is proven wrong.

Re: [RFA] New pass for sign/zero extension elimination

[Hans-Peter Nilsson]

> Date: Fri, 1 Dec 2023 08:09:08 -0700
> From: Jeff Law 

> On 11/30/23 18:08, Hans-Peter Nilsson wrote:
> >> Date: Sun, 19 Nov 2023 17:47:56 -0700
> >> From: Jeff Law 
> > 
> >> Locally we have had this enabled at -O1 and above to encourage testing,
> >> but I'm thinking that for the trunk enabling at -O2 and above is the
> >> right thing to do.
> > 
> > Yes.
> > 
> >> Thoughts, comments, recommendations?
> > 
> > Sounds great!
> > 
> > It'd be nice if its framework can be re-used for
> > target-specific passes, doing quirky sign- or zero-extend-
> > related optimizations (those that are not just sign- or
> > zero-extend removal).  Perhaps most of those opportunities
> > can be implemented as target hooks in this pass.  Definitely
> > not asking for a change, just imagining future improvements.
> > 
> > Also, I haven't followed the thread and its branches, just
> > offering a word encouragement.
> What kind of quirky target things did you have in mind?  If there's 
> overlap with things we need I might be able to find someone to take it 
> on.  Or might be able to suggest how they can be handled.

Sorry, I was hoping I'd not need to substantiate that part
outside the "not just sign- or zero-extend removal". :)

But perhaps: somewhat trivial would be where the
sign/zero-extension is hidden in an unspec, so the target
needs to be consulted regarding possible elimination and how
to do it.

If that doesn't do it, just ignore that part of the comment.
I have nothing substantial besides this pass sounding like
it'd be a great stepping-stone.  I'm having trouble making
up hypothetical cases here, and it probably wouldn't help.

I hope I'll find time to try the latest version but
definitely no promises.

brgds, H-P

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 30 Nov 2023 18:09:10 +0100

> I intend to post two alternative patches to get this
> resolved:

> 2: gcc.dg/tree-ssa/scev-[3-5].c skipped for arm*, xfailed
>only on h8300-*-* and ia32.

(Except as mentioned, the XPASS issue does not apply to
h8300; it's "i16lp32".  It remains in the set of targets
that were tested though.)

Subject: [PATCH 2/2] testsuite: Fix XPASS for gcc.dg/tree-ssa/scev-3.c, -4.c 
and -5.c [PR112786]

This is the second alternative, slightly more trivial than the first.

Tested cris-elf, arm-eabi (default), x86_64-linux, ditto -m32,
h8300-elf and shle-linux; xpassing, skipped and passing as
applicable and intended.

Ok to commit?
-- >8 --
Results differ between ARM sub-targets, thus better to skip these
tests.  They are known to fail only for ia32-elf.

PR testsuite/112786
* gcc.dg/tree-ssa/scev-3.c, gcc.dg/tree-ssa/scev-4.c,
gcc.dg/tree-ssa/scev-5.c: Revert last change.  Instead, skip dump
match for arm*.
---
 gcc/testsuite/gcc.dg/tree-ssa/scev-3.c | 3 +--
 gcc/testsuite/gcc.dg/tree-ssa/scev-4.c | 3 +--
 gcc/testsuite/gcc.dg/tree-ssa/scev-5.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
index beea9aed9fe9..6e9733504014 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
@@ -40,5 +40,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { target { ! arm*-*-* } 
xfail ia32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
index a97f75f81f65..a0cf171f01be 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
@@ -45,5 +45,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { target { ! arm*-*-* } 
xfail ia32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c 
b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
index 08f4260403c4..0bd743cc6be6 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
@@ -40,5 +40,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { target { ! arm*-*-* } 
xfail ia32 } } } */
-- 
2.30.2

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 30 Nov 2023 18:09:10 +0100

Richard B.:
> > > In the end we might need to move/duplicate the test to some
> > > gcc.target/* dir and restrict it to a specific tuning.
> 
> I intend to post two alternative patches to get this
> resolved:
> 1: Move the tests to gcc.target/i386/scev-[3-5].c

Subject: [PATCH 1/2] testsuite: Fix XPASS for gcc.dg/tree-ssa/scev-3.c, -4.c 
and -5.c [PR112786]

This is the first alternative, perhaps the more appropriate one.

Tested cris-elf, arm-eabi (default), x86_64-linux, ditto -m32,
h8300-elf and shle-linux; xpassing, skipped and passing as
applicable and intended.

Ok to commit?
-- >8 --
PR testsuite/112786
* gcc.dg/tree-ssa/scev-3.c, gcc.dg/tree-ssa/scev-4.c,
gcc.dg/tree-ssa/scev-5.c: Revert last change and move...
* gcc.target/i386/scev-3.c, gcc.target/i386/scev-4.c:
gcc.target/i386/scev-5.c: ...here.
---
 gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-3.c | 3 +--
 gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-4.c | 3 +--
 gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-5.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)
 rename gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-3.c (80%)
 rename gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-4.c (81%)
 rename gcc/testsuite/{gcc.dg/tree-ssa => gcc.target/i386}/scev-5.c (81%)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c 
b/gcc/testsuite/gcc.target/i386/scev-3.c
similarity index 80%
rename from gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
rename to gcc/testsuite/gcc.target/i386/scev-3.c
index beea9aed9fe9..ac8c8d4519e3 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
+++ b/gcc/testsuite/gcc.target/i386/scev-3.c
@@ -40,5 +40,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ia32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c 
b/gcc/testsuite/gcc.target/i386/scev-4.c
similarity index 81%
rename from gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
rename to gcc/testsuite/gcc.target/i386/scev-4.c
index a97f75f81f65..b0d594053019 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-4.c
+++ b/gcc/testsuite/gcc.target/i386/scev-4.c
@@ -45,5 +45,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ia32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c 
b/gcc/testsuite/gcc.target/i386/scev-5.c
similarity index 81%
rename from gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
rename to gcc/testsuite/gcc.target/i386/scev-5.c
index 08f4260403c4..c911a9298866 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
+++ b/gcc/testsuite/gcc.target/i386/scev-5.c
@@ -40,5 +40,4 @@ __BB(6):
 
 }
 
-/* Not all 32-bit systems fail this, but several do.  */
-/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ilp32 } } } */
+/* { dg-final { scan-tree-dump-times "" 1 "ivopts" { xfail ia32 } } } */
-- 
2.30.2

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 30 Nov 2023 18:09:10 +0100

> I intend to post two alternative patches to get this
> resolved:
> 1: Move the tests to gcc.target/i386/scev-[3-5].c
> 2: gcc.dg/tree-ssa/scev-[3-5].c skipped for arm*, xfailed
>only on h8300-*-* and ia32.

Correction: h8300-elf does not XPASS not because it's a
ilp32 that fails, but because it's not an ilp32 and does not
match the XFAIL selector, therefore it passes.

IOW, minimizing the negations: h8300-elf is apparently an
"i16lp32" and passes.  No need for special-casing.
TIL, TMRIF.

brgds, H-P

Re: [RFA] New pass for sign/zero extension elimination

[Hans-Peter Nilsson]

> Date: Sun, 19 Nov 2023 17:47:56 -0700
> From: Jeff Law 

> Locally we have had this enabled at -O1 and above to encourage testing, 
> but I'm thinking that for the trunk enabling at -O2 and above is the 
> right thing to do.

Yes.

> Thoughts, comments, recommendations?

Sounds great!

It'd be nice if its framework can be re-used for
target-specific passes, doing quirky sign- or zero-extend-
related optimizations (those that are not just sign- or
zero-extend removal).  Perhaps most of those opportunities
can be implemented as target hooks in this pass.  Definitely
not asking for a change, just imagining future improvements.

Also, I haven't followed the thread and its branches, just
offering a word encouragement.

brgds, H-P

Re: Ping: [PATCH] Fix PR112419

[Hans-Peter Nilsson]

> From: Martin Uecker 
> Cc: richard.guent...@gmail.com

> Am Montag, dem 27.11.2023 um 08:36 -0700 schrieb Jeff Law:
> > 
> > On 11/23/23 10:05, Hans-Peter Nilsson wrote:
> > > > From: Hans-Peter Nilsson 
> > > > Date: Thu, 16 Nov 2023 05:24:06 +0100
> > > > 
> > > > > From: Martin Uecker 
> > > > > Date: Tue, 07 Nov 2023 06:56:25 +0100
> > > > 
> > > > > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > > > > > 
> > > > > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > > > > This patch caused a testsuite regression: there's now an
> > > > > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > > > > after your r14-5115-g6e9ee44d96e5.  It's logged as PR112419.
> > > > > > It caused failures for just about every target ;(  Presumably it 
> > > > > > worked
> > > > > > on x86_64...
> > > > > 
> > > > > I do not think this is a true regression
> > > > > just a problem with the test on 32-bit which somehow surfaced
> > > > > due to the change.
> > > > > 
> > > > > The excess error is:
> > > > > 
> > > > > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > > > > Excess errors:
> > > > > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3:
> > > > >  warning: 'fda_n_5' specified size 4294967256 exceeds maximum object 
> > > > > size
> > > > > 2147483647 [-Wstringop-overflow=]
> > > > > 
> > > > > I think the warning was suppressed before due to the other (nonnull)
> > > > > warning which I removed in this case.
> > > > > 
> > > > > I think the simple fix might be to to turn off -Wstringop-overflow.
> > > > 
> > > > No, that trigs many of the dg-warnings that are tested.
> > > > 
> > > > (I didn't pay attention to the actual warning messages and
> > > > tried to pursue that at first.)
> > > > 
> > > > Maybe think it's best to actually expect the warning, like
> > > > so.
> > > > 
> > > > Maintainers of 16-bit targets will have to address their
> > > > concerns separately.  For example, they may choose to not
> > > > run the test at all.
> > > > 
> > > > Ok to commit?
> > > > 
> > > > Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 
> > > > 32-bit targets [PR112419]
> > > > 
> > > > PR testsuite/112419
> > > > * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for 
> > > > exceeding
> > > > maximum object size for 32-bit targets.
> > > > ---
> > > >   gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c 
> > > > b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > index 1f14fbba45df..d63e76da70a2 100644
> > > > --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> > > > @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
> > > > T (  1);  // { dg-bogus "argument 2 of variable length 
> > > > array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 
> > > > 1 value is 1" }
> > > > T (  9);  // { dg-bogus "argument 2 of variable length 
> > > > array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 
> > > > 1 value is 9" }
> > > > T (max);  // { dg-bogus "argument 2 of variable length 
> > > > array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 
> > > > 1 value is \\d+" }
> > > > +// { dg-warning "size 4294967256 exceeds maximum object size" "" { 
> > > > target ilp32 } .-1 }
> > > >   }
> > Unfortunately I think we need to go back to the original issue that 
> > Martin (I think) dismissed.
> > 
> > Specifically, this is a regression.  It's very clear that prior to the 
> > patch in question there was no diagnostic about the size of the 
> > requested memory allocation and after the patch in question we get the 
> > "exceeds maximum object size" diagnostic.
> > 
> > Now one explanation could be that the diagnostic is warranted and it was 
> > a bug that the diagnostic hadn't been emitted prior to Martin's patch. 
> > In this case some kind of dg-blah is warranted, but I don't think anyone 
> > has made this argument.
> > 
> I believe the warning is correct but was suppressed before.
> 
> 
> My plan was to split up the test case in one which is for
> -Wstringop-overflow and one which is for -Wnonnull and then
> one could turn off the -Wstringop-overflow for the tests
> which are actually for -Wnonnull.  But adding the dg-blah
> would certainly be simpler.

Sort-of-mid-week ping (only because status quo isn't clear):
Jeff, are you content with Martin U:s reply (i.e. patch ok)
or are you waiting for a follow-up?

Perhaps it's already in your overflowing queue, then please
ignore this, I'll just ping in a week. ;-)

IMHO, after looking at the test-case I'd expect *more*
warnings for ilp32 targets; i.e. it was a bug that it didn't
show up before.

brgds, H-P

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> From: Alexandre Oliva 
> Date: Thu, 30 Nov 2023 01:41:55 -0300

> On Nov 29, 2023, Hans-Peter Nilsson  wrote:
> 
> >> XPASS: gcc.dg/tree-ssa/scev-3.c scan-tree-dump-times ivopts "" 1
> >> XPASS: gcc.dg/tree-ssa/scev-4.c scan-tree-dump-times ivopts "" 1
> >> XPASS: gcc.dg/tree-ssa/scev-5.c scan-tree-dump-times ivopts "" 1
> 
> > It XPASSes on the ilp32 targets I've tried - except "ia32"
> > (as in i686-elf) and h8300-elf.  Notably XPASSing targets
> > includes a *default* configuration of arm-eabi, which in
> > part contradicts your observation above.
> 
> My arm-eabi testing then targeted tms570 (big-endian cortex-r5).
> 
> I borrowed the ilp32 vs lp64 line from an internal patch by Eric that
> we've had in gcc-11 and gcc-12, when I hit this fail while transitioning
> the first and then the second of our 32-bit targets to gcc-13.

'k, I interpret that as that ilp32 being mostly a copy-paste
of unclear origin, and that there weren't actually any
observation of more than two ilp32 targets failing (counting
ia32 and one or more non-default armeb).

> > So, ilp32 is IMO a really bad approximation for the elusive
> > property.
> 
> Yeah.  Maybe we should just drop the ilp32, so that it's an unsurprising
> fail on any targets?
> 
> > Would you please consider changing those "ilp32" to a
> > specific set of targets where these tests failed?
> 
> I'd normally have aimed for that, but the challenge is that arm-eabi is
> not uniform in the results for this test, and there doesn't seem to be
> much support or knowledge to delineate on which target variants it's
> meant to pass or not.

That situation suggests to me to *skip* it for arm*-*-*.
For other targets, skip or xfail as needed after a quick
look.  Or as per below.

>  The test expects the transformation to take
> place, as if it ought to, but there's no strong reason to expect that it
> should.  There's nothing wrong if it doesn't.  Going about trying to
> match the expectations to the current results may be useful, but
> investigating the reasons why we get the current results for each target
> is beyond my available resources for a set of tests that used to *seem*
> to pass uniformly only because of a bug in the test pattern.
> 
> I don't see much value in these tests as they are, TBH.

Richard B. seems to have been the last person doing
significant work on those tests (rewriting scev-[3-5].c to
gimple tests for ivopts, commit r7-6026-ga23e48df4514c4), so
I value his suggestion per below particularly valuable:

> > In the end we might need to move/duplicate the test to some
> > gcc.target/* dir and restrict it to a specific tuning.

I intend to post two alternative patches to get this
resolved:
1: Move the tests to gcc.target/i386/scev-[3-5].c
2: gcc.dg/tree-ssa/scev-[3-5].c skipped for arm*, xfailed
   only on h8300-*-* and ia32.

If you can help with e.g. reviewing, thanks in advance.

I opened PR112786 to get an ID for the issue.

brgds, H-P

Re: [PATCH] testsuite: scev: expect fail on ilp32

[Hans-Peter Nilsson]

> From: Rainer Orth 
> Date: Tue, 28 Nov 2023 16:13:35 +0100

> Richard Biener  writes:
> 
> > On Sun, 19 Nov 2023, Jeff Law wrote:
> >
> >> 
> >> 
> >> On 11/19/23 00:30, Alexandre Oliva wrote:
> >> > 
> >> > I've recently patched scev-3.c and scev-5.c because it only passed by
> >> > accident on ia32.  It also fails on some (but not all) arm-eabi
> >> > variants.  It seems hard to characterize the conditions in which the
> >> > optimization is supposed to pass, but expecting them to fail on ilp32
> >> > targets, though probably a little excessive and possibly noisy, is not
> >> > quite as alarming as getting a fail in test reports, so I propose
> >> > changing the xfail marker from ia32 to ilp32.
> >> > 
> >> > I'm also proposing to add a similar marker to scev-4.c.  Though it
> >> > doesn't appear to be failing for me, I've got reports that suggest it
> >> > still does for others, and it certainly did for us as well.
> >> > 
> >> > Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default
> >> > cpu on trunk, and with tms570 on gcc-13.  Ok to install?
> >> > 
> >> > 
> >> > for  gcc/testsuite/ChangeLog
> >> > 
> >> >  * gcc.dg/tree-ssa/scev-3.c: xfail on all ilp32 targets,
> >> >  though some of these do pass.
> >> >  * gcc.dg/tree-ssa/scev-4.c: Likewise.
> >> >  * gcc.dg/tree-ssa/scev-5.c: Likewise.
> >> OK.  Though hopefully someone will figure out what properties actually 
> >> cause
> >> the differences so that we can do the right thing without the noisy XPASS 
> >> at
> >> some point.
> >
> > The tests all test IVOPTs induction variable selecting results
> > (assuming every target would come to the "obvious" conclusion),
> > so it's probably not only target but also sub-target (aka -mtune)
> > sensitive ...
> >
> > In the end we might need to move/duplicate the test to some
> > gcc.target/* dir and restrict it to a specific tuning.
> 
> FWIW, since Alexandre's patch all three tests XPASS on 32-bit
> Solaris/SPARC:
> 
> XPASS: gcc.dg/tree-ssa/scev-3.c scan-tree-dump-times ivopts "" 1
> XPASS: gcc.dg/tree-ssa/scev-4.c scan-tree-dump-times ivopts "" 1
> XPASS: gcc.dg/tree-ssa/scev-5.c scan-tree-dump-times ivopts "" 1

It XPASSes on the ilp32 targets I've tried - except "ia32"
(as in i686-elf) and h8300-elf.  Notably XPASSing targets
includes a *default* configuration of arm-eabi, which in
part contradicts your observation above.  I see it even
XPASSes in H.J.'s x86_64-pc-linux-gnu -mx32 results.  Right,
that's not ia32, but it's as ilp32ish as ia32 and can be
expected to share most "interesting" properties with ia32.
Example report at
https://gcc.gnu.org/pipermail/gcc-testresults/2023-November/801862.html.

Alex, can you share the presumably plural set of targets
where you found gcc.dg/tree-ssa/scev-[3-5].c to fail before
your patch, besides "ia32"?

I see them XPASS for:
m68k-unknown-linux-gnu
(https://gcc.gnu.org/pipermail/gcc-testresults/2023-November/801839.html)
pru-unknown-elf
(https://gcc.gnu.org/pipermail/gcc-testresults/2023-November/801732.html)

and from my own testing, at r14-5608-g69741355e6dbcf:
cris-elf, c6x-elf, epiphany-elf, ft32-elf,
hppa-unknown-linux-gnu, lm32-elf, microblaze-linux,
m32r-elf, arm-eabi.

So, ilp32 is IMO a really bad approximation for the elusive
property.

Would you please consider changing those "ilp32" to a
specific set of targets where these tests failed?

I'd prefer not to complicate those expressions by adding the
right spelling of "ilp32 except { list }".

brgds, H-P

Re: [committed v2] libstdc++: Define std::ranges::to for C++23 (P1206R7) [PR111055]

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Date: Thu, 23 Nov 2023 17:51:38 +

> libstdc++-v3/ChangeLog:
> 
>   PR libstdc++/111055
>   * include/bits/ranges_base.h (from_range_t): Define new tag
>   type.
>   (from_range): Define new tag object.
>   * include/bits/version.def (ranges_to_container): Define.
>   * include/bits/version.h: Regenerate.
>   * include/std/ranges (ranges::to): Define.
>   * testsuite/std/ranges/conv/1.cc: New test.
>   * testsuite/std/ranges/conv/2_neg.cc: New test.
>   * testsuite/std/ranges/conv/version.cc: New test.

JFTR, for the list: this (r14-5794-g7a6a29c455e775) caused
another one of those wonderful "xtreme test" regressions.

Logged as PR112737: "[14 Regression]
g++.dg/modules/xtreme-header-2_b.C -std=c++2b (test for
excess errors)", and pinskia quickly linked it to the
meta-bug for modules issues, PR103524 (thanks!)

IIRC, sometimes those tests show bugs elsewhere, suggesting
lack of coverage in other tests (and not just streaming
aspects), but this one actually mentions modules in key
error messages.

brgds, H-P

[Committed] testsuite/gcc.dg/uninit-pred-9_b.c:23: Un-xfail for MMIX

[Hans-Peter Nilsson]

In a recent all-target test-round investigating XPASSes for
this file, I noticed this line XPASSing for MMIX.  From the
commit history it's obvious it was left out from related
target-xfail tweaks, now the last target xfailing a bogus
warning for this line.

* gcc.dg/uninit-pred-9_b.c: Remove xfail for MMIX from line 23.
---
 gcc/testsuite/gcc.dg/uninit-pred-9_b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/uninit-pred-9_b.c 
b/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
index 3c83d505ec0f..3e544f3f1be4 100644
--- a/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
+++ b/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
@@ -20,7 +20,7 @@ int foo (int n, int l, int m, int r)
   blah(v); /* { dg-bogus "uninitialized" "bogus warning" { xfail *-*-* } } 
*/
 
   if ( (n <= 8) &&  (m < 99)  && (r < 19) )
-  blah(v); /* { dg-bogus "uninitialized" "pr101674" { xfail mmix-*-* } } */
+  blah(v); /* { dg-bogus "uninitialized" "pr101674" } */
 
   return 0;
 }
-- 
2.30.2

[PATCH] testsuite/gcc.dg/uninit-pred-9_b.c:20: Fix XPASS for various targets

[Hans-Peter Nilsson]

While looking at the various targets, I found that the m32r
target has two options implemented as opposites:
-mbranch-cost=1 and -mbranch-cost=2, that have a bug that
makes them yield their functionally opposite effect;
i.e. -mbranch-cost=$arg, arg={1, 2} yields BRANCH_COST(x, y)
3-$arg.  Anyway, the default is 1, unknown if that's
deliberate.  (I won't add a PR, just CC:ing the maintainer.)

Tested for
XPASSing targets: m32r-elf and cris-elf
XFAILing targets: loongarch64-unknown-linux-gnuf64 and ia64-linux.

Ok to commit?

-- >8 --
The xfail for "*-*-*" here, set in r14-4089-gd45ddc2c04e471
"tree-optimization/111294 - backwards threader PHI costing"
was somewhat too general and made this test XPASS for a
number of targets.  The common factor for those targets is
that they either explicitly or by default define
LOGICAL_OP_NON_SHORT_CIRCUIT as 0 (see fold-const.cc).

Instead of changing *-*-* to a seemingly random set of
xfailed targets or inventing a new testsuite
effective-target predicate for logical-op-short-circuited
targets or the opposite, let's just force a setting that
removes the need for the xfail for all targets, by
overriding with --param=logical-op-non-short-circuit=0.

* gcc.dg/uninit-pred-9_b.c: Remove xfail for line 20.  Pass
--param=logical-op-non-short-circuit=0.  Comment why.
---
 gcc/testsuite/gcc.dg/uninit-pred-9_b.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/uninit-pred-9_b.c 
b/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
index 3e544f3f1be4..1877d5d45d6c 100644
--- a/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
+++ b/gcc/testsuite/gcc.dg/uninit-pred-9_b.c
@@ -1,5 +1,6 @@
 /* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* The param shuts up a bogus uninitialized warning at line 21.  */
+/* { dg-options "-Wuninitialized -O2 --param=logical-op-non-short-circuit=0" } 
*/
 
 int g;
 void bar();
@@ -17,7 +18,7 @@ int foo (int n, int l, int m, int r)
 
   if (l > 100)
 if ( (n <= 9) &&  (m < 100)  && (r < 19) )
-  blah(v); /* { dg-bogus "uninitialized" "bogus warning" { xfail *-*-* } } 
*/
+  blah(v); /* { dg-bogus "uninitialized" "bogus warning" } */
 
   if ( (n <= 8) &&  (m < 99)  && (r < 19) )
   blah(v); /* { dg-bogus "uninitialized" "pr101674" } */
-- 
2.30.2

[PATCH 3/3] contrib/regression/btest-gcc.sh: Optionally handle XPASS.

[Hans-Peter Nilsson]

Somewhat trivial, still tested on several runs (for
cris-elf): two starting from the same state, with/without
--handle-xpass-as-fail; the one "without" showing no change
in state compared to an unpatched baseline (with the same
input-state), and the one with --handle-xpass-as-fail some
XPASSing tests I'd noticed now correctly showed up as
regressions.  In another, separate run, with the same input
state but one of those XPASSing tests removed from "passes"
in the input-state, it correctly showed up as a
(non-regression) new FAIL.

-- >8 --
Tests with keys that match both PASS, FAIL (or now
optionally XPASS), count as fail.  XPASSes were previously
ignored.  Handling them as FAIL seems the most useful
alternative, but not counting XPASSes may be deliberate.
It's also a matter of compatibility, so make it optional.

Attempts to use --handle-xpass-as-fail was previously
flagged as a usage error.  If you pass it now, on state with
previous mixed XPASS and PASS results but doesn't change in
this run, the XPASS is discovered as a (new) regression.
For new XPASSing tests, it's handled as a new FAIL.

* btest-gcc.sh (--handle-xpass-as-fail): New option.
---
 contrib/regression/btest-gcc.sh | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/contrib/regression/btest-gcc.sh b/contrib/regression/btest-gcc.sh
index 3c031e93709b..684019f715f1 100755
--- a/contrib/regression/btest-gcc.sh
+++ b/contrib/regression/btest-gcc.sh
@@ -22,17 +22,22 @@
 
 add_passes_despite_regression=0
 dashj=''
+handle_xpass_as_fail=false
 
 #  can be
 # --add-passes-despite-regression:
 #  Add new "PASSes" despite there being some regressions.
 # -j:
 #  Pass '-j' to make.
+# --handle-xpass-as-fail:
+#  Count XPASS as a FAIL (default ignored).
 
 while : ; do
   case "$1" in
--add-passes-despite-regression)
 add_passes_despite_regression=1;;
+   --handle-xpass-as-fail)
+handle_xpass_as_fail=true;;
-j*)
 dashj=$1;;
-*) echo "Invalid option: $1"; exit 2;;
@@ -203,7 +208,11 @@ done
 # Work out what failed
 for LOG in $TESTLOGS ; do
   L=`basename $LOG`
-  awk '/^FAIL: / { print "'$L'",$2; }' $LOG || exit 1
+  if $handle_xpass_as_fail ; then
+   awk '/^(FAIL|XPASS): / { print "'$L'",$2; }' $LOG || exit 1
+  else
+   awk '/^FAIL: / { print "'$L'",$2; }' $LOG || exit 1
+  fi
 done | sort | uniq > $FAILED || exit 1
 comm -12 $FAILED $PASSES >> $REGRESS || exit 1
 NUMREGRESS=`wc -l < $REGRESS | tr -d ' '`
-- 
2.30.2

[PATCH 2/3] contrib/regression/btest-gcc.sh: Simplify option handling.

[Hans-Peter Nilsson]

Tested as with the previous patch.

-- >8 --
* btest-gcc.sh (Option handling): Break out shifts from each
option alternative.
---
 contrib/regression/btest-gcc.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/regression/btest-gcc.sh b/contrib/regression/btest-gcc.sh
index 22e8f0398662..3c031e93709b 100755
--- a/contrib/regression/btest-gcc.sh
+++ b/contrib/regression/btest-gcc.sh
@@ -32,12 +32,13 @@ dashj=''
 while : ; do
   case "$1" in
--add-passes-despite-regression)
-add_passes_despite_regression=1; shift;;
+add_passes_despite_regression=1;;
-j*)
-dashj=$1; shift;;
+dashj=$1;;
-*) echo "Invalid option: $1"; exit 2;;
*) break;;
   esac
+  shift
 done
 
 # TARGET is the target triplet.  It should be the same one as used in
-- 
2.30.2

[PATCH 1/3] contrib/regression/btest-gcc.sh: Handle multiple options.

[Hans-Peter Nilsson]

Deliberately not using getopt.  Tested by adding a line right after this
code echoing $dashj, $add_passes_despite_regression, and $1 (then exit)
and checking that I got it right for combinations of -j j4
--add-passes-despite-regression.

-- >8 --
This is a long-standing bug: passing "-j --add-passes-despite-regression"
or "--add-passes-despite-regression -j" caused the second option to be
treated as TARGET; the first non-option parameter.

* btest-gcc.sh (Option handling): Handle multiple options.
---
 contrib/regression/btest-gcc.sh | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/contrib/regression/btest-gcc.sh b/contrib/regression/btest-gcc.sh
index 1808fcc392fa..22e8f0398662 100755
--- a/contrib/regression/btest-gcc.sh
+++ b/contrib/regression/btest-gcc.sh
@@ -29,13 +29,16 @@ dashj=''
 # -j:
 #  Pass '-j' to make.
 
-case "$1" in
- --add-passes-despite-regression)
-  add_passes_despite_regression=1; shift;;
- -j*)
-  dashj=$1; shift;;
- -*) echo "Invalid option: $1"; exit 2;;
-esac
+while : ; do
+  case "$1" in
+   --add-passes-despite-regression)
+add_passes_despite_regression=1; shift;;
+   -j*)
+dashj=$1; shift;;
+   -*) echo "Invalid option: $1"; exit 2;;
+   *) break;;
+  esac
+done
 
 # TARGET is the target triplet.  It should be the same one as used in
 # constructing PREFIX.  Or it can be the keyword 'native', indicating
-- 
2.30.2

[PATCH 0/3] A few contrib/regression/btest-gcc.sh updates.

[Hans-Peter Nilsson]

Anyone using contrib/regression/btest-gcc.sh besides me?

It has, besides a copyright update, not seen love and attention in a
decade.  Also, the original author and maintainer hasn't been visibly
active with gcc.  I've contributed the other trivial, non-date changes
since 2009 and some others before that, and I use it regularly both in
my autotester and any per-target patch testing.

Thus, I volunteer to pick it up.  I intend to keep functional
compatibility.  Besides adding .sum file with optional presence as has
been done in the past, new functionality, as well as changes in
functionality, will be optional and default off, where reasonable
alternative use could otherwise break and/or have different results.

For example, I won't add an almost trivial LC_ALL=C to unify results
and avoid the problem with different LC_ALL and trying to pass state
between systems.  One such breaking setup is running btest-gcc.sh with
LC_ALL=C on one system and using the resulting state in a run with
LC_ALL=en_US.utf8 on the other.  This will break at the calls to
"comm" after "sort" between previous and current collections of
passing tests due to "g++" and "gcc" collating differently, a wart
I've discovered twice (and wrongly worked around once).  Changing that
would obviously break *some* setups.  This problem also doesn't need a
fix in btest-gcc.sh, it's trivially handled by changing its usage to
e.g. "env LC_ALL=C /path/to/btest-gcc.sh ...".  That's not a big deal
if you like me call btest-gcc.sh in *another* script.

With this mostly trivial patchset, leading up to optional handling of
XPASS, I'm sort-of testing the waters.  I knew about XPASS being
currently ignored, just didn't care enough about that as I also diff
the test-logs for my manual testing.  The biggest problem was then
that each run can't be done in parallel.

Hans-Peter Nilsson (3):
  contrib/regression/btest-gcc.sh: Handle multiple options.
  contrib/regression/btest-gcc.sh: Simplify option handling.
  contrib/regression/btest-gcc.sh: Optionally handle XPASS.

 contrib/regression/btest-gcc.sh | 27 +++
 1 file changed, 19 insertions(+), 8 deletions(-)

-- 
2.30.2

Ping: [PATCH] Fix PR112419 (was: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541])

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson 
> Date: Thu, 16 Nov 2023 05:24:06 +0100
> 
> > From: Martin Uecker 
> > Date: Tue, 07 Nov 2023 06:56:25 +0100
> 
> > Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > > 
> > > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > > This patch caused a testsuite regression: there's now an
> > > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > > targets (and 64-bit targets testing with a "-m32" option)
> > > > after your r14-5115-g6e9ee44d96e5.  It's logged as PR112419.
> > > It caused failures for just about every target ;(  Presumably it worked 
> > > on x86_64...
> > 
> > I do not think this is a true regression
> > just a problem with the test on 32-bit which somehow surfaced
> > due to the change.
> > 
> > The excess error is:
> > 
> > FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> > Excess errors:
> > /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3:
> >  warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> > 2147483647 [-Wstringop-overflow=]
> > 
> > I think the warning was suppressed before due to the other (nonnull)
> > warning which I removed in this case.
> > 
> > I think the simple fix might be to to turn off -Wstringop-overflow.
> 
> No, that trigs many of the dg-warnings that are tested.
> 
> (I didn't pay attention to the actual warning messages and
> tried to pursue that at first.)
> 
> Maybe think it's best to actually expect the warning, like
> so.
> 
> Maintainers of 16-bit targets will have to address their
> concerns separately.  For example, they may choose to not
> run the test at all.
> 
> Ok to commit?
> 
> Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit 
> targets [PR112419]
> 
>   PR testsuite/112419
>   * gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
>   maximum object size for 32-bit targets.
> ---
>  gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c 
> b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> index 1f14fbba45df..d63e76da70a2 100644
> --- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
> +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
> @@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
>T (  1);  // { dg-bogus "argument 2 of variable length array 
> 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 
> 1" }
>T (  9);  // { dg-bogus "argument 2 of variable length array 
> 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 
> 9" }
>T (max);  // { dg-bogus "argument 2 of variable length array 
> 'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 
> \\d+" }
> +// { dg-warning "size 4294967256 exceeds maximum object size" "" { target 
> ilp32 } .-1 }
>  }
>  
>  
> -- 
> 2.30.2
> 

[PATCH] testsuite: Tweak xfail bogus g++.dg/warn/Wstringop-overflow-4.C:144, PR106120

[Hans-Peter Nilsson]

I added that xfail in February for { ilp32 && c++98_only } and it
looks like it's moved on to lp64 now. :-/  Noted by Rainer
Orth, see the PR.

Tested cris-elf and x86_64-pc-linux-gnu w/wo. -m32.
Ok to commit?

-- >8 --
The conditions under which this this bogus warning is
emitted has changed to not happen for 32-bit targets
anymore.  Adjust accordingly.

PR testsuite/106120
* g++.dg/warn/Wstringop-overflow-4.C:144 XFAIL bogus warning for
lp64 targets with c++98.
---
 gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C 
b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
index 275ecac01b5f..2024f8d93ca3 100644
--- a/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
+++ b/gcc/testsuite/g++.dg/warn/Wstringop-overflow-4.C
@@ -141,7 +141,7 @@ void test_strcpy_new_int16_t (size_t n, const size_t vals[])
 
   int r_imin_imax = SR (INT_MIN, INT_MAX);
   T (S (1), new int16_t[r_imin_imax]);
-  T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into a region of 
size" "pr106120" { xfail { c++98_only } } }
+  T (S (2), new int16_t[r_imin_imax + 1]); // { dg-bogus "into a region of 
size" "pr106120" { xfail { lp64 && c++98_only } } }
   T (S (9), new int16_t[r_imin_imax * 2 + 1]);
 
   int r_0_imax = SR (0, INT_MAX);
-- 
2.30.2

Re: [RFC PATCH] Detecting lifetime-dse issues via Valgrind

[Hans-Peter Nilsson]

> From: 
> Date: Tue, 24 Oct 2023 17:11:24 +0300

> From: Daniil Frolov 
> 
> PR 66487 is asking to provide sanitizer-like detection for C++ object lifetime
> violations that are worked around with -fno-lifetime-dse in Firefox, LLVM,
> OpenJade.
> 
> The discussion in the PR was centered around extending MSan, but MSan was not
> ported to GCC (and requires rebuilding everything with instrumentation).
> 
> Instead, allow Valgrind to see lifetime boundaries by emitting client requests
> along *this = { CLOBBER }.  The client request marks the "clobbered" memory as
> undefined for Valgrind; clobbering assignments mark the beginning of ctor and
> end of dtor execution for C++ objects.  Hence, attempts to read object storage
> after the destructor, or "pre-initialize" its fields prior to the constructor
> will be caught.

A long time ago, I hacked the first version of
valgrind-testing support into gcc and I think yours is a
great idea!

A natural follow-up then would be making
-fvalgrind-emit-annotations the default when building gcc
*and* --enable-checking=valgrind is in effect (i.e. not just
expanding the in-source annotations but the explicit testing
mode).  No need to cram it into the first version though.

Also, a preprocessor macro when -fvalgrind-emit-annotations
is in effect, may help dealing with quirky corner cases.

I agree with Arsen that if no valgrind headers are found,
just don't build __valgrind_make_mem_undefined and let its
absense be a linker error rather than trapping in
__valgrind_make_mem_undefined.

I think building libgcc __valgrind_make_mem_undefined should
actually be the *default* when valgrind headers are found,
i.e. in absense of --disable-valgrind-annotations.

A hard configure-time error is suitable when an
--enable-valgrind-annotations is explicitly specified in the
absence of valgrind headers.

BTW, if you use AS_IF, beware of its quirks.  See end of
this email-thread with conclusion when libstdc++ was hit:
https://gcc.gnu.org/pipermail/libstdc++/2023-June/056113.html

brgds, H-P

Re: [PATCH 0/4] v2 of Option handling: add documentation URLs

[Hans-Peter Nilsson]

> From: David Malcolm 
> Date: Thu, 16 Nov 2023 09:28:54 -0500

> How is this looking for trunk?
> 
> Thanks
> Dave
> 
> 
> David Malcolm (4):
>   options: add gcc/regenerate-opt-urls.py
>   Add generated .opt.urls files
>   opts: add logic to generate options-urls.cc
>   options: wire up options-urls.cc into gcc_urlifier
> 
>  gcc/Makefile.in  |   29 +-
>  gcc/ada/gcc-interface/lang.opt.urls  |   30 +
>  gcc/analyzer/analyzer.opt.urls   |  206 ++
>  gcc/c-family/c.opt.urls  | 1409 ++
>  gcc/common.opt.urls  | 1832 ++
>  gcc/config/aarch64/aarch64.opt.urls  |   84 +
>  gcc/config/alpha/alpha.opt.urls  |   76 +
>  gcc/config/alpha/elf.opt.urls|2 +
>  gcc/config/arc/arc-tables.opt.urls   |2 +

[... etc .opt.urls particularly in gcc/config/*
autogenerated for each *.opt ...]

Sorry for barging in though I did try finding the relevant
discussion, but is committing this generated stuff necessary?
Is it because we don't want to depend on Python being
present at build time?

If nothing else, can you add a few lines to the commit
message why this can't be/is preferably not done at build
time?

brgds, H-P

Re: [committed] libstdc++: Fix aligned formatting of stacktrace_entry and thread::id [PR112564]

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Date: Mon, 20 Nov 2023 11:55:22 +

> The changelog entry does say "Change compile test to run."

Wow, it's right there.  The doh:est of doh:s on me.  Sorry
for wasting your time on that.

> > PS. Sorry, I have no idea why regarding the underlying multi-target problem
> 
> I have some vague speculation in PR 112541.

>From comment #1:
"It appears that __stacktrace_impl::_S_current returns an
empty sequence of frames.

It's possible that all the lambda frames are inlined, or
skip+2 in stacktrace.cc causes us to skip real frames that
we should keep, or maybe libbacktrace just doesn't work on
this target."

All more or less likely, with libbacktrace not working for
three targets maybe less likely.  Or more.

Anyway, if I haven't found time to look at this myself
within...say two months, I think I'll xfail it for cris-elf.

brgds, H-P

Re: [committed] libstdc++: Fix aligned formatting of stacktrace_entry and thread::id [PR112564]

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Date: Thu, 16 Nov 2023 17:20:09 +

>   PR libstdc++/112564
>   * include/std/stacktrace (formatter::format): Format according
>   to format-spec.
>   * include/std/thread (formatter::format): Use _Align_right as
>   default.
>   * testsuite/19_diagnostics/stacktrace/output.cc: Check
>   fill-and-align handling. Change compile test to run.
>   * testsuite/30_threads/thread/id/output.cc: Check fill-and-align
>   handling.

You already know this, so JFTR: this introduced a regression
for some targets, logged as PR112630.

Was this change deliberate:

> --- a/libstdc++-v3/testsuite/19_diagnostics/stacktrace/output.cc
> +++ b/libstdc++-v3/testsuite/19_diagnostics/stacktrace/output.cc
> @@ -1,4 +1,5 @@
> -// { dg-do compile { target c++23 } }
> +// { dg-options "-lstdc++exp" }
> +// { dg-do run { target c++23 } }
>  // { dg-require-effective-target stacktrace }
>  // { dg-add-options no_pch }

i.e. changing from dg-compile to dg-run?

I'm guessing so.  Though the changelog entry and post isn't
explicit, the use of VERIFY is rather clear and most tests
in 19_diagnostics/stacktrace are dg-run.

If so, can the "dg-run-ness" of the test please move to a
separate test and let 19_diagnostics/stacktrace/output.cc be
just dg-compile?  This particular test may not warrant the
consideration, but more so a pattern to follow for other
tests.

brgds, H-P
PS. Sorry, I have no idea why regarding the underlying multi-target problem

Re: [committed] libstdc++: Implement std::out_ptr and std::inout_ptr for C++23 [PR111667]

[Hans-Peter Nilsson]

> From: Jonathan Wakely 
> Date: Thu, 16 Nov 2023 08:12:39 +

>   PR libstdc++/111667
>   * include/Makefile.am: Add new header.
>   * include/Makefile.in: Regenerate.
>   * include/bits/out_ptr.h: New file.
>   * include/bits/shared_ptr.h (__is_shared_ptr): Move definition
>   to here ...
>   * include/bits/shared_ptr_atomic.h (__is_shared_ptr): ... from
>   here.
>   * include/bits/shared_ptr_base.h (__shared_count): Declare
>   out_ptr_t as a friend.
>   (_Sp_counted_deleter, __shared_ptr): Likewise.
>   * include/bits/unique_ptr.h (unique_ptr, unique_ptr):
>   Declare out_ptr_t and inout_ptr_t as friends.
>   (__is_unique_ptr): Define new variable template.
>   * include/bits/version.def (out_ptr): Define.
>   * include/bits/version.h: Regenerate.
>   * include/std/memory: Include new header.
>   * testsuite/20_util/smartptr.adapt/inout_ptr/1.cc: New test.
>   * testsuite/20_util/smartptr.adapt/inout_ptr/2.cc: New test.
>   * testsuite/20_util/smartptr.adapt/inout_ptr/shared_ptr_neg.cc:
>   New test.
>   * testsuite/20_util/smartptr.adapt/inout_ptr/void_ptr.cc: New
>   test.
>   * testsuite/20_util/smartptr.adapt/out_ptr/1.cc: New test.
>   * testsuite/20_util/smartptr.adapt/out_ptr/2.cc: New test.
>   * testsuite/20_util/smartptr.adapt/out_ptr/shared_ptr_neg.cc:
>   New test.
>   * testsuite/20_util/smartptr.adapt/out_ptr/void_ptr.cc: New
>   test.

This commit, r14-5524-gc7f6537db94f7c, exposed or caused, for several targets:
FAIL: g++.dg/modules/xtreme-header-4_b.C -std=c++2b (internal compiler error: 
tree check: expected class 'type', have 'declaration' (template_decl) in 
get_originating_module_decl, at cp/module.cc:18649)
FAIL: g++.dg/modules/xtreme-header-4_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2b (internal compiler error: 
tree check: expected class 'type', have 'declaration' (template_decl) in 
get_originating_module_decl, at cp/module.cc:18649)
FAIL: g++.dg/modules/xtreme-header-5_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2b (internal compiler error: 
tree check: expected class 'type', have 'declaration' (template_decl) in 
get_originating_module_decl, at cp/module.cc:18649)
FAIL: g++.dg/modules/xtreme-header_b.C -std=c++2b (test for excess errors)

See PR 112580.

(BTW, I can't tell from the archive, whether the
"Linaro-TCWG-CI" tester notified you or just sent its report
to the gcc-regression@ list.)

brgds, H-P

Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]

[Hans-Peter Nilsson]

> From: Martin Uecker 
> Date: Tue, 07 Nov 2023 06:56:25 +0100

> Am Montag, dem 06.11.2023 um 21:01 -0700 schrieb Jeff Law:
> > 
> > On 11/6/23 20:58, Hans-Peter Nilsson wrote:
> > > This patch caused a testsuite regression: there's now an
> > > "excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
> > > targets (and 64-bit targets testing with a "-m32" option)
> > > after your r14-5115-g6e9ee44d96e5.  It's logged as PR112419.
> > It caused failures for just about every target ;(  Presumably it worked 
> > on x86_64...
> 
> I do not think this is a true regression
> just a problem with the test on 32-bit which somehow surfaced
> due to the change.
> 
> The excess error is:
> 
> FAIL: gcc.dg/Wnonnull-4.c (test for excess errors)
> Excess errors:
> /home/tcwg-buildslave/workspace/tcwg_gnu_6/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.dg/Wnonnull-4.c:144:3:
>  warning: 'fda_n_5' specified size 4294967256 exceeds maximum object size
> 2147483647 [-Wstringop-overflow=]
> 
> I think the warning was suppressed before due to the other (nonnull)
> warning which I removed in this case.
> 
> I think the simple fix might be to to turn off -Wstringop-overflow.

No, that trigs many of the dg-warnings that are tested.

(I didn't pay attention to the actual warning messages and
tried to pursue that at first.)

Maybe think it's best to actually expect the warning, like
so.

Maintainers of 16-bit targets will have to address their
concerns separately.  For example, they may choose to not
run the test at all.

Ok to commit?

Subject: [PATCH] gcc.dg/Wnonnull-4.c: Handle new overflow warning for 32-bit 
targets [PR112419]

PR testsuite/112419
* gcc.dg/Wnonnull-4.c (test_fda_n_5): Expect warning for exceeding
maximum object size for 32-bit targets.
---
 gcc/testsuite/gcc.dg/Wnonnull-4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c 
b/gcc/testsuite/gcc.dg/Wnonnull-4.c
index 1f14fbba45df..d63e76da70a2 100644
--- a/gcc/testsuite/gcc.dg/Wnonnull-4.c
+++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c
@@ -142,6 +142,7 @@ void test_fda_n_5 (int r_m1)
   T (  1);  // { dg-bogus "argument 2 of variable length array 
'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 1" 
}
   T (  9);  // { dg-bogus "argument 2 of variable length array 
'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 9" 
}
   T (max);  // { dg-bogus "argument 2 of variable length array 
'double\\\[n]\\\[5]' is null but the corresponding bound argument 1 value is 
\\d+" }
+// { dg-warning "size 4294967256 exceeds maximum object size" "" { target 
ilp32 } .-1 }
 }
 
 
-- 
2.30.2

Re: [committed] testsuite: Ignore warning for unsupported option

[Hans-Peter Nilsson]

> From: Dimitar Dimitrov 
> Date: Tue, 14 Nov 2023 22:00:14 +0200

> The -w option was used in gcc.dg/20020206-1.c to ignore warnings if the
> '-fprefetch-loop-arrays' option is not supported by target.
> 
> When commit r14-5380-g5c432b0efab54e removed the -w option, some targets
> (arm-none-eabi, pru and possibly others) started failing the test:

(FWIW, all targets that don't implement and enable a
"prefetch" insn pattern.)

>   cc1: warning: '-fprefetch-loop-arrays' not supported for this target
>   FAIL: gcc.dg/20020206-1.c (test for excess errors)
> 
> Fix by instructing DejaGnu to prune the '-fprefetch-loop-arrays'
> warning.
> 
> Pushed to trunk as an obvious fix.

Another obvious fix would be to reinstate the removed -w (as
it remains on other tests passing -fprefetch-loop-arrays).

A fix I now don't have to commit, so thanks!

A much less obvious but IMHO also valid fix, would be to
remove the warning (but then for all users), as the option
is not documented to emit a warning and it appears there are
no tests for the warning.  The documentation says "If
supported by the target machine, generate instructions to
prefetch memory" with no further description on specifics of
what happens when it's not supported.  But, such a change
can always have non-obvious fallout.

brgds, H-P

Re: [PATCH v2 1/7] aarch64: Use br instead of ret for eh_return

[Hans-Peter Nilsson]

> From: Szabolcs Nagy 
> Date: Fri, 3 Nov 2023 15:36:08 +

I don't see others commenting on this patch, and you're not
mentioning this aspect, so I wonder:

>   * config/aarch64/aarch64.h (EH_RETURN_TAKEN_RTX): Define.
>   (EH_RETURN_STACKADJ_RTX): Change to R5.
>   (EH_RETURN_HANDLER_RTX): Change to R6.

Isn't this an ABI change?

(I've forgotten relevant bits of the exception machinery; if
throw and catch are always in the same object and everything
in between register-number-agnostic then the only flaw would
be not mentioning that in the commit message.)

brgds, H-P

Re: [PATCH] Reduce false positives for -Wnonnull for VLA parameters [PR98541]

[Hans-Peter Nilsson]

> From: Martin Uecker 
> Date: Tue, 31 Oct 2023 20:05:09 +0100

> Reduce false positives for -Wnonnull for VLA parameters [PR98541]
> 
> This patch limits the warning about NULL arguments to VLA
> parameters declared [static n].
> 
> PR c/98541
> 
> gcc/
> * gimple-ssa-warn-access.cc
> (pass_waccess::maybe_check_access_sizes): For VLA bounds
> in parameters, only warn about null pointers with 'static'.
> 
> gcc/testsuite:
> * gcc.dg/Wnonnull-4: Adapt test.
> * gcc.dg/Wstringop-overflow-40.c: Adapt test.

This patch caused a testsuite regression: there's now an
"excess error" failure for gcc.dg/Wnonnull-4.c for 32-bit
targets (and 64-bit targets testing with a "-m32" option)
after your r14-5115-g6e9ee44d96e5.  It's logged as PR112419.

brgds, H-P

Re: [PATCH] recog/reload: Remove old UNARY_P operand support

[Hans-Peter Nilsson]

> From: Richard Sandiford 
> Date: Tue, 24 Oct 2023 11:14:20 +0100

> reload and constrain_operands had some old code to look through unary
> operators.  E.g. an operand could be (sign_extend (reg X)), and the
> constraints would match the reg rather than the sign_extend.
> 
> This was previously used by the MIPS port.  But relying on it was a
> recurring source of problems, so Eric and I removed it in the MIPS
> rewrite from ~20 years back.  I don't know of any other port that used it.

The SH did.  I remember this being one of the ugliest warts
of reload.  IIRC, there was a bit of a discourse involving
me and Joern way-back (also IIRC some 20 years ago, at least
before IRA and LRA).  The conclusion was that removing this
misfeature would be ok, as already at that time, there was
no de-facto beneficial effect for sh, likely due to code
rot.  However, no action was taken; no code changed.

Thanks for removing the last(?) bits!

brgds, H-P

Re: Enable top-level recursive 'autoreconf'

[Hans-Peter Nilsson via Gcc]

> From: Thomas Schwinge 
> Date: Thu, 19 Oct 2023 12:42:26 +0200

> It's just GCC and Binutils/GDB, or are the top-level files also shared
> with additional projects?

Not sure if that counts as "shared", but I regularly drop
in* newlib to build simulator targets (*-elf, *-newabi).
That's git://sourceware.org/git/newlib-cygwin.git but I
extract tarballs for the merging.

"Drop in" is actually the other way round, starting with a
newlib tarball, so gcc files overwrite newlib ones.

(FWIW, I never drop in binutils like that; they're better
built separately.  "Better" for your long-term sanity.)

brgds, H-P

Re: Enable top-level recursive 'autoreconf'

[Hans-Peter Nilsson]

> From: Thomas Schwinge 
> Date: Thu, 19 Oct 2023 12:42:26 +0200

> It's just GCC and Binutils/GDB, or are the top-level files also shared
> with additional projects?

Not sure if that counts as "shared", but I regularly drop
in* newlib to build simulator targets (*-elf, *-newabi).
That's git://sourceware.org/git/newlib-cygwin.git but I
extract tarballs for the merging.

"Drop in" is actually the other way round, starting with a
newlib tarball, so gcc files overwrite newlib ones.

(FWIW, I never drop in binutils like that; they're better
built separately.  "Better" for your long-term sanity.)

brgds, H-P

Re: [committed] RISC-V: Fix INSN costing and more zicond tests

[Hans-Peter Nilsson]

> Date: Fri, 29 Sep 2023 16:37:21 -0600
> From: Jeff Law 

> So this ends up looking a lot like the bits that I had to revert several 
> weeks ago :-)
> 
> The core issue we have is given an INSN the generic code will cost the 
> SET_SRC and SET_DEST and sum them.  But that's far from ideal on a RISC 
> target.
> 
> For a register destination, the cost can be determined be looking at 
> just the SET_SRC.  Which is precisely what this patch does.  When the 
> outer code is an INSN and we're presented with a SET we take one of two 
> paths.
> 
> If the destination is a register, then we recurse just on the SET_SRC 
> and we're done.  Otherwise we fall back to the existing code which sums 
> the cost of the SET_SRC and SET_DEST.

Ackchyually...  that "otherwise" happens for calls to
set_rtx_cost (et al), but not calls to insn_cost.

IOW, with that patch, it seems you're mimicking insn_cost
behavior also for set_rtx_cost (et al).  You're likely aware
of this, but when seeing these target cost functions tweaked
for reasons that appear somewhat empirical, I felt compelled
to point out the related rabbit-hole.

While I'm ranting, these slightly different cost api:s,
somewhat arbitrarily, (or empirically) picked by callers, is
a problem by itself.  Not to mention that the default use of
set_rtx_cost means you get hit by another problem; the
default cost of 0 for registers is also a magic number to
pattern_cost to set the cost to INSN_COSTS (1).

The default insn_cost implementation, which RISC-V uses as
opposed to implementing the TARGET_INSN_COST hook, only
looks at the SET_SRC for calls to insn_cost for single-sets.
See pattern_cost.  I believe that's a bug.  Fixing that was
attempted in 2016 (by Bernd S.), a patch which was later
reverted: cf. commits r7-4866-g334442f282a9d6 and
r7-4930-g03612f25277590.  Hence rabbit-hole.  (And no,
implementing TARGET_INSN_COST doesn't automatically fix
things.  Too much of the gcc middle-end appears tuned to the
default behavior.)

Sorry for the rant; have a nice day and a better week-end.

>  That fallback path isn't great 
> and probably could be further improved (just costing SET_DEST in that 
> case is probably quite reasonable).
> 
> The difference between this version and the bits that slipped through by 
> accident several weeks ago is that old version mis-used the API due to a 
> thinko on my part.
> 
> This tightens up various zicond tests to avoid undesirable matching.
> 
> This has been tested on rv64gc -- the only difference it makes on the 
> testsuite is the new tests (included in this patch) flip from failing to 
> passing.
> 
> Pushed to the trunk.
> 
> Jeff

brgds, H-P

Re: [RFC] expr: don't clear SUBREG_PROMOTED_VAR_P flag for a promoted subreg [target/111466]

[Hans-Peter Nilsson]

> From: Vineet Gupta 
> Date: Thu, 28 Sep 2023 14:43:41 -0700

Please forgive my daftness, but...

> ```
> foo2:
>   sext.w  a6,a1 <-- this goes away
>   beq a1,zero,.L4
>   li  a5,0
>   li  a0,0
> .L3:
>   addwa4,a2,a5
>   addwa5,a3,a5
>   addwa0,a4,a0
>   bltua5,a6,.L3
>   ret
> .L4:
>   li  a0,0
>   ret
> ```

...if your patch gets rid of that sign-extension above...

> diff --git a/gcc/testsuite/gcc.target/riscv/pr111466.c 
> b/gcc/testsuite/gcc.target/riscv/pr111466.c
> new file mode 100644
> index ..007792466a51
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/pr111466.c
> @@ -0,0 +1,15 @@
> +/* Simplified varaint of gcc.target/riscv/zba-adduw.c.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zba_zbs -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" } } */
> +
> +int foo2(int unused, int n, unsigned y, unsigned delta){
> +  int s = 0;
> +  unsigned int x = 0;
> +  for (;x +s += x+y;
> +  return s;
> +}
> +
> +/* { dg-final { scan-assembler "\msext\M" } } */

...then why test for the presence of a sign-extension
instruction in the test-case?

IOW, shouldn't that be a scan-assember-not?

(What am I missing?)

brgds, H-P
PS. sorry I missed the Cauldron this year.  Hope to see you all next year!