[Bug c/55658] New: bitfields and __attribute__((packed)) generate horrible code on x86_64

[Bug c/55658] New: bitfields and __attribute__((packed)) generate horrible code on x86_64

[tudorb at fb dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

             Bug #: 55658
           Summary: bitfields and __attribute__((packed)) generate
                    horrible code on x86_64
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: tudorb@fb.com


Created attachment 28932
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28932
Simple source code to exhibit the problem; disassembly (as produced by objdump
-d)

I have a structure defined as:

struct D {
  uint64_t id_ : 63;
  bool x_ : 1;
} __attribute__((packed));

uint64_t D_id(const struct D* p) { return p->id_; }

The generated code for D_id (using gcc 4.7.1 -O2 on Linux x86_64) builds the
return value byte by byte and masks out the most significant bit from the last
byte.

Removing __attribute__((packed)) and/or changing the structure so it is no
longer a bitfield generates sane code.

This happens in both C and C++ (unsurprisingly).

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
          Component|c                           |middle-end

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-12-12 06:02:35 UTC ---
Confirmed.

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-12-12
     Ever Confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-12-12 06:11:23 UTC ---
Happens on mips64 also.

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |!SLOW_UNALIGNED_ACCESS
      Known to fail|                            |3.3.6, 4.1.2, 4.5.3, 4.6.4,
                   |                            |4.7.2, 4.8.0
           Severity|normal                      |enhancement

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> 2012-12-12 10:00:33 UTC ---
Confirmed for !SLOW_UNALIGNED_ACCESS targets.  At least at -Os it should use
an unaligned movq and mask out bits not needed.

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Mine, I am going to look into this for GCC 14 (seperately from the bitfield
lower since it also still happens with that pass).

That pass does give some extra information though on this (a slightly different
but similar enough testcase):
```
Trying to expand bitfield reference:
a_3(D)->la
base: *a_3(D) orig bitpos: 0 bytepos: 0
after bit_range bitpos: 0 bytepos: 0 bitregion_start: 0 bitregion_end: 63
align: 8 word_size: 64.
failed, get_best_mode return false.
```

So basically get_best_mode failed with the above arguments ...

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rogero at howzatt dot co.uk

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 66364 has been marked as a duplicate of this bug. ***

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
         /* ??? For historical reasons, reject modes that would normally
            receive greater alignment, even if unaligned accesses are
            acceptable.  This has both advantages and disadvantages.
            Removing this check means that something like:
....

[Bug middle-end/55658] bitfields and __attribute__((packed)) generate horrible code on x86_64

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55658

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
diff --git a/gcc/stor-layout.cc b/gcc/stor-layout.cc
index 45bf2d18639..b44335dd406 100644
--- a/gcc/stor-layout.cc
+++ b/gcc/stor-layout.cc
@@ -3089,7 +3089,8 @@ get_best_mode (int bitsize, int bitpos,
            causes store_bit_field to keep a 128-bit memory reference,
            so that the final bitfield reference still has a MEM_EXPR
            and MEM_OFFSET.  */
-        && GET_MODE_ALIGNMENT (mode) <= align
+        && (GET_MODE_ALIGNMENT (mode) <= align
+            || (!STRICT_ALIGNMENT && currently_expanding_to_rtl))
         && GET_MODE_BITSIZE (mode) <= largest_mode_bitsize)
     {
       *best_mode = mode;


Should fix it, I still need to update the comment since it does say the check
should be disable while expanding and that is what the above does ...

To get rid of the check the rest of the way is remove part of fold which does
the comparison combing which I think might be too early to that ...