[Bug middle-end/58041] New: Unaligned access to arrays in packed structure

[Bug middle-end/58041] New: Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

            Bug ID: 58041
           Summary: Unaligned access to arrays in packed structure
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bernd.edlinger at hotmail dot de

the attached test case shows unaligned accesses can be generated
on arm architecture, despite the -mno-unaligned-access option.
This does not happen at -O0 and -Og,
but it always happens at at -Os -O1 -O2 and -O3 to name a few.

assembler output for foo shows unaligned opcodes:

foo:
        @ Function supports interworking.
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        @ link register save eliminated.
        add     ip, r0, r1, asl #3
        add     r1, ip, #1
        ldmia   r1, {r0-r1}
        str     r2, [ip, #1]
        str     r3, [ip, #5]
        bx      lr

reproduced with latest trunk:

  $ ../gcc-4.9-20130728/configure --target=arm-eabi
--prefix=/home/ed/gnu/arm-eabi --with-newlib --enable-languages=c,c++
--disable-hosted-libstdcxx --disable-__cxa_atexit

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #1 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Created attachment 30579
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30579&action=edit
test case to show the bug

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[mikpe at it dot uu.se]

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

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #3 from Mikael Pettersson <mikpe at it dot uu.se> ---
Running this program throws loads of alignment exceptions when it's compiled by
gcc-4.9 or gcc-4.8, targeting armv5tel-linux-gnueabi -O2 -marm.  Adding
-mno-unaligned-access makes no difference.

When compiled by gcc-4.7 it runs cleanly without any exceptions.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[mikpe at it dot uu.se]

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

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wschmidt at gcc dot gnu.org

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> ---
Started with Bill Schmidt's PR46556 patch in r190037.  (Author CC:d.)

Comparing the generated code between 190036 and 190037 clearly shows how the
misaligned accesses were wrongly replaced by aligned accesses:

--- pr58041.s-r190036   2013-08-01 13:30:59.264514025 +0200
+++ pr58041.s-r190037   2013-08-01 13:27:38.874840851 +0200
@@ -18,37 +18,11 @@
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        @ link register save eliminated.
-       stmfd   sp!, {r4, r5, r6, r7, r8, r9, r10, fp}
        add     ip, r0, r1, asl #3
-       ldrb    r7, [ip, #2]    @ zero_extendqisi2
-       ldrb    r6, [ip, #6]    @ zero_extendqisi2
-       ldrb    r0, [ip, #1]    @ zero_extendqisi2
-       ldrb    r1, [ip, #5]    @ zero_extendqisi2
-       ldrb    r5, [ip, #3]    @ zero_extendqisi2
-       ldrb    r4, [ip, #7]    @ zero_extendqisi2
-       orr     r0, r0, r7, asl #8
-       orr     r1, r1, r6, asl #8
-       ldrb    r10, [ip, #4]   @ zero_extendqisi2
-       ldrb    r6, [ip, #8]    @ zero_extendqisi2
-       mov     fp, r2, lsr #8
-       orr     r0, r0, r5, asl #16
-       mov     r9, r2, lsr #16
-       mov     r8, r2, lsr #24
-       mov     r7, r3, lsr #8
-       orr     r1, r1, r4, asl #16
-       mov     r5, r3, lsr #16
-       mov     r4, r3, lsr #24
-       strb    fp, [ip, #2]
-       strb    r2, [ip, #1]
-       strb    r9, [ip, #3]
-       strb    r8, [ip, #4]
-       strb    r7, [ip, #6]
-       strb    r3, [ip, #5]
-       strb    r5, [ip, #7]
-       strb    r4, [ip, #8]
-       orr     r0, r0, r10, asl #24
-       orr     r1, r1, r6, asl #24
-       ldmfd   sp!, {r4, r5, r6, r7, r8, r9, r10, fp}
+       ldr     r0, [ip, #1]
+       ldr     r1, [ip, #5]
+       str     r2, [ip, #1]
+       str     r3, [ip, #5]
        bx      lr
        .size   foo, .-foo
        .section        .text.startup,"ax",%progbits

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[mikpe at it dot uu.se]

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

--- Comment #5 from Mikael Pettersson <mikpe at it dot uu.se> ---
I see the exact same failure pattern on sparc64-linux: 4.7 generates working
code, 4.8 and 4.9 generate code that SIGBUS:es, failure starts with r190037, 
-m32 or -m64 makes no difference.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #6 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
I'll investigate.  It may be a day or two before I can get to it, but this is
pretty clearly my issue.

Thanks,
Bill

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

Bill Schmidt <wschmidt at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2013-08-01
           Assignee|unassigned at gcc dot gnu.org      |wschmidt at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #7 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
This shouldn't be too hard to fix.  Looks like we are missing a check for
possibly unaligned data when STRICT_ALIGNMENT is set.  The logic for unaligned
data can be exposed from a static routine in tree-ssa-ivopts.c.  I'll work up a
trial patch.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #8 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I believe that you need to set alignment of the type of MEM_REFs you
create in replace_ref along the lines it is done in
build_ref_for_offset in tree-sra.c.

I wonder whether STRICT_ALIGNMENT has really any meaning nowadays,
given that for example x86_64 is not a strict-alignment platform
except for SSE vectors.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> ---
More specifically, if I am correct assuming that the MEM_REF
replace_ref builds always accesses exactly the same memory as the
previous access *expr does (and only the address is computed better)
and that *expr is how the function accessed that memory before, then I
think what you need is the following (untested, except that I know it
seems to fix the testcase, at least the assembly looks very different
:-)

*** /tmp/heQHTs_gimple-ssa-strength-reduction.c Thu Aug  1 18:43:45 2013
--- gcc/gimple-ssa-strength-reduction.c Thu Aug  1 18:38:31 2013
*************** dump_incr_vec (void)
*** 1728,1738 ****
  static void
  replace_ref (tree *expr, slsr_cand_t c)
  {
!   tree add_expr = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (c->base_expr),
!                              c->base_expr, c->stride);
!   tree mem_ref = fold_build2 (MEM_REF, TREE_TYPE (*expr), add_expr,
!                             double_int_to_tree (c->cand_type, c->index));
!   
    /* Gimplify the base addressing expression for the new MEM_REF tree.  */
    gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
    TREE_OPERAND (mem_ref, 0)
--- 1728,1748 ----
  static void
  replace_ref (tree *expr, slsr_cand_t c)
  {
!   tree add_expr, mem_ref, acc_type = TREE_TYPE (*expr);
!   unsigned HOST_WIDE_INT misalign;
!   unsigned align;
! 
!   get_object_alignment_1 (*expr, &align, &misalign);
!   if (misalign != 0)
!     align = (misalign & -misalign);
!   if (align < TYPE_ALIGN (acc_type))
!     acc_type = build_aligned_type (acc_type, align);
! 
!   add_expr = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (c->base_expr),
!                         c->base_expr, c->stride);
!   mem_ref = fold_build2 (MEM_REF, acc_type, add_expr,
!                        double_int_to_tree (c->cand_type, c->index));
! 
    /* Gimplify the base addressing expression for the new MEM_REF tree.  */
    gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
    TREE_OPERAND (mem_ref, 0)

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 30587
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30587&action=edit
x86_64-linux testcase

To prove the point, this is an x86_64-linux testcase.  I will
bootstrap and test the patch overnight and if it passes, I will submit
it to the mailing list.

Still, please confirm my assumptions outlined above are correct.
Thanks.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #11 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Hi Martin,

Your assumptions are correct, but I'm not sure this is the best place to handle
it.  It looks like what you are doing is replacing one already correct memory
reference with another, both of which will generate somewhat nasty code. 
Therefore there isn't much reason to do the transformation at all in the first
place.  I think I would rather analyze the reference when considering adding
the reference to the candidate table, and leaving it out of consideration
altogether.  What do you think?

For example, I'm looking at adding the following ahead of the call to
restructure_reference in slsr_process_ref:

  /* If this reference doesn't meet alignment restrictions, don't
     make it a candidate.  Logic similar to that in tree-ssa-loop-ivopts.c:
     may_be_unaligned_p(), without the STEP check.  */
  if (mode != BLKmode)
    {
      tree base_type = TREE_TYPE (base);
      unsigned base_align = get_object_alignment (base);
      unsigned mode_align = GET_MODE_ALIGNMENT (mode);
      base_align = MAX (base_align, TYPE_ALIGN (base_type));

      if (base_align < mode_align
      || (bitpos % mode_align) != 0
      || (bitpos % BITS_PER_UNIT) != 0)
    return;

      if (offset
      && (highest_pow2_factor (offset) * BITS_PER_UNIT) < mode_align)
    return;
    }

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #12 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
...which apparently is not quite right, since the candidates still appear in
the table.  Hm.  But you get the idea -- do the check earlier.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #13 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Hi,

just one question, how about the -m[no-]unaligned-access option?

If -munaligned-access had been given the code was almost right,
I mean AFAIK ldr/str should be handled in hardware but ldmia generates
an alignment exception and _may_ be emulated by an IRQ handler,
but that would not be very efficient.

When -mno-unaligned-access is given any ldr/str on unaligned
addresses have to be avoided.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #14 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #13)
> Hi,
> 
> just one question, how about the -m[no-]unaligned-access option?
> 
> If -munaligned-access had been given the code was almost right,
> I mean AFAIK ldr/str should be handled in hardware but ldmia generates
> an alignment exception and _may_ be emulated by an IRQ handler,
> but that would not be very efficient.
> 
> When -mno-unaligned-access is given any ldr/str on unaligned
> addresses have to be avoided.

Well, unfortunately -mno-unaligned-access does not provide any information to
the middle end.  It's all handled in the ARM back end.  So without directly
checking an ARM-specific option in the middle-end (evil), we don't have a good
solution for that.  That's how I initially started looking at STRICT_ALIGNMENT,
which ARM and Sparc have in common.

However, since this is supposed to be an optimization and it is common for
misaligned memory accesses to suffer a penalty, I think it is better just to
not "optimize" when the memory access is misaligned, and leave it to the target
back ends to do their normal cleanups and workarounds.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #15 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Bernd, Mikael, Martin:  Could you please test this on your respective targets?

Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c    (revision 201413)
+++ gcc/gimple-ssa-strength-reduction.c    (working copy)
@@ -845,6 +845,8 @@ slsr_process_ref (gimple gs)
   int unsignedp, volatilep;
   double_int index;
   slsr_cand_t c;
+  unsigned HOST_WIDE_INT misalign;
+  unsigned align;

   if (gimple_vdef (gs))
     ref_expr = gimple_assign_lhs (gs);
@@ -857,6 +859,16 @@ slsr_process_ref (gimple gs)
       && DECL_BIT_FIELD (TREE_OPERAND (ref_expr, 1))))
     return;

+  /* If this reference doesn't meet alignment restrictions, don't
+     make it a candidate.  */
+  get_object_alignment_1 (ref_expr, &align, &misalign);
+
+  if (misalign != 0)
+    align = (misalign & -misalign);
+
+  if (align < TYPE_ALIGN (TREE_TYPE (ref_expr)))
+    return;
+
   base = get_inner_reference (ref_expr, &bitsize, &bitpos, &offset, &mode,
                   &unsignedp, &volatilep, false);
   index = double_int::from_uhwi (bitpos);

Thanks,
Bill

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Bill Schmidt from comment #15)
> Bernd, Mikael, Martin:  Could you please test this on your respective
> targets?

Congatulations!

it works.

If I compile with -mno-unaligned-access all accesses are
ldrb/strb as it should be.
And if I compile with -mcpu=cortex-a9 -munaligned-access
the code is also OK, no ldmia's any more. The back end seems to
have fixed that for us.

foo:
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        @ link register save eliminated.
        add     ip, r0, r1, asl #3
        ldr     r0, [ip, #1]    @ unaligned
        ldr     r1, [ip, #5]    @ unaligned
        str     r2, [ip, #1]    @ unaligned
        str     r3, [ip, #5]    @ unaligned
        bx      lr

which is perfectly OK for cortex-a9.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #17 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Excellent!  Thanks for the confirmation.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[mikpe at it dot uu.se]

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

--- Comment #18 from Mikael Pettersson <mikpe at it dot uu.se> ---
(In reply to Bill Schmidt from comment #15)
> Bernd, Mikael, Martin:  Could you please test this on your respective
> targets?

This patch eliminates the misalignment faults for me on both ARMv5TE and SPARC.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #19 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Bill Schmidt from comment #15)
> Bernd, Mikael, Martin:  Could you please test this on your respective
> targets?

Well, "my target" is x86_64 but yes, it works.  

(In reply to Bill Schmidt from comment #11)
> Hi Martin,
> 
> Your assumptions are correct, but I'm not sure this is the best place to
> handle it.  It looks like what you are doing is replacing one already
> correct memory reference with another, both of which will generate somewhat
> nasty code.  Therefore there isn't much reason to do the transformation at
> all in the first place.  I think I would rather analyze the reference when
> considering adding the reference to the candidate table, and leaving it out
> of consideration altogether.  What do you think?

I don't know, at least in theory the optimization might help somewhat
anyway, especially on targets that can handle misalignment memory
accesses.  But you are right that generally misaligned access will be
slow either way.

Anyway, I don't really care, I assume you contributed the code so you
are more qualified to make a judgment and if you prefer one way over
the other, go for it.

I'll leave it to you and won't submit any patch then.  Please make
sure that the two testcases are added to the testsuite before you
close the bug.  The x86_64-linux tetcase from comment #10 is generic
enough that it can go to gcc.dg/torture/, the original ARM one needs
to go to some arm-specific place.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #20 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
After thinking it over some more, I think you are right, Martin.  We should go
ahead with the optimization with the corrected alignment attached to the type. 
Please go ahead with your patch.  I will run a round of regression testing on
PowerPC (an architecture for which the generic test produces misaligned but
legal memory references) as well.

Sorry for going back and forth on this.  I try to avoid wasting compile time on
useless transformations, but in this case we will still see some benefit in
some cases, and the code should be no worse than before when we don't.

Thanks,
Bill

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #21 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
My only comment on the patch would be to please add commentary indicating why
the change is being made, and referencing this PR.  Something along the lines
of:

  /* Ensure the memory reference carries the minimum alignment
     requirement for the data type.  Some targets (e.g., ARM)
     can't always handle an unaligned reference otherwise.  See PR58041.  */

...or something like that.

Thanks again!

Bill

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #22 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> After thinking it over some more, I think you are right, Martin.  We should
> go ahead with the optimization with the corrected alignment attached to the
> type.  Please go ahead with your patch.  I will run a round of regression
> testing on PowerPC (an architecture for which the generic test produces
> misaligned but legal memory references) as well.
> 
> Sorry for going back and forth on this.  I try to avoid wasting compile time
> on useless transformations, but in this case we will still see some benefit
> in some cases, and the code should be no worse than before when we don't.

We should be very wary of generating unaligned accesses during optimization for
targets that define SLOW_UNALIGNED_ACCESS.  And note that most architectures
supported by GCC are STRICT_ALIGNMENT, not the other way around, so it's the
common case.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #23 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #22)
> We should be very wary of generating unaligned accesses during optimization
> for targets that define SLOW_UNALIGNED_ACCESS.  And note that most
> architectures supported by GCC are STRICT_ALIGNMENT, not the other way
> around, so it's the common case.

I fully agree.  In this case, we aren't introducing new unaligned accesses, but
simply restructuring the address that points to the same (unaligned) location. 
The restructuring allows us to common the base addressing for related array
elements in the same misaligned structure, which will produce slightly better
code with the same number of unaligned accesses.  Martin's patch just makes
sure the necessary alignment is recorded on the restructured memory reference,
so the back ends can do their usual tricks to copy misaligned references in
parts, etc.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|wschmidt at gcc dot gnu.org        |jamborm at gcc dot gnu.org

--- Comment #24 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Created attachment 30594
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30594&action=edit
Proposed patch

(In reply to Bill Schmidt from comment #20)
> After thinking it over some more, I think you are right, Martin.  We should
> go ahead with the optimization with the corrected alignment attached to the
> type.  Please go ahead with your patch.  I will run a round of regression
> testing on PowerPC (an architecture for which the generic test produces
> misaligned but legal memory references) as well.
> 
> Sorry for going back and forth on this.  I try to avoid wasting compile time
> on useless transformations, but in this case we will still see some benefit
> in some cases, and the code should be no worse than before when we don't.
> 

No worries, I'm currently bootstrapping and testing the attached
patch.  I'm bootstrapping on x86_64-linux and with bugs like this one,
any additional testing on other platforms is very welcome.

(In reply to Bill Schmidt from comment #21)
> My only comment on the patch would be to please add commentary indicating
> why the change is being made, and referencing this PR.  Something along the
> lines of:
> 
>   /* Ensure the memory reference carries the minimum alignment
>      requirement for the data type.  Some targets (e.g., ARM)
>      can't always handle an unaligned reference otherwise.  See PR58041.  */
> 
> ...or something like that.
> 

The alignment information should be there regardless of the target so
I just used the first sentence and the PR reference.  I hope that is
enough.  Thanks.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #25 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Yep, that's terrific.  Thanks.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #26 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Martin's patch bootstrapped on powerpc64-unknown-linux-gnu with no new
regressions.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #27 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Martin Jambor from comment #24)
> Created attachment 30594 [details]
> Proposed patch

I think it would be safe to put my initial test case
under gcc/testsuite/gcc.target/arm/pr58041.c

It passes with your patch at least in my environment.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #28 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Thanks, for testing, I have submitted the patch for a review:

http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00224.html

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2013-08/msg00224.htm
                   |                            |l

--- Comment #29 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Richi approved the patch, I've committed it to trunk.  I'm about to
bootstrap and test n the 4.8 branch.


Author: jamborm
Date: Tue Aug  6 09:22:16 2013
New Revision: 201523

URL: http://gcc.gnu.org/viewcvs?rev=201523&root=gcc&view=rev
Log:
2013-08-06  Martin Jambor  <mjambor@suse.cz>

    PR middle-end/58041
    * gimple-ssa-strength-reduction.c (replace_ref): Make sure built
    MEM_REF has proper alignment information.

testsuite/
    * gcc.dg/torture/pr58041.c: New test.
    * gcc.target/arm/pr58041.c: Likewise.


Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr58041.c
    trunk/gcc/testsuite/gcc.target/arm/pr58041.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-ssa-strength-reduction.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #30 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Hi Martin,

I have bootstrapped this patch for i686-pc-linux-gnu and have
seen some "excess errors" in your test script:

/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c: In
function 'foo':
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
note: The ABI for passing parameters with 16-byte alignment has changed in GCC
4.6
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]
output is:
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c: In
function 'foo':
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
note: The ABI for passing parameters with 16-byte alignment has changed in GCC
4.6
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]

FAIL: gcc.dg/torture/pr58041.c  -O0  (test for excess errors)
Excess errors:
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]
/home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
warning: SSE vector argument without SSE enabled changes the ABI [enabled by
default]

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #31 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #30)
> Hi Martin,
> 
> I have bootstrapped this patch for i686-pc-linux-gnu and have
> seen some "excess errors" in your test script:
> 
> /home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c: In
> function 'foo':
> /home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
> note: The ABI for passing parameters with 16-byte alignment has changed in
> GCC 4.6
> /home/ed/gnu/gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c:15:11:
> warning: SSE vector argument without SSE enabled changes the ABI [enabled by
> default]

I can't reproduce this with the -m32 flag on my x86_64... do you still
have the compiler built on an i686?  If so, could you try and make
function foo static in that testcase and see if the error goes away?
Thanks!

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #32 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Author: jamborm
Date: Tue Aug  6 15:08:59 2013
New Revision: 201530

URL: http://gcc.gnu.org/viewcvs?rev=201530&root=gcc&view=rev
Log:
2013-08-06  Martin Jambor  <mjambor@suse.cz>

    PR middle-end/58041
    * gimple-ssa-strength-reduction.c (replace_ref): Make sure built
    MEM_REF has proper alignment information.

testsuite/
    * gcc.dg/torture/pr58041.c: New test.
    * gcc.target/arm/pr58041.c: Likewise.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr58041.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/arm/pr58041.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/gimple-ssa-strength-reduction.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #33 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Martin Jambor from comment #31)
> I can't reproduce this with the -m32 flag on my x86_64... do
> you still have the compiler built on an i686?  If so, could you try and make
> function foo static in that testcase and see if the error goes away?

static does not help.
If I add -msse the warning goes away, but the compiled executable
crashes because of illegal instruction.

Dual Pentium II, with mmx but obviously no sse, whatever that may be:
flags: fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pse36 mmx fxsr

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #34 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
by the way the initializer of "struct s a = "
seems to generate warnings at -Wall, because some brackets are missing:

changed that to
struct s a = {0,{{0,0},{0,0}}};

but somehow I wonder what forced us to generate sse instructions here?
when that same example works on a ARMv5 targe?

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #35 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #34)
> by the way the initializer of "struct s a = "
> seems to generate warnings at -Wall, because some brackets are missing:
> 
> changed that to
> struct s a = {0,{{0,0},{0,0}}};
> 
> but somehow I wonder what forced us to generate sse instructions here?
> when that same example works on a ARMv5 targe?

Strange, does the correct initializer make the warning go away?
If so, I'll fix it in the testsuite in a moment.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #36 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Martin Jambor from comment #35)
> (In reply to Bernd Edlinger from comment #34)
> by the way the initializer
> of "struct s a = "
> seems to generate warnings at -Wall, because some
> brackets are missing:
> 
> changed that to
> struct s a = {0,{{0,0},{0,0}}};
> > 
> but somehow I wonder what forced us to generate sse instructions here?
> > when that same example works on a ARMv5 targe?

> Strange, does the correct
> initializer make the warning go away?
> If so, I'll fix it in the testsuite in a moment.

no that is just a different warning with -Wall, that one did not
make the test case fail however.

and in line 6 the "typedef struct S { V v; } P __attribute__((aligned (1)));"
is superfluos too.

hmm, maybe the problem is I should not say -msse in the first place.

do you get the warning if you use -m32 -mno-sse ?

what's funny about that warning, that it does not need to be enabled with
-Wall like the other warning.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #37 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
this version fixes the warning:

--- ../gcc-4.9-20130728/gcc/testsuite/gcc.dg/torture/pr58041.c  2013-08-02
20:59:38.000000000 +0200
+++ pr58041.c   2013-08-06 18:30:51.000000000 +0200
@@ -3,8 +3,6 @@
 typedef long long V
   __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));

-typedef struct S { V v; } P __attribute__((aligned (1)));
-
 struct s
 {
   char u;
@@ -12,24 +10,24 @@
 } __attribute__((packed,aligned(1)));

 __attribute__((noinline, noclone))
-long long foo(struct s *x, int y, V z)
+long long foo(struct s *x, int y, V *z)
 {
   V a = x->v[y];
-  x->v[y] = z;
+  x->v[y] = *z;
   return a[1];
 }

-struct s a = {0,{0,0}};
+struct s a = {0,{{0,0},{0,0}}};
 int main()
 {
   V v1 = {0,1};
   V v2 = {0,2};

-  if (foo(&a,0,v1) != 0)
+  if (foo(&a,0,&v1) != 0)
     __builtin_abort();
-  if (foo(&a,0,v2) != 1)
+  if (foo(&a,0,&v2) != 1)
     __builtin_abort();
-  if (foo(&a,1,v1) != 0)
+  if (foo(&a,1,&v1) != 0)
     __builtin_abort();
   return 0;
 }

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

--- Comment #38 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #37)
> this version fixes the warning:

And I confirm that it still tests the bug.  If you want to commit it
yourself, go ahead, otherwise let me now and I'll do it before I leave
today.  Thanks a lot!

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[bernd.edlinger at hotmail dot de]

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

--- Comment #39 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Martin Jambor from comment #38)
>> (In reply to Bernd Edlinger from comment #37)
>> this version fixes the warning:
> And I confirm that it still tests the bug.  If you want to commit
> it yourself, go ahead, otherwise let me now and I'll do it before I leave
> today.  Thanks a lot!

no thanks, just go ahead.

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #40 from Martin Jambor <jamborm at gcc dot gnu.org> ---
OK, I have updated the testcase on both branches.  So I hope all is
well now and we can close this PR.  Thanks everyone for cooperation.


Author: jamborm
Date: Tue Aug  6 17:33:59 2013
New Revision: 201538

URL: http://gcc.gnu.org/viewcvs?rev=201538&root=gcc&view=rev
Log:
2013-08-06  Martin Jambor  <mjambor@suse.cz>
        Bernd Edlinger <bernd.edlinger@hotmail.de>

testsuite/
    * gcc.dg/torture/pr58041.c (foo): Accept z by reference.
    (a): Fix constructor.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/torture/pr58041.c



Author: jamborm
Date: Tue Aug  6 17:35:11 2013
New Revision: 201539

URL: http://gcc.gnu.org/viewcvs?rev=201539&root=gcc&view=rev
Log:
2013-08-06  Martin Jambor  <mjambor@suse.cz>
        Bernd Edlinger <bernd.edlinger@hotmail.de>

testsuite/
    * gcc.dg/torture/pr58041.c (foo): Accept z by reference.
    (a): Fix constructor.


Modified:
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/torture/pr58041.c

[Bug middle-end/58041] Unaligned access to arrays in packed structure

[wschmidt at gcc dot gnu.org]

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

--- Comment #41 from Bill Schmidt <wschmidt at gcc dot gnu.org> ---
Thanks, Martin!