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

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

Alan Modra <amodra@gmail.com> writes:
> On Thu, May 16, 2013 at 01:52:11PM +0100, Richard Sandiford wrote:
>> 	* 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.
>
> I like the change you've made here to put the value twiddling in
> md_apply_fix for both insn and data relocs in one place (field_value).
> I'm not so sure about trying to merge the bfd_reloc_type_lookup from
> md_assemble.  We don't currently need the "howto" in md_apply_fix, so
> that seems like doing unnecessary work.

Yeah.  To my shame yesterday, I'd forgotten that XCOFF allows
the relocation size to be specified separately from the type.
Which of course means that there should be no reason to avoid looking
up the howto in md_assemble, which in turn makes the patch as posted
pretty pointless.  I think you're being overly kind about it :-)

If you think the md_apply_fix change is worth keeping as a cleanup,
then FWIW, here's the patch with just that part.  Tested as before.

Thanks,
Richard


gas/
	* config/tc-ppc.c (ppc_reloc_info): New structure.
	(ppc_get_reloc_info): New function.
	(md_apply_fix): Use it.  Apply PC-relative conversion before
	the main code/data switch.

Index: gas/config/tc-ppc.c
===================================================================
--- gas/config/tc-ppc.c	2013-05-17 15:11:54.949365592 +0100
+++ gas/config/tc-ppc.c	2013-05-17 15:32:13.566385693 +0100
@@ -1824,7 +1824,116 @@ ppc_insert_operand (unsigned long insn,
 
   return insn;
 }
+
+/* Information about a relocation.  */
+struct ppc_reloc_info {
+  /* The number of significant bits in FIELD_VALUE, or 0 if not known.  */
+  unsigned int value_bits;
+
+  /* The PC-relative form of the relocation, or BFD_RELOC_NONE if none.
+     Only valid if VALUE_BITS is nonzero.  */
+  bfd_reloc_code_real_type pcrel_type;
+
+  /* The value of the relocation field.  The value given for split
+     fields like BFD_RELOC_PPC_VLE_LO16A is the unsplit form.  */
+  offsetT field_value;
+};
+
+/* 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)
+{
+  if (r_type == BFD_RELOC_CTOR)
+    r_type = ppc_obj64 ? BFD_RELOC_64 : BFD_RELOC_32;
+
+  info->pcrel_type = BFD_RELOC_NONE;
+  info->field_value = value;
+  info->value_bits = 0;
+  switch (r_type)
+    {
+    default:
+      break;
+
+    case BFD_RELOC_8:
+      info->value_bits = 8;
+      break;
+
+    case BFD_RELOC_16:
+    case BFD_RELOC_16_PCREL:
+      info->pcrel_type = BFD_RELOC_16_PCREL;
+      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->value_bits = 32;
+      break;
+
+    case BFD_RELOC_64:
+    case BFD_RELOC_64_PCREL:
+      info->pcrel_type = BFD_RELOC_64_PCREL;
+      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
+    case BFD_RELOC_PPC_VLE_LO16A:
+    case BFD_RELOC_PPC_VLE_LO16D:
+      info->field_value = value & 0xffff;
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_HI16:
+    case BFD_RELOC_HI16_PCREL:
+      info->pcrel_type = BFD_RELOC_HI16_PCREL;
+      /* Fall through.  */
+    case BFD_RELOC_PPC_VLE_HI16A:
+    case BFD_RELOC_PPC_VLE_HI16D:
+      info->field_value = PPC_HI (value);
+      info->value_bits = 16;
+      break;
 
+    case BFD_RELOC_HI16_S:
+    case BFD_RELOC_HI16_S_PCREL:
+      info->pcrel_type = BFD_RELOC_HI16_S_PCREL;
+      /* Fall through.  */
+    case BFD_RELOC_PPC_VLE_HA16A:
+    case BFD_RELOC_PPC_VLE_HA16D:
+      info->field_value = PPC_HA (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHER:
+      info->field_value = PPC_HIGHER (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHER_S:
+      info->field_value = PPC_HIGHERA (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHEST:
+      info->field_value = PPC_HIGHEST (value);
+      info->value_bits = 16;
+      break;
+
+    case BFD_RELOC_PPC64_HIGHEST_S:
+      info->field_value = PPC_HIGHESTA (value);
+      info->value_bits = 16;
+      break;
+    }
+};
 
 #ifdef OBJ_ELF
 /* Parse @got, etc. and return the desired relocation.  */
@@ -6287,6 +6396,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 +6435,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 (info.value_bits > 0 && 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 +6493,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 +6541,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 +6607,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 +6688,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 +6703,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 +6789,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 == fixP->fx_size * 8);
+	  if (APPLY_RELOC && fixP->fx_size > 0)
+	    md_number_to_chars (fixP->fx_frag->fr_literal + fixP->fx_where,
+				info.field_value, fixP->fx_size);
+	  break;
 	}
-
-      if (size && APPLY_RELOC)
-	md_number_to_chars (fixP->fx_frag->fr_literal + fixP->fx_where,
-			    fieldval, size);
     }
 
 #ifdef OBJ_ELF