| Summary: | [4.3/4.4 Regression] Incorrectly assumed aligned_operand | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Jakub Jelinek <jakub> |
| Component: | target | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | bergner, gcc-bugs, ht990332, hubicka, pinskia |
| Priority: | P1 | Keywords: | wrong-code |
| Version: | 4.3.1 | ||
| Target Milestone: | 4.3.2 | ||
| Host: | Target: | i686-linux | |
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | ||
aligned_operand looks wrong:
/* Look for some component that isn't known to be aligned. */
if (parts.index)
{
if (REGNO_POINTER_ALIGN (REGNO (parts.index)) * parts.scale < 32)
return 0;
}
if (parts.base)
{
if (REGNO_POINTER_ALIGN (REGNO (parts.base)) < 32)
return 0;
}
We are using the hard registers here so we can have the wrong alignment as they are shared.
Why are we looking into register pointer's alignment anyways? The MEM_ALIGN check about should give the correct information anyways.
We shouldn't be attempting to call mark_reg_pointer in set_reg_attrs_from_value for a hard reg, since they can be shared between different values. Andrew mentioned we maybe shouldn't be calling set_reg_attrs_from_value() with a hard reg, or we could do the following:
--- emit-rtl.c (revision 134986)
+++ emit-rtl.c (working copy)
@@ -969,14 +969,14 @@ set_reg_attrs_from_value (rtx reg, rtx x
if (MEM_OFFSET (x) && GET_CODE (MEM_OFFSET (x)) == CONST_INT)
REG_ATTRS (reg)
= get_reg_attrs (MEM_EXPR (x), INTVAL (MEM_OFFSET (x)) + offset);
- if (MEM_POINTER (x))
+ if (!HARD_REGISTER_P (reg) && MEM_POINTER (x))
mark_reg_pointer (reg, MEM_ALIGN (x));
}
else if (REG_P (x))
{
if (REG_ATTRS (x))
update_reg_offset (reg, x, offset);
- if (REG_POINTER (x))
+ if (!HARD_REGISTER_P (reg) && REG_POINTER (x))
mark_reg_pointer (reg, REGNO_POINTER_ALIGN (REGNO (x)));
}
}
In addition to that, I wonder whether i386's aligned_operand shouldn't prefer ORIGINAL_REGNO over REGNO when checking pointer alignment. Subject: Bug 36533 Author: jakub Date: Mon Jun 23 13:06:15 2008 New Revision: 137038 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137038 Log: PR target/36533 * emit-rtl.c (set_reg_attrs_from_value): Do nothing if REG is a hard register. * gcc.target/i386/pr36533.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr36533.c Modified: trunk/gcc/ChangeLog trunk/gcc/emit-rtl.c trunk/gcc/testsuite/ChangeLog Subject: Bug 36533 Author: jakub Date: Mon Jun 23 13:16:07 2008 New Revision: 137039 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=137039 Log: PR target/36533 * emit-rtl.c (set_reg_attrs_from_value): Do nothing if REG is a hard register. * gcc.target/i386/pr36533.c: New test. Added: branches/gcc-4_3-branch/gcc/testsuite/gcc.target/i386/pr36533.c Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/emit-rtl.c branches/gcc-4_3-branch/gcc/testsuite/ChangeLog Fixed. |
The following testcase is miscompiled on i?86 -m32 -Os: /* { dg-options "-Os" } */ typedef struct S1 { unsigned long s1; struct S1 *s2; char *s3; } S1; typedef struct { unsigned int s4; unsigned int s5; int s6; unsigned int *s7; } S2; typedef struct { unsigned int s8; unsigned short s9; unsigned char s10; unsigned char s11; char s12[255]; } S3; typedef struct { unsigned int s4; unsigned short s13; unsigned short s14; } S4; typedef struct { char s15[16]; unsigned long s16; } S5; typedef struct { char s15[48]; S5 *s17; } S6; typedef struct { S1 *s18; } S7; extern __attribute__((regparm (3))) int fn1 (const char *x, void *y, S1 *z); extern __attribute__((regparm (3))) int fn2 (const char *x, int y, S2 *z); static inline __attribute__ ((always_inline)) unsigned int fn4 (unsigned short x) { unsigned len = x; if (len == ((1 << 16) - 1)) return 1 << 16; return len; } static inline __attribute__ ((always_inline)) S3 * fn3 (S3 *p) { return (S3 *) ((char *) p + fn4 (p->s9)); } extern __attribute__((regparm (3))) int fn5 (void); static inline __attribute__ ((always_inline)) int fn6 (S3 *w, int x, S2 *y, S4 *z) { int a = 0; char *b = (char *) w; S2 c = *y; while ((char *) w < b + x) { if (w->s10 && w->s8) { fn2 (w->s12, w->s10, &c); z--; z->s4 = c.s4; z->s13 = (unsigned short) ((char *) w - b); z->s14 = w->s9; a++; fn5 (); } w = fn3 (w); } return a; } __attribute__((regparm (3))) unsigned int test (void *u, S6 *v, S1 **w, S7 *x, S2 *y, S1 *z) { unsigned b = v->s17->s16; unsigned a; S4 *c; unsigned d, e, f, i; fn1 (__func__, u, x->s18); c = (S4 *) (z->s3 + b); a = fn6 ((S3 *) (*w)->s3, b, y, c); c -= a; f = 0; e = 0; for (i = a - 1; ; i--) { if (f + (unsigned short) (c[i].s14 / 2) > b / 2) break; f += c[i].s14; e++; } d = a - e; return c[d].s4; } since the PR28690 backport. The c[i].s14 read is done using movl (%ecx), %edi # <variable>.s14, D.1321 rather than movw (%ecx), %di which is wrong in this case, as %ecx is provably not 32-bit aligned (%ecx - 6 is known to be 32-bit aligned). Shouldn't be hard to transform this into an executable testcase (put the array of S14 at the end of mmaped page such that the last c[i].s14 is right before end of page). This is a regression since 4.3.0.