[PATCH] RISC-V: Fix vec_duplicate[bimode] expander [PR119572].

2025-04-02 Thread Robin Dapp

Hi,

since r15-9062-g70391e3958db79 we perform vector bitmask initialization
via the vec_duplicate expander directly.  This triggered a latent bug in
ours where we missed to mask out the single bit which resulted in an
execution FAIL of pr119114.c

The attached patch adds the 1-masking of the broadcast operand.

Regtested on rv64gcv_zvl512b.

Regards
Robin

PR target/119572

gcc/ChangeLog:

* config/riscv/autovec.md: Mask broadcast value.
---
gcc/config/riscv/autovec.md | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index f53ed3a5e3f..9e51e3ce6a3 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -330,7 +330,15 @@ (define_expand "vec_duplicate"
  {
poly_int64 nunits = GET_MODE_NUNITS (mode);
machine_mode mode = riscv_vector::get_vector_mode (QImode, nunits).require 
();
-rtx dup = expand_vector_broadcast (mode, operands[1]);
+
+/* The 1-bit mask is in a QImode register, make sure we only use the last
+   bit.  See also PR119114 and the respective vec_init expander.  */
+rtx tmp = gen_reg_rtx (Xmode);
+emit_insn
+  (gen_rtx_SET (tmp, gen_rtx_AND (Xmode, gen_lowpart (Xmode, operands[1]),
+ CONST1_RTX (Xmode;
+
+rtx dup = expand_vector_broadcast (mode, gen_lowpart (QImode, tmp));
riscv_vector::expand_vec_cmp (operands[0], NE, dup, CONST0_RTX (mode));
DONE;
  }
--
2.48.1



[PATCH v2] RISC-V: Fixbug for slli + addw + zext.w into sh[123]add + zext.w

2025-04-02 Thread Jin Ma
Assuming we have the following variables:

unsigned long long a0, a1;
unsigned int a2;

For the expression:

a0 = (a0 << 50) >> 49;  // slli a0, a0, 50 + srli a0, a0, 49
a2 = a1 + a0;   // addw a2, a1, a0 + slli a2, a2, 32 + srli a2, a2, 32

In the optimization process of ZBA (combine pass), it would be optimized to:

a2 = a0 << 1 + a1;  // sh1add a2, a0, a1 + zext.w a2, a2

This is clearly incorrect, as it overlooks the fact that a0=a0&0x7ffe, meaning
that the bits a0[32:14] are set to zero.

gcc/ChangeLog:

* config/riscv/bitmanip.md: The optimization can only be applied if
the high bit of operands[3] is set to 1.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/zba-shNadd-09.c: New test.
* gcc.target/riscv/zba-shNadd-10.c: New test.
---
 gcc/config/riscv/bitmanip.md  |  4 +++-
 .../gcc.target/riscv/zba-shNadd-09.c  | 12 +++
 .../gcc.target/riscv/zba-shNadd-10.c  | 21 +++
 3 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/zba-shNadd-09.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/zba-shNadd-10.c

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index b29c127bcb8..5ed5e18cb36 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -80,7 +80,9 @@ (define_split
(match_operand:DI 3 
"consecutive_bits_operand")) 0)
 (subreg:SI (match_operand:DI 4 
"register_operand") 0]
   "TARGET_64BIT && TARGET_ZBA
-   && riscv_shamt_matches_mask_p (INTVAL (operands[2]), INTVAL (operands[3]))"
+   && riscv_shamt_matches_mask_p (INTVAL (operands[2]), INTVAL (operands[3]))
+   /* Ensure the mask includes all the bits in SImode.  */
+   && ((INTVAL (operands[3]) & (HOST_WIDE_INT_1U << 31)) != 0)"
   [(set (match_dup 0) (plus:DI (ashift:DI (match_dup 1) (match_dup 2)) 
(match_dup 4)))
(set (match_dup 0) (zero_extend:DI (subreg:SI (match_dup 0) 0)))])
 
diff --git a/gcc/testsuite/gcc.target/riscv/zba-shNadd-09.c 
b/gcc/testsuite/gcc.target/riscv/zba-shNadd-09.c
new file mode 100644
index 000..303f3cbb863
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zba-shNadd-09.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zba -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+long long sub (unsigned long long a, unsigned long long b)
+{
+  b = (b << 50) >> 49;
+  unsigned int x = a + b;
+  return x;
+}
+
+/* { dg-final { scan-assembler-not {\msh1add} } } */
diff --git a/gcc/testsuite/gcc.target/riscv/zba-shNadd-10.c 
b/gcc/testsuite/gcc.target/riscv/zba-shNadd-10.c
new file mode 100644
index 000..883cce271ca
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zba-shNadd-10.c
@@ -0,0 +1,21 @@
+/* { dg-do run { target { rv64 } } } */
+/* { dg-options "-march=rv64gc_zba -mabi=lp64d -O2" } */
+
+struct {
+  unsigned a : 14;
+  unsigned b : 3;
+} c;
+
+unsigned long long d;
+void e (unsigned long long *f, long p2) { *f = p2; }
+signed g;
+long i;
+
+int main () {
+  c.b = 4;
+  i = -(-c.a - (3023282U + c.a + g));
+  e (&d, i);
+  if (d != 3023282)
+__builtin_abort ();
+  __builtin_exit (0);
+}
-- 
2.25.1



Re: [PATCH] RISC-V: Fix vec_duplicate[bimode] expander [PR119572].

2025-04-02 Thread 钟居哲
LGTM



juzhe.zh...@rivai.ai
 
From: Robin Dapp
Date: 2025-04-02 15:02
To: gcc-patches
CC: pal...@dabbelt.com; kito.ch...@gmail.com; juzhe.zh...@rivai.ai; 
jeffreya...@gmail.com; pan2...@intel.com; rdapp@gmail.com
Subject: [PATCH] RISC-V: Fix vec_duplicate[bimode] expander [PR119572].
Hi,
 
since r15-9062-g70391e3958db79 we perform vector bitmask initialization
via the vec_duplicate expander directly.  This triggered a latent bug in
ours where we missed to mask out the single bit which resulted in an
execution FAIL of pr119114.c
 
The attached patch adds the 1-masking of the broadcast operand.
 
Regtested on rv64gcv_zvl512b.
 
Regards
Robin
 
PR target/119572
 
gcc/ChangeLog:
 
* config/riscv/autovec.md: Mask broadcast value.
---
gcc/config/riscv/autovec.md | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index f53ed3a5e3f..9e51e3ce6a3 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -330,7 +330,15 @@ (define_expand "vec_duplicate"
   {
 poly_int64 nunits = GET_MODE_NUNITS (mode);
 machine_mode mode = riscv_vector::get_vector_mode (QImode, nunits).require 
();
-rtx dup = expand_vector_broadcast (mode, operands[1]);
+
+/* The 1-bit mask is in a QImode register, make sure we only use the last
+   bit.  See also PR119114 and the respective vec_init expander.  */
+rtx tmp = gen_reg_rtx (Xmode);
+emit_insn
+  (gen_rtx_SET (tmp, gen_rtx_AND (Xmode, gen_lowpart (Xmode, operands[1]),
+   CONST1_RTX (Xmode;
+
+rtx dup = expand_vector_broadcast (mode, gen_lowpart (QImode, tmp));
 riscv_vector::expand_vec_cmp (operands[0], NE, dup, CONST0_RTX (mode));
 DONE;
   }
-- 
2.48.1
 
 


Re: [PATCH v2 04/18] Add microMIPS R6 support

2025-04-02 Thread Maciej W. Rozycki
On Mon, 17 Mar 2025, Aleksandar Rakic wrote:

>   * config/mips/mips.h:
>   (TARGET_MICROMIPS_R6): Define.

 Umm, this change can't go forward I'm afraid without microMIPSr6 support 
landing in binutils first.  Otherwise we have no consumer for compiled 
code available.  Also is there a way to verify this target, e.g. QEMU?

  Maciej


[PATCH] inline: Add a call to unreachable to the return block for noreturn calls [PR119599]

2025-04-02 Thread Andrew Pinski
builtin_unreachable_bb_p exacts empty basic blocks to have only one successor 
edge but
in the case after inliing a noreturn function that actually returns, we end up 
with an
empty basic block with no successor edge. fixup_cfg fixes this up by placing a 
call to
__builtin_unreachable but we don't do that before getting the function summary
(after einiline). So the fix is to have the inliner insert the call
to __builtin_unreachable (or the trap version) and not depend on the fixup if 
the return
block is reachable after inlining a noreturn function.

A small optimization is done not to insert the call if the block is NOT 
simplely reachable.
This should prevent inserting the call just to remove it during cleanupcfg.

Note this shows up only at -O0 with always_inline because when optimizations 
are turned on
returns are turned into __builtin_unreachable inside noreturn functions.

Bootstrapped and tested on x86_64-linux-gnu.

PR ipa/119599

gcc/ChangeLog:

* tree-inline.cc (expand_call_inline): Add a __builtin_unreachable call
to the return block if it is still reachable and the call that was 
inlined
was a noreturn function.

gcc/testsuite/ChangeLog:

* gcc.dg/torture/pr119599-1.c: New test.

Signed-off-by: Andrew Pinski 
---
 gcc/testsuite/gcc.dg/torture/pr119599-1.c | 27 +++
 gcc/tree-inline.cc| 15 +
 2 files changed, 42 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr119599-1.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr119599-1.c 
b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
new file mode 100644
index 000..4fbd228a359
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-einline" } */
+
+/* PR ipa/119599 */
+/* inlining a noreturn function which returns
+   can cause an ICE when dealing finding an unreachable block.
+   We should get a __builtin_unreachable after the inliing.  */
+
+
+void baz (void);
+
+static inline __attribute__((always_inline, noreturn)) void
+bar (void)
+{
+  static volatile int t = 0;
+  if (t == 0)
+baz ();
+} /* { dg-warning "function does return" } */
+
+void
+foo (void)
+{
+  bar ();
+}
+
+/* After inlining, we should have call to __builtin_unreachable now. */
+/* { dg-final { scan-tree-dump "__builtin_unreachable " "einline" } } */
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 05843b8ccf0..da403cd1456 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -4832,6 +4832,7 @@ expand_call_inline (basic_block bb, gimple *stmt, 
copy_body_data *id,
   gimple *simtenter_stmt = NULL;
   vec *simtvars_save;
   tree save_stack = NULL_TREE;
+  bool was_noreturn;
 
   /* The gimplifier uses input_location in too many places, such as
  internal_get_tmp_var ().  */
@@ -4843,6 +4844,8 @@ expand_call_inline (basic_block bb, gimple *stmt, 
copy_body_data *id,
   if (!call_stmt)
 goto egress;
 
+  was_noreturn = gimple_call_noreturn_p (call_stmt);
+
   cg_edge = id->dst_node->get_edge (stmt);
   gcc_checking_assert (cg_edge);
   /* First, see if we can figure out what function is being called.
@@ -5376,6 +5379,18 @@ expand_call_inline (basic_block bb, gimple *stmt, 
copy_body_data *id,
   if (purge_dead_abnormal_edges)
 bitmap_set_bit (to_purge, return_block->index);
 
+  /* If this was a noreturn function and if the return block is still
+ reachable, then add a call to __builtin_unreachable().  */
+  if (was_noreturn && EDGE_COUNT (return_block->preds) != 0)
+{
+  gcall *call_stmt = gimple_build_builtin_unreachable (input_location);
+  gimple_stmt_iterator gsi = gsi_last_bb (return_block);
+  gsi_insert_after (&gsi, call_stmt, GSI_NEW_STMT);
+  tree fndecl = gimple_call_fndecl (call_stmt);
+  cg_edge->caller->create_edge (cgraph_node::get_create (fndecl),
+   call_stmt, return_block->count);
+}
+
   /* If the value of the new expression is ignored, that's OK.  We
  don't warn about this for CALL_EXPRs, so we shouldn't warn about
  the equivalent inlined version either.  */
-- 
2.43.0



Re: [RFC] [C]New syntax for the argument of counted_by attribute for C language

2025-04-02 Thread Qing Zhao
Jakub,

Thank you for the comments.

> On Apr 2, 2025, at 02:48, Jakub Jelinek  wrote:
> 
> On Tue, Apr 01, 2025 at 05:13:46PM -0700, Bill Wendling wrote:
>> On Tue, Apr 1, 2025 at 8:29 AM Martin Uecker  wrote:
>>> Am Dienstag, dem 01.04.2025 um 15:01 + schrieb Qing Zhao:
> On Apr 1, 2025, at 10:04, Martin Uecker  wrote:
> Am Montag, dem 31.03.2025 um 13:59 -0700 schrieb Bill Wendling:
>>> I'd like to offer up this to solve the issues we're facing. This is a
>>> combination of everything that's been discussed here (or at least that
>>> I've been able to read in the centi-thread :-).
> 
> Thanks! I think this proposal much better as it avoids undue burden
> on parsers, but it does not address all my concerns.
> 
> 
> From my side, the issue about compromising the scoping rules of C
> is also about unintended non-local effects of code changes. In
> my opinion, a change in a library header elsewhere should not cause
> code in a local scope (which itself might also come from a macro) to
> emit a warning or require a programmer to add a workaround. So I am
> not convinced that adding warnings or a workaround such as
> __builtin_global_ref  is a good solution.
>> 
>> To clarify, I'm not in favor of adding a generalized new scoping rule
>> to C, but only for identifiers in attributes. From what I've seen in
> 
> There are existing attributes which support general expressions in their
> arguments, e.g. the assume attribute,

https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#index-assume-statement-attribute

"
assume
The assume attribute with a null statement serves as portable assumption. It 
should have a single argument, a conditional expression, which is not 
evaluated. If the argument would evaluate to true at the point where it 
appears, it has no effect, otherwise there is undefined behavior. This is a GNU 
variant of the ISO C++23 standard assume attribute, but it can be used in any 
version of both C and C++.
int
foo (int x, int y)
{
__attribute__((assume(x == 42)));
__attribute__((assume(++y == 43)));
return x + y;
}

y is not actually incremented and the compiler can but does not have to 
optimize it to just return 42 + 42;.
“

The expressions inside the assume attribute are evaluated as a regular 
expressions by following C’s scoping rules.

> OpenMP attributes etc.  Those definitely
> shouldn't change behavior if some new behavior for identifier lookup in 
> attributes
> is added.
> Many others do support identifiers as their arguments (e.g. mode attribute,
> 2 argument malloc attribute, ...).  Those can't change behavior either.
> 
> I think using either a predefined identifier or self-declared one is the
> only reasonable way to go (whether it represents something like this pointer
> in C++ or in the latter case possibly forward declaration of the member), 
> otherwise
> it will be hard to understand for users and very much error prone.

So, my understanding of your above is:
Either

1. The approach to add a new keyword, such as, __self, which represent the 
similar concept of “this” in C++;
Or
2. The approach to add the forward declaration  of the member, such as:
 __counted_by(size_t len; len + PADDING)

Is good?

Yes, I agree, and personally I prefer 1, I think it’s the best solution. And 
the implementation in GCC FE might be the
cleanest one.

However, I am okay with 2 as a compromise approach. 

thanks.

Qing

> 
> Jakub




Re: Pushed r15-9167: [PATCH] LoongArch: Make gen-evolution.awk compatible with FreeBSD awk

2025-04-02 Thread Lulu Cheng



在 2025/4/3 上午11:12, Xi Ruoyao 写道:

On Thu, 2025-04-03 at 10:13 +0800, Lulu Cheng wrote:

在 2025/4/2 上午11:19, Xi Ruoyao 写道:

Avoid using gensub that FreeBSD awk lacks, use gsub and split those
each
of gawk, mawk, and FreeBSD awk provides.

Reported-by: mp...@vip.163.com
Link: https://man.freebsd.org/cgi/man.cgi?query=awk

gcc/ChangeLog:

* config/loongarch/genopts/gen-evolution.awk: Avoid using
gensub
that FreeBSD awk lacks.
---

Manually tested the script with gawk and FreeBSD awk.  Ok for trunk?

OK.

Thanks!

Pushed now.


Could you backpoint it  to gcc 14?

Thanks.


   gcc/config/loongarch/genopts/gen-evolution.awk | 8 +---
   1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gcc/config/loongarch/genopts/gen-evolution.awk
b/gcc/config/loongarch/genopts/gen-evolution.awk
index bf16b26760e..142b658fe7a 100644
--- a/gcc/config/loongarch/genopts/gen-evolution.awk
+++ b/gcc/config/loongarch/genopts/gen-evolution.awk
@@ -33,10 +33,12 @@ BEGIN {
   {
   cpucfg_word[NR] = $1
   cpucfg_bit_in_word[NR] = $2
-    name[NR] = gensub(/-/, "_", "g", $3)
+    name[NR] = $3
+    gsub("-", "_", name[NR])
   name_capitalized[NR] = toupper(name[NR])
-    isa_version_major[NR] = gensub(/^([1-9][0-9]*)\.([0-9]+)$/,
"\\1", 1, $4)
-    isa_version_minor[NR] = gensub(/^([1-9][0-9]*)\.([0-9]+)$/,
"\\2", 1, $4)
+    split($4, isa_ver, "\\.")
+    isa_version_major[NR] = isa_ver[1]
+    isa_version_minor[NR] = isa_ver[2]
   
   $1 = $2 = $3 = $4 = ""

   sub (/^\s*/, "")




Re: [PATCH] combine: Allow 2->2 combos but limit insn searches [PR116398]

2025-04-02 Thread Richard Biener
On Tue, 1 Apr 2025, Richard Sandiford wrote:

> The problem in PR101523 was that, after each successful 2->2
> combination attempt, distribute_links would search further and
> further for the next combinable use of the i2 destination.
> The original patch for the PR dealt with that by disallowing such
> combinations.  However, that led to various optimisation regressions,
> so there was interest in allowing the combinations again, at least
> until an alternative way of getting the same results is in place.
> 
> This patch does that, but limits distribute_links to a certain number
> of instructions when i2 is unchanged.  As Segher said in the PR trail,
> it would make more conceptual sense to apply the limit unconditionally,
> but I thought it would be better to change as little as possible at
> this development stage.  Logically, in stage 1, the --param should
> be applied directly by distribute_links with no input from callers.
> 
> As I mentioned in:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116398#c28
> 
> I think it's safe to drop log links even if a use exists.  All
> processing of log links seems to handle the absence of a link
> for a particular register in a conservative way.
> 
> The initial set-up errs on the side of dropping links, since for example
> create_log_links has:
> 
>  /* flow.c claimed:
> 
>  We don't build a LOG_LINK for hard registers contained
>  in ASM_OPERANDs.  If these registers get replaced,
>  we might wind up changing the semantics of the insn,
>  even if reload can make what appear to be valid
>  assignments later.  */
>   if (regno < FIRST_PSEUDO_REGISTER
>   && asm_noperands (PATTERN (use_insn)) >= 0)
> continue;
> 
> which excludes combinations by dropping log links, rather than during
> try_combine.  And:
> 
>   /* If this register is being initialized using itself, and the
>  register is uninitialized in this basic block, and there are
>  no LOG_LINKS which set the register, then part of the
>  register is uninitialized.  In that case we can't assume
>  anything about the number of nonzero bits.
> 
>  ??? We could do better if we checked this in
>  reg_{nonzero_bits,num_sign_bit_copies}_for_combine.  Then we
>  could avoid making assumptions about the insn which initially
>  sets the register, while still using the information in other
>  insns.  We would have to be careful to check every insn
>  involved in the combination.  */
> 
>   if (insn
>   && reg_referenced_p (x, PATTERN (insn))
>   && !REGNO_REG_SET_P (DF_LR_IN (BLOCK_FOR_INSN (insn)),
>REGNO (x)))
> {
>   struct insn_link *link;
> 
>   FOR_EACH_LOG_LINK (link, insn)
> if (dead_or_set_p (link->insn, x))
>   break;
>   if (!link)
> {
>   rsp->nonzero_bits = GET_MODE_MASK (mode);
>   rsp->sign_bit_copies = 1;
>   return;
> }
> }
> 
> treats the lack of a log link is a possible sign of uninitialised data,
> but that would be a missed optimisation rather than a correctness issue.
> 
> One question is what the default --param value should be.  I went with
> Jakub's suggestion of 3000 from:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116398#c25
> 
> Also, to answer Jakub's question in that comment, I tried bisecting:
> 
>   int limit = atoi (getenv ("BISECT"));
> 
> (so applying the limit for all calls from try_combine) with an
> abort in distribute_links if the limit caused a link to be skipped.
> The minimum BISECT value that allowed an aarch64-linux-gnu bootstrap
> to succeed with --enable-languages=all --enable-checking=yes,rtl,extra
> was 142, so much lower than the parameter value.  I realised too late
> that --enable-checking=release would probably have been a more
> interesting test.
> 
> Some of the try_combine changes come from Richi's patch in:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101523#c53
> 
> Bootstrapped & regression-tested on aarch64-linux-gnu,
> powerpc64le-linux-gnu and x86_64-linux-gnu.  OK to install?
> 
> Richard
> 
> 
> gcc/
>   PR testsuite/116398
>   * params.opt (-param=max-combine-search-insns=): New param.
>   * doc/invoke.texi: Document it.
>   * combine.cc (distribute_links): Add a limit parameter.
>   (try_combine): Use the new param to limit distribute_links
>   when i2 is unchanged.  Return i3 rather than i2 if i2 is unchanged.
> 
> gcc/testsuite/
>   * gcc.target/aarch64/popcnt-le-1.c: Account for commutativity of TST.
>   * gcc.target/aarch64/popcnt-le-3.c: Likewise AND.
>   * gcc.target/aarch64/sve/pred-not-gen-1.c: Revert previous patch.
>   * gcc.target/aarch64/sve/pred-not-gen-4.c: Likewise.
>   * gcc.target/aarc

PowerPC patch ping

2025-04-02 Thread Jakub Jelinek
Hi!

I'd like to ping the following PowerPC patches:

https://gcc.gnu.org/pipermail/gcc-patches/2025-March/679312.html
rs6000: Ignore OPTION_MASK_SAVE_TOC_INDIRECT differences in inlining decisions 
[PR119327]

https://gcc.gnu.org/pipermail/gcc-patches/2025-March/676989.html
target: Adjust gcc.target/powerpc/pr103515.c test [PR117207]

Thanks

Jakub



Re: [PATCH] libstdc++: Hide TLS variables in `std::call_once`

2025-04-02 Thread Martin Storsjö

On Fri, 29 Nov 2024, LIU Hao wrote:


在 2024-11-29 23:50, Jonathan Wakely 写道:

It looks like your patch is against gcc-14 not trunk, the
GLIBCXX_15.1.0 version is already there.


Sorry, I mean GLIBCXX_3.4.34 for 15.1.0


Oops that's what I used to test the patch. Reapplied to master now.


Ping - what's the status of this patch?

It would be nice to switch to the new ABI structure for std::call_once as 
soon as possible, to ease a future switch to native TLS on mingw targets, 
once https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80881 is done. (This 
quirk with the exposed cross-DLL TLS data in std::call_once also comes up 
in a number of other situations on mingw, so it would be nice to switch to 
the new form where it is hidden.)


// Martin


Re: [PATCH] c: Fix ICEs with -fsanitize=pointer-{subtract,compare} [PR119582]

2025-04-02 Thread Marek Polacek
On Wed, Apr 02, 2025 at 06:38:12PM +0200, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase ICEs because c_fully_fold isn't performed on the
> arguments of __sanitizer_ptr_{sub,cmp} builtins and so e.g.
> C_MAYBE_CONST_EXPR can leak into the gimplifier where it ICEs.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

Ok.
 
> 2025-04-02  Jakub Jelinek  
> 
>   PR c/119582
>   * c-typeck.cc (pointer_diff, build_binary_op): Call c_fully_fold on
>   __sanitizer_ptr_sub or __sanitizer_ptr_cmp arguments.
> 
>   * gcc.dg/asan/pr119582.c: New test.
> 
> --- gcc/c/c-typeck.cc.jj  2025-03-27 09:29:36.953576540 +0100
> +++ gcc/c/c-typeck.cc 2025-04-02 15:04:47.103495567 +0200
> @@ -4824,8 +4824,8 @@ pointer_diff (location_t loc, tree op0,
>if (current_function_decl != NULL_TREE
>&& sanitize_flags_p (SANITIZE_POINTER_SUBTRACT))
>  {
> -  op0 = save_expr (op0);
> -  op1 = save_expr (op1);
> +  op0 = save_expr (c_fully_fold (op0, false, NULL));
> +  op1 = save_expr (c_fully_fold (op1, false, NULL));
>  
>tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
>*instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1);
> @@ -14455,8 +14455,8 @@ build_binary_op (location_t location, en
> && current_function_decl != NULL_TREE
> && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
>   {
> -   op0 = save_expr (op0);
> -   op1 = save_expr (op1);
> +   op0 = save_expr (c_fully_fold (op0, false, NULL));
> +   op1 = save_expr (c_fully_fold (op1, false, NULL));
>  
> tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE);
> instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1);
> --- gcc/testsuite/gcc.dg/asan/pr119582.c.jj   2025-04-02 15:11:22.351048509 
> +0200
> +++ gcc/testsuite/gcc.dg/asan/pr119582.c  2025-04-02 15:11:57.600561599 
> +0200
> @@ -0,0 +1,23 @@
> +/* PR c/119582 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fsanitize=address,pointer-subtract,pointer-compare" } 
> */
> +
> +const char v;
> +typedef __PTRDIFF_TYPE__ ptrdiff_t;
> +char a;
> +const ptrdiff_t p = &a + 1 - &a;
> +const int q = (&a + 1) != &a;
> +
> +ptrdiff_t
> +foo (void)
> +{
> +  char b;
> +  return &b + (v != '\n') - &b;
> +}
> +
> +int
> +bar (void)
> +{
> +  char b;
> +  return (&b + (v != '\n')) != &b;
> +}
> 
>   Jakub
> 

Marek



[PATCH] c++: P2280R4 and speculative constexpr folding [PR119387]

2025-04-02 Thread Patrick Palka
Bootstrapped and regtested on x86_64-pc-linux, does this look
OK  for trunk/14?

-- >8 --

The testcase in this PR uses 2.5x more memory and 6x more compile
time ever since r14-5979 which implements P2280R4.  This is because
our speculative constexpr folding now does a lot more work trying to
fold ultimately non-constant calls to constexpr functions, and in turn
produces a lot of garbage.  Sometimes, we do successfully fold more
thanks to P2280R4, but it seems to be trivial stuff like calls to
std::array::size or std::addressof.  The benefit of P2280 therefore
doesn't seem worth the cost during speculative constexpr folding, so
this patch restricts the paper to only manifestly-constant evaluation.

PR c++/119387

gcc/cp/ChangeLog:

* constexpr.cc (p2280_active_p): Define.
(cxx_eval_constant_expression) : Use it to
restrict P2280 relaxations.
: Likewise.
---
 gcc/cp/constexpr.cc | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 4820bcc84aa..ee968bc4630 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1294,6 +1294,22 @@ struct constexpr_ctx {
   mce_value manifestly_const_eval;
 };
 
+/* True if the constexpr relaxations afforded by P2280R4 for unknown
+   references and objects are in effect.  */
+
+static bool
+p2280_active_p (const constexpr_ctx *ctx)
+{
+  if (ctx->manifestly_const_eval != mce_true)
+/* Disable these relaxations during speculative constexpr folding,
+   as it can significantly increase compile time/memory use
+   (PR119387).  */
+return false;
+
+  /* P2280R4 is accepted as a DR back to C++11.  */
+  return cxx_dialect >= cxx11;
+}
+
 /* Remove T from the global values map, checking for attempts to destroy
a value that has already finished its lifetime.  */
 
@@ -7792,7 +7808,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
r = TARGET_EXPR_INITIAL (r);
   if (DECL_P (r)
  /* P2280 allows references to unknown.  */
- && !(VAR_P (t) && TYPE_REF_P (TREE_TYPE (t
+ && !(p2280_active_p (ctx) && VAR_P (t) && TYPE_REF_P (TREE_TYPE (t
{
  if (!ctx->quiet)
non_const_var_error (loc, r, /*fundef_p*/false);
@@ -7844,9 +7860,9 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, 
tree t,
  r = build_constructor (TREE_TYPE (t), NULL);
  TREE_CONSTANT (r) = true;
}
-  else if (TYPE_REF_P (TREE_TYPE (t)))
+  else if (p2280_active_p (ctx) && TYPE_REF_P (TREE_TYPE (t)))
/* P2280 allows references to unknown...  */;
-  else if (is_this_parameter (t))
+  else if (p2280_active_p (ctx) && is_this_parameter (t))
/* ...as well as the this pointer.  */;
   else
{
-- 
2.49.0.111.g5b97a56fa0



[committed] hpux: Only use long long when __cplusplus >= 201103L

2025-04-02 Thread John David Anglin
Committed to trunk.  Tested on hppa64-hp-hpux11.11.

Dave
___

hpux: Only use long long when __cplusplus >= 201103L

Fixes some test failures.

2025-04-02  John David Anglin  

libstdc++-v3/ChangeLog:
* config/os/hpux/os_defines.h: Only use long long when
__cplusplus >= 201103L.

diff --git a/libstdc++-v3/config/os/hpux/os_defines.h 
b/libstdc++-v3/config/os/hpux/os_defines.h
index 30bd4c7ba14..d3a6c5ab142 100644
--- a/libstdc++-v3/config/os/hpux/os_defines.h
+++ b/libstdc++-v3/config/os/hpux/os_defines.h
@@ -57,7 +57,7 @@
We also force _GLIBCXX_USE_LONG_LONG here so that we don't have
to bastardize configure to deal with this sillyness.  */
 
-#ifdef __cplusplus
+#if __cplusplus >= 201103L
 namespace std
 {
   extern "C"


signature.asc
Description: PGP signature


Re: [committed] hpux: Only use long long when __cplusplus >= 201103L

2025-04-02 Thread Jonathan Wakely
On Wed, 2 Apr 2025 at 20:15, John David Anglin  wrote:
>
> Committed to trunk.  Tested on hppa64-hp-hpux11.11.
>
> Dave
> ___
>
> hpux: Only use long long when __cplusplus >= 201103L
>
> Fixes some test failures.
>
> 2025-04-02  John David Anglin  
>
> libstdc++-v3/ChangeLog:
> * config/os/hpux/os_defines.h: Only use long long when
> __cplusplus >= 201103L.

This isn't what the patch does though. It disables the declarations of
strtoll for C++98, but it leaves the unconditional:

#define _GLIBCXX_USE_LONG_LONG 1

Is that intentional?

>
> diff --git a/libstdc++-v3/config/os/hpux/os_defines.h 
> b/libstdc++-v3/config/os/hpux/os_defines.h
> index 30bd4c7ba14..d3a6c5ab142 100644
> --- a/libstdc++-v3/config/os/hpux/os_defines.h
> +++ b/libstdc++-v3/config/os/hpux/os_defines.h
> @@ -57,7 +57,7 @@
> We also force _GLIBCXX_USE_LONG_LONG here so that we don't have
> to bastardize configure to deal with this sillyness.  */
>
> -#ifdef __cplusplus
> +#if __cplusplus >= 201103L
>  namespace std
>  {
>extern "C"



Re: [PATCH] cobol: Fix up cobol/{charmaps,valconv}.cc rules

2025-04-02 Thread Iain Sandoe



> On 30 Mar 2025, at 15:07, Robert Dubner  wrote:
> 
> Follow-up.  I risked wrath from my family and grabbed a minute.  Tried 
> the -L thing and eliminated all the SED statements on the .h files.
> 
> It worked.
> 
> Are there reasons not to use it?

There were coments about having explicit paths in the sources being more
obvious to development - but these are currently copies made at build time
so perhaps those comments would apply more to the idea of making an
included common text.

I think, in the end, it is your call - so long as the end result is portable.

The latter is the priority (for me, at least), given we probably do not have
much time until GCC-15 branches

cheers
Iain

> 
>> -Original Message-
>> From: Robert Dubner 
>> Sent: Sunday, March 30, 2025 09:52
>> To: Jakub Jelinek ; Iain Sandoe 
>> Cc: James K. Lowden ; Richard Biener
>> ; GCC Patches 
>> Subject: RE: [PATCH] cobol: Fix up cobol/{charmaps,valconv}.cc rules
>> 
>> Folks:  Am I missing something stupid here?
>> 
>> I just tried putting
>> 
>> CXXFLAGS_SAVE=$(CXXFLAGS)
>> override CXXFLAGS:=$(CXXFLAGS_SAVE) -I DUBNER_CXXFLAGS
>> 
>> near the beginning of cobol/Make-lang.in, and
>> 
>> override CXXFLAGS:=$(CXXFLAGS_SAVE)
>> 
>> near the end.
>> 
>> I don't have time right now to do a full check, but I am seeing
>> "DUBNER_CXXFLAGS in the output of the build.
>> 
>> Is there some reason
>> 
>> override CXXFLAGS:=$(CXXFLAGS_SAVE) -I $(srcdir)/../libgcobol
>> 
>> won't solve the whole problem?  That will give me the -I I need, and
>> presumably not pollute the Makefile space.
>> 
>> 
>> 
>> Bob D.
>> 
>> 
>>> -Original Message-
>>> From: Jakub Jelinek 
>>> Sent: Saturday, March 29, 2025 12:12
>>> To: Iain Sandoe 
>>> Cc: Robert Dubner ; James K. Lowden
>>> ; Richard Biener ; GCC
>> Patches
>>> 
>>> Subject: Re: [PATCH] cobol: Fix up cobol/{charmaps,valconv}.cc rules
>>> 
>>> On Sat, Mar 29, 2025 at 04:03:07PM +, Iain Sandoe wrote:
 
 
> On 29 Mar 2025, at 15:56, Jakub Jelinek  wrote:
> 
> On Sat, Mar 29, 2025 at 03:50:54PM +, Iain Sandoe wrote:
>>> I'm not sure if sed -E is portable enough (sure, I know it is in
>>> POSIX, but
>>> that is not enough).
>>> How about just
>>> sed -e '/^#include/s,"\([^"]*.h\)","../../libgcobol/\1",' $& >
>>> $@
>> 
>> This, unfortunately, works too well (with s/&/^) .. because it also
>>> processes #include “config.h”
>> which then points to a non-existent file.  I think we want to
>> include
>>> config for both FE and
>> library (so we cannot get around it by indenting the config.h
>> include
>>> - well we could, but …)
> 
> Neither libgcobol/charmaps.cc nor libgcobol/valconv.cc has config.h
>>> include.
 
 but it’s an approved patch (just waiting for the main config change to
>>> be reviewed) and needed
 for the other lib changes.  I’ll investigate if we could find a way to
>>> drop it fro those two files.
>>> 
>>> config.h is the only header ending with g in there, so
>>> sed -e '/^#include/s,"\([^"]*[^g"].h\)","../../libgcobol/\1",' $^ >
>>> $@
>>> then?
>>> 
>>> Jakub
> 



Re: [PATCH] APX: add nf counterparts for rotl split pattern [PR 119539]

2025-04-02 Thread Hongtao Liu
On Wed, Apr 2, 2025 at 2:58 PM Hongyu Wang  wrote:
>
> > Can we just change the output in original pattern, I think combine
> > will still match the pattern even w/ clobber flags.
>
> Yes, adjusted and updated the patch in attachment.
Ok.
>
> Liu, Hongtao  于2025年4月2日周三 08:57写道:
> >
> >
> >
> > > -Original Message-
> > > From: Uros Bizjak 
> > > Sent: Tuesday, April 1, 2025 5:24 PM
> > > To: Hongtao Liu 
> > > Cc: Wang, Hongyu ; gcc-patches@gcc.gnu.org; Liu,
> > > Hongtao 
> > > Subject: Re: [PATCH] APX: add nf counterparts for rotl split pattern [PR
> > > 119539]
> > >
> > > On Tue, Apr 1, 2025 at 10:55 AM Hongtao Liu  wrote:
> > > >
> > > > On Tue, Apr 1, 2025 at 4:40 PM Hongyu Wang 
> > > wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > For spiltter after 3_mask it now splits the
> > > > > pattern to *3_mask, causing the splitter doesn't
> > > > > generate nf variant. Add corresponding nf counterpart for
> > > > > define_insn_and_split to make the splitter also works for nf insn.
> > > > >
> > > > > Bootstrapped & regtested on x86-64-pc-linux-gnu.
> > > > >
> > > > > Ok for trunk?
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > PR target/119539
> > > > > * config/i386/i386.md (*3_mask_nf): New
> > > > > define_insn_and_split.
> > > > > (*3_mask_1_nf): Likewise.
> > > > > (*3_mask): Use force_lowpart_subreg.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > > PR target/119539
> > > > > * gcc.target/i386/apx-nf-pr119539.c: New test.
> > > > > ---
> > > > >  gcc/config/i386/i386.md   | 46 
> > > > > ++-
> > > > >  .../gcc.target/i386/apx-nf-pr119539.c |  6 +++
> > > > >  2 files changed, 50 insertions(+), 2 deletions(-)  create mode
> > > > > 100644 gcc/testsuite/gcc.target/i386/apx-nf-pr119539.c
> > > > >
> > > > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index
> > > > > f7f790d2aeb..42312f0c330 100644
> > > > > --- a/gcc/config/i386/i386.md
> > > > > +++ b/gcc/config/i386/i386.md
> > > > > @@ -18131,6 +18131,30 @@ (define_expand "3"
> > > > >DONE;
> > > > >  })
> > > > >
> > > > > +;; Avoid useless masking of count operand.
> > > > > +(define_insn_and_split "*3_mask_nf"
> > > > > +  [(set (match_operand:SWI 0 "nonimmediate_operand")
> > > > > +   (any_rotate:SWI
> > > > > + (match_operand:SWI 1 "nonimmediate_operand")
> > > > > + (subreg:QI
> > > > > +   (and
> > > > > + (match_operand 2 "int248_register_operand" "c")
> > > > > + (match_operand 3 "const_int_operand")) 0)))]
> > > > > +  "TARGET_APX_NF
> > > > > +   && ix86_binary_operator_ok (, mode, operands)
> > > > > +   && (INTVAL (operands[3]) & (GET_MODE_BITSIZE (mode)-1))
> > > > > +  == GET_MODE_BITSIZE (mode)-1
> > > > > +   && ix86_pre_reload_split ()"
> > > > > +  "#"
> > > > > +  "&& 1"
> > > > > +  [(set (match_dup 0)
> > > > > +   (any_rotate:SWI (match_dup 1)
> > > > > +   (match_dup 2)))] {
> > > > > +  operands[2] = force_lowpart_subreg (QImode, operands[2],
> > > > > + GET_MODE (operands[2]));
> > > > > +})
> > > > Can we just change the output in original pattern, I think combine
> > > > will still match the pattern even w/ clobber flags.
> > > >
> > > > like
> > > >
> > > > @@ -17851,8 +17851,17 @@ (define_insn_and_split
> > > "*3_mask"
> > > > (match_dup 2)))
> > > >(clobber (reg:CC FLAGS_REG))])]  {
> > > > -  operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
> > > > -  operands[2] = gen_lowpart (QImode, operands[2]);
> > > > +  if (TARGET_APX_F)
> > > > +{
> > > > +  emit_move_insn (operands[0],
> > > > +gen_rtx_ (mode, operands[1], 
> > > > operands[2]));
> > > > +  DONE;
> > > > +}
> > > > +  else
> > > > +{
> > > > +  operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
> > > > +  operands[2] = gen_lowpart (QImode, operands[2]);
> > >
> > > Please note we have a new "force_lowpart_subreg" function that operates on
> > > a register operand and (if possible) simplifies a subreg of a subreg, 
> > > otherwise it
> > > forces the operand to register and creates a subreg of the temporary. 
> > > Similar
> > > to the above combination, with a possibility to avoid a temporary reg.
> > >
> > > > Also we can remove constraint "c" in the original pattern.
> > >
> > > The constraint is here to handle corner case, where combine propagated an
> > > insn RTX with fixed register, other than %ecx, into shift RTX.
> > > Register allocator was not able to fix the combination, so
> > > TARGET_LEGITIMATE_COMBINED_INSN hook was introduced that rejected
> > > unwanted combinations. Please see the comment in
> > > ix86_legitimate_combined_insn function.
> > Oh, thanks for the explanation.
> > >
> > > Perhaps the above is not relevant anymore with the new register allocator
> > > (L

[PATCH] fold-const, cobol, v2: Add native_encode_wide_int and use it in COBOL FE [PR119242]

2025-04-02 Thread Jakub Jelinek
On Wed, Apr 02, 2025 at 09:22:54PM +0100, Richard Sandiford wrote:
> I'm not sure the _1 template is worth it.  wi::to_widest ->
> const wide_int_ref & should be very cheap (it doesn't for example
> construct a widest_int), so I think native_encode_int could call
> native_encode_wide_int.

You're right, I was afraid it would construct something expensive.
Looking at --enable-checking=release build, I see native_encode_int
is inlined into native_encode_expr and does
  MEM[(struct wide_int_ref_storage *)&D.261480] ={v} {CLOBBER};
  _30 = *expr_10.base.u.int_length.extended;
  _31 = (unsigned int) _30;
  _32 = &expr_10->int_cst.val[0];
  MEM  [(struct wide_int_ref_storage *)&D.261480] = _32;
  MEM  [(struct wide_int_ref_storage *)&D.261480 + 8B] = _31;
  MEM  [(struct wide_int_ref_storage *)&D.261480 + 12B] = 131072;
  _33 = expr_10->typed.type;
  _34 = native_encode_wide_int (_33, &D.261480, ptr_11, len_12, off_8);
there in optimized dump, which is
  temp.val = &TREE_INT_CST_ELT (expr, 0);
  temp.len = TREE_INT_CST_EXT_NUNITS (expr);
  temp.precision = WIDEST_INT_MAX_PRECISION;
so that seems fairly cheap.

So I'll retest:

2025-04-02  Jakub Jelinek  

PR cobol/119242
gcc/
* fold-const.h (native_encode_wide_int): Declare.
* fold-const.cc (native_encode_wide_int): New function.
(native_encode_int): Use it.
gcc/cobol/
* genapi.cc (binary_initial_from_float128): Use
native_encode_wide_int.

--- gcc/fold-const.h.jj 2025-04-02 19:26:55.300272195 +0200
+++ gcc/fold-const.h2025-04-02 22:36:19.066641205 +0200
@@ -35,6 +35,8 @@ extern bool folding_cxx_constexpr;
 extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
 extern int native_encode_initializer (tree, unsigned char *, int,
  int off = -1, unsigned char * = nullptr);
+extern int native_encode_wide_int (tree, const wide_int_ref &,
+  unsigned char *, int, int off = -1);
 extern int native_encode_real (scalar_float_mode, const REAL_VALUE_TYPE *,
   unsigned char *, int, int off = -1);
 extern tree native_interpret_expr (tree, const unsigned char *, int);
--- gcc/fold-const.cc.jj2025-04-02 19:26:55.300272195 +0200
+++ gcc/fold-const.cc   2025-04-02 22:39:04.932113465 +0200
@@ -7465,15 +7465,16 @@ fold_plusminus_mult_expr (location_t loc
   return NULL_TREE;
 }
 
-/* Subroutine of native_encode_expr.  Encode the INTEGER_CST
-   specified by EXPR into the buffer PTR of length LEN bytes.
+
+/* Subroutine of native_encode_int.  Encode the integer VAL with type TYPE
+   into the buffer PTR of length LEN bytes.
Return the number of bytes placed in the buffer, or zero
upon failure.  */
 
-static int
-native_encode_int (const_tree expr, unsigned char *ptr, int len, int off)
+int
+native_encode_wide_int (tree type, const wide_int_ref &val,
+   unsigned char *ptr, int len, int off)
 {
-  tree type = TREE_TYPE (expr);
   int total_bytes;
   if (TREE_CODE (type) == BITINT_TYPE)
 {
@@ -7516,7 +7517,7 @@ native_encode_int (const_tree expr, unsi
   int bitpos = byte * BITS_PER_UNIT;
   /* Extend EXPR according to TYPE_SIGN if the precision isn't a whole
 number of bytes.  */
-  value = wi::extract_uhwi (wi::to_widest (expr), bitpos, BITS_PER_UNIT);
+  value = wi::extract_uhwi (val, bitpos, BITS_PER_UNIT);
 
   if (total_bytes > UNITS_PER_WORD)
{
@@ -7537,6 +7538,18 @@ native_encode_int (const_tree expr, unsi
   return MIN (len, total_bytes - off);
 }
 
+/* Subroutine of native_encode_expr.  Encode the INTEGER_CST
+   specified by EXPR into the buffer PTR of length LEN bytes.
+   Return the number of bytes placed in the buffer, or zero
+   upon failure.  */
+
+static int
+native_encode_int (const_tree expr, unsigned char *ptr, int len, int off)
+{
+  return native_encode_wide_int (TREE_TYPE (expr), wi::to_widest (expr),
+ptr, len, off);
+}
+
 
 /* Subroutine of native_encode_expr.  Encode the FIXED_CST
specified by EXPR into the buffer PTR of length LEN bytes.
--- gcc/cobol/genapi.cc.jj  2025-04-02 19:26:55.265272660 +0200
+++ gcc/cobol/genapi.cc 2025-04-02 22:36:19.071641137 +0200
@@ -15216,25 +15216,19 @@ binary_initial_from_float128(cbl_field_t
   FIXED_WIDE_INT(128) i
 = FIXED_WIDE_INT(128)::from (real_to_integer (&value, &fail, 128), SIGNED);
 
-  /* ???  Use native_encode_* below.  */
   retval = (char *)xmalloc(field->data.capacity);
   switch(field->data.capacity)
 {
+  tree type;
 case 1:
-  *(signed char *)retval = (signed char)i.slow ();
-  break;
 case 2:
-  *(signed short *)retval = (signed short)i.slow ();
-  break;
 case 4:
-  *(signed int *)retval = (signed int)i.slow ();
-  break;
 case 8:
-  *(signed long *)retval = (signed long)i.slow ();
-  break;
 case 16:
-  *(unsigned long *)retval = (unsigned long)i.ulow

[RFC] Doc: Clean up musttail attribute docs some more

2025-04-02 Thread Sandra Loosemore
Looking over the recently-committed change to the musttail attribute
documentation, it appears the comment in the last example was a paste-o,
as it does not agree with either what the similar example in the
-Wmaybe-musttail-local-addr documentation says, or the actual behavior
observed when running the code.

In addition, the entire section on musttail was in need of copy-editing
to put it in the present tense, avoid reference to "the user", etc.  I've
attempted to clean it up here.

gcc/ChangeLog
* doc/extend.texi (Statement Attributes): Copy-edit the musttail
attribute documentation and correct the comment in the last
example.
---
 gcc/doc/extend.texi | 46 ++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 4302df76c7f..16ad83fc510 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -9314,7 +9314,7 @@ The @code{gnu::musttail} or @code{clang::musttail} 
standard attribute
 or @code{musttail} GNU attribute can be applied to a @code{return} statement
 with a return-value expression that is a function call.  It asserts that the
 call must be a tail call that does not allocate extra stack space, so it is
-safe to use tail recursion to implement long running loops.
+safe to use tail recursion to implement long-running loops.
 
 @smallexample
 [[gnu::musttail]] return foo();
@@ -9324,11 +9324,11 @@ safe to use tail recursion to implement long running 
loops.
 __attribute__((musttail)) return bar();
 @end smallexample
 
-If the compiler cannot generate a @code{musttail} tail call it will report
-an error.  On some targets tail calls may never be supported.
-The user asserts for @code{musttail} tail calls that lifetime of automatic
+If the compiler cannot generate a @code{musttail} tail call it reports
+an error.  On some targets, tail calls may not be supported at all.
+The @code{musttail} attribute asserts that the lifetime of automatic
 variables, function parameters and temporaries (unless they have non-trivial
-destruction) can end before the actual call instruction and that any access
+destruction) can end before the actual call instruction, and that any access
 to those from inside of the called function results is considered undefined
 behavior.  Enabling @option{-O1} or @option{-O2} can improve the success of
 tail calls.
@@ -9344,9 +9344,9 @@ baz (int *x)
   if (*x == 1)
 @{
   int a = 42;
-  /* The call will be tail called (would not be without the
- attribute), dereferencing the pointer in the callee is
- undefined behavior and there will be a warning emitted
+  /* The call is a tail call (would not be without the
+ attribute).  Dereferencing the pointer in the callee is
+ undefined behavior, and there is a warning emitted
  for this by default (@option{-Wmusttail-local-addr}).  */
   [[gnu::musttail]] return foo (&a);
 @}
@@ -9354,9 +9354,9 @@ baz (int *x)
 @{
   int a = 42;
   bar (&a);
-  /* The call will be tail called (would not be without the
- attribute), if bar stores the pointer anywhere, dereferencing
- it in foo will be undefined behavior and there will be a warning
+  /* The call is a tail call (would not be without the
+ attribute).  If bar stores the pointer anywhere, dereferencing
+ it in foo is undefined behavior.  There is a warning
  emitted for this with @option{-Wextra}, which implies
  @option{-Wmaybe-musttail-local-addr}.  */
   [[gnu::musttail]] return foo (nullptr);
@@ -9365,8 +9365,8 @@ baz (int *x)
 @{
   S s;
   /* The s variable requires non-trivial destruction which ought
- to be performed after the foo call returns, so this will
- be rejected.  */
+ to be performed after the foo call returns, so this is
+ rejected.  */
   [[gnu::musttail]] return foo (&s.s);
 @}
 @}
@@ -9374,9 +9374,9 @@ baz (int *x)
 
 To avoid the @option{-Wmaybe-musttail-local-addr} warning in the
 above @code{*x == 2} case and similar code, consider defining the
-maybe escaped variables in a separate scope which will end before the
-return statement if possible to make it clear that the variable is not
-live during the call.  So
+maybe-escaped variables in a separate scope that ends before the
+return statement, if that is possible, to make it clear that the
+variable is not live during the call.  So:
 
 @smallexample
   else if (*x == 2)
@@ -9385,17 +9385,17 @@ live during the call.  So
 int a = 42;
 bar (&a);
   @}
-  /* The call will be tail called (would not be without the
- attribute), if bar stores the pointer anywhere, dereferencing
- it in foo will be undefined behavior and there will be a warning
- emitted for this with @option{-Wextra}, which implies
- @option{-Wmaybe-musttail-local-addr}.  */
+  /* The call i

[PATCH] RISC-V: Disable unsupported vsext/vzext patterns for XTheadVector.

2025-04-02 Thread Jin Ma
XThreadVector does not support the vsext/vzext instructions; however,
due to the reuse of RVV optimizations, it may generate these instructions
in certain cases. To prevent the error "Unknown opcode 'th.vsext.vf2',"
we should disable these patterns.

gcc/ChangeLog:

* config/riscv/vector.md: Disable vsext/vzext for XTheadVector.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/xtheadvector/vsext.c: New test.
* gcc.target/riscv/rvv/xtheadvector/vzext.c: New test.
---
 gcc/config/riscv/vector.md|  6 ++---
 .../gcc.target/riscv/rvv/xtheadvector/vsext.c | 24 +++
 .../gcc.target/riscv/rvv/xtheadvector/vzext.c | 24 +++
 3 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vsext.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vzext.c

diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
index 8ee43cf0ce1..51eb64fb122 100644
--- a/gcc/config/riscv/vector.md
+++ b/gcc/config/riscv/vector.md
@@ -3939,7 +3939,7 @@ (define_insn "@pred__vf2"
  (any_extend:VWEXTI
(match_operand: 3 "register_operand" "   vr,   vr"))
  (match_operand:VWEXTI 2 "vector_merge_operand" "   vu,
0")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && !TARGET_XTHEADVECTOR"
   "vext.vf2\t%0,%3%p1"
   [(set_attr "type" "vext")
(set_attr "mode" "")])
@@ -3959,7 +3959,7 @@ (define_insn "@pred__vf4"
  (any_extend:VQEXTI
(match_operand: 3 "register_operand" "   vr,   vr"))
  (match_operand:VQEXTI 2 "vector_merge_operand"   "   vu,0")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && !TARGET_XTHEADVECTOR"
   "vext.vf4\t%0,%3%p1"
   [(set_attr "type" "vext")
(set_attr "mode" "")])
@@ -3979,7 +3979,7 @@ (define_insn "@pred__vf8"
  (any_extend:VOEXTI
(match_operand: 3 "register_operand" "   vr,   vr"))
  (match_operand:VOEXTI 2 "vector_merge_operand"  "   vu,0")))]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && !TARGET_XTHEADVECTOR"
   "vext.vf8\t%0,%3%p1"
   [(set_attr "type" "vext")
(set_attr "mode" "")])
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vsext.c 
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vsext.c
new file mode 100644
index 000..29cb29105d4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vsext.c
@@ -0,0 +1,24 @@
+/* { dg-do assemble { target { rv64 } } } */
+/* { dg-options "-march=rv64gc_xtheadvector -mabi=lp64d -O3 --save-temps " } */
+
+#include 
+
+struct a
+{
+  int b[];
+} c (vint32m4_t), d;
+
+char e;
+char *f;
+
+void g ()
+{
+  int h;
+  vint32m4_t i;
+  vint8m1_t j = __riscv_vlse8_v_i8m1 (&e, d.b[3], h);
+  vint16m2_t k = __riscv_vwadd_vx_i16m2 (j, 0, h);
+  i = __riscv_vwmacc_vx_i32m4 (i, f[0], k, h);
+  c (i);
+}
+
+/* { dg-final { scan-assembler-not {th\.vsext\.vf2} } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vzext.c 
b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vzext.c
new file mode 100644
index 000..43613170c06
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/xtheadvector/vzext.c
@@ -0,0 +1,24 @@
+/* { dg-do assemble { target { rv64 } } } */
+/* { dg-options "-march=rv64gc_xtheadvector -mabi=lp64d -O3 --save-temps " } */
+
+#include 
+
+struct a
+{
+  int b[];
+} c (vuint32m4_t), d;
+
+char e;
+char *f;
+
+void g ()
+{
+  int h;
+  vuint32m4_t i;
+  vuint8m1_t j = __riscv_vlse8_v_u8m1 (&e, d.b[3], h);
+  vuint16m2_t k = __riscv_vwaddu_vx_u16m2 (j, 0, h);
+  i = __riscv_vwmaccu_vx_u32m4 (i, f[0], k, h);
+  c (i);
+}
+
+/* { dg-final { scan-assembler-not {th\.vzext\.vf2} } } */
-- 
2.25.1



[PATCH 0/2] AArch64: Add OpenMP SVE tests

2025-04-02 Thread Tejas Belagod
This patch series adds compile and execute tests to test SVE types with various
OpenMP constructs and clauses.  This is a follow-up series that applies on

  https://gcc.gnu.org/pipermail/gcc-patches/2025-March/678086.html

posted earlier last month and tests the changes proposed in that series.

It incorporates review comments from an earlier version of the series

  https://gcc.gnu.org/pipermail/gcc-patches/2025-January/674283.html

OK for trunk?

Thanks,
Tejas

Tejas Belagod (2):
  AArch64: Add OpenMP target compile error tests
  libgomp: Add AArch64 SVE target tests to libgomp.

 .../gcc.target/aarch64/sve/gomp/gomp.exp  |   46 +
 .../aarch64/sve/gomp/target-device.c  |  201 ++
 .../gcc.target/aarch64/sve/gomp/target-link.c |   57 +
 .../gcc.target/aarch64/sve/gomp/target.c  | 2049 +
 .../libgomp.c-target/aarch64/aarch64.exp  |   57 +
 .../libgomp.c-target/aarch64/firstprivate.c   |  128 +
 .../libgomp.c-target/aarch64/lastprivate.c|  171 ++
 .../libgomp.c-target/aarch64/private.c|  111 +
 .../libgomp.c-target/aarch64/shared.c |  269 +++
 .../libgomp.c-target/aarch64/simd-aligned.c   |   52 +
 .../aarch64/simd-nontemporal.c|   51 +
 .../libgomp.c-target/aarch64/threadprivate.c  |   48 +
 .../libgomp.c-target/aarch64/udr-sve.c|  108 +
 13 files changed, 3348 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/gomp.exp
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target-device.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target-link.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/aarch64.exp
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/firstprivate.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/lastprivate.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/private.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/shared.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/simd-aligned.c
 create mode 100644 
libgomp/testsuite/libgomp.c-target/aarch64/simd-nontemporal.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/threadprivate.c
 create mode 100644 libgomp/testsuite/libgomp.c-target/aarch64/udr-sve.c

-- 
2.25.1



[PATCH 1/2] AArch64: Add OpenMP target compile error tests

2025-04-02 Thread Tejas Belagod
Add compile-only OpenMP error tests for target clause used with SVE types.

gcc/testsuite/ChangeLog:

* gcc.target/aarch64/sve/gomp/gomp.exp: Test driver.
* gcc.target/aarch64/sve/gomp/target-device.c: New test.
* gcc.target/aarch64/sve/gomp/target-link.c: Likewise.
* gcc.target/aarch64/sve/gomp/target.c: Likewise.
---
 .../gcc.target/aarch64/sve/gomp/gomp.exp  |   46 +
 .../aarch64/sve/gomp/target-device.c  |  201 ++
 .../gcc.target/aarch64/sve/gomp/target-link.c |   57 +
 .../gcc.target/aarch64/sve/gomp/target.c  | 2049 +
 4 files changed, 2353 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/gomp.exp
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target-device.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target-link.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/gomp/target.c

diff --git a/gcc/testsuite/gcc.target/aarch64/sve/gomp/gomp.exp 
b/gcc/testsuite/gcc.target/aarch64/sve/gomp/gomp.exp
new file mode 100644
index 000..376985de1ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/gomp/gomp.exp
@@ -0,0 +1,46 @@
+# Copyright (C) 2006-2025 Free Software Foundation, Inc.
+#
+# This file is part of GCC.
+#
+# GCC is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# GCC is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# .
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't an AArch64 target.
+if {![istarget aarch64*-*-*] } then {
+  return
+}
+
+# Load support procs.
+load_lib gcc-dg.exp
+
+# Initialize `dg'.
+dg-init
+
+if ![check_effective_target_fopenmp] {
+  return
+}
+
+if { [check_effective_target_aarch64_sve] } {
+set sve_flags ""
+} else {
+set sve_flags "-march=armv8.2-a+sve"
+}
+
+# Main loop.
+dg-runtest [lsort [find $srcdir/$subdir *.c]] "$sve_flags -fopenmp" ""
+
+# All done.
+dg-finish
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/gomp/target-device.c 
b/gcc/testsuite/gcc.target/aarch64/sve/gomp/target-device.c
new file mode 100644
index 000..be41a7cb0a0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/gomp/target-device.c
@@ -0,0 +1,201 @@
+/* { dg-do compile } */
+/* { dg-options "-msve-vector-bits=256 -fopenmp -O2" } */
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+
+int64_t __attribute__ ((noipa))
+omp_target_device_ptr_vla (svbool_t vp, svint32_t *vptr)
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int64_t res;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+/* { dg-error {SVE type 'svint32_t \*' not allowed in target device clauses} 
"" { target *-*-* } .+1 } */
+#pragma omp target data use_device_ptr (vptr) map (to: b, c)
+/* { dg-error {SVE type 'svint32_t \*' not allowed in target device clauses} 
"" { target *-*-* } .+1 } */
+#pragma omp target is_device_ptr (vptr) map (to: b, c) map (from: res)
+  for (i = 0; i < 8; i++)
+{
+  /* { dg-error "cannot reference 'svint32_t' object types in target 
region" "" { target *-*-* } .+1 } */
+  vb = *vptr;
+  /* { dg-error "cannot reference 'svint32_t' object types in target 
region" "" { target *-*-* } .+2 } */
+  /* { dg-error "cannot reference 'svbool_t' object types in target 
region" "" { target *-*-* } .+1 } */
+  vc = svld1_s32 (vp, c);
+  /* { dg-error "cannot reference 'svint32_t' object types in target 
region" "" { target *-*-* } .+1 } */
+  va = svadd_s32_z (vp, vb, vc);
+  res = svaddv_s32 (svptrue_b32 (), va);
+}
+
+  return res;
+}
+
+int64_t __attribute__ ((noipa))
+omp_target_device_addr_vla (svbool_t vp, svint32_t *vptr)
+{
+
+  int a[N], b[N], c[N];
+  svint32_t va, vb, vc;
+  int64_t res;
+  int i;
+
+#pragma omp parallel for
+  for (i = 0; i < N; i++)
+{
+  b[i] = i;
+  c[i] = i + 1;
+}
+
+/* { dg-error "SVE type 'svint32_t' not allowed in target device clauses" "" { 
 target *-*-* } .+1 } */
+#pragma omp target data use_device_addr (vb) map (to: b, c)
+/* { dg-error {SVE type 'svint32_t \*' not allowed in target device clauses} 
"" { target *-*-* } .+1 } */
+#pragma omp target is_device_ptr (vptr) map (to: b, c) map (from: res)
+  for (i = 0; i < 8; i++)
+{
+  /* { dg-error "cannot reference 'svint32_t' object types in target 
region" "" { target *-*-* } .+1 } */
+  vb = *vptr;
+  /* { dg-error "cannot reference 'svint32_t' object types in target 
regio

Re: [PATCH] inline: Add a call to unreachable to the return block for noreturn calls [PR119599]

2025-04-02 Thread Richard Biener
On Thu, Apr 3, 2025 at 7:10 AM Andrew Pinski  wrote:
>
> builtin_unreachable_bb_p exacts empty basic blocks to have only one successor 
> edge but
> in the case after inliing a noreturn function that actually returns, we end 
> up with an
> empty basic block with no successor edge. fixup_cfg fixes this up by placing 
> a call to
> __builtin_unreachable but we don't do that before getting the function summary
> (after einiline). So the fix is to have the inliner insert the call
> to __builtin_unreachable (or the trap version) and not depend on the fixup if 
> the return
> block is reachable after inlining a noreturn function.
>
> A small optimization is done not to insert the call if the block is NOT 
> simplely reachable.
> This should prevent inserting the call just to remove it during cleanupcfg.
>
> Note this shows up only at -O0 with always_inline because when optimizations 
> are turned on
> returns are turned into __builtin_unreachable inside noreturn functions.

Where's that done?  IMO we should do this at -O0 as well (possibly defaulting to
use the trapping variant there).

>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR ipa/119599
>
> gcc/ChangeLog:
>
> * tree-inline.cc (expand_call_inline): Add a __builtin_unreachable 
> call
> to the return block if it is still reachable and the call that was 
> inlined
> was a noreturn function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/pr119599-1.c: New test.
>
> Signed-off-by: Andrew Pinski 
> ---
>  gcc/testsuite/gcc.dg/torture/pr119599-1.c | 27 +++
>  gcc/tree-inline.cc| 15 +
>  2 files changed, 42 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr119599-1.c
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr119599-1.c 
> b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> new file mode 100644
> index 000..4fbd228a359
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr119599-1.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fdump-tree-einline" } */
> +
> +/* PR ipa/119599 */
> +/* inlining a noreturn function which returns
> +   can cause an ICE when dealing finding an unreachable block.
> +   We should get a __builtin_unreachable after the inliing.  */
> +
> +
> +void baz (void);
> +
> +static inline __attribute__((always_inline, noreturn)) void
> +bar (void)
> +{
> +  static volatile int t = 0;
> +  if (t == 0)
> +baz ();
> +} /* { dg-warning "function does return" } */
> +
> +void
> +foo (void)
> +{
> +  bar ();
> +}
> +
> +/* After inlining, we should have call to __builtin_unreachable now. */
> +/* { dg-final { scan-tree-dump "__builtin_unreachable " "einline" } } */
> diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
> index 05843b8ccf0..da403cd1456 100644
> --- a/gcc/tree-inline.cc
> +++ b/gcc/tree-inline.cc
> @@ -4832,6 +4832,7 @@ expand_call_inline (basic_block bb, gimple *stmt, 
> copy_body_data *id,
>gimple *simtenter_stmt = NULL;
>vec *simtvars_save;
>tree save_stack = NULL_TREE;
> +  bool was_noreturn;
>
>/* The gimplifier uses input_location in too many places, such as
>   internal_get_tmp_var ().  */
> @@ -4843,6 +4844,8 @@ expand_call_inline (basic_block bb, gimple *stmt, 
> copy_body_data *id,
>if (!call_stmt)
>  goto egress;
>
> +  was_noreturn = gimple_call_noreturn_p (call_stmt);
> +
>cg_edge = id->dst_node->get_edge (stmt);
>gcc_checking_assert (cg_edge);
>/* First, see if we can figure out what function is being called.
> @@ -5376,6 +5379,18 @@ expand_call_inline (basic_block bb, gimple *stmt, 
> copy_body_data *id,
>if (purge_dead_abnormal_edges)
>  bitmap_set_bit (to_purge, return_block->index);
>
> +  /* If this was a noreturn function and if the return block is still
> + reachable, then add a call to __builtin_unreachable().  */
> +  if (was_noreturn && EDGE_COUNT (return_block->preds) != 0)
> +{
> +  gcall *call_stmt = gimple_build_builtin_unreachable (input_location);
> +  gimple_stmt_iterator gsi = gsi_last_bb (return_block);
> +  gsi_insert_after (&gsi, call_stmt, GSI_NEW_STMT);
> +  tree fndecl = gimple_call_fndecl (call_stmt);
> +  cg_edge->caller->create_edge (cgraph_node::get_create (fndecl),
> +   call_stmt, return_block->count);
> +}
> +
>/* If the value of the new expression is ignored, that's OK.  We
>   don't warn about this for CALL_EXPRs, so we shouldn't warn about
>   the equivalent inlined version either.  */
> --
> 2.43.0
>


[PATCH] vect: Relax scan-tree-dump strict pattern matching [PR118597]

2025-04-02 Thread Victor Do Nascimento
Using specific SSA names in pattern matching in `dg-final' makes tests
"unstable", in that changes in passes prior to the pass whose dump is
analyzed in the particular test may change the numbering of the SSA
variables, causing the test to start failing spuriously.

We thus switch from specific SSA names to the use of a multi-line
regular expression making use of capture groups for matching particular
variables across different statements, ensuring the test will pass
more consistently across different versions of GCC.

PR testsuite/118597

gcc/testsuite/ChangeLog:

* gcc.dg/vect/vect-fncall-mask.c: Update test directives.
---
 gcc/testsuite/gcc.dg/vect/vect-fncall-mask.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/vect/vect-fncall-mask.c 
b/gcc/testsuite/gcc.dg/vect/vect-fncall-mask.c
index 554488e0630..ba1886da5ca 100644
--- a/gcc/testsuite/gcc.dg/vect/vect-fncall-mask.c
+++ b/gcc/testsuite/gcc.dg/vect/vect-fncall-mask.c
@@ -1,7 +1,7 @@
 /* { dg-do compile { target { aarch64*-*-* } } } */
-/* { dg-additional-options "-march=armv8.2-a+sve -fdump-tree-ifcvt-raw -Ofast" 
{ target { aarch64*-*-* } } } */
+/* { dg-additional-options "-march=armv8.2-a+sve -fdump-tree-ifcvt -Ofast" { 
target { aarch64*-*-* } } } */
 
-extern int __attribute__ ((simd, const)) fn (int);
+extern int __attribute__ ((simd, const)) fn (float);
 
 const int N = 20;
 const float lim = 101.0;
@@ -26,6 +26,4 @@ int main (void)
   return (0);
 }
 
-/* { dg-final { scan-tree-dump {gimple_assign } ifcvt } } */
-/* { dg-final { scan-tree-dump {gimple_assign } ifcvt } } */
-/* { dg-final { scan-tree-dump {gimple_call <.MASK_CALL, _3, fn, _2, _34>} 
ifcvt } } */
+/* { dg-final { scan-tree-dump {(_\d+) = (_\d+) > 1.01e\+2;\n\s*(_\d+) = 
~\1;\n\s*_\d+ = .MASK_CALL \(fn, \2, \3\);} ifcvt } } */
-- 
2.43.0



[PATCH] libstdc++, testsuite, Darwin: Prune a new linker warning present from XCode 16.

2025-04-02 Thread Iain Sandoe
Tested on x86_64-darwin24 with Xcode 16.2. OK for trunk?
thanks
Iain

--- 8< ---

Darwin's linker now warns when duplicate rpaths are presented - which
happens when we emit duplicate '-B' paths.  In principle, we should avoid
this in the test- suite, however at present we tend to have duplicates
because different parts of the machinery add them.  At some point, it might
be nice to have an "add_option_if_missing" and apply that across the whole
of the test infra. However this is not something for late in stage 4.  So
the solution here is to prune the warning - the effect of the duplicate in
the libstdc++ testsuite is not important it will make the exes very slightly
larger but it won't alter the paths that are presented for loading the
runtimes.

libstdc++-v3/ChangeLog:

* testsuite/lib/prune.exp: Prune XCode ld warning about duplicate
rpaths.

Signed-off-by: Iain Sandoe 
---
 libstdc++-v3/testsuite/lib/prune.exp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libstdc++-v3/testsuite/lib/prune.exp 
b/libstdc++-v3/testsuite/lib/prune.exp
index 416e851614b..a9a29937e43 100644
--- a/libstdc++-v3/testsuite/lib/prune.exp
+++ b/libstdc++-v3/testsuite/lib/prune.exp
@@ -77,6 +77,9 @@ proc libstdc++-dg-prune { system text } {
 # Ignore harmless warnings from Xcode 4+.
 regsub -all "(^|\n)\[^\n\]*ld: warning: could not create compact unwind 
for\[^\n\]*" $text "" text
 
+# Ignore duplicate path warnings from Xcode 16+.
+regsub -all "(^|\n)\[^\n\]*ld: warning: duplicate -rpath\[^\n\]*" $text "" 
text
+
 # Ignore dsymutil warning (tool bug is actually in the linker)
 regsub -all "(^|\n)\[^\n\]*could not find object file symbol for 
symbol\[^\n\]*" $text "" text
 
-- 
2.39.2 (Apple Git-143)



Re: [PATCH] libstdc++, testsuite, Darwin: Prune a new linker warning present from XCode 16.

2025-04-02 Thread Jonathan Wakely
On Wed, 2 Apr 2025, 14:53 Iain Sandoe,  wrote:

> Tested on x86_64-darwin24 with Xcode 16.2. OK for trunk?
>

OK, thanks

thanks
> Iain
>
> --- 8< ---
>
> Darwin's linker now warns when duplicate rpaths are presented - which
> happens when we emit duplicate '-B' paths.  In principle, we should avoid
> this in the test- suite, however at present we tend to have duplicates
> because different parts of the machinery add them.  At some point, it might
> be nice to have an "add_option_if_missing" and apply that across the whole
> of the test infra. However this is not something for late in stage 4.  So
> the solution here is to prune the warning - the effect of the duplicate in
> the libstdc++ testsuite is not important it will make the exes very
> slightly
> larger but it won't alter the paths that are presented for loading the
> runtimes.
>
> libstdc++-v3/ChangeLog:
>
> * testsuite/lib/prune.exp: Prune XCode ld warning about duplicate
> rpaths.
>
> Signed-off-by: Iain Sandoe 
> ---
>  libstdc++-v3/testsuite/lib/prune.exp | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/libstdc++-v3/testsuite/lib/prune.exp
> b/libstdc++-v3/testsuite/lib/prune.exp
> index 416e851614b..a9a29937e43 100644
> --- a/libstdc++-v3/testsuite/lib/prune.exp
> +++ b/libstdc++-v3/testsuite/lib/prune.exp
> @@ -77,6 +77,9 @@ proc libstdc++-dg-prune { system text } {
>  # Ignore harmless warnings from Xcode 4+.
>  regsub -all "(^|\n)\[^\n\]*ld: warning: could not create compact
> unwind for\[^\n\]*" $text "" text
>
> +# Ignore duplicate path warnings from Xcode 16+.
> +regsub -all "(^|\n)\[^\n\]*ld: warning: duplicate -rpath\[^\n\]*"
> $text "" text
> +
>  # Ignore dsymutil warning (tool bug is actually in the linker)
>  regsub -all "(^|\n)\[^\n\]*could not find object file symbol for
> symbol\[^\n\]*" $text "" text
>
> --
> 2.39.2 (Apple Git-143)
>
>


[PATCH] tree-optimization/119586 - aligned access to unaligned data

2025-04-02 Thread Richard Biener
The following reverts parts of r15-8047 which assesses alignment
analysis for VMAT_STRIDED_SLP is correct by using aligned accesses
where allowed by it.  As the PR shows this analysis is still incorrect,
so revert back to assuming we got it wrong.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

PR tree-optimization/119586
* tree-vect-stmts.cc (vectorizable_load): Assume we got
alignment analysis for VMAT_STRIDED_SLP wrong.
(vectorizable_store): Likewise.

* gcc.dg/vect/pr119586.c: New testcase.
---
 gcc/testsuite/gcc.dg/vect/pr119586.c | 21 +
 gcc/tree-vect-stmts.cc   | 21 +
 2 files changed, 34 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr119586.c

diff --git a/gcc/testsuite/gcc.dg/vect/pr119586.c 
b/gcc/testsuite/gcc.dg/vect/pr119586.c
new file mode 100644
index 000..04a00ef131e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr119586.c
@@ -0,0 +1,21 @@
+#include "tree-vect.h"
+
+void __attribute__((noipa)) foo (long *) {}
+void __attribute__((noipa))
+d()
+{
+  long e[6][8][5];
+  for (int b = 0; b < 6; b++)
+for (int c = 0; c < 8; c++)
+  {
+e[b][c][0] = 1;
+e[b][c][1] = 1;
+e[b][c][4] = 1;
+  }
+  foo (&e[0][0][0]);
+}
+int main()
+{
+  check_vect ();
+  d();
+}
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index f4a6bc968b3..37a54bf5000 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -8968,10 +8968,17 @@ vectorizable_store (vec_info *vinfo,
}
}
  unsigned align;
- if (alignment_support_scheme == dr_aligned)
-   align = known_alignment (DR_TARGET_ALIGNMENT (first_dr_info));
- else
-   align = dr_alignment (vect_dr_behavior (vinfo, first_dr_info));
+ /* ???  We'd want to use
+  if (alignment_support_scheme == dr_aligned)
+align = known_alignment (DR_TARGET_ALIGNMENT (first_dr_info));
+since doing that is what we assume we can in the above checks.
+But this interferes with groups with gaps where for example
+VF == 2 makes the group in the unrolled loop aligned but the
+fact that we advance with step between the two subgroups
+makes the access to the second unaligned.  See PR119586.
+We have to anticipate that here or adjust code generation to
+avoid the misaligned loads by means of permutations.  */
+ align = dr_alignment (vect_dr_behavior (vinfo, first_dr_info));
  /* Alignment is at most the access size if we do multiple stores.  */
  if (nstores > 1)
align = MIN (tree_to_uhwi (TYPE_SIZE_UNIT (ltype)), align);
@@ -10946,10 +10953,8 @@ vectorizable_load (vec_info *vinfo,
}
}
  unsigned align;
- if (alignment_support_scheme == dr_aligned)
-   align = known_alignment (DR_TARGET_ALIGNMENT (first_dr_info));
- else
-   align = dr_alignment (vect_dr_behavior (vinfo, first_dr_info));
+ /* ???  The above is still wrong, see vectorizable_store.  */
+ align = dr_alignment (vect_dr_behavior (vinfo, first_dr_info));
  /* Alignment is at most the access size if we do multiple loads.  */
  if (nloads > 1)
align = MIN (tree_to_uhwi (TYPE_SIZE_UNIT (ltype)), align);
-- 
2.43.0


[PATCH] rtlanal, i386: Adjust pattern_cost and x86 constant cost [PR115910]

2025-04-02 Thread Jakub Jelinek
Hi!

Below is an attempt to fix up RTX costing P1 caused by r15-775
https://gcc.gnu.org/pipermail/gcc-patches/2024-May/thread.html#652446
@@ -21562,7 +21562,8 @@ ix86_rtx_costs (rtx x, machine_mode mode, int 
outer_code_i, int opno,
   if (x86_64_immediate_operand (x, VOIDmode))
*total = 0;
  else
-   *total = 1;
+   /* movabsq is slightly more expensive than a simple instruction. */
+   *total = COSTS_N_INSNS (1) + 1;
   return true;
 
 case CONST_DOUBLE:
change.  In my understanding this was partially trying to workaround
weird code in pattern_cost, which uses
  return cost > 0 ? cost : COSTS_N_INSNS (1);
That doesn't make sense to me.  All costs smaller than COSTS_N_INSNS (1)
mean we need to have at least one instruction there which has the
COSTS_N_INSNS (1) minimal cost.  So special casing just cost 0 for the
really cheap immediates which can be used pretty much everywhere but not
ones which have just tiny bit larger cost than that (1, 2 or 3) is just
weird.

So, the following patch changes that to MAX (COSTS_N_INSNS (1), cost)
which doesn't have this weird behavior where set_src_cost 0 is considered
more expensive than set_src_cost 1.

Note, pattern_cost isn't the only spot where costs are computed and normally
we often sum the subcosts of different parts of a pattern or just query
rtx costs of different parts of subexpressions, so the jump from
1 to 5 is quite significant.

Additionally, x86_64 doesn't have just 2 kinds of constants with different
costs, it has 3, signed 32-bit ones are the ones which can appear in
almost all instructions and so using cost of 0 for those looks best,
then unsigned 32-bit ones which can be done with still cheap movl
instruction (and I think some others too) and finally full 64-bit ones
which can be done only with a single movabsq instruction and are quite
costly both in instruction size and even more expensive to execute.

The following patch attempts to restore the behavior of GCC 14 with the
pattern_cost hunk fixed for the unsigned 32-bit ones and only keeps the
bigger cost for the 64-bit ones.

Bootstrapped/regtested on x86_64-linux and i686-linux, bootstraps/regtests
in progress on aarch64-linux, powerpc64le-linux and s390x-linux.

I don't have a setup to test this on SPEC though.

Your thoughts on this?

2025-04-02  Jakub Jelinek  

PR target/115910
* rtlanal.cc (pattern_cost): Return at least COSTS_N_INSNS (1)
rather than just COSTS_N_INTNS (1) for cost <= 0.
* config/i386/i386.cc (ix86_rtx_costs): Set *total to 1 for
TARGET_64BIT x86_64_zext_immediate_operand constants.

* gcc.target/i386/pr115910.c: New test.

--- gcc/rtlanal.cc.jj   2025-03-06 11:08:20.729230232 +0100
+++ gcc/rtlanal.cc  2025-04-02 12:08:33.327409772 +0200
@@ -5772,7 +5772,7 @@ pattern_cost (rtx pat, bool speed)
 return 0;
 
   cost = set_src_cost (SET_SRC (set), GET_MODE (SET_DEST (set)), speed);
-  return cost > 0 ? cost : COSTS_N_INSNS (1);
+  return MAX (COSTS_N_INSNS (1), cost);
 }
 
 /* Calculate the cost of a single instruction.  A return value of zero
--- gcc/config/i386/i386.cc.jj  2025-03-27 23:35:17.798315113 +0100
+++ gcc/config/i386/i386.cc 2025-04-02 12:14:25.522539997 +0200
@@ -21883,7 +21883,11 @@ ix86_rtx_costs (rtx x, machine_mode mode
 case SYMBOL_REF:
   if (x86_64_immediate_operand (x, VOIDmode))
*total = 0;
- else
+  else if (TARGET_64BIT && x86_64_zext_immediate_operand (x, VOIDmode))
+   /* Consider the zext constants slightly more expensive, as they
+  can't appear in most instructions.  */
+   *total = 1;
+  else
/* movabsq is slightly more expensive than a simple instruction. */
*total = COSTS_N_INSNS (1) + 1;
   return true;
--- gcc/testsuite/gcc.target/i386/pr115910.c.jj 2025-04-02 12:27:36.199606571 
+0200
+++ gcc/testsuite/gcc.target/i386/pr115910.c2025-04-02 12:34:12.025132993 
+0200
@@ -0,0 +1,20 @@
+/* PR target/115910 */
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -march=x86-64 -mtune=generic -masm=att" } */
+/* { dg-final { scan-assembler-times {\timulq\t} 2 } } */
+/* { dg-final { scan-assembler-times {\tshrq\t\$33,} 2 } } */
+/* { dg-final { scan-assembler-not {\tsarl\t} } } */
+
+int
+foo (int x)
+{
+  if (x < 0) 
+__builtin_unreachable ();
+  return x / 3U;
+}
+
+int
+bar (int x)
+{
+  return x / 3U;
+}

Jakub



[PATCH] c: Fix ICEs with -fsanitize=pointer-{subtract,compare} [PR119582]

2025-04-02 Thread Jakub Jelinek
Hi!

The following testcase ICEs because c_fully_fold isn't performed on the
arguments of __sanitizer_ptr_{sub,cmp} builtins and so e.g.
C_MAYBE_CONST_EXPR can leak into the gimplifier where it ICEs.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2025-04-02  Jakub Jelinek  

PR c/119582
* c-typeck.cc (pointer_diff, build_binary_op): Call c_fully_fold on
__sanitizer_ptr_sub or __sanitizer_ptr_cmp arguments.

* gcc.dg/asan/pr119582.c: New test.

--- gcc/c/c-typeck.cc.jj2025-03-27 09:29:36.953576540 +0100
+++ gcc/c/c-typeck.cc   2025-04-02 15:04:47.103495567 +0200
@@ -4824,8 +4824,8 @@ pointer_diff (location_t loc, tree op0,
   if (current_function_decl != NULL_TREE
   && sanitize_flags_p (SANITIZE_POINTER_SUBTRACT))
 {
-  op0 = save_expr (op0);
-  op1 = save_expr (op1);
+  op0 = save_expr (c_fully_fold (op0, false, NULL));
+  op1 = save_expr (c_fully_fold (op1, false, NULL));
 
   tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT);
   *instrument_expr = build_call_expr_loc (loc, tt, 2, op0, op1);
@@ -14455,8 +14455,8 @@ build_binary_op (location_t location, en
  && current_function_decl != NULL_TREE
  && sanitize_flags_p (SANITIZE_POINTER_COMPARE))
{
- op0 = save_expr (op0);
- op1 = save_expr (op1);
+ op0 = save_expr (c_fully_fold (op0, false, NULL));
+ op1 = save_expr (c_fully_fold (op1, false, NULL));
 
  tree tt = builtin_decl_explicit (BUILT_IN_ASAN_POINTER_COMPARE);
  instrument_expr = build_call_expr_loc (location, tt, 2, op0, op1);
--- gcc/testsuite/gcc.dg/asan/pr119582.c.jj 2025-04-02 15:11:22.351048509 
+0200
+++ gcc/testsuite/gcc.dg/asan/pr119582.c2025-04-02 15:11:57.600561599 
+0200
@@ -0,0 +1,23 @@
+/* PR c/119582 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fsanitize=address,pointer-subtract,pointer-compare" } */
+
+const char v;
+typedef __PTRDIFF_TYPE__ ptrdiff_t;
+char a;
+const ptrdiff_t p = &a + 1 - &a;
+const int q = (&a + 1) != &a;
+
+ptrdiff_t
+foo (void)
+{
+  char b;
+  return &b + (v != '\n') - &b;
+}
+
+int
+bar (void)
+{
+  char b;
+  return (&b + (v != '\n')) != &b;
+}

Jakub



Re: [PATCH] vect: Relax scan-tree-dump strict pattern matching [PR118597]

2025-04-02 Thread Jeff Law




On 4/2/25 8:53 AM, Victor Do Nascimento wrote:

Using specific SSA names in pattern matching in `dg-final' makes tests
"unstable", in that changes in passes prior to the pass whose dump is
analyzed in the particular test may change the numbering of the SSA
variables, causing the test to start failing spuriously.

We thus switch from specific SSA names to the use of a multi-line
regular expression making use of capture groups for matching particular
variables across different statements, ensuring the test will pass
more consistently across different versions of GCC.

PR testsuite/118597

gcc/testsuite/ChangeLog:

* gcc.dg/vect/vect-fncall-mask.c: Update test directives.
Yea, we definitely should not be using specific SSA_NAME versions in a 
testcase like that.


OK for the trunk.  Please go ahead and install.

Thanks!

jeff



Re: [PATCH] testsuite: i386: Fix gcc.target/i386/pr82142?.c etc. on Solaris/x86

2025-04-02 Thread Uros Bizjak
On Wed, Apr 2, 2025 at 11:02 AM Rainer Orth  
wrote:
>
> Ping?  It's been a week:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2025-March/679330.html
>
> > Three tests FAIL on Solaris/x86 in similar ways:
> >
> > FAIL: gcc.target/i386/pr111673.c check-function-bodies advance
> > FAIL: gcc.target/i386/pr82142a.c check-function-bodies assignzero
> > FAIL: gcc.target/i386/pr82142b.c check-function-bodies assignzero
> >
> > All tests FAIL as is because they lack either or both of the .LFB0 label
> > and the .cfi_startproc directive:
> >
> > * The 32-bit pr82142b.c test lacks both, whether as or gas is in use: as
> >   lacks full support for the cfi directives and the .LSB0 label is only
> >   emitted with -fasynchronous-unwind-tables.
> >
> > * The 64-bit tests pr111673.c and pr82142a.c already work with gas, but
> >   with as the cfi directives are again missing.
> >
> > In addition, the 32-bit test (pr82142b.c) still FAILs because 32-bit
> > Solaris/x86 defaults to -mstackrealign.
> >
> > To fix all this, this patch adds -fasynchronous-unwind-tables
> > -fdwarf2-cfi-asm to all tests to force the generation of both the .LFB0
> > label and .cfi_startproc (which is ok since they are compile tests).  In
> > addition, pr82142b.c is compiled with -mno-stackrealign to avoid
> > platform differences.
> >
> > I'm a bit uncertain if we want to force all those options
> > unconditionally, though they don't cause harm.  One might only add them
> > for Solaris via dg-additional-options and/or require cfi support instead?
> >
> > Tested on i386-pc-solaris2.11 and x86_64-pc-linux-gnu.
> >
> > Ok for trunk?

OK, adding these options is an established practice to fix Solaris fallout.

Thanks,
Uros.


Re: [committed] hpux: Only use long long when __cplusplus >= 201103L

2025-04-02 Thread Dave Anglin
On 2025-04-02 3:19 p.m., Jonathan Wakely wrote:
> On Wed, 2 Apr 2025 at 20:15, John David Anglin  wrote:
>>
>> Committed to trunk.  Tested on hppa64-hp-hpux11.11.
>>
>> Dave
>> ___
>>
>> hpux: Only use long long when __cplusplus >= 201103L
>>
>> Fixes some test failures.
>>
>> 2025-04-02  John David Anglin  
>>
>> libstdc++-v3/ChangeLog:
>> * config/os/hpux/os_defines.h: Only use long long when
>> __cplusplus >= 201103L.
> 
> This isn't what the patch does though. It disables the declarations of
> strtoll for C++98, but it leaves the unconditional:

The declarations were causing some g++ tests to fail.

> #define _GLIBCXX_USE_LONG_LONG 1
> 
> Is that intentional?

I think so based on the comment below.  But maybe it could be changed as well.  
Stage1 seems
to use -std=c++14.

> 
>>
>> diff --git a/libstdc++-v3/config/os/hpux/os_defines.h 
>> b/libstdc++-v3/config/os/hpux/os_defines.h
>> index 30bd4c7ba14..d3a6c5ab142 100644
>> --- a/libstdc++-v3/config/os/hpux/os_defines.h
>> +++ b/libstdc++-v3/config/os/hpux/os_defines.h
>> @@ -57,7 +57,7 @@
>> We also force _GLIBCXX_USE_LONG_LONG here so that we don't have
>> to bastardize configure to deal with this sillyness.  */
>>
>> -#ifdef __cplusplus
>> +#if __cplusplus >= 201103L
>>  namespace std
>>  {
>>extern "C"
> 

Dave
-- 
John David Anglin  jda.par...@gmail.com


Re: [PATCH v2] RISC-V: Fixbug for slli + addw + zext.w into sh[123]add + zext.w

2025-04-02 Thread Jeff Law




On 4/2/25 1:01 AM, Jin Ma wrote:

Assuming we have the following variables:

unsigned long long a0, a1;
unsigned int a2;

For the expression:

a0 = (a0 << 50) >> 49;  // slli a0, a0, 50 + srli a0, a0, 49
a2 = a1 + a0;   // addw a2, a1, a0 + slli a2, a2, 32 + srli a2, a2, 32

In the optimization process of ZBA (combine pass), it would be optimized to:

a2 = a0 << 1 + a1;  // sh1add a2, a0, a1 + zext.w a2, a2

This is clearly incorrect, as it overlooks the fact that a0=a0&0x7ffe, meaning
that the bits a0[32:14] are set to zero.

gcc/ChangeLog:

* config/riscv/bitmanip.md: The optimization can only be applied if
the high bit of operands[3] is set to 1.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/zba-shNadd-09.c: New test.
* gcc.target/riscv/zba-shNadd-10.c: New test.

Thanks.

While there isn't a regression bug in bugzilla, this is almost certainly 
a 13, 14, 15 regression since the problematic pattern was integrated in 
late 2022.


With that in mind, I went ahead and pushed this to the trunk.

Thanks again!

jeff


[PATCH, OBVIOUS] rs6000: Add Cobol support to traceback table [PR119308]

2025-04-02 Thread Peter Bergner
The AIX traceback table documentation states the tbtab "lang" field for
Cobol should be set to 7.

Tested on powerpc64le-linux.  There are "new" FAILs with the patch (see below)
on the Cobol test cases, but that is a good thing, because without the
patch, the compiler ICEs and none of the tests are even run making them
essentially unsupported.

The FAILs below are due to PR119597 and are only hit when compiling with -O0.
The -O[123s] Cobol tests all PASS.  That is an improvement to me over the
current status, so I'm treating this patch as "obvious" and will push it
tomorrow unless someone has an objection.

Peter


gcc/
PR target/119308
* config/rs6000/rs6000-logue.cc (rs6000_output_function_epilogue):
Handle GCC COBOL for the tbtab lang field.


diff --git a/gcc/config/rs6000/rs6000-logue.cc 
b/gcc/config/rs6000/rs6000-logue.cc
index 52f44b114b0..5377ad6cee6 100644
--- a/gcc/config/rs6000/rs6000-logue.cc
+++ b/gcc/config/rs6000/rs6000-logue.cc
@@ -5351,6 +5351,8 @@ rs6000_output_function_epilogue (FILE *file)
i = 1;
   else if (! strcmp (language_string, "GNU Ada"))
i = 3;
+  else if (! strcmp (language_string, "GCC COBOL"))
+   i = 7;
   else if (! strcmp (language_string, "GNU Modula-2"))
i = 8;
   else if (lang_GNU_CXX ()





+FAIL: cobol.dg/data1.cob   -O0  execution test
+FAIL: cobol.dg/literal1.cob   -O0  execution test
+FAIL: cobol.dg/group1/declarative_1.cob   -O0  execution test
+FAIL: cobol.dg/group1/escape.cob   -O0  execution test
+FAIL: cobol.dg/group2/Complex_EVALUATE__1_.cob   -O0  execution test
+FAIL: cobol.dg/group2/Complex_EVALUATE__2_.cob   -O0  execution test
+FAIL: cobol.dg/group2/EVALUATE_WHEN_NEGATIVE.cob   -O0  execution test
+FAIL: cobol.dg/group2/EVALUATE_doubled_WHEN.cob   -O0  execution test
+FAIL: cobol.dg/group2/EVALUATE_with_WHEN_using_condition-1.cob   -O0  
execution test
+FAIL: cobol.dg/group2/Floating_continuation_indicator__1_.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/IBM_dialect_COMP_redefined_by_POINTER_as_64-bit.cob   
-O0  execution test
+FAIL: cobol.dg/group2/Indicators___-D__.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/MULTIPLY_to_FIX4.cob   -O0  execution test
+FAIL: cobol.dg/group2/PACKED-DECIMAL_basic_comp-3_comp-6__1_.cob   -O0  
execution test
+FAIL: cobol.dg/group2/PACKED-DECIMAL_basic_comp-3_comp-6__2_.cob   -O0  
execution test
+FAIL: cobol.dg/group2/Simple_floating-point_MOVE.cob   -O0  execution test
+FAIL: cobol.dg/group2/Simple_floating-point_VALUE_and_MOVE.cob   -O0  
execution test
+FAIL: cobol.dg/group2/floating-point_ADD_FORMAT_1.cob   -O0  execution test
+FAIL: cobol.dg/group2/floating-point_ADD_FORMAT_2.cob   -O0  execution test
+FAIL: cobol.dg/group2/floating-point_DIVIDE_FORMAT_1.cob   -O0  execution test
+FAIL: cobol.dg/group2/floating-point_DIVIDE_FORMAT_2.cob   -O0  execution test
+FAIL: cobol.dg/group2/floating-point_MULTIPLY_FORMAT_1.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/floating-point_MULTIPLY_FORMAT_2.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/floating-point_SUBTRACT_FORMAT_1.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/floating-point_SUBTRACT_FORMAT_2.cob   -O0  execution 
test
+FAIL: cobol.dg/group2/floating-point_literals.cob   -O0  execution test



[committed] d: Fix error using UFCS in a speculative context

2025-04-02 Thread Iain Buclaw
Hi,

This patch merges the D front-end with upstream dmd ed17b3e95d.

Reverts a change in the upstream D implementation of the compiler,
as it is no longer necessary since another fix for opDispatch got
applied in the same area (merged in r12-6003-gfd43568cc54e17).

Bootstrapped and regression tested on x86_64-linux-gnu/-m32, and
committed to mainline.

Regards,
Iain.

---
gcc/d/ChangeLog:

* dmd/MERGE: Merge upstream dmd ed17b3e95d.

Reviewed-on: https://github.com/dlang/dmd/pull/21132
---
 gcc/d/dmd/MERGE   |  2 +-
 gcc/d/dmd/expressionsem.d | 11 +--
 .../compilable/imports/test21098_phobos.d | 77 +++
 .../gdc.test/compilable/imports/test21098b.d  | 12 +++
 gcc/testsuite/gdc.test/compilable/test21098.d |  4 +
 5 files changed, 97 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/test21098_phobos.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/imports/test21098b.d
 create mode 100644 gcc/testsuite/gdc.test/compilable/test21098.d

diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 00c85187b6d..bd297b612c4 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-c6863be7206eef3c393726363a480baf0a0c6530
+ed17b3e95dc3fc3264a4c91843da824f5541f3e1
 
 The first line of this file holds the git revision number of the last
 merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/expressionsem.d b/gcc/d/dmd/expressionsem.d
index 04efa1f86f1..b0278cbaca1 100644
--- a/gcc/d/dmd/expressionsem.d
+++ b/gcc/d/dmd/expressionsem.d
@@ -1247,6 +1247,9 @@ private Expression resolveUFCS(Scope* sc, CallExp ce)
 }
 else
 {
+if (arrayExpressionSemantic(ce.arguments.peekSlice(), sc))
+return ErrorExp.get();
+
 if (Expression ey = die.dotIdSemanticProp(sc, 1))
 {
 if (ey.op == EXP.error)
@@ -1254,19 +1257,11 @@ private Expression resolveUFCS(Scope* sc, CallExp ce)
 ce.e1 = ey;
 if (isDotOpDispatch(ey))
 {
-// even opDispatch and UFCS must have valid arguments,
-// so now that we've seen indication of a problem,
-// check them for issues.
-Expressions* originalArguments = 
Expression.arraySyntaxCopy(ce.arguments);
-
 const errors = global.startGagging();
 e = ce.expressionSemantic(sc);
 if (!global.endGagging(errors))
 return e;
 
-if (arrayExpressionSemantic(originalArguments.peekSlice(), 
sc))
-return ErrorExp.get();
-
 /* fall down to UFCS */
 }
 else
diff --git a/gcc/testsuite/gdc.test/compilable/imports/test21098_phobos.d 
b/gcc/testsuite/gdc.test/compilable/imports/test21098_phobos.d
new file mode 100644
index 000..29c77eb047a
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/test21098_phobos.d
@@ -0,0 +1,77 @@
+struct Nullable(T)
+{
+static struct DontCallDestructorT
+{
+T payload;
+}
+
+DontCallDestructorT _value;
+
+string toString() const
+{
+Appender!string app;
+formatValueImpl(app, _value);
+return null;
+}
+}
+
+
+
+struct Appender(A)
+{
+InPlaceAppender!A impl;
+}
+
+struct InPlaceAppender(T)
+{
+static void toStringImpl(const T[] data)
+{
+string app;
+formatValue(app, data);
+}
+}
+
+
+
+void formatValueImpl(Writer, T)(Writer, const(T)) {}
+
+void formatValueImpl(Writer, T)(Writer w, T obj)
+if (is(T == U[], U))
+{
+formatValue(w, obj[0]);
+}
+
+enum HasToStringResult
+{
+none,
+bla
+}
+
+template hasToString(T)
+{
+static if (is(typeof(
+(T val) {
+val.toString(s);
+})))
+enum hasToString = HasToStringResult.bla;
+else
+enum hasToString = HasToStringResult.none;
+}
+
+void formatValueImpl(Writer, T)(ref Writer w, T val)
+if (is(T == struct) || is(T == union))
+{
+static if (hasToString!T)
+int dummy;
+formatElement(w, val.tupleof);
+}
+
+void formatElement(Writer, T)(Writer w, T val)
+{
+formatValueImpl(w, val);
+}
+
+void formatValue(Writer, T)(Writer w, T val)
+{
+formatValueImpl(w, val);
+}
diff --git a/gcc/testsuite/gdc.test/compilable/imports/test21098b.d 
b/gcc/testsuite/gdc.test/compilable/imports/test21098b.d
new file mode 100644
index 000..74c9fa80c87
--- /dev/null
+++ b/gcc/testsuite/gdc.test/compilable/imports/test21098b.d
@@ -0,0 +1,12 @@
+import imports.test21098_phobos : Appender, Nullable;
+
+struct Type {
+Nullable!(Type[]) templateArgs;
+}
+
+Type[] parseDeclarations() {
+Appender!(Type[]) members;
+return null;
+}
+
+enum ast = parseDeclarations();
diff --git a/gcc/testsuite/gdc.test/compilable/test21098.d 
b/gcc/testsuite/gdc.test/compilable/te

Re: [RFC] [C]New syntax for the argument of counted_by attribute for C language

2025-04-02 Thread Qing Zhao
Hi, Bill,

Thanks for the summary. 

I think that this is good.

Two more questions:

1. Shall we keep the same name of the attribute counted_by for the 2nd new 
syntax? 
Or use a new name, such as, “counted_by_exp"?

I don’t have strong preference here. As mentioned by Jacub in a 
previous email, these two syntaxes can be distinguished by the number 
of arguments of the attribute. 
  
So for GCC, there should be no issue with either old name of a new name.
However, I am not sure about CLANG. (Considering the complication with 
APPLE’s implementation)

2. The 2nd new syntax should resolve all the following new requests from Linux 
Kernel:
  2.1 Refer to a field in the nested structure
  2.2 Refer to globals or locals
  2.3 Represent simple expression
  2.4 Forward referencing

Except not very sure on 2.1: refer to a field in the nested structure

struct Y {
 int n;
 int other;
}
 
struct Z {   
 struct Y y;
 int array[]  __attribute__ ((counted_by(?y.n)));
};

in the above, what should be put instead of "?" to refer to the field "n" of 
the field "y" of the current object of this struct Z?

Based on my understanding, with the new syntax, 

struct Z {   
 struct Y y;
 int array[]  __attribute__ ((counted_by(struct Y y, y.n)));
};

i.e, the compiler will lookup “y” inside the current structure, then resolving 
the member access operator “.” as an expression. 

Is this correct understanding?

Thanks.

Qing  

> On Apr 2, 2025, at 14:35, Bill Wendling  wrote:
> 
> On Tue, Apr 1, 2025 at 11:49 PM Jakub Jelinek  wrote:
>> On Tue, Apr 01, 2025 at 05:13:46PM -0700, Bill Wendling wrote:
>>> On Tue, Apr 1, 2025 at 8:29 AM Martin Uecker  wrote:
 Am Dienstag, dem 01.04.2025 um 15:01 + schrieb Qing Zhao:
>> On Apr 1, 2025, at 10:04, Martin Uecker  wrote:
>> Am Montag, dem 31.03.2025 um 13:59 -0700 schrieb Bill Wendling:
 I'd like to offer up this to solve the issues we're facing. This is a
 combination of everything that's been discussed here (or at least that
 I've been able to read in the centi-thread :-).
>> 
>> Thanks! I think this proposal much better as it avoids undue burden
>> on parsers, but it does not address all my concerns.
>> 
>> 
>> From my side, the issue about compromising the scoping rules of C
>> is also about unintended non-local effects of code changes. In
>> my opinion, a change in a library header elsewhere should not cause
>> code in a local scope (which itself might also come from a macro) to
>> emit a warning or require a programmer to add a workaround. So I am
>> not convinced that adding warnings or a workaround such as
>> __builtin_global_ref  is a good solution.
>>> 
>>> To clarify, I'm not in favor of adding a generalized new scoping rule
>>> to C, but only for identifiers in attributes. From what I've seen in
>> 
>> There are existing attributes which support general expressions in their
>> arguments, e.g. the assume attribute, OpenMP attributes etc.  Those 
>> definitely
>> shouldn't change behavior if some new behavior for identifier lookup in 
>> attributes
>> is added.
>> Many others do support identifiers as their arguments (e.g. mode attribute,
>> 2 argument malloc attribute, ...).  Those can't change behavior either.
>> 
>> I think using either a predefined identifier or self-declared one is the
>> only reasonable way to go (whether it represents something like this pointer
>> in C++ or in the latter case possibly forward declaration of the member), 
>> otherwise
>> it will be hard to understand for users and very much error prone.
> 
> Thanks everyone. Here's an updated version of the proposal. As always,
> please correct and / or add your own rationales to these suggestions.
> 
> ---
> 1. Identifiers: For a single identifier, use the current syntax, which
> works in both GCC and Clang:
> 
> struct foo {
>  char count;
>  struct bar {
>struct {
>  int len;
>};
>struct {
>  struct {
>int *valid_use __counted_by(len); // Valid.
>  };
>};
>int *invalid_use __counted_by(count); // Invalid.
>  } b;
> };
> 
> It currently works and doesn't add any new scoping rules, even minor
> ones. Hopefully, this is the least controversial suggestion. :-)
> 
> 2. Expressions: Add forward declarations for fields used in
> expressions (other than single identifiers). For example:
> 
> const byte_size = 8;
> enum { PADDING = 42, TON_BEAR };
> struct foo {
>  int count;
>  int size;
>  int *buf __counted_by(int count, size; count * size + PADDING * byte_size);
> };
> 
> Variables forward declared inside of the attribute are fields inside
> the least enclosing, non-anonymous struct and must have the same type
> as those fields. All other identifiers are found outside the struct.
> 
> Rationale: This not only uses current C lookup rules but makes
> explicit how each identifier within an expression is meant to be
> resolved. Many compilers that aren't 

Re: [PATCH RFC] c++: optimize push_to_top_level [PR64500]

2025-04-02 Thread Marek Polacek
On Fri, Mar 28, 2025 at 10:02:28PM -0400, Patrick Palka wrote:
> On Fri, 28 Mar 2025, Jason Merrill wrote:
> 
> > Tested x86_64-pc-linux-gnu, initially with extra checking to make sure that
> > indeed nothing got saved from a namespace level.
> > 
> > This isn't a regression, but a 20% speedup for a simple change is pretty
> > attractive; what do people think about this change for GCC 15?
> > 
> > -- 8< --
> > 
> > Profiling showed that the loop to save away IDENTIFIER_BINDINGs from open
> > binding levels was taking 5% of total compilation time in the PR116285
> > testcase.  This turned out to be because we were unnecessarily trying to do
> > this for namespaces, whose bindings are found through
> > DECL_NAMESPACE_BINDINGS, not IDENTIFIER_BINDING.
> > 
> > As a result we would frequently loop through everything in std::, checking
> > whether it needs to be stored, and never storing anything.
> > 
> > This change actually appears to speed up compilation for the PR116285
> > testcase by ~20%.
> 
> Nice, this also speeds up compilation of stdc++.h by ~20%!  And std
> module compilation by ~10%.  +1

The patch looks good to me too, thanks.

Marek



Re: [PATCH] gcc.dg/analyzer/torture/switch-3.c: fix llp64 warnings

2025-04-02 Thread Jonathan Yong

On 3/30/25 2:52 AM, Jonathan Yong wrote:

Patch OK for master branch?

gcc/testsuite/ChangeLog:
   * gcc.dg/analyzer/torture/switch-3.c: fix llp64 warnings

diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/switch-3.c b/gcc/ 
testsuite/gcc.dg/analyzer/torture/switch-3.c

index 57b8acdb292..b40be664d38 100644
--- a/gcc/testsuite/gcc.dg/analyzer/torture/switch-3.c
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/switch-3.c
@@ -68,7 +68,7 @@ extern void check_init_u32 (__u32 v);
  /* Adapted/reduced from arch/x86/kernel/cpu/mtrr/if.c: mtrr_ioctl,
     which is GPL-2.0  */

-long mtrr_ioctl(unsigned int cmd, unsigned long __arg) {
+long mtrr_ioctl(unsigned int cmd, __UINTPTR_TYPE__ __arg) {
    int err = 0;
    struct mtrr_sentry sentry;
    struct mtrr_gentry gentry;



Pushed to gcc master branch.



Re: [PATCH] doc: Extend musttail attribute docs

2025-04-02 Thread Richard Biener
On Wed, 2 Apr 2025, Jakub Jelinek wrote:

> On Wed, Apr 02, 2025 at 10:32:20AM +0200, Richard Biener wrote:
> > I wonder if we can amend the documentation to suggest to end lifetime
> > of variables explicitly by proper scoping?
> 
> In the -Wmaybe-musttail-local-addr attribute description I've already
> tried to show that in the example, but if you think something like
> the following would make it clearer:

OK.

Thanks,
Richard.

> 2025-04-02  Jakub Jelinek  
> 
>   * doc/extend.texi (musttail statement attribute): Hint how
>   to avoid -Wmaybe-musttail-local-addr warnings.
> 
> --- gcc/doc/extend.texi.jj2025-04-02 10:46:40.447281167 +0200
> +++ gcc/doc/extend.texi   2025-04-02 11:24:25.042027221 +0200
> @@ -9368,6 +9368,31 @@ baz (int *x)
>  @}
>  @}
>  @end smallexample
> +
> +To avoid the @option{-Wmaybe-musttail-local-addr} warning in the
> +above @code{*x == 2} case and similar code, consider defining the
> +maybe escaped variables in a separate scope which will end before the
> +return statement if possible to make it clear that the variable is not
> +live during the call.  So
> +
> +@smallexample
> +  else if (*x == 2)
> +@{
> +  @{
> +int a = 42;
> +bar (&a);
> +  @}
> +  /* The call will be tail called (would not be without the
> + attribute), if bar stores the pointer anywhere, dereferencing
> + it in foo will be undefined behavior and there will be a warning
> + emitted for this with @option{-Wextra}, which implies
> + @option{-Wmaybe-musttail-local-addr}.  */
> +  [[gnu::musttail]] return foo (nullptr);
> +@}
> +@end smallexample
> +
> +in this case.  That is not possible if it is function argument which
> +is address taken because those are in scope for the whole function.
>  @end table
>  
>  @node Attribute Syntax
> 
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)


[PATCH] testsuite: Remove guality xfails for aarch64*-*-*

2025-04-02 Thread Christophe Lyon
Since r15-7878-ge1c49f413c8, these tests appear as XPASS on aarch64,
so we can remove the xfails introduced by r12-102-gf31ddad8ac8f11.

gcc/testsuite/ChangeLog:

* gcc.dg/guality/pr90074.c: Remove xfail for aarch64.
* gcc.dg/guality/pr90716.c: Likewise.
---
 gcc/testsuite/gcc.dg/guality/pr90074.c | 4 ++--
 gcc/testsuite/gcc.dg/guality/pr90716.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/guality/pr90074.c 
b/gcc/testsuite/gcc.dg/guality/pr90074.c
index 2fd884209f2..12949282516 100644
--- a/gcc/testsuite/gcc.dg/guality/pr90074.c
+++ b/gcc/testsuite/gcc.dg/guality/pr90074.c
@@ -25,7 +25,7 @@ int main()
  debug stmt for the final value of the loop during loop distribution
  which would fix the UNSUPPORTED cases.
  c is optimized out at -Og for no obvious reason.  */
-  optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "8" { xfail { 
aarch64*-*-* && { any-opts "-fno-fat-lto-objects" } } } } } */
-/* { dg-final { gdb-test .-1 "c + 1" "2" { xfail { aarch64*-*-* && { 
any-opts "-fno-fat-lto-objects" } } } } } */
+  optimize_me_not(); /* { dg-final { gdb-test . "i + 1" "8" } } */
+/* { dg-final { gdb-test .-1 "c + 1" "2" } } */
   return 0;
 }
diff --git a/gcc/testsuite/gcc.dg/guality/pr90716.c 
b/gcc/testsuite/gcc.dg/guality/pr90716.c
index fe7e5567625..b2f5c9d146e 100644
--- a/gcc/testsuite/gcc.dg/guality/pr90716.c
+++ b/gcc/testsuite/gcc.dg/guality/pr90716.c
@@ -20,6 +20,6 @@ int main()
  Instead test j + 1 which will make the test UNSUPPORTED if i
  is optimized out.  Since the test previously had wrong debug
  with j == 0 this is acceptable.  */
-  optimize_me_not(); /* { dg-final { gdb-test . "j + 1" "9" { xfail { 
aarch64*-*-* && { any-opts "-fno-fat-lto-objects" } } } } } */
+  optimize_me_not(); /* { dg-final { gdb-test . "j + 1" "9" } } */
   return 0;
 }
-- 
2.34.1



[ping][PATCH] libgcobol: Provide fallbacks for C32 strfromf32/64 [PR119296].

2025-04-02 Thread Iain Sandoe
Hi folks,
it would be great to reduce the in-flight patch stack a bit :)

> On 25 Mar 2025, at 16:40, Iain Sandoe  wrote:
> 
> This is on top of the C++-ify configure and random_r patches.
> Tested on x86_64,aarch64-Linux and x86_64-darwin, OK for trunk?
> thanks
> Iain
> 
> --- 8< --- 
> 
> strfrom{f,d,l,fN) are all C23 and might not be available in general.
> This uses snprintf() to provide fall-backs where the libc does not
> yet have support.
> 
>   PR cobol/119296
> 
> libgcobol/ChangeLog:
> 
>   * config.h.in: Regenerate.
>   * configure: Regenerate.
>   * configure.ac: Check for availability of strfromf32 and
>   strfromf64.
>   * libgcobol.cc (strfromf32, strfromf64): New.
> 
> Signed-off-by: Iain Sandoe 
> ---
> libgcobol/config.h.in  |  6 ++
> libgcobol/configure| 13 +++--
> libgcobol/configure.ac |  3 +++
> libgcobol/libgcobol.cc | 24 
> 4 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/libgcobol/config.h.in b/libgcobol/config.h.in
> index e7e1492b579..d61ff7ad497 100644
> --- a/libgcobol/config.h.in
> +++ b/libgcobol/config.h.in
> @@ -36,6 +36,12 @@
> /* Define to 1 if you have the  header file. */
> #undef HAVE_STDLIB_H
> 
> +/* Define to 1 if you have the `strfromf32' function. */
> +#undef HAVE_STRFROMF32
> +
> +/* Define to 1 if you have the `strfromf64' function. */
> +#undef HAVE_STRFROMF64
> +
> /* Define to 1 if you have the  header file. */
> #undef HAVE_STRINGS_H
> 
> diff --git a/libgcobol/configure b/libgcobol/configure
> index acf78646d5b..23474881f35 100755
> --- a/libgcobol/configure
> +++ b/libgcobol/configure
> @@ -2696,6 +2696,8 @@ as_fn_append ac_func_list " random_r"
> as_fn_append ac_func_list " srandom_r"
> as_fn_append ac_func_list " initstate_r"
> as_fn_append ac_func_list " setstate_r"
> +as_fn_append ac_func_list " strfromf32"
> +as_fn_append ac_func_list " strfromf64"
> # Check that the precious variables saved in the cache have kept the same
> # value.
> ac_cache_corrupted=false
> @@ -11621,7 +11623,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 11624 "configure"
> +#line 11626 "configure"
> #include "confdefs.h"
> 
> #if HAVE_DLFCN_H
> @@ -11727,7 +11729,7 @@ else
>   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>   lt_status=$lt_dlunknown
>   cat > conftest.$ac_ext <<_LT_EOF
> -#line 11730 "configure"
> +#line 11732 "configure"
> #include "confdefs.h"
> 
> #if HAVE_DLFCN_H
> @@ -17699,6 +17701,13 @@ done
> 
> 
> 
> +# These are C23, and might not be available in libc.
> +
> +
> +
> +
> +
> +
> if test "${multilib}" = "yes"; then
>   multilib_arg="--enable-multilib"
> else
> diff --git a/libgcobol/configure.ac b/libgcobol/configure.ac
> index 0356f9e9c67..6e1ea3700d0 100644
> --- a/libgcobol/configure.ac
> +++ b/libgcobol/configure.ac
> @@ -198,6 +198,9 @@ AC_SUBST(extra_ldflags_libgcobol)
> # These are GLIBC
> AC_CHECK_FUNCS_ONCE(random_r srandom_r initstate_r setstate_r)
> 
> +# These are C23, and might not be available in libc.
> +AC_CHECK_FUNCS_ONCE(strfromf32 strfromf64)
> +
> if test "${multilib}" = "yes"; then
>   multilib_arg="--enable-multilib"
> else
> diff --git a/libgcobol/libgcobol.cc b/libgcobol/libgcobol.cc
> index 6aeaaa2c142..85f016e9735 100644
> --- a/libgcobol/libgcobol.cc
> +++ b/libgcobol/libgcobol.cc
> @@ -68,6 +68,30 @@
> 
> #include "exceptl.h"
> 
> +#if !defined (HAVE_STRFROMF32)
> +# if __FLT_MANT_DIG__ == 24 && __FLT_MAX_EXP__ == 128
> +static int
> +strfromf32 (char *s, size_t n, const char *f, float v)
> +{
> +  return snprintf (s, n, f, (double) v);
> +}
> +# else
> +#  error "It looks like float on this platform is not IEEE754"
> +# endif
> +#endif
> +
> +#if !defined (HAVE_STRFROMF64)
> +# if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024
> +static int
> +strfromf64 (char *s, size_t n, const char *f, double v)
> +{
> +  return snprintf (s, n, f, v);
> +}
> +# else
> +#  error "It looks like double on this platform is not IEEE754"
> +# endif
> +#endif
> +
> // This couldn't be defined in symbols.h because it conflicts with a LEVEL66
> // in parse.h
> #define LEVEL66 (66)
> -- 
> 2.39.2 (Apple Git-143)
> 



Re: [RFC] [C]New syntax for the argument of counted_by attribute for C language

2025-04-02 Thread Bill Wendling
On Tue, Apr 1, 2025 at 11:49 PM Jakub Jelinek  wrote:
> On Tue, Apr 01, 2025 at 05:13:46PM -0700, Bill Wendling wrote:
> > On Tue, Apr 1, 2025 at 8:29 AM Martin Uecker  wrote:
> > > Am Dienstag, dem 01.04.2025 um 15:01 + schrieb Qing Zhao:
> > > > > On Apr 1, 2025, at 10:04, Martin Uecker  wrote:
> > > > > Am Montag, dem 31.03.2025 um 13:59 -0700 schrieb Bill Wendling:
> > > > > > > I'd like to offer up this to solve the issues we're facing. This 
> > > > > > > is a
> > > > > > > combination of everything that's been discussed here (or at least 
> > > > > > > that
> > > > > > > I've been able to read in the centi-thread :-).
> > > > >
> > > > > Thanks! I think this proposal much better as it avoids undue burden
> > > > > on parsers, but it does not address all my concerns.
> > > > >
> > > > >
> > > > > From my side, the issue about compromising the scoping rules of C
> > > > > is also about unintended non-local effects of code changes. In
> > > > > my opinion, a change in a library header elsewhere should not cause
> > > > > code in a local scope (which itself might also come from a macro) to
> > > > > emit a warning or require a programmer to add a workaround. So I am
> > > > > not convinced that adding warnings or a workaround such as
> > > > > __builtin_global_ref  is a good solution.
> >
> > To clarify, I'm not in favor of adding a generalized new scoping rule
> > to C, but only for identifiers in attributes. From what I've seen in
>
> There are existing attributes which support general expressions in their
> arguments, e.g. the assume attribute, OpenMP attributes etc.  Those definitely
> shouldn't change behavior if some new behavior for identifier lookup in 
> attributes
> is added.
> Many others do support identifiers as their arguments (e.g. mode attribute,
> 2 argument malloc attribute, ...).  Those can't change behavior either.
>
> I think using either a predefined identifier or self-declared one is the
> only reasonable way to go (whether it represents something like this pointer
> in C++ or in the latter case possibly forward declaration of the member), 
> otherwise
> it will be hard to understand for users and very much error prone.

Thanks everyone. Here's an updated version of the proposal. As always,
please correct and / or add your own rationales to these suggestions.

---
1. Identifiers: For a single identifier, use the current syntax, which
works in both GCC and Clang:

struct foo {
  char count;
  struct bar {
struct {
  int len;
};
struct {
  struct {
int *valid_use __counted_by(len); // Valid.
  };
};
int *invalid_use __counted_by(count); // Invalid.
  } b;
};

It currently works and doesn't add any new scoping rules, even minor
ones. Hopefully, this is the least controversial suggestion. :-)

2. Expressions: Add forward declarations for fields used in
expressions (other than single identifiers). For example:

const byte_size = 8;
enum { PADDING = 42, TON_BEAR };
struct foo {
  int count;
  int size;
  int *buf __counted_by(int count, size; count * size + PADDING * byte_size);
};

Variables forward declared inside of the attribute are fields inside
the least enclosing, non-anonymous struct and must have the same type
as those fields. All other identifiers are found outside the struct.

Rationale: This not only uses current C lookup rules but makes
explicit how each identifier within an expression is meant to be
resolved. Many compilers that aren't able to add new identifier
resolution rules based on non-codified scoping rules, and so this
syntax will allow them to implement this feature.

Bonus suggestion (by yours truly):

I still like my idea of using a function pointer, but It might not be
as necessary with option (2).

---

-bw


[committed] cobol: Plug memory leak caused by intermediate_e stack-frame [PR119521]

2025-04-02 Thread Robert Dubner
>From ce33db9c7c440be3988d4ad90f2ec41fea9c Mon Sep 17 00:00:00 2001
From: Bob Dubner mailto:rdub...@symas.com
Date: Wed, 2 Apr 2025 12:18:08 -0400
Subject: [PATCH] cobol: Plug memory leak caused by intermediate_e
stack-frame
 variables. [PR119521]

COBOL variables with attribute intermediate_e are being allocated on
the stack frame, but their data was assigned using malloc(), without
a corresponding call to free().  For numerics, the problem is solved
with a fixed allocation of sixteen bytes for the cblc_field_t::data
member (sixteen is big enough for all data types) and with a fixed
allocation of 8,192 bytes for the alphanumeric type.

In use, the intermediate numeric data types are "shrunk" to the minimum
applicable size.  The intermediate alphanumerics, generally used as
destination targets for functions, are trimmed as well.

gcc/cobol

PR cobol/119521
* genapi.cc: (parser_division): Change comment.
(parser_symbol_add): Change intermediate_t handling.
* parse.y: Multiple changes to new_alphanumeric() calls.
* parse_ante.h: Establish named constant for date function
calls.  Change declaration of new_alphanumeric() function.
* symbols.cc: (new_temporary_impl): Use named constant
for default size of temporary alphanumerics.
* symbols.h: Establish MAXIMUM_ALPHA_LENGTH constant.

libgcobol

PR cobol/119521
* intrinsic.cc: (__gg__reverse): Trim final result for
intermediate_e.
* libgcobol.cc: (__gg__adjust_dest_size): Abort on attempt to
increase
the size of a result.  (__gg__module_name): Formatting.

__gg__reverse(): Resize only intermediates
---
 gcc/cobol/genapi.cc| 85 --
 gcc/cobol/parse.y  | 73 +---
 gcc/cobol/parse_ante.h |  3 +-
 gcc/cobol/symbols.cc   |  3 +-
 gcc/cobol/symbols.h|  6 +++
 libgcobol/intrinsic.cc |  4 ++
 libgcobol/libgcobol.cc |  8 ++--
 7 files changed, 94 insertions(+), 88 deletions(-)

diff --git a/gcc/cobol/genapi.cc b/gcc/cobol/genapi.cc
index 92ab460e2c0b..4d958cfc0d4b 100644
--- a/gcc/cobol/genapi.cc
+++ b/gcc/cobol/genapi.cc
@@ -6647,7 +6647,10 @@ parser_division(cbl_division_t division,
 
   if( args[i].refer.field->attr & any_length_e )
 {
-//gg_printf("side channel 0x%lx\n",
gg_array_value(var_decl_call_parameter_lengths, rt_i), NULL_TREE);
+// gg_printf("side channel: Length of \"%s\" is %ld\n", 
+  // member(args[i].refer.field->var_decl_node,
"name"),
+  // gg_array_value(var_decl_call_parameter_lengths,
rt_i), 
+  // NULL_TREE);
 
 // Get the length from the global lengths[] side channel.
Don't
 // forget to use the length mask on the table value.
@@ -16753,55 +16756,47 @@ parser_symbol_add(struct cbl_field_t *new_var )
 
 if( bytes_to_allocate )
   {
-  if(new_var->attr & (intermediate_e)
-  && new_var->type != FldLiteralN
-  && new_var->type != FldLiteralA )
+  // We need a unique name for the allocated data for this COBOL
variable:
+  char achDataName[256];
+  if( new_var->attr & external_e )
 {
-// We'll malloc() data in initialize_variable
-data_area = null_pointer_node;
+sprintf(achDataName, "%s", new_var->name);
+}
+  else if( new_var->name[0] == '_' )
+{
+// Avoid doubling up on leading underscore
+sprintf(achDataName,
+"%s_data_%lu",
+new_var->name,
+sv_data_name_counter++);
 }
   else
 {
-// We need a unique name for the allocated data for this
COBOL variable:
-char achDataName[256];
-if( new_var->attr & external_e )
-  {
-  sprintf(achDataName, "%s", new_var->name);
-  }
-else if( new_var->name[0] == '_' )
-  {
-  // Avoid doubling up on leading underscore
-  sprintf(achDataName,
-  "%s_data_%lu",
-  new_var->name,
-  sv_data_name_counter++);
-  }
-else
-  {
-  sprintf(achDataName,
-  "_%s_data_%lu",
-  new_var->name,
-  sv_data_name_counter++);
-  }
+sprintf(achDataName,
+"_%s_data_%lu",
+new_var->name,
+sv_data_name_counter++);
+}
 
-if( new_var->attr & external_e )
-  {
-  tree array_type = build_array_type_nelts(UCHAR,
bytes_to_allocate);
-  new_var->data_decl_node = gg_define_variable(
-  array_type,
-

Re: [ping][PATCH] libgcobol: Provide fallbacks for C32 strfromf32/64 [PR119296].

2025-04-02 Thread Iain Sandoe



> On 2 Apr 2025, at 20:02, Robert Dubner  wrote:
> 
> Since you say it tests okay, I say it's "Okay for trunk".

thanks.
> 
> I am new to this, as you know, and I am still trying to figure out how to
> make all of this work.  Nor have Jim and I figured out how to handle our
> joint responsibility.

understood..
> 
> Hence delays.

… will try not to hassle too much :)

> 
> 
>> -Original Message-
>> From: Iain Sandoe 
>> Sent: Wednesday, April 2, 2025 14:24
>> To: GCC Patches 
>> Cc: James K. Lowden ; Robert Dubner
>> 
>> Subject: [ping][PATCH] libgcobol: Provide fallbacks for C32
> strfromf32/64
>> [PR119296].
>> 
>> Hi folks,
>> it would be great to reduce the in-flight patch stack a bit :)
>> 
>>> On 25 Mar 2025, at 16:40, Iain Sandoe  wrote:
>>> 
>>> This is on top of the C++-ify configure and random_r patches.
>>> Tested on x86_64,aarch64-Linux and x86_64-darwin, OK for trunk?
>>> thanks
>>> Iain
>>> 
>>> --- 8< ---
>>> 
>>> strfrom{f,d,l,fN) are all C23 and might not be available in general.
>>> This uses snprintf() to provide fall-backs where the libc does not
>>> yet have support.
>>> 
>>> PR cobol/119296
>>> 
>>> libgcobol/ChangeLog:
>>> 
>>> * config.h.in: Regenerate.
>>> * configure: Regenerate.
>>> * configure.ac: Check for availability of strfromf32 and
>>> strfromf64.
>>> * libgcobol.cc (strfromf32, strfromf64): New.
>>> 
>>> Signed-off-by: Iain Sandoe 
>>> ---
>>> libgcobol/config.h.in  |  6 ++
>>> libgcobol/configure| 13 +++--
>>> libgcobol/configure.ac |  3 +++
>>> libgcobol/libgcobol.cc | 24 
>>> 4 files changed, 44 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/libgcobol/config.h.in b/libgcobol/config.h.in
>>> index e7e1492b579..d61ff7ad497 100644
>>> --- a/libgcobol/config.h.in
>>> +++ b/libgcobol/config.h.in
>>> @@ -36,6 +36,12 @@
>>> /* Define to 1 if you have the  header file. */
>>> #undef HAVE_STDLIB_H
>>> 
>>> +/* Define to 1 if you have the `strfromf32' function. */
>>> +#undef HAVE_STRFROMF32
>>> +
>>> +/* Define to 1 if you have the `strfromf64' function. */
>>> +#undef HAVE_STRFROMF64
>>> +
>>> /* Define to 1 if you have the  header file. */
>>> #undef HAVE_STRINGS_H
>>> 
>>> diff --git a/libgcobol/configure b/libgcobol/configure
>>> index acf78646d5b..23474881f35 100755
>>> --- a/libgcobol/configure
>>> +++ b/libgcobol/configure
>>> @@ -2696,6 +2696,8 @@ as_fn_append ac_func_list " random_r"
>>> as_fn_append ac_func_list " srandom_r"
>>> as_fn_append ac_func_list " initstate_r"
>>> as_fn_append ac_func_list " setstate_r"
>>> +as_fn_append ac_func_list " strfromf32"
>>> +as_fn_append ac_func_list " strfromf64"
>>> # Check that the precious variables saved in the cache have kept the
>> same
>>> # value.
>>> ac_cache_corrupted=false
>>> @@ -11621,7 +11623,7 @@ else
>>>  lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>>>  lt_status=$lt_dlunknown
>>>  cat > conftest.$ac_ext <<_LT_EOF
>>> -#line 11624 "configure"
>>> +#line 11626 "configure"
>>> #include "confdefs.h"
>>> 
>>> #if HAVE_DLFCN_H
>>> @@ -11727,7 +11729,7 @@ else
>>>  lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>>>  lt_status=$lt_dlunknown
>>>  cat > conftest.$ac_ext <<_LT_EOF
>>> -#line 11730 "configure"
>>> +#line 11732 "configure"
>>> #include "confdefs.h"
>>> 
>>> #if HAVE_DLFCN_H
>>> @@ -17699,6 +17701,13 @@ done
>>> 
>>> 
>>> 
>>> +# These are C23, and might not be available in libc.
>>> +
>>> +
>>> +
>>> +
>>> +
>>> +
>>> if test "${multilib}" = "yes"; then
>>>  multilib_arg="--enable-multilib"
>>> else
>>> diff --git a/libgcobol/configure.ac b/libgcobol/configure.ac
>>> index 0356f9e9c67..6e1ea3700d0 100644
>>> --- a/libgcobol/configure.ac
>>> +++ b/libgcobol/configure.ac
>>> @@ -198,6 +198,9 @@ AC_SUBST(extra_ldflags_libgcobol)
>>> # These are GLIBC
>>> AC_CHECK_FUNCS_ONCE(random_r srandom_r initstate_r setstate_r)
>>> 
>>> +# These are C23, and might not be available in libc.
>>> +AC_CHECK_FUNCS_ONCE(strfromf32 strfromf64)
>>> +
>>> if test "${multilib}" = "yes"; then
>>>  multilib_arg="--enable-multilib"
>>> else
>>> diff --git a/libgcobol/libgcobol.cc b/libgcobol/libgcobol.cc
>>> index 6aeaaa2c142..85f016e9735 100644
>>> --- a/libgcobol/libgcobol.cc
>>> +++ b/libgcobol/libgcobol.cc
>>> @@ -68,6 +68,30 @@
>>> 
>>> #include "exceptl.h"
>>> 
>>> +#if !defined (HAVE_STRFROMF32)
>>> +# if __FLT_MANT_DIG__ == 24 && __FLT_MAX_EXP__ == 128
>>> +static int
>>> +strfromf32 (char *s, size_t n, const char *f, float v)
>>> +{
>>> +  return snprintf (s, n, f, (double) v);
>>> +}
>>> +# else
>>> +#  error "It looks like float on this platform is not IEEE754"
>>> +# endif
>>> +#endif
>>> +
>>> +#if !defined (HAVE_STRFROMF64)
>>> +# if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024
>>> +static int
>>> +strfromf64 (char *s, size_t n, const char *f, double v)
>>> +{
>>> +  return snprintf (s, n, f, v);
>>> +}
>>> +# else
>>> +#  error "It looks like double on this platform is not IEEE754"
>>> +# endif
>>> +#endif
>>> +
>>> // This 

RE: [ping][PATCH] libgcobol: Provide fallbacks for C32 strfromf32/64 [PR119296].

2025-04-02 Thread Robert Dubner
> -Original Message-
> From: Iain Sandoe 
> Sent: Wednesday, April 2, 2025 15:05
> To: Robert Dubner 
> Cc: GCC Patches ; James K. Lowden
> 
> Subject: Re: [ping][PATCH] libgcobol: Provide fallbacks for C32
> strfromf32/64 [PR119296].
>
>
>
> > On 2 Apr 2025, at 20:02, Robert Dubner  wrote:
> >
> > Since you say it tests okay, I say it's "Okay for trunk".
>
> thanks.
> >
> > I am new to this, as you know, and I am still trying to figure out how
> to
> > make all of this work.  Nor have Jim and I figured out how to handle our
> > joint responsibility.
>
> understood..
> >
> > Hence delays.
>
> … will try not to hassle too much :)

Go ahead and hassle.  As the old saying goes:  If I couldn't take a joke, I 
never should have signed up.


>
> >
> >
> >> -Original Message-
> >> From: Iain Sandoe 
> >> Sent: Wednesday, April 2, 2025 14:24
> >> To: GCC Patches 
> >> Cc: James K. Lowden ; Robert Dubner
> >> 
> >> Subject: [ping][PATCH] libgcobol: Provide fallbacks for C32
> > strfromf32/64
> >> [PR119296].
> >>
> >> Hi folks,
> >> it would be great to reduce the in-flight patch stack a bit :)
> >>
> >>> On 25 Mar 2025, at 16:40, Iain Sandoe  wrote:
> >>>
> >>> This is on top of the C++-ify configure and random_r patches.
> >>> Tested on x86_64,aarch64-Linux and x86_64-darwin, OK for trunk?
> >>> thanks
> >>> Iain
> >>>
> >>> --- 8< ---
> >>>
> >>> strfrom{f,d,l,fN) are all C23 and might not be available in general.
> >>> This uses snprintf() to provide fall-backs where the libc does not
> >>> yet have support.
> >>>
> >>>   PR cobol/119296
> >>>
> >>> libgcobol/ChangeLog:
> >>>
> >>>   * config.h.in: Regenerate.
> >>>   * configure: Regenerate.
> >>>   * configure.ac: Check for availability of strfromf32 and
> >>>   strfromf64.
> >>>   * libgcobol.cc (strfromf32, strfromf64): New.
> >>>
> >>> Signed-off-by: Iain Sandoe 
> >>> ---
> >>> libgcobol/config.h.in  |  6 ++
> >>> libgcobol/configure| 13 +++--
> >>> libgcobol/configure.ac |  3 +++
> >>> libgcobol/libgcobol.cc | 24 
> >>> 4 files changed, 44 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libgcobol/config.h.in b/libgcobol/config.h.in
> >>> index e7e1492b579..d61ff7ad497 100644
> >>> --- a/libgcobol/config.h.in
> >>> +++ b/libgcobol/config.h.in
> >>> @@ -36,6 +36,12 @@
> >>> /* Define to 1 if you have the  header file. */
> >>> #undef HAVE_STDLIB_H
> >>>
> >>> +/* Define to 1 if you have the `strfromf32' function. */
> >>> +#undef HAVE_STRFROMF32
> >>> +
> >>> +/* Define to 1 if you have the `strfromf64' function. */
> >>> +#undef HAVE_STRFROMF64
> >>> +
> >>> /* Define to 1 if you have the  header file. */
> >>> #undef HAVE_STRINGS_H
> >>>
> >>> diff --git a/libgcobol/configure b/libgcobol/configure
> >>> index acf78646d5b..23474881f35 100755
> >>> --- a/libgcobol/configure
> >>> +++ b/libgcobol/configure
> >>> @@ -2696,6 +2696,8 @@ as_fn_append ac_func_list " random_r"
> >>> as_fn_append ac_func_list " srandom_r"
> >>> as_fn_append ac_func_list " initstate_r"
> >>> as_fn_append ac_func_list " setstate_r"
> >>> +as_fn_append ac_func_list " strfromf32"
> >>> +as_fn_append ac_func_list " strfromf64"
> >>> # Check that the precious variables saved in the cache have kept the
> >> same
> >>> # value.
> >>> ac_cache_corrupted=false
> >>> @@ -11621,7 +11623,7 @@ else
> >>>  lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >>>  lt_status=$lt_dlunknown
> >>>  cat > conftest.$ac_ext <<_LT_EOF
> >>> -#line 11624 "configure"
> >>> +#line 11626 "configure"
> >>> #include "confdefs.h"
> >>>
> >>> #if HAVE_DLFCN_H
> >>> @@ -11727,7 +11729,7 @@ else
> >>>  lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >>>  lt_status=$lt_dlunknown
> >>>  cat > conftest.$ac_ext <<_LT_EOF
> >>> -#line 11730 "configure"
> >>> +#line 11732 "configure"
> >>> #include "confdefs.h"
> >>>
> >>> #if HAVE_DLFCN_H
> >>> @@ -17699,6 +17701,13 @@ done
> >>>
> >>>
> >>>
> >>> +# These are C23, and might not be available in libc.
> >>> +
> >>> +
> >>> +
> >>> +
> >>> +
> >>> +
> >>> if test "${multilib}" = "yes"; then
> >>>  multilib_arg="--enable-multilib"
> >>> else
> >>> diff --git a/libgcobol/configure.ac b/libgcobol/configure.ac
> >>> index 0356f9e9c67..6e1ea3700d0 100644
> >>> --- a/libgcobol/configure.ac
> >>> +++ b/libgcobol/configure.ac
> >>> @@ -198,6 +198,9 @@ AC_SUBST(extra_ldflags_libgcobol)
> >>> # These are GLIBC
> >>> AC_CHECK_FUNCS_ONCE(random_r srandom_r initstate_r setstate_r)
> >>>
> >>> +# These are C23, and might not be available in libc.
> >>> +AC_CHECK_FUNCS_ONCE(strfromf32 strfromf64)
> >>> +
> >>> if test "${multilib}" = "yes"; then
> >>>  multilib_arg="--enable-multilib"
> >>> else
> >>> diff --git a/libgcobol/libgcobol.cc b/libgcobol/libgcobol.cc
> >>> index 6aeaaa2c142..85f016e9735 100644
> >>> --- a/libgcobol/libgcobol.cc
> >>> +++ b/libgcobol/libgcobol.cc
> >>> @@ -68,6 +68,30 @@
> >>>
> >>> #include "exceptl.h"
> >>>
> >>> +#if !defined (HAVE_STRFROMF32)
> >>> +# if __FLT_MANT_DIG__ == 24 && __FL

Re: Patch ping [PATCH] tailc: Don't fail musttail calls if they use or could use local arguments, instead warn [PR119376]

2025-04-02 Thread Richard Biener
On Tue, 1 Apr 2025, Jakub Jelinek wrote:

> Hi!
> 
> On Tue, Mar 25, 2025 at 08:34:10AM +0100, Jakub Jelinek wrote:
> > As discussed here and in bugzilla, [[clang::musttail]] attribute in clang
> > not just strongly asks for tail call or error, but changes behavior.
> > To quote:
> > https://clang.llvm.org/docs/AttributeReference.html#musttail
> > "The lifetimes of all local variables and function parameters end 
> > immediately
> > before the call to the function.  This means that it is undefined behaviour
> > to pass a pointer or reference to a local variable to the called function,
> > which is not the case without the attribute.  Clang will emit a warning in
> > common cases where this happens."
> > 
> > The GCC behavior was just to error if we can't prove the musttail callee
> > could not have dereferenced escaped pointers to local vars or parameters
> > of the caller.  That is still the case for variables with non-trivial
> > destruction (even in clang), like vars with C++ non-trivial destructors or
> > variables with cleanup attribute.
> > 
> > The following patch changes the behavior to match that of clang, for all of
> > [[clang::musttail]], [[gnu::musttail]] and __attribute__((musttail)).
> > 
> > clang 20 actually added warning for some cases of it in
> > https://github.com/llvm/llvm-project/pull/109255
> > but it is under -Wreturn-stack-address warning.
> > 
> > Now, gcc doesn't have that warning, but -Wreturn-local-addr instead, and
> > IMHO it is better to have this under new warnings, because this isn't about
> > returning local address, but about passing it to a musttail call, or maybe
> > escaping to a musttail call.  And perhaps users will appreciate they can
> > control it separately as well.
> > 
> > The patch introduces 2 new warnings.
> > -Wmusttail-local-addr
> > which is turn on by default and warns for the always dumb cases of passing
> > an address of a local variable or parameter to musttail call's argument.
> > And then
> > -Wmaybe-musttail-local-addr
> > which is only diagnosed if -Wmusttail-local-addr was not diagnosed and
> > diagnoses at most one (so that we don't emit 100s of warnings for one call
> > if 100s of vars can escape) case where an address of a local var could have
> > escaped to the musttail call.  This is less severe, the code doesn't have
> > to be obviously wrong, so the warning is only enabled in -Wextra.
> > 
> > And I've adjusted also the documentation for this change and addition of
> > new warnings.
> 
> I'd like to ping the
> https://gcc.gnu.org/pipermail/gcc-patches/2025-March/679182.html
> patch.
> I know it is quite controversial and if clang wouldn't be the first
> to implement this I'd certainly not go that way; I am willing to change
> the warning option names or move the maybe one from -Wextra to -Wall
> if there is agreement on that.  Unfortunately there seems to be
> quite a lot of code in the wild that uses this attribute already
> and without this patch GCC 15 will simply not be able to compile that
> (whether it is firefox (skia etc.), protobuf, ...).
> 
> The only other option is IMHO to drop the musttail attribute support
> for GCC 15 or name it differently with different semantics.
> But not sure projects in the wild will like to annotate their calls
> with two different musttail like attributes if they satisfy behavior
> of both.

OK.

I wonder if we can amend the documentation to suggest to end lifetime
of variables explicitly by proper scoping?

Thanks,
Richard.


[PATCH] doc: Extend musttail attribute docs

2025-04-02 Thread Jakub Jelinek
On Wed, Apr 02, 2025 at 10:32:20AM +0200, Richard Biener wrote:
> I wonder if we can amend the documentation to suggest to end lifetime
> of variables explicitly by proper scoping?

In the -Wmaybe-musttail-local-addr attribute description I've already
tried to show that in the example, but if you think something like
the following would make it clearer:

2025-04-02  Jakub Jelinek  

* doc/extend.texi (musttail statement attribute): Hint how
to avoid -Wmaybe-musttail-local-addr warnings.

--- gcc/doc/extend.texi.jj  2025-04-02 10:46:40.447281167 +0200
+++ gcc/doc/extend.texi 2025-04-02 11:24:25.042027221 +0200
@@ -9368,6 +9368,31 @@ baz (int *x)
 @}
 @}
 @end smallexample
+
+To avoid the @option{-Wmaybe-musttail-local-addr} warning in the
+above @code{*x == 2} case and similar code, consider defining the
+maybe escaped variables in a separate scope which will end before the
+return statement if possible to make it clear that the variable is not
+live during the call.  So
+
+@smallexample
+  else if (*x == 2)
+@{
+  @{
+int a = 42;
+bar (&a);
+  @}
+  /* The call will be tail called (would not be without the
+ attribute), if bar stores the pointer anywhere, dereferencing
+ it in foo will be undefined behavior and there will be a warning
+ emitted for this with @option{-Wextra}, which implies
+ @option{-Wmaybe-musttail-local-addr}.  */
+  [[gnu::musttail]] return foo (nullptr);
+@}
+@end smallexample
+
+in this case.  That is not possible if it is function argument which
+is address taken because those are in scope for the whole function.
 @end table
 
 @node Attribute Syntax


Jakub



Re: [PATCH] OpenMP: Require target and/or targetsync init modifier [PR118965]

2025-04-02 Thread Tobias Burnus

Sandra Loosemore wrote:

As noted in PR 118965, the initial interop implementation overlooked
the requirement in the OpenMP spec that at least one of the "target"
and "targetsync" modifiers is required in both the interop construct
init clause and the declare variant append_args clause.


... which is not surprising with OpenMP 5.2/6.0 syntax,
which makes it easier to miss.

Additionally, some useful properties are always available
and not only when target or targetsync has been specified,
i.e. from the technical/semantic side, there is no need
for requiring target and/or target sync.

But as the spec does so, it makes sense to modify GCC to require it
and doing so has advantages related to parsing and
diagnostic. (See below.)

[BTW: In GCC, for all currently supported foreign runtimes,
there is no overhead for making all 'target' properties
available; hence, 'target' has no effect on the runtime
side.]

* * *


Adding the check was fairly straightforward, but it broke about a
gazillion existing test cases.

Thanks for going though all of them and updating them!


Since one of the effects of the change is that at least one
modifier is always required, I found that deleting all the code that was
trying to detect and handle the no-modifier case allowed for better
diagnostics.


Yes, that makes both the code and the diagnostic better.

* * *

LGTM.

Thanks for the patch!

Tobias



Re: [RFC] [C]New syntax for the argument of counted_by attribute for C language

2025-04-02 Thread Bill Wendling
On Tue, Apr 1, 2025 at 8:29 AM Martin Uecker  wrote:
> Am Dienstag, dem 01.04.2025 um 15:01 + schrieb Qing Zhao:
> > > On Apr 1, 2025, at 10:04, Martin Uecker  wrote:
> > > Am Montag, dem 31.03.2025 um 13:59 -0700 schrieb Bill Wendling:
> > > > > I'd like to offer up this to solve the issues we're facing. This is a
> > > > > combination of everything that's been discussed here (or at least that
> > > > > I've been able to read in the centi-thread :-).
> > >
> > > Thanks! I think this proposal much better as it avoids undue burden
> > > on parsers, but it does not address all my concerns.
> > >
> > >
> > > From my side, the issue about compromising the scoping rules of C
> > > is also about unintended non-local effects of code changes. In
> > > my opinion, a change in a library header elsewhere should not cause
> > > code in a local scope (which itself might also come from a macro) to
> > > emit a warning or require a programmer to add a workaround. So I am
> > > not convinced that adding warnings or a workaround such as
> > > __builtin_global_ref  is a good solution.

To clarify, I'm not in favor of adding a generalized new scoping rule
to C, but only for identifiers in attributes. From what I've seen in
the discussions, I think that's what most people have been suggesting.
This should limit any issues with changes in current code.

> > > I could see the following as a possible way forward: We only
> > > allow the following two syntaxes:
> > >
> > > 1. Single argument referring to a member.
> > >
> > > __counted_by(len)
> > >
> > > with an argument that must be a single identifier and where
> > > the identifier then must refer to a struct member.
> > >
> > > (I still think this is not ideal and potentially
> > > confusing, but in contrast to new scoping rules it is
> > > at least relatively easily to explain as a special rule.).

I'm wavering on whether it's going to be too confusing. The initial
design was to use the bare struct member name, so that indicates to me
that it's maybe a more natural way of referring to the 'count' field
than not---at least with a single identifier. As Apple has stated,
they've had no confusion among their developers with this type of
identifier usage.

> > > 2. Forward declarations.
> > >
> > > __counted_by(size_t len; len + PADDING)
> >
> > In the above, the PADDING is some constant?
>
> In principle - when considering only the name lookup rules -
> it could be a constant, a global variable, or an automatic
> variable, i.e. any ordinary identifiers which is visible at
> this point.
>
> >
> > More complicated expressions involving globals will not be supported?
>
> I think one could allow such expressions, But I think the
> expressions should be restricted to expressions which have
> no side effects.
>
> >
> > > where then the second part can also be a more complicated
> > > expression, but with the explicit requirement that all
> > > identifiers in this expression are then looked up according to
> > > regular C language rules. So except for the forward declared
> > > member(s) they are *never* looked up in the member namespace of
> > > the struct, i.e. no new name lookup rules are introduced.

I'm on record as *hating* the idea of using expressions (apart from an
identifier) in attributes. Having the compiler silently add a "ptr->"
for every identifier in an expression seems like a hack to me. This is
the reason I added my proposal for a function pointer to handle
expressions. Even though I'm in the minority on this, I'd still like
the option to use function pointers.

> > One question here:
> >
> > What’s the major issue if we’d like to add one new scoping rule, for 
> > example,
> > “Structure scope” (the same as the C++’s instance scope) to C?
> >
> > (In addition to the "VLA in structure" issue I mentioned in my previous 
> > writeup,
> > is there any other issue to prevent this new scoping rule being added into 
> > C ?).
>
> Note that the "VLA in structure" is a bit of a red herring.  The exact same
> issues apply to lookup of any other ordinary identifiers in this context.
>
> enum { MY_BUF_SIZE = 100 };
> struct foo {
>   char buf[MY_BUF_SIZE];
> };
>
>
> C++ has instance scope for member functions. The rules for C++ are also
> complex and not very consistent (see the examples I posted earlier,
> demonstrating UB and compiler divergence).  For C such a concept would
> be new and much less useful, so the trade-off seems unfavorable (in
> constrast to C++ where it is needed).  I also see others issues:  Fully
> supporting instance scope would require changes to how C is parsed,
> placing a burden on all C compilers and tooling. Depending on how you
> specify it, it would also cause a change in semantics
> for existing code, something C tries very hard to avoid. If you add
> warnings as mitigation,  it has the problem that it causes non-local
> effects where introducing a name in in enclosing scope somewhere else
> now necessitates a change to unrelated code, exactl

RE: [ping][PATCH] libgcobol: Provide fallbacks for C32 strfromf32/64 [PR119296].

2025-04-02 Thread Robert Dubner
Since you say it tests okay, I say it's "Okay for trunk".

I am new to this, as you know, and I am still trying to figure out how to
make all of this work.  Nor have Jim and I figured out how to handle our
joint responsibility.

Hence delays.


> -Original Message-
> From: Iain Sandoe 
> Sent: Wednesday, April 2, 2025 14:24
> To: GCC Patches 
> Cc: James K. Lowden ; Robert Dubner
> 
> Subject: [ping][PATCH] libgcobol: Provide fallbacks for C32
strfromf32/64
> [PR119296].
> 
> Hi folks,
> it would be great to reduce the in-flight patch stack a bit :)
> 
> > On 25 Mar 2025, at 16:40, Iain Sandoe  wrote:
> >
> > This is on top of the C++-ify configure and random_r patches.
> > Tested on x86_64,aarch64-Linux and x86_64-darwin, OK for trunk?
> > thanks
> > Iain
> >
> > --- 8< ---
> >
> > strfrom{f,d,l,fN) are all C23 and might not be available in general.
> > This uses snprintf() to provide fall-backs where the libc does not
> > yet have support.
> >
> > PR cobol/119296
> >
> > libgcobol/ChangeLog:
> >
> > * config.h.in: Regenerate.
> > * configure: Regenerate.
> > * configure.ac: Check for availability of strfromf32 and
> > strfromf64.
> > * libgcobol.cc (strfromf32, strfromf64): New.
> >
> > Signed-off-by: Iain Sandoe 
> > ---
> > libgcobol/config.h.in  |  6 ++
> > libgcobol/configure| 13 +++--
> > libgcobol/configure.ac |  3 +++
> > libgcobol/libgcobol.cc | 24 
> > 4 files changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/libgcobol/config.h.in b/libgcobol/config.h.in
> > index e7e1492b579..d61ff7ad497 100644
> > --- a/libgcobol/config.h.in
> > +++ b/libgcobol/config.h.in
> > @@ -36,6 +36,12 @@
> > /* Define to 1 if you have the  header file. */
> > #undef HAVE_STDLIB_H
> >
> > +/* Define to 1 if you have the `strfromf32' function. */
> > +#undef HAVE_STRFROMF32
> > +
> > +/* Define to 1 if you have the `strfromf64' function. */
> > +#undef HAVE_STRFROMF64
> > +
> > /* Define to 1 if you have the  header file. */
> > #undef HAVE_STRINGS_H
> >
> > diff --git a/libgcobol/configure b/libgcobol/configure
> > index acf78646d5b..23474881f35 100755
> > --- a/libgcobol/configure
> > +++ b/libgcobol/configure
> > @@ -2696,6 +2696,8 @@ as_fn_append ac_func_list " random_r"
> > as_fn_append ac_func_list " srandom_r"
> > as_fn_append ac_func_list " initstate_r"
> > as_fn_append ac_func_list " setstate_r"
> > +as_fn_append ac_func_list " strfromf32"
> > +as_fn_append ac_func_list " strfromf64"
> > # Check that the precious variables saved in the cache have kept the
> same
> > # value.
> > ac_cache_corrupted=false
> > @@ -11621,7 +11623,7 @@ else
> >   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >   lt_status=$lt_dlunknown
> >   cat > conftest.$ac_ext <<_LT_EOF
> > -#line 11624 "configure"
> > +#line 11626 "configure"
> > #include "confdefs.h"
> >
> > #if HAVE_DLFCN_H
> > @@ -11727,7 +11729,7 @@ else
> >   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> >   lt_status=$lt_dlunknown
> >   cat > conftest.$ac_ext <<_LT_EOF
> > -#line 11730 "configure"
> > +#line 11732 "configure"
> > #include "confdefs.h"
> >
> > #if HAVE_DLFCN_H
> > @@ -17699,6 +17701,13 @@ done
> >
> >
> >
> > +# These are C23, and might not be available in libc.
> > +
> > +
> > +
> > +
> > +
> > +
> > if test "${multilib}" = "yes"; then
> >   multilib_arg="--enable-multilib"
> > else
> > diff --git a/libgcobol/configure.ac b/libgcobol/configure.ac
> > index 0356f9e9c67..6e1ea3700d0 100644
> > --- a/libgcobol/configure.ac
> > +++ b/libgcobol/configure.ac
> > @@ -198,6 +198,9 @@ AC_SUBST(extra_ldflags_libgcobol)
> > # These are GLIBC
> > AC_CHECK_FUNCS_ONCE(random_r srandom_r initstate_r setstate_r)
> >
> > +# These are C23, and might not be available in libc.
> > +AC_CHECK_FUNCS_ONCE(strfromf32 strfromf64)
> > +
> > if test "${multilib}" = "yes"; then
> >   multilib_arg="--enable-multilib"
> > else
> > diff --git a/libgcobol/libgcobol.cc b/libgcobol/libgcobol.cc
> > index 6aeaaa2c142..85f016e9735 100644
> > --- a/libgcobol/libgcobol.cc
> > +++ b/libgcobol/libgcobol.cc
> > @@ -68,6 +68,30 @@
> >
> > #include "exceptl.h"
> >
> > +#if !defined (HAVE_STRFROMF32)
> > +# if __FLT_MANT_DIG__ == 24 && __FLT_MAX_EXP__ == 128
> > +static int
> > +strfromf32 (char *s, size_t n, const char *f, float v)
> > +{
> > +  return snprintf (s, n, f, (double) v);
> > +}
> > +# else
> > +#  error "It looks like float on this platform is not IEEE754"
> > +# endif
> > +#endif
> > +
> > +#if !defined (HAVE_STRFROMF64)
> > +# if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024
> > +static int
> > +strfromf64 (char *s, size_t n, const char *f, double v)
> > +{
> > +  return snprintf (s, n, f, v);
> > +}
> > +# else
> > +#  error "It looks like double on this platform is not IEEE754"
> > +# endif
> > +#endif
> > +
> > // This couldn't be defined in symbols.h because it conflicts with a
> LEVEL66
> > // in parse.h
> > #define LEVEL66 (66)
> > --
> > 2.39.2 (Apple Git-143)
> >



Re: [PATCH] fold-const, cobol: Add native_encode_wide_int and use it in COBOL FE [PR119242]

2025-04-02 Thread Richard Sandiford
Jakub Jelinek  writes:
> Hi!
>
> As has been mentioned earlier, various parts of the COBOL FE and the library
> aren't endian clean.  In the library that means that for now we have to
> live with no support for big endian targets, but in the FE that means
> that as well as not being able to build cross-compilers from big endian
> or pdp endian hosts to little endian targets which are otherwise supported.
>
> The following patch attempts to fix one such spot, where it wants to encode
> in target byte ordering wide_int constants into 1, 2, 4, 8 or 16 bytes.
>
> We could wide_int_to_tree and then native_encode_expr, but so that we don't
> need to build the constants, the following patch exports from fold-const.cc
> a variant to native_encode_int which takes type and wide_int reference
> rather than an expression.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2025-04-02  Jakub Jelinek  
>
>   PR cobol/119242
>   * fold-const.h (native_encode_wide_int): Declare.
>   * fold-const.cc (native_encode_wide_int_1): New function template.
>   (native_encode_wide_int): New function.
>   (native_encode_int): Use native_encode_wide_int_1.
>   
>   * cobol/genapi.cc (binary_initial_from_float128): Use
>   native_encode_wide_int.
>
> --- gcc/fold-const.h.jj   2025-03-28 10:38:23.165538304 +0100
> +++ gcc/fold-const.h  2025-04-02 14:31:22.921136606 +0200
> @@ -35,6 +35,8 @@ extern bool folding_cxx_constexpr;
>  extern int native_encode_expr (const_tree, unsigned char *, int, int off = 
> -1);
>  extern int native_encode_initializer (tree, unsigned char *, int,
> int off = -1, unsigned char * = nullptr);
> +extern int native_encode_wide_int (tree, const wide_int_ref &,
> +unsigned char *, int, int off = -1);
>  extern int native_encode_real (scalar_float_mode, const REAL_VALUE_TYPE *,
>  unsigned char *, int, int off = -1);
>  extern tree native_interpret_expr (tree, const unsigned char *, int);
> --- gcc/fold-const.cc.jj  2025-03-28 10:38:23.148538537 +0100
> +++ gcc/fold-const.cc 2025-04-02 14:30:15.072074312 +0200
> @@ -7465,15 +7465,17 @@ fold_plusminus_mult_expr (location_t loc
>return NULL_TREE;
>  }
>  
> -/* Subroutine of native_encode_expr.  Encode the INTEGER_CST
> -   specified by EXPR into the buffer PTR of length LEN bytes.
> +
> +/* Subroutine of native_encode_int and native_encode_wide_int.  Encode the
> +   integer VAL with type TYPE into the buffer PTR of length LEN bytes.
> Return the number of bytes placed in the buffer, or zero
> upon failure.  */
>  
> +template 
>  static int
> -native_encode_int (const_tree expr, unsigned char *ptr, int len, int off)
> +native_encode_wide_int_1 (tree type, const T &val,
> +   unsigned char *ptr, int len, int off)
>  {
> -  tree type = TREE_TYPE (expr);
>int total_bytes;
>if (TREE_CODE (type) == BITINT_TYPE)
>  {
> @@ -7516,7 +7518,7 @@ native_encode_int (const_tree expr, unsi
>int bitpos = byte * BITS_PER_UNIT;
>/* Extend EXPR according to TYPE_SIGN if the precision isn't a whole
>number of bytes.  */
> -  value = wi::extract_uhwi (wi::to_widest (expr), bitpos, BITS_PER_UNIT);
> +  value = wi::extract_uhwi (val, bitpos, BITS_PER_UNIT);
>  
>if (total_bytes > UNITS_PER_WORD)
>   {
> @@ -7537,6 +7539,31 @@ native_encode_int (const_tree expr, unsi
>return MIN (len, total_bytes - off);
>  }

I'm not sure the _1 template is worth it.  wi::to_widest ->
const wide_int_ref & should be very cheap (it doesn't for example
construct a widest_int), so I think native_encode_int could call
native_encode_wide_int.

LGTM otherwise.

Thanks,
Richard

>  
> +/* Encode the integer VAL with type TYPE into the buffer PTR of length LEN
> +   bytes.
> +   Return the number of bytes placed in the buffer, or zero
> +   upon failure.  */
> +
> +int
> +native_encode_wide_int (tree type, const wide_int_ref &val,
> + unsigned char *ptr, int len, int off)
> +{
> +  return native_encode_wide_int_1 (type, val, ptr, len, off);
> +}
> +
> +
> +/* Subroutine of native_encode_expr.  Encode the INTEGER_CST
> +   specified by EXPR into the buffer PTR of length LEN bytes.
> +   Return the number of bytes placed in the buffer, or zero
> +   upon failure.  */
> +
> +static int
> +native_encode_int (const_tree expr, unsigned char *ptr, int len, int off)
> +{
> +  return native_encode_wide_int_1 (TREE_TYPE (expr), wi::to_widest (expr),
> +ptr, len, off);
> +}
> +
>  
>  /* Subroutine of native_encode_expr.  Encode the FIXED_CST
> specified by EXPR into the buffer PTR of length LEN bytes.
> --- gcc/cobol/genapi.cc.jj2025-04-02 10:36:25.517574640 +0200
> +++ gcc/cobol/genapi.cc   2025-04-02 14:31:36.373950683 +0200
> @@ -15216,25 +15216,19 @@ binary_initial_from_float128(cbl_field_t
>FIXED_WID