Re: [PATCH v1 2/6] LoongArch: Added Loongson SX base instruction support.

2023-06-30 Thread WANG Xuerui




On 2023/6/30 10:16, Chenghui Pan wrote:

[snip]
---
  gcc/config/loongarch/constraints.md|  128 +-
  gcc/config/loongarch/loongarch-builtins.cc |   10 +
  gcc/config/loongarch/loongarch-modes.def   |   38 +
  gcc/config/loongarch/loongarch-protos.h|   31 +
  gcc/config/loongarch/loongarch.cc  | 2235 +-
  gcc/config/loongarch/loongarch.h   |   65 +-
  gcc/config/loongarch/loongarch.md  |   44 +-
  gcc/config/loongarch/lsx.md| 4490 
  gcc/config/loongarch/predicates.md |  333 +-
  9 files changed, 7184 insertions(+), 190 deletions(-)
  create mode 100644 gcc/config/loongarch/lsx.md

diff --git a/gcc/config/loongarch/constraints.md 
b/gcc/config/loongarch/constraints.md
index 7a38cd07ae9..1dd56af07c4 100644
--- a/gcc/config/loongarch/constraints.md
+++ b/gcc/config/loongarch/constraints.md
@@ -30,8 +30,7 @@
  ;; "h" <-unused
  ;; "i" "Matches a general integer constant." (Global non-architectural)
  ;; "j" SIBCALL_REGS
-;; "k" "A memory operand whose address is formed by a base register and
-;;  (optionally scaled) index register."
+;; "k" <-unused
  ;; "l" "A signed 16-bit constant."
  ;; "m" "A memory operand whose address is formed by a base register and offset
  ;;  that is suitable for use in instructions with the same addressing mode
@@ -80,13 +79,14 @@
  ;; "N" <-unused
  ;; "O" <-unused
  ;; "P" <-unused
-;; "Q" <-unused
+;; "Q" "A signed 12-bit constant"
  ;; "R" <-unused
  ;; "S" <-unused
  ;; "T" <-unused
  ;; "U" <-unused
  ;; "V" "Matches a non-offsettable memory reference." (Global 
non-architectural)
-;; "W" <-unused
+;; "W" "A memory address based on a member of @code{BASE_REG_CLASS}.  This is
+;; true for all references."
  ;; "X" "Matches anything." (Global non-architectural)
  ;; "Y" -
  ;;"Yd"
@@ -214,6 +214,63 @@ (define_constraint "Le"
(and (match_code "const_int")
 (match_test "loongarch_addu16i_imm12_operand_p (ival, SImode)")))
  
+(define_constraint "M"

+  "A constant that cannot be loaded using @code{lui}, @code{addiu}
+   or @code{ori}."
+  (and (match_code "const_int")
+   (not (match_test "IMM12_OPERAND (ival)"))
+   (not (match_test "IMM12_OPERAND_UNSIGNED (ival)"))
+   (not (match_test "LU12I_OPERAND (ival)"
+
+(define_constraint "N"
+  "A constant in the range -65535 to -1 (inclusive)."
+  (and (match_code "const_int")
+   (match_test "ival >= -0x && ival < 0")))
+
+(define_constraint "O"
+  "A signed 15-bit constant."
+  (and (match_code "const_int")
+   (match_test "ival >= -0x4000 && ival < 0x4000")))
+
+(define_constraint "P"
+  "A constant in the range 1 to 65535 (inclusive)."
+  (and (match_code "const_int")
+   (match_test "ival > 0 && ival < 0x1")))


These constraints are meant to be exposed for developers to use, right? 
If not so they should probably be marked "@internal", and if so you 
should update the docs as well.


Also these are not documented in the comment block at the top of file.


+
+;; General constraints
+
+(define_memory_constraint "R"
+  "An address that can be used in a non-macro load or store."
+  (and (match_code "mem")
+   (match_test "loongarch_address_insns (XEXP (op, 0), mode, false) == 
1")))


Similarly, is this "R" constraint meant to be exposed as well? Sure 
one's free to choose letters but "R" IMO strongly implies something 
related to registers, not addresses...



+(define_constraint "S"
+  "@internal
+   A constant call address."
+  (and (match_operand 0 "call_insn_operand")
+   (match_test "CONSTANT_P (op)")))


Additionally, IMO we probably should minimize our use of single-letter 
constraints that don't overlap with other architectures' similar usage. 
(I know that several projects have accepted LSX/LASX code well ahead of 
this series, but I don't know off my head if their code used any inline 
asm instead of C intrinsics. Intuitively this shouldn't be a concern 
though.)


Overall, I'd recommend moving all single-letter constraints added here 
to a two-letter namespace, so everything is better namespaced and easier 
to remember (e.g. if we choose something like "Vx" or "Yx" for 
everything vector-related, it'd be a lot easier to mentally associate 
the two-letter incantations with correct semantics.)



+
+(define_constraint "YG"
+  "@internal
+   A vector zero."
+  (and (match_code "const_vector")
+   (match_test "op == CONST0_RTX (mode)")))
+
+(define_constraint "YA"
+  "@internal
+   An unsigned 6-bit constant."
+  (and (match_code "const_int")
+   (match_test "UIMM6_OPERAND (ival)")))
+
+(define_constraint "YB"
+  "@internal
+   A signed 10-bit constant."
+  (and (match_code "const_int")
+   (match_test "IMM10_OPERAND (ival)")))
+
+(define_constraint "Yb"
+   "@internal"
+   (match_operand 0 "qi_mask_operand"))
+
  (define_constraint "Yd"
"@internal
 A constant @code{move_operand} that can be safely 

Re: [PATCH v1 2/6] LoongArch: Added Loongson SX base instruction support.

2023-06-30 Thread Xi Ruoyao via Gcc-patches
On Fri, 2023-06-30 at 10:16 +0800, Chenghui Pan wrote:
> +(define_c_enum "unspec" [
> +  UNSPEC_LSX_ASUB_S
> +  UNSPEC_LSX_VABSD_U
> +  UNSPEC_LSX_VAVG_S

/* ... */

To me many of them can be modeled using RTL templates, instead of an
unspec.
-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University


Re: [PATCH v1 2/6] LoongArch: Added Loongson SX base instruction support.

2023-06-30 Thread Xi Ruoyao via Gcc-patches
On Fri, 2023-06-30 at 10:16 +0800, Chenghui Pan wrote:
>  
> +  int use_vecarg_p = TARGET_VECARG
> +    && LSX_SUPPORTED_MODE_P (mode);
> +
>    memset (info, 0, sizeof (*info));
>    info->gpr_offset = cum->num_gprs;
>    info->fpr_offset = cum->num_fprs;
> @@ -535,7 +546,7 @@ loongarch_get_arg_info (struct loongarch_arg_info *info,
>  
>    /* Pass one- or two-element floating-point aggregates in FPRs.  */
>    if ((info->num_fprs
> -  = loongarch_pass_aggregate_num_fpr (type, fields))
> +  = loongarch_pass_aggregate_num_fpr (type, fields, use_vecarg_p))
>   && info->fpr_offset + info->num_fprs <= MAX_ARGS_IN_REGISTERS)
> switch (info->num_fprs)
>   {

No, this is breaking ABI.  use_vecarg_p can be only set if we invent a
new ABI (it won't be LP64D anymore), or we add some special switch for
it (like x86's -msseregparm and sseregparm attribute).

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University