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 <jehamby...@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
> 

Reply via email to