Re: FYI: patches for powerpc-aix...

[Richard Sandiford <rsandifo at linux dot vnet dot ibm dot com>]

Tristan Gingold <gingold@adacore.com> writes:
> On May 16, 2013, at 1:59 AM, Alan Modra wrote:
>
>> On Wed, May 15, 2013 at 04:38:10PM +0200, Tristan Gingold wrote:
>>> bfd/
>>> 2013-05-15  Tristan Gingold  <gingold@adacore.com>
>>> 
>>> 	* coff-rs6000.c (xcoff_howto_table): Add R_POS_16 entry.
>> 
>> I'm a little curious as to why you didn't use the R_RL howto, which
>> only differs from this one in the type and name.  The type won't be
>> used to set up an external reloc, will it?  (And if we do get an
>> external reloc, the only info I have on R_RL says "treated the same as
>> the R_POS relocation type".)

Yeah, which makes the R_RL and R_RLA entries look a bit odd.
The R_POS howto entry is (rightly) a full address field,
whereas R_RL and R_RLA have masks of just 0xffff.

>> Also, doesn't coff64-rs6000.c need an equivalent patch?
>
> So ok for this change ?

Sorry for playing catch-up, but I think we should try to avoid a fake
reloc type if at all possible.  It's hard to contain once we expose
the reloc at this level.

E.g. at the moment:

	.short	x

gives:

        Error: reloc 5 not supported by object file format

which isn't a great error message, but is at least an error. :-)
After the patch it is silently accepted and produces an R_RL reloc.

It's a bit drastic, and probably a bit ugly, but how about something
like the patch below?  Tested on powerpc-linux-gnu, powerc64-linux-gnu
and powerpc-ibm-aix6.1 (binutils only).

Something like the other part (XMC_TC0) is still needed.  I haven't had
a chance to look at that yet.

Thanks,
Richard


gas/
	* config/tc-ppc.c (ppc_reloc_info): New structure.
	(ppc_get_reloc_info): New function.
	(md_assemble): Use it instead of bfd_reloc_type_lookup.
	(md_apply_fix): Apply PC-relative conversion before code/data switch,
	using ppc_get_reloc_info to get the PC-relative form.  Use the
	field widths and values created there.

Index: gas/config/tc-ppc.c
===================================================================
--- gas/config/tc-ppc.c	2013-05-16 12:39:20.357702804 +0100
+++ gas/config/tc-ppc.c	2013-05-16 13:38:42.142955304 +0100
@@ -1824,7 +1824,157 @@ ppc_insert_operand (unsigned long insn,
 
   return insn;
 }
+
+/* Information about a relocation.  */
+struct ppc_reloc_info {
+  /* The relocation being described.  */
+  bfd_reloc_code_real_type r_type;
+
+  /* The pc-relative form of R_TYPE, or BFD_RELOC_NONE if none.
+     R_TYPE is PC-relative iff R_TYPE == PCREL_TYPE.  */
+  bfd_reloc_code_real_type pcrel_type;
+
+  /* If the full (address-sized) relocation value was known, this field
+     gives the relocation field value, otherwise it is unused.  */
+  offsetT field_value;
+
+  /* The size of the relocation field in bytes, or 0 if not known
+     (BFD_RELOC_UNUSED).  */
+  unsigned int size;
+
+  /* The number of significant bits in FIELD_VALUE, or 0 if not known.  */
+  unsigned int value_bits;
+};
+
+/* Describe R_TYPE in INFO, using VALUE to set up INFO->field_value.  */
+
+static void
+ppc_get_reloc_info (bfd_reloc_code_real_type r_type, offsetT value,
+		    struct ppc_reloc_info *info)
+{
+  reloc_howto_type *reloc_howto;
+
+  if (r_type == BFD_RELOC_CTOR)
+    r_type = ppc_obj64 ? BFD_RELOC_64 : BFD_RELOC_32;
+
+  info->r_type = r_type;
+  info->pcrel_type = BFD_RELOC_NONE;
+  info->field_value = value;
+  info->value_bits = 0;
+  switch (r_type)
+    {
+    case BFD_RELOC_UNUSED:
+      /* Used to represent operand values.  */
+      break;
+
+    case BFD_RELOC_8:
+      info->size = 1;
+      info->value_bits = 8;
+      break;
+
+    case BFD_RELOC_16:
+    case BFD_RELOC_16_PCREL:
+      info->pcrel_type = BFD_RELOC_16_PCREL;
+      info->size = 2;
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_32:
+    case BFD_RELOC_32_PCREL:
+      info->pcrel_type = BFD_RELOC_32_PCREL;
+      /* Fall through.  */
+    case BFD_RELOC_RVA:
+      info->size = 4;
+      info->value_bits = 32;
+      break;
+
+    case BFD_RELOC_64:
+    case BFD_RELOC_64_PCREL:
+      info->pcrel_type = BFD_RELOC_64_PCREL;
+      info->size = 8;
+      info->value_bits = 64;
+      break;
+
+    case BFD_RELOC_LO16:
+    case BFD_RELOC_LO16_PCREL:
+      info->pcrel_type = BFD_RELOC_LO16_PCREL;
+      /* Fall through.  */
+#ifdef OBJ_ELF
+    case BFD_RELOC_PPC64_ADDR16_LO_DS:
+#endif
+      info->size = 2;
+    lo16:
+      info->field_value = value & 0xffff;
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC_VLE_LO16A:
+    case BFD_RELOC_PPC_VLE_LO16D:
+      info->size = 4;
+      goto lo16;
+
+    case BFD_RELOC_HI16:
+    case BFD_RELOC_HI16_PCREL:
+      info->pcrel_type = BFD_RELOC_HI16_PCREL;
+      info->size = 2;
+    hi16:
+      info->field_value = PPC_HI (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC_VLE_HI16A:
+    case BFD_RELOC_PPC_VLE_HI16D:
+      info->size = 4;
+      goto hi16;
+
+    case BFD_RELOC_HI16_S:
+    case BFD_RELOC_HI16_S_PCREL:
+      info->pcrel_type = BFD_RELOC_HI16_S_PCREL;
+      info->size = 2;
+    hi16_s:
+      info->field_value = PPC_HA (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC_VLE_HA16A:
+    case BFD_RELOC_PPC_VLE_HA16D:
+      info->size = 4;
+      goto hi16_s;
+
+    case BFD_RELOC_PPC64_HIGHER:
+      info->size = 2;
+      info->field_value = PPC_HIGHER (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHER_S:
+      info->size = 2;
+      info->field_value = PPC_HIGHERA (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHEST:
+      info->size = 2;
+      info->field_value = PPC_HIGHEST (value);
+      info->value_bits = 16;
+      break;
 
+    case BFD_RELOC_PPC64_HIGHEST_S:
+      info->size = 2;
+      info->field_value = PPC_HIGHESTA (value);
+      info->value_bits = 16;
+      break;
+
+    default:
+      reloc_howto = bfd_reloc_type_lookup (stdoutput, r_type);
+      if (!reloc_howto)
+	abort ();
+      if (reloc_howto->pc_relative)
+	info->pcrel_type = r_type;
+      info->size = bfd_get_reloc_size (reloc_howto);
+      break;
+    }
+};
 
 #ifdef OBJ_ELF
 /* Parse @got, etc. and return the desired relocation.  */
@@ -3285,25 +3435,20 @@ md_assemble (char *str)
       fixS *fixP;
       if (fixups[i].reloc != BFD_RELOC_UNUSED)
 	{
-	  reloc_howto_type *reloc_howto;
-	  int size;
+	  struct ppc_reloc_info info;
 	  int offset;
 
-	  reloc_howto = bfd_reloc_type_lookup (stdoutput, fixups[i].reloc);
-	  if (!reloc_howto)
-	    abort ();
-
-	  size = bfd_get_reloc_size (reloc_howto);
-	  offset = target_big_endian ? (insn_length - size) : 0;
+	  ppc_get_reloc_info (fixups[i].reloc, 0, &info);
+	  offset = target_big_endian ? (insn_length - info.size) : 0;
 
-	  if (size < 1 || size > 4)
+	  if (info.size < 1 || info.size > 4)
 	    abort ();
 
 	  fixP = fix_new_exp (frag_now,
 			      f - frag_now->fr_literal + offset,
-			      size,
+			      info.size,
 			      &fixups[i].exp,
-			      reloc_howto->pc_relative,
+			      info.r_type == info.pcrel_type,
 			      fixups[i].reloc);
 	}
       else
@@ -6287,6 +6432,7 @@ ppc_handle_align (struct frag *fragP)
 md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
 {
   valueT value = * valP;
+  struct ppc_reloc_info info;
 
 #ifdef OBJ_ELF
   if (fixP->fx_addsy != NULL)
@@ -6325,6 +6471,38 @@ md_apply_fix (fixS *fixP, valueT *valP,
       as_bad_where (fixP->fx_file, fixP->fx_line, _("expression too complex"));
     }
 
+  ppc_get_reloc_info (fixP->fx_r_type, value, &info);
+  if (fixP->fx_pcrel)
+    {
+      if (info.pcrel_type == BFD_RELOC_NONE)
+	{
+	  if (fixP->fx_addsy)
+	    {
+	      char *sfile;
+	      unsigned int sline;
+
+	      /* Use expr_symbol_where to see if this is an
+		 expression symbol.  */
+	      if (expr_symbol_where (fixP->fx_addsy, &sfile, &sline))
+		as_bad_where (fixP->fx_file, fixP->fx_line,
+			      _("unresolved expression that must"
+				" be resolved"));
+	      else
+		as_bad_where (fixP->fx_file, fixP->fx_line,
+			      _("cannot emit PC relative %s relocation"
+				" against %s"),
+			      bfd_get_reloc_code_name (fixP->fx_r_type),
+			      S_GET_NAME (fixP->fx_addsy));
+	    }
+	  else
+	    as_bad_where (fixP->fx_file, fixP->fx_line,
+			  _("unable to resolve expression"));
+	  fixP->fx_done = 1;
+	}
+      else
+	fixP->fx_r_type = info.pcrel_type;
+    }
+
   if (fixP->fx_pcrel_adjust != 0)
     {
       /* Handle relocs in an insn.  */
@@ -6351,77 +6529,13 @@ md_apply_fix (fixS *fixP, valueT *valP,
 	  && S_GET_SEGMENT (fixP->fx_addsy) != bss_section)
 	{
 	  value = fixP->fx_offset;
+	  ppc_get_reloc_info (fixP->fx_r_type, value, &info);
 	  fixP->fx_done = 1;
 	}
 #endif
-      fieldval = value;
       switch (fixP->fx_r_type)
 	{
 #ifdef OBJ_ELF
-	case BFD_RELOC_PPC64_ADDR16_LO_DS:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  /* fall through */
-#endif
-	case BFD_RELOC_LO16:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_LO16_PCREL;
-	  /* fall through */
-	case BFD_RELOC_LO16_PCREL:
-	case BFD_RELOC_PPC_VLE_LO16A:
-	case BFD_RELOC_PPC_VLE_LO16D:
-	  fieldval = value & 0xffff;
-	sign_extend_16:
-	  if ((operand->flags & PPC_OPERAND_SIGNED) != 0)
-	    fieldval = (fieldval ^ 0x8000) - 0x8000;
-	  fixP->fx_no_overflow = 1;
-	  break;
-
-	case BFD_RELOC_HI16:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_HI16_PCREL;
-	  /* fall through */
-	case BFD_RELOC_HI16_PCREL:
-	case BFD_RELOC_PPC_VLE_HI16A:
-	case BFD_RELOC_PPC_VLE_HI16D:
-	  fieldval = PPC_HI (value);
-	  goto sign_extend_16;
-
-	case BFD_RELOC_HI16_S:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_HI16_S_PCREL;
-	  /* fall through */
-	case BFD_RELOC_HI16_S_PCREL:
-	case BFD_RELOC_PPC_VLE_HA16A:
-	case BFD_RELOC_PPC_VLE_HA16D:
-	  fieldval = PPC_HA (value);
-	  goto sign_extend_16;
-
-#ifdef OBJ_ELF
-	case BFD_RELOC_PPC64_HIGHER:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  fieldval = PPC_HIGHER (value);
-	  goto sign_extend_16;
-
-	case BFD_RELOC_PPC64_HIGHER_S:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  fieldval = PPC_HIGHERA (value);
-	  goto sign_extend_16;
-
-	case BFD_RELOC_PPC64_HIGHEST:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  fieldval = PPC_HIGHEST (value);
-	  goto sign_extend_16;
-
-	case BFD_RELOC_PPC64_HIGHEST_S:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  fieldval = PPC_HIGHESTA (value);
-	  goto sign_extend_16;
-
 	  /* The following relocs can't be calculated by the assembler.
 	     Leave the field zero.  */
 	case BFD_RELOC_PPC_TPREL16:
@@ -6463,8 +6577,6 @@ md_apply_fix (fixS *fixP, valueT *valP,
 	  gas_assert (fixP->fx_addsy != NULL);
 	  S_SET_THREAD_LOCAL (fixP->fx_addsy);
 	  fieldval = 0;
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
 	  break;
 
 	  /* These also should leave the field zero for the same
@@ -6531,12 +6643,14 @@ md_apply_fix (fixS *fixP, valueT *valP,
 	case BFD_RELOC_PPC_TLSGD:
 	case BFD_RELOC_PPC_TLSLD:
 	  fieldval = 0;
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
 	  break;
 #endif
 
 	default:
+	  fieldval = info.field_value;
+	  if (info.value_bits == 16
+	      && (operand->flags & PPC_OPERAND_SIGNED) != 0)
+	    fieldval = (fieldval ^ 0x8000) - 0x8000;
 	  break;
 	}
 
@@ -6610,79 +6724,9 @@ #define APPLY_RELOC 1
     }
   else
     {
-      int size = 0;
-      offsetT fieldval = value;
-
       /* Handle relocs in data.  */
       switch (fixP->fx_r_type)
 	{
-	case BFD_RELOC_CTOR:
-	  if (ppc_obj64)
-	    goto ctor64;
-	  /* fall through */
-
-	case BFD_RELOC_32:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_32_PCREL;
-	  /* fall through */
-
-	case BFD_RELOC_32_PCREL:
-	case BFD_RELOC_RVA:
-	  size = 4;
-	  break;
-
-	case BFD_RELOC_64:
-	ctor64:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_64_PCREL;
-	  /* fall through */
-
-	case BFD_RELOC_64_PCREL:
-	  size = 8;
-	  break;
-
-	case BFD_RELOC_16:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_16_PCREL;
-	  /* fall through */
-
-	case BFD_RELOC_16_PCREL:
-	  size = 2;
-	  break;
-
-	case BFD_RELOC_8:
-	  if (fixP->fx_pcrel)
-	    {
-#ifdef OBJ_ELF
-	    bad_pcrel:
-#endif
-	      if (fixP->fx_addsy)
-		{
-		  char *sfile;
-		  unsigned int sline;
-
-		  /* Use expr_symbol_where to see if this is an
-		     expression symbol.  */
-		  if (expr_symbol_where (fixP->fx_addsy, &sfile, &sline))
-		    as_bad_where (fixP->fx_file, fixP->fx_line,
-				  _("unresolved expression that must"
-				    " be resolved"));
-		  else
-		    as_bad_where (fixP->fx_file, fixP->fx_line,
-				  _("cannot emit PC relative %s relocation"
-				    " against %s"),
-				  bfd_get_reloc_code_name (fixP->fx_r_type),
-				  S_GET_NAME (fixP->fx_addsy));
-		}
-	      else
-		as_bad_where (fixP->fx_file, fixP->fx_line,
-			      _("unable to resolve expression"));
-	      fixP->fx_done = 1;
-	    }
-	  else
-	    size = 1;
-	  break;
-
 	case BFD_RELOC_VTABLE_INHERIT:
 	  if (fixP->fx_addsy
 	      && !S_IS_DEFINED (fixP->fx_addsy)
@@ -6695,58 +6739,6 @@ #define APPLY_RELOC 1
 	  break;
 
 #ifdef OBJ_ELF
-	  /* These can appear with @l etc. in data.  */
-	case BFD_RELOC_LO16:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_LO16_PCREL;
-	case BFD_RELOC_LO16_PCREL:
-	  size = 2;
-	  break;
-
-	case BFD_RELOC_HI16:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_HI16_PCREL;
-	case BFD_RELOC_HI16_PCREL:
-	  size = 2;
-	  fieldval = PPC_HI (value);
-	  break;
-
-	case BFD_RELOC_HI16_S:
-	  if (fixP->fx_pcrel)
-	    fixP->fx_r_type = BFD_RELOC_HI16_S_PCREL;
-	case BFD_RELOC_HI16_S_PCREL:
-	  size = 2;
-	  fieldval = PPC_HA (value);
-	  break;
-
-	case BFD_RELOC_PPC64_HIGHER:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  size = 2;
-	  fieldval = PPC_HIGHER (value);
-	  break;
-
-	case BFD_RELOC_PPC64_HIGHER_S:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  size = 2;
-	  fieldval = PPC_HIGHERA (value);
-	  break;
-
-	case BFD_RELOC_PPC64_HIGHEST:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  size = 2;
-	  fieldval = PPC_HIGHEST (value);
-	  break;
-
-	case BFD_RELOC_PPC64_HIGHEST_S:
-	  if (fixP->fx_pcrel)
-	    goto bad_pcrel;
-	  size = 2;
-	  fieldval = PPC_HIGHESTA (value);
-	  break;
-
 	case BFD_RELOC_PPC_DTPMOD:
 	case BFD_RELOC_PPC_TPREL:
 	case BFD_RELOC_PPC_DTPREL:
@@ -6833,21 +6825,13 @@ #define APPLY_RELOC 1
 	  break;
 #endif
 
-#ifdef OBJ_XCOFF
-	case BFD_RELOC_NONE:
-	  break;
-#endif
-
 	default:
-	  fprintf (stderr,
-		   _("Gas failure, reloc value %d\n"), fixP->fx_r_type);
-	  fflush (stderr);
-	  abort ();
+	  gas_assert (info.value_bits == info.size * 8);
+	  if (APPLY_RELOC)
+	    md_number_to_chars (fixP->fx_frag->fr_literal + fixP->fx_where,
+				info.field_value, info.size);
+	  break;
 	}
-
-      if (size && APPLY_RELOC)
-	md_number_to_chars (fixP->fx_frag->fr_literal + fixP->fx_where,
-			    fieldval, size);
     }
 
 #ifdef OBJ_ELF