Re: [PATCH 3/3] xtensa: Remove include of reload.h
On Mon, May 26, 2025 at 11:59 PM Takayuki 'January June' Suwa wrote: > > As one of the last steps in removing old reload. > > gcc/ChangeLog: > > * gcc/config/xtensa/xtensa.cc > Remove include of reload.h. > --- > gcc/config/xtensa/xtensa.cc | 1 - > 1 file changed, 1 deletion(-) Regtested for target=xtensa-linux-uclibc, no new regressions. Committed the whole series to master with minor fixups in descriptions. -- Thanks. -- Max
[PATCH] forwprop: Manually rename the virtual mem op for complex and vector loads prop
There are two places which forwprop replaces an original load to a few
different loads.
Both can set the vuse manually instead of relying on update_ssa.
One is doing a complex load followed by REAL/IMAG_PART only
And the other is very similar but for vector loads followed by BIT_FIELD_REF.
Since this was the last place that needed to handle updating the ssa form,
Remove the TODO_update_ssa also from the pass.
gcc/ChangeLog:
* tree-ssa-forwprop.cc (optimize_vector_load): Set the vuse manually
on the new load statements. Also remove forward declaration since
the definition is before the first use.
(pass_forwprop::execute): Likewise for complex loads.
(pass_data_forwprop): Remove TODO_update_ssa.
Signed-off-by: Andrew Pinski
---
gcc/tree-ssa-forwprop.cc | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc
index 81ea7d4195e..75901ecfbb0 100644
--- a/gcc/tree-ssa-forwprop.cc
+++ b/gcc/tree-ssa-forwprop.cc
@@ -205,7 +205,6 @@ struct _vec_perm_simplify_seq
typedef struct _vec_perm_simplify_seq *vec_perm_simplify_seq;
static bool forward_propagate_addr_expr (tree, tree, bool);
-static void optimize_vector_load (gimple_stmt_iterator *);
/* Set to true if we delete dead edges during the optimization. */
static bool cfg_changed;
@@ -3387,6 +3386,7 @@ optimize_vector_load (gimple_stmt_iterator *gsi)
gimple *stmt = gsi_stmt (*gsi);
tree lhs = gimple_assign_lhs (stmt);
tree rhs = gimple_assign_rhs1 (stmt);
+ tree vuse = gimple_vuse (stmt);
/* Gather BIT_FIELD_REFs to rewrite, looking through
VEC_UNPACK_{LO,HI}_EXPR. */
@@ -3495,6 +3495,7 @@ optimize_vector_load (gimple_stmt_iterator *gsi)
gimple *new_stmt = gimple_build_assign (tem, new_rhs);
location_t loc = gimple_location (use_stmt);
gimple_set_location (new_stmt, loc);
+ gimple_set_vuse (new_stmt, vuse);
gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
/* Perform scalar promotion. */
new_stmt = gimple_build_assign (gimple_assign_lhs (use_stmt),
@@ -3514,6 +3515,7 @@ optimize_vector_load (gimple_stmt_iterator *gsi)
new_rhs);
location_t loc = gimple_location (use_stmt);
gimple_set_location (new_stmt, loc);
+ gimple_set_vuse (new_stmt, vuse);
gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
}
gimple_stmt_iterator gsi2 = gsi_for_stmt (use_stmt);
@@ -4167,7 +4169,7 @@ const pass_data pass_data_forwprop =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- TODO_update_ssa, /* todo_flags_finish */
+ 0, /* todo_flags_finish */
};
class pass_forwprop : public gimple_opt_pass
@@ -4404,6 +4406,7 @@ pass_forwprop::execute (function *fun)
component-wise loads. */
use_operand_p use_p;
imm_use_iterator iter;
+ tree vuse = gimple_vuse (stmt);
bool rewrite = true;
FOR_EACH_IMM_USE_FAST (use_p, iter, lhs)
{
@@ -4443,6 +4446,7 @@ pass_forwprop::execute (function *fun)
location_t loc = gimple_location (use_stmt);
gimple_set_location (new_stmt, loc);
+ gimple_set_vuse (new_stmt, vuse);
gimple_stmt_iterator gsi2 = gsi_for_stmt (use_stmt);
unlink_stmt_vdef (use_stmt);
gsi_remove (&gsi2, true);
--
2.43.0
[PATCH v3] c: fix ICE related to tagged types with attributes in diagnostics [PR120380]
I had accidentally swapped dg-warning and dg-error in the
test for the last version. I will commit this one if it passes
checks.
Martin
c: fix ICE related to tagged types with attributes in diagnostics [PR120380]
get_aka_type will create a new type for diagnostics, but for tagged types
attributes will then be ignored with a warning. This can lead to reentering
warning code which leads to an ICE. Fix this by ignoring the attributes
for tagged types.
PR c/120380
gcc/c/ChangeLog:
c-objc-common.cc (get_aka_type): Ignore attributes for tagged types.
gcc/testsuite/ChangeLog:
gcc.dg/pr120380.c: New test.
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 2016eaebf17..d574bc77128 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -216,6 +216,11 @@ get_aka_type (tree type)
return canonical ? canonical : type;
}
}
+ /* For tagged types ignore attributes because they will otherwise
+ be ignored later causing a warning inside diagnostics which leads
+ to an ICE. */
+ if (RECORD_OR_UNION_TYPE_P (type) || TREE_CODE (type) == ENUMERAL_TYPE)
+return build_qualified_type (result, TYPE_QUALS (type));
return build_type_attribute_qual_variant (result, TYPE_ATTRIBUTES (type),
TYPE_QUALS (type));
}
diff --git a/gcc/testsuite/gcc.dg/pr120380.c b/gcc/testsuite/gcc.dg/pr120380.c
new file mode 100644
index 000..9e3f616dbbb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr120380.c
@@ -0,0 +1,23 @@
+/* PR c/120380 */
+/* { dg-do compile } */
+
+struct pair_t {
+ char c;
+ int i;
+};
+typedef struct foo_ {
+ struct foo_ {/* { dg-error "nested
redefinition" } */
+struct foo_ { /* { dg-error "nested redefinition" } */
+ int value;
+}
+ }/* { dg-error "does not declare
anything" } */
+ /* { dg-error "no semicolon" "" {
target *-*-* } .-1 } */
+} __attribute__((packed)) foo; /* { dg-error "does not declare
anything" } */
+ /* { dg-error "no semicolon" "" {
target *-*-* } .-1 } */
+struct pair_t p = {0, 1};
+foo *addr = (foo *)&p.i;
+int main() {
+ addr->value = 0; /* { dg-error "has no member" } */
+ return 0;
+}
+
Re: [PATCH RFA (diagnostic)] c++: modules and #pragma diagnostic
On 5/29/25 2:57 PM, David Malcolm wrote:
On Thu, 2025-05-29 at 09:11 -0400, Jason Merrill wrote:
On 5/27/25 5:12 PM, Jason Merrill wrote:
On 5/27/25 4:47 PM, Jason Merrill wrote:
On 5/27/25 1:33 PM, David Malcolm wrote:
On Fri, 2025-05-23 at 16:58 -0400, Jason Merrill wrote:
On 4/14/25 9:57 AM, Jason Merrill wrote:
On 1/9/25 10:00 PM, Jason Merrill wrote:
Tested x86_64-pc-linux-gnu. Is the diagnostic.h change
OK for
trunk?
To respect the #pragma diagnostic lines in libstdc++
headers when
compiling
with module std, we need to represent them in the module.
I think it's reasonable to make module_state a friend of
diagnostic_option_classifier to allow direct access to
the data.
This
is a
different approach from how Jakub made PCH streaming
members of
diagnostic_option_classifier, but it seems to me that
modules
handling
belongs in module.cc.
Putting it in module.cc looks good to me, though perhaps it
should be
just a friend of diagnostic_option_classifier but not of
diagnostic_context? Could the functions take a
diagnostic_option_classifier rather than a diagnostic_context?
diagnostic_context is something of a "big blob" of a class.
The friend in diagnostic_context is to be able to name
m_option_classifier. We could instead make that member public?
Thoughts? The functions could take the _classifier, or even just the
m_classification_history, but that just moves the access problem into
their callers, who would still need some way to get there from the
diagnostic_context. Do you have another idea for that?
I'm trying to eventually make all of the member data of
diagnostic_context private, so how about keeping it private, but adding
a public accessor, something like:
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index cdd6f26ba2a..cda090d55ff 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -830,6 +830,12 @@ public:
m_abort_on_error = val;
}
+ diagnostic_option_classifier &
+ get_option_classifier ()
+ {
+return m_option_classifier;
+ }
+
private:
void error_recursion () ATTRIBUTE_NORETURN;
OK, how about this?
From f57505c7c6fa05a14e285c14a81021655a43ccbc Mon Sep 17 00:00:00 2001
From: Jason Merrill
Date: Wed, 20 Nov 2024 16:20:52 +0100
Subject: [PATCH] c++: modules and #pragma diagnostic
To: gcc-patches@gcc.gnu.org
To respect the #pragma diagnostic lines in libstdc++ headers when compiling
with module std, we need to represent them in the module.
I think it's reasonable to give serializers direct access to the underlying
data, as here with get_classification_history. This is a different approach
from how Jakub made PCH streaming members of diagnostic_option_classifier,
but it seems to me that modules handling belongs in module.cc.
gcc/ChangeLog:
* diagnostic.h (diagnostic_option_classifier): Friend
diagnostic_context.
(diagnostic_context::get_classification_history): New.
gcc/cp/ChangeLog:
* module.cc (module_state::write_diagnostic_classification): New.
(module_state::write_begin): Call it.
(module_state::read_diagnostic_classification): New.
(module_state::read_initial): Call it.
gcc/testsuite/ChangeLog:
* g++.dg/modules/warn-spec-3_a.C: New test.
* g++.dg/modules/warn-spec-3_b.C: New test.
---
gcc/diagnostic.h | 10 +++
gcc/cp/module.cc | 86 +++-
gcc/testsuite/g++.dg/modules/warn-spec-3_a.C | 20 +
gcc/testsuite/g++.dg/modules/warn-spec-3_b.C | 8 ++
4 files changed, 123 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/warn-spec-3_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/warn-spec-3_b.C
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index cdd6f26ba2a..b6cab0d52ed 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -307,6 +307,9 @@ private:
diagnostic. */
vec m_classification_history;
+ /* For diagnostic_context::get_classification_history, declared later. */
+ friend class diagnostic_context;
+
/* For pragma push/pop. */
vec m_push_list;
};
@@ -830,6 +833,13 @@ public:
m_abort_on_error = val;
}
+ /* Accessor for use in serialization, e.g. by C++ modules. */
+ auto &
+ get_classification_history ()
+ {
+return m_option_classifier.m_classification_history;
+ }
+
private:
void error_recursion () ATTRIBUTE_NORETURN;
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 2fa92b514d6..3f88baa7544 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3879,6 +3879,9 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
void write_macro_maps (elf_out *to, range_t &, unsigned *crc_ptr);
bool read_macro_maps (line_map_uint_t);
+ void write_diagnostic_classification (elf_out *, diagnostic_context *, unsigned *);
+ bool read_diagnostic_classification (diagnostic_context *);
+
private:
void write_define (bytes_out &, const cpp_macro *);
cpp_macro *read_define (bytes_in &, cpp_reader *) const;
@@ -18181,6 +18184,78 @@ module_state::wr
[PATCH] DCE: Only set TODO_update_ssa when cfg has changed
SSA form is kept up to date except for VOPs when CFG was removed so only set update ssa TODO if the CFG has changed. This is still sets the TODO in more cases than it is needed but it reduces enough to make difference. Bootstrappd and tested on x86_64-linux-gnu. gcc/ChangeLog: * tree-ssa-dce.cc (perform_tree_ssa_dce): Set TODO_update_ssa only when cfg has changed Signed-off-by: Andrew Pinski --- gcc/tree-ssa-dce.cc | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc index ba9cd6536ae..f5e67c4409a 100644 --- a/gcc/tree-ssa-dce.cc +++ b/gcc/tree-ssa-dce.cc @@ -2167,7 +2167,11 @@ perform_tree_ssa_dce (bool aggressive) free_numbers_of_iterations_estimates (cfun); if (in_loop_pipeline) scev_reset (); - todo |= TODO_update_ssa | TODO_cleanup_cfg; + todo |= TODO_cleanup_cfg; + /* If the cfg has been modified, then there might +have been virtual op that needs to be updated. */ + if (cfg_altered) + todo |= TODO_update_ssa; } return todo; } -- 2.43.0
[PATCH] Also check function symbol for function declaration
Since not all CALL instructions in RTL passes have a REG_CALL_DECL note,
update get_call_fndecl to also check function symbol for function
declaration so that it can be used on CALL instructions like
(call_insn 39 38 61 7 (set (reg:SI 0 ax)
(call (mem:QI (symbol_ref:DI ("foo") [flags 0x3]
) [0 foo S1 A8])
(const_int 0 [0]))) "pr92080-15.c":24:9 1480 {*call_value}
(expr_list:REG_DEAD (reg:SI 5 di)
(expr_list:REG_DEAD (reg:SI 4 si)
(expr_list:REG_EH_REGION (const_int 0 [0])
(nil
(expr_list:SI (use (reg:SI 5 di))
(expr_list:SI (use (reg:SI 4 si))
(nil
PR other/120494
* rtlanal.cc (get_call_fndecl): Also check function symbol to
get function declaration.
--
H.J.
From 6e20aad0c0b02c688f93ebc20b160f31c23adc82 Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Sat, 31 May 2025 08:42:21 +0800
Subject: [PATCH] Also check function symbol for function declaration
Since not all CALL instructions in RTL passes have a REG_CALL_DECL note,
update get_call_fndecl to also check function symbol for function
declaration so that it can be used on CALL instructions like
(call_insn 39 38 61 7 (set (reg:SI 0 ax)
(call (mem:QI (symbol_ref:DI ("foo") [flags 0x3] ) [0 foo S1 A8])
(const_int 0 [0]))) "pr92080-15.c":24:9 1480 {*call_value}
(expr_list:REG_DEAD (reg:SI 5 di)
(expr_list:REG_DEAD (reg:SI 4 si)
(expr_list:REG_EH_REGION (const_int 0 [0])
(nil
(expr_list:SI (use (reg:SI 5 di))
(expr_list:SI (use (reg:SI 4 si))
(nil
PR other/120494
* rtlanal.cc (get_call_fndecl): Also check function symbol to
get function declaration.
Signed-off-by: H.J. Lu
---
gcc/rtlanal.cc | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index 900f53e252a..b0983f8ebfc 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -833,17 +833,28 @@ get_call_rtx_from (const rtx_insn *insn)
tree
get_call_fndecl (const rtx_insn *insn)
{
- rtx note, datum;
-
+ rtx note;
note = find_reg_note (insn, REG_CALL_DECL, NULL_RTX);
- if (note == NULL_RTX)
-return NULL_TREE;
-
- datum = XEXP (note, 0);
- if (datum != NULL_RTX)
-return SYMBOL_REF_DECL (datum);
-
- return NULL_TREE;
+ if (note)
+{
+ rtx datum = XEXP (note, 0);
+ if (datum)
+ return SYMBOL_REF_DECL (datum);
+}
+ rtx call = get_call_rtx_from (insn);
+ rtx addr = XEXP (call, 0);
+ tree fndecl = nullptr;
+ if (MEM_P (addr))
+{
+ addr = XEXP (addr, 0);
+ if (GET_CODE (addr) == SYMBOL_REF)
+ {
+ tree decl = SYMBOL_REF_DECL (addr);
+ if (decl && TREE_CODE (decl) == FUNCTION_DECL)
+ fndecl = decl;
+ }
+}
+ return fndecl;
}
/* Return the value of the integer term in X, if one is apparent;
--
2.49.0
Re: [PATCH] Have TODO_verify_* not set by any pass
On Sat, May 31, 2025 at 4:17 AM Andrew Pinski wrote:
>
> This is a follow up to the patch set starting at
> https://gcc.gnu.org/pipermail/gcc-patches/2014-April/386650.html.
>
> Currently TODO_verify_{il,all} is set by a few passes as TODOs afterwards but
> we don't need to do that any more. Those were mostly removed back in
> https://gcc.gnu.org/pipermail/gcc-patches/2014-May/387647.html
>
> And now since TODO_verify_all is no longer used, remove it too.
> Removes last_verified from the function structure since it is always 0 before
> the
> call to execute_todo that has TODO_verify_all set on it.
>
> Also this removes the ability for good to use TODO_verify_il from the passes
> and reserves
> bit 31 of the TODO for the verfification and internally only the passes.cc
> code.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> gcc/ChangeLog:
>
> * function.h (struct function): Remove last_verified.
> * gimple-harden-conditionals.cc (pass_data_harden_compares): Remove
> TODO_verify_il.
> (pass_data_harden_conditional_branches): Likewise.
> * gimple-harden-control-flow.cc
> (pass_harden_control_flow_redundancy::execute):
> Don't return TODO_verify_il.
> * ipa-strub.cc (pass_data_ipa_strub): Remove TODO_verify_il.
> * passes.cc (TODO_verify_il): Define.
> (execute_function_todo): Don't use or set last_verified.
> (clear_last_verified): Remove.
> (execute_one_ipa_transform_pass): Update comment before execute_todo.
> Assert that none of the todos have TODO_verify_il set on it.
> (execute_one_pass): Don't call clear_last_verified on all functions.
> Assert that none of the todos have TODO_verify_il set on it.
> * tree-inline.cc (initialize_cfun): Don't copy last_verified.
> * tree-pass.h (TODO_verify_all): Remove.
> * tree-vrp.cc (pass_data_early_vrp): Remove TODO_verify_all.
> (pass_data_fast_vrp): Likewise.
>
> Signed-off-by: Andrew Pinski
> ---
> gcc/function.h| 1 -
> gcc/gimple-harden-conditionals.cc | 6 ++
> gcc/gimple-harden-control-flow.cc | 3 +--
> gcc/ipa-strub.cc | 3 +--
> gcc/passes.cc | 23 +--
> gcc/tree-inline.cc| 1 -
> gcc/tree-pass.h | 4 +---
> gcc/tree-vrp.cc | 4 ++--
> 8 files changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/gcc/function.h b/gcc/function.h
> index 2260d6704ec..370629f4de2 100644
> --- a/gcc/function.h
> +++ b/gcc/function.h
> @@ -333,7 +333,6 @@ struct GTY(()) function {
>
>/* Properties used by the pass manager. */
>unsigned int curr_properties;
> - unsigned int last_verified;
>
>/* Different from normal TODO_flags which are handled right at the
> beginning or the end of one pass execution, the pending_TODOs
> diff --git a/gcc/gimple-harden-conditionals.cc
> b/gcc/gimple-harden-conditionals.cc
> index 3ad3bb98edd..d4a418d481a 100644
> --- a/gcc/gimple-harden-conditionals.cc
> +++ b/gcc/gimple-harden-conditionals.cc
> @@ -61,8 +61,7 @@ const pass_data pass_data_harden_compares = {
>0, // properties_destroyed
>0, // properties_start
>TODO_update_ssa
> - | TODO_cleanup_cfg
> - | TODO_verify_il, // properties_finish
> + | TODO_cleanup_cfg, // properties_finish
> };
>
> class pass_harden_compares : public gimple_opt_pass
> @@ -96,8 +95,7 @@ const pass_data pass_data_harden_conditional_branches = {
>0, // properties_destroyed
>0, // properties_start
>TODO_update_ssa
> - | TODO_cleanup_cfg
> - | TODO_verify_il, // properties_finish
> + | TODO_cleanup_cfg, // properties_finish
> };
>
> class pass_harden_conditional_branches : public gimple_opt_pass
> diff --git a/gcc/gimple-harden-control-flow.cc
> b/gcc/gimple-harden-control-flow.cc
> index e46acafbae4..f129ff1e077 100644
> --- a/gcc/gimple-harden-control-flow.cc
> +++ b/gcc/gimple-harden-control-flow.cc
> @@ -1549,8 +1549,7 @@ pass_harden_control_flow_redundancy::execute (function
> *fun)
>
>return
> TODO_update_ssa
> -| TODO_cleanup_cfg
> -| TODO_verify_il;
> +| TODO_cleanup_cfg;
> }
>
> /* Instantiate a hardcfr pass. */
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 99cc9d930d8..d7c0a928600 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -1770,8 +1770,7 @@ const pass_data pass_data_ipa_strub = {
>0, // properties_start
>TODO_update_ssa
>| TODO_cleanup_cfg
> - | TODO_rebuild_cgraph_edges
> - | TODO_verify_il, // properties_finish
> + | TODO_rebuild_cgraph_edges, // properties_finish
> };
>
> class pass_ipa_strub : public simple_ipa_opt_pass
> diff --git a/gcc/passes.cc b/gcc/passes.cc
> index e86aa1f2d1b..6c67ffe56ba 100644
> --- a/gcc/passes.cc
> +++ b/gcc/passes.cc
> @@ -64,6 +64,9 @@ along with GCC; see the file COPYING3. If not see
> #include "stringpool.h"
> #include
Re: [PATCH] CCP: Manually rename the virtual mem op when inserting clobbers
On Sat, May 31, 2025 at 11:07 AM Andrew Pinski wrote:
>
> Right now the only place where CCP needs to have the virtual op renamed is
> after inserting clobbers which come right before __builtin_stack_restore.
> So let's manually do the correct thing so we can remove the TODO_update_ssa
> todo.
OK.
Thanks,
Richard.
> gcc/ChangeLog:
>
> * tree-ssa-ccp.cc (insert_clobber_before_stack_restore): Update the
> virtual
> op on the inserted clobber and the stack restore function.
> (do_ssa_ccp): Don't add TODO_update_ssa to the todo.
>
> Signed-off-by: Andrew Pinski
> ---
> gcc/tree-ssa-ccp.cc | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
> index 8d2cbb384c4..6022ea446a4 100644
> --- a/gcc/tree-ssa-ccp.cc
> +++ b/gcc/tree-ssa-ccp.cc
> @@ -2567,7 +2567,12 @@ insert_clobber_before_stack_restore (tree saved_val,
> tree var,
>{
> clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
> clobber_stmt = gimple_build_assign (var, clobber);
> -
> + /* Manually update the vdef/vuse here. */
> + gimple_set_vuse (clobber_stmt, gimple_vuse (stmt));
> + gimple_set_vdef (clobber_stmt, make_ssa_name (gimple_vop (cfun)));
> + gimple_set_vuse (stmt, gimple_vdef (clobber_stmt));
> + SSA_NAME_DEF_STMT (gimple_vdef (clobber_stmt)) = clobber_stmt;
> + update_stmt (stmt);
> i = gsi_for_stmt (stmt);
> gsi_insert_before (&i, clobber_stmt, GSI_SAME_STMT);
>}
> @@ -3020,7 +3025,7 @@ do_ssa_ccp (bool nonzero_p)
>ccp_propagate.ssa_propagate ();
>if (ccp_finalize (nonzero_p || flag_ipa_bit_cp))
> {
> - todo = (TODO_cleanup_cfg | TODO_update_ssa);
> + todo = TODO_cleanup_cfg;
>
>/* ccp_finalize does not preserve loop-closed ssa. */
>loops_state_clear (LOOP_CLOSED_SSA);
> --
> 2.43.0
>
Re: [PATCH] DCE: Only set TODO_update_ssa when cfg has changed
> Am 31.05.2025 um 18:29 schrieb Andrew Pinski : > > SSA form is kept up to date except for VOPs when CFG was removed > so only set update ssa TODO if the CFG has changed. This is still > sets the TODO in more cases than it is needed but it reduces enough > to make difference. > > Bootstrappd and tested on x86_64-linux-gnu. > > gcc/ChangeLog: > >* tree-ssa-dce.cc (perform_tree_ssa_dce): Set TODO_update_ssa >only when cfg has changed > > Signed-off-by: Andrew Pinski > --- > gcc/tree-ssa-dce.cc | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc > index ba9cd6536ae..f5e67c4409a 100644 > --- a/gcc/tree-ssa-dce.cc > +++ b/gcc/tree-ssa-dce.cc > @@ -2167,7 +2167,11 @@ perform_tree_ssa_dce (bool aggressive) > free_numbers_of_iterations_estimates (cfun); > if (in_loop_pipeline) >scev_reset (); > - todo |= TODO_update_ssa | TODO_cleanup_cfg; > + todo |= TODO_cleanup_cfg; > + /* If the cfg has been modified, then there might > + have been virtual op that needs to be updated. */ > + if (cfg_altered) > +todo |= TODO_update_ssa; I believe this is due to LC SSA, so only necessary when in loop pipeline. TODO_update_SSA is free when there’s nothing to do, so I can’t see how it makes a difference? Does it because LC SSA rewrite isn’t cheap? > } > return todo; > } > -- > 2.43.0 >
[PATCH, libgfortran] PR119856 Part 2 Fix error handling for missing commas in format strings
Hi all,
The attached patch fixes a latent issue where we were saving a parsed
and checked format string that had a missing comma. This resulted in the
correct error on the first use of the string, but a missed error on
subsequent uses of the string.
New test case provided.
Regression tested on x86_64
OK for trunk and eventual backport to 15?
Regards,
Jerrycommit 078912fe348838513a9eabd9dfca3e76ff3475df
Author: Jerry DeLisle
Date: Sat May 31 08:57:22 2025 -0700
Fortran: Fix handling of parsed format strings.
Previously parsed strings with errors were being cached such
that subsequent use of the format string were not being
checked for errors.
PR libfortran/119856
libgfortran/ChangeLog:
* io/format.c (parse_format_list): Set the fmt->error
message for missing comma.
(parse_format): Do not cache the parsed format string
if a previous error ocurred.
gcc/testsuite/ChangeLog:
* gfortran.dg/pr119856.f90: New test.
diff --git a/gcc/testsuite/gfortran.dg/pr119856.f90 b/gcc/testsuite/gfortran.dg/pr119856.f90
new file mode 100644
index 000..60ada0a910e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr119856.f90
@@ -0,0 +1,15 @@
+! { dg-do run }
+! PR119856, the error should occur in both write statements.
+program badfmt
+ implicit none
+
+ character(10):: fmt = "(AI5)" ! Not a PARAMETER so not examined
+ ! at compile time
+ integer :: ioerr
+ ioerr = 0
+ write (*, fmt, iostat=ioerr) 'value =', 42
+ if (ioerr /= 5006) stop 10
+!
+ write (*, fmt, iostat=ioerr) 'value =', 43
+ if (ioerr /= 5006) stop 13
+end program badfmt
diff --git a/libgfortran/io/format.c b/libgfortran/io/format.c
index eef1d34853e..87e21a9a432 100644
--- a/libgfortran/io/format.c
+++ b/libgfortran/io/format.c
@@ -1235,9 +1235,9 @@ parse_format_list (st_parameter_dt *dtp, bool *seen_dd)
default:
/* Assume a missing comma with -std=legacy, GNU extension. */
- if (compile_options.warn_std == 0)
- goto format_item_1;
- format_error (dtp, tail, comma_missing);
+ if (compile_options.warn_std != 0)
+ fmt->error = comma_missing;
+ goto format_item_1;
}
/* Optional comma is a weird between state where we've just finished
@@ -1252,7 +1252,7 @@ parse_format_list (st_parameter_dt *dtp, bool *seen_dd)
case FMT_RPAREN:
goto finished;
-default: /* Assume that we have another format item */
+default: /* Assume that we have another format item */
fmt->saved_token = t;
break;
}
@@ -1419,7 +1419,7 @@ parse_format (st_parameter_dt *dtp)
else
fmt->error = "Missing initial left parenthesis in format";
- if (format_cache_ok)
+ if (format_cache_ok && !fmt->error)
save_parsed_format (dtp);
else
dtp->u.p.format_not_saved = 1;
[PATCH] CCP: Manually rename the virtual mem op when inserting clobbers
Right now the only place where CCP needs to have the virtual op renamed is
after inserting clobbers which come right before __builtin_stack_restore.
So let's manually do the correct thing so we can remove the TODO_update_ssa
todo.
gcc/ChangeLog:
* tree-ssa-ccp.cc (insert_clobber_before_stack_restore): Update the
virtual
op on the inserted clobber and the stack restore function.
(do_ssa_ccp): Don't add TODO_update_ssa to the todo.
Signed-off-by: Andrew Pinski
---
gcc/tree-ssa-ccp.cc | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index 8d2cbb384c4..6022ea446a4 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2567,7 +2567,12 @@ insert_clobber_before_stack_restore (tree saved_val,
tree var,
{
clobber = build_clobber (TREE_TYPE (var), CLOBBER_STORAGE_END);
clobber_stmt = gimple_build_assign (var, clobber);
-
+ /* Manually update the vdef/vuse here. */
+ gimple_set_vuse (clobber_stmt, gimple_vuse (stmt));
+ gimple_set_vdef (clobber_stmt, make_ssa_name (gimple_vop (cfun)));
+ gimple_set_vuse (stmt, gimple_vdef (clobber_stmt));
+ SSA_NAME_DEF_STMT (gimple_vdef (clobber_stmt)) = clobber_stmt;
+ update_stmt (stmt);
i = gsi_for_stmt (stmt);
gsi_insert_before (&i, clobber_stmt, GSI_SAME_STMT);
}
@@ -3020,7 +3025,7 @@ do_ssa_ccp (bool nonzero_p)
ccp_propagate.ssa_propagate ();
if (ccp_finalize (nonzero_p || flag_ipa_bit_cp))
{
- todo = (TODO_cleanup_cfg | TODO_update_ssa);
+ todo = TODO_cleanup_cfg;
/* ccp_finalize does not preserve loop-closed ssa. */
loops_state_clear (LOOP_CLOSED_SSA);
--
2.43.0
Re: [PATCH v1] RISC-V: Fix line too long format issue for autovect.md [NFC]
Inspired by the avg_ceil patches, notice there were even more lines too long from autovec.md. So fix that format issues. OK. -- Regards Robin
[PATCH] aarch64:sve: Use create_tmp_reg_or_ssa_name instead of create_tmp_var in the folder
Currently gimple_folder::convert_and_fold calls create_tmp_var; that means while in ssa form, the pass which calls fold_stmt will always have to update the ssa (via TODO_update_ssa or otherwise). This seems not very useful since we know that this will always be a ssa name, using create_tmp_reg_or_ssa_name instead is better and don't need to depend on the ssa updater. Plus this should have a small compile time performance and memory usage improvement too since this uses an anonymous ssa name rather than creating a full decl for this. Built and tested on aarch64-linux-gnu. gcc/ChangeLog: * config/aarch64/aarch64-sve-builtins.cc (gimple_folder::convert_and_fold): Use create_tmp_reg_or_ssa_name instead of create_tmp_var for the temporary. Signed-off-by: Andrew Pinski --- gcc/config/aarch64/aarch64-sve-builtins.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc index 36519262efd..329237b086d 100644 --- a/gcc/config/aarch64/aarch64-sve-builtins.cc +++ b/gcc/config/aarch64/aarch64-sve-builtins.cc @@ -3675,7 +3675,7 @@ gimple_folder::convert_and_fold (tree type, tree old_ty = TREE_TYPE (lhs); gimple_seq stmts = NULL; bool convert_lhs_p = !useless_type_conversion_p (type, old_ty); - tree lhs_conv = convert_lhs_p ? create_tmp_var (type) : lhs; + tree lhs_conv = convert_lhs_p ? create_tmp_reg_or_ssa_name (type) : lhs; unsigned int num_args = gimple_call_num_args (call); auto_vec args_conv; args_conv.safe_grow (num_args); -- 2.43.0
[PATCH] c++, coroutines: Some cleanups in build_actor_fn.
Some small cleanups found while working on other changes, tested
on x86_64-darwin, OK for trunk?
thanks
Iain
--- 8< ---
We were incorrectly guarding all the frame cleanups on the
basis of frame_needs_free (which is always set for the present
code-gen since we have no allocation elision). The net result
being that the (incorrect) code was behaving as expected.
We built, but never used, a label for the frame destruction;
in practice it is never triggered independently of the promise
and argument copy destruction.
Finally there are a few instances where we had been building
expressions manually rather than using higher-level APIs.
gcc/cp/ChangeLog:
* coroutines.cc (build_actor_fn): Remove an unused
label, guard the frame deallocation correctly, use
simpler APIs to build if and return statements.
Signed-off-by: Iain Sandoe
---
gcc/cp/coroutines.cc | 58
1 file changed, 21 insertions(+), 37 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index aa00a8a4e68..a2b6d53805a 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2557,8 +2557,8 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* Finish the resume dispatcher. */
finish_switch_stmt (dispatcher);
- finish_else_clause (lsb_if);
+ finish_else_clause (lsb_if);
finish_if_stmt (lsb_if);
/* If we reach here then we've hit UB. */
@@ -2597,69 +2597,53 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* Add in our function body with the co_returns rewritten to final form. */
add_stmt (fnbody);
- /* now do the tail of the function. */
+ /* Now do the tail of the function; first cleanups. */
r = build_stmt (loc, LABEL_EXPR, del_promise_label);
add_stmt (r);
- /* Destructors for the things we built explicitly. */
+ /* Destructors for the things we built explicitly.
+ promise... */
if (tree c = cxx_maybe_build_cleanup (promise_proxy, tf_warning_or_error))
-add_stmt (c);
-
- tree del_frame_label
-= create_named_label_with_ctx (loc, "coro.delete.frame", actor);
- r = build_stmt (loc, LABEL_EXPR, del_frame_label);
- add_stmt (r);
-
- /* Here deallocate the frame (if we allocated it), which we will have at
- present. */
- tree fnf2_x
- = coro_build_frame_access_expr (actor_frame, coro_frame_needs_free_id,
- false, tf_warning_or_error);
+finish_expr_stmt (c);
- tree need_free_if = begin_if_stmt ();
- fnf2_x = build1 (CONVERT_EXPR, integer_type_node, fnf2_x);
- tree cmp = build2 (NE_EXPR, integer_type_node, fnf2_x, integer_zero_node);
- finish_if_stmt_cond (cmp, need_free_if);
+ /* Argument copies ... */
while (!param_dtor_list->is_empty ())
{
tree parm_id = param_dtor_list->pop ();
tree a = coro_build_frame_access_expr (actor_frame, parm_id, false,
tf_warning_or_error);
if (tree dtor = cxx_maybe_build_cleanup (a, tf_warning_or_error))
- add_stmt (dtor);
+ finish_expr_stmt (dtor);
}
+ /* Here deallocate the frame (if we allocated it), which we will have at
+ present. */
+ tree fnf2_x
+ = coro_build_frame_access_expr (actor_frame, coro_frame_needs_free_id,
+ false, tf_warning_or_error);
+ tree need_free_if = begin_if_stmt ();
+ finish_if_stmt_cond (fnf2_x, need_free_if);
+
/* Build the frame DTOR. */
tree del_coro_fr
= build_coroutine_frame_delete_expr (actor_fp, frame_size,
promise_type, loc);
finish_expr_stmt (del_coro_fr);
finish_then_clause (need_free_if);
- tree scope = IF_SCOPE (need_free_if);
- IF_SCOPE (need_free_if) = NULL;
- r = do_poplevel (scope);
- add_stmt (r);
+ finish_if_stmt (need_free_if);
- /* done. */
- r = build_stmt (loc, RETURN_EXPR, NULL);
- suppress_warning (r); /* We don't want a warning about this. */
- r = maybe_cleanup_point_expr_void (r);
- add_stmt (r);
+ /* Done. */
+ finish_return_stmt (NULL_TREE);
/* This is the suspend return point. */
- r = build_stmt (loc, LABEL_EXPR, ret_label);
- add_stmt (r);
+ add_stmt (build_stmt (loc, LABEL_EXPR, ret_label));
- r = build_stmt (loc, RETURN_EXPR, NULL);
- suppress_warning (r); /* We don't want a warning about this. */
- r = maybe_cleanup_point_expr_void (r);
- add_stmt (r);
+ finish_return_stmt (NULL_TREE);
/* This is the 'continuation' return point. For such a case we have a coro
handle (from the await_suspend() call) and we want handle.resume() to
execute as a tailcall allowing arbitrary chaining of coroutines. */
- r = build_stmt (loc, LABEL_EXPR, continue_label);
- add_stmt (r);
+ add_stmt (build_stmt (loc, LABEL_EXPR, continue_label));
/* Should have been set earlier by the coro_initialized code. */
gcc_assert (void_coro_handle_address);
--
2.39.2
[PATCH] c++, coroutines: CWG2563 promise lifetime extension [PR115908].
Tested on x86_64-darwin, confirmed that the sanitizer test in
PR 118074 is fixed but we already have a suitable testcase in
PR 115908 which has now been moved to the 'torture' sub-directory
to get wider code-gen coverage.
OK for trunk?
thanks
Iain
--- 8< ---
This implements the final piece of the revised CWG2563 wording;
"It exits the scope of promise only if the coroutine completed
without suspending."
The change removes the need to track whether we have reached the ramp
return. We can now assume that the coroutine frame is valid when
constructing the ramp return value.
The cleanups for promise, arg copies and frame are now adjusted to
depend only on whether a suspend has occurred.
PR c++/115908
PR c++/118074
gcc/cp/ChangeLog:
* coroutines.cc: New GTY coro_ramp_cleanup_id.
(coro_init_identifiers): Initialize coro_ramp_cleanup_id.
(build_actor_fn): Set coro_ramp_cleanup_id false when we
exit via a suspend or continuation return. Do not clean
up if we pass the final await expression without suspending.
(cp_coroutine_transform::wrap_original_function_body):
Declare _Coro_ramp_cleanup.
(cp_coroutine_transform::build_ramp_function): Amend
cleanups to use coro_ramp_cleanup and set that to be true
on init. Remove the use of _Coro_befoe_return.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr115908.C: Move to...
* g++.dg/coroutines/torture/pr115908.C: ...here.
Signed-off-by: Iain Sandoe
---
gcc/cp/coroutines.cc | 111 +-
.../coroutines/{ => torture}/pr115908.C | 9 +-
2 files changed, 56 insertions(+), 64 deletions(-)
rename gcc/testsuite/g++.dg/coroutines/{ => torture}/pr115908.C (88%)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index a2b6d53805a..f40f42e2d35 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -348,6 +348,7 @@ static GTY(()) tree coro_resume_index_id;
static GTY(()) tree coro_self_handle_id;
static GTY(()) tree coro_actor_continue_id;
static GTY(()) tree coro_frame_i_a_r_c_id;
+static GTY(()) tree coro_ramp_cleanup_id;
/* Create the identifiers used by the coroutines library interfaces and
the implementation frame state. */
@@ -385,6 +386,7 @@ coro_init_identifiers ()
coro_resume_index_id = get_identifier ("_Coro_resume_index");
coro_self_handle_id = get_identifier ("_Coro_self_handle");
coro_actor_continue_id = get_identifier ("_Coro_actor_continue");
+ coro_ramp_cleanup_id = get_identifier ("_Coro_ramp_should_cleanup");
}
/* Trees we only need to set up once. */
@@ -2597,6 +2599,16 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* Add in our function body with the co_returns rewritten to final form. */
add_stmt (fnbody);
+ /* If we reach here without suspending, then the ramp should cleanup. */
+ tree ramp_should_cleanup
+= coro_build_frame_access_expr (actor_frame, coro_ramp_cleanup_id,
+ false, tf_warning_or_error);
+ tree ramp_cu_if = begin_if_stmt ();
+ finish_if_stmt_cond (ramp_should_cleanup, ramp_cu_if);
+ finish_return_stmt (NULL_TREE);
+ finish_then_clause (ramp_cu_if);
+ finish_if_stmt (ramp_cu_if);
+
/* Now do the tail of the function; first cleanups. */
r = build_stmt (loc, LABEL_EXPR, del_promise_label);
add_stmt (r);
@@ -2638,6 +2650,9 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
/* This is the suspend return point. */
add_stmt (build_stmt (loc, LABEL_EXPR, ret_label));
+ r = cp_build_modify_expr (loc, ramp_should_cleanup, NOP_EXPR,
+ boolean_false_node, tf_warning_or_error);
+ finish_expr_stmt (r);
finish_return_stmt (NULL_TREE);
/* This is the 'continuation' return point. For such a case we have a coro
@@ -2645,6 +2660,10 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
execute as a tailcall allowing arbitrary chaining of coroutines. */
add_stmt (build_stmt (loc, LABEL_EXPR, continue_label));
+ r = cp_build_modify_expr (loc, ramp_should_cleanup, NOP_EXPR,
+ boolean_false_node, tf_warning_or_error);
+ finish_expr_stmt (r);
+
/* Should have been set earlier by the coro_initialized code. */
gcc_assert (void_coro_handle_address);
@@ -4434,6 +4453,13 @@ cp_coroutine_transform::wrap_original_function_body ()
var_list = var;
add_decl_expr (var);
+ tree ramp_does_cleanup
+= coro_build_artificial_var (loc, coro_ramp_cleanup_id, boolean_type_node,
+orig_fn_decl, NULL_TREE);
+ DECL_CHAIN (ramp_does_cleanup) = var_list;
+ var_list = ramp_does_cleanup;
+ add_decl_expr (ramp_does_cleanup);
+
if (flag_exceptions)
{
/* Build promise.unhandled_exception(); */
@@ -4898,31 +4924,25 @@ cp_coroutine_transform::build_ramp_function ()
just set
Re: [PATCH] RISC-V: Support CPUs in -march.
On 2025-05-21, Kito Cheng wrote: On Wed, May 21, 2025 at 10:19 PM Robin Dapp wrote: > I could imagine that is a simpler way to set the march since the march > string becomes terribly long - we have an arch string more than 300 > char...so I support this, although I think this should be discuss with > LLVM community, but I think it's fine to accept as a GCC extension. > > So LGTM, go ahead to the trunk, and I will raise this topic in the > next LLVM sync up meeting. That would be great, thanks. Another related issue. i386 and other targets, but not aarch64, make -march imply -mtune. Was it a deliberate decision to _not_ do this? Could we even add it still? ;) That might help simplify things a bit further even. That sounds make sense to me, but let me think about it a few more (let me sleep first and think more tomorrow :P) also discuss with a few more people, let's step by step...I don't want to change too much at one time without sync with the LLVM community :) -- Regards Robin I stumped across this change from https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/88 and I want to express my strong disagreement with this change. Perhaps I'm accustomed to Arm's behavior, but I believe using -march= to target a specific CPU isn't ideal. * -march=X: (execution domain) Generate code that can use instructions available in the architecture X * -mtune=X: (optimization domain) Optimize for the microarchitecture X, but does not change the ABI or make assumptions about available instructions * -mcpu=X: Specify both -march= and -mtune= but can be overridden by the two options. The supported values are generally the same as -mtune=. The architecture name is inferred from X For execution domain settings, -march=X overrides -mcpu=X regardless of their positions. In cases like `-march=LOWER -mcpu=HIGHER` or `-mcpu=HIGHER -march=LOWER`, the -march= option can disable certain target features. I strongly disagree with Clang adopting this behavior. I'm not convinced by the GCC patch explanation. Suppose we have a Makefile that specifies -march=rv64gc by default. In the project specifies a lower feature set, then the compiler should respect it or the user should fix the project build system.
