public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops
@ 2022-11-12 21:29 Philipp Tomsich
  2022-11-12 21:29 ` [PATCH 1/7] RISC-V: Recognize xventanacondops extension Philipp Tomsich
                   ` (6 more replies)
  0 siblings, 7 replies; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich


Both the XVentanaCondOps (a vendor-defined extension from Ventana
Microsystems) and the proposed ZiCondOps extensions define a
conditional-zero(-or-value) instruction, which is similar to the
following C construct:
  rd = rc ? rs : 0

This functionality can be tied back into if-convertsion and also match
some typical programming idioms.  This series includes backend support
for XVentanaCondops and infrastructure to handle conditional-zero
constructions in if-conversion.

Tested against SPEC CPU 2017.



Philipp Tomsich (7):
  RISC-V: Recognize xventanacondops extension
  RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion
  RISC-V: Support noce_try_store_flag_mask as vt.maskc<n>
  RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  RISC-V: Recognize bexti in negated if-conversion
  RISC-V: Support immediates in XVentanaCondOps
  ifcvt: add if-conversion to conditional-zero instructions

 gcc/common/config/riscv/riscv-common.cc       |   2 +
 gcc/config/riscv/predicates.md                |  12 +
 gcc/config/riscv/riscv-opts.h                 |   3 +
 gcc/config/riscv/riscv.cc                     |  14 ++
 gcc/config/riscv/riscv.md                     |  27 +++
 gcc/config/riscv/riscv.opt                    |   3 +
 gcc/config/riscv/xventanacondops.md           | 150 ++++++++++++
 gcc/ifcvt.cc                                  | 214 ++++++++++++++++++
 .../gcc.target/riscv/xventanacondops-and-01.c |  16 ++
 .../gcc.target/riscv/xventanacondops-and-02.c |  15 ++
 .../gcc.target/riscv/xventanacondops-eq-01.c  |  11 +
 .../gcc.target/riscv/xventanacondops-eq-02.c  |  14 ++
 .../riscv/xventanacondops-ifconv-imm.c        |  19 ++
 .../gcc.target/riscv/xventanacondops-le-01.c  |  17 ++
 .../gcc.target/riscv/xventanacondops-lt-01.c  |  16 ++
 .../gcc.target/riscv/xventanacondops-lt-03.c  |  17 ++
 .../gcc.target/riscv/xventanacondops-ne-01.c  |  11 +
 .../gcc.target/riscv/xventanacondops-ne-03.c  |  15 ++
 .../gcc.target/riscv/xventanacondops-ne-04.c  |  15 ++
 .../gcc.target/riscv/xventanacondops-xor-01.c |  14 ++
 20 files changed, 605 insertions(+)
 create mode 100644 gcc/config/riscv/xventanacondops.md
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ifconv-imm.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-le-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-03.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-03.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-04.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/7] RISC-V: Recognize xventanacondops extension
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 22:46   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion Philipp Tomsich
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

This adds the xventanacondops extension to the option parsing and as a
default for the ventana-vt1 core:

gcc/Changelog:

	* common/config/riscv/riscv-common.cc: Recognize
          "xventanacondops" as part of an architecture string.
	* config/riscv/riscv-cores.def (RISCV_CORE): Enable
	  "xventanacondops" by default for "ventana-vt1".
	* config/riscv/riscv-opts.h (MASK_XVENTANACONDOPS): Define.
	(TARGET_XVENTANACONDOPS): Define.
	* config/riscv/riscv.opt: Add "riscv_xventanacondops".

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/common/config/riscv/riscv-common.cc | 2 ++
 gcc/config/riscv/riscv-opts.h           | 3 +++
 gcc/config/riscv/riscv.opt              | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 4b7f777c103..6b2bdda5feb 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1247,6 +1247,8 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] =
   {"svinval", &gcc_options::x_riscv_sv_subext, MASK_SVINVAL},
   {"svnapot", &gcc_options::x_riscv_sv_subext, MASK_SVNAPOT},
 
+  {"xventanacondops", &gcc_options::x_riscv_xventanacondops, MASK_XVENTANACONDOPS},
+
   {NULL, NULL, 0}
 };
 
diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 25fd85b09b1..84c987626bc 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -189,4 +189,7 @@ enum stack_protector_guard {
    ? 0 \
    : 32 << (__builtin_popcount (riscv_zvl_flags) - 1))
 
+#define MASK_XVENTANACONDOPS (1 << 0)
+#define TARGET_XVENTANACONDOPS ((riscv_xventanacondops & MASK_XVENTANACONDOPS) != 0)
+
 #endif /* ! GCC_RISCV_OPTS_H */
diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
index 7c3ca48d1cc..9595078bdd4 100644
--- a/gcc/config/riscv/riscv.opt
+++ b/gcc/config/riscv/riscv.opt
@@ -233,6 +233,9 @@ int riscv_zm_subext
 TargetVariable
 int riscv_sv_subext
 
+TargetVariable
+int riscv_xventanacondops = 0
+
 Enum
 Name(isa_spec_class) Type(enum riscv_isa_spec_class)
 Supported ISA specs (for use with the -misa-spec= option):
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
  2022-11-12 21:29 ` [PATCH 1/7] RISC-V: Recognize xventanacondops extension Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 22:49   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n> Philipp Tomsich
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

Adds a pattern to map the output of noce_try_store_flag_mask
if-conversion in the combiner onto vt.maskc<n>; the input patterns
supported are similar to the following:
  (set (reg/v/f:DI 75 [ <retval> ])
       (and:DI (neg:DI (ne:DI (reg:DI 82)
                       (const_int 0 [0])))
               (reg/v/f:DI 75 [ <retval> ])))

This reduces dynamic instruction counts for the perlbench-workload in
SPEC CPU2017 by 0.8230%, 0.4689%, and 0.2332% (respectively, for the
each of the 3 workloads in the 'ref'-workload).

To ensure that the combine-pass doesn't get confused about
profitability, we recognize the idiom as requiring a single
instruction when the XVentanaCondOps extension is present.

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_rtx_costs): Recognize idiom for
	  vt.maskc<n> as a single insn with TARGET_XVENTANACONDOPS.
	* config/riscv/riscv.md: Include xventanacondops.md.
	* config/riscv/xventanacondops.md: New file.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xventanacondops-ne-03.c: New test.
	* gcc.target/riscv/xventanacondops-ne-04.c: New test.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/config/riscv/riscv.cc                     | 14 +++++++++
 gcc/config/riscv/riscv.md                     |  1 +
 gcc/config/riscv/xventanacondops.md           | 30 +++++++++++++++++++
 .../gcc.target/riscv/xventanacondops-ne-03.c  | 15 ++++++++++
 .../gcc.target/riscv/xventanacondops-ne-04.c  | 15 ++++++++++
 5 files changed, 75 insertions(+)
 create mode 100644 gcc/config/riscv/xventanacondops.md
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-03.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-04.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 2a94482b8ed..1883b5b13a7 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2269,6 +2269,20 @@ riscv_rtx_costs (rtx x, machine_mode mode, int outer_code, int opno ATTRIBUTE_UN
       return false;
 
     case AND:
+      /* vt.maskc/vt.maskcn for XVentanaCondOps */
+      if (TARGET_XVENTANACONDOPS && mode == word_mode
+	  && GET_CODE (XEXP (x, 0)) == NEG)
+	{
+	  rtx inner = XEXP (XEXP (x, 0), 0);
+
+	  if ((GET_CODE (inner) == EQ || GET_CODE (inner) == NE)
+	      && CONST_INT_P (XEXP (inner, 1))
+	      && INTVAL (XEXP (inner, 1)) == 0)
+	    {
+	      *total = COSTS_N_INSNS (1);
+	      return true;
+	    }
+	}
       /* slli.uw pattern for zba.  */
       if (TARGET_ZBA && TARGET_64BIT && mode == DImode
 	  && GET_CODE (XEXP (x, 0)) == ASHIFT)
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 1514e10dbd1..4331842b7b2 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -3196,3 +3196,4 @@
 (include "generic.md")
 (include "sifive-7.md")
 (include "vector.md")
+(include "xventanacondops.md")
diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
new file mode 100644
index 00000000000..641cef0e44e
--- /dev/null
+++ b/gcc/config/riscv/xventanacondops.md
@@ -0,0 +1,30 @@
+;; Machine description for X-Ventana-CondOps
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GCC.
+
+;; GCC 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, or (at your option)
+;; any later version.
+
+;; GCC 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/>.
+
+(define_code_iterator eq_or_ne [eq ne])
+(define_code_attr n [(eq "n") (ne "")])
+
+(define_insn "*vt.maskc<n>"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+	(and:DI (neg:DI (eq_or_ne:DI
+			(match_operand:DI 1 "register_operand" "r")
+			(const_int 0)))
+		(match_operand:DI 2 "register_operand" "r")))]
+  "TARGET_XVENTANACONDOPS"
+  "vt.maskc<n>\t%0,%2,%1")
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-03.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-03.c
new file mode 100644
index 00000000000..87cc69480ac
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-03.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64 -mtune=thead-c906" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" "-Os" "-Oz" } } */
+
+long long ne3(long long a, long long b)
+{
+  if (a != 0)
+    return b;
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
+
+
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-04.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-04.c
new file mode 100644
index 00000000000..3a04f7e52e5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-04.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64 -mtune=thead-c906" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+long long ne4(long long a, long long b)
+{
+  if (a != 0)
+    return 0;
+
+  return b;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
+
+
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n>
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
  2022-11-12 21:29 ` [PATCH 1/7] RISC-V: Recognize xventanacondops extension Philipp Tomsich
  2022-11-12 21:29 ` [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 23:12   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps Philipp Tomsich
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

When if-conversion in noce_try_store_flag_mask starts the sequence off
with an order-operator, our patterns for vt.maskc<n> will receive the
result of the order-operator as a register argument; consequently,
they can't know that the result will be either 1 or 0.

To convey this information (and make vt.maskc<n> applicable), we wrap
the result of the order-operator in a eq/ne against (const_int 0).
This commit adds the split pattern to handle these cases.

gcc/ChangeLog:

	* config/riscv/xventanacondops.md: Add split to wrap an an
          order-operator suitably for generating vt.maskc<n>.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>

Ref vrull/gcc#157

RISC-V: Recognize 'ge<u>'/'le<u>' operators as 'slt<u>'/'sgt<u>'

During if-conversion, if noce_try_store_flag_mask succeeds, we may see
    if (cur < next) {
        next = 0;
    }
transformed into
   27: r82:SI=ltu(r76:DI,r75:DI)
      REG_DEAD r76:DI
   28: r81:SI=r82:SI^0x1
      REG_DEAD r82:SI
   29: r80:DI=zero_extend(r81:SI)
      REG_DEAD r81:SI

This currently escapes the combiner, as RISC-V does not have a pattern
to apply the 'slt' instruction to 'geu' verbs.  By adding a pattern in
this commit, we match such cases.

gcc/ChangeLog:

	* config/riscv/predicates.md (anyge_operator): Define.
	(anygt_operator): Define.
	(anyle_operator): Define.
	(anylt_operator): Define.
	* config/riscv/riscv.md (*sge<u>_<X:mode><GPR:mode>): Add a
	  pattern to map 'geu' onto slt w/ reversed operands.
	* config/riscv/riscv.md: Helpers for ge & le.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xventanacondops-le-01.c: New test.
	* gcc.target/riscv/xventanacondops-lt-03.c: New test.

---

 gcc/config/riscv/predicates.md                | 12 +++++
 gcc/config/riscv/riscv.md                     | 26 +++++++++++
 gcc/config/riscv/xventanacondops.md           | 45 +++++++++++++++++++
 .../gcc.target/riscv/xventanacondops-le-01.c  | 17 +++++++
 .../gcc.target/riscv/xventanacondops-lt-03.c  | 17 +++++++
 5 files changed, 117 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-le-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-03.c

diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index b368c11c930..490bff688a7 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -204,6 +204,18 @@
 (define_predicate "equality_operator"
   (match_code "eq,ne"))
 
+(define_predicate "anyge_operator"
+  (match_code "ge,geu"))
+
+(define_predicate "anygt_operator"
+  (match_code "gt,gtu"))
+
+(define_predicate "anyle_operator"
+  (match_code "le,leu"))
+
+(define_predicate "anylt_operator"
+  (match_code "lt,ltu"))
+
 (define_predicate "order_operator"
   (match_code "eq,ne,lt,ltu,le,leu,ge,geu,gt,gtu"))
 
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 4331842b7b2..d1f3270a3c8 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2636,6 +2636,19 @@
   [(set_attr "type" "slt")
    (set_attr "mode" "<X:MODE>")])
 
+(define_split
+  [(set (match_operand:GPR 0 "register_operand")
+	(match_operator:GPR 1 "anyle_operator"
+	       [(match_operand:X 2 "register_operand")
+		(match_operand:X 3 "register_operand")]))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 0) (match_dup 4))
+   (set (match_dup 0) (eq:GPR (match_dup 0) (const_int 0)))]
+ {
+  operands[4] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == LE ? LT : LTU,
+				<GPR:MODE>mode, operands[3], operands[2]);
+ })
+
 (define_insn "*slt<u>_<X:mode><GPR:mode>"
   [(set (match_operand:GPR           0 "register_operand" "= r")
 	(any_lt:GPR (match_operand:X 1 "register_operand" "  r")
@@ -2657,6 +2670,19 @@
   [(set_attr "type" "slt")
    (set_attr "mode" "<X:MODE>")])
 
+(define_split
+  [(set (match_operand:GPR 0 "register_operand")
+	(match_operator:GPR 1 "anyge_operator"
+	       [(match_operand:X 2 "register_operand")
+		(match_operand:X 3 "register_operand")]))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 0) (match_dup 4))
+   (set (match_dup 0) (eq:GPR (match_dup 0) (const_int 0)))]
+{
+  operands[4] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == GE ? LT : LTU,
+				<GPR:MODE>mode, operands[2], operands[3]);
+})
+
 ;;
 ;;  ....................
 ;;
diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
index 641cef0e44e..7930ef1d837 100644
--- a/gcc/config/riscv/xventanacondops.md
+++ b/gcc/config/riscv/xventanacondops.md
@@ -28,3 +28,48 @@
 		(match_operand:DI 2 "register_operand" "r")))]
   "TARGET_XVENTANACONDOPS"
   "vt.maskc<n>\t%0,%2,%1")
+
+;; Make order operators digestible to the vt.maskc<n> logic by
+;; wrapping their result in a comparison against (const_int 0).
+
+;; "a >= b" is "!(a < b)"
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(and:X (neg:X (match_operator:X 1 "anyge_operator"
+			     [(match_operand:X 2 "register_operand")
+			      (match_operand:X 3 "register_operand")]))
+	       (match_operand:X 4 "register_operand")))
+   (clobber (match_operand:X 5 "register_operand"))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 5) (match_dup 6))
+   (set (match_dup 0) (and:X (neg:X (eq:X (match_dup 5) (const_int 0)))
+			     (match_dup 4)))]
+{
+  operands[6] = gen_rtx_fmt_ee (GET_CODE (operands[1]) == GE ? LT : LTU,
+				<X:MODE>mode, operands[2], operands[3]);
+})
+
+;; "a > b"
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(and:X (neg:X (match_operator:X 1 "anygt_operator"
+			     [(match_operand:X 2 "register_operand")
+			      (match_operand:X 3 "register_operand")]))
+	       (match_operand:X 4 "register_operand")))
+   (clobber (match_operand:X 5 "register_operand"))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 5) (match_dup 1))
+   (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
+			     (match_dup 4)))])
+
+;; "a <= b" is "!(a > b)"
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(and:X (neg:X (match_operator:X 1 "anyle_operator"
+			     [(match_operand:X 2 "register_operand")
+			      (match_operand:X 3 "arith_operand")]))
+	       (match_operand:X 4 "register_operand")))
+   (clobber (match_operand:X 5 "register_operand"))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 5) (match_dup 1))
+   (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-le-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-le-01.c
new file mode 100644
index 00000000000..231b570158a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-le-01.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zba_zbb_zbs_xventanacondops -mabi=lp64 -mbranch-cost=4" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" "-Os" "-Oz"  } } */
+
+long long sink (long long);
+
+long long lt3 (long long a, long long b)
+{
+  if (a <= b) 
+    b = 0;
+
+  return sink(b);
+}
+
+/* { dg-final { scan-assembler-times "sgt\t" 1 } } */
+/* { dg-final { scan-assembler-times "vt.maskc\t" 1 } } */
+
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-03.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-03.c
new file mode 100644
index 00000000000..b693521ba2f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-03.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_zba_zbb_zbs_xventanacondops -mabi=lp64 -mbranch-cost=4" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-O1" "-Os" "-Oz"  } } */
+
+long long sink (long long);
+
+long long lt3 (long long a, long long b)
+{
+  if (a < b) 
+    b = 0;
+
+  return sink(b);
+}
+
+/* { dg-final { scan-assembler-times "slt\t" 1 } } */
+/* { dg-final { scan-assembler-times "vt.maskcn\t" 1 } } */
+
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
                   ` (2 preceding siblings ...)
  2022-11-12 21:29 ` [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n> Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 23:41   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion Philipp Tomsich
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

Users might use explicit arithmetic operations to create a mask and
then and it, in a sequence like
    cond = (bits >> SHIFT) & 1;
    mask = ~(cond - 1);
    val &= mask;
which will present as a single-bit sign-extract.

Dependening on what combination of XVentanaCondOps and Zbs are
available, this will map to the following sequences:
 - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
 - andi + vt.maskc, if only XVentanaCondOps is available and the
                    sign-extract is operating on bits 10:0 (bit
		    11 can't be reached, as the immediate is
		    sign-extended)
 - slli + srli + and, otherwise.

gcc/ChangeLog:

	* config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
	  of a single-bit followed by AND for XVentanaCondOps.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
index 7930ef1d837..3e9d5833a4b 100644
--- a/gcc/config/riscv/xventanacondops.md
+++ b/gcc/config/riscv/xventanacondops.md
@@ -73,3 +73,49 @@
   "TARGET_XVENTANACONDOPS"
   [(set (match_dup 5) (match_dup 1))
    (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
+
+;; Users might use explicit arithmetic operations to create a mask and
+;; then and it, in a sequence like
+;;    cond = (bits >> SHIFT) & 1;
+;;    mask = ~(cond - 1);
+;;    val &= mask;
+;; which will present as a single-bit sign-extract in the combiner.
+;;
+;; This will give rise to any of the following cases:
+;; - with Zbs and XVentanaCondOps: bexti + vt.maskc
+;; - with XVentanaCondOps (but w/o Zbs):
+;;   - andi + vt.maskc, if the mask is representable in the immediate
+;;                      (which requires extra care due to the immediate
+;;                       being sign-extended)
+;;   - slli + srli + and
+;; - otherwise: slli + srli + and
+
+;; With Zbb, we have bexti for all possible bits...
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(and:X (sign_extract:X (match_operand:X 1 "register_operand")
+			       (const_int 1)
+			       (match_operand 2 "immediate_operand"))
+	       (match_operand:X 3 "register_operand")))
+   (clobber (match_operand:X 4 "register_operand"))]
+  "TARGET_XVENTANACONDOPS && TARGET_ZBS"
+  [(set (match_dup 4) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2)))
+   (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 4) (const_int 0)))
+			     (match_dup 3)))])
+
+;; ...whereas RV64I only allows us access to bits 0..10 in a single andi.
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(and:X (sign_extract:X (match_operand:X 1 "register_operand")
+			       (const_int 1)
+			       (match_operand 2 "immediate_operand"))
+	       (match_operand:X 3 "register_operand")))
+   (clobber (match_operand:X 4 "register_operand"))]
+  "TARGET_XVENTANACONDOPS && !TARGET_ZBS && (UINTVAL (operands[2]) < 11)"
+  [(set (match_dup 4) (and:X (match_dup 1) (match_dup 2)))
+   (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 4) (const_int 0)))
+			     (match_dup 3)))]
+{
+  operands[2] = GEN_INT(1 << UINTVAL(operands[2]));
+})
+
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
                   ` (3 preceding siblings ...)
  2022-11-12 21:29 ` [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 23:17   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps Philipp Tomsich
  2022-11-12 21:29 ` [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions Philipp Tomsich
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

While the positive case "if ((bits >> SHAMT) & 1)" for SHAMT 0..10 can
trigger conversion into efficient branchless sequences
  - with Zbs (bexti + neg + and)
  - with XVentanaCondOps (andi + vt.maskc)
the inverted/negated case results in
  andi a5,a0,1024
  seqz a5,a5
  neg a5,a5
  and a5,a5,a1
due to how the sequence presents to the combine pass.

This adds an additional splitter to reassociate the polarity reversed
case into bexti + addi, if Zbs is present.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>

gcc/ChangeLog:

    * config/riscv/xventanacondops.md: Add split to reassociate
      "andi + seqz + neg" into "bexti + addi".

---

 gcc/config/riscv/xventanacondops.md | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
index 3e9d5833a4b..22b4b7d103a 100644
--- a/gcc/config/riscv/xventanacondops.md
+++ b/gcc/config/riscv/xventanacondops.md
@@ -119,3 +119,12 @@
   operands[2] = GEN_INT(1 << UINTVAL(operands[2]));
 })
 
+(define_split
+  [(set (match_operand:X 0 "register_operand")
+	(neg:X (eq:X (zero_extract:X (match_operand:X 1 "register_operand")
+				     (const_int 1)
+				     (match_operand 2 "immediate_operand"))
+		     (const_int 0))))]
+  "!TARGET_XVENTANACONDOPS && TARGET_ZBS"
+  [(set (match_dup 0) (zero_extract:X (match_dup 1) (const_int 1) (match_dup 2)))
+   (set (match_dup 0) (plus:X (match_dup 0) (const_int -1)))])
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
                   ` (4 preceding siblings ...)
  2022-11-12 21:29 ` [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-17 23:36   ` Jeff Law
  2022-11-12 21:29 ` [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions Philipp Tomsich
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich, Henry Brausen

When if-conversion encounters sequences using immediates, the
sequences can't trivially map back onto vt.maskc/vt.maskcn (even if
benefitial) due to vt.maskc and vt.maskcn not having immediate forms.

This adds a splitter to rewrite opportunities for XVentanaCondOps that
operate on an immediate by first putting the immediate into a register
to enable the non-immediate vt.maskc/vt.maskcn instructions to operate
on the value.

Consider code, such as

  long func2 (long a, long c)
  {
    if (c)
      a = 2;
    else
      a = 5;
    return a;
  }

which will be converted to

  func2:
	seqz	a0,a2
	neg	a0,a0
	andi	a0,a0,3
	addi	a0,a0,2
	ret

Following this change, we generate

	li	a0,3
	vt.maskcn	a0,a0,a2
	addi	a0,a0,2
	ret

This commit also introduces a simple unit test for if-conversion with
immediate (literal) values as the sources for simple sets in the THEN
and ELSE blocks. The test checks that Ventana's conditional mask
instruction (vt.maskc<n>) is emitted as part of the resultant branchless
instruction sequence.

gcc/ChangeLog:

	* config/riscv/xventanacondops.md: Support immediates for
	  vt.maskc/vt.maskcn through a splitter.

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/xventanacondops-ifconv-imm.c: New test.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
Reviewed-by: Henry Brausen <henry.brausen@vrull.eu>

---
Ref #204

 gcc/config/riscv/xventanacondops.md           | 24 +++++++++++++++++--
 .../riscv/xventanacondops-ifconv-imm.c        | 19 +++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ifconv-imm.c

diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
index 22b4b7d103a..0e09ee91a69 100644
--- a/gcc/config/riscv/xventanacondops.md
+++ b/gcc/config/riscv/xventanacondops.md
@@ -29,6 +29,26 @@
   "TARGET_XVENTANACONDOPS"
   "vt.maskc<n>\t%0,%2,%1")
 
+;; XVentanaCondOps does not have immediate forms, so we need to do extra
+;; work to support these: if we encounter a vt.maskc/n with an immediate,
+;; we split this into a load-immediate followed by a vt.maskc/n.
+(define_split
+  [(set (match_operand:DI 0 "register_operand")
+	(and:DI (neg:DI (match_operator:DI 1 "equality_operator"
+			       [(match_operand:DI 2 "register_operand")
+				(const_int 0)]))
+		(match_operand:DI 3 "immediate_operand")))
+   (clobber (match_operand:DI 4 "register_operand"))]
+  "TARGET_XVENTANACONDOPS"
+  [(set (match_dup 4) (match_dup 3))
+   (set (match_dup 0) (and:DI (neg:DI (match_dup 1))
+			      (match_dup 4)))]
+{
+  /* Eliminate the clobber/temporary, if it is not needed. */
+  if (!rtx_equal_p (operands[0], operands[2]))
+     operands[4] = operands[0];
+})
+
 ;; Make order operators digestible to the vt.maskc<n> logic by
 ;; wrapping their result in a comparison against (const_int 0).
 
@@ -37,7 +57,7 @@
   [(set (match_operand:X 0 "register_operand")
 	(and:X (neg:X (match_operator:X 1 "anyge_operator"
 			     [(match_operand:X 2 "register_operand")
-			      (match_operand:X 3 "register_operand")]))
+			      (match_operand:X 3 "arith_operand")]))
 	       (match_operand:X 4 "register_operand")))
    (clobber (match_operand:X 5 "register_operand"))]
   "TARGET_XVENTANACONDOPS"
@@ -54,7 +74,7 @@
   [(set (match_operand:X 0 "register_operand")
 	(and:X (neg:X (match_operator:X 1 "anygt_operator"
 			     [(match_operand:X 2 "register_operand")
-			      (match_operand:X 3 "register_operand")]))
+			      (match_operand:X 3 "arith_operand")]))
 	       (match_operand:X 4 "register_operand")))
    (clobber (match_operand:X 5 "register_operand"))]
   "TARGET_XVENTANACONDOPS"
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ifconv-imm.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ifconv-imm.c
new file mode 100644
index 00000000000..0012e7b669c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ifconv-imm.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+/* Each function below should emit a vt.maskcn instruction */
+
+long
+foo0 (long a, long b, long c)
+{
+  if (c)
+    a = 0;
+  else
+    a = 5;
+  return a;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskcn\t" 1 } } */
+/* { dg-final { scan-assembler-not "beqz\t" } } */
+/* { dg-final { scan-assembler-not "bnez\t" } } */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions
  2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
                   ` (5 preceding siblings ...)
  2022-11-12 21:29 ` [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps Philipp Tomsich
@ 2022-11-12 21:29 ` Philipp Tomsich
  2022-11-12 21:47   ` Andrew Pinski
  6 siblings, 1 reply; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 21:29 UTC (permalink / raw)
  To: gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Philipp Tomsich

Some architectures, as it the case on RISC-V with the proposed
ZiCondOps and the vendor-defined XVentanaCondOps, define a
conditional-zero instruction that is equivalent to:
 - the positive form:  rd = (rc != 0) ? rs : 0
 - the negated form:   rd = (rc == 0) ? rs : 0

While noce_try_store_flag_mask will somewhat work for this case, it
will generate a number of atomic RTX that will misdirect the cost
calculation and may be too long (i.e., 4 RTX and more) to successfully
merge at combine-time.

Instead, we add two new transforms that attempt to build up what we
define as the canonical form of a conditional-zero expression:

  (set (match_operand 0 "register_operand" "=r")
       (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
                           (const_int 0)))
            (match_operand 2 "register_operand" "r")))

Architectures that provide a conditional-zero are thus expected to
define an instruction matching this pattern in their backend.

Based on this, we support the following cases:
 - noce_try_condzero:
      a ? a : b
      a ? b : 0  (and then/else swapped)
     !a ? b : 0  (and then/else swapped)
 - noce_try_condzero_arith:
     conditional-plus, conditional-minus, conditional-and,
     conditional-or, conditional-xor, conditional-shift,
     conditional-and

Given that this is hooked into the CE passes, it is less powerful than
a tree-pass (e.g., it can not transform cases where an extension, such
as for uint16_t operations is in either the then or else-branch
together with the arithmetic) but already covers a good array of cases
and triggers across SPEC CPU 2017.
Adding transofmrations in a tree pass will be considered as a future
improvement.

gcc/ChangeLog:

	* ifcvt.cc (noce_emit_insn): Add prototype.
	(noce_emit_condzero): Helper for noce_try_condzero and
	noce_try_condzero_arith transforms.
	(noce_try_condzero): New transform.
	(noce_try_condzero_arith): New transform for conditional
	arithmetic that can be built up by exploiting that the
	conditional-zero instruction will inject 0, which acts
	as the neutral element for operations.
	(noce_process_if_block): Call noce_try_condzero and
	noce_try_condzero_arith.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/xventanacondops-and-01.c: New test.
	* gcc.target/riscv/xventanacondops-and-02.c: New test.
	* gcc.target/riscv/xventanacondops-eq-01.c: New test.
	* gcc.target/riscv/xventanacondops-eq-02.c: New test.
	* gcc.target/riscv/xventanacondops-lt-01.c: New test.
	* gcc.target/riscv/xventanacondops-ne-01.c: New test.
	* gcc.target/riscv/xventanacondops-xor-01.c: New test.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/ifcvt.cc                                  | 214 ++++++++++++++++++
 .../gcc.target/riscv/xventanacondops-and-01.c |  16 ++
 .../gcc.target/riscv/xventanacondops-and-02.c |  15 ++
 .../gcc.target/riscv/xventanacondops-eq-01.c  |  11 +
 .../gcc.target/riscv/xventanacondops-eq-02.c  |  14 ++
 .../gcc.target/riscv/xventanacondops-lt-01.c  |  16 ++
 .../gcc.target/riscv/xventanacondops-ne-01.c  |  11 +
 .../gcc.target/riscv/xventanacondops-xor-01.c |  14 ++
 8 files changed, 311 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index eb8efb89a89..41c58876d05 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -97,6 +97,7 @@ static int find_if_case_2 (basic_block, edge, edge);
 static int dead_or_predicable (basic_block, basic_block, basic_block,
 			       edge, int);
 static void noce_emit_move_insn (rtx, rtx);
+static rtx_insn *noce_emit_insn (rtx);
 static rtx_insn *block_has_only_trap (basic_block);
 static void need_cmov_or_rewire (basic_block, hash_set<rtx_insn *> *,
 				 hash_map<rtx_insn *, int> *);
@@ -787,6 +788,9 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **);
 static int noce_try_minmax (struct noce_if_info *);
 static int noce_try_abs (struct noce_if_info *);
 static int noce_try_sign_mask (struct noce_if_info *);
+static rtx noce_emit_condzero (struct noce_if_info *, rtx, bool = false);
+static int noce_try_condzero (struct noce_if_info *);
+static int noce_try_condzero_arith (struct noce_if_info *);
 
 /* Return the comparison code for reversed condition for IF_INFO,
    or UNKNOWN if reversing the condition is not possible.  */
@@ -1664,6 +1668,212 @@ noce_try_addcc (struct noce_if_info *if_info)
   return FALSE;
 }
 
+/* Helper to noce_try_condzero: cond ? a : 0. */
+static rtx
+noce_emit_condzero (struct noce_if_info *if_info, rtx a, bool reverse)
+{
+  /* The canonical form for a conditional-zero-or-value is:
+       (set (match_operand 0 "register_operand" "=r")
+	    (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
+				(const_int 0)))
+		 (match_operand 2 "register_operand" "r")))
+   */
+
+  machine_mode opmode = GET_MODE (if_info->x);
+  enum rtx_code code = GET_CODE (if_info->cond);
+  rtx cond;
+  rtx op_a = XEXP (if_info->cond, 0);
+  rtx op_b = XEXP (if_info->cond, 1);
+
+  /* If it is not a EQ/NE comparison against const0_rtx, canonicalize
+     by first synthesizing a truth-value and then building a NE
+     condition around it. */
+  if ((code != EQ && code != NE) || XEXP (if_info->cond, 1) != const0_rtx)
+    {
+      rtx tmp = gen_reg_rtx (opmode);
+
+      start_sequence ();
+      cond = gen_rtx_fmt_ee (code, opmode, op_a, op_b);
+      if (!noce_emit_insn (gen_rtx_SET (tmp, cond)))
+	{
+	  end_sequence ();
+
+	  /* If we can't emit this pattern, try to reverse it and
+	     invert the polarity of the second test. */
+	  start_sequence ();
+	  cond = gen_rtx_fmt_ee (reverse_condition (code), opmode, op_a, op_b);
+	  if (!noce_emit_insn (gen_rtx_SET (tmp, cond))) {
+	    end_sequence ();
+	    return NULL_RTX;
+	  }
+
+	  /* We have recovered by reversing the first comparison,
+	     so we need change the second one around as well... */
+	  reverse = !reverse;
+	}
+      rtx_insn *seq = get_insns ();
+      end_sequence ();
+      emit_insn (seq);
+
+      /* Set up the second comparison that will be embedded in the
+	 canonical conditional-zero-or-value RTX. */
+      code = NE;
+      op_a = tmp;
+      op_b = const0_rtx;
+    }
+
+  cond = gen_rtx_fmt_ee (reverse ? reverse_condition (code) : code,
+			 opmode, op_a, op_b);
+
+  /* Build (and (neg (eq_or_ne ... const0_rtx)) (reg <a>)) */
+  rtx target = gen_reg_rtx (opmode);
+  rtx czero = gen_rtx_AND (opmode, gen_rtx_NEG (opmode, cond), a);
+  noce_emit_move_insn (target, czero);
+
+  return target;
+}
+
+/* Use a conditional-zero instruction for "if (test) x = 0;", if available. */
+static int
+noce_try_condzero (struct noce_if_info *if_info)
+{
+  rtx target;
+  rtx_insn *seq;
+  int reversep = 0;
+  rtx orig_b = NULL_RTX;
+  rtx cond = if_info->cond;
+  enum rtx_code code = GET_CODE (cond);
+  rtx cond_arg0 = XEXP (cond, 0);
+  rtx cond_arg1 = XEXP (cond, 1);
+
+  if (!noce_simple_bbs (if_info))
+    return FALSE;
+
+  /* We may encounter the form "(a != 0) ? a : b", which can be
+     simplified to "a | ((a != 0) ? 0 : b)". */
+  if (code == NE && cond_arg1 == const0_rtx &&
+      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
+    {
+      orig_b = if_info->b;
+      if_info->b = const0_rtx;
+    }
+
+  /* We may encounter the form "(a != 0) ? b : a", which can be
+     simplied to "(a != 0) ? b : 0". */
+  if (code == EQ && cond_arg1 == const0_rtx &&
+      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
+    {
+      /* We know that cond_arg0 is const_0, if the THEN branch is
+	 taken... so if it is the same as if_info->b (yes, things are
+	 backwards!), we can rewrite it with that knowledge.  */
+      if_info->b = const0_rtx;
+    }
+
+  start_sequence ();
+
+  if ((if_info->a == const0_rtx
+       && (REG_P (if_info->b) || rtx_equal_p (if_info->b, if_info->x)))
+      || ((reversep = (noce_reversed_cond_code (if_info) != UNKNOWN))
+	  && if_info->b == const0_rtx
+	  && (REG_P (if_info->a) || rtx_equal_p (if_info->a, if_info->x))))
+    {
+      target = noce_emit_condzero(if_info,
+				  reversep ? if_info->a : if_info->b,
+				  reversep);
+
+      if (orig_b && target)
+	target = expand_simple_binop (GET_MODE (if_info->x), IOR, orig_b,
+				      target, if_info->x, 0, OPTAB_WIDEN);
+
+      if (target)
+	{
+	  if (target != if_info->x)
+	    noce_emit_move_insn (if_info->x, target);
+
+	  seq = end_ifcvt_sequence (if_info);
+	  if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
+	    return FALSE;
+
+	  emit_insn_before_setloc (seq, if_info->jump,
+				   INSN_LOCATION (if_info->insn_a));
+	  if_info->transform_name = "noce_try_condzero";
+
+	  return TRUE;
+	}
+    }
+
+  end_sequence ();
+
+  return FALSE;
+}
+
+/* Convert "if (test) x op= a;" to a branchless sequence using the
+   canonical form for a conditional-zero. */
+static int
+noce_try_condzero_arith (struct noce_if_info *if_info)
+{
+  rtx target;
+  rtx_insn *seq;
+  rtx_code op = GET_CODE (if_info->a);
+  const rtx arg0 = XEXP (if_info->a, 0);
+  const rtx arg1 = XEXP (if_info->a, 1);
+
+  if (!noce_simple_bbs (if_info))
+    return FALSE;
+
+  /* Check for no else condition.  */
+  if (!rtx_equal_p (if_info->x, if_info->b))
+    return FALSE;
+
+  if (op != PLUS && op != MINUS && op != IOR && op != XOR &&
+      op != ASHIFT && op != ASHIFTRT && op != LSHIFTRT && op != AND)
+    return FALSE;
+
+  if (!rtx_equal_p (if_info->x, arg0))
+    return FALSE;
+
+  start_sequence ();
+
+  target = noce_emit_condzero(if_info, arg1, op != AND ? true : false);
+
+  if (target)
+    {
+      rtx op1 = if_info->x;
+      
+      if (op == AND)
+	{
+	  /* Emit "tmp = x & val;" followed by "tmp |= !cond ? x : 0;" */
+	  op1 = expand_simple_binop (GET_MODE (if_info->x), AND, op1,
+				     arg1, NULL_RTX, 0, OPTAB_WIDEN);
+	  op = IOR;
+	}
+
+      if (op1)
+	target = expand_simple_binop (GET_MODE (if_info->x), op, op1,
+				      target, if_info->x, 0, OPTAB_WIDEN);
+    }
+
+  if (target)
+    {
+      if (target != if_info->x)
+	noce_emit_move_insn (if_info->x, target);
+
+      seq = end_ifcvt_sequence (if_info);
+      if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
+	return FALSE;
+
+      emit_insn_before_setloc(seq, if_info->jump,
+			      INSN_LOCATION(if_info->insn_a));
+      if_info->transform_name = "noce_try_condzero_arith";
+
+      return TRUE;
+    }
+
+  end_sequence ();
+
+  return FALSE;
+}
+
 /* Convert "if (test) x = 0;" to "x &= -(test == 0);"  */
 
 static int
@@ -3967,8 +4177,12 @@ noce_process_if_block (struct noce_if_info *if_info)
     {
       if (noce_try_addcc (if_info))
 	goto success;
+      if (noce_try_condzero (if_info))
+	goto success;
       if (noce_try_store_flag_mask (if_info))
 	goto success;
+      if (noce_try_condzero_arith (if_info))
+	goto success;
       if (HAVE_conditional_move
 	  && noce_try_cmove_arith (if_info))
 	goto success;
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
new file mode 100644
index 00000000000..9b26cdf0513
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+long and1(long a, long b, long c, long d)
+{
+  if (c < d)
+    a &= b;
+
+  return a;
+}
+
+/* { dg-final { scan-assembler-times "and\t" 1 } } */
+/* { dg-final { scan-assembler-times "slt" 1 } } */
+/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
+/* { dg-final { scan-assembler-times "or\t" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
new file mode 100644
index 00000000000..66d2ec10211
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+int and2(int a, int b, long c)
+{
+  if (c)
+    a &= b;
+
+  return a;
+}
+
+/* { dg-final { scan-assembler-times "and\t" 1 } } */
+/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
+/* { dg-final { scan-assembler-times "or\t" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
new file mode 100644
index 00000000000..bc877d9e81b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+long
+eq1 (long a, long b)
+{
+  return (a == 0) ? b : 0;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
new file mode 100644
index 00000000000..28317613ba8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+long
+eq2 (long a, long b)
+{
+  if (a == 0)
+    return b;
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
new file mode 100644
index 00000000000..db7498801f9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+long long sink (long long);
+
+long long lt3 (long long a, long long b)
+{
+  if (a < b) 
+    b = 0;
+
+  return sink(b);
+}
+
+/* { dg-final { scan-assembler-times "vt.maskcn\" 1 } } */
+/* { dg-final { scan-assembler-times "slt\t" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
new file mode 100644
index 00000000000..eff1486828c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
+
+long long ne1(long long a, long long b)
+{
+  return (a != 0) ? b : 0;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
+
diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
new file mode 100644
index 00000000000..43020790a22
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
+
+long xor1(long crc, long poly)
+{
+  if (crc & 1)
+    crc ^= poly;
+
+  return crc;
+}
+
+/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
+/* { dg-final { scan-assembler-times "xor\t" 1 } } */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions
  2022-11-12 21:29 ` [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions Philipp Tomsich
@ 2022-11-12 21:47   ` Andrew Pinski
  2022-11-12 22:01     ` Philipp Tomsich
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Pinski @ 2022-11-12 21:47 UTC (permalink / raw)
  To: Philipp Tomsich
  Cc: gcc-patches, Vineet Gupta, Palmer Dabbelt, Christoph Muellner,
	Kito Cheng, Jeff Law

On Sat, Nov 12, 2022 at 1:34 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> Some architectures, as it the case on RISC-V with the proposed
> ZiCondOps and the vendor-defined XVentanaCondOps, define a
> conditional-zero instruction that is equivalent to:
>  - the positive form:  rd = (rc != 0) ? rs : 0
>  - the negated form:   rd = (rc == 0) ? rs : 0
>
> While noce_try_store_flag_mask will somewhat work for this case, it
> will generate a number of atomic RTX that will misdirect the cost
> calculation and may be too long (i.e., 4 RTX and more) to successfully
> merge at combine-time.
>
> Instead, we add two new transforms that attempt to build up what we
> define as the canonical form of a conditional-zero expression:
>
>   (set (match_operand 0 "register_operand" "=r")
>        (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
>                            (const_int 0)))
>             (match_operand 2 "register_operand" "r")))


Why is it not:
(set x (if_then_else (eq_or_ne y (0)) z (0))
(set x (if_then_else (ne y (0)) (0) z)

That seems simpler to expression and is the normal a==0?0:z expression.

Also all canonical forms of RTL should be documented too.
They are documented here:
https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gccint/Insn-Canonicalizations.html
https://gcc.gnu.org/onlinedocs/gccint/machine-descriptions/canonicalization-of-instructions.html
gcc/doc/gccint/machine-descriptions/canonicalization-of-instructions.rst


Thanks,
Andrew Pinski

>
> Architectures that provide a conditional-zero are thus expected to
> define an instruction matching this pattern in their backend.
>
> Based on this, we support the following cases:
>  - noce_try_condzero:
>       a ? a : b
>       a ? b : 0  (and then/else swapped)
>      !a ? b : 0  (and then/else swapped)
>  - noce_try_condzero_arith:
>      conditional-plus, conditional-minus, conditional-and,
>      conditional-or, conditional-xor, conditional-shift,
>      conditional-and
>
> Given that this is hooked into the CE passes, it is less powerful than
> a tree-pass (e.g., it can not transform cases where an extension, such
> as for uint16_t operations is in either the then or else-branch
> together with the arithmetic) but already covers a good array of cases
> and triggers across SPEC CPU 2017.
> Adding transofmrations in a tree pass will be considered as a future
> improvement.
>
> gcc/ChangeLog:
>
>         * ifcvt.cc (noce_emit_insn): Add prototype.
>         (noce_emit_condzero): Helper for noce_try_condzero and
>         noce_try_condzero_arith transforms.
>         (noce_try_condzero): New transform.
>         (noce_try_condzero_arith): New transform for conditional
>         arithmetic that can be built up by exploiting that the
>         conditional-zero instruction will inject 0, which acts
>         as the neutral element for operations.
>         (noce_process_if_block): Call noce_try_condzero and
>         noce_try_condzero_arith.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/xventanacondops-and-01.c: New test.
>         * gcc.target/riscv/xventanacondops-and-02.c: New test.
>         * gcc.target/riscv/xventanacondops-eq-01.c: New test.
>         * gcc.target/riscv/xventanacondops-eq-02.c: New test.
>         * gcc.target/riscv/xventanacondops-lt-01.c: New test.
>         * gcc.target/riscv/xventanacondops-ne-01.c: New test.
>         * gcc.target/riscv/xventanacondops-xor-01.c: New test.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
>  gcc/ifcvt.cc                                  | 214 ++++++++++++++++++
>  .../gcc.target/riscv/xventanacondops-and-01.c |  16 ++
>  .../gcc.target/riscv/xventanacondops-and-02.c |  15 ++
>  .../gcc.target/riscv/xventanacondops-eq-01.c  |  11 +
>  .../gcc.target/riscv/xventanacondops-eq-02.c  |  14 ++
>  .../gcc.target/riscv/xventanacondops-lt-01.c  |  16 ++
>  .../gcc.target/riscv/xventanacondops-ne-01.c  |  11 +
>  .../gcc.target/riscv/xventanacondops-xor-01.c |  14 ++
>  8 files changed, 311 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
>
> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
> index eb8efb89a89..41c58876d05 100644
> --- a/gcc/ifcvt.cc
> +++ b/gcc/ifcvt.cc
> @@ -97,6 +97,7 @@ static int find_if_case_2 (basic_block, edge, edge);
>  static int dead_or_predicable (basic_block, basic_block, basic_block,
>                                edge, int);
>  static void noce_emit_move_insn (rtx, rtx);
> +static rtx_insn *noce_emit_insn (rtx);
>  static rtx_insn *block_has_only_trap (basic_block);
>  static void need_cmov_or_rewire (basic_block, hash_set<rtx_insn *> *,
>                                  hash_map<rtx_insn *, int> *);
> @@ -787,6 +788,9 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **);
>  static int noce_try_minmax (struct noce_if_info *);
>  static int noce_try_abs (struct noce_if_info *);
>  static int noce_try_sign_mask (struct noce_if_info *);
> +static rtx noce_emit_condzero (struct noce_if_info *, rtx, bool = false);
> +static int noce_try_condzero (struct noce_if_info *);
> +static int noce_try_condzero_arith (struct noce_if_info *);
>
>  /* Return the comparison code for reversed condition for IF_INFO,
>     or UNKNOWN if reversing the condition is not possible.  */
> @@ -1664,6 +1668,212 @@ noce_try_addcc (struct noce_if_info *if_info)
>    return FALSE;
>  }
>
> +/* Helper to noce_try_condzero: cond ? a : 0. */
> +static rtx
> +noce_emit_condzero (struct noce_if_info *if_info, rtx a, bool reverse)
> +{
> +  /* The canonical form for a conditional-zero-or-value is:
> +       (set (match_operand 0 "register_operand" "=r")
> +           (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
> +                               (const_int 0)))
> +                (match_operand 2 "register_operand" "r")))
> +   */
> +
> +  machine_mode opmode = GET_MODE (if_info->x);
> +  enum rtx_code code = GET_CODE (if_info->cond);
> +  rtx cond;
> +  rtx op_a = XEXP (if_info->cond, 0);
> +  rtx op_b = XEXP (if_info->cond, 1);
> +
> +  /* If it is not a EQ/NE comparison against const0_rtx, canonicalize
> +     by first synthesizing a truth-value and then building a NE
> +     condition around it. */
> +  if ((code != EQ && code != NE) || XEXP (if_info->cond, 1) != const0_rtx)
> +    {
> +      rtx tmp = gen_reg_rtx (opmode);
> +
> +      start_sequence ();
> +      cond = gen_rtx_fmt_ee (code, opmode, op_a, op_b);
> +      if (!noce_emit_insn (gen_rtx_SET (tmp, cond)))
> +       {
> +         end_sequence ();
> +
> +         /* If we can't emit this pattern, try to reverse it and
> +            invert the polarity of the second test. */
> +         start_sequence ();
> +         cond = gen_rtx_fmt_ee (reverse_condition (code), opmode, op_a, op_b);
> +         if (!noce_emit_insn (gen_rtx_SET (tmp, cond))) {
> +           end_sequence ();
> +           return NULL_RTX;
> +         }
> +
> +         /* We have recovered by reversing the first comparison,
> +            so we need change the second one around as well... */
> +         reverse = !reverse;
> +       }
> +      rtx_insn *seq = get_insns ();
> +      end_sequence ();
> +      emit_insn (seq);
> +
> +      /* Set up the second comparison that will be embedded in the
> +        canonical conditional-zero-or-value RTX. */
> +      code = NE;
> +      op_a = tmp;
> +      op_b = const0_rtx;
> +    }
> +
> +  cond = gen_rtx_fmt_ee (reverse ? reverse_condition (code) : code,
> +                        opmode, op_a, op_b);
> +
> +  /* Build (and (neg (eq_or_ne ... const0_rtx)) (reg <a>)) */
> +  rtx target = gen_reg_rtx (opmode);
> +  rtx czero = gen_rtx_AND (opmode, gen_rtx_NEG (opmode, cond), a);
> +  noce_emit_move_insn (target, czero);
> +
> +  return target;
> +}
> +
> +/* Use a conditional-zero instruction for "if (test) x = 0;", if available. */
> +static int
> +noce_try_condzero (struct noce_if_info *if_info)
> +{
> +  rtx target;
> +  rtx_insn *seq;
> +  int reversep = 0;
> +  rtx orig_b = NULL_RTX;
> +  rtx cond = if_info->cond;
> +  enum rtx_code code = GET_CODE (cond);
> +  rtx cond_arg0 = XEXP (cond, 0);
> +  rtx cond_arg1 = XEXP (cond, 1);
> +
> +  if (!noce_simple_bbs (if_info))
> +    return FALSE;
> +
> +  /* We may encounter the form "(a != 0) ? a : b", which can be
> +     simplified to "a | ((a != 0) ? 0 : b)". */
> +  if (code == NE && cond_arg1 == const0_rtx &&
> +      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
> +    {
> +      orig_b = if_info->b;
> +      if_info->b = const0_rtx;
> +    }
> +
> +  /* We may encounter the form "(a != 0) ? b : a", which can be
> +     simplied to "(a != 0) ? b : 0". */
> +  if (code == EQ && cond_arg1 == const0_rtx &&
> +      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
> +    {
> +      /* We know that cond_arg0 is const_0, if the THEN branch is
> +        taken... so if it is the same as if_info->b (yes, things are
> +        backwards!), we can rewrite it with that knowledge.  */
> +      if_info->b = const0_rtx;
> +    }
> +
> +  start_sequence ();
> +
> +  if ((if_info->a == const0_rtx
> +       && (REG_P (if_info->b) || rtx_equal_p (if_info->b, if_info->x)))
> +      || ((reversep = (noce_reversed_cond_code (if_info) != UNKNOWN))
> +         && if_info->b == const0_rtx
> +         && (REG_P (if_info->a) || rtx_equal_p (if_info->a, if_info->x))))
> +    {
> +      target = noce_emit_condzero(if_info,
> +                                 reversep ? if_info->a : if_info->b,
> +                                 reversep);
> +
> +      if (orig_b && target)
> +       target = expand_simple_binop (GET_MODE (if_info->x), IOR, orig_b,
> +                                     target, if_info->x, 0, OPTAB_WIDEN);
> +
> +      if (target)
> +       {
> +         if (target != if_info->x)
> +           noce_emit_move_insn (if_info->x, target);
> +
> +         seq = end_ifcvt_sequence (if_info);
> +         if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
> +           return FALSE;
> +
> +         emit_insn_before_setloc (seq, if_info->jump,
> +                                  INSN_LOCATION (if_info->insn_a));
> +         if_info->transform_name = "noce_try_condzero";
> +
> +         return TRUE;
> +       }
> +    }
> +
> +  end_sequence ();
> +
> +  return FALSE;
> +}
> +
> +/* Convert "if (test) x op= a;" to a branchless sequence using the
> +   canonical form for a conditional-zero. */
> +static int
> +noce_try_condzero_arith (struct noce_if_info *if_info)
> +{
> +  rtx target;
> +  rtx_insn *seq;
> +  rtx_code op = GET_CODE (if_info->a);
> +  const rtx arg0 = XEXP (if_info->a, 0);
> +  const rtx arg1 = XEXP (if_info->a, 1);
> +
> +  if (!noce_simple_bbs (if_info))
> +    return FALSE;
> +
> +  /* Check for no else condition.  */
> +  if (!rtx_equal_p (if_info->x, if_info->b))
> +    return FALSE;
> +
> +  if (op != PLUS && op != MINUS && op != IOR && op != XOR &&
> +      op != ASHIFT && op != ASHIFTRT && op != LSHIFTRT && op != AND)
> +    return FALSE;
> +
> +  if (!rtx_equal_p (if_info->x, arg0))
> +    return FALSE;
> +
> +  start_sequence ();
> +
> +  target = noce_emit_condzero(if_info, arg1, op != AND ? true : false);
> +
> +  if (target)
> +    {
> +      rtx op1 = if_info->x;
> +
> +      if (op == AND)
> +       {
> +         /* Emit "tmp = x & val;" followed by "tmp |= !cond ? x : 0;" */
> +         op1 = expand_simple_binop (GET_MODE (if_info->x), AND, op1,
> +                                    arg1, NULL_RTX, 0, OPTAB_WIDEN);
> +         op = IOR;
> +       }
> +
> +      if (op1)
> +       target = expand_simple_binop (GET_MODE (if_info->x), op, op1,
> +                                     target, if_info->x, 0, OPTAB_WIDEN);
> +    }
> +
> +  if (target)
> +    {
> +      if (target != if_info->x)
> +       noce_emit_move_insn (if_info->x, target);
> +
> +      seq = end_ifcvt_sequence (if_info);
> +      if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
> +       return FALSE;
> +
> +      emit_insn_before_setloc(seq, if_info->jump,
> +                             INSN_LOCATION(if_info->insn_a));
> +      if_info->transform_name = "noce_try_condzero_arith";
> +
> +      return TRUE;
> +    }
> +
> +  end_sequence ();
> +
> +  return FALSE;
> +}
> +
>  /* Convert "if (test) x = 0;" to "x &= -(test == 0);"  */
>
>  static int
> @@ -3967,8 +4177,12 @@ noce_process_if_block (struct noce_if_info *if_info)
>      {
>        if (noce_try_addcc (if_info))
>         goto success;
> +      if (noce_try_condzero (if_info))
> +       goto success;
>        if (noce_try_store_flag_mask (if_info))
>         goto success;
> +      if (noce_try_condzero_arith (if_info))
> +       goto success;
>        if (HAVE_conditional_move
>           && noce_try_cmove_arith (if_info))
>         goto success;
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
> new file mode 100644
> index 00000000000..9b26cdf0513
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +
> +long and1(long a, long b, long c, long d)
> +{
> +  if (c < d)
> +    a &= b;
> +
> +  return a;
> +}
> +
> +/* { dg-final { scan-assembler-times "and\t" 1 } } */
> +/* { dg-final { scan-assembler-times "slt" 1 } } */
> +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> +/* { dg-final { scan-assembler-times "or\t" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
> new file mode 100644
> index 00000000000..66d2ec10211
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +
> +int and2(int a, int b, long c)
> +{
> +  if (c)
> +    a &= b;
> +
> +  return a;
> +}
> +
> +/* { dg-final { scan-assembler-times "and\t" 1 } } */
> +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> +/* { dg-final { scan-assembler-times "or\t" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
> new file mode 100644
> index 00000000000..bc877d9e81b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> +
> +long
> +eq1 (long a, long b)
> +{
> +  return (a == 0) ? b : 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
> new file mode 100644
> index 00000000000..28317613ba8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> +
> +long
> +eq2 (long a, long b)
> +{
> +  if (a == 0)
> +    return b;
> +
> +  return 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
> new file mode 100644
> index 00000000000..db7498801f9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +
> +long long sink (long long);
> +
> +long long lt3 (long long a, long long b)
> +{
> +  if (a < b)
> +    b = 0;
> +
> +  return sink(b);
> +}
> +
> +/* { dg-final { scan-assembler-times "vt.maskcn\" 1 } } */
> +/* { dg-final { scan-assembler-times "slt\t" 1 } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
> new file mode 100644
> index 00000000000..eff1486828c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> +
> +long long ne1(long long a, long long b)
> +{
> +  return (a != 0) ? b : 0;
> +}
> +
> +/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
> +
> diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
> new file mode 100644
> index 00000000000..43020790a22
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> +
> +long xor1(long crc, long poly)
> +{
> +  if (crc & 1)
> +    crc ^= poly;
> +
> +  return crc;
> +}
> +
> +/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
> +/* { dg-final { scan-assembler-times "xor\t" 1 } } */
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions
  2022-11-12 21:47   ` Andrew Pinski
@ 2022-11-12 22:01     ` Philipp Tomsich
  0 siblings, 0 replies; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-12 22:01 UTC (permalink / raw)
  To: Andrew Pinski
  Cc: gcc-patches, Vineet Gupta, Palmer Dabbelt, Christoph Muellner,
	Kito Cheng, Jeff Law

On Sat, 12 Nov 2022 at 22:47, Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sat, Nov 12, 2022 at 1:34 PM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > Some architectures, as it the case on RISC-V with the proposed
> > ZiCondOps and the vendor-defined XVentanaCondOps, define a
> > conditional-zero instruction that is equivalent to:
> >  - the positive form:  rd = (rc != 0) ? rs : 0
> >  - the negated form:   rd = (rc == 0) ? rs : 0
> >
> > While noce_try_store_flag_mask will somewhat work for this case, it
> > will generate a number of atomic RTX that will misdirect the cost
> > calculation and may be too long (i.e., 4 RTX and more) to successfully
> > merge at combine-time.
> >
> > Instead, we add two new transforms that attempt to build up what we
> > define as the canonical form of a conditional-zero expression:
> >
> >   (set (match_operand 0 "register_operand" "=r")
> >        (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
> >                            (const_int 0)))
> >             (match_operand 2 "register_operand" "r")))
>
>
> Why is it not:
> (set x (if_then_else (eq_or_ne y (0)) z (0))
> (set x (if_then_else (ne y (0)) (0) z)
>
> That seems simpler to expression and is the normal a==0?0:z expression.

Having an if_then_else come out of if-conversion would be a bit unusual, as
transformation to branchless is the intent of the entire exercise.

Existing if-conversion via noce_try_store_flag_mask and noce_try_store_flag
already catch these sequences—if that happens, the above representation
will be present during combine: i.e., we need to implement this match anyway
(and it also matches the typical idiom, if a programmer tries to express the
idiom in a branchless way).

Consequently, we decided to use the same pattern as the canonical
representation in case that if-conversion had occurred prior.

> Also all canonical forms of RTL should be documented too.
> They are documented here:
> https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gccint/Insn-Canonicalizations.html
> https://gcc.gnu.org/onlinedocs/gccint/machine-descriptions/canonicalization-of-instructions.html
> gcc/doc/gccint/machine-descriptions/canonicalization-of-instructions.rst
>
>
> Thanks,
> Andrew Pinski
>
> >
> > Architectures that provide a conditional-zero are thus expected to
> > define an instruction matching this pattern in their backend.
> >
> > Based on this, we support the following cases:
> >  - noce_try_condzero:
> >       a ? a : b
> >       a ? b : 0  (and then/else swapped)
> >      !a ? b : 0  (and then/else swapped)
> >  - noce_try_condzero_arith:
> >      conditional-plus, conditional-minus, conditional-and,
> >      conditional-or, conditional-xor, conditional-shift,
> >      conditional-and
> >
> > Given that this is hooked into the CE passes, it is less powerful than
> > a tree-pass (e.g., it can not transform cases where an extension, such
> > as for uint16_t operations is in either the then or else-branch
> > together with the arithmetic) but already covers a good array of cases
> > and triggers across SPEC CPU 2017.
> > Adding transofmrations in a tree pass will be considered as a future
> > improvement.
> >
> > gcc/ChangeLog:
> >
> >         * ifcvt.cc (noce_emit_insn): Add prototype.
> >         (noce_emit_condzero): Helper for noce_try_condzero and
> >         noce_try_condzero_arith transforms.
> >         (noce_try_condzero): New transform.
> >         (noce_try_condzero_arith): New transform for conditional
> >         arithmetic that can be built up by exploiting that the
> >         conditional-zero instruction will inject 0, which acts
> >         as the neutral element for operations.
> >         (noce_process_if_block): Call noce_try_condzero and
> >         noce_try_condzero_arith.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/xventanacondops-and-01.c: New test.
> >         * gcc.target/riscv/xventanacondops-and-02.c: New test.
> >         * gcc.target/riscv/xventanacondops-eq-01.c: New test.
> >         * gcc.target/riscv/xventanacondops-eq-02.c: New test.
> >         * gcc.target/riscv/xventanacondops-lt-01.c: New test.
> >         * gcc.target/riscv/xventanacondops-ne-01.c: New test.
> >         * gcc.target/riscv/xventanacondops-xor-01.c: New test.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > ---
> >
> >  gcc/ifcvt.cc                                  | 214 ++++++++++++++++++
> >  .../gcc.target/riscv/xventanacondops-and-01.c |  16 ++
> >  .../gcc.target/riscv/xventanacondops-and-02.c |  15 ++
> >  .../gcc.target/riscv/xventanacondops-eq-01.c  |  11 +
> >  .../gcc.target/riscv/xventanacondops-eq-02.c  |  14 ++
> >  .../gcc.target/riscv/xventanacondops-lt-01.c  |  16 ++
> >  .../gcc.target/riscv/xventanacondops-ne-01.c  |  11 +
> >  .../gcc.target/riscv/xventanacondops-xor-01.c |  14 ++
> >  8 files changed, 311 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
> >
> > diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
> > index eb8efb89a89..41c58876d05 100644
> > --- a/gcc/ifcvt.cc
> > +++ b/gcc/ifcvt.cc
> > @@ -97,6 +97,7 @@ static int find_if_case_2 (basic_block, edge, edge);
> >  static int dead_or_predicable (basic_block, basic_block, basic_block,
> >                                edge, int);
> >  static void noce_emit_move_insn (rtx, rtx);
> > +static rtx_insn *noce_emit_insn (rtx);
> >  static rtx_insn *block_has_only_trap (basic_block);
> >  static void need_cmov_or_rewire (basic_block, hash_set<rtx_insn *> *,
> >                                  hash_map<rtx_insn *, int> *);
> > @@ -787,6 +788,9 @@ static rtx noce_get_alt_condition (struct noce_if_info *, rtx, rtx_insn **);
> >  static int noce_try_minmax (struct noce_if_info *);
> >  static int noce_try_abs (struct noce_if_info *);
> >  static int noce_try_sign_mask (struct noce_if_info *);
> > +static rtx noce_emit_condzero (struct noce_if_info *, rtx, bool = false);
> > +static int noce_try_condzero (struct noce_if_info *);
> > +static int noce_try_condzero_arith (struct noce_if_info *);
> >
> >  /* Return the comparison code for reversed condition for IF_INFO,
> >     or UNKNOWN if reversing the condition is not possible.  */
> > @@ -1664,6 +1668,212 @@ noce_try_addcc (struct noce_if_info *if_info)
> >    return FALSE;
> >  }
> >
> > +/* Helper to noce_try_condzero: cond ? a : 0. */
> > +static rtx
> > +noce_emit_condzero (struct noce_if_info *if_info, rtx a, bool reverse)
> > +{
> > +  /* The canonical form for a conditional-zero-or-value is:
> > +       (set (match_operand 0 "register_operand" "=r")
> > +           (and (neg (eq_or_ne (match_operand 1 "register_operand" "r")
> > +                               (const_int 0)))
> > +                (match_operand 2 "register_operand" "r")))
> > +   */
> > +
> > +  machine_mode opmode = GET_MODE (if_info->x);
> > +  enum rtx_code code = GET_CODE (if_info->cond);
> > +  rtx cond;
> > +  rtx op_a = XEXP (if_info->cond, 0);
> > +  rtx op_b = XEXP (if_info->cond, 1);
> > +
> > +  /* If it is not a EQ/NE comparison against const0_rtx, canonicalize
> > +     by first synthesizing a truth-value and then building a NE
> > +     condition around it. */
> > +  if ((code != EQ && code != NE) || XEXP (if_info->cond, 1) != const0_rtx)
> > +    {
> > +      rtx tmp = gen_reg_rtx (opmode);
> > +
> > +      start_sequence ();
> > +      cond = gen_rtx_fmt_ee (code, opmode, op_a, op_b);
> > +      if (!noce_emit_insn (gen_rtx_SET (tmp, cond)))
> > +       {
> > +         end_sequence ();
> > +
> > +         /* If we can't emit this pattern, try to reverse it and
> > +            invert the polarity of the second test. */
> > +         start_sequence ();
> > +         cond = gen_rtx_fmt_ee (reverse_condition (code), opmode, op_a, op_b);
> > +         if (!noce_emit_insn (gen_rtx_SET (tmp, cond))) {
> > +           end_sequence ();
> > +           return NULL_RTX;
> > +         }
> > +
> > +         /* We have recovered by reversing the first comparison,
> > +            so we need change the second one around as well... */
> > +         reverse = !reverse;
> > +       }
> > +      rtx_insn *seq = get_insns ();
> > +      end_sequence ();
> > +      emit_insn (seq);
> > +
> > +      /* Set up the second comparison that will be embedded in the
> > +        canonical conditional-zero-or-value RTX. */
> > +      code = NE;
> > +      op_a = tmp;
> > +      op_b = const0_rtx;
> > +    }
> > +
> > +  cond = gen_rtx_fmt_ee (reverse ? reverse_condition (code) : code,
> > +                        opmode, op_a, op_b);
> > +
> > +  /* Build (and (neg (eq_or_ne ... const0_rtx)) (reg <a>)) */
> > +  rtx target = gen_reg_rtx (opmode);
> > +  rtx czero = gen_rtx_AND (opmode, gen_rtx_NEG (opmode, cond), a);
> > +  noce_emit_move_insn (target, czero);
> > +
> > +  return target;
> > +}
> > +
> > +/* Use a conditional-zero instruction for "if (test) x = 0;", if available. */
> > +static int
> > +noce_try_condzero (struct noce_if_info *if_info)
> > +{
> > +  rtx target;
> > +  rtx_insn *seq;
> > +  int reversep = 0;
> > +  rtx orig_b = NULL_RTX;
> > +  rtx cond = if_info->cond;
> > +  enum rtx_code code = GET_CODE (cond);
> > +  rtx cond_arg0 = XEXP (cond, 0);
> > +  rtx cond_arg1 = XEXP (cond, 1);
> > +
> > +  if (!noce_simple_bbs (if_info))
> > +    return FALSE;
> > +
> > +  /* We may encounter the form "(a != 0) ? a : b", which can be
> > +     simplified to "a | ((a != 0) ? 0 : b)". */
> > +  if (code == NE && cond_arg1 == const0_rtx &&
> > +      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
> > +    {
> > +      orig_b = if_info->b;
> > +      if_info->b = const0_rtx;
> > +    }
> > +
> > +  /* We may encounter the form "(a != 0) ? b : a", which can be
> > +     simplied to "(a != 0) ? b : 0". */
> > +  if (code == EQ && cond_arg1 == const0_rtx &&
> > +      REG_P (if_info->b) && rtx_equal_p (if_info->b, cond_arg0))
> > +    {
> > +      /* We know that cond_arg0 is const_0, if the THEN branch is
> > +        taken... so if it is the same as if_info->b (yes, things are
> > +        backwards!), we can rewrite it with that knowledge.  */
> > +      if_info->b = const0_rtx;
> > +    }
> > +
> > +  start_sequence ();
> > +
> > +  if ((if_info->a == const0_rtx
> > +       && (REG_P (if_info->b) || rtx_equal_p (if_info->b, if_info->x)))
> > +      || ((reversep = (noce_reversed_cond_code (if_info) != UNKNOWN))
> > +         && if_info->b == const0_rtx
> > +         && (REG_P (if_info->a) || rtx_equal_p (if_info->a, if_info->x))))
> > +    {
> > +      target = noce_emit_condzero(if_info,
> > +                                 reversep ? if_info->a : if_info->b,
> > +                                 reversep);
> > +
> > +      if (orig_b && target)
> > +       target = expand_simple_binop (GET_MODE (if_info->x), IOR, orig_b,
> > +                                     target, if_info->x, 0, OPTAB_WIDEN);
> > +
> > +      if (target)
> > +       {
> > +         if (target != if_info->x)
> > +           noce_emit_move_insn (if_info->x, target);
> > +
> > +         seq = end_ifcvt_sequence (if_info);
> > +         if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
> > +           return FALSE;
> > +
> > +         emit_insn_before_setloc (seq, if_info->jump,
> > +                                  INSN_LOCATION (if_info->insn_a));
> > +         if_info->transform_name = "noce_try_condzero";
> > +
> > +         return TRUE;
> > +       }
> > +    }
> > +
> > +  end_sequence ();
> > +
> > +  return FALSE;
> > +}
> > +
> > +/* Convert "if (test) x op= a;" to a branchless sequence using the
> > +   canonical form for a conditional-zero. */
> > +static int
> > +noce_try_condzero_arith (struct noce_if_info *if_info)
> > +{
> > +  rtx target;
> > +  rtx_insn *seq;
> > +  rtx_code op = GET_CODE (if_info->a);
> > +  const rtx arg0 = XEXP (if_info->a, 0);
> > +  const rtx arg1 = XEXP (if_info->a, 1);
> > +
> > +  if (!noce_simple_bbs (if_info))
> > +    return FALSE;
> > +
> > +  /* Check for no else condition.  */
> > +  if (!rtx_equal_p (if_info->x, if_info->b))
> > +    return FALSE;
> > +
> > +  if (op != PLUS && op != MINUS && op != IOR && op != XOR &&
> > +      op != ASHIFT && op != ASHIFTRT && op != LSHIFTRT && op != AND)
> > +    return FALSE;
> > +
> > +  if (!rtx_equal_p (if_info->x, arg0))
> > +    return FALSE;
> > +
> > +  start_sequence ();
> > +
> > +  target = noce_emit_condzero(if_info, arg1, op != AND ? true : false);
> > +
> > +  if (target)
> > +    {
> > +      rtx op1 = if_info->x;
> > +
> > +      if (op == AND)
> > +       {
> > +         /* Emit "tmp = x & val;" followed by "tmp |= !cond ? x : 0;" */
> > +         op1 = expand_simple_binop (GET_MODE (if_info->x), AND, op1,
> > +                                    arg1, NULL_RTX, 0, OPTAB_WIDEN);
> > +         op = IOR;
> > +       }
> > +
> > +      if (op1)
> > +       target = expand_simple_binop (GET_MODE (if_info->x), op, op1,
> > +                                     target, if_info->x, 0, OPTAB_WIDEN);
> > +    }
> > +
> > +  if (target)
> > +    {
> > +      if (target != if_info->x)
> > +       noce_emit_move_insn (if_info->x, target);
> > +
> > +      seq = end_ifcvt_sequence (if_info);
> > +      if (!seq || !targetm.noce_conversion_profitable_p (seq, if_info))
> > +       return FALSE;
> > +
> > +      emit_insn_before_setloc(seq, if_info->jump,
> > +                             INSN_LOCATION(if_info->insn_a));
> > +      if_info->transform_name = "noce_try_condzero_arith";
> > +
> > +      return TRUE;
> > +    }
> > +
> > +  end_sequence ();
> > +
> > +  return FALSE;
> > +}
> > +
> >  /* Convert "if (test) x = 0;" to "x &= -(test == 0);"  */
> >
> >  static int
> > @@ -3967,8 +4177,12 @@ noce_process_if_block (struct noce_if_info *if_info)
> >      {
> >        if (noce_try_addcc (if_info))
> >         goto success;
> > +      if (noce_try_condzero (if_info))
> > +       goto success;
> >        if (noce_try_store_flag_mask (if_info))
> >         goto success;
> > +      if (noce_try_condzero_arith (if_info))
> > +       goto success;
> >        if (HAVE_conditional_move
> >           && noce_try_cmove_arith (if_info))
> >         goto success;
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
> > new file mode 100644
> > index 00000000000..9b26cdf0513
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-01.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> > +
> > +long and1(long a, long b, long c, long d)
> > +{
> > +  if (c < d)
> > +    a &= b;
> > +
> > +  return a;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "and\t" 1 } } */
> > +/* { dg-final { scan-assembler-times "slt" 1 } } */
> > +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> > +/* { dg-final { scan-assembler-times "or\t" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
> > new file mode 100644
> > index 00000000000..66d2ec10211
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-and-02.c
> > @@ -0,0 +1,15 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> > +
> > +int and2(int a, int b, long c)
> > +{
> > +  if (c)
> > +    a &= b;
> > +
> > +  return a;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "and\t" 1 } } */
> > +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> > +/* { dg-final { scan-assembler-times "or\t" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
> > new file mode 100644
> > index 00000000000..bc877d9e81b
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-01.c
> > @@ -0,0 +1,11 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> > +
> > +long
> > +eq1 (long a, long b)
> > +{
> > +  return (a == 0) ? b : 0;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
> > new file mode 100644
> > index 00000000000..28317613ba8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-eq-02.c
> > @@ -0,0 +1,14 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> > +
> > +long
> > +eq2 (long a, long b)
> > +{
> > +  if (a == 0)
> > +    return b;
> > +
> > +  return 0;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vt.maskcn" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
> > new file mode 100644
> > index 00000000000..db7498801f9
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-lt-01.c
> > @@ -0,0 +1,16 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> > +
> > +long long sink (long long);
> > +
> > +long long lt3 (long long a, long long b)
> > +{
> > +  if (a < b)
> > +    b = 0;
> > +
> > +  return sink(b);
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vt.maskcn\" 1 } } */
> > +/* { dg-final { scan-assembler-times "slt\t" 1 } } */
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
> > new file mode 100644
> > index 00000000000..eff1486828c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-ne-01.c
> > @@ -0,0 +1,11 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" } } */
> > +
> > +long long ne1(long long a, long long b)
> > +{
> > +  return (a != 0) ? b : 0;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
> > +
> > diff --git a/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
> > new file mode 100644
> > index 00000000000..43020790a22
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xventanacondops-xor-01.c
> > @@ -0,0 +1,14 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gc_xventanacondops -mabi=lp64" } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-Og" "-Os" "-Oz" } } */
> > +
> > +long xor1(long crc, long poly)
> > +{
> > +  if (crc & 1)
> > +    crc ^= poly;
> > +
> > +  return crc;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "vt.maskc" 1 } } */
> > +/* { dg-final { scan-assembler-times "xor\t" 1 } } */
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/7] RISC-V: Recognize xventanacondops extension
  2022-11-12 21:29 ` [PATCH 1/7] RISC-V: Recognize xventanacondops extension Philipp Tomsich
@ 2022-11-17 22:46   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 22:46 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng, Jeff Law


On 11/12/22 14:29, Philipp Tomsich wrote:
> This adds the xventanacondops extension to the option parsing and as a
> default for the ventana-vt1 core:
>
> gcc/Changelog:
>
> 	* common/config/riscv/riscv-common.cc: Recognize
>            "xventanacondops" as part of an architecture string.
> 	* config/riscv/riscv-cores.def (RISCV_CORE): Enable
> 	  "xventanacondops" by default for "ventana-vt1".
> 	* config/riscv/riscv-opts.h (MASK_XVENTANACONDOPS): Define.
> 	(TARGET_XVENTANACONDOPS): Define.
> 	* config/riscv/riscv.opt: Add "riscv_xventanacondops".

OK once we've cleared the non-technical hurdles to committing vendor 
specific extensions.


Jeff



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion
  2022-11-12 21:29 ` [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion Philipp Tomsich
@ 2022-11-17 22:49   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 22:49 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng, Jeff Law


On 11/12/22 14:29, Philipp Tomsich wrote:
> Adds a pattern to map the output of noce_try_store_flag_mask
> if-conversion in the combiner onto vt.maskc<n>; the input patterns
> supported are similar to the following:
>    (set (reg/v/f:DI 75 [ <retval> ])
>         (and:DI (neg:DI (ne:DI (reg:DI 82)
>                         (const_int 0 [0])))
>                 (reg/v/f:DI 75 [ <retval> ])))
>
> This reduces dynamic instruction counts for the perlbench-workload in
> SPEC CPU2017 by 0.8230%, 0.4689%, and 0.2332% (respectively, for the
> each of the 3 workloads in the 'ref'-workload).
>
> To ensure that the combine-pass doesn't get confused about
> profitability, we recognize the idiom as requiring a single
> instruction when the XVentanaCondOps extension is present.
>
> gcc/ChangeLog:
>
> 	* config/riscv/riscv.cc (riscv_rtx_costs): Recognize idiom for
> 	  vt.maskc<n> as a single insn with TARGET_XVENTANACONDOPS.
> 	* config/riscv/riscv.md: Include xventanacondops.md.
> 	* config/riscv/xventanacondops.md: New file.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/riscv/xventanacondops-ne-03.c: New test.
> 	* gcc.target/riscv/xventanacondops-ne-04.c: New test.

OK once we've cleared the non-technical hurdles to committing vendor 
specific extensions.



Jeff



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n>
  2022-11-12 21:29 ` [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n> Philipp Tomsich
@ 2022-11-17 23:12   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 23:12 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng, Jeff Law


On 11/12/22 14:29, Philipp Tomsich wrote:
> When if-conversion in noce_try_store_flag_mask starts the sequence off
> with an order-operator, our patterns for vt.maskc<n> will receive the
> result of the order-operator as a register argument; consequently,
> they can't know that the result will be either 1 or 0.
>
> To convey this information (and make vt.maskc<n> applicable), we wrap
> the result of the order-operator in a eq/ne against (const_int 0).
> This commit adds the split pattern to handle these cases.
>
> gcc/ChangeLog:
>
> 	* config/riscv/xventanacondops.md: Add split to wrap an an
>            order-operator suitably for generating vt.maskc<n>.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> Ref vrull/gcc#157
>
> RISC-V: Recognize 'ge<u>'/'le<u>' operators as 'slt<u>'/'sgt<u>'
>
> During if-conversion, if noce_try_store_flag_mask succeeds, we may see
>      if (cur < next) {
>          next = 0;
>      }
> transformed into
>     27: r82:SI=ltu(r76:DI,r75:DI)
>        REG_DEAD r76:DI
>     28: r81:SI=r82:SI^0x1
>        REG_DEAD r82:SI
>     29: r80:DI=zero_extend(r81:SI)
>        REG_DEAD r81:SI
>
> This currently escapes the combiner, as RISC-V does not have a pattern
> to apply the 'slt' instruction to 'geu' verbs.  By adding a pattern in
> this commit, we match such cases.
>
> gcc/ChangeLog:
>
> 	* config/riscv/predicates.md (anyge_operator): Define.
> 	(anygt_operator): Define.
> 	(anyle_operator): Define.
> 	(anylt_operator): Define.
> 	* config/riscv/riscv.md (*sge<u>_<X:mode><GPR:mode>): Add a
> 	  pattern to map 'geu' onto slt w/ reversed operands.
> 	* config/riscv/riscv.md: Helpers for ge & le.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/riscv/xventanacondops-le-01.c: New test.
> 	* gcc.target/riscv/xventanacondops-lt-03.c: New test.

Presumably the two splitters in riscv.md can't live in 
xventanacondops.md due to ordering issues?

OK once we've cleared the non-technical hurdles to committing vendor 
specific extensions.


Jeff


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion
  2022-11-12 21:29 ` [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion Philipp Tomsich
@ 2022-11-17 23:17   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 23:17 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng, Jeff Law


On 11/12/22 14:29, Philipp Tomsich wrote:
> While the positive case "if ((bits >> SHAMT) & 1)" for SHAMT 0..10 can
> trigger conversion into efficient branchless sequences
>    - with Zbs (bexti + neg + and)
>    - with XVentanaCondOps (andi + vt.maskc)
> the inverted/negated case results in
>    andi a5,a0,1024
>    seqz a5,a5
>    neg a5,a5
>    and a5,a5,a1
> due to how the sequence presents to the combine pass.
>
> This adds an additional splitter to reassociate the polarity reversed
> case into bexti + addi, if Zbs is present.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> gcc/ChangeLog:
>
>      * config/riscv/xventanacondops.md: Add split to reassociate
>        "andi + seqz + neg" into "bexti + addi".

OK once we've cleared the non-technical hurdles to committing vendor 
specific extensions.


Seeing the number of re association splitters that have been submitted 
and the fact that there's been a fair amount of commonality makes me 
wonder if we should instead be beefing up the generic split point code 
in combine.  Though IIRC that code may be strictly splitting without 
reassociation or rewriting..  Hmm, anyway, worth keeping in mind that we 
have some generic code to try and find good points to break down a 
complex insn into components that might be recognizable.


jeff



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps
  2022-11-12 21:29 ` [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps Philipp Tomsich
@ 2022-11-17 23:36   ` Jeff Law
  0 siblings, 0 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 23:36 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng,
	Jeff Law, Henry Brausen


On 11/12/22 14:29, Philipp Tomsich wrote:
> When if-conversion encounters sequences using immediates, the
> sequences can't trivially map back onto vt.maskc/vt.maskcn (even if
> benefitial) due to vt.maskc and vt.maskcn not having immediate forms.
>
> This adds a splitter to rewrite opportunities for XVentanaCondOps that
> operate on an immediate by first putting the immediate into a register
> to enable the non-immediate vt.maskc/vt.maskcn instructions to operate
> on the value.
>
> Consider code, such as
>
>    long func2 (long a, long c)
>    {
>      if (c)
>        a = 2;
>      else
>        a = 5;
>      return a;
>    }
>
> which will be converted to
>
>    func2:
> 	seqz	a0,a2
> 	neg	a0,a0
> 	andi	a0,a0,3
> 	addi	a0,a0,2
> 	ret
>
> Following this change, we generate
>
> 	li	a0,3
> 	vt.maskcn	a0,a0,a2
> 	addi	a0,a0,2
> 	ret
>
> This commit also introduces a simple unit test for if-conversion with
> immediate (literal) values as the sources for simple sets in the THEN
> and ELSE blocks. The test checks that Ventana's conditional mask
> instruction (vt.maskc<n>) is emitted as part of the resultant branchless
> instruction sequence.
>
> gcc/ChangeLog:
>
> 	* config/riscv/xventanacondops.md: Support immediates for
> 	  vt.maskc/vt.maskcn through a splitter.
>
> gcc/testsuite/ChangeLog:
>
>          * gcc.target/riscv/xventanacondops-ifconv-imm.c: New test.

OK once we've cleared the non-technical hurdles to committing vendor 
specific extensions.


Jeff



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-12 21:29 ` [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps Philipp Tomsich
@ 2022-11-17 23:41   ` Jeff Law
  2022-11-17 23:56     ` Palmer Dabbelt
  2022-11-18  0:08     ` Philipp Tomsich
  0 siblings, 2 replies; 21+ messages in thread
From: Jeff Law @ 2022-11-17 23:41 UTC (permalink / raw)
  To: Philipp Tomsich, gcc-patches
  Cc: Vineet Gupta, Palmer Dabbelt, Christoph Muellner, Kito Cheng, Jeff Law


On 11/12/22 14:29, Philipp Tomsich wrote:
> Users might use explicit arithmetic operations to create a mask and
> then and it, in a sequence like
>      cond = (bits >> SHIFT) & 1;
>      mask = ~(cond - 1);
>      val &= mask;
> which will present as a single-bit sign-extract.
>
> Dependening on what combination of XVentanaCondOps and Zbs are
> available, this will map to the following sequences:
>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
>   - andi + vt.maskc, if only XVentanaCondOps is available and the
>                      sign-extract is operating on bits 10:0 (bit
> 		    11 can't be reached, as the immediate is
> 		    sign-extended)
>   - slli + srli + and, otherwise.
>
> gcc/ChangeLog:
>
> 	* config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
> 	  of a single-bit followed by AND for XVentanaCondOps.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
>   gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>
> diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
> index 7930ef1d837..3e9d5833a4b 100644
> --- a/gcc/config/riscv/xventanacondops.md
> +++ b/gcc/config/riscv/xventanacondops.md
> @@ -73,3 +73,49 @@
>     "TARGET_XVENTANACONDOPS"
>     [(set (match_dup 5) (match_dup 1))
>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
> +
> +;; Users might use explicit arithmetic operations to create a mask and
> +;; then and it, in a sequence like

Nit.  Seems like a word is missing.  "make and then and it"??


Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?


If there's a good reason to care about the !TARGET_ZBS case, then OK 
with the nit fixed.   If we agree that the !TARGET_ZBS case isn't all 
that important, then obviously OK with that pattern removed too.

I'm about out of oomph today.  I may take a look at 7/7 tonight though.  
Given it hits target independent code we probably want to get resolution 
on that patch sooner rather than later.

jeff


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-17 23:41   ` Jeff Law
@ 2022-11-17 23:56     ` Palmer Dabbelt
  2022-11-18  0:10       ` Philipp Tomsich
  2022-11-18 14:34       ` Jeff Law
  2022-11-18  0:08     ` Philipp Tomsich
  1 sibling, 2 replies; 21+ messages in thread
From: Palmer Dabbelt @ 2022-11-17 23:56 UTC (permalink / raw)
  To: gcc-patches
  Cc: philipp.tomsich, gcc-patches, Vineet Gupta, christoph.muellner,
	Kito Cheng, jlaw

On Thu, 17 Nov 2022 15:41:26 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>
> On 11/12/22 14:29, Philipp Tomsich wrote:
>> Users might use explicit arithmetic operations to create a mask and
>> then and it, in a sequence like
>>      cond = (bits >> SHIFT) & 1;
>>      mask = ~(cond - 1);
>>      val &= mask;
>> which will present as a single-bit sign-extract.
>>
>> Dependening on what combination of XVentanaCondOps and Zbs are
>> available, this will map to the following sequences:
>>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
>>   - andi + vt.maskc, if only XVentanaCondOps is available and the
>>                      sign-extract is operating on bits 10:0 (bit
>> 		    11 can't be reached, as the immediate is
>> 		    sign-extended)
>>   - slli + srli + and, otherwise.
>>
>> gcc/ChangeLog:
>>
>> 	* config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
>> 	  of a single-bit followed by AND for XVentanaCondOps.
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>> ---
>>
>>   gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
>> index 7930ef1d837..3e9d5833a4b 100644
>> --- a/gcc/config/riscv/xventanacondops.md
>> +++ b/gcc/config/riscv/xventanacondops.md
>> @@ -73,3 +73,49 @@
>>     "TARGET_XVENTANACONDOPS"
>>     [(set (match_dup 5) (match_dup 1))
>>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
>> +
>> +;; Users might use explicit arithmetic operations to create a mask and
>> +;; then and it, in a sequence like
>
> Nit.  Seems like a word is missing.  "make and then and it"??
>
>
> Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?

I guess that's really more of a question for the Ventana folks, but 
assuming all the Ventana widgets have Zbs then it seems reasonable to 
just couple them -- there's already enough options in RISC-V land to 
test everything, might as well make sure what slips through the cracks 
isn't being built.

Probably best to have a comment saying why here, and then something to 
enforce the dependency in -march (either as an implict extension 
dependency, or just a warning/error) so users don't get tripped up on 
configs that aren't expected to work.

> If there's a good reason to care about the !TARGET_ZBS case, then OK
> with the nit fixed.   If we agree that the !TARGET_ZBS case isn't all
> that important, then obviously OK with that pattern removed too.
>
> I'm about out of oomph today.  I may take a look at 7/7 tonight though. 
> Given it hits target independent code we probably want to get resolution
> on that patch sooner rather than later.

Thanks, there's no way we would have gotten this all sorted out so fast 
without the help!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-17 23:41   ` Jeff Law
  2022-11-17 23:56     ` Palmer Dabbelt
@ 2022-11-18  0:08     ` Philipp Tomsich
  1 sibling, 0 replies; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-18  0:08 UTC (permalink / raw)
  To: Jeff Law
  Cc: gcc-patches, Vineet Gupta, Palmer Dabbelt, Christoph Muellner,
	Kito Cheng, Jeff Law

On Fri, 18 Nov 2022 at 00:41, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/12/22 14:29, Philipp Tomsich wrote:
> > Users might use explicit arithmetic operations to create a mask and
> > then and it, in a sequence like
> >      cond = (bits >> SHIFT) & 1;
> >      mask = ~(cond - 1);
> >      val &= mask;
> > which will present as a single-bit sign-extract.
> >
> > Dependening on what combination of XVentanaCondOps and Zbs are
> > available, this will map to the following sequences:
> >   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
> >   - andi + vt.maskc, if only XVentanaCondOps is available and the
> >                      sign-extract is operating on bits 10:0 (bit
> >                   11 can't be reached, as the immediate is
> >                   sign-extended)
> >   - slli + srli + and, otherwise.
> >
> > gcc/ChangeLog:
> >
> >       * config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
> >         of a single-bit followed by AND for XVentanaCondOps.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > ---
> >
> >   gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
> >   1 file changed, 46 insertions(+)
> >
> > diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
> > index 7930ef1d837..3e9d5833a4b 100644
> > --- a/gcc/config/riscv/xventanacondops.md
> > +++ b/gcc/config/riscv/xventanacondops.md
> > @@ -73,3 +73,49 @@
> >     "TARGET_XVENTANACONDOPS"
> >     [(set (match_dup 5) (match_dup 1))
> >      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
> > +
> > +;; Users might use explicit arithmetic operations to create a mask and
> > +;; then and it, in a sequence like
>
> Nit.  Seems like a word is missing.  "make and then and it"??
>
>
> Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?

While Ventana might not plan to have this combination, nothing
prevents someone to implement only a single one of these — just as
users might choose to override the -march string.  Also note that (the
proposed) ZiCondOps will share most of its infrastructure with
XVentanaCondOps, we will have the same situation there.

> If there's a good reason to care about the !TARGET_ZBS case, then OK
> with the nit fixed.   If we agree that the !TARGET_ZBS case isn't all
> that important, then obviously OK with that pattern removed too.
>
> I'm about out of oomph today.  I may take a look at 7/7 tonight though.
> Given it hits target independent code we probably want to get resolution
> on that patch sooner rather than later.
>
> jeff
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-17 23:56     ` Palmer Dabbelt
@ 2022-11-18  0:10       ` Philipp Tomsich
  2022-11-18 14:34       ` Jeff Law
  1 sibling, 0 replies; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-18  0:10 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: gcc-patches, Vineet Gupta, christoph.muellner, Kito Cheng, jlaw

On Fri, 18 Nov 2022 at 00:56, Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Thu, 17 Nov 2022 15:41:26 PST (-0800), gcc-patches@gcc.gnu.org wrote:
> >
> > On 11/12/22 14:29, Philipp Tomsich wrote:
> >> Users might use explicit arithmetic operations to create a mask and
> >> then and it, in a sequence like
> >>      cond = (bits >> SHIFT) & 1;
> >>      mask = ~(cond - 1);
> >>      val &= mask;
> >> which will present as a single-bit sign-extract.
> >>
> >> Dependening on what combination of XVentanaCondOps and Zbs are
> >> available, this will map to the following sequences:
> >>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
> >>   - andi + vt.maskc, if only XVentanaCondOps is available and the
> >>                      sign-extract is operating on bits 10:0 (bit
> >>                  11 can't be reached, as the immediate is
> >>                  sign-extended)
> >>   - slli + srli + and, otherwise.
> >>
> >> gcc/ChangeLog:
> >>
> >>      * config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
> >>        of a single-bit followed by AND for XVentanaCondOps.
> >>
> >> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> >> ---
> >>
> >>   gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
> >>   1 file changed, 46 insertions(+)
> >>
> >> diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
> >> index 7930ef1d837..3e9d5833a4b 100644
> >> --- a/gcc/config/riscv/xventanacondops.md
> >> +++ b/gcc/config/riscv/xventanacondops.md
> >> @@ -73,3 +73,49 @@
> >>     "TARGET_XVENTANACONDOPS"
> >>     [(set (match_dup 5) (match_dup 1))
> >>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
> >> +
> >> +;; Users might use explicit arithmetic operations to create a mask and
> >> +;; then and it, in a sequence like
> >
> > Nit.  Seems like a word is missing.  "make and then and it"??
> >
> >
> > Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?
>
> I guess that's really more of a question for the Ventana folks, but
> assuming all the Ventana widgets have Zbs then it seems reasonable to
> just couple them -- there's already enough options in RISC-V land to
> test everything, might as well make sure what slips through the cracks
> isn't being built.
>
> Probably best to have a comment saying why here, and then something to
> enforce the dependency in -march (either as an implict extension
> dependency, or just a warning/error) so users don't get tripped up on
> configs that aren't expected to work.

With an eye to (the proposed) ZiCondOps, I'd rather pull this in once
XVentanaCondOps is applied.
That said, we'll need to add a test-case for these.

> > If there's a good reason to care about the !TARGET_ZBS case, then OK
> > with the nit fixed.   If we agree that the !TARGET_ZBS case isn't all
> > that important, then obviously OK with that pattern removed too.
> >
> > I'm about out of oomph today.  I may take a look at 7/7 tonight though.
> > Given it hits target independent code we probably want to get resolution
> > on that patch sooner rather than later.
>
> Thanks, there's no way we would have gotten this all sorted out so fast
> without the help!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-17 23:56     ` Palmer Dabbelt
  2022-11-18  0:10       ` Philipp Tomsich
@ 2022-11-18 14:34       ` Jeff Law
  2022-11-18 14:41         ` Philipp Tomsich
  1 sibling, 1 reply; 21+ messages in thread
From: Jeff Law @ 2022-11-18 14:34 UTC (permalink / raw)
  To: Palmer Dabbelt, gcc-patches
  Cc: philipp.tomsich, Vineet Gupta, christoph.muellner, Kito Cheng, jlaw


On 11/17/22 16:56, Palmer Dabbelt wrote:
> On Thu, 17 Nov 2022 15:41:26 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>>
>> On 11/12/22 14:29, Philipp Tomsich wrote:
>>> Users might use explicit arithmetic operations to create a mask and
>>> then and it, in a sequence like
>>>      cond = (bits >> SHIFT) & 1;
>>>      mask = ~(cond - 1);
>>>      val &= mask;
>>> which will present as a single-bit sign-extract.
>>>
>>> Dependening on what combination of XVentanaCondOps and Zbs are
>>> available, this will map to the following sequences:
>>>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
>>>   - andi + vt.maskc, if only XVentanaCondOps is available and the
>>>                      sign-extract is operating on bits 10:0 (bit
>>>             11 can't be reached, as the immediate is
>>>             sign-extended)
>>>   - slli + srli + and, otherwise.
>>>
>>> gcc/ChangeLog:
>>>
>>>     * config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
>>>       of a single-bit followed by AND for XVentanaCondOps.
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>>> ---
>>>
>>>   gcc/config/riscv/xventanacondops.md | 46 
>>> +++++++++++++++++++++++++++++
>>>   1 file changed, 46 insertions(+)
>>>
>>> diff --git a/gcc/config/riscv/xventanacondops.md 
>>> b/gcc/config/riscv/xventanacondops.md
>>> index 7930ef1d837..3e9d5833a4b 100644
>>> --- a/gcc/config/riscv/xventanacondops.md
>>> +++ b/gcc/config/riscv/xventanacondops.md
>>> @@ -73,3 +73,49 @@
>>>     "TARGET_XVENTANACONDOPS"
>>>     [(set (match_dup 5) (match_dup 1))
>>>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 
>>> 0)))
>>> +
>>> +;; Users might use explicit arithmetic operations to create a mask and
>>> +;; then and it, in a sequence like
>>
>> Nit.  Seems like a word is missing.  "make and then and it"??
>>
>>
>> Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?
>
> I guess that's really more of a question for the Ventana folks, but 
> assuming all the Ventana widgets have Zbs then it seems reasonable to 
> just couple them -- there's already enough options in RISC-V land to 
> test everything, might as well make sure what slips through the cracks 
> isn't being built.

I'm pretty confident Ventana won't be making a part without Zbs which is 
why I raised the issue


I also understand Philipp's position that one could explicitly turn on 
ventanacondops and zbs off and that there's a notable possibility that 
this ultimately turns into ZICondOps independent of Ventana.


So I guess we keep it...  But it also feels like a ticking time bomb WRT 
the ability to mix and match things the way we currently allow.  I 
suspect if we were to look at the full test matrix and deeply test that 
full matrix that we'd find a number of problems where two options 
interact badly.

Jeff

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps
  2022-11-18 14:34       ` Jeff Law
@ 2022-11-18 14:41         ` Philipp Tomsich
  0 siblings, 0 replies; 21+ messages in thread
From: Philipp Tomsich @ 2022-11-18 14:41 UTC (permalink / raw)
  To: Jeff Law
  Cc: Palmer Dabbelt, gcc-patches, Vineet Gupta, christoph.muellner,
	Kito Cheng, jlaw

On Fri, 18 Nov 2022 at 15:34, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
> On 11/17/22 16:56, Palmer Dabbelt wrote:
> > On Thu, 17 Nov 2022 15:41:26 PST (-0800), gcc-patches@gcc.gnu.org wrote:
> >>
> >> On 11/12/22 14:29, Philipp Tomsich wrote:
> >>> Users might use explicit arithmetic operations to create a mask and
> >>> then and it, in a sequence like
> >>>      cond = (bits >> SHIFT) & 1;
> >>>      mask = ~(cond - 1);
> >>>      val &= mask;
> >>> which will present as a single-bit sign-extract.
> >>>
> >>> Dependening on what combination of XVentanaCondOps and Zbs are
> >>> available, this will map to the following sequences:
> >>>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
> >>>   - andi + vt.maskc, if only XVentanaCondOps is available and the
> >>>                      sign-extract is operating on bits 10:0 (bit
> >>>             11 can't be reached, as the immediate is
> >>>             sign-extended)
> >>>   - slli + srli + and, otherwise.
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>>     * config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
> >>>       of a single-bit followed by AND for XVentanaCondOps.
> >>>
> >>> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> >>> ---
> >>>
> >>>   gcc/config/riscv/xventanacondops.md | 46
> >>> +++++++++++++++++++++++++++++
> >>>   1 file changed, 46 insertions(+)
> >>>
> >>> diff --git a/gcc/config/riscv/xventanacondops.md
> >>> b/gcc/config/riscv/xventanacondops.md
> >>> index 7930ef1d837..3e9d5833a4b 100644
> >>> --- a/gcc/config/riscv/xventanacondops.md
> >>> +++ b/gcc/config/riscv/xventanacondops.md
> >>> @@ -73,3 +73,49 @@
> >>>     "TARGET_XVENTANACONDOPS"
> >>>     [(set (match_dup 5) (match_dup 1))
> >>>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int
> >>> 0)))
> >>> +
> >>> +;; Users might use explicit arithmetic operations to create a mask and
> >>> +;; then and it, in a sequence like
> >>
> >> Nit.  Seems like a word is missing.  "make and then and it"??
> >>
> >>
> >> Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?
> >
> > I guess that's really more of a question for the Ventana folks, but
> > assuming all the Ventana widgets have Zbs then it seems reasonable to
> > just couple them -- there's already enough options in RISC-V land to
> > test everything, might as well make sure what slips through the cracks
> > isn't being built.
>
> I'm pretty confident Ventana won't be making a part without Zbs which is
> why I raised the issue
>
>
> I also understand Philipp's position that one could explicitly turn on
> ventanacondops and zbs off and that there's a notable possibility that
> this ultimately turns into ZICondOps independent of Ventana.
>
>
> So I guess we keep it...  But it also feels like a ticking time bomb WRT
> the ability to mix and match things the way we currently allow.  I
> suspect if we were to look at the full test matrix and deeply test that
> full matrix that we'd find a number of problems where two options
> interact badly.

I have been worrying about the exponential growth of the test matrix
for 2 years now and still haven't come up with a good solution. It is
clear that this is a challenge for the entire RISC-V ecosystem and
that it needs to be addressed across vendors and across the entire
membership: unfortunately, that doesn't make for an easier path to a
solution.

And just as an aside: pure extensions are still less worrisome than
subtractive changes (think Zfinx and Zdinx), or the fact that we have
different options for the memory model (RVWMO vs. Ztso), or variations
in regard to what facilities are available for atomics...


Philipp.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-11-18 14:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-12 21:29 [PATCH 0/7] RISC-V: Backend support for XVentanaCondOps/ZiCondops Philipp Tomsich
2022-11-12 21:29 ` [PATCH 1/7] RISC-V: Recognize xventanacondops extension Philipp Tomsich
2022-11-17 22:46   ` Jeff Law
2022-11-12 21:29 ` [PATCH 2/7] RISC-V: Generate vt.maskc<n> on noce_try_store_flag_mask if-conversion Philipp Tomsich
2022-11-17 22:49   ` Jeff Law
2022-11-12 21:29 ` [PATCH 3/7] RISC-V: Support noce_try_store_flag_mask as vt.maskc<n> Philipp Tomsich
2022-11-17 23:12   ` Jeff Law
2022-11-12 21:29 ` [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps Philipp Tomsich
2022-11-17 23:41   ` Jeff Law
2022-11-17 23:56     ` Palmer Dabbelt
2022-11-18  0:10       ` Philipp Tomsich
2022-11-18 14:34       ` Jeff Law
2022-11-18 14:41         ` Philipp Tomsich
2022-11-18  0:08     ` Philipp Tomsich
2022-11-12 21:29 ` [PATCH 5/7] RISC-V: Recognize bexti in negated if-conversion Philipp Tomsich
2022-11-17 23:17   ` Jeff Law
2022-11-12 21:29 ` [PATCH 6/7] RISC-V: Support immediates in XVentanaCondOps Philipp Tomsich
2022-11-17 23:36   ` Jeff Law
2022-11-12 21:29 ` [PATCH 7/7] ifcvt: add if-conversion to conditional-zero instructions Philipp Tomsich
2022-11-12 21:47   ` Andrew Pinski
2022-11-12 22:01     ` Philipp Tomsich

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).