Hi,

In MIPS (and similarly for other RISC architectures) to load an absolute 
address of an object
requires a two instruction sequence: one instruction to load the high part of 
the object's address, 
and one instruction to load the low part of the object's address.  Typically 
the result from the 
calculation of the high part of the address will only be used by one 
instruction to load 
the low part of the address.  However, in certain situations (for example when 
loading or
storing double word values) the result of computing the high part of the 
address can be 
used by multiple instructions to load the low parts of an address at different 
offsets.  Lets 
show this with an example C program.

struct
{
  short s;
  unsigned long long l;
} h;

void foo (void)
{
  h.l = 0;
}

When this is compiled for MIPS it produces the following assembly:

        lui     $2,%hi(h+8)
        sw      $0,%lo(h+8)($2)
        jr      $31
        sw      $0,%lo(h+12)($2)

          ...

        .globl  h
        .section        .bss,"aw",@nobits
        .align  3
        .type   h, @object
        .size   h, 16
h:
        .space  16


Notice here that the high part of the address of object h is loaded into 
register $2,
and this is then used as part of the low part calculation by two the sw 
instructions which each
have different offsets.  In MIPS the value of a low part calculation is treated 
as a signed value.
It is therefore valid to use the result of a high part calculation with 
multiple low part calculations
containing different offsets so long as when adding the result of the high part 
to the each of the
sign extended low parts we get valid addresses.

However, if we add the packed attribute to the h structure, the fields of the 
structure will 
not be naturally aligned and we can break the previous condition.  Lets explain 
this in more 
detail with the following C program.

struct __attribute__((packed))
{
 short s;
 unsigned long long l;
} h;

void foo (void)
{
 h.l = 0;
}

When this is compiled for MIPS it produces the following assembly:

          lui     $2,%hi(h)
        addiu   $4,$2,%lo(h+2)
        addiu   $3,$2,%lo(h+6)
        swl     $0,3($4)
        swr     $0,%lo(h+2)($2)
        swl     $0,3($3)
        jr      $31
        swr     $0,%lo(h+6)($2)

        ...

          .globl  h
        .section        .bss,"aw",@nobits
        .align  2
        .type   h, @object
        .size   h, 10
h:
        .space  10


There are two things to highlight here.  Firstly the alignment of the h 
structure has decreased
from 8 bytes to 4 bytes.  Secondly we have a low part calculation which adds an 
offset of 6
to the address of the h structure which is greater than its alignment.

When the MIPS linker resolves a HI relocation (i.e. %hi(h)) it finds the next 
LO 
relocation (i.e. %lo(h+2)) in the relocation table and using the information 
from both 
of these relocations it computes the object's address and extracts its high 
part.  Then, when the 
MIPS linker resolves a LO relocation it adds the offset to the object's address 
and then extracts 
the low part.

Lets assume that object h has an address of 0x80007ffc.  When the MIPS linker 
resolves the value 
of the HI relocation for object h, it will also use the value of the LO 
relocation for object h 
with an offset of 2.  The high part value is therefore:

HIGH (0x80007ffc + 2) = HIGH (0x80007ffe) = 0x8000


Then the MIPS linker resolves the value of LO relocation for object h with an 
offset of 2:

LO (0x80007ffc + 2) = LO (0x80007ffe) = 0x7ffe


Finally the MIPS linker resolves the value of the LO relocation for object h 
with an offset of 6:

LO (0x80007ffc + 6) = LO (0x80008002) = 0x8002

In MIPS the value of a LO relocation is treated as a signed value, so when the 
program is run the address 
of h+6 will be 0x7fff8002 when it should be 0x80008002.



To fix this issue I have changed the mips_split_symbol function in the case 
when it generates a set of 
instructions to compute the high and low part of a symbol's address.  If the 
symbol is unaligned the 
low-part calculation is forced into a register.  I have also added a condition 
into mips_classify_address 
that prevents lo_sums from being used if they contain an unaligned symbol.  
This stops GCC from trying to 
merge the result of a lo_sum that is currently in register back into an address 
calculation.

I have tested the patch on the mips-mti-elf toolchain and there have been no 
regressions.

The patch and ChangeLog are below.


Ok to commit?

Many thanks,



Andrew



gcc/
        * config/mips/mips.c (mips_valid_lo_sum_p): New function.
        (mips_classify_address): Call mips_valid_lo_sum_p to check
        if we have a valid lo_sum.
        (mips_split_symbol): Force the lo_sum to a register if
        mips_valid_lo_sum_p is false.
    
testsuite/
        * gcc.target/mips/hi-lo-reloc-offset1: New test.
        * gcc.target/mips/hi-lo-reloc-offset2: New test.
        * gcc.target/mips/hi-lo-reloc-offset3: New test.
        * gcc.target/mips/hi-lo-reloc-offset4: New test.
        * gcc.target/mips/hi-lo-reloc-offset5: New test.
        * gcc.target/mips/hi-lo-reloc-offset6: New test.
        * gcc.target/mips/hi-lo-reloc-offset7: New test.
        * gcc.target/mips/hi-lo-reloc-offset8: New test.
        * gcc.target/mips/hi-lo-reloc-offset9: New test.
        * gcc.target/mips/hi-lo-reloc-offset10: New test.
        * gcc.target/mips/hi-lo-reloc-offset11: New test.



diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 6cdda3b..f07e433 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -2354,6 +2354,38 @@ mips_valid_lo_sum_p (enum mips_symbol_type symbol_type, 
machine_mode mode)
   return true;
 }
 
+/* Return true if X in LO_SUM (REG, X) is a valid.  */
+
+bool
+mips_valid_lo_sum_lo_part_p (machine_mode mode, rtx reg, rtx x)
+{
+   rtx symbol = NULL;
+
+   if (mips_abi != ABI_32)
+     return true;
+
+   if (mode == BLKmode)
+     return true;
+
+   if (reg && REG_P (reg) && REGNO (reg) == GLOBAL_POINTER_REGNUM)
+     return true;
+
+   if (GET_CODE (x) == CONST
+       && GET_CODE (XEXP (x, 0)) == PLUS
+       && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF)
+     symbol = XEXP (XEXP (x, 0), 0);
+   else if (GET_CODE (x) == SYMBOL_REF)
+     symbol = x;
+
+   if (symbol
+       && SYMBOL_REF_DECL (symbol)
+       && (GET_MODE_ALIGNMENT (mode) / BITS_PER_UNIT) >
+          (DECL_ALIGN_UNIT (SYMBOL_REF_DECL (symbol))))
+     return false;
+
+   return true;
+}
+
 /* Return true if X is a valid address for machine mode MODE.  If it is,
    fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
    effect.  */
@@ -2394,7 +2426,8 @@ mips_classify_address (struct mips_address_info *info, 
rtx x,
       info->symbol_type
        = mips_classify_symbolic_expression (info->offset, SYMBOL_CONTEXT_MEM);
       return (mips_valid_base_register_p (info->reg, mode, strict_p)
-             && mips_valid_lo_sum_p (info->symbol_type, mode));
+             && mips_valid_lo_sum_p (info->symbol_type, mode)
+             && mips_valid_lo_sum_lo_part_p (mode, info->reg, info->offset));
 
     case CONST_INT:
       /* Small-integer addresses don't occur very often, but they
@@ -3143,6 +3176,8 @@ mips_split_symbol (rtx temp, rtx addr, machine_mode mode, 
rtx *low_out)
                high = gen_rtx_HIGH (Pmode, copy_rtx (addr));
                high = mips_force_temporary (temp, high);
                *low_out = gen_rtx_LO_SUM (Pmode, high, addr);
+                if (!mips_valid_lo_sum_lo_part_p (mode, NULL, addr))
+                 *low_out = mips_force_temporary (temp, *low_out);
                break;
              }
          return true;
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c
new file mode 100644
index 0000000..dc81fd9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset1.c
@@ -0,0 +1,14 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler-not "%lo\\(h\\+6\\)" } } */
+
+struct __attribute__((packed))
+{
+ short s;
+ unsigned long long l;
+} h;
+
+void foo (void)
+{
+ h.l = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c
new file mode 100644
index 0000000..da80b67
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset10.c
@@ -0,0 +1,19 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32 -mfpxx isa=2" } */
+/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */
+/* { dg-final { scan-assembler "%lo\\(f\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_d\\)" } } */
+
+double d;
+double __attribute__((aligned(1))) a_d;
+float f;
+float __attribute__((aligned(1))) a_f;
+
+void foo (void)
+{
+ d = 0;
+ f = 0;
+ a_d = 0;
+ a_f = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c
new file mode 100644
index 0000000..b2679f9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset11.c
@@ -0,0 +1,17 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */
+
+struct __attribute__((packed))
+{
+ char c;
+ short s;
+ int i;
+} h;
+
+void foo (void)
+{
+ h.c = 0;
+ h.s = 0;
+ h.i = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c
new file mode 100644
index 0000000..1b9a0b1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset2.c
@@ -0,0 +1,13 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler-not "%lo\\(a_l\\+4\\)" } } */
+/* { dg-final { scan-assembler "%lo\\(l\\+4\\)" } } */
+
+unsigned long long l;
+unsigned long long __attribute__((aligned(1))) a_l;
+
+void foo (void)
+{
+ l = 0;
+ a_l = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c
new file mode 100644
index 0000000..a01c1d8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset3.c
@@ -0,0 +1,15 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler-not "%lo\\(a_l\\+4\\)" } } */
+/* { dg-final { scan-assembler "%lo\\(l\\+4\\)" } } */
+
+long long l;
+double d;
+
+long long __attribute__((aligned(1))) a_l;
+double __attribute__((aligned(1))) a_d;
+
+void foo (void)
+{
+   d = (double) l;
+   a_d = (double) a_l;
+}
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c
new file mode 100644
index 0000000..496c078
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset4.c
@@ -0,0 +1,16 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */
+
+struct __attribute__((packed))
+{
+ short s;
+ double d;
+ float f;
+} h;
+
+void foo (void)
+{
+ h.d = 0;
+ h.f = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c
new file mode 100644
index 0000000..828c328
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset5.c
@@ -0,0 +1,20 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */
+/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(f\\)" } } */
+/* { dg-final { scan-assembler-not "%lo\\(a_d\\+4\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */
+
+double d;
+double __attribute__((aligned(1))) a_d;
+
+float f;
+float __attribute__((aligned(1))) a_f;
+
+void foo (void)
+{
+ d = 0;
+ f = 0;
+ a_d = 0;
+ a_f = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c
new file mode 100644
index 0000000..2eb8f9a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset6.c
@@ -0,0 +1,25 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32" } */
+/* { dg-final { scan-assembler "\tsb\t\\\$\[0-9\],%lo\\(c\\)" } } */
+/* { dg-final { scan-assembler "\tsh\t\\\$\[0-9\],%lo\\(s\\)" } } */
+/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(i\\)" } } */
+/* { dg-final { scan-assembler "\tsb\t\\\$\[0-9\],%lo\\(a_c\\)" } } */
+/* { dg-final { scan-assembler-not "\tsh\t\\\$\[0-9\],%lo\\(a_s\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_i\\)" } } */
+
+char __attribute__((aligned(1))) a_c;
+short __attribute__((aligned(1))) a_s;
+int __attribute__((aligned(1))) a_i;
+char c;
+short s;
+int i;
+
+void foo (void)
+{
+ c = 0;
+ s = 0;
+ i = 0;
+ a_c = 0;
+ a_s = 0;
+ a_i = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c
new file mode 100644
index 0000000..8f348fb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset7.c
@@ -0,0 +1,16 @@
+/* { dg-options "-mcode-readable=no -mabi=32 -mfp32 isa=1" } */
+/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */
+
+struct __attribute__((packed))
+{
+ short s;
+ double d;
+ float f;
+} h;
+
+void foo (void)
+{
+ h.d = 0;
+ h.f = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c
new file mode 100644
index 0000000..9070b53
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset8.c
@@ -0,0 +1,20 @@
+/* { dg-options "-mcode-readable=no -mno-gpopt -mabi=32 -mfp32 isa=1" } */
+/* { dg-final { scan-assembler "%lo\\(d\\+4\\)" } } */
+/* { dg-final { scan-assembler "\tsw\t\\\$\[0-9\],%lo\\(f\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_d\\)" } } */
+/* { dg-final { scan-assembler-not "\tsw\t\\\$\[0-9\],%lo\\(a_f\\)" } } */
+
+double d;
+double __attribute__((aligned(1))) a_d;
+
+float f;
+float __attribute__((aligned(1))) a_f;
+
+void foo (void)
+{
+ d = 0;
+ f = 0;
+ a_d = 0;
+ a_f = 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c 
b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c
new file mode 100644
index 0000000..ee11366
--- /dev/null
+++ b/gcc/testsuite/gcc.target/mips/hi-lo-reloc-offset9.c
@@ -0,0 +1,16 @@
+/* { dg-options "-mcode-readable=no -mabi=32 -mfpxx isa=2" } */
+/* { dg-final { scan-assembler-not "%lo\\(h\\+\[1-9\]\\)" } } */
+
+struct __attribute__((packed))
+{
+ short s;
+ double d;
+ float f;
+} h;
+
+void foo (void)
+{
+ h.d = 0;
+ h.f = 0;
+}
+

Reply via email to