public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-6804] PR middle-end/104140: bootstrap ICE on riscv.
@ 2022-01-21 18:50 Roger Sayle
  0 siblings, 0 replies; only message in thread
From: Roger Sayle @ 2022-01-21 18:50 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:886e9779581102caf97cd05dea80d9be87c24784

commit r12-6804-g886e9779581102caf97cd05dea80d9be87c24784
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Fri Jan 21 18:42:30 2022 +0000

    PR middle-end/104140: bootstrap ICE on riscv.
    
    This patch resolves the P1 "ice-on-valid-code" regression boostrapping
    GCC on risv-unknown-linux-gnu caused by my recent MULT_HIGHPART_EXPR
    functionality.  RISC-V differs from x86_64 and many targets by
    supporting a usmusidi3 instruction, basically a widening multiply
    where one operand is signed and the other is unsigned.  Alas the
    final version of my patch to recognize MULT_HIGHPART_EXPR didn't
    sufficiently defend against the operands of WIDEN_MULT_EXPR having
    different signedness.  This is fixed by the two-line change to
    tree-ssa-math-opts.cc's convert_mult_to_highpart in the patch below.
    
    The majority of the rest of the patch is to the documentation
    (in tree.def and generic.texi).  It turns out that WIDEN_MULT_EXPR
    wasn't previously documented in generic.texi, let alone the slightly
    unusual semantics of allowing mismatched (signed vs unsigned) operands.
    This also clarifies that MULT_HIGHPART_EXPR currently requires the
    signedness of operands to match [but this might change in a future
    release of GCC to support targets with usmul<mode>3_highpart].
    
    The one final chunk of this patch (that is hopefully sufficiently
    close to obvious for stage 4) is a similar (NULL pointer) sanity
    check in riscv_cpu_cpp_builtins.  Currently running cc1 from the
    command line (or from gdb) without specifying -march results in a
    segmentation fault (ICE).  This is a minor annoyance tracking down
    issues (in cross compilers) for riscv, and trivially fixed as below.
    
    2022-01-22  Roger Sayle  <roger@nextmovesoftware.com>
    
    gcc/ChangeLog
            PR middle-end/104140
            * tree-ssa-math-opts.cc (convert_mult_to_highpart): Check that the
            operands of the widening multiplication are either both signed or
            both unsigned, and abort the conversion if mismatched.
            * doc/generic.texi (WIDEN_MULT_EXPR): Describe expression node.
            (MULT_HIGHPART_EXPR): Clarify that operands must have the same
            signedness.
            * tree.def (MULT_HIGHPART_EXPR): Document both operands must have
            integer types with the same precision and signedness.
            (WIDEN_MULT_EXPR): Document that operands must have integer types
            with the same precision, but possibly differing signedness.
            * config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins): Defend against
            riscv_current_subset_list returning a NULL pointer (empty list).
    
    gcc/testsuite/ChangeLog
            PR middle-end/104140
            * gcc.target/riscv/pr104140.c: New test case.

Diff:
---
 gcc/config/riscv/riscv-c.cc               |  3 +++
 gcc/doc/generic.texi                      |  9 +++++++++
 gcc/testsuite/gcc.target/riscv/pr104140.c | 11 +++++++++++
 gcc/tree-ssa-math-opts.cc                 |  4 ++++
 gcc/tree.def                              | 14 +++++++++-----
 5 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
index 211472fdcad..73c62f41274 100644
--- a/gcc/config/riscv/riscv-c.cc
+++ b/gcc/config/riscv/riscv-c.cc
@@ -108,6 +108,9 @@ riscv_cpu_cpp_builtins (cpp_reader *pfile)
   builtin_define_with_int_value ("__riscv_arch_test", 1);
 
   const riscv_subset_list *subset_list = riscv_current_subset_list ();
+  if (!subset_list)
+    return;
+
   size_t max_ext_len = 0;
 
   /* Figure out the max length of extension name for reserving buffer.   */
diff --git a/gcc/doc/generic.texi b/gcc/doc/generic.texi
index 3e5b06a5dc7..e5f9d1be8ea 100644
--- a/gcc/doc/generic.texi
+++ b/gcc/doc/generic.texi
@@ -1318,6 +1318,7 @@ The type of the node specifies the alignment of the access.
 @tindex PLUS_EXPR
 @tindex MINUS_EXPR
 @tindex MULT_EXPR
+@tindex WIDEN_MULT_EXPR
 @tindex MULT_HIGHPART_EXPR
 @tindex RDIV_EXPR
 @tindex TRUNC_DIV_EXPR
@@ -1532,10 +1533,18 @@ one operand is of floating type and the other is of integral type.
 The behavior of these operations on signed arithmetic overflow is
 controlled by the @code{flag_wrapv} and @code{flag_trapv} variables.
 
+@item WIDEN_MULT_EXPR
+This node represents a widening multiplication.  The operands have
+integral types with same @var{b} bits of precision, producing an
+integral type result with at least @math{2@var{b}} bits of precision.
+The behaviour is equivalent to extending both operands, possibly of
+different signedness, to the result type, then multiplying them.
+
 @item MULT_HIGHPART_EXPR
 This node represents the ``high-part'' of a widening multiplication.
 For an integral type with @var{b} bits of precision, the result is
 the most significant @var{b} bits of the full @math{2@var{b}} product.
+Both operands must have the same precision and same signedness.
 
 @item RDIV_EXPR
 This node represents a floating point division operation.
diff --git a/gcc/testsuite/gcc.target/riscv/pr104140.c b/gcc/testsuite/gcc.target/riscv/pr104140.c
new file mode 100644
index 00000000000..648e131109f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr104140.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv32im -mabi=ilp32" } */
+int x;
+unsigned u, v;
+void f (void)
+{
+  long long y = x;
+  u = y * v >> 32;
+}
+void g (void) { f (); }
+
diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc
index 3d7104aac00..ac277756d24 100644
--- a/gcc/tree-ssa-math-opts.cc
+++ b/gcc/tree-ssa-math-opts.cc
@@ -4608,6 +4608,10 @@ convert_mult_to_highpart (gassign *stmt, gimple_stmt_iterator *gsi)
   if (bits < prec || bits >= 2 * prec)
     return false;
 
+  /* For the time being, require operands to have the same sign.  */
+  if (unsignedp != TYPE_UNSIGNED (TREE_TYPE (mop2)))
+    return false;
+
   machine_mode mode = TYPE_MODE (optype);
   optab tab = unsignedp ? umul_highpart_optab : smul_highpart_optab;
   if (optab_handler (tab, mode) == CODE_FOR_nothing)
diff --git a/gcc/tree.def b/gcc/tree.def
index bd43dbcf515..62650b6934b 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -700,7 +700,9 @@ DEFTREECODE (POINTER_PLUS_EXPR, "pointer_plus_expr", tcc_binary, 2)
 DEFTREECODE (POINTER_DIFF_EXPR, "pointer_diff_expr", tcc_binary, 2)
 
 /* Highpart multiplication.  For an integral type with precision B,
-   returns bits [2B-1, B] of the full 2*B product.  */
+   returns bits [2B-1, B] of the full 2*B product.  Both operands
+   and the result should have integer types of the same precision
+   and signedness.  */
 DEFTREECODE (MULT_HIGHPART_EXPR, "mult_highpart_expr", tcc_binary, 2)
 
 /* Division for integer result that rounds the quotient toward zero.  */
@@ -1349,10 +1351,12 @@ DEFTREECODE (WIDEN_SUM_EXPR, "widen_sum_expr", tcc_binary, 2)
 DEFTREECODE (SAD_EXPR, "sad_expr", tcc_expression, 3)
 
 /* Widening multiplication.
-   The two arguments are of type t1.
-   The result is of type t2, such that t2 is at least twice
-   the size of t1. WIDEN_MULT_EXPR is equivalent to first widening (promoting)
-   the arguments from type t1 to type t2, and then multiplying them.  */
+   The two arguments are of type t1 and t2, both integral types that
+   have the same precision, but possibly different signedness.
+   The result is of integral type t3, such that t3 is at least twice
+   the size of t1/t2. WIDEN_MULT_EXPR is equivalent to first widening
+   (promoting) the arguments from type t1 to type t3, and from t2 to
+   type t3 and then multiplying them.  */
 DEFTREECODE (WIDEN_MULT_EXPR, "widen_mult_expr", tcc_binary, 2)
 
 /* Widening multiply-accumulate.


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-01-21 18:50 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 18:50 [gcc r12-6804] PR middle-end/104140: bootstrap ICE on riscv Roger Sayle

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