On Tue, 2020-06-09 at 09:17 +0200, Richard Biener wrote: > On Mon, Jun 8, 2020 at 11:13 PM Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > We're seeing some failures due to the local-alignment pass. For example on > > sh4: > > > > Tests that now fail, but worked before (16 tests): > > > > gcc.dg/pr48335-1.c (test for excess errors) > > gcc.dg/pr48335-2.c (test for excess errors) > > gcc.dg/pr48335-5.c (test for excess errors) > > gcc.dg/pr48335-6.c (test for excess errors) > > gcc.dg/torture/pr48493.c -O0 (test for excess errors) > > gcc.dg/torture/pr48493.c -O0 (test for excess errors) > > gcc.dg/torture/pr48493.c -O1 (test for excess errors) > > gcc.dg/torture/pr48493.c -O1 (test for excess errors) > > gcc.dg/torture/pr48493.c -O2 (test for excess errors) > > gcc.dg/torture/pr48493.c -O2 (test for excess errors) > > gcc.dg/torture/pr48493.c -O2 -flto -fno-use-linker-plugin -flto- > > partition=none (test for excess errors) > > gcc.dg/torture/pr48493.c -O2 -flto -fno-use-linker-plugin -flto- > > partition=none (test for excess errors) > > gcc.dg/torture/pr48493.c -O3 -g (test for excess errors) > > gcc.dg/torture/pr48493.c -O3 -g (test for excess errors) > > gcc.dg/torture/pr48493.c -Os (test for excess errors) > > gcc.dg/torture/pr48493.c -Os (test for excess errors) > > > > Or on x86_64: > > > > during GIMPLE pass: adjust_alignment > > /home/jenkins/linux/arch/x86/boot/string.c: In function ‘simple_strtoull’: > > /home/jenkins/linux/arch/x86/boot/string.c:121:20: internal compiler error: > > in > > execute, at adjust-alignment.c:74 > > 121 | unsigned long long simple_strtoull(const char *cp, char **endp, > > unsigned > > int base) > > | ^~~~~~~~~~~~~~~ > > 0x79c5b3 execute > > ../../../../../gcc/gcc/adjust-alignment.c:74 > > Please submit a full bug report, > > > > There may be others, I haven't searched the failing targets extensively for > > this > > issue. > > > > AFAICT the adjust-alignment pass is supposed to increase alignments of > > locals > > when needed. It has an assert to ensure that alignments are only increased. > > > > However, if the object was declared with an alignment attribute that is > > larger > > than what LOCAL_ALIGNMENT would produce for the object, then the > > adjust-alignment > > pass trips the assert. > > > > Rather than asserting, just ignoring such objects seems better. But I'm not > > intimately familiar with the issues here. > > > > Bootstrapped and regression tested on x86_64, also verified this fixes the > > sh4 > > issue. All the *-elf targets have also built/tested with this patch with no > > regressions. > > > > OK for the trunk? > > While technically OK the issue is that it does not solve the x86 issue > which with incoming stack alignment < 8 bytes does not perform > dynamic re-alignment to align 'long long' variables appropriately. > Currently the x86 backend thinks it can fixup by lowering alignment > via LOCAL_DECL_ALIGNMENT but this is a latent wrong-code issue > because the larger alignment is present in the IL for a long (previously > RTL expansion, now adjust-aligment) time and your patch makes that > wrong info last forever (or alternatively cause dynamic stack alignment > which we do _not_ want to perform here). I've never looked at the dynamic realignment stuff -- is there a good reason why we don't dynamically realign when we've got a local with a requested alignment? Isn't that a huge oversight in the whole concept of dynamic realignment?
> > So I prefer to wait for a proper x86 solution before cutting the ICEs. > (did you verify effects on code generation of your patch?) I don't mind waiting. And no, I didn't look at the codegen at all. jeff
