[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 Richard Biener changed: What|Removed |Added Target Milestone|13.2|13.3 --- Comment #13 from Richard Biener --- GCC 13.2 is being released, retargeting bugs to GCC 13.3.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 Richard Biener changed: What|Removed |Added Target Milestone|13.0|13.2 --- Comment #12 from Richard Biener --- GCC 13.1 is being released, retargeting bugs to GCC 13.2.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 Martin Liška changed: What|Removed |Added Target Milestone|--- |13.0 Keywords||patch --- Comment #11 from Martin Liška --- Patch candidate: https://gcc.gnu.org/pipermail/gcc-patches/2023-April/615049.html
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #10 from Martin Liška --- (In reply to Martin Jambor from comment #9) > (In reply to Martin Liška from comment #6) > > > > @Martin: Do we have a declaration cloning code for functions somewhere? > > See e.g. cgraph_node::create_virtual_clone in cgraphclones.cc. Unless > you want to mess with the parameters, > > new_decl = copy_node (old_decl); > > should be enough (and it should copy over the DECL_PURE and > TREE_READLONY bit soo, I believe). Thanks! So apparently, we call make_dispatcher_decl which makes only a partial copy of a function_decl: original decl: > QI size unit-size align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x77656d20 arg-types > pointer_to_this > volatile nothrow public static decl_5 QI /home/marxin/Programming/testcases/pr106816.c:2:6 align:8 warn_if_not_align:0 context attributes > value readonly constant static "default\000">> chain >>> initial result ignored VOID /home/marxin/Programming/testcases/pr106816.c:2:1 align:8 warn_if_not_align:0 context > full-name "void f()" struct-function 0x777df000 chain > dispatcher: (gdb) p debug_tree(func_decl) > QI size unit-size align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x77656d20 arg-types > pointer_to_this > nothrow public external QI /home/marxin/Programming/testcases/pr106816.c:15:1 align:8 warn_if_not_align:0> So we modify the name, drop 'target' attribute, and so on. Thus we can't use copy_node. > > I am not sure at what point the duplication happens, in order to > duplicate also all the various IPA summaries, cgraph machinery has to > be involved and call all the summary hooks. So > cgraph_node::create_clone should probably be used (or a part of it, or > perhaps even one of its users?). It can happen during the multiple_target.cc IPA pass, so lemme combine the H.J.'s approach and cgraph_node::create_clone.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #9 from Martin Jambor --- (In reply to Martin Liška from comment #6) > > @Martin: Do we have a declaration cloning code for functions somewhere? See e.g. cgraph_node::create_virtual_clone in cgraphclones.cc. Unless you want to mess with the parameters, new_decl = copy_node (old_decl); should be enough (and it should copy over the DECL_PURE and TREE_READLONY bit soo, I believe). I am not sure at what point the duplication happens, in order to duplicate also all the various IPA summaries, cgraph machinery has to be involved and call all the summary hooks. So cgraph_node::create_clone should probably be used (or a part of it, or perhaps even one of its users?).
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #8 from Martin Liška --- > @Martin: Do we have a declaration cloning code for functions somewhere? @jamborm: PING
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 Martin Liška changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Status|NEW |ASSIGNED --- Comment #7 from Martin Liška --- Let me assign it to myself.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816
--- Comment #6 from Martin Liška ---
(In reply to Richard Biener from comment #5)
> The function should probably inherit all of the IPA pure/const/modref
> analysis result, that is all "IPA" state should be copied. I think we want
> some helper
> here - IPA clone creation must have something, no?
Well, in IPA clones utilize so-called function_summary that is a typical place
where we store analysis results. And we don't typically create a new tree
declarations when we clone cgraph_nodes (clones share the same FE declaration).
However, I noticed we want to do likely something similar to what cp/decl.c
does:
static void
merge_attribute_bits (tree newdecl, tree olddecl)
{
TREE_THIS_VOLATILE (newdecl) |= TREE_THIS_VOLATILE (olddecl);
TREE_THIS_VOLATILE (olddecl) |= TREE_THIS_VOLATILE (newdecl);
TREE_NOTHROW (newdecl) |= TREE_NOTHROW (olddecl);
TREE_NOTHROW (olddecl) |= TREE_NOTHROW (newdecl);
TREE_READONLY (newdecl) |= TREE_READONLY (olddecl);
TREE_READONLY (olddecl) |= TREE_READONLY (newdecl);
DECL_IS_MALLOC (newdecl) |= DECL_IS_MALLOC (olddecl);
DECL_IS_MALLOC (olddecl) |= DECL_IS_MALLOC (newdecl);
DECL_PURE_P (newdecl) |= DECL_PURE_P (olddecl);
DECL_PURE_P (olddecl) |= DECL_PURE_P (newdecl);
DECL_UNINLINABLE (newdecl) |= DECL_UNINLINABLE (olddecl);
DECL_UNINLINABLE (olddecl) |= DECL_UNINLINABLE (newdecl);
}
...
/* Merge the noreturn bit. */
TREE_THIS_VOLATILE (olddecl) = TREE_THIS_VOLATILE (newdecl);
TREE_READONLY (olddecl) = TREE_READONLY (newdecl);
TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
DECL_IS_MALLOC (olddecl) = DECL_IS_MALLOC (newdecl);
DECL_PURE_P (olddecl) = DECL_PURE_P (newdecl);
I can see IPA passes doing similar declaration clonning for VAR_DECLs
(gcc/ipa-param-manipulation.cc, gcc/tree-inline.c).
@Martin: Do we have a declaration cloning code for functions somewhere?
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #5 from Richard Biener --- The function should probably inherit all of the IPA pure/const/modref analysis result, that is all "IPA" state should be copied. I think we want some helper here - IPA clone creation must have something, no?
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #4 from Simon Rainer --- That's weird, I still get the following with your patch applied: main: .LFB2: .cfi_startproc subq$8, %rsp .cfi_def_cfa_offset 16 call_Z5_Z1fvv@PLT movl$1, %eax addq$8, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc I double checked that and reran a full bootstrap, but maybe I'm doing something wrong. I would also be surprised if information about volatile, readonly, and pure are enough to detect that the function is noreturn, wouldn't that need to be a separate information?
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #3 from H.J. Lu --- (In reply to Simon Rainer from comment #2) > (In reply to H.J. Lu from comment #1) > > Like this? > > > > diff --git a/gcc/config/i386/i386-features.cc > > b/gcc/config/i386/i386-features.cc > > index fd212262f50..4904e4d71b3 100644 > > --- a/gcc/config/i386/i386-features.cc > > +++ b/gcc/config/i386/i386-features.cc > > @@ -3269,6 +3269,9 @@ ix86_get_function_versions_dispatcher (void *decl) > >/* Right now, the dispatching is done via ifunc. */ > >dispatch_decl = make_dispatcher_decl (default_node->decl); > >TREE_NOTHROW (dispatch_decl) = TREE_NOTHROW (fn); > > + TREE_THIS_VOLATILE (dispatch_decl) = TREE_THIS_VOLATILE (fn); > > + TREE_READONLY (dispatch_decl) = TREE_READONLY (fn); > > + DECL_PURE_P (dispatch_decl) = DECL_PURE_P (fn); > > > >dispatcher_node = cgraph_node::get_create (dispatch_decl); > >gcc_assert (dispatcher_node != NULL); > > I tried something like that, but I didn't see changes in the generated > assembly. I don't know if that is because something else is preventing > optimization or some other member needs to be set correctly. I got main: .LFB2: .cfi_startproc subq$8, %rsp .cfi_def_cfa_offset 16 call_Z5_Z1fvv .cfi_endproc It looks correct.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 --- Comment #2 from Simon Rainer --- (In reply to H.J. Lu from comment #1) > Like this? > > diff --git a/gcc/config/i386/i386-features.cc > b/gcc/config/i386/i386-features.cc > index fd212262f50..4904e4d71b3 100644 > --- a/gcc/config/i386/i386-features.cc > +++ b/gcc/config/i386/i386-features.cc > @@ -3269,6 +3269,9 @@ ix86_get_function_versions_dispatcher (void *decl) >/* Right now, the dispatching is done via ifunc. */ >dispatch_decl = make_dispatcher_decl (default_node->decl); >TREE_NOTHROW (dispatch_decl) = TREE_NOTHROW (fn); > + TREE_THIS_VOLATILE (dispatch_decl) = TREE_THIS_VOLATILE (fn); > + TREE_READONLY (dispatch_decl) = TREE_READONLY (fn); > + DECL_PURE_P (dispatch_decl) = DECL_PURE_P (fn); > >dispatcher_node = cgraph_node::get_create (dispatch_decl); >gcc_assert (dispatcher_node != NULL); I tried something like that, but I didn't see changes in the generated assembly. I don't know if that is because something else is preventing optimization or some other member needs to be set correctly.
[Bug ipa/106816] noreturn/pure attributes are not set correctly on multiversioned functions
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106816 H.J. Lu changed: What|Removed |Added Last reconfirmed||2022-09-02 Status|UNCONFIRMED |NEW Ever confirmed|0 |1 --- Comment #1 from H.J. Lu --- Like this? diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc index fd212262f50..4904e4d71b3 100644 --- a/gcc/config/i386/i386-features.cc +++ b/gcc/config/i386/i386-features.cc @@ -3269,6 +3269,9 @@ ix86_get_function_versions_dispatcher (void *decl) /* Right now, the dispatching is done via ifunc. */ dispatch_decl = make_dispatcher_decl (default_node->decl); TREE_NOTHROW (dispatch_decl) = TREE_NOTHROW (fn); + TREE_THIS_VOLATILE (dispatch_decl) = TREE_THIS_VOLATILE (fn); + TREE_READONLY (dispatch_decl) = TREE_READONLY (fn); + DECL_PURE_P (dispatch_decl) = DECL_PURE_P (fn); dispatcher_node = cgraph_node::get_create (dispatch_decl); gcc_assert (dispatcher_node != NULL);
