[Patch] S/390: Fix symbol ref alignment

[Patch] S/390: Fix symbol ref alignment

[Robin Dapp]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

Hi,

the attached patch fixes the treatment of symbol ref alignments for
arrays and structs in S390. Until now, the NOT_NATURALLY_ALIGNED flag
was not correctly set for array elements and structs larger than 8
bytes. Therefore, load relative instructions that require a specific
alignment would not always be generated. This patch uses separate flags
for 2-, 4-, and 8-byte alignment to fix the problem.

Bootstrapped, no regressions on s390.

Regards
 Robin


gcc/testsuite/ChangeLog:

2015-10-23  Robin Dapp  <rdapp@linux.vnet.ibm.com>

        * gcc.target/s390/load-relative-check.c: New test to check
        generation of load relative instructions.


gcc/ChangeLog:

2015-10-23  Robin Dapp  <rdapp@linux.vnet.ibm.com>

        * config/s390/s390.h: Add new symref flags, _NOTALIGN2 etc.
        * config/s390/s390.c (s390_check_symref_alignment): Use new
        symref flags, early abort on wrong alignment
        (s390_secondary_reload): Use new symref flags.
        (s390_encode_section_info): Likewise.
        * config/s390/predicates.md: Likewise.

[-- Attachment #2: symref-alignment.patch --]
[-- Type: text/x-patch, Size: 7874 bytes --]

diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md
index eeaf1ae..893092b 100644
--- a/gcc/config/s390/predicates.md
+++ b/gcc/config/s390/predicates.md
@@ -122,7 +122,7 @@
   if (GET_CODE (op) == LABEL_REF)
     return true;
   if (GET_CODE (op) == SYMBOL_REF)
-    return (!SYMBOL_REF_ALIGN1_P (op)
+    return (!SYMBOL_FLAG_NOTALIGN2_P (op)
 	    && SYMBOL_REF_TLS_MODEL (op) == 0
 	    && (!flag_pic || SYMBOL_REF_LOCAL_P (op)));
 
@@ -147,7 +147,7 @@
   if (GET_CODE (op) == LABEL_REF)
     return true;
   if (GET_CODE (op) == SYMBOL_REF)
-    return ((SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_ALIGN1) == 0
+    return (!SYMBOL_FLAG_NOTALIGN2_P (op)
 	    && SYMBOL_REF_TLS_MODEL (op) == 0
 	    && (!flag_pic || SYMBOL_REF_LOCAL_P (op)));
 
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 12563a0..ccebd62 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -3922,15 +3922,30 @@ s390_check_symref_alignment (rtx addr, HOST_WIDE_INT alignment)
   HOST_WIDE_INT addend;
   rtx symref;
 
+  /* The "required alignment" might be 0 (e.g. for certain structs
+     accessed via BLKmode).  Early abort in this case, as well as when
+     an alignment > 8 is required.  */
+  if (alignment < 2 || alignment > 8)
+    return false;
+
   if (!s390_loadrelative_operand_p (addr, &symref, &addend))
     return false;
 
   if (addend & (alignment - 1))
     return false;
 
-  if (GET_CODE (symref) == SYMBOL_REF
-      && !SYMBOL_REF_NOT_NATURALLY_ALIGNED_P (symref))
-    return true;
+  if (GET_CODE (symref) == SYMBOL_REF)
+    {
+      /* We have load-relative instructions for 2-byte, 4-byte, and
+         8-byte alignment so allow only these.  */
+      switch (alignment)
+	{
+	case 8:	return !SYMBOL_FLAG_NOTALIGN8_P (symref);
+	case 4:	return !SYMBOL_FLAG_NOTALIGN4_P (symref);
+	case 2:	return !SYMBOL_FLAG_NOTALIGN2_P (symref);
+	default: return false;
+	}
+    }
 
   if (GET_CODE (symref) == UNSPEC
       && alignment <= UNITS_PER_LONG)
@@ -4062,7 +4077,7 @@ s390_secondary_reload (bool in_p, rtx x, reg_class_t rclass_i,
       if (in_p
 	  && s390_loadrelative_operand_p (x, &symref, &offset)
 	  && mode == Pmode
-	  && !SYMBOL_REF_ALIGN1_P (symref)
+	  && !SYMBOL_FLAG_NOTALIGN2_P (symref)
 	  && (offset & 1) == 1)
 	sri->icode = ((mode == DImode) ? CODE_FOR_reloaddi_larl_odd_addend_z10
 		      : CODE_FOR_reloadsi_larl_odd_addend_z10);
@@ -11813,29 +11828,39 @@ s390_encode_section_info (tree decl, rtx rtl, int first)
 
   if (TREE_CODE (decl) == VAR_DECL)
     {
-      /* If a variable has a forced alignment to < 2 bytes, mark it
-	 with SYMBOL_FLAG_ALIGN1 to prevent it from being used as LARL
-	 operand.  */
-      if (DECL_USER_ALIGN (decl) && DECL_ALIGN (decl) < 16)
-	SYMBOL_REF_FLAGS (XEXP (rtl, 0)) |= SYMBOL_FLAG_ALIGN1;
-      if (!DECL_SIZE (decl)
-	  || !DECL_ALIGN (decl)
-	  || !tree_fits_shwi_p (DECL_SIZE (decl))
-	  || (DECL_ALIGN (decl) <= 64
-	      && DECL_ALIGN (decl) != tree_to_shwi (DECL_SIZE (decl))))
-	SYMBOL_REF_FLAGS (XEXP (rtl, 0)) |= SYMBOL_FLAG_NOT_NATURALLY_ALIGNED;
+      /* Store the alignment to be able to check if we can use
+	 a larl/load-relative instruction.  We only handle the cases
+	 that can go wrong (i.e. no FUNC_DECLs).  If a symref does
+	 not have any flag we assume it to be correctly aligned.  */
+
+      if (DECL_ALIGN (decl) % 64)
+	SYMBOL_FLAG_SET_NOTALIGN8 (XEXP (rtl, 0));
+
+      if (DECL_ALIGN (decl) % 32)
+	SYMBOL_FLAG_SET_NOTALIGN4 (XEXP (rtl, 0));
+
+      if (DECL_ALIGN (decl) == 0 || DECL_ALIGN (decl) % 16)
+	SYMBOL_FLAG_SET_NOTALIGN2 (XEXP (rtl, 0));
     }
 
   /* Literal pool references don't have a decl so they are handled
      differently here.  We rely on the information in the MEM_ALIGN
-     entry to decide upon natural alignment.  */
+     entry to decide upon the alignment.  */
   if (MEM_P (rtl)
       && GET_CODE (XEXP (rtl, 0)) == SYMBOL_REF
       && TREE_CONSTANT_POOL_ADDRESS_P (XEXP (rtl, 0))
-      && (MEM_ALIGN (rtl) == 0
-	  || GET_MODE_BITSIZE (GET_MODE (rtl)) == 0
-	  || MEM_ALIGN (rtl) < GET_MODE_BITSIZE (GET_MODE (rtl))))
-    SYMBOL_REF_FLAGS (XEXP (rtl, 0)) |= SYMBOL_FLAG_NOT_NATURALLY_ALIGNED;
+      && MEM_ALIGN (rtl) != 0
+      && GET_MODE_BITSIZE (GET_MODE (rtl)) != 0)
+    {
+      if (MEM_ALIGN (rtl) % 64)
+	SYMBOL_FLAG_SET_NOTALIGN8 (XEXP (rtl, 0));
+
+      if (MEM_ALIGN (rtl) % 32)
+	SYMBOL_FLAG_SET_NOTALIGN4 (XEXP (rtl, 0));
+
+      if (MEM_ALIGN (rtl) == 0 || MEM_ALIGN (rtl) % 16)
+	SYMBOL_FLAG_SET_NOTALIGN2 (XEXP (rtl, 0));
+    }
 }
 
 /* Output thunk to FILE that implements a C++ virtual function call (with
diff --git a/gcc/config/s390/s390.h b/gcc/config/s390/s390.h
index a0faf13..0f9225c 100644
--- a/gcc/config/s390/s390.h
+++ b/gcc/config/s390/s390.h
@@ -963,12 +963,35 @@ do {									\
 #define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) ((VALUE) = 64, 1)
 
 /* Machine-specific symbol_ref flags.  */
-#define SYMBOL_FLAG_ALIGN1	          (SYMBOL_FLAG_MACH_DEP << 0)
-#define SYMBOL_REF_ALIGN1_P(X)		\
-  ((SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_ALIGN1))
-#define SYMBOL_FLAG_NOT_NATURALLY_ALIGNED (SYMBOL_FLAG_MACH_DEP << 1)
-#define SYMBOL_REF_NOT_NATURALLY_ALIGNED_P(X) \
-  ((SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_NOT_NATURALLY_ALIGNED))
+#define SYMBOL_FLAG_ALIGN_SHIFT	  SYMBOL_FLAG_MACH_DEP_SHIFT
+#define SYMBOL_FLAG_ALIGN_MASK    \
+  ((SYMBOL_FLAG_MACH_DEP << 0) | (SYMBOL_FLAG_MACH_DEP << 1))
+
+#define SYMBOL_FLAG_SET_ALIGN(X, A) \
+    (SYMBOL_REF_FLAGS (X) = (SYMBOL_REF_FLAGS (X) & ~SYMBOL_FLAG_ALIGN_MASK) \
+     | (A << SYMBOL_FLAG_ALIGN_SHIFT))
+
+#define SYMBOL_FLAG_GET_ALIGN(X) \
+    ((SYMBOL_REF_FLAGS (X) & SYMBOL_FLAG_ALIGN_MASK) >> SYMBOL_FLAG_ALIGN_SHIFT)
+
+/* Helpers to access symbol_ref flags.  They are used in
+   check_symref_alignment() and larl_operand to detect if the
+   available alignment matches the required one.  We do not use
+   a positive check like _ALIGN2 because in that case we would have
+   to annotate every symbol_ref.  However, we only want to touch
+   the symbol_refs that can be misaligned and assume that the others
+   are correctly aligned.  Hence, if a symbol_ref does not have
+   a _NOTALIGN flag it is supposed to be correctly aligned.  */
+#define SYMBOL_FLAG_SET_NOTALIGN2(X) SYMBOL_FLAG_SET_ALIGN(X, 1)
+#define SYMBOL_FLAG_SET_NOTALIGN4(X) SYMBOL_FLAG_SET_ALIGN(X, 2)
+#define SYMBOL_FLAG_SET_NOTALIGN8(X) SYMBOL_FLAG_SET_ALIGN(X, 3)
+
+#define SYMBOL_FLAG_NOTALIGN2_P(X) (SYMBOL_FLAG_GET_ALIGN(X) == 1)
+#define SYMBOL_FLAG_NOTALIGN4_P(X) (SYMBOL_FLAG_GET_ALIGN(X) == 2 \
+				    || SYMBOL_FLAG_GET_ALIGN(X) == 1)
+#define SYMBOL_FLAG_NOTALIGN8_P(X) (SYMBOL_FLAG_GET_ALIGN(X) == 3 \
+				    || SYMBOL_FLAG_GET_ALIGN(X) == 2 \
+				    || SYMBOL_FLAG_GET_ALIGN(X) == 1)
 
 /* Check whether integer displacement is in range for a short displacement.  */
 #define SHORT_DISP_IN_RANGE(d) ((d) >= 0 && (d) <= 4095)
diff --git a/gcc/testsuite/gcc.target/s390/load-relative-check.c b/gcc/testsuite/gcc.target/s390/load-relative-check.c
new file mode 100644
index 0000000..5290045
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/load-relative-check.c
@@ -0,0 +1,46 @@
+/* Check if load-relative instructions are created */
+/* { dg-do compile { target { s390*-*-* } } } */
+/* { dg-options "-O2 -march=z10" } */
+
+/* { dg-final { scan-assembler "lgfrl\t%r.?,b.4" } } */
+/* { dg-final { scan-assembler "lgfrl\t%r.?,s.12" } } */
+/* { dg-final { scan-assembler "lgrl\t%r.?,s" } } */
+/* { dg-final { scan-assembler "larl\t%r.?,.L.?" } } */
+
+int b[20];
+
+struct s
+{
+  long a;
+  int  b;
+  int  c;
+} s;
+
+struct __attribute__((packed)) s2
+{
+  char a;
+  char b;
+  char c;
+} s2;
+
+char __attribute__((aligned(1))) arr[10];
+
+int foo()
+  {
+    return b[1];
+  }
+
+int bar()
+  {
+    return s.c;
+  }
+
+long bar2()
+  {
+    return s.a;
+  }
+
+int baz()
+  {
+    return arr[1];
+  }

Re: [Patch] S/390: Fix symbol ref alignment

[Andreas Krebbel]

On 10/23/2015 02:12 PM, Robin Dapp wrote:
> gcc/testsuite/ChangeLog:
> 
> 2015-10-23  Robin Dapp  <rdapp@linux.vnet.ibm.com>
> 
>         * gcc.target/s390/load-relative-check.c: New test to check
>         generation of load relative instructions.
> 
> 
> gcc/ChangeLog:
> 
> 2015-10-23  Robin Dapp  <rdapp@linux.vnet.ibm.com>
> 
>         * config/s390/s390.h: Add new symref flags, _NOTALIGN2 etc.
>         * config/s390/s390.c (s390_check_symref_alignment): Use new
>         symref flags, early abort on wrong alignment
>         (s390_secondary_reload): Use new symref flags.
>         (s390_encode_section_info): Likewise.
>         * config/s390/predicates.md: Likewise.

Applied. Thanks!

-Andreas-