public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Palmer Dabbelt <palmer@dabbelt.com>
To: gcc-patches@gcc.gnu.org
Cc: rjiejie@linux.alibaba.com
Subject: Re: [PATCH 2/2] RISC-V: Add ldr/str instruction for T-HEAD.
Date: Tue, 13 Jul 2021 11:06:22 -0700 (PDT)	[thread overview]
Message-ID: <mhng-83d8d9dc-d9cc-439d-8f61-2c506c36ffaa@palmerdabbelt-glaptop> (raw)
In-Reply-To: <20210629081107.28391-3-rjiejie@linux.alibaba.com>

On Tue, 29 Jun 2021 01:11:07 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> 	gcc/
> 	* gcc/config/riscv/riscv-opts.h (TARGET_LDR): New.
> 	(TARGET_LDUR): Likewise.
> 	* gcc/config/riscv/riscv.h (INDEX_REG_CLASS): Use TARGET_LDR.
> 	(REGNO_OK_FOR_INDEX_P): Use TARGET_LDR.
> 	(REG_OK_FOR_INDEX_P): Use REGNO_OK_FOR_INDEX_P.
> 	* gcc/config/riscv/riscv.c (riscv_address_type): Add ADDRESS_REG_REG,
> 	ADDRESS_REG_UREG.
> 	(riscv_address_info): Add shift.
> 	(riscv_classify_address_index): New.
> 	(riscv_classify_address): Use riscv_classify_address_index.
> 	(riscv_legitimize_address_index_p): New.
> 	(riscv_output_move_index): New.
> 	(riscv_output_move): Add parameter, Use riscv_output_move_index.
> 	(riscv_print_operand_address): Use ADDRESS_REG_REG, ADDRESS_REG_UREG.
> 	* gcc/config/riscv/riscv-protos.h (riscv_output_move): Update riscv_output_move.
> 	* gcc/config/riscv/riscv.md (zero_extendsidi2): Use riscv_output_move.
> 	(zero_extendhi<GPR:mode>2): Likewise.
> 	(zero_extendqi<SUPERQI:mode>2): Likewise.
> 	(extendsidi2): Likewise.
> 	(extend<SHORT:mode><SUPERQI:mode>2): Likewise.
> 	* gcc/config/riscv/predicates.md (sync_memory_operand): New.
> 	* gcc/config/riscv/sync.md (atomic_store<mode>): Use sync_memory_operand.
> 	(atomic_<atomic_optab><mode>): Likewise.
> 	(atomic_fetch_<atomic_optab><mode>): Likewise.
> 	(atomic_exchange<mode>): Likewise.
> 	(atomic_cas_value_strong<mode>): Likewise.
> 	(atomic_compare_and_swap<mode>): Likewise.
> 	(atomic_test_and_set): Likewise.
>
> 	gcc/testsuite/
> 	* gcc.target/riscv/xthead/riscv-xthead.exp: New.
> 	* gcc.target/riscv/xthead/ldr.c: Likewise.
> ---
>  gcc/config/riscv/predicates.md                |   4 +
>  gcc/config/riscv/riscv-opts.h                 |   3 +
>  gcc/config/riscv/riscv-protos.h               |   2 +-
>  gcc/config/riscv/riscv.c                      | 234 ++++++++++++++++--
>  gcc/config/riscv/riscv.h                      |   7 +-
>  gcc/config/riscv/riscv.md                     |  50 ++--
>  gcc/config/riscv/sync.md                      |  14 +-
>  gcc/testsuite/gcc.target/riscv/xthead/ldr.c   |  34 +++
>  .../gcc.target/riscv/xthead/riscv-xthead.exp  |  41 +++
>  9 files changed, 348 insertions(+), 41 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xthead/ldr.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xthead/riscv-xthead.exp
>
> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> index 232115135544..802e7a40e880 100644
> --- a/gcc/config/riscv/predicates.md
> +++ b/gcc/config/riscv/predicates.md
> @@ -217,3 +217,7 @@
>  {
>    return riscv_gpr_save_operation_p (op);
>  })
> +
> +(define_predicate "sync_memory_operand"
> +  (and (match_operand 0 "memory_operand")
> +       (match_code "reg" "0")))

This should be split out into a standalone patch: it's really 
preparatory work for the reg/reg instructions, having it standalone will 
make it easier to review.

> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> index a2d84a66f037..d3163cb2377c 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -76,4 +76,7 @@ enum stack_protector_guard {
>  #define MASK_XTHEAD_C (1 << 0)
>  #define TARGET_XTHEAD_C ((riscv_x_subext & MASK_XTHEAD_C) != 0)
>
> +#define TARGET_LDR (TARGET_XTHEAD_C)
> +#define TARGET_LDUR (TARGET_XTHEAD_C)
> +
>  #endif /* ! GCC_RISCV_OPTS_H */
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 43d7224d6941..3a218f327c42 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -52,9 +52,9 @@ extern bool riscv_legitimize_move (machine_mode, rtx, rtx);
>  extern rtx riscv_subword (rtx, bool);
>  extern bool riscv_split_64bit_move_p (rtx, rtx);
>  extern void riscv_split_doubleword_move (rtx, rtx);
> -extern const char *riscv_output_move (rtx, rtx);
>  extern const char *riscv_output_return ();
>  #ifdef RTX_CODE
> +extern const char *riscv_output_move (rtx, rtx, rtx_code outer = UNKNOWN);
>  extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx);
>  extern void riscv_expand_float_scc (rtx, enum rtx_code, rtx, rtx);
>  extern void riscv_expand_conditional_branch (rtx, enum rtx_code, rtx, rtx);
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index 576960bb37cb..7d321826f669 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -80,6 +80,12 @@ along with GCC; see the file COPYING3.  If not see
>         A natural register + offset address.  The register satisfies
>         riscv_valid_base_register_p and the offset is a const_arith_operand.
>
> +  ADDRESS_REG_REG
> +       A base register indexed by (optionally scaled) register.
> +
> +  ADDRESS_REG_UREG
> +       A base register indexed by (optionally scaled) zero-extended register.
> +
>     ADDRESS_LO_SUM
>         A LO_SUM rtx.  The first operand is a valid base register and
>         the second operand is a symbolic address.
> @@ -91,6 +97,8 @@ along with GCC; see the file COPYING3.  If not see
>         A constant symbolic address.  */
>  enum riscv_address_type {
>    ADDRESS_REG,
> +  ADDRESS_REG_REG,
> +  ADDRESS_REG_UREG,
>    ADDRESS_LO_SUM,
>    ADDRESS_CONST_INT,
>    ADDRESS_SYMBOLIC
> @@ -175,6 +183,11 @@ struct riscv_arg_info {
>     ADDRESS_REG
>         REG is the base register and OFFSET is the constant offset.
>
> +  ADDRESS_REG_REG
> +  ADDRESS_REG_UREG
> +       REG is the base register and OFFSET is the indexed register,
> +       and SHIFT is scaled value.
> +
>     ADDRESS_LO_SUM
>         REG and OFFSET are the operands to the LO_SUM and SYMBOL_TYPE
>         is the type of symbol it references.
> @@ -186,6 +199,7 @@ struct riscv_address_info {
>    rtx reg;
>    rtx offset;
>    enum riscv_symbol_type symbol_type;
> +  int shift;
>  };
>
>  /* One stage in a constant building sequence.  These sequences have
> @@ -821,6 +835,104 @@ riscv_valid_lo_sum_p (enum riscv_symbol_type sym_type, machine_mode mode,
>    return true;
>  }
>
> +/* Return true if address offset is a valid index.  If it is, fill in INFO

"valid index" is kind of vague here: IIUC it's really a valid index for 
this specific flavor of reg+reg addressing.

> +   appropriately.  STRICT_P is true if REG_OK_STRICT is in effect.  */
> +
> +bool
> +riscv_classify_address_index (struct riscv_address_info *info, rtx x,
> +      machine_mode mode, bool strict_p)

This should be static, as it's only called internally.

> +{
> +  enum riscv_address_type type = ADDRESS_REG_REG;;
> +  rtx index;
> +  int shift = 0;
> +
> +  if (!TARGET_LDR)
> +    return false;
> +
> +  if (FLOAT_MODE_P (mode))
> +    return false;
> +
> +  /* (reg:P) */
> +  if ((REG_P (x) || GET_CODE (x) == SUBREG)
> +      && GET_MODE (x) == Pmode)
> +    {
> +      index = x;
> +      shift = 0;
> +    }
> +  /* (zero_extend:DI (reg:SI)) */
> +  else if (TARGET_LDUR
> +	   && GET_CODE (x) == ZERO_EXTEND
> +	   && GET_MODE (x) == DImode
> +	   && GET_MODE (XEXP (x, 0)) == SImode)
> +    {
> +      type = ADDRESS_REG_UREG;
> +      index = XEXP (x, 0);
> +      shift = 0;
> +    }
> +  /* (mult:DI (zero_extend:DI (reg:SI)) (const_int scale)) */
> +  else if (TARGET_LDUR
> +	   && GET_CODE (x) == MULT
> +	   && GET_CODE (XEXP (x, 0)) == ZERO_EXTEND
> +	   && GET_MODE (XEXP (x, 0)) == DImode
> +	   && GET_MODE (XEXP (XEXP (x, 0), 0)) == SImode
> +	   && CONST_INT_P (XEXP (x, 1)))
> +    {
> +      type = ADDRESS_REG_UREG;
> +      index = XEXP (XEXP (x, 0), 0);
> +      shift = exact_log2 (INTVAL (XEXP (x, 1)));
> +    }
> +  /* (ashift:DI (zero_extend:DI (reg:SI)) (const_int shift)) */
> +  else if (TARGET_LDUR
> +	   && GET_CODE (x) == ASHIFT
> +	   && GET_CODE (XEXP (x, 0)) == ZERO_EXTEND
> +	   && GET_MODE (XEXP (x, 0)) == DImode
> +	   && GET_MODE (XEXP (XEXP (x, 0), 0)) == SImode
> +	   && CONST_INT_P (XEXP (x, 1)))
> +    {
> +      type = ADDRESS_REG_UREG;
> +      index = XEXP (XEXP (x, 0), 0);
> +      shift = INTVAL (XEXP (x, 1));
> +    }
> +  /* (mult:P (reg:P) (const_int scale)) */
> +  else if (GET_CODE (x) == MULT
> +	   && GET_MODE (x) == Pmode
> +	   && GET_MODE (XEXP (x, 0)) == Pmode
> +	   && CONST_INT_P (XEXP (x, 1)))
> +    {
> +      index = XEXP (x, 0);
> +      shift = exact_log2 (INTVAL (XEXP (x, 1)));
> +    }
> +  /* (ashift:P (reg:P) (const_int shift)) */
> +  else if (GET_CODE (x) == ASHIFT
> +	   && GET_MODE (x) == Pmode
> +	   && GET_MODE (XEXP (x, 0)) == Pmode
> +	   && CONST_INT_P (XEXP (x, 1)))
> +    {
> +      index = XEXP (x, 0);
> +      shift = INTVAL (XEXP (x, 1));
> +    }
> +  else
> +    return false;

This kind of stuff seem like it's better done in an MD file.  Is there a 
reason we can't just match these patterns and emit the relevant 
instructions over there?

> +
> +  if (shift != 0 && !IN_RANGE (shift, 1, 3))
> +    return false;
> +
> +  if (!strict_p
> +      && GET_CODE (index) == SUBREG
> +      && contains_reg_of_mode[GENERAL_REGS][GET_MODE (SUBREG_REG (index))])
> +    index = SUBREG_REG (index);
> +
> +  if (riscv_valid_base_register_p (index, mode, strict_p))
> +    {
> +      info->type = type;
> +      info->offset = index;
> +      info->shift = shift;
> +      return true;
> +    }
> +
> +  return false;
> +}
> +
>  /* Return true if X is a valid address for machine mode MODE.  If it is,
>     fill in INFO appropriately.  STRICT_P is true if REG_OK_STRICT is in
>     effect.  */
> @@ -839,6 +951,18 @@ riscv_classify_address (struct riscv_address_info *info, rtx x,
>        return riscv_valid_base_register_p (info->reg, mode, strict_p);
>
>      case PLUS:
> +      if (riscv_valid_base_register_p (XEXP (x, 0), mode, strict_p)
> +	  && riscv_classify_address_index (info, XEXP (x, 1), mode, strict_p))
> +	{
> +	  info->reg = XEXP (x, 0);
> +	  return true;
> +	}
> +      else if (riscv_valid_base_register_p (XEXP (x, 1), mode, strict_p)
> +	       && riscv_classify_address_index (info, XEXP (x, 0), mode, strict_p))
> +	{
> +	  info->reg = XEXP (x, 1);
> +	  return true;
> +	}
>        info->type = ADDRESS_REG;
>        info->reg = XEXP (x, 0);
>        info->offset = XEXP (x, 1);
> @@ -2047,11 +2171,70 @@ riscv_split_doubleword_move (rtx dest, rtx src)
>       }
>  }
>  \f
> +/* Return TRUE if X is a legitimate address index.  */
> +
> +bool
> +riscv_legitimize_address_index_p (rtx x, machine_mode mode, bool uindex)
> +{
> +  struct riscv_address_info addr;
> +  rtx op0, op1;
> +
> +  if (GET_CODE (x) != PLUS)
> +    return false;
> +
> +  op0 = XEXP (x, 0);
> +  op1 = XEXP (x, 1);
> +
> +  return ((riscv_valid_base_register_p (op0, mode, false)
> +	   && riscv_classify_address_index (&addr, op1, mode, false))
> +	  || (riscv_valid_base_register_p (op1, mode, false)
> +	      && riscv_classify_address_index (&addr, op0, mode, false)))
> +	 && (!uindex || addr.type == ADDRESS_REG_UREG);
> +}
> +
> +/* Return the LDR or STR instructions.  Assume
> +   that X is MEM operand.  */

That should be checked and provide an ICE rather than generating invalid 
assembly.

> +
> +const char *
> +riscv_output_move_index (rtx x, machine_mode mode, bool ldr, rtx_code outer)
> +{
> +  static char buf[128] = {0};
> +
> +  int index = exact_log2 (GET_MODE_SIZE (mode));
> +  if (!IN_RANGE (index, 0, 3))
> +    return NULL;
> +
> +  if (!riscv_legitimize_address_index_p (x, mode, false))
> +    return NULL;
> +
> +  bool uindex = riscv_legitimize_address_index_p (x, mode, true);
> +
> +  const char *const insn[][4] =
> +  {
> +    {
> +      "s%srb\t%%z1,%%0",
> +      "s%srh\t%%z1,%%0",
> +      "s%srw\t%%z1,%%0",
> +      "s%srd\t%%z1,%%0"
> +    },
> +    {
> +      "l%srbu\t%%0,%%1",
> +      "l%srhu\t%%0,%%1",
> +      "l%srw%s\t%%0,%%1",
> +      "l%srd\t%%0,%%1"
> +    }

I haven't seen assembler patches for these.  IIUC these are t-head 
instructions (as opposed to RISC-V instructions), so they should be 
prefixed somehow to avoid possibly conflicting with RISC-V instructions 
that may show up in the future.

> +  };
> +
> +  snprintf (buf, sizeof (buf), insn[ldr][index], uindex ? "u" : "", outer == ZERO_EXTEND ? "u" : "");
> +
> +  return buf;
> +}
> +
>  /* Return the appropriate instructions to move SRC into DEST.  Assume
>     that SRC is operand 1 and DEST is operand 0.  */
>
>  const char *
> -riscv_output_move (rtx dest, rtx src)
> +riscv_output_move (rtx dest, rtx src, rtx_code outer)
>  {
>    enum rtx_code dest_code, src_code;
>    machine_mode mode;
> @@ -2071,13 +2254,20 @@ riscv_output_move (rtx dest, rtx src)
>  	return dbl_p ? "fmv.x.d\t%0,%1" : "fmv.x.w\t%0,%1";
>
>        if (src_code == MEM)
> -	switch (GET_MODE_SIZE (mode))
> -	  {
> -	  case 1: return "lbu\t%0,%1";
> -	  case 2: return "lhu\t%0,%1";
> -	  case 4: return "lw\t%0,%1";
> -	  case 8: return "ld\t%0,%1";
> -	  }
> +	{
> +	  const char *insn = NULL;
> +	  insn = riscv_output_move_index (XEXP (src, 0), GET_MODE (src), true, outer);
> +	  if (insn)
> +	    return insn;
> +
> +	  switch (GET_MODE_SIZE (GET_MODE (src)))
> +	    {
> +	    case 1: return "lbu\t%0,%1";
> +	    case 2: return "lhu\t%0,%1";
> +	    case 4: return outer == ZERO_EXTEND ? "lwu\t%0,%1" : "lw\t%0,%1";
> +	    case 8: return "ld\t%0,%1";
> +	    }
> +	}
>
>        if (src_code == CONST_INT)
>  	return "li\t%0,%1";
> @@ -2114,13 +2304,20 @@ riscv_output_move (rtx dest, rtx src)
>  	    }
>  	}
>        if (dest_code == MEM)
> -	switch (GET_MODE_SIZE (mode))
> -	  {
> -	  case 1: return "sb\t%z1,%0";
> -	  case 2: return "sh\t%z1,%0";
> -	  case 4: return "sw\t%z1,%0";
> -	  case 8: return "sd\t%z1,%0";
> -	  }
> +	{
> +	  const char *insn = NULL;
> +	  insn = riscv_output_move_index (XEXP (dest, 0), GET_MODE (dest), false, outer);
> +	  if (insn)
> +	    return insn;
> +
> +	  switch (GET_MODE_SIZE (mode))
> +	    {
> +	    case 1: return "sb\t%z1,%0";
> +	    case 2: return "sh\t%z1,%0";
> +	    case 4: return "sw\t%z1,%0";
> +	    case 8: return "sd\t%z1,%0";
> +	    }
> +	}
>      }
>    if (src_code == REG && FP_REG_P (REGNO (src)))
>      {
> @@ -3504,6 +3701,13 @@ riscv_print_operand_address (FILE *file, machine_mode mode ATTRIBUTE_UNUSED, rtx
>  	fprintf (file, "(%s)", reg_names[REGNO (addr.reg)]);
>  	return;
>
> +      case ADDRESS_REG_REG:
> +      case ADDRESS_REG_UREG:
> +	fprintf (file, "%s,%s,%u", reg_names[REGNO (addr.reg)],
> +				   reg_names[REGNO (addr.offset)],
> +				   addr.shift);
> +	return;
> +
>        case ADDRESS_LO_SUM:
>  	riscv_print_operand_reloc (file, addr.offset, false);
>  	fprintf (file, "(%s)", reg_names[REGNO (addr.reg)]);
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index f47d5b40a66a..d54b81270da1 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -492,7 +492,7 @@ enum reg_class
>     factor or added to another register (as well as added to a
>     displacement).  */
>
> -#define INDEX_REG_CLASS NO_REGS
> +#define INDEX_REG_CLASS (TARGET_LDR ? GR_REGS : NO_REGS)
>
>  /* We generally want to put call-clobbered registers ahead of
>     call-saved ones.  (IRA expects this.)  */
> @@ -636,7 +636,8 @@ typedef struct {
>
>  /* Addressing modes, and classification of registers for them.  */
>
> -#define REGNO_OK_FOR_INDEX_P(REGNO) 0
> +#define REGNO_OK_FOR_INDEX_P(REGNO) \
> +  (TARGET_LDR ? riscv_regno_mode_ok_for_base_p (REGNO, VOIDmode, 1) : 0)
>  #define REGNO_MODE_OK_FOR_BASE_P(REGNO, MODE) \
>    riscv_regno_mode_ok_for_base_p (REGNO, MODE, 1)
>
> @@ -659,7 +660,7 @@ typedef struct {
>    riscv_regno_mode_ok_for_base_p (REGNO (X), MODE, 1)
>  #endif
>
> -#define REG_OK_FOR_INDEX_P(X) 0
> +#define REG_OK_FOR_INDEX_P(X) REGNO_OK_FOR_INDEX_P (REGNO (X))
>
>  /* Maximum number of registers that can appear in a valid memory address.  */
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index f88877fd5966..5229b00fb76c 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -1307,9 +1307,13 @@
>  	(zero_extend:DI
>  	    (match_operand:SI 1 "nonimmediate_operand" " r,m")))]
>    "TARGET_64BIT"
> -  "@
> -   #
> -   lwu\t%0,%1"
> +  {
> +    switch (which_alternative)
> +      {
> +      default: return "#";
> +      case 1: return riscv_output_move (operands[0], operands[1], ZERO_EXTEND);
> +      }
> +  }
>    "&& reload_completed
>     && REG_P (operands[1])
>     && !paradoxical_subreg_p (operands[0])"
> @@ -1326,9 +1330,13 @@
>  	(zero_extend:GPR
>  	    (match_operand:HI 1 "nonimmediate_operand" " r,m")))]
>    ""
> -  "@
> -   #
> -   lhu\t%0,%1"
> +  {
> +    switch (which_alternative)
> +      {
> +      default: return "#";
> +      case 1: return riscv_output_move (operands[0], operands[1]);
> +      }
> +  }
>    "&& reload_completed
>     && REG_P (operands[1])
>     && !paradoxical_subreg_p (operands[0])"
> @@ -1348,9 +1356,13 @@
>  	(zero_extend:SUPERQI
>  	    (match_operand:QI 1 "nonimmediate_operand" " r,m")))]
>    ""
> -  "@
> -   andi\t%0,%1,0xff
> -   lbu\t%0,%1"
> +  {
> +    switch (which_alternative)
> +      {
> +      default: return "andi\t%0,%1,0xff";
> +      case 1: return riscv_output_move (operands[0], operands[1]);
> +      }
> +  }
>    [(set_attr "move_type" "andi,load")
>     (set_attr "mode" "<SUPERQI:MODE>")])
>
> @@ -1366,9 +1378,13 @@
>  	(sign_extend:DI
>  	    (match_operand:SI 1 "nonimmediate_operand" " r,m")))]
>    "TARGET_64BIT"
> -  "@
> -   sext.w\t%0,%1
> -   lw\t%0,%1"
> +  {
> +    switch (which_alternative)
> +      {
> +      default: return "sext.w\t%0,%1";
> +      case 1: return riscv_output_move (operands[0], operands[1]);
> +      }
> +  }
>    [(set_attr "move_type" "move,load")
>     (set_attr "mode" "DI")])
>
> @@ -1377,9 +1393,13 @@
>  	(sign_extend:SUPERQI
>  	    (match_operand:SHORT 1 "nonimmediate_operand" " r,m")))]
>    ""
> -  "@
> -   #
> -   l<SHORT:size>\t%0,%1"
> +  {
> +    switch (which_alternative)
> +      {
> +      default: return "#";
> +      case 1: return riscv_output_move (operands[0], operands[1]);
> +      }
> +  }
>    "&& reload_completed
>     && REG_P (operands[1])
>     && !paradoxical_subreg_p (operands[0])"
> diff --git a/gcc/config/riscv/sync.md b/gcc/config/riscv/sync.md
> index 747a799e2377..5ad1d48a6ce0 100644
> --- a/gcc/config/riscv/sync.md
> +++ b/gcc/config/riscv/sync.md
> @@ -59,7 +59,7 @@
>
>  ;; Implement atomic stores with amoswap.  Fall back to fences for atomic loads.
>  (define_insn "atomic_store<mode>"
> -  [(set (match_operand:GPR 0 "memory_operand" "=A")
> +  [(set (match_operand:GPR 0 "sync_memory_operand" "=A")
>      (unspec_volatile:GPR
>        [(match_operand:GPR 1 "reg_or_0_operand" "rJ")
>         (match_operand:SI 2 "const_int_operand")]      ;; model
> @@ -69,7 +69,7 @@
>    [(set (attr "length") (const_int 8))])
>
>  (define_insn "atomic_<atomic_optab><mode>"
> -  [(set (match_operand:GPR 0 "memory_operand" "+A")
> +  [(set (match_operand:GPR 0 "sync_memory_operand" "+A")
>  	(unspec_volatile:GPR
>  	  [(any_atomic:GPR (match_dup 0)
>  		     (match_operand:GPR 1 "reg_or_0_operand" "rJ"))
> @@ -81,7 +81,7 @@
>
>  (define_insn "atomic_fetch_<atomic_optab><mode>"
>    [(set (match_operand:GPR 0 "register_operand" "=&r")
> -	(match_operand:GPR 1 "memory_operand" "+A"))
> +	(match_operand:GPR 1 "sync_memory_operand" "+A"))
>     (set (match_dup 1)
>  	(unspec_volatile:GPR
>  	  [(any_atomic:GPR (match_dup 1)
> @@ -95,7 +95,7 @@
>  (define_insn "atomic_exchange<mode>"
>    [(set (match_operand:GPR 0 "register_operand" "=&r")
>  	(unspec_volatile:GPR
> -	  [(match_operand:GPR 1 "memory_operand" "+A")
> +	  [(match_operand:GPR 1 "sync_memory_operand" "+A")
>  	   (match_operand:SI 3 "const_int_operand")] ;; model
>  	  UNSPEC_SYNC_EXCHANGE))
>     (set (match_dup 1)
> @@ -106,7 +106,7 @@
>
>  (define_insn "atomic_cas_value_strong<mode>"
>    [(set (match_operand:GPR 0 "register_operand" "=&r")
> -	(match_operand:GPR 1 "memory_operand" "+A"))
> +	(match_operand:GPR 1 "sync_memory_operand" "+A"))
>     (set (match_dup 1)
>  	(unspec_volatile:GPR [(match_operand:GPR 2 "reg_or_0_operand" "rJ")
>  			      (match_operand:GPR 3 "reg_or_0_operand" "rJ")
> @@ -121,7 +121,7 @@
>  (define_expand "atomic_compare_and_swap<mode>"
>    [(match_operand:SI 0 "register_operand" "")   ;; bool output
>     (match_operand:GPR 1 "register_operand" "")  ;; val output
> -   (match_operand:GPR 2 "memory_operand" "")    ;; memory
> +   (match_operand:GPR 2 "sync_memory_operand" "")    ;; memory
>     (match_operand:GPR 3 "reg_or_0_operand" "")  ;; expected value
>     (match_operand:GPR 4 "reg_or_0_operand" "")  ;; desired value
>     (match_operand:SI 5 "const_int_operand" "")  ;; is_weak
> @@ -154,7 +154,7 @@
>
>  (define_expand "atomic_test_and_set"
>    [(match_operand:QI 0 "register_operand" "")     ;; bool output
> -   (match_operand:QI 1 "memory_operand" "+A")    ;; memory
> +   (match_operand:QI 1 "sync_memory_operand" "+A")    ;; memory
>     (match_operand:SI 2 "const_int_operand" "")]   ;; model
>    "TARGET_ATOMIC"
>  {
> diff --git a/gcc/testsuite/gcc.target/riscv/xthead/ldr.c b/gcc/testsuite/gcc.target/riscv/xthead/ldr.c
> new file mode 100644
> index 000000000000..bee469392881
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xthead/ldr.c
> @@ -0,0 +1,34 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "test ldr/str insns" { *-*-* } { "*" } { "-march=*xtheadc* -mabi=lp64*" } } */
> +
> +void foo_w(int *dest, int *src, int index)
> +{
> +    dest[index] = src[index];
> +}
> +
> +/* { dg-final { scan-assembler "lrw" } } */
> +/* { dg-final { scan-assembler "srw" } } */
> +
> +void foo_d(long *dest, long *src, int index)
> +{
> +    dest[index] = src[index];
> +}
> +
> +/* { dg-final { scan-assembler "lrd" } } */
> +/* { dg-final { scan-assembler "srd" } } */
> +
> +void foo_w_ui(int *dest, int *src, unsigned index)
> +{
> +    dest[index] = src[index];
> +}
> +
> +/* { dg-final { scan-assembler "lurw" } } */
> +/* { dg-final { scan-assembler "surw" } } */
> +
> +void foo_d_ui(long *dest, long *src, unsigned index)
> +{
> +    dest[index] = src[index];
> +}
> +
> +/* { dg-final { scan-assembler "lurd" } } */
> +/* { dg-final { scan-assembler "surd" } } */

This gets us into the question of how this is going to be tested.  IMO 
it's not really sufficient to just rely on generating assembly output 
for correctness, we need some way to run the GCC test suite with these 
instructions enabled to make sure there are no regressions.

I don't actually have one of these t-head boards, so I can't run it.  I 
also don't see any test results posted anywhere.

Additionally: are you planning on putting together a QEMU port?  I don't 
think that's strictly necessary, as the hardware is a better target, but 
it does make it easier for the rest of us to run the tests.

> diff --git a/gcc/testsuite/gcc.target/riscv/xthead/riscv-xthead.exp b/gcc/testsuite/gcc.target/riscv/xthead/riscv-xthead.exp
> new file mode 100644
> index 000000000000..2b83bf9e5a23
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xthead/riscv-xthead.exp
> @@ -0,0 +1,41 @@
> +# Copyright (C) 2017-2021 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with GCC; see the file COPYING3.  If not see
> +# <http://www.gnu.org/licenses/>.
> +
> +# GCC testsuite that uses the `dg.exp' driver.
> +
> +# Exit immediately if this isn't a RISC-V target.
> +if ![istarget riscv*-*-*] then {
> +  return
> +}
> +
> +# Load support procs.
> +load_lib gcc-dg.exp
> +
> +# If a testcase doesn't have special options, use these.
> +global DEFAULT_CFLAGS
> +if ![info exists DEFAULT_CFLAGS] then {
> +    set DEFAULT_CFLAGS " -ansi -pedantic-errors"
> +}
> +
> +# Initialize `dg'.
> +dg-init
> +
> +# Main loop.
> +dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cS\]]] \
> +	"" $DEFAULT_CFLAGS
> +
> +# All done.
> +dg-finish

  reply	other threads:[~2021-07-13 18:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-29  8:11 [PATCH 0/2] " Jojo R
2021-06-29  8:11 ` [PATCH 1/2] RISC-V: Add arch flags " Jojo R
2021-07-13 18:06   ` Palmer Dabbelt
2021-07-21 20:53     ` Jim Wilson
2021-07-22  2:16       ` Jojo R
2021-08-27  3:22         ` Jojo R
2021-06-29  8:11 ` [PATCH 2/2] RISC-V: Add ldr/str instruction " Jojo R
2021-07-13 18:06   ` Palmer Dabbelt [this message]
2021-07-09  1:30 ` [PATCH 0/2] " ALO
2021-07-11  2:31   ` ALO
2021-07-13 18:06     ` Palmer Dabbelt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mhng-83d8d9dc-d9cc-439d-8f61-2c506c36ffaa@palmerdabbelt-glaptop \
    --to=palmer@dabbelt.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rjiejie@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).