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). So I prefer to wait for a proper x86 solution before cutting the ICEs. (did you verify effects on code generation of your patch?) Thanks, Richard. > > Jeff > >
