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

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

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"

--

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"
> 
> --

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?

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