Hi Jakub,

On 9/22/21 11:33 AM, Jakub Jelinek wrote:
On Wed, Sep 22, 2021 at 05:02:15PM +0200, Jakub Jelinek via Gcc-patches wrote:
@@ -6298,7 +6298,8 @@ rs6000_aggregate_candidate (const_tree type, machine_mode 
*modep,
          return -1;
        count = rs6000_aggregate_candidate (TREE_TYPE (type), modep,
-                                           empty_base_seen);
+                                           empty_base_seen,
+                                           zero_width_bf_seen);
        if (count == -1
            || !index
            || !TYPE_MAX_VALUE (index)
@@ -6336,6 +6337,12 @@ rs6000_aggregate_candidate (const_tree type, 
machine_mode *modep,
            if (TREE_CODE (field) != FIELD_DECL)
              continue;
+           if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
+             {
+               *zero_width_bf_seen = 1;
+               continue;
+             }
So, from what you wrote, :0 in the ppc* psABIs the intent is that :0 is not
ignored, right?
In that case I don't really understand the above (the continue in
particular).  Because the continue means it is ignored for C++ and not
ignored for C, so basically you return to the 4.5-11 ABI incompatibility
between C and C++.
C++ :0 will have DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD set, C :0 will not...
To be more precise, I'd expect what most targets want to do for the
actual ABI decisions not to use DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD at all.
I.e. do:
   if (TREE_CODE (field) != FIELD_DECL)
     continue;
   if (DECL_BIT_FIELD (field) && integer_zerop (DECL_SIZE (field)))
     {
       // :0
       // in some psABIs, ignore it, i.e. continue;
       // in others psABIs, take them into account, i.e. do nothing.
     }
and use DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD only for the -Wpsabi purposes.

The only exception would be for targets that decide to keep GCC 4.5-11
compatibility with the C incompatible with C++.

I think you're misunderstanding what I'm trying to do with this patch.  I am not changing the code generation at all (your patch did that).  All I'm doing is detecting when the old code generation and new code generation will differ, and emitting a diagnostic in that case.

The way I do that is to allow rs6000_aggregate_candidate to *think* that something is a homogeneous aggregate even when zero-width bitfields are present (hence the continue), but record the fact that we saw one.  This gives the same answer as we gave before your patch.  Then, in rs6000_discover_homogeneous_aggregate, once we think we have a homogeneous aggregate, we check whether we actually had a zero-width bitfield present.  If so, then we diagnose the change in code generation and return false, indicating that we didn't actually find a homogeneous aggregate.  Other than the diagnostic, this matches the behavior after your patch.

I've verified that we didn't change code generation for C code with zero-width bitfields as a result of either your patch or mine. Before and after, a C struct containing a zero-width bitfield causes us to avoid generating code for a homogeneous aggregate, just as we do for C++ after your patch.

I hope this helps clear things up, and I apologize for not giving a better description of my intent.

Thanks!
Bill

        Jakub


Reply via email to