Patches to fix GCC’s C++ exception handling on NetBSD/VAX

Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

Hi all,

For several years I’ve been eager to find the time to fix the bugs in C++ exceptions on VAX to get them working on NetBSD, because they’ve been broken for many years and it looked like only a few changes were needed to get them working. Without C++ exceptions, the NetBSD test suite can’t be run. The good news is that I was able to fix all the bugs in the VAX machine description to make C++ exceptions work in GCC 4.8.5 (version unimportant). I wrote a blog post explaining the bugs, with patches:

> http://jakehamby.com/?p=7

Here’s a short summary, with the diffs in text form at the end of this email.

1) Replace #define FRAME_POINTER_CFA_OFFSET(FNDECL) 0 with #define ARG_POINTER_CFA_OFFSET(FNDECL) 0 in gcc/config/vax/elf.h and gcc/config/vax/vax.h. This changes the definition of __builtin_dwarf_cfa() to return %ap instead of %fp, which correctly points to CFA. Previously, the stack unwinder was crashing in _Unwind_RaiseException() trying to follow bad pointers from the initial CFA.

2) Define EH_RETURN_DATA_REGNO(N) to include only R2 and R3 (instead of R2-R5) and add code to vax_expand_prologue() in gcc/config/vax/vax.c to add R2-R3 to the procedure entry mask but only if crtl->calls_eh_return is set. This fixes a crash when the stack unwinder tried to write values to R2 and R3 in the previous stack frame via __builtin_eh_return_data_regno (0) and __builtin_eh_return_data_regno (1).

3) Removed definitions of EH_RETURN_STACKADJ_RTX and STARTING_FRAME_OFFSET from gcc/config/vax/elf.h. It’s not necessary to remember the stack adjustment or to waste four bytes on every stack frame for a value that’s not needed. Also remove the suspicious changes in gcc/config/vax/vax.md to the definitions of call_pop and call_value regarding DW_CFA_GNU_args_size and EH unwinding. I reverted to the previous versions from an older version of GCC, adding a few useful comments that had been removed.

4) The last bug is the one I understand the least. I’m hoping someone reading this can implement a correct fix. What I was seeing after making all the previous changes to fix the other bugs is that my test program failed to catch any exceptions, but instead returned normally to the original return path.

Investigation revealed that GCC was correctly generating the necessary move instruction to copy the second parameter passed to __builtin_eh_return() into the return address, because EH_RETURN_HANDLER_RTX had been defined correctly in config/vax/elf.h. Here’s what the call looks like in gcc/except.c:

#ifdef EH_RETURN_HANDLER_RTX
      rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler);
#else
      error ("__builtin_eh_return not supported on this target");
#endif

The problem was that the optimizer is deleting the final move instruction when I compile with -O or higher. The assembly code at -O0 (no optimization) generated for the __builtin_eh_return() call at the end of _Unwind_RaiseException() looked like:

	calls $2,_Unwind_DebugHook
	movl -12(%fp),%r1
	movl %r1,16(%fp)
	ret
	.cfi_endproc

But then when I compiled with -O1 or -O2, all I saw was:

	calls $2,_Unwind_DebugHook
	ret
	.cfi_endproc

This was a mystery for me and I don’t know enough about how the final peephole optimizer works to really track down why it thinks it can remove the move call to store the previous return address. My workaround was to add a call to RTX_FRAME_RELATED_P (insn) = 1; after the emit_move_insn() in gcc/except.c, which was used in vax_expand_prologue() to mark the procedure entry mask.

By making this change, the optimizer no longer removes the call to write the value to the previous stack pointer, but it adds an extra line of .cfi exception info, which seems unnecessary since the code is immediately going to return from the call and any adjustment made by the DWARF stack unwinder will already have been done. Here’s what the optimized code looks like with the patch (%r6 had been loaded earlier):

	calls $2,_Unwind_DebugHook
	movl %r6,16(%fp)
	.cfi_offset 6, -36
	ret
	.cfi_endproc

With that final change, C++ exception handling now finally works on NetBSD/vax, and I was able to successfully run the vast majority of the tests in the ATF testsuite, which had been completely inaccessible when I started due to both atf-run and atf-report immediately dumping core due to the bad pointers that I fixed. Now I have a bunch of new bugs to track down fixes for, but I think this was the hardest set of problems that needed to be solved to bring NetBSD on VAX up to the level of the other NetBSD ports.

Here are the diffs I have so far. They should apply to any recent version of GCC (tested on GCC 4.8.5). With the exception of the hack to gcc/except.c, the other diffs are ready to submit to NetBSD as well as to upstream GCC. The fix I’d like to see for the final problem I discovered of the emit_move_insn() being deleted by the optimizer would be another patch to one of the files in the gcc/config/vax directory to explain to the optimizer that writing to 16(%fp) is important and not something to be deleted from the epilogue (perhaps it thinks it’s writing to a local variable in the frame that’s about to be destroyed?).

I didn’t see any indication that any other GCC ports required anything special to tell the optimizer not to delete the move instruction to EH_RETURN_HANDLER_RTX, so the other suspicion I have is that there may be a bug specific to VAX’s peephole optimizer or related functions. Any ideas?

Index: dist/gcc/except.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/except.c,v
retrieving revision 1.3
diff -u -r1.3 except.c
--- dist/gcc/except.c	23 Sep 2015 03:39:10 -0000	1.3
+++ dist/gcc/except.c	21 Mar 2016 20:58:33 -0000
@@ -2207,7 +2207,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;
 #else
       error ("__builtin_eh_return not supported on this target");
 #endif
Index: dist/gcc/config/vax/elf.h
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/elf.h,v
retrieving revision 1.3
diff -u -r1.3 elf.h
--- dist/gcc/config/vax/elf.h	23 Sep 2015 03:39:18 -0000	1.3
+++ dist/gcc/config/vax/elf.h	21 Mar 2016 20:58:33 -0000
@@ -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: dist/gcc/config/vax/vax.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.c,v
retrieving revision 1.3
diff -u -r1.3 vax.c
--- dist/gcc/config/vax/vax.c	23 Sep 2015 03:39:18 -0000	1.3
+++ dist/gcc/config/vax/vax.c	21 Mar 2016 20:58:33 -0000
@@ -164,7 +164,8 @@
 
   offset = 20;
   for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
-    if (df_regs_ever_live_p (regno) && !call_used_regs[regno])
+    if ((df_regs_ever_live_p (regno) && !call_used_regs[regno])
+	|| (crtl->calls_eh_return && regno >= 2 && regno < 4))
       {
         mask |= 1 << regno;
         offset += 4;
Index: dist/gcc/config/vax/vax.h
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.h,v
retrieving revision 1.3
diff -u -r1.3 vax.h
--- dist/gcc/config/vax/vax.h	23 Sep 2015 03:39:18 -0000	1.3
+++ dist/gcc/config/vax/vax.h	21 Mar 2016 20:58:33 -0000
@@ -168,12 +168,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
 
Index: dist/gcc/config/vax/vax.md
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.md,v
retrieving revision 1.3
diff -u -r1.3 vax.md
--- dist/gcc/config/vax/vax.md	23 Sep 2015 03:39:18 -0000	1.3
+++ dist/gcc/config/vax/vax.md	21 Mar 2016 20:58:33 -0000
@@ -1306,6 +1309,11 @@
   ""
   "decl %0\;jgequ %l1")
 

+;; Note that operand 1 is total size of args, in bytes,
+;; and what the call insn wants is the number of words.
+;; It is used in the call instruction as a byte, but in the addl2 as
+;; a word.  Since the only time we actually use it in the call instruction
+;; is when it is a constant, SImode (for addl2) is the proper mode.
 (define_expand "call_pop"
   [(parallel [(call (match_operand:QI 0 "memory_operand" "")
 		    (match_operand:SI 1 "const_int_operand" ""))
@@ -1314,24 +1322,17 @@
 			    (match_operand:SI 3 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[3]) <= 255 * 4 && INTVAL (operands[3]) % 4 == 0);
-
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[3]) + 4);
+  gcc_assert (INTVAL (operands[1]) <= 255 * 4);
+  operands[1] = GEN_INT ((INTVAL (operands[1]) + 3) / 4);
 })
 
 (define_insn "*call_pop"
   [(call (match_operand:QI 0 "memory_operand" "m")
 	 (match_operand:SI 1 "const_int_operand" "n"))
    (set (reg:SI VAX_SP_REGNUM) (plus:SI (reg:SI VAX_SP_REGNUM)
-					(match_operand:SI 2 "immediate_operand" "i")))]
+			     (match_operand:SI 2 "immediate_operand" "i")))]
   ""
-{
-  operands[1] = GEN_INT ((INTVAL (operands[1]) - 4) / 4);
-  return "calls %1,%0";
-})
+  "calls %1,%0")
 
 (define_expand "call_value_pop"
   [(parallel [(set (match_operand 0 "" "")
@@ -1342,12 +1343,8 @@
 			    (match_operand:SI 4 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[4]) <= 255 * 4 && INTVAL (operands[4]) % 4 == 0);
-
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[4]) + 4);
+  gcc_assert (INTVAL (operands[2]) <= 255 * 4);
+  operands[2] = GEN_INT ((INTVAL (operands[2]) + 3) / 4);
 })
 
 (define_insn "*call_value_pop"
@@ -1357,47 +1354,20 @@
    (set (reg:SI VAX_SP_REGNUM) (plus:SI (reg:SI VAX_SP_REGNUM)
 					(match_operand:SI 3 "immediate_operand" "i")))]
   ""
-  "*
-{
-  operands[2] = GEN_INT ((INTVAL (operands[2]) - 4) / 4);
-  return \"calls %2,%1\";
-}")
-
-(define_expand "call"
-  [(call (match_operand:QI 0 "memory_operand" "")
-      (match_operand:SI 1 "const_int_operand" ""))]
-  ""
-  "
-{
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[1]) + 4);
-}")
+  "calls %2,%1")
 
-(define_insn "*call"
-   [(call (match_operand:QI 0 "memory_operand" "m")
-	  (match_operand:SI 1 "const_int_operand" ""))]
+;; Define another set of these for the case of functions with no operands.
+;; These will allow the optimizers to do a slightly better job.
+(define_insn "call"
+  [(call (match_operand:QI 0 "memory_operand" "m")
+	 (const_int 0))]
   ""
   "calls $0,%0")
 
-(define_expand "call_value"
-  [(set (match_operand 0 "" "")
-      (call (match_operand:QI 1 "memory_operand" "")
-	    (match_operand:SI 2 "const_int_operand" "")))]
-  ""
-  "
-{
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[2]) + 4);
-}")
-
-(define_insn "*call_value"
+(define_insn "call_value"
   [(set (match_operand 0 "" "")
 	(call (match_operand:QI 1 "memory_operand" "m")
-	      (match_operand:SI 2 "const_int_operand" "")))]
+	      (const_int 0)))]
   ""
   "calls $0,%1")
 

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Christos Zoulas]

In article <F48D0C6B-A6DB-410B-BC97-C30D4E8B4612@me.com>,
Jake Hamby  <jehamby420@me.com> wrote:

Hi,

Thanks a lot for your patch. I applied it to our gcc-5 in the tree.
Unfortunately gcc-5 seems that it was never tested to even compile.
I fixed the simple compilation issue, but it fails to compile some
files with an internal error trying to construct a dwarf frame info
element with the wrong register. If you have some time, can you
take a look? I will too.

Thanks,

christos

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

> On Mar 23, 2016, at 05:56, Christos Zoulas <christos@astron.com> wrote:
> 
> In article <F48D0C6B-A6DB-410B-BC97-C30D4E8B4612@me.com>,
> Jake Hamby  <jehamby420@me.com> wrote:
> 
> Hi,
> 
> Thanks a lot for your patch. I applied it to our gcc-5 in the tree.
> Unfortunately gcc-5 seems that it was never tested to even compile.
> I fixed the simple compilation issue, but it fails to compile some
> files with an internal error trying to construct a dwarf frame info
> element with the wrong register. If you have some time, can you
> take a look? I will too.
> 
> Thanks,
> 
> christos

Hi Christos,

I just rebased my patches on top of GCC 5.3, which I see you have recently switched to. Here’s a brief explanation of how I patched the dwarf frame error.

The problem is that FIRST_PSEUDO_REGISTER should be increased from 16 to 17, because PSW is considered a real register for DWARF debug purposes. This necessitated changing a few other macros, but nothing major. Since it’s marked as one of the FIXED_REGISTERS, it never actually gets used. Currently I’m doing a full build with GCC 5.3 and CONFIGURE_ARGS += —enable-checking=all, which is very much slower, of course.

One bug I discovered with —enable-checking=all on GCC 4.8.5 is a call to XEXP() that may not be valid, but which can be prevented by checking the other conditions first, and then calling XEXP() if the other conditions are true.

There seems to be a code generation bug with C++ that only affects a few things. Unfortunately, GCC itself (the native version, not the cross compiler) is one of the programs affected. The symptom when compiling almost anything complex in GCC 4.8.5 is a stack overflow as it recursively loops around trying to expand bits and pieces of the insns. It seems to be branching the wrong way.

In looking at this, I discovered one really broken issue in the current vax.md, namely the three peephole optimizations at the bottom. The comment on the bottom one that says “Leave this commented out until we can determine whether the second move precedes a jump which relies on the CC flags being set correctly.” is absolutely correct and I believe all three should be removed. I’ve done so in the diff below, and added a comment explaining why.

I have a theory that the source of any code generation bugs in GCC’s output (and I fear that GCC 5.3 won’t necessarily fix, even if the system itself is completely stable), is that the CC0 notification handler isn’t doing the right thing. I’ll send another email if I make any progress on this issue, but what I’ve discovered so far is that it makes no sense for VAX to switch away from the CC0 style of condition handling, because almost every single instruction updates PSW flags, and in a similar way. So the older style is really optimized for VAX, but it took me a very long time to understand exactly what vax_notice_update_cc() is doing and why correct behavior of it is so important.

The idea is that some part of the optimizer is able to remove explicit tst / cmp insns when a previous one has already set the flags in a way that’s useful. So my theory is that the current version mostly works, but it’s very difficult to understand as it’s written, and it may be very occasionally giving the wrong results due to either bugs in its logic or the instructions emitted not setting the flags in the usual way. Unfortunately, the m68k version of NOTICE_UPDATE_CC (the only other arch supported by NetBSD that uses the CC0 style) is even more convoluted.

So what I’d like to try next is rewriting the VAX version of notice_update_cc to use the style that the AVR backend and others use which is to add a “cc” attribute to the instructions themselves in the .md file describing what each one does in terms of flags. Then the notify function itself becomes much simpler, and, hopefully, more likely to be correct. I did spend a few hours yesterday investigating whether it would make sense to convert VAX to the newer condition codes style that all of the modern CPUs use, but because nearly every instruction, including all of the move instructions, clobbers / updates PSW, it would be much uglier than adding a cc attribute to every opcode, and, what’s worse, it caused all sorts of breakage in the compiler when I tried it because the (clobber (reg:CC VAX_PSW_REGNUM)) lines I had added prevented it from matching instructions.

I did find a patch on the GCC Bugzilla for a different architecture, where someone had gone down the lines of fixing the emit stuff where I found breakage. But VAX is so well tuned to the CC0 style that it makes more sense to refactor it in place in the style of Atmel AVR, NEC V850, Renesas H8/300, all of which are built from a similar template. The m68k backend could benefit from being refactored in a similar way, but I’ll focus on VAX for now. :-)

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50582

Even with the patches attached to that bug, and the FIRST_PSEUDO_REGISTER that I’ve included below, it was still crashing because it can’t find any move insns that don’t clobber PSW. So it seems futile to switch away from CC0, because it’s so much better tuned to VAX, where almost every instruction compares the dest to 0, or to the source, which is usually where a branch would want to go.

The crash I’m seeing in GCC itself is in C++ code, so I had hoped that by commenting out the peephole optimizations at the bottom of vax.md I might fix it, but sadly, no. I’ll try it with the GCC 5.3 build as soon as it finishes. Here’s my current patch set. I’m very excited that the new binutils work (with this patch), and also “-O2” is as stable as the “-O1…” special CFLAGS that are defined now for VAX (which can be removed). Here are the patches I have now. The GCC ones have only been tested with GCC 4.8.5, but I rebased them and they compile and run, so they should be reasonably safe.

Index: external/gpl3/binutils/dist/gas/config/tc-vax.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/binutils/dist/gas/config/tc-vax.c,v
retrieving revision 1.10
diff -u -u -r1.10 tc-vax.c
--- external/gpl3/binutils/dist/gas/config/tc-vax.c	14 Feb 2016 19:00:04 -0000	1.10
+++ external/gpl3/binutils/dist/gas/config/tc-vax.c	26 Mar 2016 10:42:56 -0000
@@ -3430,11 +3430,12 @@
     }
 }
 
+static char *vax_cons_special_reloc;
+
 bfd_reloc_code_real_type
 vax_cons (expressionS *exp, int size)
 {
   char *save;
-  char *vax_cons_special_reloc;
 
   SKIP_WHITESPACE ();
   vax_cons_special_reloc = NULL;
@@ -3560,7 +3561,22 @@
 	 : nbytes == 2 ? BFD_RELOC_16
 	 : BFD_RELOC_32);
 
+  if (vax_cons_special_reloc)
+    {
+      if (*vax_cons_special_reloc == 'p')
+	{
+	  switch (nbytes)
+	    {
+	    case 1: r = BFD_RELOC_8_PCREL; break;
+	    case 2: r = BFD_RELOC_16_PCREL; break;
+	    case 4: r = BFD_RELOC_32_PCREL; break;
+	    default: abort ();
+	    }
+	}
+    }
+
   fix_new_exp (frag, where, (int) nbytes, exp, 0, r);
+  vax_cons_special_reloc = NULL;
 }
 
 char *
@@ -3598,6 +3614,8 @@
 void
 vax_cfi_emit_pcrel_expr (expressionS *expP, unsigned int nbytes)
 {
+  vax_cons_special_reloc = "pcrel";
   expP->X_add_number += nbytes;
   emit_expr (expP, nbytes);
+  vax_cons_special_reloc = NULL;
 }
Index: external/gpl3/gcc/dist/gcc/except.c
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/except.c,v
retrieving revision 1.3
diff -u -u -r1.3 except.c
--- external/gpl3/gcc/dist/gcc/except.c	23 Mar 2016 15:51:36 -0000	1.3
+++ external/gpl3/gcc/dist/gcc/except.c	26 Mar 2016 10:42:41 -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;
 #else
       error ("__builtin_eh_return not supported on this target");
 #endif
Index: external/gpl3/gcc/dist/gcc/config/m68k/m68k.md
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/m68k/m68k.md,v
retrieving revision 1.4
diff -u -u -r1.4 m68k.md
--- external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	24 Jan 2016 09:43:33 -0000	1.4
+++ external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	26 Mar 2016 10:42:41 -0000
@@ -2132,9 +2132,9 @@
 ;; into the kernel to emulate fintrz.  They should also be faster
 ;; than calling the subroutines fixsfsi or fixdfsi.
 
-(define_insn "fix_truncdfsi2"
+(define_insn "fix_trunc<mode>si2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=dm")
-	(fix:SI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:SI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"
@@ -2143,9 +2143,9 @@
   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.l %1,%0\;fmovem%.l %2,%!";
 })
 
-(define_insn "fix_truncdfhi2"
+(define_insn "fix_trunc<mode>hi2"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=dm")
-	(fix:HI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:HI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"
@@ -2154,9 +2154,9 @@
   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.w %1,%0\;fmovem%.l %2,%!";
 })
 
-(define_insn "fix_truncdfqi2"
+(define_insn "fix_trunc<mode>qi2"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=dm")
-	(fix:QI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:QI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"
Index: external/gpl3/gcc/dist/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 -u -r1.6 elf.h
--- external/gpl3/gcc/dist/gcc/config/vax/elf.h	23 Mar 2016 15:51:37 -0000	1.6
+++ external/gpl3/gcc/dist/gcc/config/vax/elf.h	26 Mar 2016 10:42:41 -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: external/gpl3/gcc/dist/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 -u -r1.5 vax-protos.h
--- external/gpl3/gcc/dist/gcc/config/vax/vax-protos.h	23 Mar 2016 21:09:04 -0000	1.5
+++ external/gpl3/gcc/dist/gcc/config/vax/vax-protos.h	26 Mar 2016 10:42:41 -0000
@@ -30,7 +30,7 @@
 extern void print_operand (FILE *, rtx, int);
 extern void vax_notice_update_cc (rtx, rtx);
 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: external/gpl3/gcc/dist/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 -u -r1.15 vax.c
--- external/gpl3/gcc/dist/gcc/config/vax/vax.c	24 Mar 2016 04:27:29 -0000	1.15
+++ external/gpl3/gcc/dist/gcc/config/vax/vax.c	26 Mar 2016 10:42:41 -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,13 +191,17 @@
 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])
+  /* We only care about r0 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.  */
+  for (regno = 0; regno < VAX_AP_REGNUM; regno++)
+    if ((df_regs_ever_live_p (regno) && !call_used_regs[regno])
+	|| (crtl->calls_eh_return && regno >= 2 && regno < 4))
       {
         mask |= 1 << regno;
         offset += 4;
@@ -240,8 +244,10 @@
   vax_add_reg_cfa_offset (insn, 16, pc_rtx);
 
   offset = 20;
-  for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
-    if (mask & (1 << regno))
+
+  unsigned int testbit = 1;	/* Used to avoid calculating (1 << regno). */
+  for (regno = 0; regno < VAX_AP_REGNUM; regno++, testbit <<= 1)
+    if (mask & testbit)
       {
 	vax_add_reg_cfa_offset (insn, offset, gen_rtx_REG (SImode, regno));
 	offset += 4;
@@ -1909,12 +1915,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 +2380,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 +2433,4 @@
 
   return rtx_equal_p(lo, hi) && lo_offset + 4 == hi_offset;
 }
+#endif
Index: external/gpl3/gcc/dist/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 -u -r1.7 vax.h
--- external/gpl3/gcc/dist/gcc/config/vax/vax.h	23 Mar 2016 15:51:37 -0000	1.7
+++ external/gpl3/gcc/dist/gcc/config/vax/vax.h	26 Mar 2016 10:42:41 -0000
@@ -120,13 +120,14 @@
    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
 
 /* 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 +135,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 +170,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 +396,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 +425,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
 
@@ -548,7 +551,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: external/gpl3/gcc/dist/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 -u -r1.11 vax.md
--- external/gpl3/gcc/dist/gcc/config/vax/vax.md	23 Mar 2016 15:51:37 -0000	1.11
+++ external/gpl3/gcc/dist/gcc/config/vax/vax.md	26 Mar 2016 10:42:41 -0000
@@ -532,13 +532,13 @@
 
 ;This is left out because it is very slow;
 ;we are better off programming around the "lack" of this insn.
+;; It's also unclear whether the condition flags would be correct.
 ;(define_insn "divmoddisi4"
-;  [(set (match_operand:SI 0 "general_operand" "=g")
-;	(div:SI (match_operand:DI 1 "general_operand" "g")
-;		(match_operand:SI 2 "general_operand" "g")))
-;   (set (match_operand:SI 3 "general_operand" "=g")
-;	(mod:SI (match_operand:DI 1 "general_operand" "g")
-;		(match_operand:SI 2 "general_operand" "g")))]
+;  [(parallel [(set (match_operand:SI 0 "general_operand" "=g")
+;		   (div:SI (match_operand:DI 1 "general_operand" "nrmT")
+;			   (match_operand:SI 2 "general_operand" "nrmT")))
+;	      (set (match_operand:SI 3 "general_operand" "=g")
+;		   (mod:SI (match_dup 1) (match_dup 2)))])]
 ;  ""
 ;  "ediv %2,%1,%0,%3")
 

@@ -742,7 +742,7 @@
 ;; Rotate right on the VAX works by negating the shift count.
 (define_expand "rotrsi3"
   [(set (match_operand:SI 0 "general_operand" "=g")
-	(rotatert:SI (match_operand:SI 1 "general_operand" "g")
+	(rotatert:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (match_operand:QI 2 "general_operand" "g")))]
   ""
   "
@@ -789,6 +789,9 @@
 ;; netbsd changed this to REG_P (operands[0]) || (MEM_P (operands[0]) && ...
 ;; but gcc made it just !MEM_P (operands[0]) || ...
 
+;; netbsd changed this to REG_P (operands[0]) || (MEM_P (operands[0]) && ...
+;; but gcc made it just !MEM_P (operands[0]) || ...
+
 (define_insn ""
   [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+ro")
 			 (match_operand:QI 1 "const_int_operand" "n")
@@ -1216,7 +1219,7 @@
 	 (gt (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int -1))
 	     (const_int 0))
-	 (label_ref (match_operand 1 "" ""))
+	 (label_ref (match_operand 1))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1230,7 +1233,7 @@
 	 (ge (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int -1))
 	     (const_int 0))
-	 (label_ref (match_operand 1 "" ""))
+	 (label_ref (match_operand 1))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1245,7 +1248,7 @@
 	 (lt (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int 1))
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1258,7 +1261,7 @@
 	(if_then_else
 	 (lt (match_operand:SI 0 "nonimmediate_operand" "+g")
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1272,7 +1275,7 @@
 	 (le (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int 1))
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1285,7 +1288,7 @@
 	(if_then_else
 	 (le (match_operand:SI 0 "nonimmediate_operand" "+g")
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
@@ -1309,6 +1312,11 @@
   ""
   "decl %0\;jgequ %l1")
 

+;; Note that operand 1 is total size of args, in bytes,
+;; and what the call insn wants is the number of words.
+;; It is used in the call instruction as a byte, but in the addl2 as
+;; a word.  Since the only time we actually use it in the call instruction
+;; is when it is a constant, SImode (for addl2) is the proper mode.
 (define_expand "call_pop"
   [(parallel [(call (match_operand:QI 0 "memory_operand" "")
 		    (match_operand:SI 1 "const_int_operand" ""))
@@ -1317,12 +1325,7 @@
 			    (match_operand:SI 3 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[3]) <= 255 * 4 && INTVAL (operands[3]) % 4 == 0);
-
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[3]) + 4);
+  gcc_assert (INTVAL (operands[1]) <= 255 * 4);
 })
 
 (define_insn "*call_pop"
@@ -1332,10 +1335,11 @@
 					(match_operand:SI 2 "immediate_operand" "i")))]
   ""
 {
-  operands[1] = GEN_INT ((INTVAL (operands[1]) - 4) / 4);
+  operands[1] = GEN_INT ((INTVAL (operands[1]) + 3) / 4);
   return "calls %1,%0";
 })
 
+
 (define_expand "call_value_pop"
   [(parallel [(set (match_operand 0 "" "")
 		   (call (match_operand:QI 1 "memory_operand" "")
@@ -1345,12 +1349,7 @@
 			    (match_operand:SI 4 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[4]) <= 255 * 4 && INTVAL (operands[4]) % 4 == 0);
-
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[4]) + 4);
+  gcc_assert (INTVAL (operands[2]) <= 255 * 4);
 })
 
 (define_insn "*call_value_pop"
@@ -1360,47 +1359,24 @@
    (set (reg:SI VAX_SP_REGNUM) (plus:SI (reg:SI VAX_SP_REGNUM)
 					(match_operand:SI 3 "immediate_operand" "i")))]
   ""
-  "*
 {
-  operands[2] = GEN_INT ((INTVAL (operands[2]) - 4) / 4);
-  return \"calls %2,%1\";
-}")
+  operands[2] = GEN_INT ((INTVAL (operands[2]) + 3) / 4);
+  return "calls %2,%1";
+})
 
-(define_expand "call"
-  [(call (match_operand:QI 0 "memory_operand" "")
-      (match_operand:SI 1 "const_int_operand" ""))]
-  ""
-  "
-{
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[1]) + 4);
-}")
 
-(define_insn "*call"
-   [(call (match_operand:QI 0 "memory_operand" "m")
-	  (match_operand:SI 1 "const_int_operand" ""))]
+;; Define another set of these for the case of functions with no operands.
+;; These will allow the optimizers to do a slightly better job.
+(define_insn "call"
+  [(call (match_operand:QI 0 "memory_operand" "m")
+	 (const_int 0))]
   ""
   "calls $0,%0")
 
-(define_expand "call_value"
-  [(set (match_operand 0 "" "")
-      (call (match_operand:QI 1 "memory_operand" "")
-	    (match_operand:SI 2 "const_int_operand" "")))]
-  ""
-  "
-{
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[2]) + 4);
-}")
-
-(define_insn "*call_value"
+(define_insn "call_value"
   [(set (match_operand 0 "" "")
 	(call (match_operand:QI 1 "memory_operand" "m")
-	      (match_operand:SI 2 "const_int_operand" "")))]
+	      (const_int 0)))]
   ""
   "calls $0,%1")
 
@@ -1682,47 +1658,15 @@
 
 (include "builtins.md")
 
-(define_peephole2
-  [(set (match_operand:SI 0 "push_operand" "")
-        (const_int 0))
-   (set (match_dup 0)
-        (match_operand:SI 1 "const_int_operand" ""))]
-  "INTVAL (operands[1]) >= 0"
-  [(set (match_dup 0)
-        (match_dup 1))]
-  "operands[0] = gen_rtx_MEM(DImode, XEXP (operands[0], 0));")
-
-(define_peephole2
-  [(set (match_operand:SI 0 "push_operand" "")
-        (match_operand:SI 1 "general_operand" ""))
-   (set (match_dup 0)
-        (match_operand:SI 2 "general_operand" ""))]
-  "vax_decomposed_dimode_operand_p (operands[2], operands[1])"
-  [(set (match_dup 0)
-        (match_dup 2))]
-  "{
-    operands[0] = gen_rtx_MEM(DImode, XEXP (operands[0], 0));
-    operands[2] = REG_P (operands[2])
-      ? gen_rtx_REG(DImode, REGNO (operands[2]))
-      : gen_rtx_MEM(DImode, XEXP (operands[2], 0));
-}")
-
-; Leave this commented out until we can determine whether the second move
-; precedes a jump which relies on the CC flags being set correctly.
-(define_peephole2
-  [(set (match_operand:SI 0 "nonimmediate_operand" "")
-        (match_operand:SI 1 "general_operand" ""))
-   (set (match_operand:SI 2 "nonimmediate_operand" "")
-        (match_operand:SI 3 "general_operand" ""))]
-  "0 && vax_decomposed_dimode_operand_p (operands[1], operands[3])
-   && vax_decomposed_dimode_operand_p (operands[0], operands[2])"
-  [(set (match_dup 0)
-        (match_dup 1))]
-  "{
-    operands[0] = REG_P (operands[0])
-      ? gen_rtx_REG(DImode, REGNO (operands[0]))
-      : gen_rtx_MEM(DImode, XEXP (operands[0], 0));
-    operands[1] = REG_P (operands[1])
-      ? gen_rtx_REG(DImode, REGNO (operands[1]))
-      : gen_rtx_MEM(DImode, XEXP (operands[1], 0));
-}")
+;; The earlier peephole definitions have been removed because they weren't
+;; setting the flags equivalently to the 32-bit instructions they replaced.
+;; It's also not possible to readily determine whether the second 32-bit move
+;; precedes a jump which relies on the CC flags being set correctly, as the
+;; previous comment noted (for one of the definitions that had been disabled).
+;;
+;; It's probably not worthwhile to make the CC0 handler any more complicated
+;; than it already is, to combine two adjacent 32-bit values into a 64-bit one,
+;; but only if there is no later branch with a dependency on the flag settings.
+;; This seems to happen mostly with C++ code that branches after object copying.
+;; It's also questionable whether any real VAX hardware would benefit from this.
+;; Note that 64-bit arithmetic is already optimized via longlong.h macros.

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

As an added bonus, I see that my patch set also included an old m68k patch that had been sitting in my tree, which fixes a crash when -m68040 is defined. I may have submitted it to port-m68k before. It hasn’t been tested with the new compiler either. Here’s that patch separately. It only matter when TARGET_68881 && TUNE_68040.

-Jake


Index: external/gpl3/gcc/dist/gcc/config/m68k/m68k.md
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/m68k/m68k.md,v
retrieving revision 1.4
diff -u -u -r1.4 m68k.md
--- external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	24 Jan 2016 09:43:33 -0000	1.4
+++ external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	26 Mar 2016 10:42:41 -0000
@@ -2132,9 +2132,9 @@
 ;; into the kernel to emulate fintrz.  They should also be faster
 ;; than calling the subroutines fixsfsi or fixdfsi.
 
-(define_insn "fix_truncdfsi2"
+(define_insn "fix_trunc<mode>si2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=dm")
-	(fix:SI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:SI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"
@@ -2143,9 +2143,9 @@
   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.l %1,%0\;fmovem%.l %2,%!";
 })
 
-(define_insn "fix_truncdfhi2"
+(define_insn "fix_trunc<mode>hi2"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=dm")
-	(fix:HI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:HI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"
@@ -2154,9 +2154,9 @@
   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.w %1,%0\;fmovem%.l %2,%!";
 })
 
-(define_insn "fix_truncdfqi2"
+(define_insn "fix_trunc<mode>qi2"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=dm")
-	(fix:QI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
+	(fix:QI (match_operand:FP 1 "register_operand" "f")))
    (clobber (match_scratch:SI 2 "=d"))
    (clobber (match_scratch:SI 3 "=d"))]
   "TARGET_68881 && TUNE_68040"

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

Unfortunately, my previous patch that included a change to gcc/config/vax/vax.h that increased FIRST_PSEUDO_REGISTER from 16 to 17 breaks the C++ exception handling that I’d worked so hard to get right with the rest of the patch. I believe I need to define DWARF_FRAME_REGISTERS to 16 in the same file to fix the size of the array that libgcc/unwind-dw2.c creates. The i386 backend and several others also define it their .h file for the same reason (compatibility with hardcoded frame offsets).

Here’s the first part of the patch to vax.h that increases FIRST_PSEUDO_REGISTER and also adds a definition of DWARF_FRAME_REGISTERS as 16, with suitable comment. I’m testing it now. I know that C++ exceptions were working before I increased FIRST_PSEUDO_REGISTER to 17.

Regards,
Jake

Index: external/gpl3/gcc.old/dist/gcc/config/vax/vax.h
===================================================================
RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.h,v
retrieving revision 1.3
diff -u -r1.3 vax.h
--- external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	23 Sep 2015 03:39:18 -0000	1.3
+++ external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	26 Mar 2016 14:34:29 -0000
@@ -119,13 +119,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

Re: Patches to fix GCC's C++ exception_handling on NetBSD/VAX

[Mikael Pettersson]

Jake Hamby writes:
 > As an added bonus, I see that my patch set also included an old m68k patch
 > that had been sitting in my tree, which fixes a crash when -m68040 is defined.
 > I may have submitted it to port-m68k before. It hasn't been tested with the
 > new compiler either. Here's that patch separately. It only matter when
 > TARGET_68881 && TUNE_68040.

Do you have a test case or some recipe for reproducing the crash?
I'd be happy to test this patch on Linux/M68K.

/Mikael

 > 
 > -Jake
 > 
 > 
 > Index: external/gpl3/gcc/dist/gcc/config/m68k/m68k.md
 > ====================================================================
 > RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/m68k/m68k.md,v
 > retrieving revision 1.4
 > diff -u -u -r1.4 m68k.md
 > --- external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	24 Jan 2016 09:43:33 -0000	1.4
 > +++ external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	26 Mar 2016 10:42:41 -0000
 > @@ -2132,9 +2132,9 @@
 >  ;; into the kernel to emulate fintrz.  They should also be faster
 >  ;; than calling the subroutines fixsfsi or fixdfsi.
 > 
 > -(define_insn "fix_truncdfsi2"
 > +(define_insn "fix_trunc<mode>si2"
 >    [(set (match_operand:SI 0 "nonimmediate_operand" "=dm")
 > -	(fix:SI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
 > +	(fix:SI (match_operand:FP 1 "register_operand" "f")))
 >     (clobber (match_scratch:SI 2 "=d"))
 >     (clobber (match_scratch:SI 3 "=d"))]
 >    "TARGET_68881 && TUNE_68040"
 > @@ -2143,9 +2143,9 @@
 >    return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.l %1,%0\;fmovem%.l %2,%!";
 >  })
 > 
 > -(define_insn "fix_truncdfhi2"
 > +(define_insn "fix_trunc<mode>hi2"
 >    [(set (match_operand:HI 0 "nonimmediate_operand" "=dm")
 > -	(fix:HI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
 > +	(fix:HI (match_operand:FP 1 "register_operand" "f")))
 >     (clobber (match_scratch:SI 2 "=d"))
 >     (clobber (match_scratch:SI 3 "=d"))]
 >    "TARGET_68881 && TUNE_68040"
 > @@ -2154,9 +2154,9 @@
 >    return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.w %1,%0\;fmovem%.l %2,%!";
 >  })
 > 
 > -(define_insn "fix_truncdfqi2"
 > +(define_insn "fix_trunc<mode>qi2"
 >    [(set (match_operand:QI 0 "nonimmediate_operand" "=dm")
 > -	(fix:QI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
 > +	(fix:QI (match_operand:FP 1 "register_operand" "f")))
 >     (clobber (match_scratch:SI 2 "=d"))
 >     (clobber (match_scratch:SI 3 "=d"))]
 >    "TARGET_68881 && TUNE_68040"

-- 

Re: Patches to fix GCC's C++ exception_handling on NetBSD/VAX

[Jake Hamby]

Thank you for the offer. I already tested it on an Amiga 3000 w/ 68040 card when I made the fix. The bug manifested as the cross-compiler crashing with a failure to find a suitable insn, because it couldn’t find the correct FP instruction to expand to. I believe it manifested when running ./build.sh release with “-m68040” set in CPUFLAGS. I will test it myself and see if it’s still an issue without the patch. If you look at the .md file, there’s an entirely different code path to generate the same instructions when "TARGET_68881 && TUNE_68040" aren't defined.

At the time I made the change, I had already been testing the code on an Amiga 3000 w/ 68040 card, so I know that the generated code is likely correct (also, the assembler accepted it). So I’m assuming that it’s a fairly safe thing. It was the difference between the build succeeding or failing, and not an issue with the generated code.

So the only thing I can suggest is that you can try a build with the patch and make sure it's stable. I was never able to produce a build without it, because "TARGET_68881 && TUNE_68040" was excluding the other choices when building, I believe, libm or libc or the kernel or something like that. I do have a test case for C++ exceptions on VAX, which I will send separately.

Thanks,
Jake


> On Mar 27, 2016, at 10:08, Mikael Pettersson <mikpelinux@gmail.com> wrote:
> 
> Jake Hamby writes:
>> As an added bonus, I see that my patch set also included an old m68k patch
>> that had been sitting in my tree, which fixes a crash when -m68040 is defined.
>> I may have submitted it to port-m68k before. It hasn't been tested with the
>> new compiler either. Here's that patch separately. It only matter when
>> TARGET_68881 && TUNE_68040.
> 
> Do you have a test case or some recipe for reproducing the crash?
> I'd be happy to test this patch on Linux/M68K.
> 
> /Mikael
> 
>> 
>> -Jake
>> 
>> 
>> Index: external/gpl3/gcc/dist/gcc/config/m68k/m68k.md
>> ====================================================================
>> RCS file: /cvsroot/src/external/gpl3/gcc/dist/gcc/config/m68k/m68k.md,v
>> retrieving revision 1.4
>> diff -u -u -r1.4 m68k.md
>> --- external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	24 Jan 2016 09:43:33 -0000	1.4
>> +++ external/gpl3/gcc/dist/gcc/config/m68k/m68k.md	26 Mar 2016 10:42:41 -0000
>> @@ -2132,9 +2132,9 @@
>> ;; into the kernel to emulate fintrz.  They should also be faster
>> ;; than calling the subroutines fixsfsi or fixdfsi.
>> 
>> -(define_insn "fix_truncdfsi2"
>> +(define_insn "fix_trunc<mode>si2"
>>   [(set (match_operand:SI 0 "nonimmediate_operand" "=dm")
>> -	(fix:SI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
>> +	(fix:SI (match_operand:FP 1 "register_operand" "f")))
>>    (clobber (match_scratch:SI 2 "=d"))
>>    (clobber (match_scratch:SI 3 "=d"))]
>>   "TARGET_68881 && TUNE_68040"
>> @@ -2143,9 +2143,9 @@
>>   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.l %1,%0\;fmovem%.l %2,%!";
>> })
>> 
>> -(define_insn "fix_truncdfhi2"
>> +(define_insn "fix_trunc<mode>hi2"
>>   [(set (match_operand:HI 0 "nonimmediate_operand" "=dm")
>> -	(fix:HI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
>> +	(fix:HI (match_operand:FP 1 "register_operand" "f")))
>>    (clobber (match_scratch:SI 2 "=d"))
>>    (clobber (match_scratch:SI 3 "=d"))]
>>   "TARGET_68881 && TUNE_68040"
>> @@ -2154,9 +2154,9 @@
>>   return "fmovem%.l %!,%2\;moveq #16,%3\;or%.l %2,%3\;and%.w #-33,%3\;fmovem%.l %3,%!\;fmove%.w %1,%0\;fmovem%.l %2,%!";
>> })
>> 
>> -(define_insn "fix_truncdfqi2"
>> +(define_insn "fix_trunc<mode>qi2"
>>   [(set (match_operand:QI 0 "nonimmediate_operand" "=dm")
>> -	(fix:QI (fix:DF (match_operand:DF 1 "register_operand" "f"))))
>> +	(fix:QI (match_operand:FP 1 "register_operand" "f")))
>>    (clobber (match_scratch:SI 2 "=d"))
>>    (clobber (match_scratch:SI 3 "=d"))]
>>   "TARGET_68881 && TUNE_68040"
> 
> -- 

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

I'm very pleased to report that I was able to successfully build a NetBSD/vax system using the checked-in GCC 5.3, with the patches I've submitted, setting FIRST_PSEUDO_REGISTER to 17 and DWARF_FRAME_REGISTERS to 16. The kernel produced with GCC 5.3 crashes (on a real VS4000/90 and also SimH) in UVM, which may be a bug in the kernel that different optimization exposed, or a bug in GCC's generated code.

If you don't set DWARF_FRAME_REGISTERS to 16, then C++ exceptions break again, and GDB suddenly can't deal with the larger debug frames because of the data structure size mismatch between GCC and GDB. But with that additional define, you can raise FIRST_PSEUDO_REGISTER to include PSW (which is correct, since DWARF already uses that meaning), remove the "#ifdef notworking" around the asserts that Christos added in the NetBSD tree, and everything works as well as it did with #define FIRST_PSEUDO_REGISTER 16.

Here's the C++ test case I've been using to verify that the stack unwinding works and that different simple types can be thrown and caught. My ultimate goal is to be able to run GCC's testsuite because I'm far from certain that the OS, or even the majority of packages, really exercise all of the different paths in this very CISC architecture.

#include <string>
#include <stdio.h>

int recursive_throw(int i) {
  printf("enter recursive_throw(%d)\n", i);
  if (i > 0) {
    printf("calling recursive_throw(%d)\n", i - 1);
    recursive_throw(i - 1);
  } else {
    printf("throwing exception\n");
    throw 456;
  }
  printf("exit recursive_throw(%d)\n", i);
}

/* Test several kinds of throws. */
int throwtest(int test) {
  switch (test) {
    case 0:
    case 1:
      return test;

    case 2:
      throw 123;

    case 3:
      throw 123.45;

    case 4:
      throw 678.9f;

    case 5:
      recursive_throw(6);
      return 666;  // fail

    default:
      return 999;  // not used in test
  }
}

int main() {
  for (int i = 0; i < 6; i++) {
    try {
      int ret = throwtest(i);
      printf("throwtest(%d) returned %d\n", i, ret);
    } catch (int e) {
      printf("Caught int exception: %d\n", e);
    } catch (double d) {
      printf("Caught double exception: %f\n", d);
    } catch (float f) {
      printf("Caught float exception: %f\n", (double)f);
    }
  }
}

I'm pleased that I got it working, but the change I made to except.c to add:

RTX_FRAME_RELATED_P (insn) = 1;

below:

#ifdef EH_RETURN_HANDLER_RTX
      rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler);

isn't really correct, I don't think. It adds an additional .cfi directive that wasn't there before, and a GCC ./buildsh release fails building unwind-dw2.c (that's the place where the build either succeeds or fails or generates bad code) if you try to compile with assertions (and it doesn't without my change to except.c).

Unfortunately, I don't have a patch for the root cause for me having to add that line to except.c, which is that the required mov instruction to copy the __builtin_eh_return() return address into the old stack frame is being deleted for some reason, otherwise. Since I know the #ifdef EH_RETURN_HANDLER_RTX line *is* staying in the final code on other archs, I presume the problem is something VAX-related in the .md file.

If anyone knows about GCC's liveness detection, and specifically any potential problems that might cause this to be happening (removing a required "emit_move_insn (EH_RETURN_HANDLER_RTX, ...)" before a return call that was added in expand_eh_return () but then deleted if -O or higher is used), any advice would be appreciated as to where to look.

What I'm working on now is cleaning up and refactoring the .md insn definitions, but I'm not ready to share that code until it works and does something useful. I'm hoping to be able to optimize the removal of unneeded tst / cmp instructions when the condition codes have been set to something useful by a previous insn. I don't think the code in vax_notice_update_cc () is necessarily correct, and it's very difficult to understand exactly how it's working, because it's trying to determine this based entirely on looking at the RTL of the insn (set, call, zero_extract, etc), which I think may have become out of sync with the types of instructions that are actually emitted in vax.md for those operations.

So I've just finished tagging the define_insn calls in vax.md with a "cc" attribute (like the avr backend) which my (hopefully more correct and more optimized) version of vax_notice_update_cc will use to know exactly what the flag status is after the insn, for Z, N, and C. Here's my definition of "cc". I'll share the rest after I'm sure that it works.

;; Condition code settings.  On VAX, the default value is "clobber".
;; The V flag is often set to zero, or else it has a special meaning,
;; usually related to testing for a signed integer range overflow.
;; "cmp_czn", "cmp_zn", and "cmp_z" are all assumed to modify V, and
;; none is expected to modify C.  "plus" is used after an add/sub,
;; when the flags, including C, return info about the result,
;; including a carry bit to use with, e.g. "adwc".

;; The important thing to note is that the C flag, in the case of "plus",
;; doesn't reflect the results of a signed integer comparison,
;; as "cmp_czn" does.  Finally, "cmp_zn_use_c" and "cmp_z_use_cn" indicate
;; that all four flags are updated: Z and N, or Z alone, will be a comparison,
;; but C is set to 0, or some other value, instead of retaining its old value.
;; Only instructions of type "compare" set the C, Z, and N flags correctly
;; for both signed and unsigned ordered comparisons.

;; For branching, only Z is needed to test for equality, between two
;; operands or between the first operand and 0.  The N flag is necessary
;; to test for less than zero, and for FP or signed integer comparisons.
;; Both N and Z are required to branch on signed (a <= b) or (a > b).
;; For comparing unsigned integers, the C flag is used instead of N.
;; Both C and Z are required to branch on unsigned (a <= b) or (a > b).

;; The VAX instruction set is biased towards leaving C alone, relative to
;; the other flags, which tend to be modified on almost every instruction.
;; It's possible to cache the results of two signed int comparison,
;; as long as they're of the form (a < b) or (a >= b), where b may be 0,
;; in the C flag, while other instructions modify the other flags. Then,
;; a test for a branch can be saved later by referring to the previous value.
;; The cc attributes are intended so that this optimization may be performed.

(define_attr "cc" "none,cmp_czn,cmp_zn,cmp_zn_use_c,
			cmp_z,cmp_z_use_czn,plus,clobber"
  (const_string "clobber"))


I'll send another email once I have something substantial to contribute. I give my permission to NetBSD and the FSF to integrate any or all of my changes under the copyright terms of the original files. Please let me know if I have to do any additional legal stuff for code contributions. I'm doing this on my own time and not on behalf of any company or organization.

Best regards,
Jake


> On Mar 26, 2016, at 07:38, Jake Hamby <jehamby420@me.com> wrote:
> 
> Unfortunately, my previous patch that included a change to gcc/config/vax/vax.h that increased FIRST_PSEUDO_REGISTER from 16 to 17 breaks the C++ exception handling that I’d worked so hard to get right with the rest of the patch. I believe I need to define DWARF_FRAME_REGISTERS to 16 in the same file to fix the size of the array that libgcc/unwind-dw2.c creates. The i386 backend and several others also define it their .h file for the same reason (compatibility with hardcoded frame offsets).
> 
> Here’s the first part of the patch to vax.h that increases FIRST_PSEUDO_REGISTER and also adds a definition of DWARF_FRAME_REGISTERS as 16, with suitable comment. I’m testing it now. I know that C++ exceptions were working before I increased FIRST_PSEUDO_REGISTER to 17.
> 
> Regards,
> Jake
> 
> Index: external/gpl3/gcc.old/dist/gcc/config/vax/vax.h
> ===================================================================
> RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.h,v
> retrieving revision 1.3
> diff -u -r1.3 vax.h
> --- external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	23 Sep 2015 03:39:18 -0000	1.3
> +++ external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	26 Mar 2016 14:34:29 -0000
> @@ -119,13 +119,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
> 

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jake Hamby]

The results you want to see for the test program are the following:

throwtest(0) returned 0
throwtest(1) returned 1
Caught int exception: 123
Caught double exception: 123.450000
Caught float exception: 678.900024
enter recursive_throw(6)
calling recursive_throw(5)
enter recursive_throw(5)
calling recursive_throw(4)
enter recursive_throw(4)
calling recursive_throw(3)
enter recursive_throw(3)
calling recursive_throw(2)
enter recursive_throw(2)
calling recursive_throw(1)
enter recursive_throw(1)
calling recursive_throw(0)
enter recursive_throw(0)
throwing exception
Caught int exception: 456

Before I made the changes I've submitted, this worked on m68k and presumably everywhere else but VAX. On VAX, it crashed due to the pointer size discrepancies that I already talked about. I believe that it should be possible to improve GCC's backend by allowing %ap to be used as an additional general register (removing it from FIXED_REGISTERS, but leaving it in CALL_USED_REGISTERS, since it's modified on CALLS), without breaking the DWARF stack unwinding stuff, since the .cfi information it's emitting notes the correct %fp offset to find the frame, which it actually uses instead of the %ap in stack unwinding.

Gaining an additional general register to use within a function would be a nice win if it worked. But there are at two problems that must be solved before doing this (that I know of). The first is that certain combinations of VAX instructions and addressing modes seem to have problems when %ap, %fp, and/or %sp are used. I discovered this in the OpenVMS VAX Macro reference (which is essentially an updated version of the 1977 VAX architecture handbook), in Table 8-5, General Register Addressing.

An additional source of info on which modes fail with address faults when AP or above is used, SimH's vax_cpu.c correctly handles this, and you can trace these macros to find the conditions:

#define CHECK_FOR_PC    if (rn == nPC) \
                            RSVD_ADDR_FAULT
#define CHECK_FOR_SP    if (rn >= nSP) \
                            RSVD_ADDR_FAULT
#define CHECK_FOR_AP    if (rn >= nAP) \
                            RSVD_ADDR_FAULT

So as long as the correct code is added to vax.md and vax.c to never emit move instructions under the wrong circumstances when %ap is involved, it could be used as a general register. I wonder if the use of %ap to find address arguments is a special case that happens to never emit anything that would fail (with a SIGILL, I believe).

But the other problem with making %ap available as a general (with a few special cases) register is that it would break one part of the patch I submitted at the beginning of the thread to fix C++ exceptions. One necessary part of that fix was to change "#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0" to "#define ARG_POINTER_CFA_OFFSET(FNDECL) 0", which correctly generates the code to return the value for __builtin_dwarf_cfa () (as an offset of 0 from %ap).

When I was working on that fix, it seemed like it should be possible, since the DWARF / CFA code that's in there now is using an offset from %fp that it knows, that an improved fix would define FRAME_POINTER_CFA_OFFSET(FNDECL) as something that knows how to return the current CFA (canonical frame address) as an offset from %fp, since that's what it's using for all the .cfi_* directives. But I don't know what a correct definition of FRAME_POINTER_CFA_OFFSET should be in order for it to return that value, instead of 0, because I know that a 0 offset from %fp is definitely wrong, and it's not a fixed offset either (it depends on the number of registers saved in the procedure entry mask). Fortunately, %ap points directly to CFA, so that always works.

Just some thoughts on future areas for improval... I'm very happy to be able to run the NetBSD testsuite on VAX now. It gives me a lot of confidence as to what works and what doesn't. Most of the stuff I expected to fail (like libm tests, since it's not IEEE FP) failed, and most of the rest succeeded.

-Jake


> On Mar 27, 2016, at 15:34, Jake Hamby <jehamby420@me.com> wrote:
> 
> I'm very pleased to report that I was able to successfully build a NetBSD/vax system using the checked-in GCC 5.3, with the patches I've submitted, setting FIRST_PSEUDO_REGISTER to 17 and DWARF_FRAME_REGISTERS to 16. The kernel produced with GCC 5.3 crashes (on a real VS4000/90 and also SimH) in UVM, which may be a bug in the kernel that different optimization exposed, or a bug in GCC's generated code.
> 
> If you don't set DWARF_FRAME_REGISTERS to 16, then C++ exceptions break again, and GDB suddenly can't deal with the larger debug frames because of the data structure size mismatch between GCC and GDB. But with that additional define, you can raise FIRST_PSEUDO_REGISTER to include PSW (which is correct, since DWARF already uses that meaning), remove the "#ifdef notworking" around the asserts that Christos added in the NetBSD tree, and everything works as well as it did with #define FIRST_PSEUDO_REGISTER 16.
> 
> Here's the C++ test case I've been using to verify that the stack unwinding works and that different simple types can be thrown and caught. My ultimate goal is to be able to run GCC's testsuite because I'm far from certain that the OS, or even the majority of packages, really exercise all of the different paths in this very CISC architecture.
> 
> #include <string>
> #include <stdio.h>
> 
> int recursive_throw(int i) {
>  printf("enter recursive_throw(%d)\n", i);
>  if (i > 0) {
>    printf("calling recursive_throw(%d)\n", i - 1);
>    recursive_throw(i - 1);
>  } else {
>    printf("throwing exception\n");
>    throw 456;
>  }
>  printf("exit recursive_throw(%d)\n", i);
> }
> 
> /* Test several kinds of throws. */
> int throwtest(int test) {
>  switch (test) {
>    case 0:
>    case 1:
>      return test;
> 
>    case 2:
>      throw 123;
> 
>    case 3:
>      throw 123.45;
> 
>    case 4:
>      throw 678.9f;
> 
>    case 5:
>      recursive_throw(6);
>      return 666;  // fail
> 
>    default:
>      return 999;  // not used in test
>  }
> }
> 
> int main() {
>  for (int i = 0; i < 6; i++) {
>    try {
>      int ret = throwtest(i);
>      printf("throwtest(%d) returned %d\n", i, ret);
>    } catch (int e) {
>      printf("Caught int exception: %d\n", e);
>    } catch (double d) {
>      printf("Caught double exception: %f\n", d);
>    } catch (float f) {
>      printf("Caught float exception: %f\n", (double)f);
>    }
>  }
> }
> 
> I'm pleased that I got it working, but the change I made to except.c to add:
> 
> RTX_FRAME_RELATED_P (insn) = 1;
> 
> below:
> 
> #ifdef EH_RETURN_HANDLER_RTX
>      rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler);
> 
> isn't really correct, I don't think. It adds an additional .cfi directive that wasn't there before, and a GCC ./buildsh release fails building unwind-dw2.c (that's the place where the build either succeeds or fails or generates bad code) if you try to compile with assertions (and it doesn't without my change to except.c).
> 
> Unfortunately, I don't have a patch for the root cause for me having to add that line to except.c, which is that the required mov instruction to copy the __builtin_eh_return() return address into the old stack frame is being deleted for some reason, otherwise. Since I know the #ifdef EH_RETURN_HANDLER_RTX line *is* staying in the final code on other archs, I presume the problem is something VAX-related in the .md file.
> 
> If anyone knows about GCC's liveness detection, and specifically any potential problems that might cause this to be happening (removing a required "emit_move_insn (EH_RETURN_HANDLER_RTX, ...)" before a return call that was added in expand_eh_return () but then deleted if -O or higher is used), any advice would be appreciated as to where to look.
> 
> What I'm working on now is cleaning up and refactoring the .md insn definitions, but I'm not ready to share that code until it works and does something useful. I'm hoping to be able to optimize the removal of unneeded tst / cmp instructions when the condition codes have been set to something useful by a previous insn. I don't think the code in vax_notice_update_cc () is necessarily correct, and it's very difficult to understand exactly how it's working, because it's trying to determine this based entirely on looking at the RTL of the insn (set, call, zero_extract, etc), which I think may have become out of sync with the types of instructions that are actually emitted in vax.md for those operations.
> 
> So I've just finished tagging the define_insn calls in vax.md with a "cc" attribute (like the avr backend) which my (hopefully more correct and more optimized) version of vax_notice_update_cc will use to know exactly what the flag status is after the insn, for Z, N, and C. Here's my definition of "cc". I'll share the rest after I'm sure that it works.
> 
> ;; Condition code settings.  On VAX, the default value is "clobber".
> ;; The V flag is often set to zero, or else it has a special meaning,
> ;; usually related to testing for a signed integer range overflow.
> ;; "cmp_czn", "cmp_zn", and "cmp_z" are all assumed to modify V, and
> ;; none is expected to modify C.  "plus" is used after an add/sub,
> ;; when the flags, including C, return info about the result,
> ;; including a carry bit to use with, e.g. "adwc".
> 
> ;; The important thing to note is that the C flag, in the case of "plus",
> ;; doesn't reflect the results of a signed integer comparison,
> ;; as "cmp_czn" does.  Finally, "cmp_zn_use_c" and "cmp_z_use_cn" indicate
> ;; that all four flags are updated: Z and N, or Z alone, will be a comparison,
> ;; but C is set to 0, or some other value, instead of retaining its old value.
> ;; Only instructions of type "compare" set the C, Z, and N flags correctly
> ;; for both signed and unsigned ordered comparisons.
> 
> ;; For branching, only Z is needed to test for equality, between two
> ;; operands or between the first operand and 0.  The N flag is necessary
> ;; to test for less than zero, and for FP or signed integer comparisons.
> ;; Both N and Z are required to branch on signed (a <= b) or (a > b).
> ;; For comparing unsigned integers, the C flag is used instead of N.
> ;; Both C and Z are required to branch on unsigned (a <= b) or (a > b).
> 
> ;; The VAX instruction set is biased towards leaving C alone, relative to
> ;; the other flags, which tend to be modified on almost every instruction.
> ;; It's possible to cache the results of two signed int comparison,
> ;; as long as they're of the form (a < b) or (a >= b), where b may be 0,
> ;; in the C flag, while other instructions modify the other flags. Then,
> ;; a test for a branch can be saved later by referring to the previous value.
> ;; The cc attributes are intended so that this optimization may be performed.
> 
> (define_attr "cc" "none,cmp_czn,cmp_zn,cmp_zn_use_c,
> 			cmp_z,cmp_z_use_czn,plus,clobber"
>  (const_string "clobber"))
> 
> 
> I'll send another email once I have something substantial to contribute. I give my permission to NetBSD and the FSF to integrate any or all of my changes under the copyright terms of the original files. Please let me know if I have to do any additional legal stuff for code contributions. I'm doing this on my own time and not on behalf of any company or organization.
> 
> Best regards,
> Jake
> 
> 
>> On Mar 26, 2016, at 07:38, Jake Hamby <jehamby420@me.com> wrote:
>> 
>> Unfortunately, my previous patch that included a change to gcc/config/vax/vax.h that increased FIRST_PSEUDO_REGISTER from 16 to 17 breaks the C++ exception handling that I’d worked so hard to get right with the rest of the patch. I believe I need to define DWARF_FRAME_REGISTERS to 16 in the same file to fix the size of the array that libgcc/unwind-dw2.c creates. The i386 backend and several others also define it their .h file for the same reason (compatibility with hardcoded frame offsets).
>> 
>> Here’s the first part of the patch to vax.h that increases FIRST_PSEUDO_REGISTER and also adds a definition of DWARF_FRAME_REGISTERS as 16, with suitable comment. I’m testing it now. I know that C++ exceptions were working before I increased FIRST_PSEUDO_REGISTER to 17.
>> 
>> Regards,
>> Jake
>> 
>> Index: external/gpl3/gcc.old/dist/gcc/config/vax/vax.h
>> ===================================================================
>> RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.h,v
>> retrieving revision 1.3
>> diff -u -r1.3 vax.h
>> --- external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	23 Sep 2015 03:39:18 -0000	1.3
>> +++ external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	26 Mar 2016 14:34:29 -0000
>> @@ -119,13 +119,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
>> 
> 

Bad CC0 optimizer in VAX backend (was Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX)

[Jake Hamby]

I have some bad news and some good news. The bad news is that there has been a nasty optimizer bug lurking in the VAX backend for GCC for many years, which has to do with over-optimistic removal of necessary tst/cmp instructions under certain circumstances. This manifests at -O or higher and the symptoms are strange behavior like stack overflows because of branches going the wrong way.

The good news is that my suspicions about the CC0 handler appear to be correct, and better yet, I'm currently testing a patch that isn't too big and I believe will fix this bug. The bad behavior is in vax_notice_update_cc (), which is supposed to call CC_STATUS_INIT if the CC flags have been reset, or set cc_status.value1 and possibly cc_status.value2 with the destination of the current (set...) instruction, whose signed comparison to 0 will be stored in the N and Z flags as a side effect (most VAX instructions do this).

The first bug is that most, but not all of the define_insn patterns in vax.md actually do this. The paths that emit movz* instructions will not set N and Z, and there are some code paths that can't be trusted because they handle a 64-bit integer in two 32-bit pieces, for example, so N and Z won't reflect the desired result (comparison of operand 0 to 0).

The current version of vax_notice_update_cc has a specific check for this: it won't set the cc_status cache if GET_CODE (SET_DEST (exp)) != ZERO_EXTRACT. The problem is that this is checking the RTL expression for (zero_extract...) and not whether what was actually emitted is a movz* or not. That's why all of the define_insn()'s have to be annotated with their actual behavior vis-a-vis compare to 0, and then the change to vax.c to read the CC attribute makes it really much easier to get the correct behavior.

I need to do a lot of testing today to see if this really does help fix GCC's VAX backend, but I'm hopeful that this is at least a real bug that needs to be fixed, and that I have a fix for it that should work. The other good news is that you only need three cc enums: "none" (doesn't alter Z or N flags), "compare" (Z and N reflect comparison of operand 0 to literal 0), and "clobber" (call CC_STATUS_INIT to reset cc_status cache).

One thing to note is that the current version of vax.c only sets CC_NO_OVERFLOW on certain paths, while it should actually always set CC_NO_OVERFLOW so as to avoid emitting any unsigned branch instructions that would use an incorrect value of the C flag (which most VAX instructions do NOT set). The code that handles CC_NO_OVERFLOW is in final.c, and what it does is change signed comparisons with 0 into unsigned ones that do the same thing. (a < 0) is always false (for unsigned ints), (a >= 0) is always true, (a <= 0) turns into (a == 0), and (a > 0) turns into (a != 0). Here's the patch to vax.c. I'll send the rest after I've tested that it does what I think it should do. The diff to vax.md will be larger, but it's mostly adding the "cc" attributes and not code.

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 22:28:10 -0000
@@ -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;
 }
 

> On Mar 27, 2016, at 16:06, Jake Hamby <jehamby420@me.com> wrote:
> 
> The results you want to see for the test program are the following:
> 
> throwtest(0) returned 0
> throwtest(1) returned 1
> Caught int exception: 123
> Caught double exception: 123.450000
> Caught float exception: 678.900024
> enter recursive_throw(6)
> calling recursive_throw(5)
> enter recursive_throw(5)
> calling recursive_throw(4)
> enter recursive_throw(4)
> calling recursive_throw(3)
> enter recursive_throw(3)
> calling recursive_throw(2)
> enter recursive_throw(2)
> calling recursive_throw(1)
> enter recursive_throw(1)
> calling recursive_throw(0)
> enter recursive_throw(0)
> throwing exception
> Caught int exception: 456
> 
> Before I made the changes I've submitted, this worked on m68k and presumably everywhere else but VAX. On VAX, it crashed due to the pointer size discrepancies that I already talked about. I believe that it should be possible to improve GCC's backend by allowing %ap to be used as an additional general register (removing it from FIXED_REGISTERS, but leaving it in CALL_USED_REGISTERS, since it's modified on CALLS), without breaking the DWARF stack unwinding stuff, since the .cfi information it's emitting notes the correct %fp offset to find the frame, which it actually uses instead of the %ap in stack unwinding.
> 
> Gaining an additional general register to use within a function would be a nice win if it worked. But there are at two problems that must be solved before doing this (that I know of). The first is that certain combinations of VAX instructions and addressing modes seem to have problems when %ap, %fp, and/or %sp are used. I discovered this in the OpenVMS VAX Macro reference (which is essentially an updated version of the 1977 VAX architecture handbook), in Table 8-5, General Register Addressing.
> 
> An additional source of info on which modes fail with address faults when AP or above is used, SimH's vax_cpu.c correctly handles this, and you can trace these macros to find the conditions:
> 
> #define CHECK_FOR_PC    if (rn == nPC) \
>                            RSVD_ADDR_FAULT
> #define CHECK_FOR_SP    if (rn >= nSP) \
>                            RSVD_ADDR_FAULT
> #define CHECK_FOR_AP    if (rn >= nAP) \
>                            RSVD_ADDR_FAULT
> 
> So as long as the correct code is added to vax.md and vax.c to never emit move instructions under the wrong circumstances when %ap is involved, it could be used as a general register. I wonder if the use of %ap to find address arguments is a special case that happens to never emit anything that would fail (with a SIGILL, I believe).
> 
> But the other problem with making %ap available as a general (with a few special cases) register is that it would break one part of the patch I submitted at the beginning of the thread to fix C++ exceptions. One necessary part of that fix was to change "#define FRAME_POINTER_CFA_OFFSET(FNDECL) 0" to "#define ARG_POINTER_CFA_OFFSET(FNDECL) 0", which correctly generates the code to return the value for __builtin_dwarf_cfa () (as an offset of 0 from %ap).
> 
> When I was working on that fix, it seemed like it should be possible, since the DWARF / CFA code that's in there now is using an offset from %fp that it knows, that an improved fix would define FRAME_POINTER_CFA_OFFSET(FNDECL) as something that knows how to return the current CFA (canonical frame address) as an offset from %fp, since that's what it's using for all the .cfi_* directives. But I don't know what a correct definition of FRAME_POINTER_CFA_OFFSET should be in order for it to return that value, instead of 0, because I know that a 0 offset from %fp is definitely wrong, and it's not a fixed offset either (it depends on the number of registers saved in the procedure entry mask). Fortunately, %ap points directly to CFA, so that always works.
> 
> Just some thoughts on future areas for improval... I'm very happy to be able to run the NetBSD testsuite on VAX now. It gives me a lot of confidence as to what works and what doesn't. Most of the stuff I expected to fail (like libm tests, since it's not IEEE FP) failed, and most of the rest succeeded.
> 
> -Jake
> 
> 
>> On Mar 27, 2016, at 15:34, Jake Hamby <jehamby420@me.com> wrote:
>> 
>> I'm very pleased to report that I was able to successfully build a NetBSD/vax system using the checked-in GCC 5.3, with the patches I've submitted, setting FIRST_PSEUDO_REGISTER to 17 and DWARF_FRAME_REGISTERS to 16. The kernel produced with GCC 5.3 crashes (on a real VS4000/90 and also SimH) in UVM, which may be a bug in the kernel that different optimization exposed, or a bug in GCC's generated code.
>> 
>> If you don't set DWARF_FRAME_REGISTERS to 16, then C++ exceptions break again, and GDB suddenly can't deal with the larger debug frames because of the data structure size mismatch between GCC and GDB. But with that additional define, you can raise FIRST_PSEUDO_REGISTER to include PSW (which is correct, since DWARF already uses that meaning), remove the "#ifdef notworking" around the asserts that Christos added in the NetBSD tree, and everything works as well as it did with #define FIRST_PSEUDO_REGISTER 16.
>> 
>> Here's the C++ test case I've been using to verify that the stack unwinding works and that different simple types can be thrown and caught. My ultimate goal is to be able to run GCC's testsuite because I'm far from certain that the OS, or even the majority of packages, really exercise all of the different paths in this very CISC architecture.
>> 
>> #include <string>
>> #include <stdio.h>
>> 
>> int recursive_throw(int i) {
>> printf("enter recursive_throw(%d)\n", i);
>> if (i > 0) {
>>   printf("calling recursive_throw(%d)\n", i - 1);
>>   recursive_throw(i - 1);
>> } else {
>>   printf("throwing exception\n");
>>   throw 456;
>> }
>> printf("exit recursive_throw(%d)\n", i);
>> }
>> 
>> /* Test several kinds of throws. */
>> int throwtest(int test) {
>> switch (test) {
>>   case 0:
>>   case 1:
>>     return test;
>> 
>>   case 2:
>>     throw 123;
>> 
>>   case 3:
>>     throw 123.45;
>> 
>>   case 4:
>>     throw 678.9f;
>> 
>>   case 5:
>>     recursive_throw(6);
>>     return 666;  // fail
>> 
>>   default:
>>     return 999;  // not used in test
>> }
>> }
>> 
>> int main() {
>> for (int i = 0; i < 6; i++) {
>>   try {
>>     int ret = throwtest(i);
>>     printf("throwtest(%d) returned %d\n", i, ret);
>>   } catch (int e) {
>>     printf("Caught int exception: %d\n", e);
>>   } catch (double d) {
>>     printf("Caught double exception: %f\n", d);
>>   } catch (float f) {
>>     printf("Caught float exception: %f\n", (double)f);
>>   }
>> }
>> }
>> 
>> I'm pleased that I got it working, but the change I made to except.c to add:
>> 
>> RTX_FRAME_RELATED_P (insn) = 1;
>> 
>> below:
>> 
>> #ifdef EH_RETURN_HANDLER_RTX
>>     rtx insn = emit_move_insn (EH_RETURN_HANDLER_RTX, crtl->eh.ehr_handler);
>> 
>> isn't really correct, I don't think. It adds an additional .cfi directive that wasn't there before, and a GCC ./buildsh release fails building unwind-dw2.c (that's the place where the build either succeeds or fails or generates bad code) if you try to compile with assertions (and it doesn't without my change to except.c).
>> 
>> Unfortunately, I don't have a patch for the root cause for me having to add that line to except.c, which is that the required mov instruction to copy the __builtin_eh_return() return address into the old stack frame is being deleted for some reason, otherwise. Since I know the #ifdef EH_RETURN_HANDLER_RTX line *is* staying in the final code on other archs, I presume the problem is something VAX-related in the .md file.
>> 
>> If anyone knows about GCC's liveness detection, and specifically any potential problems that might cause this to be happening (removing a required "emit_move_insn (EH_RETURN_HANDLER_RTX, ...)" before a return call that was added in expand_eh_return () but then deleted if -O or higher is used), any advice would be appreciated as to where to look.
>> 
>> What I'm working on now is cleaning up and refactoring the .md insn definitions, but I'm not ready to share that code until it works and does something useful. I'm hoping to be able to optimize the removal of unneeded tst / cmp instructions when the condition codes have been set to something useful by a previous insn. I don't think the code in vax_notice_update_cc () is necessarily correct, and it's very difficult to understand exactly how it's working, because it's trying to determine this based entirely on looking at the RTL of the insn (set, call, zero_extract, etc), which I think may have become out of sync with the types of instructions that are actually emitted in vax.md for those operations.
>> 
>> So I've just finished tagging the define_insn calls in vax.md with a "cc" attribute (like the avr backend) which my (hopefully more correct and more optimized) version of vax_notice_update_cc will use to know exactly what the flag status is after the insn, for Z, N, and C. Here's my definition of "cc". I'll share the rest after I'm sure that it works.
>> 
>> ;; Condition code settings.  On VAX, the default value is "clobber".
>> ;; The V flag is often set to zero, or else it has a special meaning,
>> ;; usually related to testing for a signed integer range overflow.
>> ;; "cmp_czn", "cmp_zn", and "cmp_z" are all assumed to modify V, and
>> ;; none is expected to modify C.  "plus" is used after an add/sub,
>> ;; when the flags, including C, return info about the result,
>> ;; including a carry bit to use with, e.g. "adwc".
>> 
>> ;; The important thing to note is that the C flag, in the case of "plus",
>> ;; doesn't reflect the results of a signed integer comparison,
>> ;; as "cmp_czn" does.  Finally, "cmp_zn_use_c" and "cmp_z_use_cn" indicate
>> ;; that all four flags are updated: Z and N, or Z alone, will be a comparison,
>> ;; but C is set to 0, or some other value, instead of retaining its old value.
>> ;; Only instructions of type "compare" set the C, Z, and N flags correctly
>> ;; for both signed and unsigned ordered comparisons.
>> 
>> ;; For branching, only Z is needed to test for equality, between two
>> ;; operands or between the first operand and 0.  The N flag is necessary
>> ;; to test for less than zero, and for FP or signed integer comparisons.
>> ;; Both N and Z are required to branch on signed (a <= b) or (a > b).
>> ;; For comparing unsigned integers, the C flag is used instead of N.
>> ;; Both C and Z are required to branch on unsigned (a <= b) or (a > b).
>> 
>> ;; The VAX instruction set is biased towards leaving C alone, relative to
>> ;; the other flags, which tend to be modified on almost every instruction.
>> ;; It's possible to cache the results of two signed int comparison,
>> ;; as long as they're of the form (a < b) or (a >= b), where b may be 0,
>> ;; in the C flag, while other instructions modify the other flags. Then,
>> ;; a test for a branch can be saved later by referring to the previous value.
>> ;; The cc attributes are intended so that this optimization may be performed.
>> 
>> (define_attr "cc" "none,cmp_czn,cmp_zn,cmp_zn_use_c,
>> 			cmp_z,cmp_z_use_czn,plus,clobber"
>> (const_string "clobber"))
>> 
>> 
>> I'll send another email once I have something substantial to contribute. I give my permission to NetBSD and the FSF to integrate any or all of my changes under the copyright terms of the original files. Please let me know if I have to do any additional legal stuff for code contributions. I'm doing this on my own time and not on behalf of any company or organization.
>> 
>> Best regards,
>> Jake
>> 
>> 
>>> On Mar 26, 2016, at 07:38, Jake Hamby <jehamby420@me.com> wrote:
>>> 
>>> Unfortunately, my previous patch that included a change to gcc/config/vax/vax.h that increased FIRST_PSEUDO_REGISTER from 16 to 17 breaks the C++ exception handling that I’d worked so hard to get right with the rest of the patch. I believe I need to define DWARF_FRAME_REGISTERS to 16 in the same file to fix the size of the array that libgcc/unwind-dw2.c creates. The i386 backend and several others also define it their .h file for the same reason (compatibility with hardcoded frame offsets).
>>> 
>>> Here’s the first part of the patch to vax.h that increases FIRST_PSEUDO_REGISTER and also adds a definition of DWARF_FRAME_REGISTERS as 16, with suitable comment. I’m testing it now. I know that C++ exceptions were working before I increased FIRST_PSEUDO_REGISTER to 17.
>>> 
>>> Regards,
>>> Jake
>>> 
>>> Index: external/gpl3/gcc.old/dist/gcc/config/vax/vax.h
>>> ===================================================================
>>> RCS file: /cvsroot/src/external/gpl3/gcc.old/dist/gcc/config/vax/vax.h,v
>>> retrieving revision 1.3
>>> diff -u -r1.3 vax.h
>>> --- external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	23 Sep 2015 03:39:18 -0000	1.3
>>> +++ external/gpl3/gcc.old/dist/gcc/config/vax/vax.h	26 Mar 2016 14:34:29 -0000
>>> @@ -119,13 +119,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
>>> 
>> 
> 

Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jake Hamby]

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.

-Jake

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
 ;; register-register mov instructions take 3 bytes and 2 CPU cycles.  clrl
@@ -138,8 +164,7 @@
   [(set (match_operand:SI 0 "nonimmediate_operand" "")
 	(match_operand:SI 1 "general_operand" ""))]
   ""
-  "
-{
+  "{
 #ifdef NO_EXTERNAL_INDIRECT_ADDRESS
   if (flag_pic
       && GET_CODE (operands[1]) == CONST
@@ -154,24 +179,35 @@
       DONE;
     }
 #endif
-}")
+  }")
 
 (define_insn "movsi_2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(match_operand:SI 1 "nonsymbolic_operand" "nrmT"))]
   ""
-  "* return vax_output_int_move (insn, operands, SImode);")
+  "* return vax_output_int_move (insn, operands, SImode);"
+  [(set_attr "cc" "clobber")])
 
 (define_insn "mov<mode>"
   [(set (match_operand:VAXintQH 0 "nonimmediate_operand" "=g")
 	(match_operand:VAXintQH 1 "general_operand" "g"))]
   ""
-  "* return vax_output_int_move (insn, operands, <MODE>mode);")
+  "* return vax_output_int_move (insn, operands, <MODE>mode);"
+  [(set_attr "cc" "clobber")])
+
+;; These cases all set Z and N to comparison with zero.
+;; The case of a constant from 64 to 255 is handled below.
+(define_expand "movstricthi"
+  [(set (strict_low_part (match_operand:HI 0 "register_operand" "+g"))
+	(match_operand:HI 1 "general_operand" "g"))])
 
-(define_insn "movstricthi"
+(define_insn "*movstricthi"
   [(set (strict_low_part (match_operand:HI 0 "register_operand" "+g"))
 	(match_operand:HI 1 "general_operand" "g"))]
-  ""
+  "!CONST_INT_P (operands[1])
+    || ((unsigned int)INTVAL (operands[1]) < 64)
+    || ((unsigned int)~INTVAL (operands[1]) < 64)
+    || ((unsigned int)INTVAL (operands[1]) > 255)"
   "*
 {
   if (CONST_INT_P (operands[1]))
@@ -184,11 +220,22 @@
       else if ((unsigned int)~i < 64)
 	return \"mcomw %H1,%0\";
       else if ((unsigned int)i < 256)
-	return \"movzbw %1,%0\";
+	gcc_unreachable ();
+	/* return \"movzbw %1,%0\"; */
     }
   return \"movw %1,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
+;; This variant is different because it only sets the Z flag.
+(define_insn "*movstricthi_2"
+  [(set (strict_low_part (match_operand:HI 0 "register_operand" "+g"))
+	(match_operand:HI 1 "general_operand" "g"))]
+  "CONST_INT_P (operands[1]) && ((unsigned int)INTVAL (operands[1]) < 256)"
+  "movzbw %1,%0"
+  [(set_attr "cc" "clobber")])
+
+;; This insn is missing the "movzbw" variant of the previous one.
 (define_insn "movstrictqi"
   [(set (strict_low_part (match_operand:QI 0 "register_operand" "+g"))
 	(match_operand:QI 1 "general_operand" "g"))]
@@ -204,7 +251,8 @@
 	return \"mcomb %B1,%0\";
     }
   return \"movb %1,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
 ;; This is here to accept 4 arguments and pass the first 3 along
 ;; to the movmemhi1 pattern that really does the work.
@@ -235,7 +283,8 @@
 	(match_operand:BLK 1 "memory_operand" "B"))
    (use (match_operand:SI 2 "const_int_operand" "g"))]
   "INTVAL (operands[2]) <= 48"
-  "* return vax_output_movmemsi (insn, operands);")
+  "* return vax_output_movmemsi (insn, operands);"
+  [(set_attr "cc" "clobber")])
 
 (define_insn "movmemhi1"
   [(set (match_operand:BLK 0 "memory_operand" "=o")
@@ -248,75 +297,94 @@
    (clobber (reg:SI 4))
    (clobber (reg:SI 5))]
   ""
-  "movc3 %2,%1,%0")
+  "movc3 %2,%1,%0"
+  [(set_attr "cc" "clobber")])
 

 ;; Extension and truncation insns.
 
+;; The cvt instructions update all four flags.  Z and N can be used for
+;; equality to 0 and for signed integer comparison to 0.  C is set to 0.
+;; The V flag indicates an overflow on truncation to a smaller type.
+
 (define_insn "truncsiqi2"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=g")
 	(truncate:QI (match_operand:SI 1 "nonimmediate_operand" "nrmT")))]
   ""
-  "cvtlb %1,%0")
+  "cvtlb %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "truncsihi2"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=g")
 	(truncate:HI (match_operand:SI 1 "nonimmediate_operand" "nrmT")))]
   ""
-  "cvtlw %1,%0")
+  "cvtlw %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "trunchiqi2"
   [(set (match_operand:QI 0 "nonimmediate_operand" "=g")
 	(truncate:QI (match_operand:HI 1 "nonimmediate_operand" "g")))]
   ""
-  "cvtwb %1,%0")
+  "cvtwb %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "extendhisi2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "g")))]
   ""
-  "cvtwl %1,%0")
+  "cvtwl %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "extendqihi2"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=g")
 	(sign_extend:HI (match_operand:QI 1 "nonimmediate_operand" "g")))]
   ""
-  "cvtbw %1,%0")
+  "cvtbw %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "extendqisi2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(sign_extend:SI (match_operand:QI 1 "nonimmediate_operand" "g")))]
   ""
-  "cvtbl %1,%0")
+  "cvtbl %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "extendsfdf2"
   [(set (match_operand:DF 0 "nonimmediate_operand" "=g")
 	(float_extend:DF (match_operand:SF 1 "general_operand" "gF")))]
   ""
-  "cvtf%# %1,%0")
+  "cvtf%# %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "truncdfsf2"
   [(set (match_operand:SF 0 "nonimmediate_operand" "=g")
 	(float_truncate:SF (match_operand:DF 1 "general_operand" "gF")))]
   ""
-  "cvt%#f %1,%0")
+  "cvt%#f %1,%0"
+  [(set_attr "cc" "compare")])
+
+;; The movzbw, movzbl, movzwl instructions set the Z flag for (%0 == 0).
+;; N and V are cleared and C is not modified.  Only Z is useful to test.
 
 (define_insn "zero_extendhisi2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(zero_extend:SI (match_operand:HI 1 "nonimmediate_operand" "g")))]
   ""
-  "movzwl %1,%0")
+  "movzwl %1,%0"
+  [(set_attr "cc" "clobber")])
 
 (define_insn "zero_extendqihi2"
   [(set (match_operand:HI 0 "nonimmediate_operand" "=g")
 	(zero_extend:HI (match_operand:QI 1 "nonimmediate_operand" "g")))]
   ""
-  "movzbw %1,%0")
+  "movzbw %1,%0"
+  [(set_attr "cc" "clobber")])
 
 (define_insn "zero_extendqisi2"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "g")))]
   ""
-  "movzbl %1,%0")
+  "movzbl %1,%0"
+  [(set_attr "cc" "clobber")])
 

 ;; Fix-to-float conversion insns.
 
@@ -324,7 +392,8 @@
   [(set (match_operand:VAXfp 0 "nonimmediate_operand" "=g")
 	(float:VAXfp (match_operand:VAXint 1 "nonimmediate_operand" "g")))]
   ""
-  "cvt<VAXint:isfx><VAXfp:fsfx> %1,%0")
+  "cvt<VAXint:isfx><VAXfp:fsfx> %1,%0"
+  [(set_attr "cc" "compare")])
 
 ;; Float-to-fix conversion insns.
 
@@ -332,12 +401,12 @@
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g")
 	(fix:VAXint (match_operand:VAXfp 1 "general_operand" "gF")))]
   ""
-  "cvt<VAXfp:fsfx><VAXint:isfx> %1,%0")
+  "cvt<VAXfp:fsfx><VAXint:isfx> %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_expand "fixuns_trunc<VAXfp:mode><VAXint:mode>2"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "")
-	(fix:VAXint (match_operand:VAXfp 1 "general_operand")))]
-  "")
+	(fix:VAXint (match_operand:VAXfp 1 "general_operand")))])
 

 ;;- All kinds of add instructions.
 
@@ -349,42 +418,48 @@
   "@
    add<VAXfp:fsfx>2 %2,%0
    add<VAXfp:fsfx>2 %1,%0
-   add<VAXfp:fsfx>3 %1,%2,%0")
+   add<VAXfp:fsfx>3 %1,%2,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "pushlclsymreg"
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(plus:SI (match_operand:SI 1 "register_operand" "%r")
 		 (match_operand:SI 2 "local_symbolic_operand" "i")))]
   "flag_pic"
-  "pushab %a2[%1]")
+  "pushab %a2[%1]"
+  [(set_attr "cc" "compare")])
 
 (define_insn "pushextsymreg"
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(plus:SI (match_operand:SI 1 "register_operand" "%r")
 		 (match_operand:SI 2 "external_symbolic_operand" "i")))]
   "flag_pic"
-  "pushab %a2[%1]")
+  "pushab %a2[%1]"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movlclsymreg"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(plus:SI (match_operand:SI 1 "register_operand" "%r")
 		 (match_operand:SI 2 "local_symbolic_operand" "i")))]
   "flag_pic"
-  "movab %a2[%1],%0")
+  "movab %a2[%1],%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movextsymreg"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(plus:SI (match_operand:SI 1 "register_operand" "%r")
 		 (match_operand:SI 2 "external_symbolic_operand" "i")))]
   "flag_pic"
-  "movab %a2[%1],%0")
+  "movab %a2[%1],%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "add<mode>3"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g")
 	(plus:VAXint (match_operand:VAXint 1 "general_operand" "nrmT")
 		     (match_operand:VAXint 2 "general_operand" "nrmT")))]
   ""
-  "* return vax_output_int_add (insn, operands, <MODE>mode);")
+  "* return vax_output_int_add (insn, operands, <MODE>mode);"
+  [(set_attr "cc" "clobber")])
 
 (define_expand "adddi3"
   [(set (match_operand:DI 0 "nonimmediate_operand" "=g")
@@ -398,7 +473,8 @@
 	(plus:DI (match_operand:DI 1 "general_addsub_di_operand" "%0")
 		 (match_operand:DI 2 "general_addsub_di_operand" "nRr")))]
   "TARGET_QMATH"
-  "* return vax_output_int_add (insn, operands, DImode);")
+  "* return vax_output_int_add (insn, operands, DImode);"
+  [(set_attr "cc" "clobber")])
 
 ;; The add-with-carry (adwc) instruction only accepts two operands.
 (define_insn "adddi3_old"
@@ -406,7 +482,8 @@
 	(plus:DI (match_operand:DI 1 "general_operand" "%0,ro>")
 		 (match_operand:DI 2 "general_operand" "Fsro,Fs")))]
   "!TARGET_QMATH"
-  "* return vax_output_int_add (insn, operands, DImode);")
+  "* return vax_output_int_add (insn, operands, DImode);"
+  [(set_attr "cc" "clobber")])
 

 ;;- All kinds of subtract instructions.
 
@@ -417,7 +494,8 @@
   ""
   "@
    sub<VAXfp:fsfx>2 %2,%0
-   sub<VAXfp:fsfx>3 %2,%1,%0")
+   sub<VAXfp:fsfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "sub<mode>3"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g,g")
@@ -426,7 +504,8 @@
   ""
   "@
    sub<VAXint:isfx>2 %2,%0
-   sub<VAXint:isfx>3 %2,%1,%0")
+   sub<VAXint:isfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_expand "subdi3"
   [(set (match_operand:DI 0 "nonimmediate_operand" "=g")
@@ -440,7 +519,8 @@
 	(minus:DI (match_operand:DI 1 "general_addsub_di_operand" "0,I")
 		  (match_operand:DI 2 "general_addsub_di_operand" "nRr,Rr")))]
   "TARGET_QMATH"
-  "* return vax_output_int_subtract (insn, operands, DImode);")
+  "* return vax_output_int_subtract (insn, operands, DImode);"
+  [(set_attr "cc" "clobber")])
 
 ;; The subtract-with-carry (sbwc) instruction only takes two operands.
 (define_insn "subdi3_old"
@@ -448,7 +528,8 @@
 	(minus:DI (match_operand:DI 1 "general_operand" "0,or>")
 		  (match_operand:DI 2 "general_operand" "Fsor,Fs")))]
   "!TARGET_QMATH"
-  "* return vax_output_int_subtract (insn, operands, DImode);")
+  "* return vax_output_int_subtract (insn, operands, DImode);"
+  [(set_attr "cc" "clobber")])
 

 ;;- Multiply instructions.
 
@@ -460,7 +541,8 @@
   "@
    mul<VAXfp:fsfx>2 %2,%0
    mul<VAXfp:fsfx>2 %1,%0
-   mul<VAXfp:fsfx>3 %1,%2,%0")
+   mul<VAXfp:fsfx>3 %1,%2,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "mul<mode>3"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g,g,g")
@@ -470,7 +552,8 @@
   "@
    mul<VAXint:isfx>2 %2,%0
    mul<VAXint:isfx>2 %1,%0
-   mul<VAXint:isfx>3 %1,%2,%0")
+   mul<VAXint:isfx>3 %1,%2,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "mulsidi3"
   [(set (match_operand:DI 0 "nonimmediate_operand" "=g")
@@ -479,7 +562,8 @@
 		 (sign_extend:DI
 		  (match_operand:SI 2 "nonimmediate_operand" "nrmT"))))]
   ""
-  "emul %1,%2,$0,%0")
+  "emul %1,%2,$0,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:DI 0 "nonimmediate_operand" "=g")
@@ -490,7 +574,8 @@
 		   (match_operand:SI 2 "nonimmediate_operand" "nrmT")))
 	 (sign_extend:DI (match_operand:SI 3 "nonimmediate_operand" "g"))))]
   ""
-  "emul %1,%2,%3,%0")
+  "emul %1,%2,%3,%0"
+  [(set_attr "cc" "compare")])
 
 ;; 'F' constraint means type CONST_DOUBLE
 (define_insn ""
@@ -508,7 +593,8 @@
   if (CONST_DOUBLE_HIGH (operands[3]))
     operands[3] = GEN_INT (CONST_DOUBLE_LOW (operands[3]));
   return \"emul %1,%2,%3,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 

 ;;- Divide instructions.
 
@@ -519,7 +605,8 @@
   ""
   "@
    div<VAXfp:fsfx>2 %2,%0
-   div<VAXfp:fsfx>3 %2,%1,%0")
+   div<VAXfp:fsfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "div<mode>3"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g,g")
@@ -528,19 +615,21 @@
   ""
   "@
    div<VAXint:isfx>2 %2,%0
-   div<VAXint:isfx>3 %2,%1,%0")
+   div<VAXint:isfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 ;This is left out because it is very slow;
 ;we are better off programming around the "lack" of this insn.
-;(define_insn "divmoddisi4"
-;  [(set (match_operand:SI 0 "general_operand" "=g")
-;	(div:SI (match_operand:DI 1 "general_operand" "g")
-;		(match_operand:SI 2 "general_operand" "g")))
-;   (set (match_operand:SI 3 "general_operand" "=g")
-;	(mod:SI (match_operand:DI 1 "general_operand" "g")
-;		(match_operand:SI 2 "general_operand" "g")))]
-;  ""
-;  "ediv %2,%1,%0,%3")
+;; FIXME: uncommented to see if it is ever used.
+(define_insn "divmoddisi4"
+  [(parallel [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+		   (div:SI (match_operand:DI 1 "general_operand" "nrmT")
+			   (match_operand:SI 2 "general_operand" "nrmT")))
+	      (set (match_operand:SI 3 "nonimmediate_operand" "=g")
+		   (mod:SI (match_dup 1) (match_dup 2)))])]
+  ""
+  "ediv %2,%1,%0,%3"
+  [(set_attr "cc" "compare")])
 

 ;; Bit-and on the VAX is done with a clear-bits insn.
 (define_expand "and<mode>3"
@@ -573,7 +662,8 @@
   ""
   "@
    bic<VAXint:isfx>2 %1,%0
-   bic<VAXint:isfx>3 %1,%2,%0")
+   bic<VAXint:isfx>3 %1,%2,%0"
+  [(set_attr "cc" "compare")])
 
 ;; The following used to be needed because constant propagation can
 ;; create them starting from the bic insn patterns above.  This is no
@@ -587,7 +677,8 @@
   ""
   "@
    bic<VAXint:isfx>2 %<VAXint:iprefx>2,%0
-   bic<VAXint:isfx>3 %<VAXint:iprefx>2,%1,%0")
+   bic<VAXint:isfx>3 %<VAXint:iprefx>2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 

 ;;- Bit set instructions.
@@ -600,7 +691,8 @@
   "@
    bis<VAXint:isfx>2 %2,%0
    bis<VAXint:isfx>2 %1,%0
-   bis<VAXint:isfx>3 %2,%1,%0")
+   bis<VAXint:isfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 ;;- xor instructions.
 
@@ -612,26 +704,30 @@
   "@
    xor<VAXint:isfx>2 %2,%0
    xor<VAXint:isfx>2 %1,%0
-   xor<VAXint:isfx>3 %2,%1,%0")
+   xor<VAXint:isfx>3 %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 

 (define_insn "neg<mode>2"
   [(set (match_operand:VAXfp 0 "nonimmediate_operand" "=g")
 	(neg:VAXfp (match_operand:VAXfp 1 "general_operand" "gF")))]
   ""
-  "mneg<VAXfp:fsfx> %1,%0")
+  "mneg<VAXfp:fsfx> %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "neg<mode>2"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g")
 	(neg:VAXint (match_operand:VAXint 1 "general_operand" "nrmT")))]
   ""
-  "mneg<VAXint:isfx> %1,%0")
+  "mneg<VAXint:isfx> %1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "one_cmpl<mode>2"
   [(set (match_operand:VAXint 0 "nonimmediate_operand" "=g")
 	(not:VAXint (match_operand:VAXint 1 "general_operand" "nrmT")))]
   ""
-  "mcom<VAXint:isfx> %1,%0")
+  "mcom<VAXint:isfx> %1,%0"
+  [(set_attr "cc" "compare")])
 
 

 ;; Arithmetic right shift on the VAX works by negating the shift count,
@@ -655,14 +751,16 @@
 	(ashiftrt:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (match_operand:QI 2 "const_int_operand" "n")))]
   ""
-  "ashl $%n2,%1,%0")
+  "ashl $%n2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(ashiftrt:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (neg:QI (match_operand:QI 2 "general_operand" "g"))))]
   ""
-  "ashl %2,%1,%0")
+  "ashl %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "ashlsi3"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
@@ -692,7 +790,8 @@
 	}
     }
   return \"ashl %2,%1,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
 ;; Arithmetic right shift on the VAX works by negating the shift count.
 (define_expand "ashrdi3"
@@ -710,14 +809,16 @@
 	(ashift:DI (match_operand:DI 1 "general_operand" "g")
 		   (match_operand:QI 2 "general_operand" "g")))]
   ""
-  "ashq %2,%D1,%0")
+  "ashq %2,%D1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:DI 0 "nonimmediate_operand" "=g")
 	(ashiftrt:DI (match_operand:DI 1 "general_operand" "g")
 		     (neg:QI (match_operand:QI 2 "general_operand" "g"))))]
   ""
-  "ashq %2,%D1,%0")
+  "ashq %2,%D1,%0"
+  [(set_attr "cc" "compare")])
 
 ;; We used to have expand_shift handle logical right shifts by using extzv,
 ;; but this make it very difficult to do lshrdi3.  Since the VAX is the
@@ -742,7 +843,7 @@
 ;; Rotate right on the VAX works by negating the shift count.
 (define_expand "rotrsi3"
   [(set (match_operand:SI 0 "general_operand" "=g")
-	(rotatert:SI (match_operand:SI 1 "general_operand" "g")
+	(rotatert:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (match_operand:QI 2 "general_operand" "g")))]
   ""
   "
@@ -756,21 +857,24 @@
 	(rotate:SI (match_operand:SI 1 "general_operand" "nrmT")
 		   (match_operand:QI 2 "general_operand" "g")))]
   ""
-  "rotl %2,%1,%0")
+  "rotl %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(rotatert:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (match_operand:QI 2 "const_int_operand" "n")))]
   ""
-  "rotl %R2,%1,%0")
+  "rotl %R2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(rotatert:SI (match_operand:SI 1 "general_operand" "nrmT")
 		     (neg:QI (match_operand:QI 2 "general_operand" "g"))))]
   ""
-  "rotl %2,%1,%0")
+  "rotl %2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 ;This insn is probably slower than a multiply and an add.
 ;(define_insn ""
@@ -779,45 +883,64 @@
 ;			  (match_operand:SI 2 "general_operand" "g"))
 ;		 (match_operand:SI 3 "general_operand" "g")))]
 ;  ""
-;  "index %1,$0x80000000,$0x7fffffff,%3,%2,%0")
+;  "index %1,$0x80000000,$0x7fffffff,%3,%2,%0"
+;  [(set_attr "cc" "compare")])
 

 ;; Special cases of bit-field insns which we should
 ;; recognize in preference to the general case.
 ;; These handle aligned 8-bit and 16-bit fields,
 ;; which can usually be done with move instructions.
 
-;; netbsd changed this to REG_P (operands[0]) || (MEM_P (operands[0]) && ...
-;; but gcc made it just !MEM_P (operands[0]) || ...
+;; Split into two insns.  This variant works for register destinations
+;; where operand 2 is a multiple of operand 1.  The insv instruction
+;; is somewhat unusual for VAX in not modifying any of the flags.
+(define_insn ""
+  [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+ro")
+			 (match_operand:QI 1 "const_int_operand" "n")
+			 (match_operand:SI 2 "const_int_operand" "n"))
+	(match_operand:SI 3 "general_operand" "g"))]
+   "(INTVAL (operands[1]) == 8 || INTVAL (operands[1]) == 16)
+   && (INTVAL (operands[2]) != 0) && REG_P (operands[0])
+   && (INTVAL (operands[2]) % INTVAL (operands[1]) == 0)"
+  "insv %3,%2,%1,%0"
+  [(set_attr "cc" "none")])
 
+;; This case handles a destination in memory.
+;; TODO: investigate mode_dependent_address_p () and its meaning.
 (define_insn ""
   [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+ro")
 			 (match_operand:QI 1 "const_int_operand" "n")
 			 (match_operand:SI 2 "const_int_operand" "n"))
 	(match_operand:SI 3 "general_operand" "g"))]
    "(INTVAL (operands[1]) == 8 || INTVAL (operands[1]) == 16)
-   && INTVAL (operands[2]) % INTVAL (operands[1]) == 0
-   && (REG_P (operands[0])
-       || (MEM_P (operands[0])
-          && ! mode_dependent_address_p (XEXP (operands[0], 0),
-				       MEM_ADDR_SPACE (operands[0]))))"
+   && (INTVAL (operands[2]) % INTVAL (operands[1]) == 0)
+   && MEM_P (operands[0])
+   && ! mode_dependent_address_p ( XEXP (operands[0], 0),
+				   MEM_ADDR_SPACE (operands[0]))"
   "*
 {
-  if (REG_P (operands[0]))
-    {
-      if (INTVAL (operands[2]) != 0)
-	return \"insv %3,%2,%1,%0\";
-    }
-  else
-    operands[0]
-      = adjust_address (operands[0],
+  operands[0] = adjust_address (operands[0],
 			INTVAL (operands[1]) == 8 ? QImode : HImode,
 			INTVAL (operands[2]) / 8);
 
-  CC_STATUS_INIT;
   if (INTVAL (operands[1]) == 8)
     return \"movb %3,%0\";
   return \"movw %3,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
+
+;; The "extzv" variant sets N and Z, but movz* sets Z only.
+;; Split them into two definitions for different cc attributes.
+
+(define_insn ""
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=&g")
+	(zero_extract:SI (match_operand:SI 1 "register_operand" "ro")
+			 (match_operand:QI 2 "const_int_operand" "n")
+			 (match_operand:SI 3 "const_int_operand" "n")))]
+  "REG_P (operands[1]) && INTVAL (operands[3]) != 0
+   && (INTVAL (operands[3]) + INTVAL (operands[2]) < 32)"
+  "extzv %3,%2,%1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=&g")
@@ -826,19 +949,12 @@
 			 (match_operand:SI 3 "const_int_operand" "n")))]
   "(INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16)
    && INTVAL (operands[3]) % INTVAL (operands[2]) == 0
-   && (REG_P (operands[1])
-       || (MEM_P (operands[1])
-          && ! mode_dependent_address_p (XEXP (operands[1], 0),
-				      MEM_ADDR_SPACE (operands[1]))))"
+   && MEM_P (operands[1])
+   && ! mode_dependent_address_p (XEXP (operands[1], 0),
+				  MEM_ADDR_SPACE (operands[1]))"
   "*
 {
-  if (REG_P (operands[1]))
-    {
-      if (INTVAL (operands[3]) != 0)
-	return \"extzv %3,%2,%1,%0\";
-    }
-  else
-    operands[1]
+  operands[1]
       = adjust_address (operands[1],
 			INTVAL (operands[2]) == 8 ? QImode : HImode,
 			INTVAL (operands[3]) / 8);
@@ -846,7 +962,8 @@
   if (INTVAL (operands[2]) == 8)
     return \"movzbl %1,%0\";
   return \"movzwl %1,%0\";
-}")
+}"
+  [(set_attr "cc" "clobber")])
 
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
@@ -875,7 +992,8 @@
   if (INTVAL (operands[2]) == 8)
     return \"cvtbl %1,%0\";
   return \"cvtwl %1,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 

 ;; Register-only SImode cases of bit-field insns.
 
@@ -887,7 +1005,8 @@
 			  (match_operand:SI 2 "general_operand" "nrmT"))
 	 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
-  "cmpv %2,%1,%0,%3")
+  "cmpv %2,%1,%0,%3"
+  [(set_attr "cc" "clobber")])
 
 (define_insn ""
   [(set (cc0)
@@ -897,7 +1016,8 @@
 			  (match_operand:SI 2 "general_operand" "nrmT"))
 	 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
-  "cmpzv %2,%1,%0,%3")
+  "cmpzv %2,%1,%0,%3"
+  [(set_attr "cc" "clobber")])
 
 ;; When the field position and size are constant and the destination
 ;; is a register, extv and extzv are much slower than a rotate followed
@@ -919,29 +1039,47 @@
   if (INTVAL (operands[2]) == 8)
     return \"rotl %R3,%1,%0\;cvtbl %0,%0\";
   return \"rotl %R3,%1,%0\;cvtwl %0,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
+;; Split into two insns to mark the movzbl/movzwl variants as clobber.
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(zero_extract:SI (match_operand:SI 1 "register_operand" "ro")
 			 (match_operand:QI 2 "general_operand" "g")
 			 (match_operand:SI 3 "general_operand" "nrmT")))]
-  ""
+  "! CONST_INT_P (operands[3]) || ! CONST_INT_P (operands[2])
+   || ! REG_P (operands[0])
+   || (INTVAL (operands[2]) != 8 && INTVAL (operands[2]) != 16)"
   "*
 {
   if (! CONST_INT_P (operands[3]) || ! CONST_INT_P (operands[2])
       || ! REG_P (operands[0]))
     return \"extzv %3,%2,%1,%0\";
-  if (INTVAL (operands[2]) == 8)
-    return \"rotl %R3,%1,%0\;movzbl %0,%0\";
-  if (INTVAL (operands[2]) == 16)
-    return \"rotl %R3,%1,%0\;movzwl %0,%0\";
   if (INTVAL (operands[3]) & 31)
     return \"rotl %R3,%1,%0\;bicl2 %M2,%0\";
   if (rtx_equal_p (operands[0], operands[1]))
     return \"bicl2 %M2,%0\";
   return \"bicl3 %M2,%1,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
+
+(define_insn ""
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+	(zero_extract:SI (match_operand:SI 1 "register_operand" "ro")
+			 (match_operand:QI 2 "general_operand" "g")
+			 (match_operand:SI 3 "general_operand" "nrmT")))]
+  "INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16"
+  "*
+{
+  if (INTVAL (operands[2]) == 8)
+    return \"rotl %R3,%1,%0\;movzbl %0,%0\";
+  else if (INTVAL (operands[2]) == 16)
+    return \"rotl %R3,%1,%0\;movzwl %0,%0\";
+  else
+    gcc_unreachable ();
+}"
+  [(set_attr "cc" "clobber")])
 
 ;; Non-register cases.
 ;; nonimmediate_operand is used to make sure that mode-ambiguous cases
@@ -955,7 +1093,8 @@
 			  (match_operand:SI 2 "general_operand" "nrmT"))
 	 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
-  "cmpv %2,%1,%0,%3")
+  "cmpv %2,%1,%0,%3"
+  [(set_attr "cc" "clobber")])
 
 (define_insn ""
   [(set (cc0)
@@ -965,7 +1104,8 @@
 			  (match_operand:SI 2 "general_operand" "nrmT"))
 	 (match_operand:SI 3 "general_operand" "nrmT")))]
   ""
-  "cmpzv %2,%1,%0,%3")
+  "cmpzv %2,%1,%0,%3"
+  [(set_attr "cc" "clobber")])
 
 (define_insn "extv"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
@@ -987,7 +1127,8 @@
   if (INTVAL (operands[2]) == 8)
     return \"rotl %R3,%1,%0\;cvtbl %0,%0\";
   return \"rotl %R3,%1,%0\;cvtwl %0,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
 (define_expand "extzv"
   [(set (match_operand:SI 0 "general_operand" "")
@@ -997,26 +1138,49 @@
   ""
   "")
 
+;; This was split into three insns so the CC attribute can be set correctly.
 (define_insn ""
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(zero_extract:SI (match_operand:QI 1 "memory_operand" "m")
 			 (match_operand:QI 2 "general_operand" "g")
 			 (match_operand:SI 3 "general_operand" "nrmT")))]
-  ""
+  "! REG_P (operands[0]) || ! CONST_INT_P (operands[2])
+   || ! CONST_INT_P (operands[3])
+   || INTVAL (operands[2]) + INTVAL (operands[3]) > 32
+   || side_effects_p (operands[1])
+   || (MEM_P (operands[1])
+       && mode_dependent_address_p (XEXP (operands[1], 0),
+				       MEM_ADDR_SPACE (operands[1])))"
+  "extzv %3,%2,%1,%0"
+  [(set_attr "cc" "compare")])
+
+;; These insns end in movzbl/movzwl, so CC can't be used for comparison.
+(define_insn ""
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+	(zero_extract:SI (match_operand:QI 1 "memory_operand" "m")
+			 (match_operand:QI 2 "general_operand" "g")
+			 (match_operand:SI 3 "general_operand" "nrmT")))]
+  "INTVAL (operands[2]) == 8 || INTVAL (operands[2]) == 16"
   "*
 {
-  if (! REG_P (operands[0]) || ! CONST_INT_P (operands[2])
-      || ! CONST_INT_P (operands[3])
-      || INTVAL (operands[2]) + INTVAL (operands[3]) > 32
-      || side_effects_p (operands[1])
-      || (MEM_P (operands[1])
-	  && mode_dependent_address_p (XEXP (operands[1], 0),
-				       MEM_ADDR_SPACE (operands[1]))))
-    return \"extzv %3,%2,%1,%0\";
   if (INTVAL (operands[2]) == 8)
     return \"rotl %R3,%1,%0\;movzbl %0,%0\";
-  if (INTVAL (operands[2]) == 16)
+  else if (INTVAL (operands[2]) == 16)
     return \"rotl %R3,%1,%0\;movzwl %0,%0\";
+  else
+    gcc_unreachable ();
+}"
+  [(set_attr "cc" "clobber")])
+
+;; The final insn in these sequences should set Z and N correctly.
+(define_insn ""
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
+	(zero_extract:SI (match_operand:QI 1 "memory_operand" "m")
+			 (match_operand:QI 2 "general_operand" "g")
+			 (match_operand:SI 3 "general_operand" "nrmT")))]
+  ""
+  "*
+{
   if (MEM_P (operands[1])
       && GET_CODE (XEXP (operands[1], 0)) == PLUS
       && REG_P (XEXP (XEXP (operands[1], 0), 0))
@@ -1040,16 +1204,21 @@
 	return \"extzv %3,%2,%1,%0\";
     }
   return \"rotl %R3,%1,%0\;bicl2 %M2,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
-(define_expand "insv"
+;; Changed name from "insv" to "insvsi" to match standard name for type.
+(define_expand "insvsi"
   [(set (zero_extract:SI (match_operand:SI 0 "general_operand" "")
 			 (match_operand:QI 1 "general_operand" "")
 			 (match_operand:SI 2 "general_operand" ""))
-	(match_operand:SI 3 "general_operand" ""))]
-  ""
-  "")
+	(match_operand:SI 3 "general_operand" ""))])
 
+;; The final insv doesn't touch any flags, but the added instructions to
+;; prepare the operands for it probably will. Also, the condition codes
+;; are unpredictable if a reserved operand fault occurs due to size or
+;; pos being out of the range of the 32-bit destination field.
+;; It's best to tag the entire sequence as clobbering cc.
 (define_insn ""
   [(set (zero_extract:SI (match_operand:QI 0 "memory_operand" "+g")
 			 (match_operand:QI 1 "general_operand" "g")
@@ -1079,22 +1248,29 @@
 	}
     }
   return \"insv %3,%2,%1,%0\";
-}")
+}"
+  [(set_attr "cc" "clobber")])
 
+;; FIXME: this could raise a reserved operand fault if %1 or %3 are
+;; out of the acceptable ranges for a 32-bit field.  Under normal
+;; circumstances, the condition codes should all be unmodified, which
+;; is unusual for a VAX instruction that moves data.
 (define_insn ""
   [(set (zero_extract:SI (match_operand:SI 0 "register_operand" "+r")
 			 (match_operand:QI 1 "general_operand" "g")
 			 (match_operand:SI 2 "general_operand" "nrmT"))
 	(match_operand:SI 3 "general_operand" "nrmT"))]
   ""
-  "insv %3,%2,%1,%0")
+  "insv %3,%2,%1,%0"
+  [(set_attr "cc" "none")])
 

 ;; Unconditional jump
 (define_insn "jump"
   [(set (pc)
 	(label_ref (match_operand 0 "" "")))]
   ""
-  "jbr %l0")
+  "jbr %l0"
+  [(set_attr "cc" "clobber")])
 
 ;; Conditional jumps
 
@@ -1130,7 +1306,8 @@
 		      (label_ref (match_operand 1 "" ""))
 		      (pc)))]
   ""
-  "j%c0 %l1")
+  "j%c0 %l1"
+  [(set_attr "cc" "none")])
 
 ;; Recognize reversed jumps.
 (define_insn "*branch_reversed"
@@ -1141,7 +1318,8 @@
 		      (pc)
 		      (label_ref (match_operand 1 "" ""))))]
   ""
-  "j%C0 %l1") ; %C0 negates condition
+  "j%C0 %l1" ; %C0 negates condition
+  [(set_attr "cc" "none")])
 

 ;; Recognize jbs, jlbs, jbc and jlbc instructions.  Note that the operand
 ;; of jlbs and jlbc insns are SImode in the hardware.  However, if it is
@@ -1160,7 +1338,8 @@
   ""
   "@
    jlbs %0,%l2
-   jbs %1,%0,%l2")
+   jbs %1,%0,%l2"
+  [(set_attr "cc" "none")])
 
 (define_insn ""
   [(set (pc)
@@ -1174,7 +1353,8 @@
   ""
   "@
    jlbc %0,%l2
-   jbc %1,%0,%l2")
+   jbc %1,%0,%l2"
+  [(set_attr "cc" "none")])
 
 (define_insn ""
   [(set (pc)
@@ -1188,7 +1368,8 @@
   ""
   "@
    jlbs %0,%l2
-   jbs %1,%0,%l2")
+   jbs %1,%0,%l2"
+  [(set_attr "cc" "none")])
 
 (define_insn ""
   [(set (pc)
@@ -1202,7 +1383,8 @@
   ""
   "@
    jlbc %0,%l2
-   jbc %1,%0,%l2")
+   jbc %1,%0,%l2"
+  [(set_attr "cc" "none")])
 

 ;; Subtract-and-jump and Add-and-jump insns.
 ;; These are not used when output is for the Unix assembler
@@ -1216,13 +1398,14 @@
 	 (gt (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int -1))
 	     (const_int 0))
-	 (label_ref (match_operand 1 "" ""))
+	 (label_ref (match_operand 1))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int -1)))]
   "!TARGET_UNIX_ASM"
-  "jsobgtr %0,%l1")
+  "jsobgtr %0,%l1"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (pc)
@@ -1230,13 +1413,14 @@
 	 (ge (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int -1))
 	     (const_int 0))
-	 (label_ref (match_operand 1 "" ""))
+	 (label_ref (match_operand 1))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int -1)))]
   "!TARGET_UNIX_ASM"
-  "jsobgeq %0,%l1")
+  "jsobgeq %0,%l1"
+  [(set_attr "cc" "compare")])
 
 ;; Normal aob insns.  Define a version for when operands[1] is a constant.
 (define_insn ""
@@ -1245,26 +1429,28 @@
 	 (lt (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int 1))
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int 1)))]
   "!TARGET_UNIX_ASM"
-  "jaoblss %1,%0,%l2")
+  "jaoblss %1,%0,%l2"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (pc)
 	(if_then_else
 	 (lt (match_operand:SI 0 "nonimmediate_operand" "+g")
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int 1)))]
   "!TARGET_UNIX_ASM && CONST_INT_P (operands[1])"
-  "jaoblss %P1,%0,%l2")
+  "jaoblss %P1,%0,%l2"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (pc)
@@ -1272,26 +1458,28 @@
 	 (le (plus:SI (match_operand:SI 0 "nonimmediate_operand" "+g")
 		      (const_int 1))
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int 1)))]
   "!TARGET_UNIX_ASM"
-  "jaobleq %1,%0,%l2")
+  "jaobleq %1,%0,%l2"
+  [(set_attr "cc" "compare")])
 
 (define_insn ""
   [(set (pc)
 	(if_then_else
 	 (le (match_operand:SI 0 "nonimmediate_operand" "+g")
 	     (match_operand:SI 1 "general_operand" "nrmT"))
-	 (label_ref (match_operand 2 "" ""))
+	 (label_ref (match_operand 2))
 	 (pc)))
    (set (match_dup 0)
 	(plus:SI (match_dup 0)
 		 (const_int 1)))]
   "!TARGET_UNIX_ASM && CONST_INT_P (operands[1])"
-  "jaobleq %P1,%0,%l2")
+  "jaobleq %P1,%0,%l2"
+  [(set_attr "cc" "compare")])
 
 ;; Something like a sob insn, but compares against -1.
 ;; This finds `while (foo--)' which was changed to `while (--foo != -1)'.
@@ -1307,8 +1495,14 @@
 	(plus:SI (match_dup 0)
 		 (const_int -1)))]
   ""
-  "decl %0\;jgequ %l1")
+  "decl %0\;jgequ %l1"
+  [(set_attr "cc" "compare")])
 

+;; Note that operand 1 is total size of args, in bytes,
+;; and what the call insn wants is the number of words.
+;; It is used in the call instruction as a byte, but in the addl2 as
+;; a word.  Since the only time we actually use it in the call instruction
+;; is when it is a constant, SImode (for addl2) is the proper mode.
 (define_expand "call_pop"
   [(parallel [(call (match_operand:QI 0 "memory_operand" "")
 		    (match_operand:SI 1 "const_int_operand" ""))
@@ -1317,12 +1511,7 @@
 			    (match_operand:SI 3 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[3]) <= 255 * 4 && INTVAL (operands[3]) % 4 == 0);
-
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[3]) + 4);
+  gcc_assert (INTVAL (operands[1]) <= 255 * 4);
 })
 
 (define_insn "*call_pop"
@@ -1332,9 +1521,11 @@
 					(match_operand:SI 2 "immediate_operand" "i")))]
   ""
 {
-  operands[1] = GEN_INT ((INTVAL (operands[1]) - 4) / 4);
+  operands[1] = GEN_INT ((INTVAL (operands[1]) + 3) / 4);
   return "calls %1,%0";
-})
+}
+  [(set_attr "cc" "none")])
+
 
 (define_expand "call_value_pop"
   [(parallel [(set (match_operand 0 "" "")
@@ -1345,12 +1536,7 @@
 			    (match_operand:SI 4 "immediate_operand" "")))])]
   ""
 {
-  gcc_assert (INTVAL (operands[4]) <= 255 * 4 && INTVAL (operands[4]) % 4 == 0);
-
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[4]) + 4);
+  gcc_assert (INTVAL (operands[2]) <= 255 * 4);
 })
 
 (define_insn "*call_value_pop"
@@ -1360,49 +1546,29 @@
    (set (reg:SI VAX_SP_REGNUM) (plus:SI (reg:SI VAX_SP_REGNUM)
 					(match_operand:SI 3 "immediate_operand" "i")))]
   ""
-  "*
 {
-  operands[2] = GEN_INT ((INTVAL (operands[2]) - 4) / 4);
-  return \"calls %2,%1\";
-}")
+  operands[2] = GEN_INT ((INTVAL (operands[2]) + 3) / 4);
+  return "calls %2,%1";
+}
+  [(set_attr "cc" "none")])
 
-(define_expand "call"
-  [(call (match_operand:QI 0 "memory_operand" "")
-      (match_operand:SI 1 "const_int_operand" ""))]
-  ""
-  "
-{
-  /* Operand 1 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[1] = GEN_INT (INTVAL (operands[1]) + 4);
-}")
-
-(define_insn "*call"
-   [(call (match_operand:QI 0 "memory_operand" "m")
-	  (match_operand:SI 1 "const_int_operand" ""))]
-  ""
-  "calls $0,%0")
 
-(define_expand "call_value"
-  [(set (match_operand 0 "" "")
-      (call (match_operand:QI 1 "memory_operand" "")
-	    (match_operand:SI 2 "const_int_operand" "")))]
+;; Define another set of these for the case of functions with no operands.
+;; These will allow the optimizers to do a slightly better job.
+(define_insn "call"
+  [(call (match_operand:QI 0 "memory_operand" "m")
+	 (const_int 0))]
   ""
-  "
-{
-  /* Operand 2 is the number of bytes to be popped by DW_CFA_GNU_args_size
-     during EH unwinding.  We must include the argument count pushed by
-     the calls instruction.  */
-  operands[2] = GEN_INT (INTVAL (operands[2]) + 4);
-}")
+  "calls $0,%0"
+  [(set_attr "cc" "none")])
 
-(define_insn "*call_value"
+(define_insn "call_value"
   [(set (match_operand 0 "" "")
 	(call (match_operand:QI 1 "memory_operand" "m")
-	      (match_operand:SI 2 "const_int_operand" "")))]
+	      (const_int 0)))]
   ""
-  "calls $0,%1")
+  "calls $0,%1"
+  [(set_attr "cc" "none")])
 
 ;; Call subroutine returning any type.
 
@@ -1439,7 +1605,8 @@
 (define_insn "blockage"
   [(unspec_volatile [(const_int 0)] VUNSPEC_BLOCKAGE)]
   ""
-  "")
+  ""
+  [(set_attr "cc" "clobber")])
 
 (define_insn "procedure_entry_mask"
   [(unspec_volatile [(match_operand 0 "const_int_operand")] VUNSPEC_PEM)]
@@ -1449,7 +1616,8 @@
 (define_insn "return"
   [(return)]
   ""
-  "ret")
+  "ret"
+  [(set_attr "cc" "clobber")])
 
 (define_expand "prologue"
   [(const_int 0)]
@@ -1471,7 +1639,8 @@
 (define_insn "nop"
   [(const_int 0)]
   ""
-  "nop")
+  "nop"
+  [(set_attr "cc" "none")])
 
 ;; This had a wider constraint once, and it had trouble.
 ;; If you are tempted to try `g', please don't--it's not worth
@@ -1479,7 +1648,8 @@
 (define_insn "indirect_jump"
   [(set (pc) (match_operand:SI 0 "register_operand" "r"))]
   ""
-  "jmp (%0)")
+  "jmp (%0)"
+  [(set_attr "cc" "clobber")])
 
 ;; This is here to accept 5 arguments (as passed by expand_end_case)
 ;; and pass the first 4 along to the casesi1 pattern that really does
@@ -1542,31 +1712,43 @@
 			  (pc))))
 		 (label_ref:SI (match_operand 2 "" ""))))]
   ""
-  "casel %0,$0,%1")
+  "casel %0,$0,%1"
+  [(set_attr "cc" "clobber")])
 

+;; Note that the flags will be set based on the value pushed to stack %0,
+;; which will be the address of operand 1.  V is cleared, C is unmodified.
+;; I believe these pattern names are used to patch up linker references to
+;; external symbols in shared libraries.  If the assembler or linker ever
+;; converts such a reference to more than the single "pushab" operand,
+;; then cc should be changed to "clobber".  If it's only used to patch up
+;; a numeric reference, and the optimizer isn't confused, VZN should work.
 (define_insn "pushextsym"
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(match_operand:SI 1 "external_symbolic_operand" "i"))]
   ""
-  "pushab %a1")
+  "pushab %a1"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movextsym"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(match_operand:SI 1 "external_symbolic_operand" "i"))]
   ""
-  "movab %a1,%0")
+  "movab %a1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "pushlclsym"
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(match_operand:SI 1 "local_symbolic_operand" "i"))]
   ""
-  "pushab %a1")
+  "pushab %a1"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movlclsym"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(match_operand:SI 1 "local_symbolic_operand" "i"))]
   ""
-  "movab %a1,%0")
+  "movab %a1,%0"
+  [(set_attr "cc" "compare")])
 

 ;;- load or push effective address
 ;; These come after the move and add/sub patterns
@@ -1580,25 +1762,29 @@
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(match_operand:VAXintQHSD 1 "address_operand" "p"))]
   ""
-  "pusha<VAXintQHSD:isfx> %a1")
+  "pusha<VAXintQHSD:isfx> %a1"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movaddr<mode>"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(match_operand:VAXintQHSD 1 "address_operand" "p"))]
   ""
-  "mova<VAXintQHSD:isfx> %a1,%0")
+  "mova<VAXintQHSD:isfx> %a1,%0"
+  [(set_attr "cc" "compare")])
 
 (define_insn "pushaddr<mode>"
   [(set (match_operand:SI 0 "push_operand" "=g")
 	(match_operand:VAXfp 1 "address_operand" "p"))]
   ""
-  "pusha<VAXfp:fsfx> %a1")
+  "pusha<VAXfp:fsfx> %a1"
+  [(set_attr "cc" "compare")])
 
 (define_insn "movaddr<mode>"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=g")
 	(match_operand:VAXfp 1 "address_operand" "p"))]
   ""
-  "mova<VAXfp:fsfx> %a1,%0")
+  "mova<VAXfp:fsfx> %a1,%0"
+  [(set_attr "cc" "compare")])
 

 ;; These used to be peepholes, but it is more straightforward to do them
 ;; as single insns.  However, we must force the output to be a register
@@ -1628,7 +1814,8 @@
     operands[3] = GEN_INT (mask1 & mask2);
 
   return \"rotl %R2,%1,%0\;bicl2 %N3,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
 ;; left-shift and mask
 ;; The only case where `ashl' is better is if the mask only turns off
@@ -1646,13 +1833,15 @@
   operands[3]
     = GEN_INT (INTVAL (operands[3]) & ~((1 << INTVAL (operands[2])) - 1));
   return \"rotl %2,%1,%0\;bicl2 %N3,%0\";
-}")
+}"
+  [(set_attr "cc" "compare")])
 
 ;; Instruction sequence to sync the VAX instruction stream.
 (define_insn "sync_istream"
   [(unspec_volatile [(const_int 0)] VUNSPEC_SYNC_ISTREAM)]
   ""
-  "movpsl -(%|sp)\;pushal 1(%|pc)\;rei")
+  "movpsl -(%|sp)\;pushal 1(%|pc)\;rei"
+  [(set_attr "cc" "clobber")])
 
 (define_expand "nonlocal_goto"
   [(use (match_operand 0 "general_operand" ""))
@@ -1682,47 +1871,6 @@
 
 (include "builtins.md")
 
-(define_peephole2
-  [(set (match_operand:SI 0 "push_operand" "")
-        (const_int 0))
-   (set (match_dup 0)
-        (match_operand:SI 1 "const_int_operand" ""))]
-  "INTVAL (operands[1]) >= 0"
-  [(set (match_dup 0)
-        (match_dup 1))]
-  "operands[0] = gen_rtx_MEM(DImode, XEXP (operands[0], 0));")
-
-(define_peephole2
-  [(set (match_operand:SI 0 "push_operand" "")
-        (match_operand:SI 1 "general_operand" ""))
-   (set (match_dup 0)
-        (match_operand:SI 2 "general_operand" ""))]
-  "vax_decomposed_dimode_operand_p (operands[2], operands[1])"
-  [(set (match_dup 0)
-        (match_dup 2))]
-  "{
-    operands[0] = gen_rtx_MEM(DImode, XEXP (operands[0], 0));
-    operands[2] = REG_P (operands[2])
-      ? gen_rtx_REG(DImode, REGNO (operands[2]))
-      : gen_rtx_MEM(DImode, XEXP (operands[2], 0));
-}")
-
-; Leave this commented out until we can determine whether the second move
-; precedes a jump which relies on the CC flags being set correctly.
-(define_peephole2
-  [(set (match_operand:SI 0 "nonimmediate_operand" "")
-        (match_operand:SI 1 "general_operand" ""))
-   (set (match_operand:SI 2 "nonimmediate_operand" "")
-        (match_operand:SI 3 "general_operand" ""))]
-  "0 && vax_decomposed_dimode_operand_p (operands[1], operands[3])
-   && vax_decomposed_dimode_operand_p (operands[0], operands[2])"
-  [(set (match_dup 0)
-        (match_dup 1))]
-  "{
-    operands[0] = REG_P (operands[0])
-      ? gen_rtx_REG(DImode, REGNO (operands[0]))
-      : gen_rtx_MEM(DImode, XEXP (operands[0], 0));
-    operands[1] = REG_P (operands[1])
-      ? gen_rtx_REG(DImode, REGNO (operands[1]))
-      : gen_rtx_MEM(DImode, XEXP (operands[1], 0));
-}")
+;; The earlier peephole definitions have been removed because they weren't
+;; setting the flags equivalently to the 32-bit instructions they replaced.
+;; TODO: replace them after fixing the condition code notify function.

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

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)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jeff Law]

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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jake Hamby]

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)

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Bernd Schmidt]

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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jake Hamby]

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
> 

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 3147 bytes --]

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

-- 
      Jan-Benedict Glaw      jbglaw@lug-owl.de              +49-172-7608481
Signature of:      [Nach Firefox-Update gibt es Empfehlungen, wenn man einen neuen Tab aufmacht.]
the second  :                                   13:26 <@jbglaw> "Hide the new tab page"
                    13:27 <@jbglaw> Warum zum Teufel sind gerade Kacheln so ungeheuer angesagt?!
              13:57 <@andrel> die Mozilla Foundation hat eine LKW Ladung Fugenkitt gespendet bekommen?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Maciej W. Rozycki]

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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jake Hamby]

> 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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Maciej W. Rozycki]

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

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jeff Law]

On 03/26/2016 06:02 AM, Jake Hamby wrote:
> As an added bonus, I see that my patch set also included an old m68k
> patch that had been sitting in my tree, which fixes a crash when
> -m68040 is defined. I may have submitted it to port-m68k before. It
> hasn’t been tested with the new compiler either. Here’s that patch
> separately. It only matter when TARGET_68881 && TUNE_68040.

Do you have a testcase for this failure?

Re: Bad CC0 optimizer in VAX backend (was Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX)

[Jeff Law]

On 03/28/2016 04:29 PM, Jake Hamby wrote:
> I have some bad news and some good news. The bad news is that there
> has been a nasty optimizer bug lurking in the VAX backend for GCC for
> many years, which has to do with over-optimistic removal of necessary
> tst/cmp instructions under certain circumstances. This manifests at
> -O or higher and the symptoms are strange behavior like stack
> overflows because of branches going the wrong way.
So let's get that addressed.  Matt would be the best person to review 
this change as he's the Vax maintainer.  But if it's simple and 
straightforward others can handle.

It will make Matt's job easier if you can create a self-contained 
testcases which show these problems.  Ideally it'll exit (0) when the 
test passes and abort() when the test fails.  Matt (or someone else) can 
use that to help verify exactly what is happening *and* the test can be 
added to the regression testsuite.

There are hints for testcase reduction on the gcc website.

>
> The good news is that my suspicions about the CC0 handler appear to
> be correct, and better yet, I'm currently testing a patch that isn't
> too big and I believe will fix this bug. The bad behavior is in
> vax_notice_update_cc (), which is supposed to call CC_STATUS_INIT if
> the CC flags have been reset, or set cc_status.value1 and possibly
> cc_status.value2 with the destination of the current (set...)
> instruction, whose signed comparison to 0 will be stored in the N and
> Z flags as a side effect (most VAX instructions do this).
Right.  Pretty standard stuff for a cc0 target.


>
> The first bug is that most, but not all of the define_insn patterns
> in vax.md actually do this. The paths that emit movz* instructions
> will not set N and Z, and there are some code paths that can't be
> trusted because they handle a 64-bit integer in two 32-bit pieces,
> for example, so N and Z won't reflect the desired result (comparison
> of operand 0 to 0).
Understood.  So it sounds like there's two bugs, which implies we'd 
really like two independent tests.

One is the movz instructions don't set the cc0 codes, but 
notice_update_cc isn't aware of that or doesn't handle it properly.

Second is the handling of notice_update_cc for double-word instructions. 
  Many ports use CC_STATUS_INIT for those as tracking the status bits 
can be painful.

>
> The current version of vax_notice_update_cc has a specific check for
> this: it won't set the cc_status cache if GET_CODE (SET_DEST (exp))
> != ZERO_EXTRACT. The problem is that this is checking the RTL
> expression for (zero_extract...) and not whether what was actually
> emitted is a movz* or not. That's why all of the define_insn()'s have
> to be annotated with their actual behavior vis-a-vis compare to 0,
> and then the change to vax.c to read the CC attribute makes it really
> much easier to get the correct behavior.
Right.  That's similar to how other cc0 ports work -- they have 
attributes which clearly indicate the cc0 status for each alternative. 
Then you just need to add the attribute to each insn and check it in 
notice_update_cc.  See the v850 port as a fairly simple example of how 
this can be handled.


>
> I need to do a lot of testing today to see if this really does help
> fix GCC's VAX backend, but I'm hopeful that this is at least a real
> bug that needs to be fixed, and that I have a fix for it that should
> work. The other good news is that you only need three cc enums:
> "none" (doesn't alter Z or N flags), "compare" (Z and N reflect
> comparison of operand 0 to literal 0), and "clobber" (call
> CC_STATUS_INIT to reset cc_status cache).
It's not uncommon to need an attribute that says the cc0 bits aren't 
modified, but operand0 is modified.  In fact, that may be what you want 
for the movz instructions on the vax from what I've read above.

As far as the patch itself, I'd drop all the grubbing around in RTL, 
except for the case where the cc0 bits are set from a comparison. 
Essentially you let the insn's attributes entirely describe the cc0 
status.   Again, see v850.c::notice_update_cc and the attributes in v850.md.

Jeff

Re: Patches to fix GCC's C++ exception_handling on NetBSD/VAX

[Jeff Law]

On 03/27/2016 04:09 PM, Jake Hamby wrote:
> Thank you for the offer. I already tested it on an Amiga 3000 w/
> 68040 card when I made the fix. The bug manifested as the
> cross-compiler crashing with a failure to find a suitable insn,
> because it couldn’t find the correct FP instruction to expand to. I
> believe it manifested when running ./build.sh release with “-m68040”
> set in CPUFLAGS. I will test it myself and see if it’s still an issue
> without the patch. If you look at the .md file, there’s an entirely
> different code path to generate the same instructions when
> "TARGET_68881 && TUNE_68040" aren't defined.
>
> At the time I made the change, I had already been testing the code on
> an Amiga 3000 w/ 68040 card, so I know that the generated code is
> likely correct (also, the assembler accepted it). So I’m assuming
> that it’s a fairly safe thing. It was the difference between the
> build succeeding or failing, and not an issue with the generated
> code.
>
> So the only thing I can suggest is that you can try a build with the
> patch and make sure it's stable. I was never able to produce a build
> without it, because "TARGET_68881 && TUNE_68040" was excluding the
> other choices when building, I believe, libm or libc or the kernel or
> something like that. I do have a test case for C++ exceptions on VAX,
> which I will send separately.
We'd really like to have a test for this -- a blind change without 
analysis is strongly discouraged.  Analysis is virtually impossible 
without a testcase.

My suggestion would be to remove your hack, rebuild (and yes, I know 
it'll take a *long* time) & capture the failure.  We'll want  the .i/.ii 
preprocessed code as well as the options used to trigger the failure. 
Given those things it ought to be relatively easy for someone to analyze 
the problem, determine if your patch or something else is the best 
solution, and reduce the test for use within the testsuite.

jeff

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jeff Law]

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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jeff Law]

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

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jeff Law]

On 03/26/2016 05:56 AM, Jake Hamby wrote:
>> On Mar 23, 2016, at 05:56, Christos Zoulas <christos@astron.com>
>> wrote:
>>
>> In article <F48D0C6B-A6DB-410B-BC97-C30D4E8B4612@me.com>, Jake
>> Hamby  <jehamby420@me.com> wrote:
>>
>> Hi,
>>
>> Thanks a lot for your patch. I applied it to our gcc-5 in the
>> tree. Unfortunately gcc-5 seems that it was never tested to even
>> compile. I fixed the simple compilation issue, but it fails to
>> compile some files with an internal error trying to construct a
>> dwarf frame info element with the wrong register. If you have some
>> time, can you take a look? I will too.
>>
>> Thanks,
>>
>> christos
>
> Hi Christos,
>
> I just rebased my patches on top of GCC 5.3, which I see you have
> recently switched to. Here’s a brief explanation of how I patched the
> dwarf frame error.
>
> The problem is that FIRST_PSEUDO_REGISTER should be increased from 16
> to 17, because PSW is considered a real register for DWARF debug
> purposes. This necessitated changing a few other macros, but nothing
> major. Since it’s marked as one of the FIXED_REGISTERS, it never
> actually gets used. Currently I’m doing a full build with GCC 5.3 and
> CONFIGURE_ARGS += —enable-checking=all, which is very much slower, of
> course.
This patch could probably be pulled out and installed independent of the 
rest.  It's simple, well isolated and should be reviewable even by folks 
without intimate knowledge of hte vax port.

>
> One bug I discovered with —enable-checking=all on GCC 4.8.5 is a call
> to XEXP() that may not be valid, but which can be prevented by
> checking the other conditions first, and then calling XEXP() if the
> other conditions are true.
Where specifically?  And what's the hunk of RTL that's being examined 
incorrectly (hint, debug_rtx will dump a hunk of RTL symbolically).

>
> There seems to be a code generation bug with C++ that only affects a
> few things. Unfortunately, GCC itself (the native version, not the
> cross compiler) is one of the programs affected. The symptom when
> compiling almost anything complex in GCC 4.8.5 is a stack overflow as
> it recursively loops around trying to expand bits and pieces of the
> insns. It seems to be branching the wrong way.
Can't do much with this.  Given the known problems with set-cc0 
elimination on this port, perhaps we address those, then come back to 
this problem.

>
> In looking at this, I discovered one really broken issue in the
> current vax.md, namely the three peephole optimizations at the
> bottom. The comment on the bottom one that says “Leave this commented
> out until we can determine whether the second move precedes a jump
> which relies on the CC flags being set correctly.” is absolutely
> correct and I believe all three should be removed. I’ve done so in
> the diff below, and added a comment explaining why.
The comment is essentially useless because it refers back to patterns 
that don't exist.  But it's also not clear what patterns you're 
referring to.  I don't see any define_peepholes in vax.md going back to 
gcc-4.9.  What sources are you using and who's hacked on them?

Note the 0 && in the condition of the last peephole you removed 
essentially disabled that peephole.

It would really help if you were submitting patches against the current 
GCC trunk.

>
> The idea is that some part of the optimizer is able to remove
> explicit tst / cmp insns when a previous one has already set the
> flags in a way that’s useful. So my theory is that the current
> version mostly works, but it’s very difficult to understand as it’s
> written, and it may be very occasionally giving the wrong results due
> to either bugs in its logic or the instructions emitted not setting
> the flags in the usual way. Unfortunately, the m68k version of
> NOTICE_UPDATE_CC (the only other arch supported by NetBSD that uses
> the CC0 style) is even more convoluted.
This optimization happens in final.c

Essentially it tracks the state of the cc0 bits and a few other things. 
When it finds a comparison (cc0-setter), it refers back to the state of 
the bits and if hte bits are in a usable state, it'll "delete" the 
cc0-setting insn.

>
> So what I’d like to try next is rewriting the VAX version of
> notice_update_cc to use the style that the AVR backend and others use
> which is to add a “cc” attribute to the instructions themselves in
> the .md file describing what each one does in terms of flags.
This would be a step forward.  I would do this work independently of 
fixing hte dwarf/EH stuff.  Ideally patch submissions are independent 
changes rather than fixing several unrelated problems in a single patch.

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jeff Law]

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

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Jeff Law]

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

Re: Patches to fix GCC’s C++ exception handling on NetBSD/VAX

[Jeff Law]

On 03/26/2016 08:38 AM, Jake Hamby wrote:
> Unfortunately, my previous patch that included a change to gcc/config/vax/vax.h that increased FIRST_PSEUDO_REGISTER from 16 to 17 breaks the C++ exception handling that I’d worked so hard to get right with the rest of the patch. I believe I need to define DWARF_FRAME_REGISTERS to 16 in the same file to fix the size of the array that libgcc/unwind-dw2.c creates. The i386 backend and several others also define it their .h file for the same reason (compatibility with hardcoded frame offsets).
>
> Here’s the first part of the patch to vax.h that increases FIRST_PSEUDO_REGISTER and also adds a definition of DWARF_FRAME_REGISTERS as 16, with suitable comment. I’m testing it now. I know that C++ exceptions were working before I increased FIRST_PSEUDO_REGISTER to 17.

So do you consider this patch ready to officially submit to GCC?  Or is 
there something else you're waiting on or dependent upon?

jeff

Re: Patches to fix optimizer bug & C++ exceptions for GCC VAX backend

[Maciej W. Rozycki]

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