On Fri, 22 Mar 2019, Bernd Edlinger wrote: > On 3/21/19 12:15 PM, Richard Biener wrote: > > On Sun, 10 Mar 2019, Bernd Edlinger wrote: > > Finally... > > > > Index: gcc/function.c > > =================================================================== > > --- gcc/function.c (revision 269264) > > +++ gcc/function.c (working copy) > > @@ -2210,6 +2210,12 @@ use_register_for_decl (const_tree decl) > > if (DECL_MODE (decl) == BLKmode) > > return false; > > > > + if (STRICT_ALIGNMENT && TREE_CODE (decl) == PARM_DECL > > + && DECL_INCOMING_RTL (decl) && MEM_P (DECL_INCOMING_RTL (decl)) > > + && GET_MODE_ALIGNMENT (DECL_MODE (decl)) > > + > MEM_ALIGN (DECL_INCOMING_RTL (decl))) > > + return false; > > + > > /* If -ffloat-store specified, don't put explicit float variables > > into registers. */ > > /* ??? This should be checked after DECL_ARTIFICIAL, but tree-ssa > > > > I wonder if it is necessary to look at DECL_INCOMING_RTL here > > and why such RTL may not exist? That is, iff DECL_INCOMING_RTL > > doesn't exist then shouldn't we return false for safety reasons? > > > > I think that happens a few times already before the INCOMING_RTL > is assigned. I thought that might be too pessimistic. > > > Similarly the very same issue should exist on x86_64 which is > > !STRICT_ALIGNMENT, it's just the ABI seems to provide the appropriate > > alignment on the caller side. So the STRICT_ALIGNMENT check is > > a wrong one. > > > > I may be plain wrong here, but I thought that !STRICT_ALIGNMENT targets > just use MEM_ALIGN to select the right instructions. MEM_ALIGN > is always 32-bit align on the DImode memory. The x86_64 vector instructions > would look at MEM_ALIGN and do the right thing, yes?
No, they need to use the movmisalign optab and end up with UNSPECs for example. > It seems to be the definition of STRICT_ALIGNMENT targets that all RTL > instructions need to have MEM_ALIGN >= GET_MODE_ALIGNMENT, so the target > does not even have to look at MEM_ALIGN except in the mov_misalign_optab, > right? Yes, I think we never losened that. Note that RTL expansion has to fix this up for them. Note that strictly speaking SLOW_UNALIGNED_ACCESS specifies that x86 is strict-align wrt vector modes. > The other hunk, where I admit I did not fully understand the comment, tries > only to increase the MEM_ALIGN to 64-bit if the stack slot is > 64-bit aligned although the target said it only needs 32-bit alignment. > So that it is no longer necessary to copy the incoming value. > > > > Which makes me think that a proper fix is not here, but in > > target(hook) code. > > > > Changing use_register_for_decl sounds odd anyways since if we return true > > we for the testcase still end up in memory, no? > > > > It seems to make us use the incoming register _or_ stack slot if this function > returns true here. > > If it returns false here, a new stack slot is allocated, but only if the > original stack slot was not aligned. This works together with the > other STRICT_ALIGNMENT check in assign_parm_adjust_stack_rtl. Yes, I understood this - but then the check should be in that code deciding whether to copy, not in use_register_for_decl? > Where also for !STRICT_ALIGNMENT target TYPE_ALIGN and MEM_ALIGN > are checked, but this seems to have only an effect if an address > is taken, in that case I see use_register_for_decl return false > due to TREE_ADDRESSABLE (decl), and whoops, we have an aligned copy > of the unaligned stack slot. > > So I believe that there was already a fix for unaligned stack positions, > that relied on the addressability of the parameter, while the target > relied on the 8-byte alignment of the DImode access. > > > The hunk obviously misses a comment since the effect that this > > will cause a copy to be emitted isn't obvious (and relying on > > this probably fragile). > > > > Yes, also that the copy is done using movmisalign optab is important. > > > Thanks > Bernd. > -- Richard Biener <rguent...@suse.de> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
