| Message ID | 20120710060955.GC3117@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 189345) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -6409,7 +6409,7 @@ rs6000_mode_dependent_address (const_rtx addr) && GET_CODE (XEXP (addr, 1)) == CONST_INT) { unsigned HOST_WIDE_INT val = INTVAL (XEXP (addr, 1)); - return val + 12 + 0x8000 >= 0x10000; + return val + 0x8000 >= 0x10000 - (TARGET_POWERPC64 ? 8 : 12); } break;
Hi David, I've been auditing rs6000.c after looking at https://bugzilla.redhat.com/show_bug.cgi?id=837630 We have problems all over the place. :-( For starters, this code in rs6000_legitimate_offset_address_p. case TFmode: ... case TDmode: case TImode: if (mode == TFmode || mode == TDmode || !TARGET_POWERPC64) extra = 12; else if (offset & 3) return false; else extra = 8; break; Notice how a TFmode when TARGET_POWERPC64 gets extra==12. How can that be corrent? Both general and floating point registers are 8 bytes. However, this is not the main problem I see here and in many other places. The problem is that the mode is only a hint as to the register used in the insn. You can easily have a floating point mode in gprs or an integer mode in fprs, as shown by the following examples. These are all quite contrived, but a similar situation may arise in real code due to register pressure. double a, b; void f1 (void) { double tmp = a; asm ("#" : "+r" (tmp) : : "fr0", "fr1", "fr2","fr3", "fr4", "fr5", "fr6", "fr7", "fr8", "fr9", "fr10", "fr11", "fr12", "fr13", "fr14", "fr15", "fr16", "fr17", "fr18", "fr19", "fr20", "fr21", "fr22", "fr23", "fr24", "fr25", "fr26", "fr27", "fr28", "fr29", "fr30", "fr31"); b = tmp; } long long x, y; void f2 (void) { long long tmp = x; asm ("#" : "+f" (tmp) : : "r0", "r1", "r2","r3", "r4", "r5", "r6", "r7", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"); y = tmp; } long double la, lb; void f3 (void) { long double tmp = la; asm ("#" : "+r" (tmp) : : "fr0", "fr1", "fr2","fr3", "fr4", "fr5", "fr6", "fr7", "fr8", "fr9", "fr10", "fr11", "fr12", "fr13", "fr14", "fr15", "fr16", "fr17", "fr18", "fr19", "fr20", "fr21", "fr22", "fr23", "fr24", "fr25", "fr26", "fr27", "fr28", "fr29", "fr30", "fr31"); lb = tmp; } So I think that rs6000_legitimate_offset_address_p code snippet ought to be case TFmode: ... case TDmode: case TImode: if (!TARGET_POWERPC64) extra = 12; else if (offset & 3) return false; else extra = 8; break; We can't assume that TFmode or TDmode are in fprs, so for 32-bit that means a possible addition of 12 to the offset and for 64-bit it means the offset must be a multiple of 4. If I go ahead and fix the many places in rs6000.c we will see some code degradation for oddball cases. For example, loading a mis-aligned double (well, not 4-byte aligned) on ppc64 to a fpr will use direct addressing rather than offset addressing. Maybe we could do something about that with a peephole.. I know the above isn't anything new. When this sort of problem came up in the past we didn't want to lose code quality in the common case, and tried to avoid the correctness issue with hacks that made incorrect code unlikely to occur. The thing is that with section anchors we are much more likely to see offset addresses near 32k boundaries. So it is worth trying to fix this properly? If it is, here's the first baby step toward that aim. Pegs the negative limit at -32768 instead of -32780, and increases the positive limit a little for powerpc64. This patch is a prerequisite to making rs6000_legitimate_offset_address_p fixes since this one affects insn operand constraints, while other places affect predicates; You can't have insn contraints not covering all operands allowed by the predicate, but the converse is fine. Bootstrapped etc. powerpc64-linux and powerpc-linux. * config/rs6000/rs6000.c (rs6000_mode_dependent_address): Correct offset addressing limits.