[Bug target/106180] New: [13 Regression] ICE in extract_insn, at recog.cc:2791 since 73f942c08deef3cce312263b3347f7a44dd44150

[Bug target/106180] New: [13 Regression] ICE in extract_insn, at recog.cc:2791 since 73f942c08deef3cce312263b3347f7a44dd44150

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

            Bug ID: 106180
           Summary: [13 Regression] ICE in extract_insn, at recog.cc:2791
                    since 73f942c08deef3cce312263b3347f7a44dd44150
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: guihaoc at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-linux-gnu

The following ICEs:

$ cat az.ii
struct PointT {
  double x, y;
};
using PointF = PointT;
template <int _Nm> struct __array_traits { typedef PointT _Type[_Nm]; };
template <int _Nm> struct array {
  typename __array_traits<_Nm>::_Type _M_elems;
};
float SampleGrid_low, SampleGrid_high;
using QuadrilateralF = array<4>;
struct PerspectiveTransform {
  PerspectiveTransform(QuadrilateralF, QuadrilateralF);
};
void SampleGrid() {
  PerspectiveTransform{{PointF{SampleGrid_high, SampleGrid_low},
SampleGrid_low,
                        SampleGrid_high},
                       {}};
}

$ g++ az.ii -O3 -c -ffloat-store
az.ii: In function ‘void SampleGrid()’:
az.ii:18:1: error: unrecognizable insn:
   18 | }
      | ^
(insn 20 19 21 2 (set (reg:V2DF 92)
        (float_extend:V2DF (vec_select:V2SF (mem:V4SF (plus:DI (reg/f:DI 77
virtual-stack-vars)
                        (const_int -176 [0xffffffffffffff50])) [4  S16 A128])
                (parallel [
                        (const_int 0 [0])
                        (const_int 1 [0x1])
                    ])))) "az.ii":15:3 -1
     (nil))
during RTL pass: vregs
az.ii:18:1: internal compiler error: in extract_insn, at recog.cc:2791
0x1807b59 _fatal_insn(char const*, rtx_def const*, char const*, int, char
const*)
        /home/marxin/Programming/gcc/gcc/rtl-error.cc:108
0x1807ba3 _fatal_insn_not_found(rtx_def const*, char const*, int, char const*)
        /home/marxin/Programming/gcc/gcc/rtl-error.cc:116
0x17a5e2c extract_insn(rtx_insn*)
        /home/marxin/Programming/gcc/gcc/recog.cc:2791
0x137b1dc instantiate_virtual_regs_in_insn
        /home/marxin/Programming/gcc/gcc/function.cc:1611
0x137ca6f instantiate_virtual_regs
        /home/marxin/Programming/gcc/gcc/function.cc:1985
0x137cb58 execute
        /home/marxin/Programming/gcc/gcc/function.cc:2034
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13 Regression] ICE in      |[13 Regression] ICE in
                   |extract_insn, at            |extract_insn, at
                   |recog.cc:2791 since         |recog.cc:2791 since
                   |73f942c08deef3cce312263b334 |r13-1418-g73f942c08deef3
                   |7f7a44dd44150               |
                 CC|                            |uros at gcc dot gnu.org
   Last reconfirmed|                            |2022-07-04
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The ICE is because
(define_expand "vec_unpacks_lo_v4sf"
  [(set (match_operand:V2DF 0 "register_operand")
        (float_extend:V2DF
          (vec_select:V2SF
            (match_operand:V4SF 1 "vector_operand")
            (parallel [(const_int 0) (const_int 1)]))))]
  "TARGET_SSE2")
used to match the sse2_cvtps2pd both when operands[1] was REG or MEM, but now
it doesn't.
I think the r13-1418 change was just wrong.  It is fine to add a pattern with
V2SF input rather than vec_select of first half of V4SF input, but I don't
understand why you need to restrict one to memory_operand and the other to
register_operand, why vector_operand "vm" can't be used for both.
Not doing that ties hands of the register allocator, if something is memory
during expansion, it would be always in memory, if something isn't memory, it
couldn't ever be memory.
Is your concern not getting a SIGSEGV if first 2 SF elts are at the end of a
page and 2 further SF elts are in a non-mapped page?

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-*
   Target Milestone|---                         |13.0

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[wwwhhhyyy333 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

Hongyu Wang <wwwhhhyyy333 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wwwhhhyyy333 at gmail dot com

--- Comment #2 from Hongyu Wang <wwwhhhyyy333 at gmail dot com> ---
(In reply to Jakub Jelinek from comment #1)

> I think the r13-1418 change was just wrong.  It is fine to add a pattern
> with V2SF input rather than vec_select of first half of V4SF input, but I
> don't understand why you need to restrict one to memory_operand and the
> other to register_operand, why vector_operand "vm" can't be used for both.
> Not doing that ties hands of the register allocator, if something is memory
> during expansion, it would be always in memory, if something isn't memory,
> it couldn't ever be memory.
> Is your concern not getting a SIGSEGV if first 2 SF elts are at the end of a
> page and 2 further SF elts are in a non-mapped page?

The instruction cvtps2pd takes m64 as memory input, so the original pattern is
not proper since it allows V4SF memory input, although the generated code may
work since for unpack_lo the address is same. The cross-page issue is one of
the potential problem we can meet.

For this pattern, I think we can add

if (MEM_P (operands[1]))
  operands[1] = gen_lowpart (V2SFmode, operands[1])

There are many other unpacks_low expanders allowing memory input, but they
directly falls to cvt instructions. We plan to fix all them recently.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That doesn't really matter, many instructions just use the g, q etc. modifiers
so that the correct Intel syntax is written, the RTL can have larger MEM just
fine, the only thing that is wrong is e.g. if the expander has V2SF operand to
widen it  to V4SF using a subreg.  But if you match an instruction that had
V4SF operand, the fact that you actually just read 8 bytes from it rather than
16 isn't that bad.  Especially for the unpacks which are paired, one hi and one
lo which together read the whole memory anyway.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[haochen.jiang at intel dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #4 from Haochen Jiang <haochen.jiang at intel dot com> ---
Created attachment 53261
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53261&action=edit
This patch aims to handle memory issue when unpacking in cvtps2pd

I am trying to solve this ICE problem with this patch in this case and
regtested on x86_64-pc-linux-gnu.
I believe that  this is logically correct. Although it seems a little bit
complicated than previous pattern.
BTW, I plan to fix all that pattern if this patch is ok.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

David Binderman <dcb314 at hotmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dcb314 at hotmail dot com

--- Comment #5 from David Binderman <dcb314 at hotmail dot com> ---
I have something similar:

struct PointT {
  double y;
};
template <int _Nm> struct __array_traits { using _Type = PointT[_Nm]; };
template <int _Nm> struct array {
  typename __array_traits<_Nm>::_Type _M_elems;
};
float SampleGrid_low, SampleGrid_high;
using QuadrilateralF = array<4>;
struct PerspectiveTransform {
  PerspectiveTransform(QuadrilateralF, QuadrilateralF);
};
void SampleGrid() {
  PerspectiveTransform{
      {SampleGrid_high, SampleGrid_low, SampleGrid_high, SampleGrid_high}, {}};
}

which won't compile on x86_64 with flags -O2 -ffloat-store -std=c++17.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
Comment on attachment 53261
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53261
This patch aims to handle memory issue when unpacking in cvtps2pd

>@@ -9270,7 +9270,15 @@
> 	  (vec_select:V2SF
> 	    (match_operand:V4SF 1 "vector_operand")
> 	    (parallel [(const_int 0) (const_int 1)]))))]
>-  "TARGET_SSE2")
>+  "TARGET_SSE2"
>+{
>+  if (MEM_P (operands[1]))
>+    {
>+      operands[1] = gen_lowpart (V2SFmode, operands[1]);
>+      emit_insn (gen_sse2_cvtps2pd_1 (operands[0], operands[1]));
>+      DONE;
>+    }
>+})

Does adjust_address_nv work here instead of gen_lowpart?

Uros.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[haochen.jiang at intel dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #7 from Haochen Jiang <haochen.jiang at intel dot com> ---
(In reply to Uroš Bizjak from comment #6)
> Comment on attachment 53261 [details]
> This patch aims to handle memory issue when unpacking in cvtps2pd
> 
> >@@ -9270,7 +9270,15 @@
> > 	  (vec_select:V2SF
> > 	    (match_operand:V4SF 1 "vector_operand")
> > 	    (parallel [(const_int 0) (const_int 1)]))))]
> >-  "TARGET_SSE2")
> >+  "TARGET_SSE2"
> >+{
> >+  if (MEM_P (operands[1]))
> >+    {
> >+      operands[1] = gen_lowpart (V2SFmode, operands[1]);
> >+      emit_insn (gen_sse2_cvtps2pd_1 (operands[0], operands[1]));
> >+      DONE;
> >+    }
> >+})
> 
> Does adjust_address_nv work here instead of gen_lowpart?
> 
> Uros.

I just did a quick test on that. It seems that it works.

I will send out the patch after fully tested.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[haochen.jiang at intel dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #8 from Haochen Jiang <haochen.jiang at intel dot com> ---
Created attachment 53269
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53269&action=edit
This patch aims to handle memory issue when unpacking in cvtps2pd (version 2)

Just fully tested on this patch. Changed to adjust_address_nv to reduce
function calls.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[haochen.jiang at intel dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #9 from Haochen Jiang <haochen.jiang at intel dot com> ---
(In reply to Haochen Jiang from comment #8)
> Created attachment 53269 [details]
> This patch aims to handle memory issue when unpacking in cvtps2pd (version 2)
> 
> Just fully tested on this patch. Changed to adjust_address_nv to reduce
> function calls.

What I mean by fully tested is regtested on x86_64-pc-linux-gnu.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Haochen Jiang <jianghc@gcc.gnu.org>:

https://gcc.gnu.org/g:bf3695691f4fc964a3b1c8274a6949d844e3edff

commit r13-1570-gbf3695691f4fc964a3b1c8274a6949d844e3edff
Author: Haochen Jiang <haochen.jiang@intel.com>
Date:   Tue Jul 5 14:12:18 2022 +0800

    i386: Handle memory operand for direct call to cvtps2pd in unpack

    gcc/ChangeLog:

            PR target/106180
            * config/i386/sse.md (sse2_cvtps2pd<mask_name>_1):
            Rename from *sse2_cvtps2pd<mask_name>_1.
            (vec_unpacks_lo_v4sf): Add handler for memory operand.

    gcc/testsuite/ChangeLog:

            PR target/106180
            * g++.target/i386/pr106180-1.C: New test.

[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed I assume.