| Message ID | 20240924102748.625307-1-pan2.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] Widening-Mul: Fix one ICE for SAT_SUB matching operand checking | expand |
On Tue, Sep 24, 2024 at 12:29 PM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@intel.com> > > This patch would like to fix the following ICE for -O2 -m32 of x86_64. > > during RTL pass: expand > JackMidiAsyncWaitQueue.cpp.cpp: In function 'void DequeueEvent(unsigned > int)': > JackMidiAsyncWaitQueue.cpp.cpp:3:6: internal compiler error: in > expand_fn_using_insn, at internal-fn.cc:263 > 3 | void DequeueEvent(unsigned frame) { > | ^~~~~~~~~~~~ > 0x27b580d diagnostic_context::diagnostic_impl(rich_location*, > diagnostic_metadata const*, diagnostic_option_id, char const*, > __va_list_tag (*) [1], diagnostic_t) > ???:0 > 0x27c4a3f internal_error(char const*, ...) > ???:0 > 0x27b3994 fancy_abort(char const*, int, char const*) > ???:0 > 0xf25ae5 expand_fn_using_insn(gcall*, insn_code, unsigned int, unsigned int) > ???:0 > 0xf2a124 expand_direct_optab_fn(internal_fn, gcall*, optab_tag, unsigned int) > ???:0 > 0xf2c87c expand_SAT_SUB(internal_fn, gcall*) > ???:0 > > We allowed the operand convert when matching SAT_SUB in match.pd, to support > the zip benchmark SAT_SUB pattern. Aka, > > (convert? (minus (convert1? @0) (convert1? @1))) for below sample code. > > void test (uint16_t *x, unsigned b, unsigned n) > { > unsigned a = 0; > register uint16_t *p = x; > > do { > a = *--p; > *p = (uint16_t)(a >= b ? a - b : 0); // Truncate after .SAT_SUB > } while (--n); > } > > The pattern match for SAT_SUB itself may also act on below scalar sample > code too. > > unsigned long long GetTimeFromFrames(int); > unsigned long long GetMicroSeconds(); > > void DequeueEvent(unsigned frame) { > long long frame_time = GetTimeFromFrames(frame); > unsigned long long current_time = GetMicroSeconds(); > DequeueEvent(frame_time < current_time ? 0 : frame_time - current_time); > } > > Aka: > > uint32_t a = (uint32_t)SAT_SUB(uint64_t, uint64_t); > > Then there will be a problem when ia32 or -m32 is given when compiling. > Because we only check the lhs (aka uint32_t) type is supported by ifn > instead of the operand (aka uint64_t). Mostly DImode is disabled for > 32 bits target like ia32 or rv32gcv, and then trigger ICE when expanding. OK. Thanks, Richard. > The below test suites are passed for this patch. > * The rv64gcv fully regression test. > * The x86 bootstrap test. > * The x86 fully regression test. > > PR middle-end/116814 > > gcc/ChangeLog: > > * tree-ssa-math-opts.cc (build_saturation_binary_arith_call): Make > ifn is_supported type check based on operand instead of lhs. > > gcc/testsuite/ChangeLog: > > * g++.dg/torture/pr116814-1.C: New test. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/testsuite/g++.dg/torture/pr116814-1.C | 12 ++++++++++++ > gcc/tree-ssa-math-opts.cc | 2 +- > 2 files changed, 13 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/torture/pr116814-1.C > > diff --git a/gcc/testsuite/g++.dg/torture/pr116814-1.C b/gcc/testsuite/g++.dg/torture/pr116814-1.C > new file mode 100644 > index 00000000000..dd6f29daa7c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/torture/pr116814-1.C > @@ -0,0 +1,12 @@ > +/* { dg-do compile { target { ia32 } } } */ > +/* { dg-options "-O2" } */ > + > +unsigned long long GetTimeFromFrames(int); > +unsigned long long GetMicroSeconds(); > + > +void DequeueEvent(unsigned frame) { > + long long frame_time = GetTimeFromFrames(frame); > + unsigned long long current_time = GetMicroSeconds(); > + > + DequeueEvent(frame_time < current_time ? 0 : frame_time - current_time); > +} > diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc > index d61668aacfc..8c622514dbd 100644 > --- a/gcc/tree-ssa-math-opts.cc > +++ b/gcc/tree-ssa-math-opts.cc > @@ -4042,7 +4042,7 @@ build_saturation_binary_arith_call (gimple_stmt_iterator *gsi, gphi *phi, > internal_fn fn, tree lhs, tree op_0, > tree op_1) > { > - if (direct_internal_fn_supported_p (fn, TREE_TYPE (lhs), OPTIMIZE_FOR_BOTH)) > + if (direct_internal_fn_supported_p (fn, TREE_TYPE (op_0), OPTIMIZE_FOR_BOTH)) > { > gcall *call = gimple_build_call_internal (fn, 2, op_0, op_1); > gimple_call_set_lhs (call, lhs); > -- > 2.43.0 >
diff --git a/gcc/testsuite/g++.dg/torture/pr116814-1.C b/gcc/testsuite/g++.dg/torture/pr116814-1.C new file mode 100644 index 00000000000..dd6f29daa7c --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr116814-1.C @@ -0,0 +1,12 @@ +/* { dg-do compile { target { ia32 } } } */ +/* { dg-options "-O2" } */ + +unsigned long long GetTimeFromFrames(int); +unsigned long long GetMicroSeconds(); + +void DequeueEvent(unsigned frame) { + long long frame_time = GetTimeFromFrames(frame); + unsigned long long current_time = GetMicroSeconds(); + + DequeueEvent(frame_time < current_time ? 0 : frame_time - current_time); +} diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc index d61668aacfc..8c622514dbd 100644 --- a/gcc/tree-ssa-math-opts.cc +++ b/gcc/tree-ssa-math-opts.cc @@ -4042,7 +4042,7 @@ build_saturation_binary_arith_call (gimple_stmt_iterator *gsi, gphi *phi, internal_fn fn, tree lhs, tree op_0, tree op_1) { - if (direct_internal_fn_supported_p (fn, TREE_TYPE (lhs), OPTIMIZE_FOR_BOTH)) + if (direct_internal_fn_supported_p (fn, TREE_TYPE (op_0), OPTIMIZE_FOR_BOTH)) { gcall *call = gimple_build_call_internal (fn, 2, op_0, op_1); gimple_call_set_lhs (call, lhs);