Message ID | 46F08A58-7790-4D2E-8146-9E6B5A3930B0@me.com |
---|---|
State | New |
Headers | show |
Hi Jake! On Mon, 2016-03-28 16:34:56 -0700, Jake Hamby <jehamby420@me.com> wrote: > Amazingly enough, my patch worked well enough that my NetBSD VAX > kernel built with GCC 5.3 is no longer crashing. I feel pretty good > about what I have so far so here's the complete diff for both the > C++ exception fix and the bad condition codes optimizer bug. It > should be good enough to check into NetBSD current, at least, and I > believe it does fix most, if not all, of the bad code generation > bugs with optimization on VAX. I'd like to suggest to also Cc Matt Thomas <matt@3am-software.com>, at least once the patch is tested with GCC trunk/HEAD/master, instead of 5.3. There isn't probably much of a difference, but you've done substancial and important work here, so let's see it's applicable to upstream GCC. And keep in mind that a ChangeLog entry is also needed. MfG, JBG
On 03/31/2016 08:30 AM, Jan-Benedict Glaw wrote: > Hi Jake! > > On Mon, 2016-03-28 16:34:56 -0700, Jake Hamby <jehamby420@me.com> wrote: >> Amazingly enough, my patch worked well enough that my NetBSD VAX >> kernel built with GCC 5.3 is no longer crashing. I feel pretty good >> about what I have so far so here's the complete diff for both the >> C++ exception fix and the bad condition codes optimizer bug. It >> should be good enough to check into NetBSD current, at least, and I >> believe it does fix most, if not all, of the bad code generation >> bugs with optimization on VAX. > > I'd like to suggest to also Cc Matt Thomas <matt@3am-software.com>, at > least once the patch is tested with GCC trunk/HEAD/master, instead of > 5.3. There isn't probably much of a difference, but you've done > substancial and important work here, so let's see it's applicable to > upstream GCC. > > And keep in mind that a ChangeLog entry is also needed. FWIW, I put this in my gcc-7 queue. jeff
Hi JBG, Thanks for the interest! Unfortunately, I need a few more days to work on this patch to clean it up and fix a few more bugs, then I'll send out a new version to NetBSD port-vax for testing, with ChangeLog entry. Please consider what I sent out earlier to be a work-in-progress at this point. The version I have on my machine is now generating bad code, after trying to change a few "clobbers" to "compares", so I need to fix those bugs and also further clean up some stuff that I know is broken. For example, there's some old code in vax.c marked "#if HOST_BITS_PER_WIDE_INT == 32" and "if (HOST_BITS_PER_WIDE_INT == 32)" that will never be used because HOST_WIDE_INT is now always 64 (in hwint.h). I found another bug in a NetBSD command (/usr/sbin/pkg_info) processing command-line arguments in main() that goes away at "-O0". That bug should be easy to track down considering the small size of the program and that it's failing immediately in main(). There's one more thing that's broken in the VAX backend which I'd *really* like to fix: GCC can't compile many of its own files at -O2, as well as a few other .c files in the NetBSD tree, because it can't expand an insn pattern. The Makefiles have hacks to add "-O0" on VAX, and when I remove that workaround, I get GCC assertion failures, all of the form: /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c: In function 'void maybe_emit_chk_warning(tree, built_in_function)': /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: error: unrecognizable insn: } ^ (insn 295 294 296 25 (set (reg:SI 111) (subreg:SI (mem:DI (plus:SI (mult:SI (reg:SI 109) (const_int 8 [0x8])) (reg/f:SI 45 [ D.85015 ])) [7 *_98+0 S8 A32]) 4)) /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/wide-int.h:799 -1 (nil)) /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: internal compiler error: in extract_insn, at recog.c:2343 0xbd0365 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*) /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:110 0xbd03fa _fatal_insn_not_found(rtx_def const*, char const*, int, char const*) /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:118 0xb92a2d extract_insn(rtx_insn*) /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/recog.c:2343 0x9612cd instantiate_virtual_regs_in_insn /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1598 0x9612cd instantiate_virtual_regs /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1966 0x9612cd execute /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:2015 The failures all seem to be related to trying to read a value from an array of 64-bit values and loading it into a 32-bit register. It seems like there should be a special insn defined for this sort of array access, since VAX has mova* and pusha* variants to set a value from an address plus an index into a byte, word, long, or 64-bit array (it uses movab/pushab, put not the other variants). The addressing modes, constraints, and predicates all get very complicated, and I still need to understand a bit better what is actually required, and what could be simplified and cleaned up. If anyone has suggestions on how to define an instruction that would solve the previous failure, please let me know. Even without a special pattern for the "(plus:SI (mult:SI (reg:SI) (const_int 8)))", it should be able to generate something. It seems like it's expanding something and then the insn that's supposed to go with it isn't matching. I tried adding define_insns for "movstrictsi" and for "truncdisi2", hoping that one of them would solve the "(set (reg:SI (subreg:SI (mem:DI (...)))))" part of the situation, but it didn't help. The "(subreg:SI)" stuff is strange, and I don't understand exactly what GCC is expecting the backend to define. I'll keep working on things and as soon as I have something that I think is in a contributable state and doesn't generate bad code, I'll email it. Best regards, Jake > On Mar 31, 2016, at 07:30, Jan-Benedict Glaw <jbglaw@lug-owl.de> wrote: > > Hi Jake! > > On Mon, 2016-03-28 16:34:56 -0700, Jake Hamby <jehamby420@me.com> wrote: >> Amazingly enough, my patch worked well enough that my NetBSD VAX >> kernel built with GCC 5.3 is no longer crashing. I feel pretty good >> about what I have so far so here's the complete diff for both the >> C++ exception fix and the bad condition codes optimizer bug. It >> should be good enough to check into NetBSD current, at least, and I >> believe it does fix most, if not all, of the bad code generation >> bugs with optimization on VAX. > > I'd like to suggest to also Cc Matt Thomas <matt@3am-software.com>, at > least once the patch is tested with GCC trunk/HEAD/master, instead of > 5.3. There isn't probably much of a difference, but you've done > substancial and important work here, so let's see it's applicable to > upstream GCC. > > And keep in mind that a ChangeLog entry is also needed. > > MfG, JBG > > -- > Jan-Benedict Glaw jbglaw@lug-owl.de +49-172-7608481 > Signature of: Ich hatte in letzter Zeit ein bißchen viel Realitycheck. > the second : Langsam möchte ich mal wieder weiterträumen können. > -- Maximilian Wilhelm (18. Mai 2005, #lug-owl.de)
Cc'ing Matt Thomas who is listed as the vax maintainer; most of the patch should be reviewed by him IMO. If he is no longer active I'd frankly rather deprecate the port rather than invest effort in keeping it running. > Index: gcc/except.c > =================================================================== > RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/except.c,v > retrieving revision 1.3 > diff -u -r1.3 except.c > --- gcc/except.c 23 Mar 2016 15:51:36 -0000 1.3 > +++ gcc/except.c 28 Mar 2016 23:24:40 -0000 > @@ -2288,7 +2288,8 @@ > #endif > { > #ifdef EH_RETURN_HANDLER_RTX > - emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); > + rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); > + RTX_FRAME_RELATED_P (insn) = 1; // XXX FIXME in VAX backend? > #else > error ("__builtin_eh_return not supported on this target"); > #endif This part looks highly suspicious and I think there needs to be further analysis. Bernd
Hi, I apologize for the poor quality of the initial version of the patch that I submitted. I think I may have also mangled it by not disabling the "smart quotes" feature on my Mac before I pasted in the diff from the terminal window. I intentionally did not use gmail for fear of adding word wraps or converting tabs to spaces, but apparently I mangled the patch anyway. I emailed Christos a .tar.gz version separately. Yes, the file you highlighted is definitely not correct and I need to figure out how to fix it properly. For some reason the optimizer is deleting the emit_move_insn() on VAX, while it seems to work on all the other platforms that define EH_RETURN_HANDLER_RTX() and depend on that instruction. So I'm looking into fixing it in gcc/config/vax/something. My next step to try to figure out what's going on is to dump the trees for all the phases when building unwind-dw2.o (the only file where __builtin_eh_return() has to work in GCC when libgcc is compiled in order for C++ exceptions to work) with and without "-O", and figure out when the instruction is being deleted and why. This only affects functions that call __builtin_eh_return() instead of return, but I think cc1plus may also need it to be defined correctly for some code that it generates. My theory is that it has to do with liveness detection and a write into the stack frame being incorrectly seen as updating a local variable, but that could be completely wrong. I was hoping that by cc'ing gcc-patches that somebody more familiar with why some phase of the optimizer might decide to delete this particular insn that copies data into the stack (to overwrite the return address, e.g. to move to EH_RETURN_HANDLER_RTX) immediately before returning. So far I haven't found any actual bugs in GCC that should be fixed. Perhaps it isn't correct to check in a hack like the change to gcc/except.c into netbsd-current except temporarily, until there's a correct fix for that part of the issue, which is what I'd like to figure out. In the meantime, I would highly recommend adding an #ifdef __vax around that line to avoid trouble with the other ports. Now that I think about it, please do not check in the patch to gcc/dist/gcc/except.c without an #ifdef __vax, and certainly this isn't ready to go into GCC mainline. But I think it will be ready with a few more adjustments. The important thing is that I think most, if not all of the necessary fixes will be entirely modifications to VAX-related files that can be locally patched in NetBSD regardless of whether the GCC maintainers accept them or not. To be honest, my hope by sending out my work now, even in such a rough state, would be to try to avoid deprecating the GCC port to VAX, if only because: 1) there doesn't seem to be a compelling reason to remove support for it since it doesn't use any really old code paths that aren't also used by other backends (e.g. m68k and Atmel AVR use cc0, IBM S/390 uses non-IEEE FP formats), so it doesn't seem to be preventing any optimizations or code refactoring elsewhere in GCC that I could see, 2) even though NetBSD could continue to support VAX GCC, I noticed in the ChangeLogs that whenever somebody has made a change to a definition that affects the backends, they're usually very good about updating all of the backends so that they continue to compile, at least. So leaving the VAX backend in the tree would be beneficial from a maintenance standpoint for users of it, 3) the VAX backend is perhaps somewhat useful as a test case for GCC because so many unusual RTX standard instructions were obviously defined *for* it (although x86 defines a lot of them, too), although my interest is personally in preserving an interesting piece of computer history, and for nostalgia purposes. I sent an earlier email to port-vax suggesting that future discussions of this project aren't relevant to gcc-patches, but I did want to get it on a few people's radar on the NetBSD side and try to solicit a bit of help on the questions I had as to how to avoid having to add that hack to gcc/except.c to make the optimizer not delete the insns. All of the other stuff can be worked on in NetBSD-current and avoid bothering the 99% of people who subscribe to gcc-patches who have no interest in the VAX backend. Best regards, Jake > On Apr 1, 2016, at 04:37, Bernd Schmidt <bschmidt@redhat.com> wrote: > > Cc'ing Matt Thomas who is listed as the vax maintainer; most of the patch should be reviewed by him IMO. If he is no longer active I'd frankly rather deprecate the port rather than invest effort in keeping it running. > >> Index: gcc/except.c >> =================================================================== >> RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/except.c,v >> retrieving revision 1.3 >> diff -u -r1.3 except.c >> --- gcc/except.c 23 Mar 2016 15:51:36 -0000 1.3 >> +++ gcc/except.c 28 Mar 2016 23:24:40 -0000 >> @@ -2288,7 +2288,8 @@ >> #endif >> { >> #ifdef EH_RETURN_HANDLER_RTX >> - emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); >> + rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); >> + RTX_FRAME_RELATED_P (insn) = 1; // XXX FIXME in VAX backend? >> #else >> error ("__builtin_eh_return not supported on this target"); >> #endif > > This part looks highly suspicious and I think there needs to be further analysis. > > > Bernd >
On Fri, 2016-04-01 12:06:20 -0700, Jake Hamby <jehamby420@me.com> wrote: > I apologize for the poor quality of the initial version of the patch > that I submitted. I think I may have also mangled it by not Don't apologize! Posting work early enables others to comment on it. GCC is a highly complex beast; nobody will produce a perfectly looking patch on their first try. [...] > To be honest, my hope by sending out my work now, even in such a > rough state, would be to try to avoid deprecating the GCC port to > VAX, if only because: 1) there doesn't seem to be a compelling > reason to remove support for it since it doesn't use any really old > code paths that aren't also used by other backends (e.g. m68k and > Atmel AVR use cc0, IBM S/390 uses non-IEEE FP formats), so it > doesn't seem to be preventing any optimizations or code refactoring > elsewhere in GCC that I could see, 2) even though NetBSD could > continue to support VAX GCC, I noticed in the ChangeLogs that > whenever somebody has made a change to a definition that affects the > backends, they're usually very good about updating all of the > backends so that they continue to compile, at least. So leaving the > VAX backend in the tree would be beneficial from a maintenance > standpoint for users of it, 3) the VAX backend is perhaps somewhat > useful as a test case for GCC because so many unusual RTX standard > instructions were obviously defined *for* it (although x86 defines a > lot of them, too), although my interest is personally in preserving > an interesting piece of computer history, and for nostalgia > purposes. As of now, ther VAX backend isn't near deprecation IMO. There'a maintainer (Matt), who did quite a revamp a few years ago bringing the VAX backend quite forward. I also quite care for that backend and the Build Robot I'm running is primarily(!) running to detect VAX breakages early. (In fact, quite often Matt and I communicate over submitted patches to the VAX backend.) > I sent an earlier email to port-vax suggesting that future > discussions of this project aren't relevant to gcc-patches, but I > did want to get it on a few people's radar on the NetBSD side and > try to solicit a bit of help on the questions I had as to how to > avoid having to add that hack to gcc/except.c to make the optimizer > not delete the insns. All of the other stuff can be worked on in > NetBSD-current and avoid bothering the 99% of people who subscribe > to gcc-patches who have no interest in the VAX backend. You should /for sure/ bother the gcc-patches people! Please keep Cc'ing patches to that mailing list. MfG, JBG
On Thu, 31 Mar 2016, Jake Hamby wrote: > There's one more thing that's broken in the VAX backend which I'd > *really* like to fix: GCC can't compile many of its own files at -O2, as > well as a few other .c files in the NetBSD tree, because it can't expand > an insn pattern. The Makefiles have hacks to add "-O0" on VAX, and when > I remove that workaround, I get GCC assertion failures, all of the form: > > /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c: In function 'void maybe_emit_chk_warning(tree, built_in_function)': > /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: error: unrecognizable insn: > } > ^ > (insn 295 294 296 25 (set (reg:SI 111) > (subreg:SI (mem:DI (plus:SI (mult:SI (reg:SI 109) > (const_int 8 [0x8])) > (reg/f:SI 45 [ D.85015 ])) [7 *_98+0 S8 A32]) 4)) /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/wide-int.h:799 -1 > (nil)) > /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: internal compiler error: in extract_insn, at recog.c:2343 > 0xbd0365 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*) > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:110 > 0xbd03fa _fatal_insn_not_found(rtx_def const*, char const*, int, char const*) > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:118 > 0xb92a2d extract_insn(rtx_insn*) > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/recog.c:2343 > 0x9612cd instantiate_virtual_regs_in_insn > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1598 > 0x9612cd instantiate_virtual_regs > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1966 > 0x9612cd execute > /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:2015 > > The failures all seem to be related to trying to read a value from an > array of 64-bit values and loading it into a 32-bit register. It seems > like there should be a special insn defined for this sort of array > access, since VAX has mova* and pusha* variants to set a value from an > address plus an index into a byte, word, long, or 64-bit array (it uses > movab/pushab, put not the other variants). The addressing modes, > constraints, and predicates all get very complicated, and I still need > to understand a bit better what is actually required, and what could be > simplified and cleaned up. Please note however that this RTL instruction is a memory reference, not an address load. So the suitable hardware instruction would be MOVAQ for an indexed DI mode memory load. If used to set a SI mode subreg at byte number 4 (this would be the second hardware register of a pair a DI mode value is held in) as seen here, it would have to address the immediately preceding register however (so you can't load R0 this way) and it would clobber it. So offhand I think you need an RTL instruction splitter to express this, and then avoid fetching 64 bits worth of data from memory -- for the sake of matching the indexed addressing mode -- where you only need 32 bits. At the hardware instruction level I'd use a scratch register (as with MOVAQ you'd have to waste one anyway) to scale the index and then use MOVAL instead with the modified index. Where no index is used it gets simpler even as you can just bump up the displacement according to the subreg offset. HTH, Maciej
> On Apr 4, 2016, at 07:51, Maciej W. Rozycki <macro@linux-mips.org> wrote: > > On Thu, 31 Mar 2016, Jake Hamby wrote: > >> There's one more thing that's broken in the VAX backend which I'd >> *really* like to fix: GCC can't compile many of its own files at -O2, as >> well as a few other .c files in the NetBSD tree, because it can't expand >> an insn pattern. The Makefiles have hacks to add "-O0" on VAX, and when >> I remove that workaround, I get GCC assertion failures, all of the form: >> >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c: In function 'void maybe_emit_chk_warning(tree, built_in_function)': >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: error: unrecognizable insn: >> } >> ^ >> (insn 295 294 296 25 (set (reg:SI 111) >> (subreg:SI (mem:DI (plus:SI (mult:SI (reg:SI 109) >> (const_int 8 [0x8])) >> (reg/f:SI 45 [ D.85015 ])) [7 *_98+0 S8 A32]) 4)) /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/wide-int.h:799 -1 >> (nil)) >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: internal compiler error: in extract_insn, at recog.c:2343 >> 0xbd0365 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:110 >> 0xbd03fa _fatal_insn_not_found(rtx_def const*, char const*, int, char const*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:118 >> 0xb92a2d extract_insn(rtx_insn*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/recog.c:2343 >> 0x9612cd instantiate_virtual_regs_in_insn >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1598 >> 0x9612cd instantiate_virtual_regs >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1966 >> 0x9612cd execute >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:2015 >> >> The failures all seem to be related to trying to read a value from an >> array of 64-bit values and loading it into a 32-bit register. It seems >> like there should be a special insn defined for this sort of array >> access, since VAX has mova* and pusha* variants to set a value from an >> address plus an index into a byte, word, long, or 64-bit array (it uses >> movab/pushab, put not the other variants). The addressing modes, >> constraints, and predicates all get very complicated, and I still need >> to understand a bit better what is actually required, and what could be >> simplified and cleaned up. > > Please note however that this RTL instruction is a memory reference, not > an address load. So the suitable hardware instruction would be MOVAQ for > an indexed DI mode memory load. If used to set a SI mode subreg at byte > number 4 (this would be the second hardware register of a pair a DI mode > value is held in) as seen here, it would have to address the immediately > preceding register however (so you can't load R0 this way) and it would > clobber it. > > So offhand I think you need an RTL instruction splitter to express this, > and then avoid fetching 64 bits worth of data from memory -- for the sake > of matching the indexed addressing mode -- where you only need 32 bits. > At the hardware instruction level I'd use a scratch register (as with > MOVAQ you'd have to waste one anyway) to scale the index and then use > MOVAL instead with the modified index. Where no index is used it gets > simpler even as you can just bump up the displacement according to the > subreg offset. Thanks for the info! I've discovered a few additional clues which should help, namely the optimizer pass that's introducing the problem. Through process of elimination, I discovered that adding "-fno-tree-ter" will prevent the unrecognizable insn error. Strangely, I can't cause the problem by using "-ftree-ter" and -O0, which seems odd, unless the code is checking directly for a -O flag. Here's an example of a similar line of code (it seems to be triggered by accessing a 64-bit int array embedded inside a struct) that fails w/ -O, but succeeded with the addition of -fno-tree-ter (assembly output with "-dP"): #(insn 682 680 686 (set (reg:DI 0 %r0 [orig:136 D.5219 ] [136]) # (mem:DI (plus:SI (plus:SI (mult:SI (reg/v:SI 11 %r11 [orig:138 j ] [138]) # (const_int 8 [0x8])) # (reg/v/f:SI 6 %r6 [orig:55 dp ] [55])) # (const_int 112 [0x70])) [5 MEM[base: dp_3, index: _569, step: 8, offset: 112B]+0 S8 A32])) /home/netbsd/current/src/sbin/fsck_ffs/pass1.c:345 11 {movdi} # (expr_list:REG_EQUIV (mem:DI (plus:SI (plus:SI (mult:SI (reg/v:SI 11 %r11 [orig:138 j ] [138]) # (const_int 8 [0x8])) # (reg/v/f:SI 6 %r6 [orig:55 dp ] [55])) # (const_int 112 [0x70])) [5 MEM[base: dp_3, index: _569, step: 8, offset: 112B]+0 S8 A32]) # (nil))) movq 112(%r6)[%r11],%r0 # 682 movdi That's a pretty complex instruction: base address + (array offset * 8) plus a constant offset! It's the attempt to transform this into setting a reg:SI from a subreg:SI from the mem:DI that it fails to find a matching insn. I'll definitely look into the areas you mentioned, because I think you're right about where the basic problem is. It sounds like there isn't a bug in the particular optimization pass, but the addressing mode is so complicated that when you change the movq to a movl, you suddenly can't do the array offset correctly in one insn. I also think you're absolutely correct about the need for a scratch register, and that the movaq/movq version would have wasted one anyway. The other problem area I want to fix is why the generated move instruction to overwrite the return address in expand_eh_return() is being deleted by the optimizer (my guess is that it's a bad dead store elimination, but I could be off-base). That's the part I hacked around to get C++ exceptions working for now with the RTX_FRAME_RELATED_P line I added in gcc/except.c: #ifdef EH_RETURN_HANDLER_RTX rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); RTX_FRAME_RELATED_P (insn) = 1; // XXX FIXME in VAX backend? #else That hack shouldn't be necessary, and it introduces other problems (adds unnecessary .cfi metadata and caused "-Os" compiles to fail for me). So there has to be something else going on, especially since other platforms define EH_RETURN_HANDLER_RTX and seem to depend on the same behavior for their __builtin_eh_return(). But then most of those platforms put the return address in a register, or relative to %sp, not %fp. I could easily see some machine-independent part of the code thinking the frame-pointer reference meant it was a local variable store and not important. I'll continue to clean up the diffs that I've been working on, and send out something when I've made more progress. I like the "cc" attr code that I've added to fix the overaggressive elimination of CC0 tests, but the problem is that now it's too conservative, because many insns are defined as calls into C code which may or may not generate insns that set the condition codes. I have a few ideas on how to clean up and refactor that code, but first I had to convince myself that most of what's in there now are actually useful optimizations, and they seem to be. Thanks for the help! -Jake
On Fri, 8 Apr 2016, Jake Hamby wrote: > Thanks for the info! I've discovered a few additional clues which should > help, namely the optimizer pass that's introducing the problem. Through > process of elimination, I discovered that adding "-fno-tree-ter" will > prevent the unrecognizable insn error. Strangely, I can't cause the > problem by using "-ftree-ter" and -O0, which seems odd, unless the code > is checking directly for a -O flag. You can't turn most optimisations on at -O0, you need to globally enable optimisation in the first place -- any -O setting will do, e.g. -O, -Os, etc., as per the GCC manual: "Most optimizations are only enabled if an `-O' level is set on the command line. Otherwise they are disabled, even if individual optimization flags are specified." So to enable a single optimisation only you'll have to use e.g. -O combined with a list of negated -f options to disable this level's optimisation defaults. Yes, I agree this sounds like an awkward UI; I guess it dates back to GCC's early days and nobody bothered to fix it. Maybe we need -Onone or suchlike. > I'll continue to clean up the diffs that I've been working on, and send > out something when I've made more progress. I like the "cc" attr code > that I've added to fix the overaggressive elimination of CC0 tests, but > the problem is that now it's too conservative, because many insns are > defined as calls into C code which may or may not generate insns that > set the condition codes. I have a few ideas on how to clean up and > refactor that code, but first I had to convince myself that most of > what's in there now are actually useful optimizations, and they seem to > be. Good luck! > Thanks for the help! You are welcome! Maciej
On 04/04/2016 08:51 AM, Maciej W. Rozycki wrote: > On Thu, 31 Mar 2016, Jake Hamby wrote: > >> There's one more thing that's broken in the VAX backend which I'd >> *really* like to fix: GCC can't compile many of its own files at -O2, as >> well as a few other .c files in the NetBSD tree, because it can't expand >> an insn pattern. The Makefiles have hacks to add "-O0" on VAX, and when >> I remove that workaround, I get GCC assertion failures, all of the form: >> >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c: In function 'void maybe_emit_chk_warning(tree, built_in_function)': >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: error: unrecognizable insn: >> } >> ^ >> (insn 295 294 296 25 (set (reg:SI 111) >> (subreg:SI (mem:DI (plus:SI (mult:SI (reg:SI 109) >> (const_int 8 [0x8])) >> (reg/f:SI 45 [ D.85015 ])) [7 *_98+0 S8 A32]) 4)) /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/wide-int.h:799 -1 >> (nil)) >> /home/netbsd/current/src/external/gpl3/gcc/dist/gcc/builtins.c:11549:1: internal compiler error: in extract_insn, at recog.c:2343 >> 0xbd0365 _fatal_insn(char const*, rtx_def const*, char const*, int, char const*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:110 >> 0xbd03fa _fatal_insn_not_found(rtx_def const*, char const*, int, char const*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/rtl-error.c:118 >> 0xb92a2d extract_insn(rtx_insn*) >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/recog.c:2343 >> 0x9612cd instantiate_virtual_regs_in_insn >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1598 >> 0x9612cd instantiate_virtual_regs >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:1966 >> 0x9612cd execute >> /home/netbsd/current/src/tools/gcc/../../external/gpl3/gcc/dist/gcc/function.c:2015 >> >> The failures all seem to be related to trying to read a value from an >> array of 64-bit values and loading it into a 32-bit register. It seems >> like there should be a special insn defined for this sort of array >> access, since VAX has mova* and pusha* variants to set a value from an >> address plus an index into a byte, word, long, or 64-bit array (it uses >> movab/pushab, put not the other variants). The addressing modes, >> constraints, and predicates all get very complicated, and I still need >> to understand a bit better what is actually required, and what could be >> simplified and cleaned up. > > Please note however that this RTL instruction is a memory reference, not > an address load. So the suitable hardware instruction would be MOVAQ for > an indexed DI mode memory load. If used to set a SI mode subreg at byte > number 4 (this would be the second hardware register of a pair a DI mode > value is held in) as seen here, it would have to address the immediately > preceding register however (so you can't load R0 this way) and it would > clobber it. > > So offhand I think you need an RTL instruction splitter to express this, > and then avoid fetching 64 bits worth of data from memory -- for the sake > of matching the indexed addressing mode -- where you only need 32 bits. > At the hardware instruction level I'd use a scratch register (as with > MOVAQ you'd have to waste one anyway) to scale the index and then use > MOVAL instead with the modified index. Where no index is used it gets > simpler even as you can just bump up the displacement according to the > subreg offset. Note you shouldn't need an expander for this. That insn is just a 32bit load. I would have expected something to simplify the subreg expression, likely requiring loading the address into a register in the process. Jeff
On 04/01/2016 05:37 AM, Bernd Schmidt wrote: > Cc'ing Matt Thomas who is listed as the vax maintainer; most of the > patch should be reviewed by him IMO. If he is no longer active I'd > frankly rather deprecate the port rather than invest effort in keeping > it running. > >> Index: gcc/except.c >> =================================================================== >> RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/except.c,v >> retrieving revision 1.3 >> diff -u -r1.3 except.c >> --- gcc/except.c 23 Mar 2016 15:51:36 -0000 1.3 >> +++ gcc/except.c 28 Mar 2016 23:24:40 -0000 >> @@ -2288,7 +2288,8 @@ >> #endif >> { >> #ifdef EH_RETURN_HANDLER_RTX >> - emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); >> + rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, >> crtl->eh.ehr_handler); >> + RTX_FRAME_RELATED_P (insn) = 1; // XXX FIXME in VAX backend? >> #else >> error ("__builtin_eh_return not supported on this target"); >> #endif > > This part looks highly suspicious and I think there needs to be further > analysis. Agreed 100%. This is a symptom of a problem elsewhere. jeff
On 03/31/2016 02:29 PM, Jake Hamby wrote: > > The failures all seem to be related to trying to read a value from an > array of 64-bit values and loading it into a 32-bit register. It > seems like there should be a special insn defined for this sort of > array access, since VAX has mova* and pusha* variants to set a value > from an address plus an index into a byte, word, long, or 64-bit > array (it uses movab/pushab, put not the other variants). The > addressing modes, constraints, and predicates all get very > complicated, and I still need to understand a bit better what is > actually required, and what could be simplified and cleaned up. > > If anyone has suggestions on how to define an instruction that would > solve the previous failure, please let me know. Even without a > special pattern for the "(plus:SI (mult:SI (reg:SI) (const_int 8)))", > it should be able to generate something. It seems like it's expanding > something and then the insn that's supposed to go with it isn't > matching. > > I tried adding define_insns for "movstrictsi" and for "truncdisi2", > hoping that one of them would solve the "(set (reg:SI (subreg:SI > (mem:DI (...)))))" part of the situation, but it didn't help. The > "(subreg:SI)" stuff is strange, and I don't understand exactly what > GCC is expecting the backend to define. I'll keep working on things > and as soon as I have something that I think is in a contributable > state and doesn't generate bad code, I'll email it. subregs have two meanings depending on how they're used. If the mode of the subreg is wider than the mode of the inner object, then it's what we often call a paradoxical subreg. Essentially we're referring to the inner object in a wider mode with the additional bits all having undefined values. subregs may also be used when referring to parts of an object. In this case the subreg refers to a 32bit hunk of a 64bit memory object. I suspect something should have simplified that particular subreg before it got into the IL and the compiler tried to recognize it. I would suggest extracting a .i/.ii file which shows this problem and the necessary flags and reporting it as a bug. jeff
On 04/01/2016 01:06 PM, Jake Hamby wrote: > > My theory is that it has to do with liveness detection and a write > into the stack frame being incorrectly seen as updating a local > variable, but that could be completely wrong. I was hoping that by > cc'ing gcc-patches that somebody more familiar with why some phase of > the optimizer might decide to delete this particular insn that copies > data into the stack (to overwrite the return address, e.g. to move to > EH_RETURN_HANDLER_RTX) immediately before returning. Dead store elimination is the most likely candidate. It "knows" that stores into the local frame are dead when the function returns and uses that information to eliminate such stores. You may just need to set the volatile flag on the MEM when you generate it in your backend. For example see config/pa/pa.c::pa_eh_return_handler_rtx. Jeff
On Tue, 26 Apr 2016, Jeff Law wrote: > > So offhand I think you need an RTL instruction splitter to express this, > > and then avoid fetching 64 bits worth of data from memory -- for the sake > > of matching the indexed addressing mode -- where you only need 32 bits. > > At the hardware instruction level I'd use a scratch register (as with > > MOVAQ you'd have to waste one anyway) to scale the index and then use > > MOVAL instead with the modified index. Where no index is used it gets > > simpler even as you can just bump up the displacement according to the > > subreg offset. > Note you shouldn't need an expander for this. > > That insn is just a 32bit load. I would have expected something to simplify > the subreg expression, likely requiring loading the address into a register in > the process. Hmm, producing a MOVAQ/MOVL sequence (rather than fiddling with the index register) will ensure any increment/decrement mode works just fine. This observation also makes me agree with you in that we should always just load the result of the original address expression somewhere; likely a register, but on a VAX it could well be memory (though the indirect address mode is not offsettable; not in the sense perhaps needed here), so maybe we don't have to restrict that. Maciej
Index: gcc/except.c =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/except.c,v retrieving revision 1.3 diff -u -r1.3 except.c --- gcc/except.c 23 Mar 2016 15:51:36 -0000 1.3 +++ gcc/except.c 28 Mar 2016 23:24:40 -0000 @@ -2288,7 +2288,8 @@ #endif { #ifdef EH_RETURN_HANDLER_RTX - emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); + rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler); + RTX_FRAME_RELATED_P (insn) = 1; // XXX FIXME in VAX backend? #else error ("__builtin_eh_return not supported on this target"); #endif Index: gcc/config/vax/builtins.md =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/builtins.md,v retrieving revision 1.5 diff -u -r1.5 builtins.md --- gcc/config/vax/builtins.md 24 Jan 2016 09:43:34 -0000 1.5 +++ gcc/config/vax/builtins.md 28 Mar 2016 23:24:40 -0000 @@ -50,7 +50,8 @@ (ctz:SI (match_operand:SI 1 "general_operand" "nrmT"))) (set (cc0) (match_dup 0))] "" - "ffs $0,$32,%1,%0") + "ffs $0,$32,%1,%0" + [(set_attr "cc" "clobber")]) (define_expand "sync_lock_test_and_set<mode>" [(set (match_operand:VAXint 0 "nonimmediate_operand" "=&g") @@ -88,7 +89,8 @@ (match_dup 1)) (const_int 1))])] "" - "jbssi %1,%0,%l2") + "jbssi %1,%0,%l2" + [(set_attr "cc" "none")]) (define_insn "jbbssihi" [(parallel @@ -105,7 +107,8 @@ (match_dup 1)) (const_int 1))])] "" - "jbssi %1,%0,%l2") + "jbssi %1,%0,%l2" + [(set_attr "cc" "none")]) (define_insn "jbbssisi" [(parallel @@ -122,8 +125,8 @@ (match_dup 1)) (const_int 1))])] "" - "jbssi %1,%0,%l2") - + "jbssi %1,%0,%l2" + [(set_attr "cc" "none")]) (define_expand "sync_lock_release<mode>" [(set (match_operand:VAXint 0 "memory_operand" "+m") @@ -160,7 +163,8 @@ (match_dup 1)) (const_int 0))])] "" - "jbcci %1,%0,%l2") + "jbcci %1,%0,%l2" + [(set_attr "cc" "none")]) (define_insn "jbbccihi" [(parallel @@ -177,7 +181,8 @@ (match_dup 1)) (const_int 0))])] "" - "jbcci %1,%0,%l2") + "jbcci %1,%0,%l2" + [(set_attr "cc" "none")]) (define_insn "jbbccisi" [(parallel @@ -194,4 +199,5 @@ (match_dup 1)) (const_int 0))])] "" - "jbcci %1,%0,%l2") + "jbcci %1,%0,%l2" + [(set_attr "cc" "none")]) Index: gcc/config/vax/elf.h =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/elf.h,v retrieving revision 1.6 diff -u -r1.6 elf.h --- gcc/config/vax/elf.h 23 Mar 2016 15:51:37 -0000 1.6 +++ gcc/config/vax/elf.h 28 Mar 2016 23:24:40 -0000 @@ -26,7 +26,7 @@ #define REGISTER_PREFIX "%" #define REGISTER_NAMES \ { "%r0", "%r1", "%r2", "%r3", "%r4", "%r5", "%r6", "%r7", \ - "%r8", "%r9", "%r10", "%r11", "%ap", "%fp", "%sp", "%pc", } + "%r8", "%r9", "%r10", "%r11", "%ap", "%fp", "%sp", "%pc", "%psw", } #undef SIZE_TYPE #define SIZE_TYPE "long unsigned int" @@ -45,18 +45,8 @@ count pushed by the CALLS and before the start of the saved registers. */ #define INCOMING_FRAME_SP_OFFSET 0 -/* Offset from the frame pointer register value to the top of the stack. */ -#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0 - -/* We use R2-R5 (call-clobbered) registers for exceptions. */ -#define EH_RETURN_DATA_REGNO(N) ((N) < 4 ? (N) + 2 : INVALID_REGNUM) - -/* Place the top of the stack for the DWARF2 EH stackadj value. */ -#define EH_RETURN_STACKADJ_RTX \ - gen_rtx_MEM (SImode, \ - plus_constant (Pmode, \ - gen_rtx_REG (Pmode, FRAME_POINTER_REGNUM),\ - -4)) +/* We use R2-R3 (call-clobbered) registers for exceptions. */ +#define EH_RETURN_DATA_REGNO(N) ((N) < 2 ? (N) + 2 : INVALID_REGNUM) /* Simple store the return handler into the call frame. */ #define EH_RETURN_HANDLER_RTX \ @@ -66,10 +56,6 @@ 16)) -/* Reserve the top of the stack for exception handler stackadj value. */ -#undef STARTING_FRAME_OFFSET -#define STARTING_FRAME_OFFSET -4 - /* The VAX wants no space between the case instruction and the jump table. */ #undef ASM_OUTPUT_BEFORE_CASE_LABEL #define ASM_OUTPUT_BEFORE_CASE_LABEL(FILE, PREFIX, NUM, TABLE) Index: gcc/config/vax/vax-protos.h =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/vax-protos.h,v retrieving revision 1.5 diff -u -r1.5 vax-protos.h --- gcc/config/vax/vax-protos.h 23 Mar 2016 21:09:04 -0000 1.5 +++ gcc/config/vax/vax-protos.h 28 Mar 2016 23:24:40 -0000 @@ -28,9 +28,9 @@ extern const char *rev_cond_name (rtx); extern void print_operand_address (FILE *, rtx); extern void print_operand (FILE *, rtx, int); -extern void vax_notice_update_cc (rtx, rtx); +extern void vax_notice_update_cc (rtx, rtx_insn *insn); extern void vax_expand_addsub_di_operands (rtx *, enum rtx_code); -extern bool vax_decomposed_dimode_operand_p (rtx, rtx); +/* extern bool vax_decomposed_dimode_operand_p (rtx, rtx); */ extern const char * vax_output_int_move (rtx, rtx *, machine_mode); extern const char * vax_output_int_add (rtx, rtx *, machine_mode); extern const char * vax_output_int_subtract (rtx, rtx *, machine_mode); Index: gcc/config/vax/vax.c =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/vax.c,v retrieving revision 1.15 diff -u -r1.15 vax.c --- gcc/config/vax/vax.c 24 Mar 2016 04:27:29 -0000 1.15 +++ gcc/config/vax/vax.c 28 Mar 2016 23:24:40 -0000 @@ -1,4 +1,4 @@ -/* Subroutines for insn-output.c for VAX. +/* Subroutines used for code generation on VAX. Copyright (C) 1987-2015 Free Software Foundation, Inc. This file is part of GCC. @@ -191,15 +191,24 @@ vax_expand_prologue (void) { int regno, offset; - int mask = 0; + unsigned int mask = 0; HOST_WIDE_INT size; rtx insn; - offset = 20; - for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++) - if (df_regs_ever_live_p (regno) && !call_used_regs[regno]) + offset = 20; // starting offset + + /* We only care about r2 to r11 here. AP, FP, and SP are saved by CALLS. + Always save r2 and r3 when eh_return is called, to reserve space for + the stack unwinder to update them in the stack frame on exceptions. + r0 and r1 are always available for function return values and are + also used by C++. */ + + unsigned int testbit = (1 << 2); + for (regno = 2; regno < VAX_AP_REGNUM; regno++, testbit <<= 1) + if ((df_regs_ever_live_p (regno) && !call_used_regs[regno]) + || (crtl->calls_eh_return /* && regno >= 2 */ && regno < 4)) { - mask |= 1 << regno; + mask |= testbit; offset += 4; } @@ -228,20 +237,16 @@ insn = emit_insn (gen_blockage ()); RTX_FRAME_RELATED_P (insn) = 1; -#ifdef notyet - /* - * We can't do this, the dwarf code asserts and we don't have yet a - * way to get to the psw - */ vax_add_reg_cfa_offset (insn, 4, gen_rtx_REG (Pmode, PSW_REGNUM)); -#endif vax_add_reg_cfa_offset (insn, 8, arg_pointer_rtx); vax_add_reg_cfa_offset (insn, 12, frame_pointer_rtx); vax_add_reg_cfa_offset (insn, 16, pc_rtx); - offset = 20; - for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++) - if (mask & (1 << regno)) + offset = 20; // reset to starting value. + + testbit = (1 << 2); // reset to starting bit for r2. + for (regno = 2; regno < VAX_AP_REGNUM; regno++, testbit <<= 1) + if (mask & testbit) { vax_add_reg_cfa_offset (insn, offset, gen_rtx_REG (SImode, regno)); offset += 4; @@ -1131,61 +1136,51 @@ /* Worker function for NOTICE_UPDATE_CC. */ void -vax_notice_update_cc (rtx exp, rtx insn ATTRIBUTE_UNUSED) +vax_notice_update_cc (rtx exp, rtx_insn *insn) { + attr_cc cc = get_attr_cc (insn); + if (GET_CODE (exp) == SET) { if (GET_CODE (SET_SRC (exp)) == CALL) CC_STATUS_INIT; - else if (GET_CODE (SET_DEST (exp)) != ZERO_EXTRACT - && GET_CODE (SET_DEST (exp)) != PC) + else if (GET_CODE (SET_DEST (exp)) != PC + && cc == CC_COMPARE) { - cc_status.flags = 0; - /* The integer operations below don't set carry or + /* Some of the integer operations don't set carry or set it in an incompatible way. That's ok though as the Z bit is all we need when doing unsigned comparisons on the result of these insns (since they're always with 0). Set CC_NO_OVERFLOW to generate the correct unsigned branches. */ - switch (GET_CODE (SET_SRC (exp))) - { - case NEG: - if (GET_MODE_CLASS (GET_MODE (exp)) == MODE_FLOAT) - break; - case AND: - case IOR: - case XOR: - case NOT: - case CTZ: - case MEM: - case REG: - cc_status.flags = CC_NO_OVERFLOW; - break; - default: - break; - } + cc_status.flags = CC_NO_OVERFLOW; cc_status.value1 = SET_DEST (exp); cc_status.value2 = SET_SRC (exp); } + else if (cc != CC_NONE) + CC_STATUS_INIT; } else if (GET_CODE (exp) == PARALLEL && GET_CODE (XVECEXP (exp, 0, 0)) == SET) { - if (GET_CODE (SET_SRC (XVECEXP (exp, 0, 0))) == CALL) + rtx exp0 = XVECEXP (exp, 0, 0); + if (GET_CODE (SET_SRC (exp0)) == CALL) CC_STATUS_INIT; - else if (GET_CODE (SET_DEST (XVECEXP (exp, 0, 0))) != PC) + else if (GET_CODE (SET_DEST (exp0)) != PC + && cc == CC_COMPARE) { - cc_status.flags = 0; - cc_status.value1 = SET_DEST (XVECEXP (exp, 0, 0)); - cc_status.value2 = SET_SRC (XVECEXP (exp, 0, 0)); + cc_status.flags = CC_NO_OVERFLOW; + cc_status.value1 = SET_DEST (exp0); + cc_status.value2 = SET_SRC (exp0); } - else + else if (cc != CC_NONE) /* PARALLELs whose first element sets the PC are aob, sob insns. They do change the cc's. */ CC_STATUS_INIT; } - else + else if (cc != CC_NONE) CC_STATUS_INIT; + if (cc_status.value1 && REG_P (cc_status.value1) && cc_status.value2 && reg_overlap_mentioned_p (cc_status.value1, cc_status.value2)) @@ -1909,12 +1904,20 @@ return true; if (indirectable_address_p (x, strict, false)) return true; - xfoo0 = XEXP (x, 0); - if (MEM_P (x) && indirectable_address_p (xfoo0, strict, true)) - return true; - if ((GET_CODE (x) == PRE_DEC || GET_CODE (x) == POST_INC) - && BASE_REGISTER_P (xfoo0, strict)) - return true; + /* Note: avoid calling XEXP until needed. It may not be a valid type. + This fixes an assertion failure when RTX checking is enabled. */ + if (MEM_P (x)) + { + xfoo0 = XEXP (x, 0); + if (indirectable_address_p (xfoo0, strict, true)) + return true; + } + if (GET_CODE (x) == PRE_DEC || GET_CODE (x) == POST_INC) + { + xfoo0 = XEXP (x, 0); + if (BASE_REGISTER_P (xfoo0, strict)) + return true; + } return false; } @@ -2366,6 +2369,9 @@ : (int_size_in_bytes (type) + 3) & ~3); } +#if 0 +/* This is commented out because the only usage of it was the buggy + 32-to-64-bit peephole optimizations that have been commented out. */ bool vax_decomposed_dimode_operand_p (rtx lo, rtx hi) { @@ -2416,3 +2422,4 @@ return rtx_equal_p(lo, hi) && lo_offset + 4 == hi_offset; } +#endif Index: gcc/config/vax/vax.h =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/vax.h,v retrieving revision 1.7 diff -u -r1.7 vax.h --- gcc/config/vax/vax.h 23 Mar 2016 15:51:37 -0000 1.7 +++ gcc/config/vax/vax.h 28 Mar 2016 23:24:40 -0000 @@ -120,13 +120,17 @@ The hardware registers are assigned numbers for the compiler from 0 to just below FIRST_PSEUDO_REGISTER. All registers that the compiler knows about must be given numbers, - even those that are not normally considered general registers. */ -#define FIRST_PSEUDO_REGISTER 16 + even those that are not normally considered general registers. + This includes PSW, which the VAX backend did not originally include. */ +#define FIRST_PSEUDO_REGISTER 17 + +/* For compatibility, DWARF_FRAME_REGISTERS must still be 16. */ +#define DWARF_FRAME_REGISTERS 16 /* 1 for registers that have pervasive standard uses and are not available for the register allocator. - On the VAX, these are the AP, FP, SP and PC. */ -#define FIXED_REGISTERS {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1} + On the VAX, these are the AP, FP, SP, PC, and PSW. */ +#define FIXED_REGISTERS {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1} /* 1 for registers not available across function calls. These must include the FIXED_REGISTERS and also any @@ -134,7 +138,7 @@ The latter must include the registers where values are returned and the register where structure-value addresses are passed. Aside from that, you can include as many other registers as you like. */ -#define CALL_USED_REGISTERS {1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1} +#define CALL_USED_REGISTERS {1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1} /* Return number of consecutive hard regs needed starting at reg REGNO to hold something of mode MODE. @@ -169,12 +173,12 @@ /* Base register for access to local variables of the function. */ #define FRAME_POINTER_REGNUM VAX_FP_REGNUM -/* Offset from the frame pointer register value to the top of stack. */ -#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0 - /* Base register for access to arguments of the function. */ #define ARG_POINTER_REGNUM VAX_AP_REGNUM +/* Offset from the argument pointer register value to the CFA. */ +#define ARG_POINTER_CFA_OFFSET(FNDECL) 0 + /* Register in which static-chain is passed to a function. */ #define STATIC_CHAIN_REGNUM 0 @@ -395,9 +399,9 @@ allocation. */ #define REGNO_OK_FOR_INDEX_P(regno) \ - ((regno) < FIRST_PSEUDO_REGISTER || reg_renumber[regno] >= 0) + ((regno) <= VAX_PC_REGNUM || reg_renumber[regno] >= 0) #define REGNO_OK_FOR_BASE_P(regno) \ - ((regno) < FIRST_PSEUDO_REGISTER || reg_renumber[regno] >= 0) + ((regno) <= VAX_PC_REGNUM || reg_renumber[regno] >= 0) /* Maximum number of registers that can appear in a valid memory address. */ @@ -424,11 +428,11 @@ /* Nonzero if X is a hard reg that can be used as an index or if it is a pseudo reg. */ -#define REG_OK_FOR_INDEX_P(X) 1 +#define REG_OK_FOR_INDEX_P(X) ((regno) != VAX_PSW_REGNUM) /* Nonzero if X is a hard reg that can be used as a base reg or if it is a pseudo reg. */ -#define REG_OK_FOR_BASE_P(X) 1 +#define REG_OK_FOR_BASE_P(X) ((regno) != VAX_PSW_REGNUM) #else @@ -508,12 +512,6 @@ #define NOTICE_UPDATE_CC(EXP, INSN) \ vax_notice_update_cc ((EXP), (INSN)) - -#define OUTPUT_JUMP(NORMAL, FLOAT, NO_OV) \ - { if (cc_status.flags & CC_NO_OVERFLOW) \ - return NO_OV; \ - return NORMAL; \ - } /* Control the assembler format that we output. */ @@ -548,7 +546,7 @@ #define REGISTER_PREFIX "" #define REGISTER_NAMES \ { "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", \ - "r8", "r9", "r10", "r11", "ap", "fp", "sp", "pc", } + "r8", "r9", "r10", "r11", "ap", "fp", "sp", "pc", "psw", } /* This is BSD, so it wants DBX format. */ Index: gcc/config/vax/vax.md =================================================================== RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/vax/vax.md,v retrieving revision 1.11 diff -u -r1.11 vax.md --- gcc/config/vax/vax.md 23 Mar 2016 15:51:37 -0000 1.11 +++ gcc/config/vax/vax.md 28 Mar 2016 23:24:40 -0000 @@ -62,6 +62,24 @@ (include "constraints.md") (include "predicates.md") +;; Condition code attribute. This is used to track the meaning of flags +;; after an insn executes. Often, explicit compare insns can be deleted +;; if a conditional branch can be generated to use an existing CC value. +;; This is for the CC0 mechanism, also used by the m68k and avr backends. +;; +;; On VAX, the default "cc" is "clobber". This means that the Z and N +;; flags can not be used to compare operand 0 to numeric 0 later. If the +;; "cc" is "compare" then the Z and N flags can be used in this way. +;; Unsigned integer comparisons are handled correctly by always setting +;; CC_NO_OVERFLOW in vax_notice_update_cc, so that the C flag is ignored, +;; and unsigned comparisons only use equality/inequality testing with Z. +;; A "cc" of "none" means that Z and N will never be modified by this insn. + +(define_attr "cc" "none,compare,clobber" (const_string "clobber")) + +;; Comparison instructions. tst and cmp set the flags based on (%0 < %1), +;; so the "cc" attr is set to "clobber", unless operand 1 is 0. + (define_insn "*cmp<mode>" [(set (cc0) (compare (match_operand:VAXint 0 "nonimmediate_operand" "nrmT,nrmT") @@ -69,7 +87,8 @@ "" "@ tst<VAXint:isfx> %0 - cmp<VAXint:isfx> %0,%1") + cmp<VAXint:isfx> %0,%1" + [(set_attr "cc" "compare,clobber")]) (define_insn "*cmp<mode>" [(set (cc0) @@ -78,7 +97,12 @@ "" "@ tst<VAXfp:fsfx> %0 - cmp<VAXfp:fsfx> %0,%1") + cmp<VAXfp:fsfx> %0,%1" + [(set_attr "cc" "compare,clobber")]) + +;; The bit instruction has different behavior from the last two insns, +;; performing a logical and on %0 and %1, then comparing the result to 0. +;; We can't make any assumptions about operand 0 itself from this test. (define_insn "*bit<mode>" [(set (cc0) @@ -86,7 +110,8 @@ (match_operand:VAXint 1 "general_operand" "nrmT")) (const_int 0)))] "" - "bit<VAXint:isfx> %0,%1") + "bit<VAXint:isfx> %0,%1" + [(set_attr "cc" "clobber")]) ;; The VAX has no sCOND insns. It does have add/subtract with carry ;; which could be used to implement the sltu and sgeu patterns. However, @@ -96,26 +121,27 @@ ;; and has been deleted. +;; The mov instruction sets Z and N flags by comparing operand 0 to 0. +;; The V flag is cleared to 0, and C is unchanged. clr has the same +;; semantics, but N is always cleared and Z is always set (0 == 0). (define_insn "mov<mode>" [(set (match_operand:VAXfp 0 "nonimmediate_operand" "=g,g") (match_operand:VAXfp 1 "general_operand" "G,gF"))] "" "@ clr<VAXfp:fsfx> %0 - mov<VAXfp:fsfx> %1,%0") - -;; Some VAXen don't support this instruction. -;;(define_insn "movti" -;; [(set (match_operand:TI 0 "general_operand" "=g") -;; (match_operand:TI 1 "general_operand" "g"))] -;; "" -;; "movh %1,%0") + mov<VAXfp:fsfx> %1,%0" + [(set_attr "cc" "compare")]) +;; The variety of possible returns from vax_output_int_move() is such that +;; it's best to consider the flags to be clobbered, rather than to expect +;; any particular compare-to-zero behavior from the sequence. (define_insn "movdi" [(set (match_operand:DI 0 "nonimmediate_operand" "=g") (match_operand:DI 1 "general_operand" "g"))] "" - "* return vax_output_int_move (insn, operands, DImode);") + "* return vax_output_int_move (insn, operands, DImode);" + [(set_attr "cc" "clobber")]) ;; The VAX move instructions have space-time tradeoffs. On a MicroVAX