On Tue, Jun 23, 2020 at 5:31 PM Sunil K Pandey via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Sunil K Pandey <skpg...@gmail.com>
>
> Default for this hook is NOP. For x86, in 32 bit mode, this hook
> sets alignment of long long on stack to 32 bits if preferred stack
> boundary is 32 bits.
>
> - This patch fixes
> gcc.target/i386/pr69454-2.c
> gcc.target/i386/stackalign/longlong-1.c
> - Regression test on x86-64, no new fail introduced.
I think the name is badly chosen, TARGET_LOWER_LOCAL_DECL_ALIGNMENT
would be better suited (and then asks for LOCAL_DECL_ALIGNMENT to be
renamed to INCREASE_LOCAL_DECL_ALIGNMENT).
You're calling it from do_type_align which IMHO is dangerous since that's
invoked from FIELD_DECL layout as well. Instead invoke it from
layout_decl itself where we do
if (code != FIELD_DECL)
/* For non-fields, update the alignment from the type. */
do_type_align (type, decl);
and invoke the hook _after_ do_type_align. Also avoid
invoking the hook on globals or hard regs and only
invoke it on VAR_DECLs, thus only
if (VAR_P (decl) && !is_global_var (decl) && !DECL_HARD_REGISTER (decl))
Comments on the hook itself below.
> Tested on x86-64.
>
> gcc/ChangeLog:
>
> PR target/95237
> * config/i386/i386.c (ix86_update_decl_alignment): New
> function.
> (TARGET_UPDATE_DECL_ALIGNMENT): Define.
> * doc/tm.texi: Regenerate.
> * doc/tm.texi.in (TARGET_UPDATE_DECL_ALIGNMENT): New hook.
> * stor-layout.c (do_type_align): Call target hook to update
> decl alignment.
> * target.def (update_decl_alignment): New hook.
>
> gcc/testsuite/ChangeLog:
>
> PR target/95237
> * gcc.target/i386/pr95237-1.c: New test.
> * gcc.target/i386/pr95237-2.c: New test.
> * gcc.target/i386/pr95237-3.c: New test.
> * gcc.target/i386/pr95237-4.c: New test.
> * gcc.target/i386/pr95237-5.c: New test.
> ---
> gcc/config/i386/i386.c | 22 ++++++++++++++++++++++
> gcc/doc/tm.texi | 5 +++++
> gcc/doc/tm.texi.in | 2 ++
> gcc/stor-layout.c | 2 ++
> gcc/target.def | 7 +++++++
> gcc/testsuite/gcc.target/i386/pr95237-1.c | 16 ++++++++++++++++
> gcc/testsuite/gcc.target/i386/pr95237-2.c | 10 ++++++++++
> gcc/testsuite/gcc.target/i386/pr95237-3.c | 10 ++++++++++
> gcc/testsuite/gcc.target/i386/pr95237-4.c | 10 ++++++++++
> gcc/testsuite/gcc.target/i386/pr95237-5.c | 16 ++++++++++++++++
> 10 files changed, 100 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr95237-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr95237-2.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr95237-3.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr95237-4.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr95237-5.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 37aaa49996d..bcd9abd5303 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -16917,6 +16917,25 @@ ix86_minimum_alignment (tree exp, machine_mode mode,
>
> return align;
> }
> +
> +/* Implement TARGET_UPDATE_DECL_ALIGNMENT. */
> +
> +static void
> +ix86_update_decl_alignment (tree decl)
> +{
> + tree type = TREE_TYPE (decl);
> +
> + if (cfun != NULL
> + && !TARGET_64BIT
> + && DECL_ALIGN (decl) == 64
> + && ix86_preferred_stack_boundary < 64
> + && !is_global_var (decl)
> + && (DECL_MODE (decl) == E_DImode
> + || (type && TYPE_MODE (type) == E_DImode))
> + && (!type || !TYPE_USER_ALIGN (type))
> + && (!decl || !DECL_USER_ALIGN (decl)))
I'd simply do
unsigned new_align = LOCAL_DECL_ALIGNMENT (decl);
if (new_align < DECL_ALIGN (decl))
SET_DECL_ALIGN (decl, new_align);
to avoid spreading the logic to multiple places.
Thanks,
Richard.
> + SET_DECL_ALIGN (decl, 32);
> +}
>
> /* Find a location for the static chain incoming to a nested function.
> This is a register, unless all free registers are used by arguments. */
> @@ -23519,6 +23538,9 @@ ix86_run_selftests (void)
> #undef TARGET_CAN_CHANGE_MODE_CLASS
> #define TARGET_CAN_CHANGE_MODE_CLASS ix86_can_change_mode_class
>
> +#undef TARGET_UPDATE_DECL_ALIGNMENT
> +#define TARGET_UPDATE_DECL_ALIGNMENT ix86_update_decl_alignment
> +
> #undef TARGET_STATIC_RTX_ALIGNMENT
> #define TARGET_STATIC_RTX_ALIGNMENT ix86_static_rtx_alignment
> #undef TARGET_CONSTANT_ALIGNMENT
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index 6e7d9dc54a9..c11ef5dca89 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -1086,6 +1086,11 @@ On 32-bit ELF the largest supported section alignment
> in bits is
> @samp{(0x80000000 * 8)}, but this is not representable on 32-bit hosts.
> @end defmac
>
> +@deftypefn {Target Hook} void TARGET_UPDATE_DECL_ALIGNMENT (tree @var{decl})
> +Define this hook to update alignment of decl
> +@samp{(@var{decl}}.
> +@end deftypefn
> +
> @deftypefn {Target Hook} HOST_WIDE_INT TARGET_STATIC_RTX_ALIGNMENT
> (machine_mode @var{mode})
> This hook returns the preferred alignment in bits for a
> statically-allocated rtx, such as a constant pool entry. @var{mode}
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 3be984bbd5c..618acd73a1e 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -1036,6 +1036,8 @@ On 32-bit ELF the largest supported section alignment
> in bits is
> @samp{(0x80000000 * 8)}, but this is not representable on 32-bit hosts.
> @end defmac
>
> +@hook TARGET_UPDATE_DECL_ALIGNMENT
> +
> @hook TARGET_STATIC_RTX_ALIGNMENT
>
> @defmac DATA_ALIGNMENT (@var{type}, @var{basic-align})
> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> index bde6fa22b58..0687a68ba29 100644
> --- a/gcc/stor-layout.c
> +++ b/gcc/stor-layout.c
> @@ -605,6 +605,8 @@ do_type_align (tree type, tree decl)
> if (TYPE_ALIGN (type) > DECL_ALIGN (decl))
> {
> SET_DECL_ALIGN (decl, TYPE_ALIGN (type));
> + /* Update decl alignment */
> + targetm.update_decl_alignment (decl);
> if (TREE_CODE (decl) == FIELD_DECL)
> DECL_USER_ALIGN (decl) = TYPE_USER_ALIGN (type);
> }
> diff --git a/gcc/target.def b/gcc/target.def
> index 07059a87caf..e1695753470 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -3348,6 +3348,13 @@ HOOK_VECTOR_END (addr_space)
> #undef HOOK_PREFIX
> #define HOOK_PREFIX "TARGET_"
>
> +DEFHOOK
> +(update_decl_alignment,
> + "Define this hook to update alignment of decl\n\
> +@samp{(@var{decl}}.",
> + void, (tree decl),
> + hook_void_tree)
> +
> DEFHOOK
> (static_rtx_alignment,
> "This hook returns the preferred alignment in bits for a\n\
> diff --git a/gcc/testsuite/gcc.target/i386/pr95237-1.c
> b/gcc/testsuite/gcc.target/i386/pr95237-1.c
> new file mode 100644
> index 00000000000..bc8a84ee0db
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95237-1.c
> @@ -0,0 +1,16 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-mpreferred-stack-boundary=2" } */
> +typedef __UINTPTR_TYPE__ uintptr_t;
> +void __attribute__((noipa)) foo (long long *p, uintptr_t a)
> +{
> + if ((uintptr_t)p & (a-1))
> + __builtin_abort ();
> +}
> +int main()
> +{
> + long long x;
> + uintptr_t a = __alignof__(x);
> + foo(&x, a);
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr95237-2.c
> b/gcc/testsuite/gcc.target/i386/pr95237-2.c
> new file mode 100644
> index 00000000000..82ff777669a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95237-2.c
> @@ -0,0 +1,10 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-mpreferred-stack-boundary=2" } */
> +long long x;
> +int main()
> +{
> + if (__alignof__(x) != 8)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr95237-3.c
> b/gcc/testsuite/gcc.target/i386/pr95237-3.c
> new file mode 100644
> index 00000000000..2fb1f630362
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95237-3.c
> @@ -0,0 +1,10 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-mpreferred-stack-boundary=2" } */
> +int main()
> +{
> + long long x;
> + if (__alignof__(x) != 4)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr95237-4.c
> b/gcc/testsuite/gcc.target/i386/pr95237-4.c
> new file mode 100644
> index 00000000000..d52a770d703
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95237-4.c
> @@ -0,0 +1,10 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target ia32 } */
> +/* { dg-options "-mpreferred-stack-boundary=4" } */
> +int main()
> +{
> + long long x;
> + if (__alignof__(x) != 8)
> + __builtin_abort();
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr95237-5.c
> b/gcc/testsuite/gcc.target/i386/pr95237-5.c
> new file mode 100644
> index 00000000000..4d9be06a045
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr95237-5.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile { target ia32 } } */
> +/* { dg-options "-mpreferred-stack-boundary=2 -Os -w" } */
> +
> +int a;
> +
> +long long
> +b (void)
> +{
> +}
> +
> +void
> +c (void)
> +{
> + if (b())
> + a = 1;
> +}
> --
> 2.25.4
>
