Re: [PATCH] PR middle-end/104140: bootstrap ICE on riscv.

[Richard Biener via Gcc-patches]

> Am 21.01.2022 um 15:59 schrieb Roger Sayle <ro...@nextmovesoftware.com>:
> 
> 
> 
> 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.c'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 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.
> 
> 
> 
> 
> 
> This patch has been tested both on x86_64-pc-linux-gnu with a full
> 
> make bootstrap and make -k check, and on a cross-compiler to
> 
> riscv-unknown-linux-gnu where I was able to confirm the new test
> 
> case now passes.  Ok for mainline?

Ok.
Thanks,
Richard 

> 
> 
> 
> 
> 2022-01-22  Roger Sayle  <ro...@nextmovesoftware.com>
> 
> 
> 
> gcc/ChangeLog
> 
> * tree-ssa-math-opts.c (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/risc-v.c (riscv_cpu_cpp_builtins): Defend against
> 
> riscv_current_subset_list returning a NULL pointer (empty list).
> 
> 
> 
> gcc/testsuite/ChangeLog
> 
> * gcc.target/riscv/pr104140.c: New test case.
> 
> 
> 
> 
> 
> Thanks in advance (and sorry for the inconvenience).
> 
> Roger
> 
> --
> 
> 
> 
> 
> 
> <patch.txt>