Hi,

on 2023/11/15 11:01, Peter Bergner wrote:
> PCREL data accesses are only officially supported on ELFv2.  We currently
> incorrectly enable PCREL on all Power10 compiles in which prefix instructions
> are also enabled.  Rework the option override code so we only enable PCREL
> for those ABIs that actually support it.
> 
> Jeevitha has confirmed this patch fixes the testsuite fallout seen with her
> PR110320 patch.
> 
> This has been bootstrapped and regtested with no regressions on the following
> builds: powerpc64le-linux, powerpc64le-linux --with-cpu=power10 and
> powerpc64-linux - testsuite run in both 32-bit and 64-bit modes.
> Ok for trunk?

OK for trunk and backporting, but wait for two days or so in case Segher and
David have some comments, thanks!

BR,
Kewen

> 
> Ok for the release branches after some burn-in on trunk?
> 
> Peter
> 
> 
> gcc/
>       PR target/111045
>       * config/rs6000/linux64.h (PCREL_SUPPORTED_BY_OS): Only test the ABI.
>       * config/rs6000/rs6000-cpus.def (RS6000_CPU): Remove OPTION_MASK_PCREL
>       from power10.
>       * config/rs6000/predicates.md: Use TARGET_PCREL.
>       * config/rs6000/rs6000-logue.cc (rs6000_decl_ok_for_sibcall): Likewise.
>       (rs6000_global_entry_point_prologue_needed_p): Likewise.
>       (rs6000_output_function_prologue): Likewise.
>       * config/rs6000/rs6000.md: Likewise.
>       * config/rs6000/rs6000.cc (rs6000_option_override_internal): Rework
>       the logic for enabling PCREL by default.
>       (rs6000_legitimize_tls_address): Use TARGET_PCREL.
>       (rs6000_call_template_1): Likewise.
>       (rs6000_indirect_call_template_1): Likewise.
>       (rs6000_longcall_ref): Likewise.
>       (rs6000_call_aix): Likewise.
>       (rs6000_sibcall_aix): Likewise.
>       (rs6000_pcrel_p): Remove.
>       * config/rs6000/rs6000-protos.h (rs6000_pcrel_p): Likewise.
> 
> gcc/testsuite/
>       PR target/111045
>       * gcc.target/powerpc/pr111045.c: New test.
>       * gcc.target/powerpc/float128-constant.c: Add instruction counts for
>       non-pcrel compiles.
> 
> diff --git a/gcc/config/rs6000/linux64.h b/gcc/config/rs6000/linux64.h
> index 98b7255c95f..5b77bd7fd51 100644
> --- a/gcc/config/rs6000/linux64.h
> +++ b/gcc/config/rs6000/linux64.h
> @@ -563,8 +563,5 @@ extern int dot_symbols;
>  #define TARGET_FLOAT128_ENABLE_TYPE 1
>  
>  /* Enable using prefixed PC-relative addressing on POWER10 if the ABI
> -   supports it.  The ELF v2 ABI only supports PC-relative relocations for
> -   the medium code model.  */
> -#define PCREL_SUPPORTED_BY_OS        (TARGET_POWER10 && TARGET_PREFIXED      
> \
> -                              && ELFv2_ABI_CHECK                     \
> -                              && TARGET_CMODEL == CMODEL_MEDIUM)
> +   supports it.  */
> +#define PCREL_SUPPORTED_BY_OS        (ELFv2_ABI_CHECK)
> diff --git a/gcc/config/rs6000/rs6000-cpus.def 
> b/gcc/config/rs6000/rs6000-cpus.def
> index 4f350da378c..fe01a2312ae 100644
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -256,7 +256,8 @@ RS6000_CPU ("power8", PROCESSOR_POWER8, MASK_POWERPC64 | 
> ISA_2_7_MASKS_SERVER
>           | OPTION_MASK_HTM)
>  RS6000_CPU ("power9", PROCESSOR_POWER9, MASK_POWERPC64 | ISA_3_0_MASKS_SERVER
>           | OPTION_MASK_HTM)
> -RS6000_CPU ("power10", PROCESSOR_POWER10, MASK_POWERPC64 | 
> ISA_3_1_MASKS_SERVER)
> +RS6000_CPU ("power10", PROCESSOR_POWER10, MASK_POWERPC64
> +         | (ISA_3_1_MASKS_SERVER & ~OPTION_MASK_PCREL))
>  RS6000_CPU ("powerpc", PROCESSOR_POWERPC, 0)
>  RS6000_CPU ("powerpc64", PROCESSOR_POWERPC64, OPTION_MASK_PPC_GFXOPT
>           | MASK_POWERPC64)
> diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
> index ef7d3f214c4..0b76541fc0a 100644
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1216,7 +1216,7 @@
>                        && SYMBOL_REF_DECL (op) != NULL
>                        && TREE_CODE (SYMBOL_REF_DECL (op)) == FUNCTION_DECL
>                        && (rs6000_fndecl_pcrel_p (SYMBOL_REF_DECL (op))
> -                          != rs6000_pcrel_p ()))")))
> +                          != TARGET_PCREL))")))
>  
>  ;; Return 1 if this operand is a valid input for a move insn.
>  (define_predicate "input_operand"
> diff --git a/gcc/config/rs6000/rs6000-logue.cc 
> b/gcc/config/rs6000/rs6000-logue.cc
> index 98846f781ec..9e08d9bb4d2 100644
> --- a/gcc/config/rs6000/rs6000-logue.cc
> +++ b/gcc/config/rs6000/rs6000-logue.cc
> @@ -1106,7 +1106,7 @@ rs6000_decl_ok_for_sibcall (tree decl)
>        r2 for its caller's TOC.  Such a function may make sibcalls to any
>        function, whether local or external, without restriction based on
>        TOC-save/restore rules.  */
> -      if (rs6000_pcrel_p ())
> +      if (TARGET_PCREL)
>       return true;
>  
>        /* Otherwise, under the AIX or ELFv2 ABIs we can't allow sibcalls
> @@ -2583,7 +2583,7 @@ rs6000_global_entry_point_prologue_needed_p (void)
>      return false;
>  
>    /* PC-relative functions never generate a global entry point prologue.  */
> -  if (rs6000_pcrel_p ())
> +  if (TARGET_PCREL)
>      return false;
>  
>    /* Ensure we have a global entry point for thunks.   ??? We could
> @@ -4045,7 +4045,7 @@ rs6000_output_function_prologue (FILE *file)
>                                              patch_area_entry == 0);
>      }
>  
> -  else if (rs6000_pcrel_p ())
> +  else if (TARGET_PCREL)
>      {
>        const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 
> 0);
>        /* All functions compiled to use PC-relative addressing will
> diff --git a/gcc/config/rs6000/rs6000-protos.h 
> b/gcc/config/rs6000/rs6000-protos.h
> index f70118ea40f..8ffec295d09 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -158,7 +158,6 @@ extern rtx rs6000_allocate_stack_temp (machine_mode, 
> bool, bool);
>  extern align_flags rs6000_loop_align (rtx);
>  extern void rs6000_split_logical (rtx [], enum rtx_code, bool, bool, bool);
>  extern bool rs6000_function_pcrel_p (struct function *);
> -extern bool rs6000_pcrel_p (void);
>  extern bool rs6000_fndecl_pcrel_p (const_tree);
>  extern void rs6000_output_addr_vec_elt (FILE *, int);
>  
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 5f56c3ed85b..0c208cc2c0a 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -4378,19 +4378,22 @@ rs6000_option_override_internal (bool global_init_p)
>    /* If the ABI has support for PC-relative relocations, enable it by 
> default.
>       This test depends on the sub-target tests above setting the code model 
> to
>       medium for ELF v2 systems.  */
> -  if (PCREL_SUPPORTED_BY_OS
> -      && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
> -    rs6000_isa_flags |= OPTION_MASK_PCREL;
> -
> -  /* -mpcrel requires -mcmodel=medium, but we can't check TARGET_CMODEL until
> -      after the subtarget override options are done.  */
> -  else if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
> +  if (PCREL_SUPPORTED_BY_OS)
>      {
> -      if ((rs6000_isa_flags_explicit & OPTION_MASK_PCREL) != 0)
> +      /* PCREL on ELFv2 currently requires -mcmodel=medium, but we can't 
> check
> +      TARGET_CMODEL until after the subtarget override options are done.  */
> +      if (TARGET_PCREL && TARGET_CMODEL != CMODEL_MEDIUM)
>       error ("%qs requires %qs", "-mpcrel", "-mcmodel=medium");
>  
> -      rs6000_isa_flags &= ~OPTION_MASK_PCREL;
> +      if (!TARGET_PCREL
> +       && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0
> +       && TARGET_POWER10
> +       && TARGET_PREFIXED
> +       && TARGET_CMODEL == CMODEL_MEDIUM)
> +     rs6000_isa_flags |= OPTION_MASK_PCREL;
>      }
> +  else if (TARGET_PCREL)
> +    error ("use of %qs is invalid for this target", "-mpcrel");
>  
>    /* Enable -mmma by default on power10 systems.  */
>    if (TARGET_POWER10 && (rs6000_isa_flags_explicit & OPTION_MASK_MMA) == 0)
> @@ -9645,7 +9648,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model 
> model)
>  
>    dest = gen_reg_rtx (Pmode);
>    if (model == TLS_MODEL_LOCAL_EXEC
> -      && (rs6000_tls_size == 16 || rs6000_pcrel_p ()))
> +      && (rs6000_tls_size == 16 || TARGET_PCREL))
>      {
>        rtx tlsreg;
>  
> @@ -9692,7 +9695,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model 
> model)
>        them in the .got section.  So use a pointer to the .got section,
>        not one to secondary TOC sections used by 64-bit -mminimal-toc,
>        or to secondary GOT sections used by 32-bit -fPIC.  */
> -      if (rs6000_pcrel_p ())
> +      if (TARGET_PCREL)
>       got = const0_rtx;
>        else if (TARGET_64BIT)
>       got = gen_rtx_REG (Pmode, 2);
> @@ -9757,7 +9760,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model 
> model)
>         rtx uns = gen_rtx_UNSPEC (Pmode, vec, UNSPEC_TLS_GET_ADDR);
>         set_unique_reg_note (get_last_insn (), REG_EQUAL, uns);
>  
> -       if (rs6000_tls_size == 16 || rs6000_pcrel_p ())
> +       if (rs6000_tls_size == 16 || TARGET_PCREL)
>           {
>             if (TARGET_64BIT)
>               insn = gen_tls_dtprel_64 (dest, tmp1, addr);
> @@ -9798,7 +9801,7 @@ rs6000_legitimize_tls_address (rtx addr, enum tls_model 
> model)
>         else
>           insn = gen_tls_got_tprel_32 (tmp2, got, addr);
>         emit_insn (insn);
> -       if (rs6000_pcrel_p ())
> +       if (TARGET_PCREL)
>           {
>             if (TARGET_64BIT)
>               insn = gen_tls_tls_pcrel_64 (dest, tmp2, addr);
> @@ -14866,7 +14869,7 @@ rs6000_call_template_1 (rtx *operands, unsigned int 
> funop, bool sibcall)
>           ? "+32768" : ""));
>  
>    static char str[32];  /* 1 spare */
> -  if (rs6000_pcrel_p ())
> +  if (TARGET_PCREL)
>      sprintf (str, "b%s %s@notoc%s", sibcall ? "" : "l", z, arg);
>    else if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
>      sprintf (str, "b%s %s%s%s", sibcall ? "" : "l", z, arg,
> @@ -15006,7 +15009,7 @@ rs6000_indirect_call_template_1 (rtx *operands, 
> unsigned int funop,
>                    rel64);
>       }
>  
> -      const char *notoc = rs6000_pcrel_p () ? "_NOTOC" : "";
> +      const char *notoc = TARGET_PCREL ? "_NOTOC" : "";
>        const char *addend = (DEFAULT_ABI == ABI_V4 && TARGET_SECURE_PLT
>                           && flag_pic == 2 ? "+32768" : "");
>        if (!speculate)
> @@ -15023,7 +15026,7 @@ rs6000_indirect_call_template_1 (rtx *operands, 
> unsigned int funop,
>    else if (!speculate)
>      s += sprintf (s, "crset 2\n\t");
>  
> -  if (rs6000_pcrel_p ())
> +  if (TARGET_PCREL)
>      {
>        if (speculate)
>       sprintf (s, "b%%T%ul", funop);
> @@ -20685,7 +20688,7 @@ rs6000_longcall_ref (rtx call_ref, rtx arg)
>      {
>        rtx base = const0_rtx;
>        int regno = 12;
> -      if (rs6000_pcrel_p ())
> +      if (TARGET_PCREL)
>       {
>         rtx reg = gen_rtx_REG (Pmode, regno);
>         rtx u = gen_rtx_UNSPEC_VOLATILE (Pmode,
> @@ -25934,7 +25937,7 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx 
> tlsarg, rtx cookie)
>    if (!SYMBOL_REF_P (func)
>        || (DEFAULT_ABI == ABI_AIX && !SYMBOL_REF_FUNCTION_P (func)))
>      {
> -      if (!rs6000_pcrel_p ())
> +      if (!TARGET_PCREL)
>       {
>         /* Save the TOC into its reserved slot before the call,
>            and prepare to restore it after the call.  */
> @@ -26040,7 +26043,7 @@ rs6000_call_aix (rtx value, rtx func_desc, rtx 
> tlsarg, rtx cookie)
>    else
>      {
>        /* No TOC register needed for calls from PC-relative callers.  */
> -      if (!rs6000_pcrel_p ())
> +      if (!TARGET_PCREL)
>       /* Direct calls use the TOC: for local calls, the callee will
>          assume the TOC register is set; for non-local calls, the
>          PLT stub needs the TOC register.  */
> @@ -26089,7 +26092,7 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx 
> tlsarg, rtx cookie)
>      {
>        /* PCREL can do a sibling call to a longcall function
>        because we don't need to restore the TOC register.  */
> -      gcc_assert (rs6000_pcrel_p ());
> +      gcc_assert (TARGET_PCREL);
>        func_desc = rs6000_longcall_ref (func_desc, tlsarg);
>      }
>    else
> @@ -26116,7 +26119,7 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx 
> tlsarg, rtx cookie)
>    insn = emit_call_insn (insn);
>  
>    /* Note use of the TOC register.  */
> -  if (!rs6000_pcrel_p ())
> +  if (!TARGET_PCREL)
>      use_reg (&CALL_INSN_FUNCTION_USAGE (insn),
>            gen_rtx_REG (Pmode, TOC_REGNUM));
>  
> @@ -26416,16 +26419,6 @@ rs6000_function_pcrel_p (struct function *fn)
>    return rs6000_fndecl_pcrel_p (fn->decl);
>  }
>  
> -/* Return whether we should generate PC-relative code for the current
> -   function.  */
> -bool
> -rs6000_pcrel_p ()
> -{
> -  return (DEFAULT_ABI == ABI_ELFv2
> -       && (rs6000_isa_flags & OPTION_MASK_PCREL) != 0
> -       && TARGET_CMODEL == CMODEL_MEDIUM);
> -}
> -
>  
>  /* Given an address (ADDR), a mode (MODE), and what the format of the
>     non-prefixed address (NON_PREFIXED_FORMAT) is, return the instruction 
> format
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 2a1b5ecfaee..97de6940e07 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -11333,7 +11333,7 @@
>                           (match_operand:P 3 "" "")]
>                          UNSPECV_PLT_PCREL))]
>    "HAVE_AS_PLTSEQ && TARGET_ELF
> -   && rs6000_pcrel_p ()"
> +   && TARGET_PCREL"
>  {
>    return rs6000_pltseq_template (operands, RS6000_PLTSEQ_PLT_PCREL34);
>  }
> @@ -11418,7 +11418,7 @@
>    else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS)
>      output_asm_insn ("creqv 6,6,6", operands);
>  
> -  if (rs6000_pcrel_p ())
> +  if (TARGET_PCREL)
>      return "bl %z0@notoc";
>    return (DEFAULT_ABI == ABI_V4 && flag_pic) ? "bl %z0@local" : "bl %z0";
>  }
> @@ -11439,7 +11439,7 @@
>    else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
>      output_asm_insn ("creqv 6,6,6", operands);
>  
> -  if (rs6000_pcrel_p ())
> +  if (TARGET_PCREL)
>      return "bl %z1@notoc";
>    return (DEFAULT_ABI == ABI_V4 && flag_pic) ? "bl %z1@local" : "bl %z1";
>  }
> @@ -11614,7 +11614,7 @@
>  }
>    [(set_attr "type" "branch")
>     (set (attr "length")
> -     (if_then_else (match_test "rs6000_pcrel_p ()")
> +     (if_then_else (match_test "TARGET_PCREL")
>         (const_int 4)
>         (const_int 8)))])
>  
> @@ -11631,7 +11631,7 @@
>  }
>    [(set_attr "type" "branch")
>     (set (attr "length")
> -     (if_then_else (match_test "rs6000_pcrel_p ()")
> +     (if_then_else (match_test "TARGET_PCREL")
>           (const_int 4)
>           (const_int 8)))])
>  
> @@ -11705,7 +11705,7 @@
>        (match_operand 1))
>     (use (match_operand:SI 2 "immediate_operand" "n,n,n"))
>     (clobber (reg:P LR_REGNO))]
> -  "rs6000_pcrel_p ()"
> +  "TARGET_PCREL"
>  {
>    return rs6000_indirect_call_template (operands, 0);
>  }
> @@ -11742,7 +11742,7 @@
>             (match_operand:P 2 "unspec_tls" "")))
>     (use (match_operand:SI 3 "immediate_operand" "n,n,n"))
>     (clobber (reg:P LR_REGNO))]
> -  "rs6000_pcrel_p ()"
> +  "TARGET_PCREL"
>  {
>    return rs6000_indirect_call_template (operands, 1);
>  }
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr111045.c 
> b/gcc/testsuite/gcc.target/powerpc/pr111045.c
> new file mode 100644
> index 00000000000..6fd2773eb35
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr111045.c
> @@ -0,0 +1,15 @@
> +/* PR target/111045 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +
> +/* Verify we do not generate a PCREL access on ABIs that doesn't support it. 
>  */
> +
> +static unsigned short s;
> +
> +unsigned short *
> +foo (void)
> +{
> +  return &s;
> +}
> +
> +/* { dg-final { scan-assembler-not {\m@pcrel\M} { target { ! powerpc_pcrel } 
> } } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/float128-constant.c 
> b/gcc/testsuite/gcc.target/powerpc/float128-constant.c
> index e3286a786a5..4728f4891da 100644
> --- a/gcc/testsuite/gcc.target/powerpc/float128-constant.c
> +++ b/gcc/testsuite/gcc.target/powerpc/float128-constant.c
> @@ -155,6 +155,7 @@ return_longlong_neg_0 (void)
>  }
>  
>  /* { dg-final { scan-assembler-times {\mlxvkq\M}    19 } } */
> -/* { dg-final { scan-assembler-times {\mplxv\M}      3 } } */
> +/* { dg-final { scan-assembler-times {\mplxv\M}      3 { target { 
> powerpc_pcrel } } } } */
> +/* { dg-final { scan-assembler-times {\mlxv\M}       3 { target { ! 
> powerpc_pcrel } } } } */
>  /* { dg-final { scan-assembler-times {\mxxspltib\M}  1 } } */
>  

Reply via email to