This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [PATCH, MIPS] Relax alignment check for LDPC
- From: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Fri, 19 Dec 2014 15:20:59 +0000
- Subject: RE: [PATCH, MIPS] Relax alignment check for LDPC
- Authentication-results: sourceware.org; auth=none
- References: <6D39441BF12EF246A7ABCE6654B0235320F882C9 at LEMAIL01 dot le dot imgtec dot org> <87oar3fhln dot fsf at googlemail dot com> <6D39441BF12EF246A7ABCE6654B0235320F88AD0 at LEMAIL01 dot le dot imgtec dot org> <87vbl7erv7 dot fsf at googlemail dot com>
Richard Sandiford <rdsandiford@googlemail.com> writes:
> Matthew Fortune <Matthew.Fortune@imgtec.com> writes:
> > Richard Sandiford <rdsandiford@googlemail.com> writes:
> >> only safe if the target has 8-byte alignment at assembly time. I.e.
> the
> >> section that the target is in should have at least 8 bytes alignment
> and
> >> the symbol itself should have an offset that's divisible by 8.
> >>
> >> So I think we should either check S_GET_VALUE (fx_addsy) + fx_offset is
> >> aligned or (probably better, if it works) check that the low 3 bits of
> >> *valP are the same as the low three bits of:
> >>
> >> fixP->fx_where + fixP->fx_frag->fr_address
> >
> > OK. I almost did something like this but essentially wasn't confident
> enough.
> > I'm not sure I follow on the low three bits matching part?
>
> It should have been the low 3 bits of -*valP. I just thought that any
> "misalignment" from 8 bytes could only come directly from the address
> of the LDPC itself. I'm happy with not doing that though.
>
> > So the extra questions are is it OK that:
> >
> > LD <Reg>,<off>($pc) requires 'off' to be a multiple of 8 and the
> calculation
> > here is ($pc&~7 + off).
> >
> 2> LDPC <reg>, <4-byte-aligned-label>+<4-byte-multiple-offset> is
> > meaningless and will break for REL.
> >
> > Personally I think the former is not overly harmful but I suspect some
> would
> > prefer the latter to raise an error rather than silently lose bits.
>
> Yeah. I take the point about n32 and n64 being RELA, but we also have
> o64 and 64-bit EABI, both of which are REL. How about requiring the
> symbol and offset to be individually aligned?
Indeed, o64 and EABI suffer from a lack of love from me. I'm happy with
applying the alignment check to both symbol and offset. Patch below.
I'm just running a wider set of configurations on it but the basic
mips64-linux-gnu is OK.
Thanks,
Matthew
gas/
* config/tc-mips.c (md_apply_fix): Apply alignment check
to the symbol and offset rather than *valP for
BFD_RELOC_MIPS_18_PCREL_S3. Also update the error message
for BFD_RELOC_MIPS_19_PCREL_S2.
gas/testsuite/
* gas/mips/r6-64.s: Remove .align directives from LDPC
instructions and add further tests for LDPC.
* gas/mips/r6-64-n32.d: remove the NOPs from LDPC expected
output and update for new tests.
* gas/mips/r6-64-n64.d: Likewise.
* gas/mips/ldpc-unalign.l: New file.
* gas/mips/ldpc-unalign.s: Likewise.
* gas/mips/mips.exp: Run ldpc-unalign test.
---
gas/config/tc-mips.c | 12 ++++++++----
gas/testsuite/gas/mips/ldpc-unalign.l | 9 +++++++++
gas/testsuite/gas/mips/ldpc-unalign.s | 18 ++++++++++++++++++
gas/testsuite/gas/mips/mips.exp | 1 +
gas/testsuite/gas/mips/r6-64-n32.d | 17 ++++++++++-------
gas/testsuite/gas/mips/r6-64-n64.d | 25 +++++++++++++++++--------
gas/testsuite/gas/mips/r6-64.s | 19 ++++++++++++-------
7 files changed, 75 insertions(+), 26 deletions(-)
create mode 100644 gas/testsuite/gas/mips/ldpc-unalign.l
create mode 100644 gas/testsuite/gas/mips/ldpc-unalign.s
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index c9266db..8ecf0c6 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -15009,10 +15009,14 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
break;
case BFD_RELOC_MIPS_18_PCREL_S3:
- if ((*valP & 0x7) != 0)
+ if ((S_GET_VALUE (fixP->fx_addsy) & 0x7) != 0)
as_bad_where (fixP->fx_file, fixP->fx_line,
- _("PC-relative access to misaligned address (%lx)"),
- (long) *valP);
+ _("PC-relative access using misaligned symbol (%lx)"),
+ (long) S_GET_VALUE (fixP->fx_addsy));
+ if ((fixP->fx_offset & 0x7) != 0)
+ as_bad_where (fixP->fx_file, fixP->fx_line,
+ _("PC-relative access using misaligned offset (%lx)"),
+ (long) fixP->fx_offset);
gas_assert (!fixP->fx_done);
break;
@@ -15021,7 +15025,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
if ((*valP & 0x3) != 0)
as_bad_where (fixP->fx_file, fixP->fx_line,
_("PC-relative access to misaligned address (%lx)"),
- (long) *valP);
+ (long) (S_GET_VALUE (fixP->fx_addsy) + fixP->fx_offset));
gas_assert (!fixP->fx_done);
break;
diff --git a/gas/testsuite/gas/mips/ldpc-unalign.l b/gas/testsuite/gas/mips/ldpc-unalign.l
new file mode 100644
index 0000000..ddc515b
--- /dev/null
+++ b/gas/testsuite/gas/mips/ldpc-unalign.l
@@ -0,0 +1,9 @@
+.*: Assembler messages:
+.*:2: Error: PC-relative access using misaligned offset \(4\)
+.*:3: Error: PC-relative access using misaligned offset \(4\)
+.*:4: Error: PC-relative access using misaligned symbol \(1c\)
+.*:5: Error: PC-relative access using misaligned symbol \(1c\)
+.*:6: Error: PC-relative access using misaligned symbol \(1c\)
+.*:6: Error: PC-relative access using misaligned offset \(4\)
+.*:7: Error: PC-relative access using misaligned symbol \(1c\)
+.*:7: Error: PC-relative access using misaligned offset \(4\)
diff --git a/gas/testsuite/gas/mips/ldpc-unalign.s b/gas/testsuite/gas/mips/ldpc-unalign.s
new file mode 100644
index 0000000..8b926cf
--- /dev/null
+++ b/gas/testsuite/gas/mips/ldpc-unalign.s
@@ -0,0 +1,18 @@
+ .text
+ ldpc $4, 1f+4
+ ldpc $4, 1f+4
+ ldpc $4, 2f
+ ldpc $4, 2f
+ ldpc $4, 2f+4
+ ldpc $4, 2f+4
+ .align 3
+1:
+ nop
+2:
+ nop
+ nop
+ nop
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 2
+ .space 8
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index b0de13a..d2cbc50 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1450,6 +1450,7 @@ if { [istarget mips*-*-vxworks*] } {
run_dump_test_arches "r6-n64" [mips_arch_list_matching mips64r6]
run_dump_test_arches "r6-64-n32" [mips_arch_list_matching mips64r6]
run_dump_test_arches "r6-64-n64" [mips_arch_list_matching mips64r6]
+ run_list_test_arches "ldpc-unalign" "-64" [mips_arch_list_matching mips64r6]
}
run_list_test_arches "r6-removed" "-32" [mips_arch_list_matching mips32r6]
run_list_test_arches "r6-64-removed" [mips_arch_list_matching mips64r6]
diff --git a/gas/testsuite/gas/mips/r6-64-n32.d b/gas/testsuite/gas/mips/r6-64-n32.d
index 6d471c6..fcef172 100644
--- a/gas/testsuite/gas/mips/r6-64-n32.d
+++ b/gas/testsuite/gas/mips/r6-64-n32.d
@@ -44,18 +44,21 @@ Disassembly of section .text:
[ ]*78: R_MIPS_PC19_S2 L0.+0xffffc
0+007c <[^>]*> ec940000 lwupc a0,fff0007c <[^>]*>
0+0080 <[^>]*> ec93ffff lwupc a0,0010007c <[^>]*>
-0+0084 <[^>]*> 00000000 nop
+0+0084 <[^>]*> ec980000 ldpc a0,00000080 <[^>]*>
+[ ]*84: R_MIPS_PC18_S3 \.L1.1
0+0088 <[^>]*> ec980000 ldpc a0,00000088 <[^>]*>
[ ]*88: R_MIPS_PC18_S3 \.L1.1
0+008c <[^>]*> 00000000 nop
0+0090 <[^>]*> ec980000 ldpc a0,00000090 <[^>]*>
-[ ]*90: R_MIPS_PC18_S3 L0.-0x100000
-0+0094 <[^>]*> 00000000 nop
+[ ]*90: R_MIPS_PC18_S3 \.L3.1-0x100000
+0+0094 <[^>]*> ec980000 ldpc a0,00000090 <[^>]*>
+[ ]*94: R_MIPS_PC18_S3 \.L3.1-0x100000
0+0098 <[^>]*> ec980000 ldpc a0,00000098 <[^>]*>
-[ ]*98: R_MIPS_PC18_S3 L0.+0xffff8
-0+009c <[^>]*> 00000000 nop
+[ ]*98: R_MIPS_PC18_S3 \.L3.2\+0xffff8
+0+009c <[^>]*> ec980000 ldpc a0,00000098 <[^>]*>
+[ ]*9c: R_MIPS_PC18_S3 \.L3.2\+0xffff8
0+00a0 <[^>]*> ec9a0000 ldpc a0,fff000a0 <[^>]*>
-0+00a4 <[^>]*> 00000000 nop
+0+00a4 <[^>]*> ec9a0000 ldpc a0,fff000a0 <[^>]*>
0+00a8 <[^>]*> ec99ffff ldpc a0,001000a0 <[^>]*>
-0+00ac <[^>]*> 00000000 nop
+0+00ac <[^>]*> ec99ffff ldpc a0,001000a0 <[^>]*>
\.\.\.
diff --git a/gas/testsuite/gas/mips/r6-64-n64.d b/gas/testsuite/gas/mips/r6-64-n64.d
index 2202820..42d8e53 100644
--- a/gas/testsuite/gas/mips/r6-64-n64.d
+++ b/gas/testsuite/gas/mips/r6-64-n64.d
@@ -50,24 +50,33 @@ Disassembly of section .text:
[ ]*78: R_MIPS_NONE \*ABS\*\+0xffffc
0+007c <[^>]*> ec940000 lwupc a0,f+ff0007c <[^>]*>
0+0080 <[^>]*> ec93ffff lwupc a0,0+010007c <[^>]*>
-0+0084 <[^>]*> 00000000 nop
+0+0084 <[^>]*> ec980000 ldpc a0,0+0000080 <[^>]*>
+[ ]*84: R_MIPS_PC18_S3 .L1.1
+[ ]*84: R_MIPS_NONE \*ABS\*
+[ ]*84: R_MIPS_NONE \*ABS\*
0+0088 <[^>]*> ec980000 ldpc a0,0+0000088 <[^>]*>
-[ ]*88: R_MIPS_PC18_S3 \.L1.1
+[ ]*88: R_MIPS_PC18_S3 .L1.1
[ ]*88: R_MIPS_NONE \*ABS\*
[ ]*88: R_MIPS_NONE \*ABS\*
0+008c <[^>]*> 00000000 nop
0+0090 <[^>]*> ec980000 ldpc a0,0+0000090 <[^>]*>
-[ ]*90: R_MIPS_PC18_S3 L0.-0x100000
+[ ]*90: R_MIPS_PC18_S3 .L3.1-0x100000
[ ]*90: R_MIPS_NONE \*ABS\*-0x100000
[ ]*90: R_MIPS_NONE \*ABS\*-0x100000
-0+0094 <[^>]*> 00000000 nop
+0+0094 <[^>]*> ec980000 ldpc a0,0+0000090 <[^>]*>
+[ ]*94: R_MIPS_PC18_S3 .L3.1-0x100000
+[ ]*94: R_MIPS_NONE \*ABS\*-0x100000
+[ ]*94: R_MIPS_NONE \*ABS\*-0x100000
0+0098 <[^>]*> ec980000 ldpc a0,0+0000098 <[^>]*>
-[ ]*98: R_MIPS_PC18_S3 L0.\+0xffff8
+[ ]*98: R_MIPS_PC18_S3 .L3.2\+0xffff8
[ ]*98: R_MIPS_NONE \*ABS\*\+0xffff8
[ ]*98: R_MIPS_NONE \*ABS\*\+0xffff8
-0+009c <[^>]*> 00000000 nop
+0+009c <[^>]*> ec980000 ldpc a0,0+0000098 <[^>]*>
+[ ]*9c: R_MIPS_PC18_S3 .L3.2\+0xffff8
+[ ]*9c: R_MIPS_NONE \*ABS\*\+0xffff8
+[ ]*9c: R_MIPS_NONE \*ABS\*\+0xffff8
0+00a0 <[^>]*> ec9a0000 ldpc a0,f+ff000a0 <[^>]*>
-0+00a4 <[^>]*> 00000000 nop
+0+00a4 <[^>]*> ec9a0000 ldpc a0,f+ff000a0 <[^>]*>
0+00a8 <[^>]*> ec99ffff ldpc a0,0+01000a0 <[^>]*>
-0+00ac <[^>]*> 00000000 nop
+0+00ac <[^>]*> ec99ffff ldpc a0,0+01000a0 <[^>]*>
\.\.\.
diff --git a/gas/testsuite/gas/mips/r6-64.s b/gas/testsuite/gas/mips/r6-64.s
index 7a97ad2..2d577d8 100644
--- a/gas/testsuite/gas/mips/r6-64.s
+++ b/gas/testsuite/gas/mips/r6-64.s
@@ -40,19 +40,24 @@
lwu $4, (-262144 << 2)($pc)
lwu $4, (262143 << 2)($pc)
- .align 3
ldpc $4, 1f
- .align 3
- ldpc $4, .+(-131072 << 3)
- .align 3
- ldpc $4, .+(131071 << 3)
- .align 3
+ ldpc $4, 1f
+ .align 3
+3:
+ ldpc $4, 3b+(-131072 << 3)
+ ldpc $4, 3b+(-131072 << 3)
+ .align 3
+3:
+ ldpc $4, 3b+(131071 << 3)
+ ldpc $4, 3b+(131071 << 3)
ld $4, (-131072 << 3)($pc)
- .align 3
+ ld $4, (-131072 << 3)($pc)
+ ld $4, (131071 << 3)($pc)
ld $4, (131071 << 3)($pc)
.align 3
1:
nop
+ nop
# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
.align 2
--
1.9.4