Patchwork [RS6000] offset addressing

login
register
mail settings
Submitter Alan Modra
Date July 10, 2012, 6:09 a.m.
Message ID <20120710060955.GC3117@bubble.grove.modra.org>
Download mbox | patch
Permalink /patch/170052/
State New
Headers show

Comments

Alan Modra - July 10, 2012, 6:09 a.m.
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.

Patch

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;