Isn't this too conservative? Shouldn't CONSTANT_ALIGNMENT increase
the alignment of STRING_CST to word-aligned? The only problem is
structs?
- David
On Wed, Mar 2, 2011 at 2:34 AM, Alan Modra <amo...@gmail.com> wrote:
> Prior to a fix I committed recently on the gcc-4.5-ibm branch, we used
> to fail with a linker error: "relocation R_PPC64_TOC16_LO_DS not a
> multiple of 4" on code loading the "md_backup_data-1" string in the
> following testcase.
>
> typedef __signed__ char __s8;
> typedef unsigned char __u8;
> typedef __signed__ short __s16;
> typedef unsigned short __u16;
> typedef __signed__ int __s32;
> typedef unsigned int __u32;
> typedef __signed__ long __s64;
> typedef unsigned long __u64;
>
> static struct mdp_backup_super {
> char magic[16];
> __u8 set_uuid[16];
> __u64 mtime;
>
> __u64 devstart;
> __u64 arraystart;
> __u64 length;
> __u32 sb_csum;
> __u32 pad1;
> __u64 devstart2;
> __u64 arraystart2;
> __u64 length2;
> __u32 sb_csum2;
> __u8 pad[512-68-32];
> } __attribute__((aligned(512))) bsb;
>
> void f (void)
> {
> __builtin_memcpy (bsb.magic, "md_backup_data-1", 16);
> }
>
> The memcpy gets optimised to two ld insns followed by two std insns,
> with both the address of bsb and the string being toc-relative.
> bsb.magic is sufficiently aligned for the stores to be OK, but the
> string is not aligned at all; The loads may well be at an address
> that is not a multiple of four. It is also quite possible for the
> string to straddle a 32k boundary, resulting in rubbish being loaded
> in the second dword.
>
> On mainline, the situation is a little different due to changes in
> memref handling. I seem to always get a VAR_DECL for the string,
> correctly reporting an alignment of one, so we don't use the invalid
> toc-relative address. ie. I can't find a testcase for this potential
> problem on mainline. What worries me is that this may simply be due
> to not trying enough compiler options. Also, future changes in gcc
> might result in STRING_CST trees appearing as they do on 4.5. So I'd
> like to apply the following patch. Bootstrapped etc. powerpc64-linux.
> OK?
>
> * config/rs6000/rs6000.c (offsettable_ok_by_alignment): Ensure
> STRING_CST returns false. Add assert.
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c (revision 170373)
> +++ gcc/config/rs6000/rs6000.c (working copy)
> @@ -7301,12 +7301,14 @@ offsettable_ok_by_alignment (tree decl)
> if (!decl)
> return true;
>
> - if (TREE_CODE (decl) != VAR_DECL
> - && TREE_CODE (decl) != PARM_DECL
> - && TREE_CODE (decl) != RESULT_DECL
> - && TREE_CODE (decl) != FIELD_DECL)
> + if (TREE_CODE (decl) == FUNCTION_DECL)
> return true;
>
> + if (CONSTANT_CLASS_P (decl))
> + return TREE_CODE (decl) != STRING_CST;
> +
> + gcc_assert (DECL_P (decl));
> +
> if (!DECL_SIZE_UNIT (decl))
> return false;
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
>
