[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 John David Anglin changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #22 from John David Anglin --- Fixed.
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #21 from John David Anglin --- Author: danglin Date: Fri May 24 23:20:25 2019 New Revision: 271614 URL: https://gcc.gnu.org/viewcvs?rev=271614&root=gcc&view=rev Log: PR target/90530 * config/pa/pa.c (pa_cannot_change_mode_class): Accept mode changes from DImode to SImode in floating-point registers on 64-bit target. * config/pa/pa.md (umulsidi3): Change nonimmediate_operand to register_operand in xmpyu patterns. Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/config/pa/pa.c branches/gcc-7-branch/gcc/config/pa/pa.md
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #20 from John David Anglin --- Author: danglin Date: Fri May 24 23:17:09 2019 New Revision: 271613 URL: https://gcc.gnu.org/viewcvs?rev=271613&root=gcc&view=rev Log: PR target/90530 * config/pa/pa.c (pa_can_change_mode_class): Accept mode changes from DImode to SImode in floating-point registers on 64-bit target. * config/pa/pa.md (umulsidi3): Change nonimmediate_operand to register_operand in xmpyu patterns. Modified: branches/gcc-8-branch/gcc/ChangeLog branches/gcc-8-branch/gcc/config/pa/pa.c branches/gcc-8-branch/gcc/config/pa/pa.md
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #19 from John David Anglin --- Author: danglin Date: Fri May 24 23:15:49 2019 New Revision: 271612 URL: https://gcc.gnu.org/viewcvs?rev=271612&root=gcc&view=rev Log: PR target/90530 * config/pa/pa.c (pa_can_change_mode_class): Accept mode changes from DImode to SImode in floating-point registers on 64-bit target. * config/pa/pa.md (umulsidi3): Change nonimmediate_operand to register_operand in xmpyu patterns. Modified: branches/gcc-9-branch/gcc/ChangeLog branches/gcc-9-branch/gcc/config/pa/pa.c branches/gcc-9-branch/gcc/config/pa/pa.md
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #18 from John David Anglin --- Author: danglin Date: Fri May 24 23:12:16 2019 New Revision: 271611 URL: https://gcc.gnu.org/viewcvs?rev=271611&root=gcc&view=rev Log: PR target/90530 * config/pa/pa.c (pa_can_change_mode_class): Accept mode changes from DImode to SImode in floating-point registers on 64-bit target. * config/pa/pa.md (umulsidi3): Change nonimmediate_operand to register_operand in xmpyu patterns. Modified: trunk/gcc/ChangeLog trunk/gcc/config/pa/pa.c trunk/gcc/config/pa/pa.md
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #17 from dave.anglin at bell dot net --- On 2019-05-20 8:37 a.m., rguenth at gcc dot gnu.org wrote: > AFAICS pa is using LRA now. I wish.
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #16 from dave.anglin at bell dot net --- On 2019-05-20 8:14 a.m., rguenth at gcc dot gnu.org wrote: >> My feeling is reload should respect pa_can_change_mode_class(). > Maybe it's asking wrong since you have > > if (GET_MODE_SIZE (from) == GET_MODE_SIZE (to)) > return true; > > and the size of the SUBREG_REG is the same as the FP reg? Good point!
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #15 from Richard Biener --- (In reply to Richard Biener from comment #14) > (In reply to dave.anglin from comment #13) > > On 2019-05-20 6:26 a.m., rguenth at gcc dot gnu.org wrote: > > > most definitely a reload as > > > > > > +(insn 177 176 178 2 (set (reg:SI 52 %fr24) > > > +(subreg:SI (reg:DI 51 %fr23) 4)) -1 > > > + (nil)) > > > > > > which isn't recognized isn't good. I don't know PA on what it can > > > and what not but these kind of sets from subregs appear during RTL > > > expansion for > > This isn't recognized because the only way to do it in general is to spill > > reg:DI 51 to memory > > and to read it back in SI mode. > > Yeah, and for the C testcase it does... > > > Maybe we are lucky with the const_int > > because some values > > are forced constant pool. > > > > There are no convert instructions other than sf<=>df mode and loads don't > > zero extend as > > they do to the integer registers. > > > > As I said, pa_can_change_mode_class() is written to prevent mode changes in > > the FP registers: > > > > /* There is no way to load QImode or HImode values directly from memory > > to a FP register. SImode loads to the FP registers are not zero > > extended. On the 64-bit target, this conflicts with the definition > > of LOAD_EXTEND_OP. Thus, we can't allow changing between modes with > > different sizes in the floating-point registers. */ > > if (MAYBE_FP_REG_CLASS_P (rclass)) > > return false; > > > > My feeling is reload should respect pa_can_change_mode_class(). > > Maybe it's asking wrong since you have > > if (GET_MODE_SIZE (from) == GET_MODE_SIZE (to)) > return true; > > and the size of the SUBREG_REG is the same as the FP reg? You'd need > to debug here. The only uses of can_change_mode_class are from old reload.c and via REG_CAN_CHANGE_MODE_P (also mostly used from old reload). AFAICS pa is using LRA now.
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 Richard Biener changed: What|Removed |Added Keywords||ra CC||vmakarov at gcc dot gnu.org --- Comment #14 from Richard Biener --- (In reply to dave.anglin from comment #13) > On 2019-05-20 6:26 a.m., rguenth at gcc dot gnu.org wrote: > > most definitely a reload as > > > > +(insn 177 176 178 2 (set (reg:SI 52 %fr24) > > +(subreg:SI (reg:DI 51 %fr23) 4)) -1 > > + (nil)) > > > > which isn't recognized isn't good. I don't know PA on what it can > > and what not but these kind of sets from subregs appear during RTL > > expansion for > This isn't recognized because the only way to do it in general is to spill > reg:DI 51 to memory > and to read it back in SI mode. Yeah, and for the C testcase it does... > Maybe we are lucky with the const_int > because some values > are forced constant pool. > > There are no convert instructions other than sf<=>df mode and loads don't > zero extend as > they do to the integer registers. > > As I said, pa_can_change_mode_class() is written to prevent mode changes in > the FP registers: > > /* There is no way to load QImode or HImode values directly from memory > to a FP register. SImode loads to the FP registers are not zero > extended. On the 64-bit target, this conflicts with the definition > of LOAD_EXTEND_OP. Thus, we can't allow changing between modes with > different sizes in the floating-point registers. */ > if (MAYBE_FP_REG_CLASS_P (rclass)) > return false; > > My feeling is reload should respect pa_can_change_mode_class(). Maybe it's asking wrong since you have if (GET_MODE_SIZE (from) == GET_MODE_SIZE (to)) return true; and the size of the SUBREG_REG is the same as the FP reg? You'd need to debug here.
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #13 from dave.anglin at bell dot net --- On 2019-05-20 6:26 a.m., rguenth at gcc dot gnu.org wrote: > most definitely a reload as > > +(insn 177 176 178 2 (set (reg:SI 52 %fr24) > +(subreg:SI (reg:DI 51 %fr23) 4)) -1 > + (nil)) > > which isn't recognized isn't good. I don't know PA on what it can > and what not but these kind of sets from subregs appear during RTL > expansion for This isn't recognized because the only way to do it in general is to spill reg:DI 51 to memory and to read it back in SI mode. Maybe we are lucky with the const_int because some values are forced constant pool. There are no convert instructions other than sf<=>df mode and loads don't zero extend as they do to the integer registers. As I said, pa_can_change_mode_class() is written to prevent mode changes in the FP registers: /* There is no way to load QImode or HImode values directly from memory to a FP register. SImode loads to the FP registers are not zero extended. On the 64-bit target, this conflicts with the definition of LOAD_EXTEND_OP. Thus, we can't allow changing between modes with different sizes in the floating-point registers. */ if (MAYBE_FP_REG_CLASS_P (rclass)) return false; My feeling is reload should respect pa_can_change_mode_class(). The left and right halves of the float doubles are separately addressable, so in this particular case the pattern could be implemented using register renaming, but that was never done.
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530 --- Comment #12 from John David Anglin --- Created attachment 46383 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46383&action=edit Patch
[Bug target/90530] [9/10 Regression] Invalid SUBREG insn generated by reload
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90530
Richard Biener changed:
What|Removed |Added
Component|middle-end |target
--- Comment #11 from Richard Biener ---
I wonder what the difference to PA is between the reloaded
+(insn 20 181 182 2 (set (reg:DI 50 %fr22 [95])
+(mult:DI (zero_extend:DI (reg:SI 52 %fr24))
+(reg:DI 53 %fr25))) 179 {*pa.md:5338}
and the original
-(insn 20 10 21 2 (set (reg:DI 95)
-(mult:DI (zero_extend:DI (subreg:SI (reg/f:DI 146) 4))
-(const_int 4294967295 [0x]))) 179 {*pa.md:5338}
most definitely a reload as
+(insn 177 176 178 2 (set (reg:SI 52 %fr24)
+(subreg:SI (reg:DI 51 %fr23) 4)) -1
+ (nil))
which isn't recognized isn't good. I don't know PA on what it can
and what not but these kind of sets from subregs appear during RTL
expansion for
;; _26 = _27 * 18446744073709551613;
and RTL opts push them into the mult.
To me it looks like a testcase like
int a;
__SIZE_TYPE__ foo ()
{
return (__SIZE_TYPE__)&a * 18446744073709551613ull;
}
should trigger the same issue. It creates the same RTL with
-O2 -fno-tree-vrp -fdisable-tree-evrp but the doesn't ICE beause
we reload
(insn 16 10 19 2 (set (reg:DI 77)
(mult:DI (zero_extend:DI (subreg:SI (reg:DI 81) 4))
(const_int 4294967293 [0xfffd]))) "t.c":6:14 179 {*pa.md:5338}
via memory as
(insn 47 52 48 2 (set (mem/c:DI (plus:DI (reg/f:DI 30 %r30)
(const_int -56 [0xffc8])) [0 S8 A64])
(reg:DI 19 %r19)) "t.c":6:14 128 {*pa.md:4178}
(nil))
(insn 48 47 16 2 (set (reg:DI 51 %fr23)
(mem/c:DI (plus:DI (reg/f:DI 30 %r30)
(const_int -56 [0xffc8])) [0 S8 A64])) "t.c":6:14
128 {*pa.md:4178}
(nil))
(insn 16 48 53 2 (set (reg:DI 52 %fr24)
(mult:DI (zero_extend:DI (reg:SI 51 %fr23))
(reg:DI 52 %fr24))) "t.c":6:14 179 {*pa.md:5338}
(nil))
not sure if we're just lucky here or what.
Anyhow this is IMHO clearly a target issue.
