This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [Patch] Gas support for MIPS Compact EH



> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Monday, March 17, 2014 9:52 AM
> To: Moore, Catherine
> Cc: Schmidt, Bernd; binutils@sourceware.org
> Subject: Re: [Patch] Gas support for MIPS Compact EH
> 
> "Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> >> -----Original Message-----
> >> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> >> Sent: Tuesday, February 25, 2014 3:29 AM
> >> To: Moore, Catherine
> >> Cc: Schmidt, Bernd; binutils@sourceware.org
> >> Subject: Re: [Patch] Gas support for MIPS Compact EH
> >>
> >> "Moore, Catherine" <Catherine_Moore@mentor.com> writes:
> >> >> -----Original Message-----
> >> >> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> >> >> Sent: Thursday, February 20, 2014 6:37 AM
> >> >> To: Moore, Catherine
> >> >> Cc: Schmidt, Bernd; binutils@sourceware.org
> >> >> Subject: Re: [Patch] Gas support for MIPS Compact EH
> >> >>
> >> >> >
> >> >> > I'd like to take this approach in the next patch:
> >> >> > 1. Keep the R_MIPS_EH relocation 2. Let the linker choose the
> >> >> > appropriate encoding 3. Clean up the assembler inconsistencies
> >> >>
> >> >> That's OK with me, but just to clarify: (3) IMO means that
> >> >> R_MIPS_EH is never associated with a specific encoding in the
> assembler.  I.e.
> >> >> R_MIPS_EH is purely for an as-yet unknown encoding that is chosen
> >> >> by the linker rather than the assembler or the assembly author.
> >> >> And AIUI the only place that happens is in .eh_frame_entry.
> >> >>
> >> >> Perhaps one way of doing that would to have a generic
> >> >> BFD_RELOC_EH_FRAME_HDR_32 that R_MIPS_EH maps to.  Then
> when
> >> emitting
> >> >> the .eh_frame_entry addresses, the assembler unconditionally uses
> >> >> that BFD_RELOC_ rather than a target hook.
> >> >>
> >> >> What do you plan to do for .ehword?  Since the assembler generates
> >> >> the .eh_frame_entry itself, and since R_MIPS_EH should only be
> >> >> used there (since that's the only place where the linker controls
> >> >> the encoding), I don't think there are any valid uses of an
> >> >> R_MIPS_EH-
> >> producing .ehword.
> >> >>
> >> >
> >> > The current version of the patch generates a BFD_RELOC_32_PCREL
> >> > when it sees the .ehword directive.
> >> > That should be okay going forward, agreed?
> >>
> >> Well, PC-relative can be expressed as:
> >>
> >> 	.word	foo-.
> >>
> >> I think instead we should make that work and drop .ehword, to avoid
> >> confusion with the old behaviour.
> >>
> > I've hit a snag with the removal of the .ehword directive.  You
> > suggested PC-relative, but does the MIPS assembler handle that?
> >
> > The current implementation has  this:
> >
> > .ehword	_ZTIi
> >
> > where _ZTIi is defined in libstdc++.a(fundamental_type_info.o).
> >
> > Changing the assembler to:
> >
> > .word	_ZTIi-.
> >
> > results in:
> > eh8.s:212: Error: can't resolve `_ZTIi' {*UND* section} - `L0'
> > {.gnu_extab section}
> 
> Right.  By "make that work" I meant getting it to assemble :-)
> 
> The point is that until you guys added R_MIPS_PC32 we had no 32-bit PC-
> relative relocation, so ".word X-." wasn't supported.  We should support it
> now that we do have a 32-bit PC-relative relocation we can use.
> 
> Try assembling the same thing on x86_64 and you'll see it produces an
> R_X86_64_PC32.  We should do the same with R_MIPS_PC32.

I've been working on getting ".word X-." to work in the assembler.  It looks like I can produce the relocation, but I'm seeing failures for  the lui-1.s and lui-2.s tests.
I've added these two definitions to tc-mips.h:

+#define DIFF_EXPR_OK
+#define md_register_arithmetic 0

The first thing is that I'm not sure if the setting for md_register_arithmetic is correct.  The MIPS port currently uses the default of 1 (that's got to be wrong).  
There is interaction between DIFF_EXPR_OK, md_register_arithmetic and the FORCE_RELOC macros that is confusing.

This definition of TC_FORCE_RELOCATION_SUB_LOCAL (write.c) needs to have md_register_arithmetic defined to zero, when DIFF_EXPR_OK is set.

#ifndef TC_FORCE_RELOCATION_SUB_LOCAL
#ifdef DIFF_EXPR_OK
#define TC_FORCE_RELOCATION_SUB_LOCAL(FIX, SEG) \
  (!md_register_arithmetic && (SEG) == reg_section)
#else
#define TC_FORCE_RELOCATION_SUB_LOCAL(FIX, SEG) 1
#endif
#endif

Needs to have md_register_arithmetic set to zero if DIFF_EXPR_OK is set.

Assuming that defining md_register_arithmetic to zero is the correct thing to do, then the error message in md_pcrel_from looks wrong:

long
md_pcrel_from (fixS *fixP)
{
  valueT addr = fixP->fx_where + fixP->fx_frag->fr_address;
  switch (fixP->fx_r_type)
    {
    case BFD_RELOC_MICROMIPS_7_PCREL_S1:
    case BFD_RELOC_MICROMIPS_10_PCREL_S1:
      /* Return the address of the delay slot.  */
      return addr + 2;

    case BFD_RELOC_MICROMIPS_16_PCREL_S1:
    case BFD_RELOC_MICROMIPS_JMP:
    case BFD_RELOC_16_PCREL_S2:
    case BFD_RELOC_MIPS_JMP:
      /* Return the address of the delay slot.  */
      return addr + 4;

    case BFD_RELOC_32_PCREL:
      return addr;

    default:
      /* We have no relocation type for PC relative MIPS16 instructions.  */
      if (fixP->fx_addsy && S_GET_SEGMENT (fixP->fx_addsy) != now_seg)
        as_bad_where (fixP->fx_file, fixP->fx_line,
                      _("PC relative MIPS16 instruction references"
                        " a different section"));
      return addr;
    }
}


fixup_segment now treats many relocs as "possibly pc-relative" and waits for md_apply_fix to make the call.
Should the default be to return addr for any reloc that's encountered and try to process errors later?  I don't have any state on how a MIPS16 reloc ends up here.

Then there is an assert in md_apply_fix that assumes a pc relative reloc is one of   a handful of relocs.  The fx_pcrel field needs to be reset for relocations that aren't pc-relative.  Is md_apply_fix the correct place to do that?

Then the test cases in question.   This is lui-1.s:

        .text
foo:
        lui     $2, -1
        lui     $2, 65536
        lui     $2, 0x10000000000000000
        lui     $2, $3
        lui     $2, ($3)
        lui     $2, 0+$3  <<--- this statement
        lui     $2, (($3))

Old error message:
lui-1.s:10: Error: register value used as expression `lui $2,0+$3'

New error message:
None, for the entire test case, but a new file with just the offending line causes a assertion failure in bfd_elf32_swap_symbol_out.  (That's wrong, I need to track it down).

Then lui-2.s:

        .text
foo:
        lui     $2, bar - foo
        lui     $2, baz - bar
        lui     $2, foo - baz
        lui     $2, bar / baz

Should any of these be supported now?

Old error messages:
lui-2.s:10: Error: invalid operands (*UND* and *UND* sections) for `/'
lui-2.s:7: Error: can't resolve `bar' {*UND* section} - `foo' {.text section}
lui-2.s:8: Error: can't resolve `baz' {*UND* section} - `bar' {*UND* section}
lui-2.s:9: Error: can't resolve `.text' {.text section} - `baz' {*UND* section}

New error messages:
lui-2.s:10: Error: invalid operands (*UND* and *UND* sections) for `/'
lui-2.s:8: Error: can't resolve `baz' {*UND* section} - `bar' {*UND* section}
lui-2.s:9: Error: can't resolve `.text' {.text section} - `baz' {*UND* section}

Notice that lui $2 bar-foo seems to be accepted.  Should it be?

Thanks for any help. 
Catherine

The WIP patch:

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 74c7a10..24d2a32 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -14057,15 +14057,13 @@ md_pcrel_from (fixS *fixP)
       /* Return the address of the delay slot.  */
       return addr + 4;

-    case BFD_RELOC_32_PCREL:
-      return addr;
-
     default:
-      /* We have no relocation type for PC relative MIPS16 instructions.  */
+#if 0
       if (fixP->fx_addsy && S_GET_SEGMENT (fixP->fx_addsy) != now_seg)
        as_bad_where (fixP->fx_file, fixP->fx_line,
-                     _("PC relative MIPS16 instruction references"
+                     _("PC relative instruction references"
                        " a different section"));
+#endif
       return addr;
     }
 }
@@ -14280,11 +14278,16 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)

   buf = fixP->fx_frag->fr_literal + fixP->fx_where;

-  gas_assert (!fixP->fx_pcrel || fixP->fx_r_type == BFD_RELOC_16_PCREL_S2
-             || fixP->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
-             || fixP->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
-             || fixP->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1
-             || fixP->fx_r_type == BFD_RELOC_32_PCREL);
+  if (fixP->fx_pcrel && fixP->fx_r_type == BFD_RELOC_32)
+    fixP->fx_r_type = BFD_RELOC_32_PCREL;
+
+  if (fixP->fx_pcrel
+      && fixP->fx_r_type != BFD_RELOC_16_PCREL_S2
+      && fixP->fx_r_type != BFD_RELOC_MICROMIPS_7_PCREL_S1
+      && fixP->fx_r_type != BFD_RELOC_MICROMIPS_10_PCREL_S1
+      && fixP->fx_r_type != BFD_RELOC_MICROMIPS_16_PCREL_S1
+      && fixP->fx_r_type != BFD_RELOC_32_PCREL)
+    fixP->fx_pcrel = 0;

   /* Don't treat parts of a composite relocation as done.  There are two
      reasons for this:
diff --git a/gas/config/tc-mips.h b/gas/config/tc-mips.h
index 97627df..de3010e 100644
--- a/gas/config/tc-mips.h
+++ b/gas/config/tc-mips.h
@@ -136,6 +136,9 @@ extern int mips_fix_adjustable (struct fix *);
 #define EXTERN_FORCE_RELOC                     \
   (OUTPUT_FLAVOR == bfd_target_elf_flavour)

+#define DIFF_EXPR_OK
+#define md_register_arithmetic 0
+







Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]