diff mbox series

PR target/97682 - Fix to reuse t1 register between call address and epilogue.

Message ID 20201110071544.41066-1-monk.chiang@sifive.com
State New
Headers show
Series PR target/97682 - Fix to reuse t1 register between call address and epilogue. | expand

Commit Message

Monk Chiang Nov. 10, 2020, 7:15 a.m. UTC 
- When expanding the call pattern, choose t1 register be a jump register.
    Epilogue also uses a t1 register to adjust Stack point. The call pattern
    and epilogue will initial t1 twice, if both are generated in the same
    function. The call pattern will emit 'la t1,symbol' and 'jalr t1'instructions.
    Epilogue also emits 'li t1,4096' and 'addi sp,sp,t1' instructions.
    But li and addi instructions will be placed between la and jalr instructions.
    The la instruction will be removed by some optimizations,
    because t1 register define twice, the first define instruction look
    likes duplicate.

  - To resolve this issue, Prologue and Epilogue use the t0 register
    be temp register, the call pattern use the t1 register be tmp register.

  gcc/ChangeLog:

	PR target/97682
	* config/riscv/riscv.h (RISCV_PROLOGUE_TEMP_REGNUM): Change register to t0.
	(RISCV_CALL_ADDRESS_TEMP_REGNUM): New Marco, define t1 register.
	(RISCV_CALL_ADDRESS_TEMP): Use it for call instructions.
	* config/riscv/riscv.c (riscv_legitimize_call_address): Use
	RISCV_CALL_ADDRESS_TEMP.
	(riscv_trampoline_init): Adjust comment.

  gcc/testsuite/ChangeLog

	PR target/97682
	* g++.target/riscv/pr97682.C: New test.
---
 gcc/config/riscv/riscv.c                 |  14 +-
 gcc/config/riscv/riscv.h                 |   6 +-
 gcc/testsuite/g++.target/riscv/pr97682.C | 160 +++++++++++++++++++++++
 3 files changed, 172 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/riscv/pr97682.C

Comments

Jim Wilson Nov. 12, 2020, 10:29 p.m. UTC | #1 
On Mon, Nov 9, 2020 at 11:15 PM Monk Chiang <monk.chiang@sifive.com> wrote:

> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 172c7ca7c98..3bd1993c4c9 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -342,9 +342,13 @@ extern const char *riscv_default_mtune (int argc,
> const char **argv);
>     The epilogue temporary mustn't conflict with the return registers,
>     the frame pointer, the EH stack adjustment, or the EH data registers.
> */
>
> -#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST + 1)
> +#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST)
>  #define RISCV_PROLOGUE_TEMP(MODE) gen_rtx_REG (MODE,
> RISCV_PROLOGUE_TEMP_REGNUM)
>
> +#define RISCV_CALL_ADDRESS_TEMP_REGNUM (GP_TEMP_FIRST + 1)
> +#define RISCV_CALL_ADDRESS_TEMP(MODE) \
> +  gen_rtx_REG (MODE, RISCV_CALL_ADDRESS_TEMP_REGNUM)
>

This looks generally OK, however there is a minor problem that we have code
in riscv_compute_frame_info to save t1 in an interrupt handler register
with a large stack frame, as we know the prologue code will clobber t1 in
this case.  However, with this patch, the prologue now clobbers t0
instead.  So riscv_computer_frame_info needs to be fixed.  I'd suggest
changing the T1_REGNUM to RISCV_PROLOGUE_TEMP_REGNUM to prevent this from
happening again, that is probably my fault.  And the interrupt_save_t1
variable should be renamed, maybe to interupt_save_prologue_temp.

You can see the problem with gcc/testsuite/gcc.target/riscv/interrupt-3.c
if you compile with -O0 and we get
foo:
addi sp,sp,-32
sw t1,28(sp)
sw s0,24(sp)
addi s0,sp,32
li t0,-4096
addi t0,t0,16
add sp,sp,t0
so we are saving t1 and then clobbering t0 with your patch.

Otherwise this looks good.

Jim
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index 989a9f15250..ac4b04540e6 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -3110,7 +3110,7 @@  riscv_legitimize_call_address (rtx addr)
 {
   if (!call_insn_operand (addr, VOIDmode))
     {
-      rtx reg = RISCV_PROLOGUE_TEMP (Pmode);
+      rtx reg = RISCV_CALL_ADDRESS_TEMP (Pmode);
       riscv_emit_move (reg, addr);
       return reg;
     }
@@ -4902,9 +4902,9 @@  riscv_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
 
       rtx target_function = force_reg (Pmode, XEXP (DECL_RTL (fndecl), 0));
       /* lui     t2, hi(chain)
-	 lui     t1, hi(func)
+	 lui     t0, hi(func)
 	 addi    t2, t2, lo(chain)
-	 jr      r1, lo(func)
+	 jr      t0, lo(func)
       */
       unsigned HOST_WIDE_INT lui_hi_chain_code, lui_hi_func_code;
       unsigned HOST_WIDE_INT lo_chain_code, lo_func_code;
@@ -4929,7 +4929,7 @@  riscv_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
       mem = adjust_address (m_tramp, SImode, 0);
       riscv_emit_move (mem, lui_hi_chain);
 
-      /* Gen lui t1, hi(func).  */
+      /* Gen lui t0, hi(func).  */
       rtx hi_func = riscv_force_binary (SImode, PLUS, target_function,
 					fixup_value);
       hi_func = riscv_force_binary (SImode, AND, hi_func,
@@ -4956,7 +4956,7 @@  riscv_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
       mem = adjust_address (m_tramp, SImode, 2 * GET_MODE_SIZE (SImode));
       riscv_emit_move (mem, addi_lo_chain);
 
-      /* Gen jr r1, lo(func).  */
+      /* Gen jr t0, lo(func).  */
       rtx lo_func = riscv_force_binary (SImode, AND, target_function,
 					imm12_mask);
       lo_func = riscv_force_binary (SImode, ASHIFT, lo_func, GEN_INT (20));
@@ -4975,9 +4975,9 @@  riscv_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
       target_function_offset = static_chain_offset + GET_MODE_SIZE (ptr_mode);
 
       /* auipc   t2, 0
-	 l[wd]   t1, target_function_offset(t2)
+	 l[wd]   t0, target_function_offset(t2)
 	 l[wd]   t2, static_chain_offset(t2)
-	 jr      t1
+	 jr      t0
       */
       trampoline[0] = OPCODE_AUIPC | (STATIC_CHAIN_REGNUM << SHIFT_RD);
       trampoline[1] = (Pmode == DImode ? OPCODE_LD : OPCODE_LW)
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 172c7ca7c98..3bd1993c4c9 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -342,9 +342,13 @@  extern const char *riscv_default_mtune (int argc, const char **argv);
    The epilogue temporary mustn't conflict with the return registers,
    the frame pointer, the EH stack adjustment, or the EH data registers. */
 
-#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST + 1)
+#define RISCV_PROLOGUE_TEMP_REGNUM (GP_TEMP_FIRST)
 #define RISCV_PROLOGUE_TEMP(MODE) gen_rtx_REG (MODE, RISCV_PROLOGUE_TEMP_REGNUM)
 
+#define RISCV_CALL_ADDRESS_TEMP_REGNUM (GP_TEMP_FIRST + 1)
+#define RISCV_CALL_ADDRESS_TEMP(MODE) \
+  gen_rtx_REG (MODE, RISCV_CALL_ADDRESS_TEMP_REGNUM)
+
 #define MCOUNT_NAME "_mcount"
 
 #define NO_PROFILE_COUNTERS 1
diff --git a/gcc/testsuite/g++.target/riscv/pr97682.C b/gcc/testsuite/g++.target/riscv/pr97682.C
new file mode 100644
index 00000000000..03c7a447de5
--- /dev/null
+++ b/gcc/testsuite/g++.target/riscv/pr97682.C
@@ -0,0 +1,160 @@ 
+/* PR target/97682 */
+/* { dg-do compile } */
+/* { dg-options "-fPIC -O2 -march=rv64g -mabi=lp64" } */
+
+template <typename ab, int ac> struct g { ab b[ac]; };
+long i, m;
+
+namespace std
+{
+  template <typename c> struct t
+  {
+    int a;
+    c h;
+  };
+
+  struct ad
+  {
+    enum { j };
+  };
+
+  template <typename k> k l(k);
+  struct al {};
+  template <typename k> k aa(k);
+
+  struct v
+  {
+    template <typename n, typename q> static q o(n, n, q);
+  };
+
+  template <int, typename n, typename q> void p(n z, n ao, q ap)
+  {
+    v::o(z, ao, ap);
+  }
+
+  template <int ae, typename n, typename q> void r(n z, n ao, q ap)
+  {
+    p<ae>(z, ao, ap);
+  }
+
+  template <int ae, typename n, typename q> void af(n z, n ao, q)
+  {
+    r<ae>(aa(z), aa(ao), 0);
+  }
+
+  template <typename n, typename q> void ag(n z, n ao, q ap)
+  {
+    af<ad::j>(l(z), l(ao), ap);
+  }
+
+  template <typename> class allocator;
+  template <typename ah, typename ai, typename aj> void ak(ah, ai, aj);
+
+  template <typename s> class aq
+  {
+    template <typename am> struct ar { using f = am *; };
+  public:
+    using an = typename ar<s>::f;
+  };
+
+  template <typename s> class as
+  {
+  public:
+    using an = typename aq<s>::an;
+    an operator->();
+  };
+
+  struct ay
+  {
+    int at();
+  };
+
+  template <typename s, typename = allocator<s>> class vector : ay
+  {
+  public:
+    long au();
+    long x;
+    void av() { _M_default_append(x); }
+    void _M_default_append(unsigned long);
+    void aw();
+    long ax(int);
+  };
+
+  template <typename s, typename y>
+  void vector<s, y>::_M_default_append(unsigned long z)
+  {
+    long az = au();
+    int w = at(), bc = at();
+    i = ax(w);
+    m = ax(w);
+    if (i || m)
+      aw();
+    ak(az, z, bc);
+  }
+}
+
+namespace llvm
+{
+  template <int bd> class bh
+  {
+    enum { bf = bd } * bg[bf];
+  };
+
+  template <class> class bi;
+
+  class bm
+  {
+    using bj = bi<int>;
+    std::as<bj> bk;
+    void bl();
+  };
+
+  template <class> struct bn;
+
+  class br
+  {
+    bh<8> bo;
+  };
+
+  class ca
+  {
+    int *d;
+    int e;
+  };
+
+  template <class bp> class bv : std::al, br
+  {
+    g<std::t<ca>, 8> b;
+  };
+
+  template <class ab> bv<ab> bt(ab);
+
+  class BlockFrequencyInfoImplBase
+  {
+  public:
+    struct FrequencyData;
+    std::vector<FrequencyData> bu;
+  };
+
+  template <class> struct cb { using bw = int; };
+  template <class bx> class bi : BlockFrequencyInfoImplBase
+  {
+    using bw = typename cb<bx>::bw;
+  public:
+    void bl();
+  };
+
+  template <class bx> void bi<bx>::bl()
+  {
+    const bw *by;
+    bv<const int *> bz;
+    ag(bz, bt(by), 0);
+    bu.av();
+  }
+
+  template <> struct bn<const int *> { using u = ca; };
+  void bm::bl() { bk->bl(); }
+}
+
+/* The t1 register is to initial symbol reference for call instruction.  */
+/* { dg-final { scan-assembler "la\tt1,.*FrequencyData.*_M_default_append.*" } } */