> Another problem is that in way too many cases we decide to choose > BIGGEST_ALIGNMENT for stack slots, even when not strictly needed. E.g. any > BLKmode stack slot requests that BIGGEST_ALIGNMENT, even if TYPE_ALIGN is > much smaller and assign_stack_local_1 even asserts that for BLKmode the > alignment must be BIGGEST_ALIGNMENT. E.g. on mingw with -mavx or -mavx512f > BIGGEST_ALIGNMENT is 256 or 512 bits, but MAX_SUPPORTED_STACK_ALIGNMENT is > 128 bits. So the PR84877 change, even if it didn't cause wrong-code, causes > huge amounts of stack slots to be unnecessarily overaligned with dynamic > realignment.
Yes, I think that we need to make sure that we dynamically realign only when this is necessary and... > The following patch reverts to the previous behavior and moves this dynamic > stack realignment to the caller that needs it, which doesn't do caching and > where we can do it solely for this overaligned DECL_ALIGN. ...this modified implementation looks much safer than the original one. > If there are other spots that need this, wondering about: > else > copy = assign_temp (type, 1, 0); > in calls.c, either it can be done by using the variable-sized object > case in the then block, or could be done using assign_stack_local + > this short realignment too. The latter I'd say. > Bootstrapped/regtested on x86_64-linux, i686-linux, powerpc64le-linux, > bootstrapped on powerpc64-linux (regtest still pending there). > > Ok for trunk? > > 2019-01-09 Jakub Jelinek <ja...@redhat.com> > > PR middle-end/84877 > PR bootstrap/88450 > * function.c (assign_stack_local_1): Revert the 2018-11-21 changes. > (assign_parm_setup_block): Do the argument slot realignment here > instead. FWIW this looks OK to me. -- Eric Botcazou
