[Bug target/86069] New: MIPS out-of-range branch expansion can break with -fpic and shrink wrapping
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86069 Bug ID: 86069 Summary: MIPS out-of-range branch expansion can break with -fpic and shrink wrapping Product: gcc Version: 7.3.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: target Assignee: unassigned at gcc dot gnu.org Reporter: mpf at gcc dot gnu.org Target Milestone: --- Created attachment 44241 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=44241&action=edit test case MIPS determines whether a branch is out of range quite late on and replaces an out of range branch with either a direct or indirect jump. When compiling PIC code then the only option for a long branch is to perform an indirect jump as direct jumps are not PC-relative (at least not in the obvious way). The label to jump to has to be obtained from the GOT as there is no way to calculate a PC-relative offset directly either. The last-minute expansion of this code has to grab the GOT pointer from the stack and then load the destination. The expansion looks like this: beq $4,$2,.L4 into bne $4,$2,.L7 .setnoat lw $1,16($sp) lw $1,%got(.L4)($1) addiu $1,$1,%lo(.L4) jr $1 .setat .L7: With shrink-wrapping it is possible for the compiler to move the prologue (that sets up the GOT pointer and stashes it on the stack) to be moved after a potentially out-of-range branch. The branch instructions do not acknowledge the potential for them to be expanded into a GOT dependent sequence so there is no way for the shrink-wrap code to do the right thing. The obvious/simple solution may well be to mark all conditional branches as using the GOT pointer for PIC code but this will effectively disable shrink wrapping. To do anything smarter would require handling out-of-range branches much earlier which is also unlikely to be a good idea. Test case attached in dejagnu format. It fails at O1 and above. The original failure was found as part of building LLVM with GCC and has been simplified to this test case by Simon Dardis.
[Bug target/86069] MIPS out-of-range branch expansion can break with -fpic and shrink wrapping
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86069 mpf at gcc dot gnu.org changed: What|Removed |Added Target||mips-* Status|UNCONFIRMED |NEW Last reconfirmed||2018-06-06 Target Milestone|--- |9.0 Ever confirmed|0 |1 Known to fail||6.3.0, 7.3.0 --- Comment #1 from mpf at gcc dot gnu.org --- Confirmed as a bug.
[Bug target/78176] [MIPS] miscompiles ldxc1 with large pointers on 32-bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176 --- Comment #29 from mpf at gcc dot gnu.org --- I don't remember the detail of this issue but I believe I was convinced that it is down to the lack of setting PX appropriately in HW. UX==0, PX==1. The PX control bit forces address calculations i.e. base + imm or base + reg to be performed with 32-bit rules but allows 64 instruction usage. Since there is a processor mode that is perfectly capable of meeting the requirements of a program with 64bit data and 32bit pointers then the solution is to set PX for N32 rather than UX.
[Bug rtl-optimization/84071] [7/8 regression] wrong elimination of zero-extension after sign-extended load
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84071 mpf at gcc dot gnu.org changed: What|Removed |Added CC||mpf at gcc dot gnu.org --- Comment #21 from mpf at gcc dot gnu.org --- (In reply to Eric Botcazou from comment #20) > > How is this any different from 32-bit operations in say MIPS? The only > > difference seems to be that MIPS sign-extends 32-bit operations to 64 bit > > while AArch64 zero-extends. If that's correct for MIPS it seems that > > WORD_REGISTER_OPERATIONS only applies to types smaller than SImode. > > Interesting question. One indeed could argue that, if 64-bit MIPS defines > it, then Aarch64 could do it too since they are symmetric wrt > sign/zero-extension > but I don't have a definitive answer as I don't really know the history of > WORD_REGISTER_OPERATIONS on MIPS. I believe the critical part for MIPS is that our 32-bit instructions do care what the upper 32-bits of a 64-bit register contain. It is undefined if they operate on a non-canonical 32-bit form and the instructions that are width agnostic (conditional branch and logical ops) rely on canonical forms to work correctly with 32-bit values. So MIPS fundamentally needs this feature to work correctly; whether AArch64 needs it or may just benefit from it depends on a lot of detailed knowledge of the ISA and architecture. Given Richard Sandiford is currently working on ARM ports but also fully understands the MIPS arch then he may be a good person to consult.
[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327 mpf at gcc dot gnu.org changed: What|Removed |Added CC||mpf at gcc dot gnu.org --- Comment #12 from mpf at gcc dot gnu.org --- (In reply to Tom de Vries from comment #11) > (In reply to Vladimir Makarov from comment #10) > > Any news about the patch testing on MIPS. It would be nice to move forward > > with the PR. > > Pinged Matthew here: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01392.html Hi Tom, Sorry for my general lack of engagement I'm struggling to do much at all to support the community at the moment. I have set off a couple of builds to target MIPS16; I don't have any recipe for bootstrapping with MIPS16 enabled so the most efficient way I can give some assurance is just the GCC testsuite run cross compiled with a simulator. I've started two build of GCC without and with the patch and will aim to report back on the testsuite runs as soon as I can. Matthew
[Bug target/79912] [7/8 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 --- Comment #20 from mpf at gcc dot gnu.org --- Palmer: There is a commit listed for this bug, did that fix the issue and can the bug be marked fixed?
[Bug target/81803] [7/8 regression] miscompilation at -O1 on mips64el
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81803 --- Comment #7 from mpf at gcc dot gnu.org --- (In reply to Eric Botcazou from comment #6) > > I have just noticed this which seems curious. Is the 39 -> 40 combine really > > a valid transformation? It seems we've lost the sign extension and we're > > just putting a 32-bit value into a 64-bit register without trying to clear > > the upper bits anymore? > > Yes, for WORD_REGISTER_OPERATIONS architectures the combiner can do things > like that, although there might be bugs lurking of course. Yes, this looks like a valid transformation to me. The upper 63 bits on r235 are guaranteed to be zero so the sign extension can be eliminated trivially but also it is actually a zero extension and could be eliminated because the LOAD_EXTEND_OP is ZERO_EXT for HImode. Not looking forward to investigating this!
[Bug target/81803] [7/8 regression] miscompilation at -O1 on mips64el
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81803 --- Comment #9 from mpf at gcc dot gnu.org --- Created attachment 42075 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42075&action=edit Proposed fix Off-thread James pointed out that one of my patches I did last year appeared to fix this issue but it was one I reverted owing to breaking ARM (and probably others). The thread was: https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00518.html At the time I thought I had fixed the same problem twice and that the changes to simplify_operand_subreg were sufficient. It occurs to me however that simplify_operand_subreg only has the opportunity to fix WORD_REGISTER_OPERATIONS issues affecting subreg(mem) patterns not subreg(reg) patterns where reg is a pseudo that may yet be spilled. I can't say that I am 100% convinced yet with my thinking here but I've attached an updated version of the original patch with some changes: * Incorporated Eric's feedback on the original patch to check GET_MODE_PRECISION instead of GET_MODE_SIZE for comparing whether a mode is strictly narrower * Limited the test to word sized inner modes or smaller * Limited the test to OP_OUT or OP_INOUT as I can't see any reason why it would matter if we do a narrower input reload This is barely tested but does fix testcase-c which is the only one I can get to trigger.
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 mpf at gcc dot gnu.org changed: What|Removed |Added Priority|P3 |P2 Status|WAITING |ASSIGNED CC||mpf at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |mpf at gcc dot gnu.org Severity|normal |major --- Comment #10 from mpf at gcc dot gnu.org --- While mips64el-unknown-linux is not a specific primary platform I see no reason why this bug would not show on mipsisa64-elf. Bumping to P2.
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #13 from mpf at gcc dot gnu.org --- (In reply to Eric Botcazou from comment #12) > > Maybe the load sign-extends instead of zero-extending as specified > > initially. > > But I'm not sure that this matters here, since: > > (insn 58 57 59 3 (set (subreg:SI (reg:DI 316 [ iftmp.3_114 ]) 0) > (ne:SI (reg/f:DI 469 [ current_scope.1_1->bindings ]) > (const_int 0 [0]))) > "/althome/mips/v6/src/gcc/gcc/c/c-decl.me.c":915 501 {*sne_zero_disi} > (expr_list:REG_DEAD (reg/f:DI 469 [ current_scope.1_1->bindings ]) > (nil))) > > can put only 0 or 1 in the SUBREG, can't it? Yes, that is true but the upper 32-bits still need to be 'zero'. What happens later on is that the (subreg:SI (reg:DI 316)) is spilled, spilling only 32-bits to the stack but it gets reloaded as DImode/64-bit. The upper-32 bits are junk. I don't believe that is an LRA bug as it is doing exactly what is described by the subreg. The original zero_extend in insn 58 to DImode is therefore very important here and cannot be converted to a subreg. (I haven't got to the specific combine rule yet that is doing this substitution to see which decision is bad.)
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #16 from mpf at gcc dot gnu.org --- (In reply to Eric Botcazou from comment #15) > That's incorrect, see what reload1.c:eliminate_regs_1 says about it: > > if (MEM_P (new_rtx) > && ((x_size < new_size > /* On RISC machines, combine can create rtl of the form > (set (subreg:m1 (reg:m2 R) 0) ...) > where m1 < m2, and expects something interesting to > happen to the entire word. Moreover, it will use the > (reg:m2 R) later, expecting all bits to be preserved. > So if the number of words is the same, preserve the > subreg so that push_reload can see it. */ > && !(WORD_REGISTER_OPERATIONS > && (x_size - 1) / UNITS_PER_WORD > == (new_size -1 ) / UNITS_PER_WORD)) > || x_size == new_size) > ) > return adjust_address_nv (new_rtx, GET_MODE (x), SUBREG_BYTE (x)); > else > return gen_rtx_SUBREG (GET_MODE (x), new_rtx, SUBREG_BYTE (x)); I was just contemplating your comments on LRA and coming to the conclusion it must be LRA in the end. The implications of the subreg with WORD_REGISTER_OPERATIONS are quite a brain teaser. I'll move on to LRA instead. I forgot to say that while reviewing this issue I saw that your original patch in combine removed a decent amount of zero extensions from MIPS. Thanks!
[Bug target/78176] [MIPS] miscompiles ldxc1 with large pointers on 32-bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2017-01-16 CC||mpf at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |dgilmore at mips dot com Ever confirmed|0 |1 --- Comment #13 from mpf at gcc dot gnu.org --- (In reply to Maciej W. Rozycki from comment #11) > TBH this does look like trying to rely on UB to me, as per section > 6.5.6 "Additive operators" clause 8 of the C language standard, which > states (among others): > > "If both the pointer operand and the result point to elements of the > same array object, or one past the last element of the array object, > the evaluation shall not produce an overflow; otherwise, the behavior > is undefined." > > Here under the triggering conditions the pointer the integer is added > to with LDXC1 does not point to an element of the array operated on (or > to one past the last), so the hardware operation matches the standard's > semantics WRT overflow and I don't think we ought to be pushing it. Aren't language rules mostly irrelevant at this stage though? We don't really have the concept of a pointer after generating RTL we just happen to have some values in a mode that is the same as Pmode. mips.h comment for Pmode: /* Specify the machine mode that pointers have. After generation of rtl, the compiler makes no further distinction between pointers and any other objects of this machine mode. */ > So it looks like a middle end bug to me and the backend is fine in > faithfully assuming its RTL pattern won't be passed operands leading to > UB. I can't see any feature/option/setting that gives this guarantee. Why do you think the backend can make this assumption?
[Bug target/78176] [MIPS] miscompiles ldxc1 with large pointers on 32-bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176 --- Comment #14 from mpf at gcc dot gnu.org --- Author: mpf Date: Thu Jan 19 16:05:59 2017 New Revision: 244640 URL: https://gcc.gnu.org/viewcvs?rev=244640&root=gcc&view=rev Log: MIPS: PR target/78176 add -mlxc1-sxc1. gcc/ PR target/78176 * config.gcc (supported_defaults): Add lxc1-sxc1. (with_lxc1_sxc1): Add validation. (all_defaults): Add lxc1-sxc1. * config/mips/mips.opt (mlxc1-sxc1): New option. * gcc/config/mips/mips.h (OPTION_DEFAULT_SPECS): Add a default for mlxc1-sxc1. (TARGET_CPU_CPP_BUILTINS): Add builtin_define for __mips_no_lxc1_sxc1. (ISA_HAS_LXC1_SXC1): Gate with mips_lxc1_sxc1. * gcc/doc/invoke.texi (-mlxc1-sxc1): Document the new option. * doc/install.texi (--with-lxc1-sxc1): Document the new option. gcc/testsuite/ * gcc.target/mips/lxc1-sxc1-1.c: New file. * gcc.target/mips/lxc1-sxc1-2.c: Likewise. * gcc.target/mips/mips.exp (mips_option_groups): Add ghost option HAS_LXC1. (mips_option_groups): Add -m[no-]lxc1-sxc1. (mips-dg-init): Detect default -mno-lxc1-sxc1. (mips-dg-options): Handle HAS_LXC1 arch upgrade/downgrade. Added: trunk/gcc/testsuite/gcc.target/mips/lxc1-sxc1-1.c trunk/gcc/testsuite/gcc.target/mips/lxc1-sxc1-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/config.gcc trunk/gcc/config/mips/mips.h trunk/gcc/config/mips/mips.opt trunk/gcc/doc/install.texi trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/mips/mips.exp
[Bug target/78012] -mfpxx produces assembly code using odd FP registers on MIPS
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012 mpf at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED CC||mpf at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |mpf at gcc dot gnu.org
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #18 from mpf at gcc dot gnu.org --- Author: mpf Date: Mon Feb 20 12:07:06 2017 New Revision: 245599 URL: https://gcc.gnu.org/viewcvs?rev=245599&root=gcc&view=rev Log: Tighten condition for converting SUBREG reloads from OP_OUT to OP_INOUT gcc/ PR target/78660 * lra-constraints.c (curr_insn_transform): Tighten condition for converting SUBREG reloads from OP_OUT to OP_INOUT. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #17 from mpf at gcc dot gnu.org --- Author: mpf Date: Mon Feb 20 12:06:56 2017 New Revision: 245598 URL: https://gcc.gnu.org/viewcvs?rev=245598&root=gcc&view=rev Log: Handle WORD_REGISTER_OPERATIONS when reloading (subreg (reg)) gcc/ PR target/78660 * lra-constraints.c (curr_insn_transform): Handle WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c
[Bug target/78012] -mfpxx produces assembly code using odd FP registers on MIPS
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012 --- Comment #6 from mpf at gcc dot gnu.org --- Author: mpf Date: Mon Feb 20 12:07:23 2017 New Revision: 245601 URL: https://gcc.gnu.org/viewcvs?rev=245601&root=gcc&view=rev Log: Ensure the mode used to create split registers is suppported gcc/ PR target/78012 * lra-constraints.c (split_reg): Check requested split mode is supported by the register. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c
[Bug rtl-optimization/79660] [7 regression] Arm register allocation failure with thumb1 building libgcc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79660 mpf at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED CC||mpf at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |mpf at gcc dot gnu.org --- Comment #3 from mpf at gcc dot gnu.org --- Created attachment 40801 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40801&action=edit Proposed fix Here is a proposed fix. I have only built GCC for MIPS and not tested nor bootstrapped yet. Test will be quick-ish bootstrap much longer so if it fixes ARM it may be worthwhile going ahead with it. I also corrected a test to use mode precision instead of size when choosing to upgrade an OP_OUT to an OP_INOUT reload.
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #19 from mpf at gcc dot gnu.org --- Author: mpf Date: Tue Feb 21 13:29:07 2017 New Revision: 245626 URL: https://gcc.gnu.org/viewcvs?rev=245626&root=gcc&view=rev Log: Revert r245598 gcc/ PR target/78660 Revert: 2017-02-20 Matthew Fortune * lra-constraints.c (curr_insn_transform): Handle WORD_REGISTER_OPERATIONS requirements when reloading SUBREGs. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c
[Bug rtl-optimization/79660] [7 regression] Arm register allocation failure with thumb1 building libgcc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79660 mpf at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #4 from mpf at gcc dot gnu.org --- I have reverted my original commit. Please ignore my proposed fix it has other issues, as do another 2 attempts I made without posting them. I have either fixed my original issue twice (this instance being the wrong place) or I don't understand this enough (still!!) to make the right change. I'm going to retest MIPS without this change. I will at least do a build of arm-none-eabi before further changes in this area! Apologies again.
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 --- Comment #20 from mpf at gcc dot gnu.org --- Author: mpf Date: Wed Feb 22 17:20:14 2017 New Revision: 245655 URL: https://gcc.gnu.org/viewcvs?rev=245655&root=gcc&view=rev Log: Support WORD_REGISTER_OPERATIONS requirements in simplify_operand_subreg gcc/ PR target/78660 * lra-constraints.c (simplify_operand_subreg): Handle WORD_REGISTER_OPERATIONS targets. Modified: trunk/gcc/ChangeLog trunk/gcc/lra-constraints.c
[Bug rtl-optimization/79150] ICE: in commit_one_edge_insertion, at cfgrtl.c:2077 for the gcc.dg/pr77834.c test
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79150 mpf at gcc dot gnu.org changed: What|Removed |Added Target|mips-mti-* |mips* Priority|P3 |P2 Status|UNCONFIRMED |NEW Last reconfirmed||2017-02-24 CC||segher at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from mpf at gcc dot gnu.org --- Updating to P2 as this test case can be tweaked to show the same ICE on MIPS64 making it a bug on a primary target. To show this on MIPS64 (n32/n64) then a vector size of 128 is required rather than 64.
[Bug rtl-optimization/79150] ICE: in commit_one_edge_insertion, at cfgrtl.c:2077 for the gcc.dg/pr77834.c test
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79150 mpf at gcc dot gnu.org changed: What|Removed |Added Priority|P2 |P3 Known to fail||4.8.5, 4.9.4, 7.0.1 --- Comment #3 from mpf at gcc dot gnu.org --- (In reply to Richard Biener from comment #2) > Priority on non-regression bugs is meaningless. So - can you identify a > working release (list it in Known-to-work)? Ah, OK. It looks like I have never actually read all the details from the managing bugs page! I've just gone all the way down to GCC 4.8 and the testcase is failing from then clearly showing the code is rare. The ICE in the testsuite results make this failure seem worse than it is I guess. Just an unfortunate testcase that exposes a long term bug.
[Bug target/79473] -mload-store-pairs option is not documented in invoke.texi
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79473 --- Comment #1 from mpf at gcc dot gnu.org --- Author: mpf Date: Fri Feb 24 22:35:59 2017 New Revision: 245725 URL: https://gcc.gnu.org/viewcvs?rev=245725&root=gcc&view=rev Log: Add documentation for -mload-store-pairs gcc/ PR target/79473 * doc/invoke.texi: Document -mload-store-pairs. Modified: trunk/gcc/ChangeLog trunk/gcc/doc/invoke.texi
[Bug target/79914] VEC_SELECT bugs in mips patterns
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79914 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2017-03-06 Assignee|unassigned at gcc dot gnu.org |prachigodbole at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from mpf at gcc dot gnu.org --- Prachi, could you confirm your patch in... https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00229.html is the fix for these. The patch to fix them happens to have crossed over with being reported.
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2017-03-06 CC||mpf at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |mpf at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #2 from mpf at gcc dot gnu.org --- I'll take a look shortly. It may be a day to get initial feedback so I can build tools and check dumps and RISCV description in GCC.
[Bug target/78660] [7 Regression] 7.0 bootstrap fail on mips64el-unknow-linux: configure-stage2-target-libgcc' failed
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78660 mpf at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #21 from mpf at gcc dot gnu.org --- Bootstrap now succeeds.
[Bug target/78176] [MIPS] miscompiles ldxc1 with large pointers on 32-bits
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176 mpf at gcc dot gnu.org changed: What|Removed |Added Known to work||7.0 Target Milestone|--- |8.0 Known to fail||6.2.0 --- Comment #21 from mpf at gcc dot gnu.org --- Moving target to GCC 8.0 albeit that it may not be solvable. Mitigated in GCC 7.
[Bug target/78012] -mfpxx produces assembly code using odd FP registers on MIPS
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78012 mpf at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||7.0 Resolution|--- |FIXED Known to fail|7.0 | --- Comment #7 from mpf at gcc dot gnu.org --- This is fixed for GCC 7. It may need a backport to GCC 6 though.
[Bug target/79914] VEC_SELECT bugs in mips patterns
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79914 mpf at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Known to work||7.0 Resolution|--- |FIXED --- Comment #3 from mpf at gcc dot gnu.org --- Fixed by r245911.
[Bug target/79473] -mload-store-pairs option is not documented in invoke.texi
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79473 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED CC||mpf at gcc dot gnu.org Known to work||7.0 Resolution|--- |FIXED --- Comment #2 from mpf at gcc dot gnu.org --- Fixed on trunk.
[Bug rtl-optimization/66669] FAIL: gcc.dg/loop-8.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=9 mpf at gcc dot gnu.org changed: What|Removed |Added Status|NEW |RESOLVED CC||mpf at gcc dot gnu.org Known to work||7.0 Resolution|--- |FIXED --- Comment #5 from mpf at gcc dot gnu.org --- This issue is fixed for all reported targets by skipping the test.
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 --- Comment #5 from mpf at gcc dot gnu.org --- I think there are some relatively serious issues in the riscv backend here. What I understand so far: 1) Only floating point modes are allowed in FPRs 2) There is an alternative in the movqi_internal pattern that allows FPR to GPR moves but from (1) this is impossible. The presence of this alternative skews the error from LRA. Removing these alternatives changes the LRA error to max reloads reached. 3) The preferred reload class for riscv is always FP_REGS if the requested class is ALL_REGS but there is no check to ensure the mode of the expression being reloaded can go in FP_REGS. 4) LRA always generates reloads as ALL_REGS to start with and refines 5) Therefore a QImode reload can end up trying to reload through an FP_REG which is completely impossible. I expect similar FPR-GPR alternatives in other integer move patterns may also be bugs. Spilling non-eliminable hard regs: 8 Creating newreg=182, assigning class FP_REGS to slow mem r182 Creating newreg=183, assigning class FP_REGS to slow mem r183 194: r180:QI=r183:QI REG_DEAD r102:SI Inserting slow mem reload before: 196: r182:SI=[frame:SI-0x108] 197: r183:QI=r182:SI#0 Simply removing the preferred_reload_class hook allows LRA to succeed but presumably it is desirable to try and reload through FPRs for float modes where possible. I suspect the preferred_reload_class should be: static reg_class_t riscv_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass) { machine_mode mode = GET_MODE (x); if ((GET_MODE_CLASS (mode) == MODE_FLOAT || GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT) && reg_class_subset_p (FP_REGS, rclass)) return FP_REGS; return reg_class_subset_p (GR_REGS, rclass) ? GR_REGS : rclass; } But it needs a riscv maintainer to comment on what they were trying to achieve and the constraints on movqi_internal, movhi_internal, movsi_internal need checking. From my read through of the backend I believe all the alternatives involving 'f' on these are invalid but I can't be sure.
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 --- Comment #7 from mpf at gcc dot gnu.org --- The same fix will resolve soft-float as well I think. In the soft-float case I believe it is reasonably logical that preferred_reload_class is wrong as there are no registers in FPR_REGS available at all.
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 --- Comment #10 from mpf at gcc dot gnu.org --- (In reply to Kito Cheng from comment #8) > [1] > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c > index 89567f7..148967b 100644 > --- a/gcc/config/riscv/riscv.c > +++ b/gcc/config/riscv/riscv.c > @@ -3581,10 +3581,6 @@ riscv_hard_regno_mode_ok_p (unsigned int regno, enum > machine_mode mode) >if (!FP_REG_P (regno + nregs - 1)) > return false; > > - if (GET_MODE_CLASS (mode) != MODE_FLOAT > - && GET_MODE_CLASS (mode) != MODE_COMPLEX_FLOAT) > - return false; > - >/* Only use callee-saved registers if a potential callee is guaranteed > to spill the requisite width. */ >if (GET_MODE_UNIT_SIZE (mode) > UNITS_PER_FP_REG > @@ -3634,7 +3630,7 @@ riscv_class_max_nregs (reg_class_t rclass, enum > machine_mode mode) > static reg_class_t > riscv_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass) > { > - return reg_class_subset_p (FP_REGS, rclass) ? FP_REGS : > + return reg_class_subset_p (FP_REGS, rclass) && TARGET_HARD_FLOAT ? > FP_REGS : > reg_class_subset_p (GR_REGS, rclass) ? GR_REGS : > rclass; > } I don't think you want to do this really. Allowing integer modes in FPRs can make a real mess and lead to extra cost moving to and from FPRs which can be slow as well as using additional register banks that would not normally be required. I.e. consider code that only uses integer types hitting the FPU registers. Assuming the FPRs can be managed in a lazy context fashion then this means you can introduce additional context switch overhead for non-floating-point processes which is additional waste. Doesn't my original fix work for you? It should just lead to the loads being a different width but not using FPRs; I guess it could break something else though. The problem here is that WORD_REGISTER_OPERATIONS allows a subreg and a reg of the same hard-register to be used without need for sign/zero extension but instead relying on the LOAD_EXTEND_OP rules. The 'true' value is that of the inner mode and there can be loads/stores in that inner-mode elsewhere that expect the full width of the inner-mode to be valid in memory. If you do an output reload in the outer-mode and only store outer-mode-width in memory then any inner mode consumer will get junk in the upper bits. There are of course occasions where this does not matter. In particular that means input reloads could be done in the outer-mode (when that is narrower) as long as and output reloads for the same instruction are done in the inner-mode i.e. keeping memory content consistent but reducing the size of loads. Doing that kind of optimisation and getting it correct is far too invasive for stage 4 and the aspects of the current behavior are necessary for correctness. I recommend that on balance for all targets the current behavior is a reasonable compromise. I have said elsewhere that I am happy to continue working in this area and would welcome any further help to evaluate the effects of further work. EricB has offered his assistance and any additional help would also be good as this issue affects targets in different ways.
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 mpf at gcc dot gnu.org changed: What|Removed |Added Assignee|mpf at gcc dot gnu.org |palmer at dabbelt dot com --- Comment #13 from mpf at gcc dot gnu.org --- (In reply to Palmer Dabbelt from comment #12) > I believe this is a problem in the RISC-V port, so you're welcome to assign > the bug to me. I might not have time to fix this today, this way I won't > lose it. Thanks Palmer, glad I could find something specific to try and resolve this as it could have easily been much harder! Sorry to have exposed a bug so late in the release but I suspect there were other ways to hit this already so it may just have caught it early. Matthew
[Bug target/79912] [7 regression] LRA unable to generate reloads after r245655
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79912 --- Comment #16 from mpf at gcc dot gnu.org --- (In reply to Palmer Dabbelt from comment #15) > Created attachment 40968 [details] > glibc file that loops > > The suggested patch causes an infinate loop while building glibc for RISC-V. > The preprocessed source is attached, but I think the right way to fix this > might be to actually fix the int-in-FP problem in our backend. If I > understand correctly we don't actually need riscv_preferred_reload_class() > but I can instead remove all the integer->FP modes from the mov{q,h,s,d} > patterns. OK, I expect the conditions would need even more fine tuning but... > > With just > > static reg_class_t > riscv_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass) > { > return rclass; > } Yes, or remove the hook entirely of course. I don't think the preferred_reload_class is really achieving anything in its current form anyway as I think it boils down to representing the exact same thing as can be inferred from the register classes that modes are allowed in anyway. I.e. the register class will naturally reduce to the right one because some registers are simply not allowed to hold some modes. I believe, but am not an expert, that this hook is only really necessary for more complex rules but yours are relatively simple. > (which I assume is the same as removing preferred_reload_class()) everything > builds, but I haven't even bothered running the test suite yet. Looking at > the mov patterns I can't actually find any that are illegal. I'm looking > for something like > > [(set (match_operand:QI 0 "nonimmediate_operand" "=f") > (match_operand:QI 1 "move_operand" " r"))] > > which would allow an integer to end up in a FP register, as opposed to It would if an FPR was allowed to hold a register of mode QI but it is not so this alternative will always fail. It was just confusing LRA and diagnosing the true issue was harder because of this being present. Better to remove so it doesn't mislead future debugging efforts. > [(set (match_operand:QI 0 "nonimmediate_operand" "=r") > (match_operand:QI 1 "move_operand" " f"))] > This is equally wrong. It will never succeed just like above but it is describing a situation where a QImode register is in an FPR just like above except it is already in an FPR here rather than moving to one. There is no float-mode involved in this pattern. Did you perhaps just mix up the reference to an FPR regclass with the use of floats? > which from my understanding is legal, since we can store floats in GPRs. I > can understand how the f-operand versions of movqi and movhi don't make any > sense, so I'll get rid of those, but I think the all our others are OK? > > What am I missing? Not much from what I can see, just the mix-up that FPR does not imply float.
[Bug target/80057] typo in mips.opt: Virtualization Application Specific
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80057 mpf at gcc dot gnu.org changed: What|Removed |Added CC||mpf at gcc dot gnu.org --- Comment #3 from mpf at gcc dot gnu.org --- I agree there is an inconsistency here (or several actually but not all can be fixed). We refer to the Virtualization Application-Specific Extension with shortened forms of Virtualization ASE or VZ ASE. Unfortunately the option name -mvirt is not -mvz but I can't fix that. We can fix the description though so it matches terms used elsewhere: Use Virtualization (VZ) instructions. I'll push a patch for it.
[Bug target/80086] ICE: in extract_constrain_insn, at recog.c:2213 for the gcc.dg/vect/slp-perm-6.c test on MIPS
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80086 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #1 from mpf at gcc dot gnu.org --- Same issue as reported against GCC 5 and 6.
[Bug middle-end/79915] ICE in final_scan_insn, at final.c:2982 (could not split insn) on mips* when compiling libstdc++ with -mlong-calls -mno-abicalls
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79915 mpf at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2017-03-31 CC||mpf at gcc dot gnu.org Ever confirmed|0 |1 Known to fail||7.0.1 --- Comment #5 from mpf at gcc dot gnu.org --- This is a bug coming from LRA rematerialisation. The issue is that one or more LEA64 instructions get rematerialized at a point where there are no other spills to resolve. Each LEA64 instruction introduces a new scratch which is a copy of the original scratch but is unallocated. lra_need_for_spills_p says false in this circumstance but is 'wrong'. One option is to simply go round the LRA loop one more time (and ensure that the lra_spill code does not assert because of there being no spills to deal with; it effectively becomes a NOP). There are plenty of alternative solutions which would involve determining whether remat introduced any new scratches and only running the loop again in that situation. We could also optimise to avoid running the lra_spill code as we really need to get to the top of the loop to re-run assignment I believe. It almost seems cleaner to continue round the LRA loop after remat and rely on the single exit point but I assume it was an optimisation to exit immediately after remat if it resolves all remaining spills. Vladimir: Is my description detailed enough for you to understand the issue or should I add dumps as well for a testcase? Is there anything 'safe enough' to do at this point of release? I'm testing my quick fix as below in case it is a suitable solution. diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c index 492fc18..863136d 100644 --- a/gcc/lra-spills.c +++ b/gcc/lra-spills.c @@ -565,7 +565,6 @@ lra_spill (void) /* We do not want to assign memory for former scratches. */ && ! lra_former_scratch_p (i)) pseudo_regnos[n++] = i; - lra_assert (n > 0); pseudo_slots = XNEWVEC (struct pseudo_slot, regs_num); for (i = FIRST_PSEUDO_REGISTER; i < regs_num; i++) { diff --git a/gcc/lra.c b/gcc/lra.c index ed1f062..c906d94 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -2490,8 +2490,6 @@ lra (FILE *f) /* We need full live info -- see the comment above. */ lra_create_live_ranges (lra_reg_spill_p, true); live_p = true; - if (! lra_need_for_spills_p ()) - break; } lra_spill (); /* Assignment of stack slots changes elimination offsets for
[Bug target/80057] typo in mips.opt: Virtualization Application Specific
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80057 --- Comment #4 from mpf at gcc dot gnu.org --- Author: mpf Date: Mon Apr 10 13:44:39 2017 New Revision: 246807 URL: https://gcc.gnu.org/viewcvs?rev=246807&root=gcc&view=rev Log: Update MIPS -mvirt option description gcc/ PR target/80057 * config/mips/mips.opt (-mvirt): Update description. * doc/invoke.texi (-mvirt): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/config/mips/mips.opt trunk/gcc/doc/invoke.texi
[Bug target/80057] typo in mips.opt: Virtualization Application Specific
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80057 mpf at gcc dot gnu.org changed: What|Removed |Added Status|WAITING |RESOLVED Resolution|--- |FIXED --- Comment #5 from mpf at gcc dot gnu.org --- Fixed on trunk.
[Bug target/81803] [7/8 regression] miscompilation at -O1 on mips64el
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81803 mpf at gcc dot gnu.org changed: What|Removed |Added Attachment #42075|0 |1 is obsolete|| --- Comment #14 from mpf at gcc dot gnu.org --- Created attachment 42498 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42498&action=edit Updated fix I'm really sorry for my lack of work on this. Too much going on in day to day work for quite a while now. (In reply to Eric Botcazou from comment #13) > > I can't say that I am 100% convinced yet with my thinking here but I've > > attached an updated version of the original patch with some changes: > > > > * Incorporated Eric's feedback on the original patch to check > > GET_MODE_PRECISION instead of GET_MODE_SIZE for comparing whether a mode is > > strictly narrower > > * Limited the test to word sized inner modes or smaller > > * Limited the test to OP_OUT or OP_INOUT as I can't see any reason why it > > would matter if we do a narrower input reload > > I'm not convinced for the 3rd restriction though, as push_reload treats the > IN and OUT cases exactly the same wrt WORD_REGISTER_OPERATIONS. I don't think the restriction is required for functional correctness but I thought we may as well take advantage of a narrower load in the OP_IN case if we could get away with it. The patch has a serious bug that I started testing but failed to report here. The bracketing was wrong by one level, an updated version is attached.
[Bug target/81803] [7/8 regression] miscompilation at -O1 on mips64el
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81803 --- Comment #16 from mpf at gcc dot gnu.org --- (In reply to Eric Botcazou from comment #15) > > I don't think the restriction is required for functional correctness but I > > thought we may as well take advantage of a narrower load in the OP_IN case > > if we could get away with it. > > I don't think you're guaranteed that this reload will yield a load though. If it ends up as a simple register move though then that shouldn't be harmful either as it would be a WORD_REGISTER_OPERATION. I have no objection to removing the restriction though; it is easier to reason about without the special case.
[Bug target/71657] Wrong code on trunk gcc (std::out_of_range), westmere
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71657 mpf at gcc dot gnu.org changed: What|Removed |Added CC||mpf at gcc dot gnu.org --- Comment #11 from mpf at gcc dot gnu.org --- (In reply to Tom de Vries from comment #10) > Meaning that this PR may still occur for the other archs that define the > target hook: mips and arc. > > Shouldn't we disable this optimization in the common code? Thanks for the cc Tom. Is there any indication of what is going wrong in LRA in the failing cases? Infrastructure improvements suggests there is a known issue to address. Is the x86 case about using a vector register to spill a narrower mode? If so is there some chance of it being subreg related?
[Bug tree-optimization/77654] restrict pointer attribute not preserved with -fprefetch-loop-arrays
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77654 --- Comment #4 from mpf at gcc dot gnu.org --- Author: mpf Date: Fri Sep 23 15:48:01 2016 New Revision: 240439 URL: https://gcc.gnu.org/viewcvs?rev=240439&root=gcc&view=rev Log: Ensure points-to information is maintained for prefetch. gcc/ PR tree-optimization/77654 * tree-ssa-alias.c (issue_prefetch_ref): Add call to duplicate_ssa_name_ptr_info. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-loop-prefetch.c
[Bug tree-optimization/77808] [7 Regression] ICE in duplicate_ssa_name_ptr_info, at tree-ssanames.c:630 starting with r240439
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77808 --- Comment #1 from mpf at gcc dot gnu.org --- Author: mpf Date: Tue Oct 4 15:28:23 2016 New Revision: 240749 URL: https://gcc.gnu.org/viewcvs?rev=240749&root=gcc&view=rev Log: Fix PR tree-optimization/77808 gcc/ PR tree-optimization/77808 * tree-ssa-loop-prefetch.c (issue_prefetch_ref): Check base_addr and addr are different before copying points-to information. gcc/testsuite/ PR tree-optimization/77808 * gcc.dg/tree-ssa/pr77808.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/tree-ssa/pr77808.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-loop-prefetch.c
[Bug target/64569] [4.8/4.9 only] [MIPS] Unable to build soft-float in conjunction with binutils 2.25
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64569 --- Comment #2 from mpf at gcc dot gnu.org --- Author: mpf Date: Thu Feb 26 10:56:09 2015 New Revision: 221001 URL: https://gcc.gnu.org/viewcvs?rev=221001&root=gcc&view=rev Log: Add missing bug number to r221000 PR target/64569 * See r221000 Modified: branches/gcc-4_9-branch/gcc/ChangeLog
[Bug c/65345] ICE with _Generic selection on _Atomic int
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65345 --- Comment #28 from mpf at gcc dot gnu.org --- Author: mpf Date: Tue Aug 9 12:36:18 2016 New Revision: 239278 URL: https://gcc.gnu.org/viewcvs?rev=239278&root=gcc&view=rev Log: MIPS: Use create_tmp_var_raw in mips_atomic_assign_expand_fenv gcc/ PR c/65345 * config/mips/mips.c (mips_atomic_assign_expand_fenv): Use create_tmp_var_raw instead of create_tmp_var. Modified: trunk/gcc/ChangeLog trunk/gcc/config/mips/mips.c
[Bug rtl-optimization/66669] FAIL: gcc.dg/loop-8.c
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=9 --- Comment #3 from mpf at gcc dot gnu.org --- Author: mpf Date: Tue Aug 9 14:36:45 2016 New Revision: 239288 URL: https://gcc.gnu.org/viewcvs?rev=239288&root=gcc&view=rev Log: MIPS: Skip gcc.dg/loop-8.c due to additional invariants gcc/ PR rtl-optimization/9 * gcc.dg/loop-8.c: Skip for MIPS due to extra invariants. Modified: trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/loop-8.c