Bug 71083 - [5/6/7 Regression] Unaligned bit-field address when predictive commoning
Summary: [5/6/7 Regression] Unaligned bit-field address when predictive commoning
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.0
: P2 major
Target Milestone: 5.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-05-12 10:45 UTC by Claudiu Zissulescu
Modified: 2016-08-12 19:32 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.7.4
Known to fail: 4.8.5
Last reconfirmed: 2016-05-12 00:00:00


Attachments
proposed patch (1.30 KB, patch)
2016-08-07 19:52 UTC, Bernd Edlinger
Details | Diff
proposed patch (1.52 KB, patch)
2016-08-08 16:06 UTC, Bernd Edlinger
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Claudiu Zissulescu 2016-05-12 10:45:11 UTC
I've been trying the following simple test case on latest gcc, and it seems to produce unwanted unaligned accesses for bit-fields.

Test cases:

struct lock_chain {
  unsigned int irq_context: 2,
    depth: 6,
    base: 24;
};

struct lock_chain * foo (struct lock_chain *chain)
{
  int i;
  for (i = 0; i < 100; i++)
    {
      chain[i+1].base = chain[i].base;
    }
  return chain;
}

GCC options -O3 (we need predictive commoning to kick in).

The result for ARM:

        .cpu arm926ej-s
        .eabi_attribute 20, 1
        .eabi_attribute 21, 1
        .eabi_attribute 23, 3
        .eabi_attribute 24, 1
        .eabi_attribute 25, 1
        .eabi_attribute 26, 1
        .eabi_attribute 30, 2
        .eabi_attribute 34, 0
        .eabi_attribute 18, 4
        .file   "t01.c"
        .text
        .align  2
        .global foo
        .syntax unified
        .arm
        .fpu softvfp
        .type   foo, %function
foo:
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        @ link register save eliminated.
        ldr     r1, [r0, #1]                    <<<<<<<<<<<< Unaligned access
        add     r3, r0, #4
        lsl     r1, r1, #8
        add     ip, r0, #404
.L2:
        ldrb    r2, [r3]        @ zero_extendqisi2
        orr     r2, r1, r2
        str     r2, [r3], #4
        cmp     r3, ip
        bne     .L2
        bx      lr
        .size   foo, .-foo
        .ident  "GCC: (GNU) 7.0.0 20160502 (experimental)"

Please observe the ldr instruction will access an unaligned address.

I observed this issue for ARC processor, but because the ARM is more popular, I've use it for this case.
Comment 1 Richard Biener 2016-05-12 10:58:41 UTC
Confirmed also on x86_64 (where it is harmless of course):

;; _19 = BIT_FIELD_REF <MEM[(struct lock_chain *)chain_11(D) + 1B], 24, 0>;

(insn 10 9 11 (set (reg:DI 99)
        (zero_extend:DI (mem:QI (plus:DI (reg/v/f:DI 98 [ chain ])
                    (const_int 1 [0x1])) [1 MEM[(struct lock_chain *)chain_11(D) + 1B]+0 S1 A32]))) -1
     (nil))
Comment 2 Richard Biener 2016-05-12 11:00:24 UTC
Conservative patch:

Index: gcc/tree-predcom.c
===================================================================
--- gcc/tree-predcom.c  (revision 236159)
+++ gcc/tree-predcom.c  (working copy)
@@ -1391,9 +1395,10 @@ ref_at_iteration (data_reference_p dr, i
       && DECL_BIT_FIELD (TREE_OPERAND (DR_REF (dr), 1)))
     {
       tree field = TREE_OPERAND (DR_REF (dr), 1);
+      tree type = build_aligned_type (DECL_BIT_FIELD_TYPE (field),
+                                     BITS_PER_UNIT);
       return build3 (BIT_FIELD_REF, TREE_TYPE (DR_REF (dr)),
-                    build2 (MEM_REF, DECL_BIT_FIELD_TYPE (field),
-                            addr, alias_ptr),
+                    build2 (MEM_REF, type, addr, alias_ptr),
                     DECL_SIZE (field), bitsize_zero_node);
     }
   else


better for archs where unaligned matters for code-gen would be to
base the ref off DECL_BIT_FIELD_REPRESENTATIVE (though that doesn't
seem to have meaningful alignment either in this case).
Comment 3 Richard Biener 2016-05-12 11:03:16 UTC
Broken since GCC 4.8, GCC 4.7 doesn't apply predictive commoning here.
Comment 4 Richard Biener 2016-08-03 12:02:45 UTC
GCC 4.9 branch is being closed
Comment 5 Mikael Pettersson 2016-08-07 09:09:03 UTC
Just adding that this test case fails with a SIGBUS on SPARC, using a recent trunk gcc:

foo:
        ld      [%o0+1], %g3

(Checked by adding a main() and calling foo() with a struct lock_chain array.)
Comment 6 Bernd Edlinger 2016-08-07 12:10:18 UTC
just wanted to add that the patch in comment#2
does not fix this similar test case:

struct lock_chain {
  char x;
  unsigned short base;
} __attribute((packed));

struct lock_chain * foo (struct lock_chain *chain)
{
  int i;
  for (i = 0; i < 100; i++)
    {
      chain[i+1].base = chain[i].base;
    }
  return chain;
}

foo:
	@ Function supports interworking.
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	ldrh	r2, [r0, #1] <- unaligned
	add	r1, r0, #300
	and	ip, r2, #255
	add	r1, r1, #3
	lsr	r2, r2, #8
	add	r3, r0, #3
.L2:
	strb	ip, [r3, #1]
	strb	r2, [r3, #2]
	add	r3, r3, #3
	cmp	r3, r1
	bne	.L2
	bx	lr
Comment 7 Bernd Edlinger 2016-08-07 19:52:06 UTC
Created attachment 39068 [details]
proposed patch
Comment 8 Bernd Edlinger 2016-08-08 16:06:20 UTC
Created attachment 39077 [details]
proposed patch

the previous version hit an assertion,
because it can happen that inner == NULL and coff != 0 ...

fixed assertion and survives reg-testing.
Comment 9 Bernd Edlinger 2016-08-11 13:08:00 UTC
Author: edlinger
Date: Thu Aug 11 13:07:29 2016
New Revision: 239362

URL: https://gcc.gnu.org/viewcvs?rev=239362&root=gcc&view=rev
Log:
2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * tree-predcom.c (ref_at_iteration): Correctly align the
        reference type.

testsuite:
2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * gcc.c-torture/execute/pr71083.c: New test.
        * gnat.dg/loop_optimization23.adb: New test.
        * gnat.dg/loop_optimization23_pkg.ads: New test.
        * gnat.dg/loop_optimization23_pkg.adb: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr71083.c
    trunk/gcc/testsuite/gnat.dg/loop_optimization23.adb
    trunk/gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb
    trunk/gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-predcom.c
Comment 10 Bernd Edlinger 2016-08-12 17:07:16 UTC
Author: edlinger
Date: Fri Aug 12 17:06:44 2016
New Revision: 239423

URL: https://gcc.gnu.org/viewcvs?rev=239423&root=gcc&view=rev
Log:
2016-08-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        Backport from mainline
        2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * tree-predcom.c (ref_at_iteration): Correctly align the
        reference type.

testsuite:
2016-08-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        Backport from mainline
        2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * gcc.c-torture/execute/pr71083.c: New test.
        * gnat.dg/loop_optimization23.adb: New test.
        * gnat.dg/loop_optimization23_pkg.ads: New test.
        * gnat.dg/loop_optimization23_pkg.adb: New test.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.c-torture/execute/pr71083.c
    branches/gcc-6-branch/gcc/testsuite/gnat.dg/loop_optimization23.adb
    branches/gcc-6-branch/gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb
    branches/gcc-6-branch/gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
    branches/gcc-6-branch/gcc/tree-predcom.c
Comment 11 Bernd Edlinger 2016-08-12 18:57:02 UTC
Author: edlinger
Date: Fri Aug 12 18:56:30 2016
New Revision: 239424

URL: https://gcc.gnu.org/viewcvs?rev=239424&root=gcc&view=rev
Log:
2016-08-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        Backport from mainline
        2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * tree-predcom.c (ref_at_iteration): Correctly align the
        reference type.

testsuite:
2016-08-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        Backport from mainline
        2016-08-11  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * gcc.c-torture/execute/pr71083.c: New test.
        * gnat.dg/loop_optimization23.adb: New test.
        * gnat.dg/loop_optimization23_pkg.ads: New test.
        * gnat.dg/loop_optimization23_pkg.adb: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.c-torture/execute/pr71083.c
    branches/gcc-5-branch/gcc/testsuite/gnat.dg/loop_optimization23.adb
    branches/gcc-5-branch/gcc/testsuite/gnat.dg/loop_optimization23_pkg.adb
    branches/gcc-5-branch/gcc/testsuite/gnat.dg/loop_optimization23_pkg.ads
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
    branches/gcc-5-branch/gcc/tree-predcom.c
Comment 12 Bernd Edlinger 2016-08-12 19:31:11 UTC
Author: edlinger
Date: Fri Aug 12 19:30:39 2016
New Revision: 239426

URL: https://gcc.gnu.org/viewcvs?rev=239426&root=gcc&view=rev
Log:
2016-08-12  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        PR tree-optimization/71083
        * tree-predcom.c (ref_at_iteration): Use a COMPONENT_REF for the
        bitfield access when possible.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-predcom.c
Comment 13 Bernd Edlinger 2016-08-12 19:32:51 UTC
fixed.