On 6/29/20 5:00 AM, Richard Biener wrote:
On Fri, Jun 26, 2020 at 10:11 PM H.J. Lu <hjl.to...@gmail.com> wrote:
On Thu, Jun 25, 2020 at 1:10 AM Richard Biener
<richard.guent...@gmail.com> wrote:
On Thu, Jun 25, 2020 at 2:53 AM Sunil Pandey <skpg...@gmail.com> wrote:
On Wed, Jun 24, 2020 at 12:30 AM Richard Biener
<richard.guent...@gmail.com> wrote:
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
Yes, I can change the target hook name.
would be better suited (and then asks for LOCAL_DECL_ALIGNMENT to be
renamed to INCREASE_LOCAL_DECL_ALIGNMENT).
It seems like LOCAL_DECL_ALIGNMENT macro documentation is incorrect.
It increases as well as decreases alignment based on condition(-m32
-mpreferred-stack-boundary=2)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95885
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))
It seems like decl property is not fully populated at this point call
to is_global_var (decl) on global variable return false.
$ cat foo.c
long long x;
int main()
{
if (__alignof__(x) != 8)
__builtin_abort();
return 0;
}
Breakpoint 1, layout_decl (decl=0x7ffff7ffbb40, known_align=0)
at /local/skpandey/gccwork/gccwork/gcc/gcc/stor-layout.c:674
674 do_type_align (type, decl);
Missing separate debuginfos, use: dnf debuginfo-install
gmp-6.1.2-10.fc31.x86_64 isl-0.16.1-9.fc31.x86_64
libmpc-1.1.0-4.fc31.x86_64 mpfr-3.1.6-5.fc31.x86_64
zlib-1.2.11-20.fc31.x86_64
(gdb) call debug_tree(decl)
<var_decl 0x7ffff7ffbb40 x
type <integer_type 0x7fffea801888 long long int DI
size <integer_cst 0x7fffea7e8d38 constant 64>
unit-size <integer_cst 0x7fffea7e8d50 constant 8>
align:64 warn_if_not_align:0 symtab:0 alias-set -1
canonical-type 0x7fffea801888 precision:64 min <integer_cst
0x7fffea7e8fd8 -9223372036854775808> max <integer_cst 0x7fffea806000
9223372036854775807>
pointer_to_this <pointer_type 0x7fffea8110a8>>
DI foo.c:1:11 size <integer_cst 0x7fffea7e8d38 64> unit-size
<integer_cst 0x7fffea7e8d50 8>
align:1 warn_if_not_align:0>
(gdb) p is_global_var(decl)
$1 = false
(gdb)
What about calling hook here
603 do_type_align (tree type, tree decl)
604 {
605 if (TYPE_ALIGN (type) > DECL_ALIGN (decl))
606 {
607 SET_DECL_ALIGN (decl, TYPE_ALIGN (type));
608 if (TREE_CODE (decl) == FIELD_DECL)
609 DECL_USER_ALIGN (decl) = TYPE_USER_ALIGN (type);
610 else
611 /* Lower local decl alignment */
612 if (VAR_P (decl)
613 && !is_global_var (decl)
614 && !DECL_HARD_REGISTER (decl)
615 && cfun != NULL)
616 targetm.lower_local_decl_alignment (decl);
617 }
But that doesn't change anything (obviously). layout_decl
is called quite early, too early it looks like.
Now there doesn't seem to be any other good place where
we are sure to catch the decl before we evaluate things
like __alignof__
void __attribute__((noipa))
foo (__SIZE_TYPE__ align, long long *p)
{
if ((__SIZE_TYPE__)p & (align-1))
__builtin_abort ();
}
int main()
{
long long y;
foo (_Alignof y, &y);
return 0;
}
Joseph/Jason - do you have a good recommendation
how to deal with targets where natural alignment
is supposed to be lowered for optimization purposes?
(this case is for i?86 to avoid dynamic stack re-alignment
to align long long to 8 bytes with -mpreferred-stack-boundary=2)
I note that for -mincoming-stack-boundary=2 we do perform
dynamic stack re-alignment already.
I can't find a suitable existing target macro/hook for this,
but my gut feeling is that the default alignment should
instead be the lower one and instead the alignment for
globals should be raised as optimization?
Here is the updated patch from Sunil.
It does not address the fundamental issue that during
do_type_align the is_global_var predicate is not
reliable. This means that for
int main()
{
extern long z;
}
the new hook (with -m32 -mpreferred-stack-boundary=2)
will lower the alignment of 'z' which looks wrong. During
layout_decl we can unfortunately not distinguish between
locals and globals. We need to find another spot to adjust
alignment of locals. For C that might be in finish_decl,
for C++ there's probably another suitable place.
cp_finish_decl could work, but layout_decl seems like the right spot; if
the problem is that the appropriate flags currently aren't being set in
time, can't we fix that?
Note it needs to be a place before the frontends possibly
inspect the alignment of the decl
In C++ constexpr evalualtion might also expose alignment
"early" so we really need a frontend solution here.
Yes, we need to know the alignment right after the declaration.
Jason
