This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] decl alignment not respected


This patch cures a problem with ICF of read-only variables at the
intersection of -fsection-anchors, -ftree-loop-vectorize, and targets
with alignment restrictions.  The testcase results in
/usr/local/powerpc64le-linux/bin/ld: pack.o: In function `main':
pack.c:(.text.startup+0xc): error: R_PPC64_TOC16_LO_DS not a multiple of 4
/usr/local/powerpc64le-linux/bin/ld: final link failed: Bad value
on powerpc64le-linux.

What happens is:
- "c" is referenced in a constructor, thus make_decl_rtl for "c",
- make_decl_rtl puts "c" in an anchor block (-fsection-anchors),
- anchor block contents can't move, so "c" alignment can't change by
  ipa_increase_alignment (-ftree-loop-vectorize),
- however "a" alignment can be increased,
- ICF aliases "a" to "c".
So we have a decl for "a" saying it is aligned to 128 bits, using mem
for "c" which is only 16 bit aligned.  The supposed increased
alignment causes "a" to be accessed as a 64-bit word, and the
powerpc64 backend to use a ds-form addressing mode.

Somewhere this chain of events needs to be broken.  It isn't possible
to stop ipa_increase_alignment changing "a", because at that stage
ICF aliases don't exist.  So it seemed to me that ICF needed to be
taught not to create a problematic alias.  Not being very familiar
with this code, I don't know if the following is the best place to
change, but sem_variable::merge does throw away aliases for quite a
lot of other reasons.  Another possibility is
sem_variable::equals_wpa, where there's a comment about DECL_ALIGN
being safe to merge "because we will always choose the largest
alignment out of all aliases".  Except with this testcase we don't
choose the largest alignment, and indeed can't (I think) due to "c"
being used in a constructor.

Bootstrapped and regression tested powerpc64le-linux.  OK to apply?

gcc/
	PR ipa/69990
	* ipa-icf.c (sem_variable::merge): Do not merge an alias with
	larger alignment.
gcc/testsuite/
	gcc.dg/pr69990.c: New.

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index ef04c55..d82eb87 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -2209,6 +2209,16 @@ sem_variable::merge (sem_item *alias_item)
 		 "adress of original and alias may be compared.\n\n");
       return false;
     }
+
+  if (DECL_ALIGN (original->decl) < DECL_ALIGN (alias->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Not unifying; "
+		 "original and alias have incompatible alignments\n\n");
+
+      return false;
+    }
+
   if (DECL_COMDAT_GROUP (original->decl) != DECL_COMDAT_GROUP (alias->decl))
     {
       if (dump_file)
diff --git a/gcc/testsuite/gcc.dg/pr69990.c b/gcc/testsuite/gcc.dg/pr69990.c
new file mode 100644
index 0000000..efb835e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69990.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-require-effective-target section_anchors } */
+/* { dg-options "-O2 -fsection-anchors -ftree-loop-vectorize" } */
+
+#pragma pack(1)
+struct S0 {
+  volatile int f0:12;
+} static a[] = {{15}}, c[] = {{15}};
+
+struct S0 b[] = {{7}};
+
+int __attribute__ ((noinline, noclone))
+ok (int a, int b, int c)
+{
+  return a == 15 && b == 7 && c == 15 ? 0 : 1;
+}
+
+int
+main (void)
+{
+  struct S0 *f[] = { c, b };
+
+  return ok (a[0].f0, b[0].f0, f[0]->f0);
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]