Re: RFA: Merge definitions of get_some_local_dynamic_name

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Sep 2, 2014 at 8:36 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Several targets define a function like i386's get_some_local_dynamic_name.
> The function looks through the current output function and returns the first
> (arbitrary) local-dynamic symbol that it finds.  The result can be used in
> a call to __tls_get_addr, since all local-dynamic symbols have the same base.
>
> This patch replaces the various target functions with a single generic one.
> The only difference between the implementations was that s390 checked
> for constant pool references while the others didn't need to (because
> they don't allow TLS symbols to be forced into the pool).  Checking for
> constant pool references is unnecessary but harmless for the other ports.
> Also, the walk is needed only once per TLS-referencing output function,
> so it's hardly critical in terms of compile time.
>
> All uses of this function are in final.  In general it wouldn't be
> safe to call the function earlier than that, since the symbol reference
> could in principle be deleted by any rtl pass.  I've therefore cached
> it in a variable local to final rather than in cfun (which is where
> the ports used to cache it).
>
> Also, i386 was robust against uses of %& in inline asm.  The patch
> makes sure the other ports are too.  Using %& in inline asm would
> often be a mistake, but it should at least trigger a proper error
> rather than an ICE.
>
> Tested on x86_64-linux-gnu.  Also tested by building cross compilers
> before and after the change on:
>
>   alpha-linux-gnu powerpc64-linux-gnu s390x-linux-gnu sparc64-linux-gnu
>
> OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * output.h (get_some_local_dynamic_name): Declare.
>         * final.c (some_local_dynamic_name): New variable.
>         (get_some_local_dynamic_name): New function.
>         (final_end_function): Clear some_local_dynamic_name.
>         * config/alpha/alpha.c (machine_function): Remove some_ld_name.
>         (get_some_local_dynamic_name, get_some_local_dynamic_name_1): Delete.
>         (print_operand): Report an error if '%&' is used inappropriately.
>         * config/i386/i386.c (get_some_local_dynamic_name): Delete.
>         (get_some_local_dynamic_name_1): Delete.
>         * config/rs6000/rs6000.c (machine_function): Remove some_ld_name.
>         (rs6000_get_some_local_dynamic_name): Delete.
>         (rs6000_get_some_local_dynamic_name_1): Delete.
>         (print_operand): Report an error if '%&' is used inappropriately.
>         * config/s390/s390.c (machine_function): Remove some_ld_name.
>         (get_some_local_dynamic_name, get_some_local_dynamic_name_1): Delete.
>         (print_operand): Assert that get_some_local_dynamic_name is nonnull.
>         * config/sparc/sparc.c: Include rtl-iter.h.
>         (machine_function): Remove some_ld_name.
>         (sparc_print_operand): Report an error if '%&' is used inappropriately.
>         (get_some_local_dynamic_name, get_some_local_dynamic_name_1): Delete.
>
> Index: gcc/output.h
> ===================================================================
> --- gcc/output.h        2014-08-31 21:05:04.701330252 +0100
> +++ gcc/output.h        2014-09-02 19:02:59.820482510 +0100
> @@ -52,6 +52,8 @@ extern int get_attr_min_length (rtx);
>     any branches of variable length if possible.  */
>  extern void shorten_branches (rtx_insn *);
>
> +const char *get_some_local_dynamic_name ();
> +
>  /* Output assembler code for the start of a function,
>     and initialize some of the variables in this file
>     for the new function.  The label for the function and associated
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c 2014-08-31 21:05:04.701330252 +0100
> +++ gcc/final.c 2014-09-02 19:17:08.573876805 +0100
> @@ -1719,6 +1719,38 @@ reemit_insn_block_notes (void)
>    reorder_blocks ();
>  }
>
> +static const char *some_local_dynamic_name;
> +
> +/* Locate some local-dynamic symbol still in use by this function
> +   so that we can print its name in local-dynamic base patterns.
> +   Return null if there are no local-dynamic references.  */
> +
> +const char *
> +get_some_local_dynamic_name ()
> +{
> +  subrtx_iterator::array_type array;
> +  rtx_insn *insn;
> +
> +  if (some_local_dynamic_name)
> +    return some_local_dynamic_name;
> +
> +  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> +    if (NONDEBUG_INSN_P (insn))
> +      FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
> +       {
> +         const_rtx x = *iter;
> +         if (GET_CODE (x) == SYMBOL_REF)
> +           {
> +             if (SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
> +               return some_local_dynamic_name = XSTR (x, 0);
> +             if (CONSTANT_POOL_ADDRESS_P (x))
> +               iter.substitute (get_pool_constant (x));
> +           }
> +       }
> +
> +  return 0;
> +}
> +
>  /* Output assembler code for the start of a function,
>     and initialize some of the variables in this file
>     for the new function.  The label for the function and associated
> @@ -1904,6 +1936,8 @@ final_end_function (void)
>    if (!dwarf2_debug_info_emitted_p (current_function_decl)
>        && dwarf2out_do_frame ())
>      dwarf2out_end_epilogue (last_linenum, last_filename);
> +
> +  some_local_dynamic_name = 0;
>  }
>
>
> Index: gcc/config/alpha/alpha.c
> ===================================================================
> --- gcc/config/alpha/alpha.c    2014-08-31 21:05:04.701330252 +0100
> +++ gcc/config/alpha/alpha.c    2014-09-02 19:02:59.552479551 +0100
> @@ -4858,9 +4858,6 @@ struct GTY(()) alpha_links;
>
>  struct GTY(()) machine_function
>  {
> -  /* For OSF.  */
> -  const char *some_ld_name;
> -
>    /* For flag_reorder_blocks_and_partition.  */
>    rtx gp_save_rtx;
>
> @@ -5120,40 +5117,6 @@ get_round_mode_suffix (void)
>    gcc_unreachable ();
>  }
>
> -/* Locate some local-dynamic symbol still in use by this function
> -   so that we can print its name in some movdi_er_tlsldm pattern.  */
> -
> -static int
> -get_some_local_dynamic_name_1 (rtx *px, void *data ATTRIBUTE_UNUSED)
> -{
> -  rtx x = *px;
> -
> -  if (GET_CODE (x) == SYMBOL_REF
> -      && SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
> -    {
> -      cfun->machine->some_ld_name = XSTR (x, 0);
> -      return 1;
> -    }
> -
> -  return 0;
> -}
> -
> -static const char *
> -get_some_local_dynamic_name (void)
> -{
> -  rtx_insn *insn;
> -
> -  if (cfun->machine->some_ld_name)
> -    return cfun->machine->some_ld_name;
> -
> -  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> -    if (INSN_P (insn)
> -       && for_each_rtx (&PATTERN (insn), get_some_local_dynamic_name_1, 0))
> -      return cfun->machine->some_ld_name;
> -
> -  gcc_unreachable ();
> -}
> -
>  /* Print an operand.  Recognize special options, documented below.  */
>
>  void
> @@ -5169,7 +5132,11 @@ print_operand (FILE *file, rtx x, int co
>        break;
>
>      case '&':
> -      assemble_name (file, get_some_local_dynamic_name ());
> +      if (const char *name = get_some_local_dynamic_name ())
> +       assemble_name (file, name);
> +      else
> +       output_operand_lossage ("'%%&' used without any "
> +                               "local dynamic TLS references");
>        break;
>
>      case '/':
> Index: gcc/config/i386/i386.c
> ===================================================================
> --- gcc/config/i386/i386.c      2014-08-31 21:05:04.701330252 +0100
> +++ gcc/config/i386/i386.c      2014-09-02 19:02:59.669480843 +0100
> @@ -14756,41 +14756,6 @@ print_reg (rtx x, int code, FILE *file)
>      }
>  }
>
> -/* Locate some local-dynamic symbol still in use by this function
> -   so that we can print its name in some tls_local_dynamic_base
> -   pattern.  */
> -
> -static int
> -get_some_local_dynamic_name_1 (rtx *px, void *)
> -{
> -  rtx x = *px;
> -
> -  if (GET_CODE (x) == SYMBOL_REF
> -      && SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
> -    {
> -      cfun->machine->some_ld_name = XSTR (x, 0);
> -      return 1;
> -    }
> -
> -  return 0;
> -}
> -
> -static const char *
> -get_some_local_dynamic_name (void)
> -{
> -  rtx_insn *insn;
> -
> -  if (cfun->machine->some_ld_name)
> -    return cfun->machine->some_ld_name;
> -
> -  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> -    if (NONDEBUG_INSN_P (insn)
> -       && for_each_rtx (&PATTERN (insn), get_some_local_dynamic_name_1, 0))
> -      return cfun->machine->some_ld_name;
> -
> -  return NULL;
> -}
> -
>  /* Meaning of CODE:
>     L,W,B,Q,S,T -- print the opcode suffix for specified size of operand.
>     C -- print opcode suffix for set/cmov insn.
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  2014-08-31 21:05:04.701330252 +0100
> +++ gcc/config/rs6000/rs6000.c  2014-09-02 19:02:59.699481175 +0100
> @@ -142,8 +142,6 @@ typedef struct rs6000_stack {
>     This is added to the cfun structure.  */
>  typedef struct GTY(()) machine_function
>  {
> -  /* Some local-dynamic symbol.  */
> -  const char *some_ld_name;
>    /* Whether the instruction chain has been scanned already.  */
>    int insn_chain_scanned_p;
>    /* Flags if __builtin_return_address (n) with n >= 1 was used.  */
> @@ -1103,7 +1101,6 @@ static void is_altivec_return_reg (rtx,
>  int easy_vector_constant (rtx, enum machine_mode);
>  static rtx rs6000_debug_legitimize_address (rtx, rtx, enum machine_mode);
>  static rtx rs6000_legitimize_tls_address (rtx, enum tls_model);
> -static int rs6000_get_some_local_dynamic_name_1 (rtx *, void *);
>  static rtx rs6000_darwin64_record_arg (CUMULATIVE_ARGS *, const_tree,
>                                        bool, bool);
>  #if TARGET_MACHO
> @@ -17937,46 +17934,6 @@ extract_ME (rtx op)
>    return i;
>  }
>
> -/* Locate some local-dynamic symbol still in use by this function
> -   so that we can print its name in some tls_ld pattern.  */
> -
> -static const char *
> -rs6000_get_some_local_dynamic_name (void)
> -{
> -  rtx_insn *insn;
> -
> -  if (cfun->machine->some_ld_name)
> -    return cfun->machine->some_ld_name;
> -
> -  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> -    if (INSN_P (insn)
> -       && for_each_rtx (&PATTERN (insn),
> -                        rs6000_get_some_local_dynamic_name_1, 0))
> -      return cfun->machine->some_ld_name;
> -
> -  gcc_unreachable ();
> -}
> -
> -/* Helper function for rs6000_get_some_local_dynamic_name.  */
> -
> -static int
> -rs6000_get_some_local_dynamic_name_1 (rtx *px, void *data ATTRIBUTE_UNUSED)
> -{
> -  rtx x = *px;
> -
> -  if (GET_CODE (x) == SYMBOL_REF)
> -    {
> -      const char *str = XSTR (x, 0);
> -      if (SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
> -       {
> -         cfun->machine->some_ld_name = str;
> -         return 1;
> -       }
> -    }
> -
> -  return 0;
> -}
> -
>  /* Write out a function code label.  */
>
>  void
> @@ -18652,7 +18609,11 @@ print_operand (FILE *file, rtx x, int co
>        return;
>
>      case '&':
> -      assemble_name (file, rs6000_get_some_local_dynamic_name ());
> +      if (const char *name = get_some_local_dynamic_name ())
> +       assemble_name (file, name);
> +      else
> +       output_operand_lossage ("'%%&' used without any "
> +                               "local dynamic TLS references");
>        return;
>
>      default:
> Index: gcc/config/s390/s390.c
> ===================================================================
> --- gcc/config/s390/s390.c      2014-08-31 21:05:04.701330252 +0100
> +++ gcc/config/s390/s390.c      2014-09-02 19:02:59.742481649 +0100
> @@ -387,9 +387,6 @@ struct GTY(()) machine_function
>    /* True if we may need to perform branch splitting.  */
>    bool split_branches_pending_p;
>
> -  /* Some local-dynamic TLS symbol name.  */
> -  const char *some_ld_name;
> -
>    bool has_landing_pad_p;
>
>    /* True if the current function may contain a tbegin clobbering
> @@ -5197,48 +5194,6 @@ print_shift_count_operand (FILE *file, r
>      fprintf (file, "(%s)", reg_names[REGNO (base)]);
>  }
>
> -/* See 'get_some_local_dynamic_name'.  */
> -
> -static int
> -get_some_local_dynamic_name_1 (rtx *px, void *data ATTRIBUTE_UNUSED)
> -{
> -  rtx x = *px;
> -
> -  if (GET_CODE (x) == SYMBOL_REF && CONSTANT_POOL_ADDRESS_P (x))
> -    {
> -      x = get_pool_constant (x);
> -      return for_each_rtx (&x, get_some_local_dynamic_name_1, 0);
> -    }
> -
> -  if (GET_CODE (x) == SYMBOL_REF
> -      && tls_symbolic_operand (x) == TLS_MODEL_LOCAL_DYNAMIC)
> -    {
> -      cfun->machine->some_ld_name = XSTR (x, 0);
> -      return 1;
> -    }
> -
> -  return 0;
> -}
> -
> -/* Locate some local-dynamic symbol still in use by this function
> -   so that we can print its name in local-dynamic base patterns.  */
> -
> -static const char *
> -get_some_local_dynamic_name (void)
> -{
> -  rtx_insn *insn;
> -
> -  if (cfun->machine->some_ld_name)
> -    return cfun->machine->some_ld_name;
> -
> -  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> -    if (INSN_P (insn)
> -        && for_each_rtx (&PATTERN (insn), get_some_local_dynamic_name_1, 0))
> -      return cfun->machine->some_ld_name;
> -
> -  gcc_unreachable ();
> -}
> -
>  /* Returns -1 if the function should not be made hotpatchable.  Otherwise it
>     returns a number >= 0 that is the desired size of the hotpatch trampoline
>     in halfwords. */
> @@ -5508,7 +5463,9 @@ print_operand (FILE *file, rtx x, int co
>        else if (GET_CODE (x) == UNSPEC && XINT (x, 1) == UNSPEC_TLSLDM)
>         {
>           fprintf (file, "%s", ":tls_ldcall:");
> -         assemble_name (file, get_some_local_dynamic_name ());
> +         const char *name = get_some_local_dynamic_name ();
> +         gcc_assert (name);
> +         assemble_name (file, name);
>         }
>        else
>         output_operand_lossage ("invalid reference for 'J' output modifier");
> Index: gcc/config/sparc/sparc.c
> ===================================================================
> --- gcc/config/sparc/sparc.c    2014-08-31 21:05:04.701330252 +0100
> +++ gcc/config/sparc/sparc.c    2014-09-02 19:02:59.800482290 +0100
> @@ -70,6 +70,7 @@ the Free Software Foundation; either ver
>  #include "context.h"
>  #include "wide-int.h"
>  #include "builtins.h"
> +#include "rtl-iter.h"
>
>  /* Processor costs */
>
> @@ -500,9 +501,6 @@ struct GTY(()) machine_function
>    rtx frame_base_reg;
>    HOST_WIDE_INT frame_base_offset;
>
> -  /* Some local-dynamic TLS symbol name.  */
> -  const char *some_ld_name;
> -
>    /* Number of global or FP registers to be saved (as 4-byte quantities).  */
>    int n_global_fp_regs;
>
> @@ -584,8 +582,6 @@ static struct machine_function * sparc_i
>  static bool sparc_cannot_force_const_mem (enum machine_mode, rtx);
>  static rtx sparc_tls_get_addr (void);
>  static rtx sparc_tls_got (void);
> -static const char *get_some_local_dynamic_name (void);
> -static int get_some_local_dynamic_name_1 (rtx *, void *);
>  static int sparc_register_move_cost (enum machine_mode,
>                                      reg_class_t, reg_class_t);
>  static bool sparc_rtx_costs (rtx, int, int, int, int *, bool);
> @@ -8790,7 +8786,11 @@ sparc_print_operand (FILE *file, rtx x,
>        return;
>      case '&':
>        /* Print some local dynamic TLS name.  */
> -      assemble_name (file, get_some_local_dynamic_name ());
> +      if (const char *name = get_some_local_dynamic_name ())
> +       assemble_name (file, name);
> +      else
> +       output_operand_lossage ("'%%&' used without any "
> +                               "local dynamic TLS references");
>        return;
>
>      case 'Y':
> @@ -11484,41 +11484,6 @@ sparc_init_machine_status (void)
>    return ggc_cleared_alloc<machine_function> ();
>  }
>
> -/* Locate some local-dynamic symbol still in use by this function
> -   so that we can print its name in local-dynamic base patterns.  */
> -
> -static const char *
> -get_some_local_dynamic_name (void)
> -{
> -  rtx_insn *insn;
> -
> -  if (cfun->machine->some_ld_name)
> -    return cfun->machine->some_ld_name;
> -
> -  for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> -    if (INSN_P (insn)
> -       && for_each_rtx (&PATTERN (insn), get_some_local_dynamic_name_1, 0))
> -      return cfun->machine->some_ld_name;
> -
> -  gcc_unreachable ();
> -}
> -
> -static int
> -get_some_local_dynamic_name_1 (rtx *px, void *data ATTRIBUTE_UNUSED)
> -{
> -  rtx x = *px;
> -
> -  if (x
> -      && GET_CODE (x) == SYMBOL_REF
> -      && SYMBOL_REF_TLS_MODEL (x) == TLS_MODEL_LOCAL_DYNAMIC)
> -    {
> -      cfun->machine->some_ld_name = XSTR (x, 0);
> -      return 1;
> -    }
> -
> -  return 0;
> -}
> -
>  /* This is called from dwarf2out.c via TARGET_ASM_OUTPUT_DWARF_DTPREL.
>     We need to emit DTP-relative relocations.  */
>