[PATCH v2 2/2] c++/modules: Fix instantiation of imported temploid friends [PR114275]
I'm not a huge fan of always streaming 'imported_temploid_friends' for all decls, but I don't think it adds much performance cost over adding a new flag to categorise decls that might be marked as such. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This patch fixes a number of issues with the handling of temploid friend declarations. The primary issue is that instantiations of friend declarations should attach the declaration to the same module as the befriending class, by [module.unit] p7.1 and [temp.friend] p2; this could be a different module from the current TU, and so needs special handling. The other main issue here is that we can't assume that just because name lookup didn't find a definition for a hidden template class, it doesn't mean that it doesn't exist: it could be a non-exported entity that we've nevertheless streamed in from an imported module. We need to ensure that when instantiating friend classes that we return the same TYPE_DECL that we got from our imports, otherwise we will get later issues with 'duplicate_decls' (rightfully) complaining that they're different. This doesn't appear necessary for functions due to the existing name lookup handling already finding these hidden declarations. PR c++/105320 PR c++/114275 gcc/cp/ChangeLog: * cp-tree.h (propagate_defining_module): Declare. (lookup_imported_hidden_friend): Declare. * decl.cc (duplicate_decls): Also check if hidden declarations can be redeclared in this module. * module.cc (imported_temploid_friends): New map. (init_modules): Initialize it. (trees_out::decl_value): Write it. (trees_in::decl_value): Read it. (get_originating_module_decl): Follow the owning decl for an imported temploid friend. (propagate_defining_module): New function. * name-lookup.cc (lookup_imported_hidden_friend): New function. * pt.cc (tsubst_friend_function): Propagate defining module for new friend functions. (tsubst_friend_class): Lookup imported hidden friends. Check for valid redeclaration. Propagate defining module for new friend classes. gcc/testsuite/ChangeLog: * g++.dg/modules/tpl-friend-10_a.C: New test. * g++.dg/modules/tpl-friend-10_b.C: New test. * g++.dg/modules/tpl-friend-10_c.C: New test. * g++.dg/modules/tpl-friend-11_a.C: New test. * g++.dg/modules/tpl-friend-11_b.C: New test. * g++.dg/modules/tpl-friend-12_a.C: New test. * g++.dg/modules/tpl-friend-12_b.C: New test. * g++.dg/modules/tpl-friend-12_c.C: New test. * g++.dg/modules/tpl-friend-12_d.C: New test. * g++.dg/modules/tpl-friend-12_e.C: New test. * g++.dg/modules/tpl-friend-12_f.C: New test. * g++.dg/modules/tpl-friend-13_a.C: New test. * g++.dg/modules/tpl-friend-13_b.C: New test. * g++.dg/modules/tpl-friend-13_c.C: New test. * g++.dg/modules/tpl-friend-13_d.C: New test. * g++.dg/modules/tpl-friend-13_e.C: New test. * g++.dg/modules/tpl-friend-9.C: New test. Signed-off-by: Nathaniel Shead --- gcc/cp/cp-tree.h | 2 + gcc/cp/decl.cc| 36 +++-- gcc/cp/module.cc | 52 +++ gcc/cp/name-lookup.cc | 42 +++ gcc/cp/pt.cc | 19 +++ .../g++.dg/modules/tpl-friend-10_a.C | 15 ++ .../g++.dg/modules/tpl-friend-10_b.C | 5 ++ .../g++.dg/modules/tpl-friend-10_c.C | 7 +++ .../g++.dg/modules/tpl-friend-11_a.C | 14 + .../g++.dg/modules/tpl-friend-11_b.C | 5 ++ .../g++.dg/modules/tpl-friend-12_a.C | 10 .../g++.dg/modules/tpl-friend-12_b.C | 9 .../g++.dg/modules/tpl-friend-12_c.C | 10 .../g++.dg/modules/tpl-friend-12_d.C | 8 +++ .../g++.dg/modules/tpl-friend-12_e.C | 7 +++ .../g++.dg/modules/tpl-friend-12_f.C | 8 +++ .../g++.dg/modules/tpl-friend-13_a.C | 12 + .../g++.dg/modules/tpl-friend-13_b.C | 9 .../g++.dg/modules/tpl-friend-13_c.C | 11 .../g++.dg/modules/tpl-friend-13_d.C | 7 +++ .../g++.dg/modules/tpl-friend-13_e.C | 14 + gcc/testsuite/g++.dg/modules/tpl-friend-9.C | 13 + 22 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C create mode 100644
[PATCH v2 1/2] c++: Standardise errors for module_may_redeclare
I took another look at this patch and have split it into two, one (this
one) to standardise the error messages used and prepare
'module_may_redeclare' for use with temploid friends, and another
followup patch to actually handle them correctly.
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
Currently different places calling 'module_may_redeclare' all emit very
similar but slightly different error messages, and handle different
kinds of declarations differently. This patch makes the function
perform its own error messages so that they're all in one place, and
prepares it for use with temploid friends (PR c++/114275).
gcc/cp/ChangeLog:
* cp-tree.h (module_may_redeclare): Add default parameter.
* decl.cc (duplicate_decls): Don't emit errors for failed
module_may_redeclare.
(xref_tag): Likewise.
(start_enum): Likewise.
* semantics.cc (begin_class_definition): Likewise.
* module.cc (module_may_redeclare): Clean up logic. Emit error
messages on failure.
gcc/testsuite/ChangeLog:
* g++.dg/modules/enum-12.C: Update error message.
* g++.dg/modules/friend-5_b.C: Likewise.
* g++.dg/modules/shadow-1_b.C: Likewise.
Signed-off-by: Nathaniel Shead
---
gcc/cp/cp-tree.h | 2 +-
gcc/cp/decl.cc| 28 +
gcc/cp/module.cc | 120 ++
gcc/cp/semantics.cc | 6 +-
gcc/testsuite/g++.dg/modules/enum-12.C| 2 +-
gcc/testsuite/g++.dg/modules/friend-5_b.C | 2 +-
gcc/testsuite/g++.dg/modules/shadow-1_b.C | 5 +-
7 files changed, 89 insertions(+), 76 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1dbb577a38d..faa7a0052a5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7401,7 +7401,7 @@ inline bool module_exporting_p ()
extern module_state *get_module (tree name, module_state *parent = NULL,
bool partition = false);
-extern bool module_may_redeclare (tree decl);
+extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL);
extern bool module_global_init_needed ();
extern bool module_determine_import_inits ();
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 65ab64885ff..aa66da4829d 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool
hiding, bool was_hidden)
&& TREE_CODE (olddecl) != NAMESPACE_DECL
&& !hiding)
{
- if (!module_may_redeclare (olddecl))
- {
- if (DECL_ARTIFICIAL (olddecl))
- error ("declaration %qD conflicts with builtin", newdecl);
- else
- {
- error ("declaration %qD conflicts with import", newdecl);
- inform (olddecl_loc, "import declared %q#D here", olddecl);
- }
-
- return error_mark_node;
- }
+ if (!module_may_redeclare (olddecl, newdecl))
+ return error_mark_node;
tree not_tmpl = STRIP_TEMPLATE (olddecl);
if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name,
{
tree decl = TYPE_NAME (t);
if (!module_may_redeclare (decl))
- {
- auto_diagnostic_group d;
- error ("cannot declare %qD in a different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- return error_mark_node;
- }
+ return error_mark_node;
tree not_tmpl = STRIP_TEMPLATE (decl);
if (DECL_LANG_SPECIFIC (not_tmpl)
@@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree
underlying_type,
{
tree decl = TYPE_NAME (enumtype);
if (!module_may_redeclare (decl))
- {
- auto_diagnostic_group d;
- error ("cannot declare %qD in different module", decl);
- inform (DECL_SOURCE_LOCATION (decl), "previously declared here");
- enumtype = error_mark_node;
- }
+ enumtype = error_mark_node;
else
set_instantiating_module (decl);
}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 001430a4a8f..e2d2910ae48 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible)
return module->mod;
}
-/* Is it permissible to redeclare DECL. */
+/* Is it permissible to redeclare OLDDECL with NEWDECL.
+
+ If NEWDECL is NULL, assumes that OLDDECL will be redeclared using
+ the current scope's module and attachment. */
bool
-module_may_redeclare (tree decl)
+module_may_redeclare (tree olddecl, tree newdecl)
{
+ tree decl = olddecl;
for (;;)
{
tree ctx = CP_DECL_CONTEXT (decl);
@@ -19010,58 +19014,94 @@ module_may_redeclare (tree decl)
decl = TYPE_NAME (ctx);
}
- tree not_tmpl =
Re: [PATCH] x86: Allow TImode offsettable memory only with 8-bit constant
On Sat, Apr 13, 2024 at 6:42 AM H.J. Lu wrote: > > The x86 instruction size limit is 15 bytes. If a NDD instruction has > a segment prefix byte, a 4-byte opcode prefix, a MODRM byte, a SIB byte, > a 4-byte displacement and a 4-byte immediate, adding an address size > prefix will exceed the size limit. Change TImode ADD, AND, OR and XOR > to allow offsettable memory only with 8-bit signed integer constant, > which is encoded with a 1-byte immediate, if the address size prefix > is used. Ok. > > gcc/ > > PR target/114696 > * config/i386/i386.md (isa): Add apx_ndd_64. > (enabled): Likewise. > (*add3_doubleword): Change rjO to r,ro,jO with 8-bit > signed integer constant and enable jO only for apx_ndd_64. > (*add3_doubleword_cc_overflow_1): Likewise. > (*and3_doubleword): Likewise. > (*3_doubleword): Likewise. > > gcc/testsuite/ > > PR target/114696 > * gcc.target/i386/apx-ndd-x32-2a.c: New test. > * gcc.target/i386/apx-ndd-x32-2b.c: Likewise. > * gcc.target/i386/apx-ndd-x32-2c.c: Likewise. > * gcc.target/i386/apx-ndd-x32-2d.c: Likewise. > --- > gcc/config/i386/i386.md | 36 ++- > .../gcc.target/i386/apx-ndd-x32-2a.c | 13 +++ > .../gcc.target/i386/apx-ndd-x32-2b.c | 6 > .../gcc.target/i386/apx-ndd-x32-2c.c | 6 > .../gcc.target/i386/apx-ndd-x32-2d.c | 6 > 5 files changed, 50 insertions(+), 17 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/apx-ndd-x32-2a.c > create mode 100644 gcc/testsuite/gcc.target/i386/apx-ndd-x32-2b.c > create mode 100644 gcc/testsuite/gcc.target/i386/apx-ndd-x32-2c.c > create mode 100644 gcc/testsuite/gcc.target/i386/apx-ndd-x32-2d.c > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index d4ce3809e6d..adab1ef9e04 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -568,7 +568,7 @@ (define_attr "unit" "integer,i387,sse,mmx,unknown" > > ;; Used to control the "enabled" attribute on a per-instruction basis. > (define_attr "isa" "base,x64,nox64,x64_sse2,x64_sse4,x64_sse4_noavx, > - x64_avx,x64_avx512bw,x64_avx512dq,apx_ndd, > + x64_avx,x64_avx512bw,x64_avx512dq,apx_ndd,apx_ndd_64, > sse_noavx,sse2,sse2_noavx,sse3,sse3_noavx,sse4,sse4_noavx, > > avx,noavx,avx2,noavx2,bmi,bmi2,fma4,fma,avx512f,avx512f_512, > noavx512f,avx512bw,avx512bw_512,noavx512bw,avx512dq, > @@ -968,6 +968,8 @@ (define_attr "enabled" "" >(symbol_ref "TARGET_VPCLMULQDQ && TARGET_AVX512VL") > (eq_attr "isa" "apx_ndd") >(symbol_ref "TARGET_APX_NDD") > +(eq_attr "isa" "apx_ndd_64") > + (symbol_ref "TARGET_APX_NDD && Pmode == DImode") > (eq_attr "isa" "vaes_avx512vl") >(symbol_ref "TARGET_VAES && TARGET_AVX512VL") > > @@ -6302,10 +6304,10 @@ (define_expand "add3" > }) > > (define_insn_and_split "*add3_doubleword" > - [(set (match_operand: 0 "nonimmediate_operand" "=ro,r,,,") > + [(set (match_operand: 0 "nonimmediate_operand" "=ro,r,") > (plus: > - (match_operand: 1 "nonimmediate_operand" "%0,0,ro,rjO,r") > - (match_operand: 2 "x86_64_hilo_general_operand" > "r,o,r,,r"))) > + (match_operand: 1 "nonimmediate_operand" "%0,0,ro,r,ro,jO,r") > + (match_operand: 2 "x86_64_hilo_general_operand" > "r,o,r,,K,,r"))) > (clobber (reg:CC FLAGS_REG))] >"ix86_binary_operator_ok (PLUS, mode, operands, TARGET_APX_NDD)" >"#" > @@ -6344,7 +6346,7 @@ (define_insn_and_split "*add3_doubleword" >DONE; > } > } > -[(set_attr "isa" "*,*,apx_ndd,apx_ndd,apx_ndd")]) > +[(set_attr "isa" "*,*,apx_ndd,apx_ndd,apx_ndd,apx_ndd_64,apx_ndd")]) > > (define_insn_and_split "*add3_doubleword_zext" >[(set (match_operand: 0 "nonimmediate_operand" "=r,o,,") > @@ -9515,10 +9517,10 @@ (define_insn_and_split > "*add3_doubleword_cc_overflow_1" >[(set (reg:CCC FLAGS_REG) > (compare:CCC > (plus: > - (match_operand: 1 "nonimmediate_operand" "%0,0,ro,rjO,r") > - (match_operand: 2 "x86_64_hilo_general_operand" > "r,o,r,,o")) > + (match_operand: 1 "nonimmediate_operand" "%0,0,ro,r,ro,jO,r") > + (match_operand: 2 "x86_64_hilo_general_operand" > "r,o,r,,K,,o")) > (match_dup 1))) > - (set (match_operand: 0 "nonimmediate_operand" "=ro,r,,,") > + (set (match_operand: 0 "nonimmediate_operand" "=ro,r,") > (plus: (match_dup 1) (match_dup 2)))] >"ix86_binary_operator_ok (PLUS, mode, operands, TARGET_APX_NDD)" >"#" > @@ -9560,7 +9562,7 @@ (define_insn_and_split > "*add3_doubleword_cc_overflow_1" >else > operands[6] = gen_rtx_ZERO_EXTEND (mode, operands[5]); > } > -[(set_attr "isa" "*,*,apx_ndd,apx_ndd,apx_ndd")]) > +[(set_attr "isa"
[Backport 2/2] middle-end/114599 - fix bitmap allocation for check_ifunc_callee_symtab_nodes
From: Richard Biener
There's no default bitmap obstack during global CTORs, so allocate the
bitmap locally.
PR middle-end/114599
PR gcov-profile/114115
* symtab.cc (ifunc_ref_map): Do not use auto_bitmap.
(is_caller_ifunc_resolver): Optimize bitmap_bit_p/bitmap_set_bit
pair.
(symtab_node::check_ifunc_callee_symtab_nodes): Properly
allocate ifunc_ref_map here.
(cherry picked from commit 9ab8fdfeef5b1a47b358e08a98177b2fad65fed9)
---
gcc/symtab.cc | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index df09def81e9..10ec6d03842 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -1383,7 +1383,7 @@ check_ifunc_resolver (cgraph_node *node, void *data)
return false;
}
-static auto_bitmap ifunc_ref_map;
+static bitmap ifunc_ref_map;
/* Return true if any caller of NODE is an ifunc resolver. */
@@ -1404,9 +1404,8 @@ is_caller_ifunc_resolver (cgraph_node *node)
/* Skip if it has been visited. */
unsigned int uid = e->caller->get_uid ();
- if (bitmap_bit_p (ifunc_ref_map, uid))
+ if (!bitmap_set_bit (ifunc_ref_map, uid))
continue;
- bitmap_set_bit (ifunc_ref_map, uid);
if (is_caller_ifunc_resolver (e->caller))
{
@@ -1437,6 +1436,9 @@ symtab_node::check_ifunc_callee_symtab_nodes (void)
{
symtab_node *node;
+ bitmap_obstack_initialize (NULL);
+ ifunc_ref_map = BITMAP_ALLOC (NULL);
+
FOR_EACH_SYMBOL (node)
{
cgraph_node *cnode = dyn_cast (node);
@@ -1455,7 +1457,8 @@ symtab_node::check_ifunc_callee_symtab_nodes (void)
cnode->called_by_ifunc_resolver = true;
}
- bitmap_clear (ifunc_ref_map);
+ BITMAP_FREE (ifunc_ref_map);
+ bitmap_obstack_release (NULL);
}
/* Verify symbol table for internal consistency. */
--
2.44.0
[Backport 1/2] tree-profile: Disable indirect call profiling for IFUNC resolvers
We can't profile indirect calls to IFUNC resolvers nor their callees as
it requires TLS which hasn't been set up yet when the dynamic linker is
resolving IFUNC symbols.
Add an IFUNC resolver caller marker to cgraph_node and set it if the
function is called by an IFUNC resolver. Disable indirect call profiling
for IFUNC resolvers and their callees.
Tested with profiledbootstrap on Fedora 39/x86-64.
gcc/ChangeLog:
PR tree-optimization/114115
* cgraph.h (symtab_node): Add check_ifunc_callee_symtab_nodes.
(cgraph_node): Add called_by_ifunc_resolver.
* cgraphunit.cc (symbol_table::compile): Call
symtab_node::check_ifunc_callee_symtab_nodes.
* symtab.cc (check_ifunc_resolver): New.
(ifunc_ref_map): Likewise.
(is_caller_ifunc_resolver): Likewise.
(symtab_node::check_ifunc_callee_symtab_nodes): Likewise.
* tree-profile.cc (gimple_gen_ic_func_profiler): Disable indirect
call profiling for IFUNC resolvers and their callees.
gcc/testsuite/ChangeLog:
PR tree-optimization/114115
* gcc.dg/pr114115.c: New test.
(cherry picked from commit cab32bacaea268ec062b1fb4fc662d90c9d1cfce)
---
gcc/cgraph.h| 6 +++
gcc/cgraphunit.cc | 2 +
gcc/symtab.cc | 89 +
gcc/testsuite/gcc.dg/pr114115.c | 24 +
gcc/tree-profile.cc | 8 ++-
5 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr114115.c
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index c1a3691b6f5..430c87d8bb7 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -479,6 +479,9 @@ public:
Return NULL if there's no such node. */
static symtab_node *get_for_asmname (const_tree asmname);
+ /* Check symbol table for callees of IFUNC resolvers. */
+ static void check_ifunc_callee_symtab_nodes (void);
+
/* Verify symbol table for internal consistency. */
static DEBUG_FUNCTION void verify_symtab_nodes (void);
@@ -896,6 +899,7 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public
symtab_node
redefined_extern_inline (false), tm_may_enter_irr (false),
ipcp_clone (false), declare_variant_alt (false),
calls_declare_variant_alt (false), gc_candidate (false),
+ called_by_ifunc_resolver (false),
m_uid (uid), m_summary_id (-1)
{}
@@ -1491,6 +1495,8 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
public symtab_node
is set for local SIMD clones when they are created and cleared if the
vectorizer uses them. */
unsigned gc_candidate : 1;
+ /* Set if the function is called by an IFUNC resolver. */
+ unsigned called_by_ifunc_resolver : 1;
private:
/* Unique id of the node. */
diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
index bccd2f2abb5..40dcceccca5 100644
--- a/gcc/cgraphunit.cc
+++ b/gcc/cgraphunit.cc
@@ -2313,6 +2313,8 @@ symbol_table::compile (void)
symtab_node::checking_verify_symtab_nodes ();
+ symtab_node::check_ifunc_callee_symtab_nodes ();
+
timevar_push (TV_CGRAPHOPT);
if (pre_ipa_mem_report)
dump_memory_report ("Memory consumption before IPA");
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index 0470509a98d..df09def81e9 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -1369,6 +1369,95 @@ symtab_node::verify (void)
timevar_pop (TV_CGRAPH_VERIFY);
}
+/* Return true and set *DATA to true if NODE is an ifunc resolver. */
+
+static bool
+check_ifunc_resolver (cgraph_node *node, void *data)
+{
+ if (node->ifunc_resolver)
+{
+ bool *is_ifunc_resolver = (bool *) data;
+ *is_ifunc_resolver = true;
+ return true;
+}
+ return false;
+}
+
+static auto_bitmap ifunc_ref_map;
+
+/* Return true if any caller of NODE is an ifunc resolver. */
+
+static bool
+is_caller_ifunc_resolver (cgraph_node *node)
+{
+ bool is_ifunc_resolver = false;
+
+ for (cgraph_edge *e = node->callers; e; e = e->next_caller)
+{
+ /* Return true if caller is known to be an IFUNC resolver. */
+ if (e->caller->called_by_ifunc_resolver)
+ return true;
+
+ /* Check for recursive call. */
+ if (e->caller == node)
+ continue;
+
+ /* Skip if it has been visited. */
+ unsigned int uid = e->caller->get_uid ();
+ if (bitmap_bit_p (ifunc_ref_map, uid))
+ continue;
+ bitmap_set_bit (ifunc_ref_map, uid);
+
+ if (is_caller_ifunc_resolver (e->caller))
+ {
+ /* Return true if caller is an IFUNC resolver. */
+ e->caller->called_by_ifunc_resolver = true;
+ return true;
+ }
+
+ /* Check if caller's alias is an IFUNC resolver. */
+ e->caller->call_for_symbol_and_aliases (check_ifunc_resolver,
+ _ifunc_resolver,
+ true);
+ if (is_ifunc_resolver)
+ {
+ /* Return true if caller's alias is an IFUNC resolver. */
+
Re: [PATCH V4 1/3] aarch64: Place target independent and dependent changed code in one file
Hello Alex:
On 12/04/24 11:02 pm, Ajit Agarwal wrote:
> Hello Alex:
>
> On 12/04/24 8:15 pm, Alex Coplan wrote:
>> On 12/04/2024 20:02, Ajit Agarwal wrote:
>>> Hello Alex:
>>>
>>> On 11/04/24 7:55 pm, Alex Coplan wrote:
On 10/04/2024 23:48, Ajit Agarwal wrote:
> Hello Alex:
>
> On 10/04/24 7:52 pm, Alex Coplan wrote:
>> Hi Ajit,
>>
>> On 10/04/2024 15:31, Ajit Agarwal wrote:
>>> Hello Alex:
>>>
>>> On 10/04/24 1:42 pm, Alex Coplan wrote:
Hi Ajit,
On 09/04/2024 20:59, Ajit Agarwal wrote:
> Hello Alex:
>
> On 09/04/24 8:39 pm, Alex Coplan wrote:
>> On 09/04/2024 20:01, Ajit Agarwal wrote:
>>> Hello Alex:
>>>
>>> On 09/04/24 7:29 pm, Alex Coplan wrote:
On 09/04/2024 17:30, Ajit Agarwal wrote:
>
>
> On 05/04/24 10:03 pm, Alex Coplan wrote:
>> On 05/04/2024 13:53, Ajit Agarwal wrote:
>>> Hello Alex/Richard:
>>>
>>> All review comments are incorporated.
>>
>>> @@ -2890,8 +3018,8 @@ ldp_bb_info::merge_pairs (insn_list_t
>>> _list,
>>> // of accesses. If we find two sets of adjacent accesses, call
>>> // merge_pairs.
>>> void
>>> -ldp_bb_info::transform_for_base (int encoded_lfs,
>>> -access_group )
>>> +pair_fusion_bb_info::transform_for_base (int encoded_lfs,
>>> +access_group )
>>> {
>>>const auto lfs = decode_lfs (encoded_lfs);
>>>const unsigned access_size = lfs.size;
>>> @@ -2909,7 +3037,7 @@ ldp_bb_info::transform_for_base (int
>>> encoded_lfs,
>>>access.cand_insns,
>>>lfs.load_p,
>>>access_size);
>>> - skip_next = access.cand_insns.empty ();
>>> + skip_next = bb_state->cand_insns_empty_p
>>> (access.cand_insns);
>>
>> As above, why is this needed?
>
> For rs6000 we want to return always true. as load store pair
> that are to be merged with 8/16 16/32 32/64 is occuring for
> rs6000.
> And we want load store pair to 8/16 32/64. Thats why we want
> to generate always true for rs6000 to skip pairs as above.
Hmm, sorry, I'm not sure I follow. Are you saying that for rs6000
you have
load/store pair instructions where the two arms of the access are
storing
operands of different sizes? Or something else?
As it stands the logic is to skip the next iteration only if we
exhausted all the candidate insns for the current access. In the
case
that we didn't exhaust all such candidates, then the idea is that
when
access becomes prev_access, we can attempt to use those candidates
as
the "left-hand side" of a pair in the next iteration since we
failed to
use them as the "right-hand side" of a pair in the current
iteration.
I don't see why you wouldn't want that behaviour. Please can you
explain?
>>>
>>> In merge_pair we get the 2 load candiates one load from 0 offset and
>>> other load is from 16th offset. Then in next iteration we get load
>>> from 16th offset and other load from 32 offset. In next iteration
>>> we get load from 32 offset and other load from 48 offset.
>>>
>>> For example:
>>>
>>> Currently we get the load candiates as follows.
>>>
>>> pairs:
>>>
>>> load from 0th offset.
>>> load from 16th offset.
>>>
>>> next pairs:
>>>
>>> load from 16th offset.
>>> load from 32th offset.
>>>
>>> next pairs:
>>>
>>> load from 32th offset
>>> load from 48th offset.
>>>
>>> Instead in rs6000 we should get:
>>>
>>> pairs:
>>>
>>> load from 0th offset
>>> load from 16th offset.
>>>
>>> next pairs:
>>>
>>> load from 32th offset
>>> load from 48th offset.
>>
>> Hmm, so then I guess my question is: why wouldn't you consider
>> merging
>> the pair with offsets (16,32) for rs6000? Is it because you have a
>> stricter alignment requirement on the base pair offsets (i.e. they
>> have
>> to be a multiple of 32 when the operand size is 16)? So the pair
>> offsets have to be a
Re: [wwwdocs] gcc-14: Mention that some warnings are now errors
On Sun, Apr 14, 2024 at 05:55:03PM +0200, Gerald Pfeifer wrote: > Hi Sebastian, > > On Sat, 13 Apr 2024, Sebastian Huber wrote: > > + The following warnings are now errors (see also > > +Porting to GCC 14): > > + > > + -Werror=declaration-missing-parameter-type > > + -Werror=implicit-function-declaration > > + -Werror=implicit-int > > + -Werror=incompatible-pointer-types > > + -Werror=int-conversion > > + -Werror=return-mismatch > > + > > thanks for putting this together. Just a question, and maybe I'm confused: > these don't look like warnings to me? > > Are you saying that what used to be -Wint-conversion is now an error by > default and does not need to activated as such with any further options? > > (That is, it appears as if those -Werror=... flags were used?) > > And does this apply to all C versions/dialects? No, so I think the change is certainly not appropriate in that form. The porting_to.html, in particular porting_to.html#warnings-as-errors has detailed explanation on what actually changed etc., so I think we just want one short sentence that certain C warnings are now errors and refer to particular porting_to.html#warnings-as-errors for details. Because the above shortcut is certainly not true. Jakub
Re: [wwwdocs] gcc-14: Mention that some warnings are now errors
Hi Sebastian, On Sat, 13 Apr 2024, Sebastian Huber wrote: > + The following warnings are now errors (see also > +Porting to GCC 14): > + > + -Werror=declaration-missing-parameter-type > + -Werror=implicit-function-declaration > + -Werror=implicit-int > + -Werror=incompatible-pointer-types > + -Werror=int-conversion > + -Werror=return-mismatch > + thanks for putting this together. Just a question, and maybe I'm confused: these don't look like warnings to me? Are you saying that what used to be -Wint-conversion is now an error by default and does not need to activated as such with any further options? (That is, it appears as if those -Werror=... flags were used?) And does this apply to all C versions/dialects? Gerald
Re: [pushed] c++/modules: make bits_in/out move-constructible
On Sat, 13 Apr 2024, Patrick Palka wrote: > Pushed as obvious after verifying C++11 bootstrap is restored. Thank you, Patrick! x86_64-unknown-freebsd13.2 is back to bootstrap again as well (with clang version 16.0.6). Gerald
Re: [C PATCH, v2] Fix ICE with -g and -std=c23 related to incomplete types [PR114361]
(new email for Joseph)
Am Sonntag, dem 14.04.2024 um 14:30 +0200 schrieb Martin Uecker:
> I had to revert the old patch because it broke LTO which lead
> to PR114574. We now set TYPE_STRUCTURAL_EQUALITY and properly
> update TYPE_CANONICAL for such types and also for pointers
> to such types via a new function c_update_type_canonical
> (thanks to Jakob).
>
>
> Bootstrapped and regession tested on x86_64.
>
>
>
> Fix ICE with -g and -std=c23 related to incomplete types [PR114361]
>
> We did not update TYPE_CANONICAL for incomplete variants when
> completing a structure. We now set TYPE_STRUCTURAL_EQUALITY for
> incomplete structure and union types and then update TYPE_CANONICAL
> later. See PR114574 for discussion.
>
>
> 2024-04-12 Martin Uecker
> Jakub Jelinek
>
> PR lto/114574
> PR c/114361
> gcc/
> * ipa-free-lang-data.cc (fld_incomplete_type_of): Allow
> either of the types in the assert to have TYPE_STRUCTURAL_EQUALITY_P.
> gcc/c/
> * c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not
> ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY.
> (parser_xref_tag): Likewise.
> (start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY.
> (c_update_type_canonical): New function.
> (finish_struct): Put NULL as second == operand rather than first.
> Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical.
> * c-typeck.cc (composite_type_internal): Use
> SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix.
> gcc/testsuite/
> * gcc.dg/pr114574-1.c: New test.
> * gcc.dg/pr114574-2.c: New test.
> * gcc.dg/pr114361.c: New test.
> * gcc.dg/c23-tag-incomplete-1.c: New test.
> * gcc.dg/c23-tag-incomplete-2.c: New test.
> ---
> gcc/c/c-decl.cc | 47 -
> gcc/c/c-typeck.cc | 4 +-
> gcc/ipa-free-lang-data.cc | 4 +-
> gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c | 14 ++
> gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c | 13 ++
> gcc/testsuite/gcc.dg/pr114361.c | 11 +
> gcc/testsuite/gcc.dg/pr114574-1.c | 10 +
> gcc/testsuite/gcc.dg/pr114574-2.c | 10 +
> 8 files changed, 110 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c
> create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c
> create mode 100644 gcc/testsuite/gcc.dg/pr114361.c
> create mode 100644 gcc/testsuite/gcc.dg/pr114574-1.c
> create mode 100644 gcc/testsuite/gcc.dg/pr114574-2.c
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 345090dae38..54917297b6a 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declspecs *declspecs,
> int warned)
> if (t == NULL_TREE)
> {
> t = make_node (code);
> + if (flag_isoc23 && code != ENUMERAL_TYPE)
> + SET_TYPE_STRUCTURAL_EQUALITY (t);
> pushtag (input_location, name, t);
> }
> }
> @@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tree_code code,
> tree name,
> the forward-reference will be altered into a real type. */
>
>ref = make_node (code);
> + if (flag_isoc23 && code != ENUMERAL_TYPE)
> +SET_TYPE_STRUCTURAL_EQUALITY (ref);
>if (code == ENUMERAL_TYPE)
> {
>/* Give the type a default layout like unsigned int
> @@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_code code, tree
> name,
>if (ref == NULL_TREE || TREE_CODE (ref) != code)
> {
>ref = make_node (code);
> + if (flag_isoc23)
> + SET_TYPE_STRUCTURAL_EQUALITY (ref);
>pushtag (loc, name, ref);
> }
>
> @@ -9347,6 +9353,43 @@ is_flexible_array_member_p (bool is_last_field,
>return false;
> }
>
> +/* Recompute TYPE_CANONICAL for qualified versions of the type and
> + related pointer types after an aggregate type has been finalized.
> + Will not update array types, pointers to array types, function
> + types and other derived types created while the type was still
> + incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */
> +
> +static void
> +c_update_type_canonical (tree t)
> +{
> + for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
> +{
> + if (x != t
> + && TYPE_STRUCTURAL_EQUALITY_P (x)
> + && check_qualified_type (x, t, TYPE_QUALS (x)))
> + {
> + if (TYPE_CANONICAL (t) != t)
> + TYPE_CANONICAL (x)
> + = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> + else
> + TYPE_CANONICAL (x) = x;
> + }
> + else if (x != t)
> + continue;
> + for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p))
> + {
> + if (!TYPE_STRUCTURAL_EQUALITY_P (p))
> + continue;
> + if (TYPE_CANONICAL (x) != x ||
[C PATCH, v2] Fix ICE with -g and -std=c23 related to incomplete types [PR114361]
I had to revert the old patch because it broke LTO which lead
to PR114574. We now set TYPE_STRUCTURAL_EQUALITY and properly
update TYPE_CANONICAL for such types and also for pointers
to such types via a new function c_update_type_canonical
(thanks to Jakob).
Bootstrapped and regession tested on x86_64.
Fix ICE with -g and -std=c23 related to incomplete types [PR114361]
We did not update TYPE_CANONICAL for incomplete variants when
completing a structure. We now set TYPE_STRUCTURAL_EQUALITY for
incomplete structure and union types and then update TYPE_CANONICAL
later. See PR114574 for discussion.
2024-04-12 Martin Uecker
Jakub Jelinek
PR lto/114574
PR c/114361
gcc/
* ipa-free-lang-data.cc (fld_incomplete_type_of): Allow
either of the types in the assert to have TYPE_STRUCTURAL_EQUALITY_P.
gcc/c/
* c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not
ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY.
(parser_xref_tag): Likewise.
(start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY.
(c_update_type_canonical): New function.
(finish_struct): Put NULL as second == operand rather than first.
Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical.
* c-typeck.cc (composite_type_internal): Use
SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix.
gcc/testsuite/
* gcc.dg/pr114574-1.c: New test.
* gcc.dg/pr114574-2.c: New test.
* gcc.dg/pr114361.c: New test.
* gcc.dg/c23-tag-incomplete-1.c: New test.
* gcc.dg/c23-tag-incomplete-2.c: New test.
---
gcc/c/c-decl.cc | 47 -
gcc/c/c-typeck.cc | 4 +-
gcc/ipa-free-lang-data.cc | 4 +-
gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c | 14 ++
gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c | 13 ++
gcc/testsuite/gcc.dg/pr114361.c | 11 +
gcc/testsuite/gcc.dg/pr114574-1.c | 10 +
gcc/testsuite/gcc.dg/pr114574-2.c | 10 +
8 files changed, 110 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c
create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr114361.c
create mode 100644 gcc/testsuite/gcc.dg/pr114574-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr114574-2.c
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 345090dae38..54917297b6a 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declspecs *declspecs,
int warned)
if (t == NULL_TREE)
{
t = make_node (code);
+ if (flag_isoc23 && code != ENUMERAL_TYPE)
+ SET_TYPE_STRUCTURAL_EQUALITY (t);
pushtag (input_location, name, t);
}
}
@@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tree_code code,
tree name,
the forward-reference will be altered into a real type. */
ref = make_node (code);
+ if (flag_isoc23 && code != ENUMERAL_TYPE)
+SET_TYPE_STRUCTURAL_EQUALITY (ref);
if (code == ENUMERAL_TYPE)
{
/* Give the type a default layout like unsigned int
@@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_code code, tree
name,
if (ref == NULL_TREE || TREE_CODE (ref) != code)
{
ref = make_node (code);
+ if (flag_isoc23)
+ SET_TYPE_STRUCTURAL_EQUALITY (ref);
pushtag (loc, name, ref);
}
@@ -9347,6 +9353,43 @@ is_flexible_array_member_p (bool is_last_field,
return false;
}
+/* Recompute TYPE_CANONICAL for qualified versions of the type and
+ related pointer types after an aggregate type has been finalized.
+ Will not update array types, pointers to array types, function
+ types and other derived types created while the type was still
+ incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */
+
+static void
+c_update_type_canonical (tree t)
+{
+ for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
+{
+ if (x != t
+ && TYPE_STRUCTURAL_EQUALITY_P (x)
+ && check_qualified_type (x, t, TYPE_QUALS (x)))
+ {
+ if (TYPE_CANONICAL (t) != t)
+ TYPE_CANONICAL (x)
+ = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
+ else
+ TYPE_CANONICAL (x) = x;
+ }
+ else if (x != t)
+ continue;
+ for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p))
+ {
+ if (!TYPE_STRUCTURAL_EQUALITY_P (p))
+ continue;
+ if (TYPE_CANONICAL (x) != x || TYPE_REF_CAN_ALIAS_ALL (p))
+ TYPE_CANONICAL (p)
+ = build_pointer_type_for_mode (TYPE_CANONICAL (x), TYPE_MODE (p),
+false);
+ else
+ TYPE_CANONICAL (p) = p;
+
Re: [PATCH] c++/modules: Setup aliases imported from modules [PR106820]
On Fri, Apr 12, 2024 at 03:54:21PM -0400, Jason Merrill wrote:
> On 3/26/24 09:24, Nathaniel Shead wrote:
> >
> > I wonder if more generally we need to be doing more work when importing
> > definitions from header units especially to handle all the work that
> > 'make_rtl_for_nonlocal_decl' and 'rest_of_decl_compilation' would have
> > been performing.
>
> Can we just call those functions?
>
Probably at least 'rest_of_decl_compilation' can (should?) be called
from modules, but I haven't worked out exactly in which circumstances
yet and whether there's any other work it does that doesn't make sense
in a modules context. I don't think 'make_rtl_for_nonlocal_decl' can
easily be called directly though.
At the very least it'll probably need to get called in
`read_definition` which doesn't clash with aliases (which are never
definitions, as far as I can tell), so this will still be necessary.
> > But this patch fixes at least one missing step.
>
> OK.
>
Thanks, pushed as r14-9959-g62a0ef0d02cbb7.
> > PR c++/106820
> >
> > gcc/cp/ChangeLog:
> >
> > * module.cc (trees_in::decl_value): Assemble alias when needed.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/pr106820_a.H: New test.
> > * g++.dg/modules/pr106820_b.C: New test.
> >
> > Signed-off-by: Nathaniel Shead
> > ---
> > gcc/cp/module.cc | 9 +
> > gcc/testsuite/g++.dg/modules/pr106820_a.H | 5 +
> > gcc/testsuite/g++.dg/modules/pr106820_b.C | 8
> > 3 files changed, 22 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr106820_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/pr106820_b.C
> >
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 8aab9ea0bae..b4e3b38c6fe 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -217,6 +217,7 @@ Classes used:
> > #include "dumpfile.h"
> > #include "bitmap.h"
> > #include "cgraph.h"
> > +#include "varasm.h"
> > #include "tree-iterator.h"
> > #include "cpplib.h"
> > #include "mkdeps.h"
> > @@ -8302,6 +8303,14 @@ trees_in::decl_value ()
> > if (state->is_header ()
> > && decl_tls_wrapper_p (decl))
> > note_vague_linkage_fn (decl);
> > +
> > + /* Setup aliases for the declaration. */
> > + if (tree alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)))
> > + {
> > + alias = TREE_VALUE (TREE_VALUE (alias));
> > + alias = get_identifier (TREE_STRING_POINTER (alias));
> > + assemble_alias (decl, alias);
> > + }
> > }
> > else
> > {
> > diff --git a/gcc/testsuite/g++.dg/modules/pr106820_a.H
> > b/gcc/testsuite/g++.dg/modules/pr106820_a.H
> > new file mode 100644
> > index 000..7d32d4e5fc3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr106820_a.H
> > @@ -0,0 +1,5 @@
> > +// PR c++/106820
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi {} }
> > +
> > +static int __gthrw___pthread_key_create()
> > __attribute__((__weakref__("foo")));
> > diff --git a/gcc/testsuite/g++.dg/modules/pr106820_b.C
> > b/gcc/testsuite/g++.dg/modules/pr106820_b.C
> > new file mode 100644
> > index 000..247fe26e778
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/pr106820_b.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/106820
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import "pr106820_a.H";
> > +
> > +int main() {
> > + __gthrw___pthread_key_create();
> > +}
>
