[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752 --- Comment #15 from Jakub Jelinek --- Author: jakub Date: Wed Mar 20 11:26:42 2019 New Revision: 269819 URL: https://gcc.gnu.org/viewcvs?rev=269819=gcc=rev Log: PR target/89752 * lra-constraints.c (process_alt_operands) : For BLKmode, don't update this_alternative nor this_alternative_set. * g++.target/aarch64/aarch64.exp: New file. * g++.target/aarch64/pr89752.C: New test. Added: trunk/gcc/testsuite/g++.target/aarch64/aarch64.exp trunk/gcc/testsuite/g++.target/aarch64/pr89752.C Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c trunk/gcc/testsuite/ChangeLog
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #14 from Jakub Jelinek ---
The following patch fixes the remaining ICE for me:
--- gcc/lra-constraints.c.jj2019-03-16 22:17:21.060937047 +0100
+++ gcc/lra-constraints.c 2019-03-19 11:49:11.982058568 +0100
@@ -2350,6 +2350,8 @@ process_alt_operands (int only_alternati
break;
reg:
+ if (mode == BLKmode)
+ break;
this_alternative = reg_class_subunion[this_alternative][cl];
IOR_HARD_REG_SET (this_alternative_set,
reg_class_contents[cl]);
@@ -2360,8 +2362,6 @@ process_alt_operands (int only_alternati
IOR_HARD_REG_SET (this_costly_alternative_set,
reg_class_contents[cl]);
}
- if (mode == BLKmode)
- break;
winreg = true;
if (REG_P (op))
{
What in my understanding happens is that even when the r constraint for the
BLKmode MEM doesn't win, this_alternative is still updated (to GENERAL_REGS in
this case) and as the m constraint matches, we still process it as if
GENERAL_REGS is an option. It is not, BLKmode must live in memory.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752 --- Comment #13 from Jakub Jelinek --- Fixed on the originally provided testcase, not on the #c7 testcase, that needs to be fixed in LRA not to try to reload BLKmode MEMs into REGs.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752 --- Comment #12 from Jakub Jelinek --- Author: jakub Date: Tue Mar 19 08:11:25 2019 New Revision: 269793 URL: https://gcc.gnu.org/viewcvs?rev=269793=gcc=rev Log: PR target/89752 * gimplify.c (gimplify_asm_expr): For output argument with TREE_ADDRESSABLE type, clear allows_reg if it allows memory, otherwise diagnose error. * g++.dg/ext/asm15.C: Check for particular diagnostic wording. * g++.dg/ext/asm16.C: Likewise. * g++.dg/ext/asm17.C: New test. Added: trunk/gcc/testsuite/g++.dg/ext/asm17.C Modified: trunk/gcc/ChangeLog trunk/gcc/gimplify.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/g++.dg/ext/asm15.C trunk/gcc/testsuite/g++.dg/ext/asm16.C
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #11 from Jakub Jelinek ---
Actually can't bisect, as gcc 8 I have installed is no longer able to build
r215000 or revisions around it (some error on wide-int.h:
../../gcc/wide-int.h:372:10: error: too many template-parameter-lists
struct binary_traits
^~
etc.).
That said, if we want to fix the:
struct A { A (); ~A (); short c; };
void
foo ()
{
A a0, a1;
__asm volatile ("" : "+rm" (a0), "+rm" (a1));
}
in the provided testcase, we could use:
--- gcc/gimplify.c.jj 2019-03-07 20:45:39.168938360 +0100
+++ gcc/gimplify.c 2019-03-18 16:18:16.515466234 +0100
@@ -6155,6 +6155,19 @@ gimplify_asm_expr (tree *expr_p, gimple_
is_inout = false;
}
+ /* If we can't make copies, we can only accept memory. */
+ if (TREE_ADDRESSABLE (TREE_TYPE (TREE_VALUE (link
+ {
+ if (allows_mem)
+ allows_reg = 0;
+ else
+ {
+ error ("impossible constraint in %");
+ error ("non-memory output %d must stay in memory", i);
+ return GS_ERROR;
+ }
+ }
+
if (!allows_reg && allows_mem)
mark_addressable (TREE_VALUE (link));
which is something we already do for the input args, just for some strange
reason not for output args as well (if we wanted to backport it, I'd leave the
error part out).
That said, because users can write even that:
struct A { A (); ~A (); short c; };
void
foo ()
{
A a0, a1;
__asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
}
we need something in LRA too, that will simply treat BLKmode matching
constraints when both registers and memory are allowed as if only memory was
allowed.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #10 from Wilco ---
It seems that rewriting "+rm" into "=rm" and "0" is not equivalent. Eg.
__asm__ ("" : [a0] "=m" (A0) : "0" (A0));
gives a million warnings "matching constraint does not allow a register", so
"0" appears to imply a register operand. Reload seems to deal with "=rm" and
"rm" or "=rm" and "0m", but a single "0" prefers a register and that's why it
tries to insert an incorrect BLK move.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752 --- Comment #9 from Jakub Jelinek --- Bisecting now, r21 still works, r215000 ICEs.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #8 from rguenther at suse dot de ---
On Mon, 18 Mar 2019, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
>
> Jakub Jelinek changed:
>
>What|Removed |Added
>
> CC||jakub at gcc dot gnu.org
>
> --- Comment #7 from Jakub Jelinek ---
> I've reduced it to:
> struct A { A (); ~A (); short c; };
>
> void
> foo ()
> {
> A a0, a1;
> __asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
> }
> which better matches what is going on (the type is TYPE_ADDRESSABLE and that
> is
> why it has BLKmode).
And it works "fine" in x86_64 ...
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #7 from Jakub Jelinek ---
I've reduced it to:
struct A { A (); ~A (); short c; };
void
foo ()
{
A a0, a1;
__asm volatile ("" : "=rm" (a0), "=rm" (a1) : "0" (a0), "1" (a1));
}
which better matches what is going on (the type is TYPE_ADDRESSABLE and that is
why it has BLKmode).
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #6 from Jakub Jelinek ---
It is on:
__asm__("" : "a0" "=rm" A0, "a1" "=rm" A1 : "0" A0, "1" A1);
where A0 and A1 are variables with LhsPacket type, which is 2 byte
TYPE_ADDRESSABLE aggregate type.
The r in the constraints looks completely bogus to me for addressable vars that
must live in memory, but guess we shouldn't ICE even on this invalid stuff.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #5 from rguenther at suse dot de ---
On Mon, 18 Mar 2019, wilco at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
>
> --- Comment #4 from Wilco ---
> Small example which generates the same ICE on every GCC version:
>
> typedef struct { int x, y, z; } X;
>
> void f(void)
> {
> X A0, A1;
> __asm__ ("" : [a0] "+rm" (A0),[a1] "+rm" (A1));
> }
>
> So it's completely invalid inline asm...
Looks like an attempt to provide some barrier?
We still shouldn't ICE.
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #4 from Wilco ---
Small example which generates the same ICE on every GCC version:
typedef struct { int x, y, z; } X;
void f(void)
{
X A0, A1;
__asm__ ("" : [a0] "+rm" (A0),[a1] "+rm" (A1));
}
So it's completely invalid inline asm...
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
--- Comment #3 from Wilco ---
Full instruction:
(insn 531 530 532 19 (parallel [
(set (mem/c:BLK (reg:DI 3842) [29 A0+0 S2 A64])
(asm_operands:BLK ("") ("=rm") 0 [
(mem/c:BLK (reg:DI 3846) [29 A0+0 S2 A64])
(mem/c:BLK (reg:DI 3848) [29 A1+0 S2 A128])
]
[
(asm_input:BLK ("0")
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418)
(asm_input:BLK ("1")
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418)
]
[]
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418))
(set (mem/c:BLK (reg:DI 3844) [29 A1+0 S2 A128])
(asm_operands:BLK ("") ("=rm") 1 [
(mem/c:BLK (reg:DI 3846) [29 A0+0 S2 A64])
(mem/c:BLK (reg:DI 3848) [29 A1+0 S2 A128])
]
[
(asm_input:BLK ("0")
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418)
(asm_input:BLK ("1")
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418)
]
[]
external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h:1418))
])
"external/eigen_archive/unsupported/Eigen/CXX11/../../../Eigen/src/Core/products/GeneralBlockPanelKernel.h":1418:511
-1
(nil))
I guess this comes from the source like this with some of the arguments being
immediates due to template substitution:
__asm__ ("" : [a0] "+rm" (A0),[a1] "+rm" (A1));
It's not obvious what this is supposed to achieve, let alone whether it is
valid...
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752
Wilco changed:
What|Removed |Added
Status|UNCONFIRMED |NEW
Last reconfirmed||2019-03-18
CC||wilco at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #2 from Wilco ---
Confirmed. It ICEs in Eigen::internal::gebp_kernel, 2, 4,
false, false>::operator()
It seems to choke on this asm during reload:
531: {[r3842:DI]=asm_operands;[r3844:DI]=asm_operands;}
and somehow emit a move between these operands:
(reg:BLK 3849) (mem/c:BLK (reg:DI 3846) [29 A0+0 S2 A64])
[Bug target/89752] [8/9 Regression] ICE in emit_move_insn, at expr.c:3723
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89752 Richard Biener changed: What|Removed |Added Keywords||ice-on-valid-code, ||needs-reduction Target||aarch64 Target Milestone|--- |8.4 Known to fail||8.3.1, 9.0 --- Comment #1 from Richard Biener --- Note the regression state isn't exactly clear. I'm trying brute-force reduction (have no easy way to check testcase validity lacking a compiler that builds this w/o an ICE). The original issue was reported with -O2 -std=c++11 -mlittle-endian -mabi=lp64 -fstack-protector -fno-omit-frame-pointer -ffunction-sections -fdata-sections -fPIC -fno-exceptions -ftemplate-depth=900 -fno-canonical-system-headers.
