On Fri, Dec 16, 2016 at 9:29 PM, Andreas Krebbel
<kreb...@linux.vnet.ibm.com> wrote:
> When pushing a value into the literal pool the resulting decl might
> get a higher alignment than the original expression depending on how a
> target defines CONSTANT_ALIGNMENT. Generating an RTX for the constant
> pool access we currently use the alignment from the original
> expression. Changed with the attached patch.
And it might be even smaller alignment... or do we not allow that?
> This fixes a GCC 6 regression for S/390. For arrays of string
> constants as in the attached testcase encode_section_info is not able
> to figure out that the constant pool slot is already properly aligned
> since the mem_align field in the rtx is not set properly.
>
> Another question is why (at the end of build_constant_desc) we invoke
> the encode_section_info hook with the original expression instead of
> the newly generated var_decl? I think the hook is supposed to be
> invoked with a decl?!
No idea...
> /* Set flags or add text to the name to record information, such as
> that it is a local symbol. If the name is changed, the macro
> ASM_OUTPUT_LABELREF will have to know how to strip this
> information. This call might invalidate our local variable
> SYMBOL; we can't use it afterward. */
> targetm.encode_section_info (exp, rtl, true);
>
> desc->rtl = rtl;
>
> return desc;
> }
>
> Bootstrapped and regtested on x86_64 and s390x.
>
> No regressions.
>
> Ok?
Ok.
Richard.
> -Andreas-
>
> gcc/ChangeLog:
>
> 2016-12-16 Andreas Krebbel <kreb...@linux.vnet.ibm.com>
>
> * varasm.c (build_constant_desc): Use the alignment of the var
> decl instead of the original expression.
>
> gcc/testsuite/ChangeLog:
>
> 2016-12-16 Andreas Krebbel <kreb...@linux.vnet.ibm.com>
>
> * gcc.target/s390/litpool-str-1.c: New test.
> ---
> gcc/testsuite/gcc.target/s390/litpool-str-1.c | 22 ++++++++++++++++++++++
> gcc/varasm.c | 4 ++++
> 2 files changed, 26 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/s390/litpool-str-1.c
>
> diff --git a/gcc/testsuite/gcc.target/s390/litpool-str-1.c
> b/gcc/testsuite/gcc.target/s390/litpool-str-1.c
> new file mode 100644
> index 0000000..cd921d2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/litpool-str-1.c
> @@ -0,0 +1,22 @@
> +/* Make sure strings are recognized as being accessible through larl.
> + This requires the symbol ref alignment properly propagated to
> + encode_section_info. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-march=z900 -O2 -fpic" } */
> +
> +
> +extern void foo(const char*, const char*, const char*);
> +
> +void bar(int i)
> +{
> + const char t1[10] = "test";
> + const char t2[10] = "test2";
> + const char t3[2][10] = {
> + "foofoofoo",
> + "barbarbar",
> + };
> + foo(t1, t2, t3[i]);
> +}
> +
> +/* { dg-final { scan-assembler-not "GOTOFF" } } */
> diff --git a/gcc/varasm.c b/gcc/varasm.c
> index 5b15847..68a7ba2 100644
> --- a/gcc/varasm.c
> +++ b/gcc/varasm.c
> @@ -3296,6 +3296,10 @@ build_constant_desc (tree exp)
> set_mem_attributes (rtl, exp, 1);
> set_mem_alias_set (rtl, 0);
>
> + /* Putting EXP into the literal pool might have imposed a larger
> + alignment which should be visible in the RTX as well. */
> + set_mem_align (rtl, DECL_ALIGN (decl));
> +
> /* We cannot share RTX'es in pool entries.
> Mark this piece of RTL as required for unsharing. */
> RTX_FLAG (rtl, used) = 1;
> --
> 2.9.1
>
