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); +}