[Bug c/107389] New: Alignment not inferred from type at -O0

[Bug c/107389] New: Alignment not inferred from type at -O0

[rth at gcc dot gnu.org]

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

            Bug ID: 107389
           Summary: Alignment not inferred from type at -O0
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rth at gcc dot gnu.org
  Target Milestone: ---

Consider

typedef __uint128_t aligned_type __attribute__((aligned(16)));
_Static_assert(__alignof(aligned_type) == 16);
__uint128_t foo(aligned_type *p) { return __atomic_load_n(p, 0); }

For s390x, atomic_loadti should expand this to LPQ.

For my purposes, it must also do this at -O0, not just with
optimization.  But the alignment seen by gen_atomic_loadti
is only 8, so it FAILs the expansion and falls back to libatomic.

The following appears to solve the problem:

--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -468,8 +468,11 @@ get_pointer_alignment_1
        }
       else
        {
+         /* Assume alignment from the type. */
+         tree ptr_type = TREE_TYPE (exp);
+         tree obj_type = TREE_TYPE (ptr_type);
+         *alignp = TYPE_ALIGN (obj_type);
          *bitposp = 0;
-         *alignp = BITS_PER_UNIT;
          return false;
        }
     }

but I have an inkling that would have undesired effects
for other usages.  If so, perhaps a special case could be
made for the usage in get_builtin_sync_mem.

[Bug c/107389] Alignment not inferred from type at -O0

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yes, that's definitely a no-go since pointer types have no semantic meanings.

I don't understand s390 assembly much but I see calls to __atomic_load_16@PLT
for both -O0 and -O2 so I wonder if it is the testcase you are seeing a
difference for?

If __atomic_load_n (and others) imply certain alignment of the pointer
arguments then their expanders have to explicitely add that.  But I guess
there's no requirement that __builtin_atomic_load (p, 16) has p 16 byte
aligned?

There was talks to "rectify" specified type alignment for pointers in
function arguments but IIRC that has never materialized.  See duplicate
bugreports about memcpy involving properly align-typed function parameters.

That said, iff __atomic_* constraints are not exploited we should do that.

[Bug c/107389] Alignment not inferred from type at -O0

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Btw, with

typedef __uint128_t aligned_type __attribute__((aligned(16)));
_Static_assert(__alignof(aligned_type) == 16);
__uint128_t foo(aligned_type *p) { p = __builtin_assume_aligned (p, 16); return
__atomic_load_n(p, 0); }

I see

foo:
.LFB0:
        .cfi_startproc
        lpq     %r4,0(%r3)
        stmg    %r4,%r5,0(%r2)
        br      %r14

at -O2 but without the __builtin_assume_aligned optimization doesn't help much.
And without optimization but __builtin_assume_aligned in place we simply
leave that around - we probably should have elided it in
fold-all-builtins and set alignment on the destination SSA name also
when not optimizing (we do that there when optimizing), or do the same
during RTL expansion.

[Bug c/107389] Alignment not inferred from type at -O0

[rth at gcc dot gnu.org]

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

--- Comment #3 from Richard Henderson <rth at gcc dot gnu.org> ---
If __builtin_assume_aligned were to work at -O0,
that would also work for me.  Better, probably,
than fiddling with __attribute__((aligned)).

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[rth at gcc dot gnu.org]

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

Richard Henderson <rth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
            Summary|Alignment not inferred from |Always propagate
                   |type at -O0                 |__builtin_assume_aligned
            Version|unknown                     |12.2.1
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-10-26
           Severity|normal                      |enhancement

--- Comment #4 from Richard Henderson <rth at gcc dot gnu.org> ---
Rename and re-categorise as enhancement.

As mentioned, in comments above, need to
be able to rely on __builtin_assume_aligned
even with -O0 to avoid link errors when
not including libatomic.

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[rguenth at gcc dot gnu.org]

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

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

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

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 53784
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53784&action=edit
prototype

This implements an -O0 fold-builtins pass.  I've disabled some but not all
"optimizations" and instead of just throwing away __builtin_assume_aligned
I'm processing it at -O0 (the machinery from CCP relies on a lattice, with
optimization we should at least merge with the alignment info on the LHS).

On s390 I then see

foo:
.LFB0:
        .cfi_startproc
        stmg    %r11,%r15,88(%r15)
        .cfi_offset 11, -72
        .cfi_offset 12, -64
        .cfi_offset 13, -56
        .cfi_offset 14, -48
        .cfi_offset 15, -40
        aghi    %r15,-176
        .cfi_def_cfa_offset 336
        lgr     %r11,%r15
        .cfi_def_cfa_register 11
        stg     %r2,168(%r11)
        stg     %r3,160(%r11)
        lg      %r1,160(%r11)
        lpq     %r2,0(%r1)
        lg      %r1,168(%r11)
        stmg    %r2,%r3,0(%r1)
        lg      %r2,168(%r11)
        lmg     %r11,%r15,264(%r11)
        .cfi_restore 15
        .cfi_restore 14
        .cfi_restore 13
        .cfi_restore 12
        .cfi_restore 11
        .cfi_def_cfa 15, 160
        br      %r14
        .cfi_endproc

specifically I did not disable __atomic_add_fetch_* optimizations to
.ATOMIC_ADD_FETCH_CMP_0 and friends and also kept optimizing
stack_save/restore.

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:7899582a059a9d8c25bfff305cd236d219dc4f20

commit r13-3810-g7899582a059a9d8c25bfff305cd236d219dc4f20
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Nov 8 13:49:16 2022 +0100

    tree-optimization/107389 - honor __builtin_assume_alignment at -O0

    The following makes sure to set alignment information on the LHS
    of __builtin_assume_alignment calls even when not optimizing so
    uses as arguments to builtin functions like memcpy or __atomic_load_n
    can be reflected at RTL expansion time.

            PR tree-optimization/107389
            * gimple-low.cc (lower_builtin_assume_aligned): New.
            (lower_stmt): Call it.

            * gcc.dg/pr107389.c: New testcase.

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |13.0

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for GCC 13.

[Bug middle-end/107389] Always propagate __builtin_assume_aligned

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0