| Message ID | 20240904210521.3310478-1-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Handle attributes in the global namespace for aarch64_lookup_shared_state_flags [PR116598] | expand |
On Wed, Sep 04, 2024 at 02:05:21PM -0700, Andrew Pinski wrote: > The code in aarch64_lookup_shared_state_flags all C++11 attributes on the function type > had a namespace associated with them. But with the addition of reproducible/unsequenced, > this was no longer true. > This is the simple fix to ignore attributes in the global namespace since we are looking > for ones in the `arm` namespace instead. > > Built and tested for aarch64-linux-gnu. > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc (aarch64_lookup_shared_state_flags): Ignore > attributes in the global namespace. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/config/aarch64/aarch64.cc | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index 27e24ba70ab..3f7bc572edc 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -597,6 +597,10 @@ aarch64_lookup_shared_state_flags (tree attrs, const char *state_name) > if (!cxx11_attribute_p (attr)) > continue; > > + /* Skip the attributes in the global namespace. */ > + if (!TREE_PURPOSE (TREE_PURPOSE (attr))) > + continue; > + > auto ns = IDENTIFIER_POINTER (TREE_PURPOSE (TREE_PURPOSE (attr))); > if (strcmp (ns, "arm") != 0) > continue; Take it or leave it, but I think the whole thing could be just tree ns = get_attribute_namespace (attr); if (!ns || !id_equal (ns, "arm")) continue; You could also use get_attribute_name below for attr_name. Marek
On Wed, Sep 4, 2024 at 2:36 PM Marek Polacek <polacek@redhat.com> wrote: > > On Wed, Sep 04, 2024 at 02:05:21PM -0700, Andrew Pinski wrote: > > The code in aarch64_lookup_shared_state_flags all C++11 attributes on the function type > > had a namespace associated with them. But with the addition of reproducible/unsequenced, > > this was no longer true. > > This is the simple fix to ignore attributes in the global namespace since we are looking > > for ones in the `arm` namespace instead. > > > > Built and tested for aarch64-linux-gnu. > > > > gcc/ChangeLog: > > > > * config/aarch64/aarch64.cc (aarch64_lookup_shared_state_flags): Ignore > > attributes in the global namespace. > > > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > > --- > > gcc/config/aarch64/aarch64.cc | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > > index 27e24ba70ab..3f7bc572edc 100644 > > --- a/gcc/config/aarch64/aarch64.cc > > +++ b/gcc/config/aarch64/aarch64.cc > > @@ -597,6 +597,10 @@ aarch64_lookup_shared_state_flags (tree attrs, const char *state_name) > > if (!cxx11_attribute_p (attr)) > > continue; > > > > + /* Skip the attributes in the global namespace. */ > > + if (!TREE_PURPOSE (TREE_PURPOSE (attr))) > > + continue; > > + > > auto ns = IDENTIFIER_POINTER (TREE_PURPOSE (TREE_PURPOSE (attr))); > > if (strcmp (ns, "arm") != 0) > > continue; > > Take it or leave it, but I think the whole thing could be just > > tree ns = get_attribute_namespace (attr); > if (!ns || !id_equal (ns, "arm")) > continue; Actually I think it could be reduced further down to just: if (!is_attribute_namespace_p ("arm", attr)) continue; In this case. If I get some time this weekend I will submit a patch to clean up this code. Thanks, Andrew Pinski > > You could also use get_attribute_name below for attr_name. > > Marek >
On Wed, Sep 4, 2024 at 2:44 PM Andrew Pinski <pinskia@gmail.com> wrote: > > On Wed, Sep 4, 2024 at 2:36 PM Marek Polacek <polacek@redhat.com> wrote: > > > > On Wed, Sep 04, 2024 at 02:05:21PM -0700, Andrew Pinski wrote: > > > The code in aarch64_lookup_shared_state_flags all C++11 attributes on the function type > > > had a namespace associated with them. But with the addition of reproducible/unsequenced, > > > this was no longer true. > > > This is the simple fix to ignore attributes in the global namespace since we are looking > > > for ones in the `arm` namespace instead. > > > > > > Built and tested for aarch64-linux-gnu. > > > > > > gcc/ChangeLog: > > > > > > * config/aarch64/aarch64.cc (aarch64_lookup_shared_state_flags): Ignore > > > attributes in the global namespace. > > > > > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > > > --- > > > gcc/config/aarch64/aarch64.cc | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > > > index 27e24ba70ab..3f7bc572edc 100644 > > > --- a/gcc/config/aarch64/aarch64.cc > > > +++ b/gcc/config/aarch64/aarch64.cc > > > @@ -597,6 +597,10 @@ aarch64_lookup_shared_state_flags (tree attrs, const char *state_name) > > > if (!cxx11_attribute_p (attr)) > > > continue; > > > > > > + /* Skip the attributes in the global namespace. */ > > > + if (!TREE_PURPOSE (TREE_PURPOSE (attr))) > > > + continue; > > > + > > > auto ns = IDENTIFIER_POINTER (TREE_PURPOSE (TREE_PURPOSE (attr))); > > > if (strcmp (ns, "arm") != 0) > > > continue; > > > > Take it or leave it, but I think the whole thing could be just > > > > tree ns = get_attribute_namespace (attr); > > if (!ns || !id_equal (ns, "arm")) > > continue; > > Actually I think it could be reduced further down to just: > if (!is_attribute_namespace_p ("arm", attr)) > continue; > > In this case. > > If I get some time this weekend I will submit a patch to clean up this code. In the end I did it today. New patch: https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662325.html Thanks again for the suggestion and the review. Thanks, Andrew > > Thanks, > Andrew Pinski > > > > > You could also use get_attribute_name below for attr_name. > > > > Marek > >
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 27e24ba70ab..3f7bc572edc 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -597,6 +597,10 @@ aarch64_lookup_shared_state_flags (tree attrs, const char *state_name) if (!cxx11_attribute_p (attr)) continue; + /* Skip the attributes in the global namespace. */ + if (!TREE_PURPOSE (TREE_PURPOSE (attr))) + continue; + auto ns = IDENTIFIER_POINTER (TREE_PURPOSE (TREE_PURPOSE (attr))); if (strcmp (ns, "arm") != 0) continue;
The code in aarch64_lookup_shared_state_flags all C++11 attributes on the function type had a namespace associated with them. But with the addition of reproducible/unsequenced, this was no longer true. This is the simple fix to ignore attributes in the global namespace since we are looking for ones in the `arm` namespace instead. Built and tested for aarch64-linux-gnu. gcc/ChangeLog: * config/aarch64/aarch64.cc (aarch64_lookup_shared_state_flags): Ignore attributes in the global namespace. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/config/aarch64/aarch64.cc | 4 ++++ 1 file changed, 4 insertions(+)