[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #11 from Richard Biener --- Fixed I assume.
[Bug target/106180] [13 Regression] ICE in extract_insn, at recog.cc:2791 since r13-1418-g73f942c08deef3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 --- Comment #10 from CVS Commits --- The master branch has been updated by Haochen Jiang : https://gcc.gnu.org/g:bf3695691f4fc964a3b1c8274a6949d844e3edff commit r13-1570-gbf3695691f4fc964a3b1c8274a6949d844e3edff Author: Haochen Jiang 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_1): Rename from *sse2_cvtps2pd_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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 --- Comment #9 from Haochen Jiang --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 --- Comment #8 from Haochen Jiang --- Created attachment 53269 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53269=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180
--- Comment #7 from Haochen Jiang ---
(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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180
--- Comment #6 from Uroš Bizjak ---
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180
David Binderman changed:
What|Removed |Added
CC||dcb314 at hotmail dot com
--- Comment #5 from David Binderman ---
I have something similar:
struct PointT {
double y;
};
template struct __array_traits { using _Type = PointT[_Nm]; };
template 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 --- Comment #4 from Haochen Jiang --- Created attachment 53261 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53261=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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 --- Comment #3 from Jakub Jelinek --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 Hongyu Wang changed: What|Removed |Added CC||wwwhhhyyy333 at gmail dot com --- Comment #2 from Hongyu Wang --- (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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 Richard Biener 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106180 Martin Liška 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
