diff mbox

Only define JMP_BUF_SIZE in backends that also define DONT_USE_BUILTIN_SETJMP

Message ID CABu31nP=10OdiCX=dOKb1DWu=ksBYQ2TPb-eSCVgUsbqWSu=Tg@mail.gmail.com
State New
Headers show

Commit Message

Steven Bosscher June 24, 2012, 9:37 p.m. UTC
Hello,

For targets that define DONT_USE_BUILTIN_SETJMP, the JMP_BUF_SIZE
macro can be used to set the size of the exception jump buffer. The
JMP_BUF_SIZE macro is not used unless DONT_USE_BUILTIN_SETJMP is
defined. See gcc/except.c and libgcc/unwind-sjlj.c.

The PA and SPARC back ends do not define  DONT_USE_BUILTIN_SETJMP, so
they also do not have to define JMP_BUF_SIZE. So:

        * config/sparc/sparc.h (JMP_BUF_SIZE): Do not define.
        * config/pa/pa.h (JMP_BUF_SIZE): Likewise.

OK for trunk?

The rl78 apparently doesn't know what it wants to do:

/* NOTE: defined but zero means dwarf2 debugging, but sjlj EH.  */
#define DWARF2_UNWIND_INFO 0
/*#define DONT_USE_BUILTIN_SETJMP 1*/
#undef DONT_USE_BUILTIN_SETJMP
#define JMP_BUF_SIZE (8*3+8)

But I'll leave that to an rl78 maintainer.

Ciao!
Steven

Comments

DJ Delorie June 24, 2012, 10:31 p.m. UTC | #1
> The rl78 apparently doesn't know what it wants to do:
> 
> /* NOTE: defined but zero means dwarf2 debugging, but sjlj EH.  */
> #define DWARF2_UNWIND_INFO 0
> /*#define DONT_USE_BUILTIN_SETJMP 1*/
> #undef DONT_USE_BUILTIN_SETJMP
> #define JMP_BUF_SIZE (8*3+8)
> 
> But I'll leave that to an rl78 maintainer.

RL78 has a return address size different than pointer size, so the
default GCC code won't work for EH.  For similar reasons, GCC's
internal setjmp won't work either.
Eric Botcazou June 25, 2012, 7:13 a.m. UTC | #2
> The PA and SPARC back ends do not define  DONT_USE_BUILTIN_SETJMP, so
> they also do not have to define JMP_BUF_SIZE. So:
>
>         * config/sparc/sparc.h (JMP_BUF_SIZE): Do not define.
>         * config/pa/pa.h (JMP_BUF_SIZE): Likewise.
>
> OK for trunk?

OK for the SPARC, thanks.  What about the Stormy16 and the picoChip?  It seems 
like a cleanup is possible in this area for them as well.
Steven Bosscher June 25, 2012, 8:13 a.m. UTC | #3
On Mon, Jun 25, 2012 at 12:31 AM, DJ Delorie <dj@redhat.com> wrote:
>
>> The rl78 apparently doesn't know what it wants to do:
>>
>> /* NOTE: defined but zero means dwarf2 debugging, but sjlj EH.  */
>> #define DWARF2_UNWIND_INFO 0
>> /*#define DONT_USE_BUILTIN_SETJMP 1*/
>> #undef DONT_USE_BUILTIN_SETJMP
>> #define JMP_BUF_SIZE (8*3+8)
>>
>> But I'll leave that to an rl78 maintainer.
>
> RL78 has a return address size different than pointer size, so the
> default GCC code won't work for EH.  For similar reasons, GCC's
> internal setjmp won't work either.

Right. And rl78 does *not* define DONT_USE_BUILTIN_SETJMP.
In other words, your port *does* use the internal setjmp code right now.

If that doesn't work for EH, your port has a bug.

Ciao!
Steven
Steven Bosscher June 25, 2012, 8:26 a.m. UTC | #4
On Mon, Jun 25, 2012 at 9:13 AM, Eric Botcazou <ebotcazou@libertysurf.fr> wrote:
>> The PA and SPARC back ends do not define  DONT_USE_BUILTIN_SETJMP, so
>> they also do not have to define JMP_BUF_SIZE. So:
>>
>>         * config/sparc/sparc.h (JMP_BUF_SIZE): Do not define.
>>         * config/pa/pa.h (JMP_BUF_SIZE): Likewise.
>>
>> OK for trunk?
>
> OK for the SPARC, thanks.

And thanks to you for the ack! :-)

>  What about the Stormy16 and the picoChip?  It seems
> like a cleanup is possible in this area for them as well.

Right, I didn't look at the details of the uses of
DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE but it looks like the ones in
picochip and stormy16 are redundant too:

For picochip, this port defines DONT_USE_BUILTIN_SETJMP but in the .c
file (and after all #includes), so it is never exported and not
otherwise referenced in picochip.c, hence dead:
config/picochip/picochip.c:#undef DONT_USE_BUILTIN_SETJMP
config/picochip/picochip.c:#define DONT_USE_BUILTIN_SETJMP 1

stormy16 only undefines DONT_USE_BUILTIN_SETJMP (which is the default
anyway) so these two lines are redundant:
config/stormy16/stormy16.h:#undef  DONT_USE_BUILTIN_SETJMP
config/stormy16/stormy16.h:#define JMP_BUF_SIZE  8

I will remove those lines with the same commit.

That leaves only rl78 and ia64:
$ egrep "DONT_USE_BUILTIN_SETJMP|JMP_BUF_SIZE" config/* config/*/*
config/ia64/freebsd.h:#define JMP_BUF_SIZE  76
config/ia64/hpux.h:#define JMP_BUF_SIZE  (8 * 76)
config/ia64/ia64.h:#define DONT_USE_BUILTIN_SETJMP
config/ia64/linux.h:#define JMP_BUF_SIZE  76
config/ia64/vms.h:#define JMP_BUF_SIZE  (8 * 76)
config/rl78/rl78.h:/*#define DONT_USE_BUILTIN_SETJMP 1*/
config/rl78/rl78.h:#undef DONT_USE_BUILTIN_SETJMP
config/rl78/rl78.h:#define JMP_BUF_SIZE (8*3+8)

The ia64 ones have no useful comments. The 76 is a magic number and it
seems strange to me that the size of JMP_BUF_SIZE depends on the OS
(linux and freebsd have 76, hpux has 8*76, and vms copied the hpux
example with the comment: "Maybe same as HPUX?  Needs to be
checked."). And anyway, these macros are only relevant for SJLJ
exceptions - who uses that on ia64??

The rl78 one is, I think, a bug in the port.

Ciao!
Steven
Tristan Gingold June 25, 2012, 8:39 a.m. UTC | #5
On Jun 25, 2012, at 10:26 AM, Steven Bosscher wrote:

> On Mon, Jun 25, 2012 at 9:13 AM, Eric Botcazou <ebotcazou@libertysurf.fr> wrote:
>>> The PA and SPARC back ends do not define  DONT_USE_BUILTIN_SETJMP, so
>>> they also do not have to define JMP_BUF_SIZE. So:
>>> 
>>>         * config/sparc/sparc.h (JMP_BUF_SIZE): Do not define.
>>>         * config/pa/pa.h (JMP_BUF_SIZE): Likewise.
>>> 
>>> OK for trunk?
>> 
>> OK for the SPARC, thanks.
> 
> And thanks to you for the ack! :-)
> 
>>  What about the Stormy16 and the picoChip?  It seems
>> like a cleanup is possible in this area for them as well.
> 
> Right, I didn't look at the details of the uses of
> DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE but it looks like the ones in
> picochip and stormy16 are redundant too:
> 
> For picochip, this port defines DONT_USE_BUILTIN_SETJMP but in the .c
> file (and after all #includes), so it is never exported and not
> otherwise referenced in picochip.c, hence dead:
> config/picochip/picochip.c:#undef DONT_USE_BUILTIN_SETJMP
> config/picochip/picochip.c:#define DONT_USE_BUILTIN_SETJMP 1
> 
> stormy16 only undefines DONT_USE_BUILTIN_SETJMP (which is the default
> anyway) so these two lines are redundant:
> config/stormy16/stormy16.h:#undef  DONT_USE_BUILTIN_SETJMP
> config/stormy16/stormy16.h:#define JMP_BUF_SIZE  8
> 
> I will remove those lines with the same commit.
> 
> That leaves only rl78 and ia64:
> $ egrep "DONT_USE_BUILTIN_SETJMP|JMP_BUF_SIZE" config/* config/*/*
> config/ia64/freebsd.h:#define JMP_BUF_SIZE  76
> config/ia64/hpux.h:#define JMP_BUF_SIZE  (8 * 76)
> config/ia64/ia64.h:#define DONT_USE_BUILTIN_SETJMP
> config/ia64/linux.h:#define JMP_BUF_SIZE  76
> config/ia64/vms.h:#define JMP_BUF_SIZE  (8 * 76)
> config/rl78/rl78.h:/*#define DONT_USE_BUILTIN_SETJMP 1*/
> config/rl78/rl78.h:#undef DONT_USE_BUILTIN_SETJMP
> config/rl78/rl78.h:#define JMP_BUF_SIZE (8*3+8)
> 
> The ia64 ones have no useful comments. The 76 is a magic number and it
> seems strange to me that the size of JMP_BUF_SIZE depends on the OS
> (linux and freebsd have 76, hpux has 8*76, and vms copied the hpux
> example with the comment: "Maybe same as HPUX?  Needs to be
> checked."). And anyway, these macros are only relevant for SJLJ
> exceptions - who uses that on ia64??

FTR, until recently I think it was not possible to use SJLJ on ia64 due to a typo.

On hpux, the definition of jmp_buf is:

#   define _JBLEN               320
    typedef __float80           jmp_buf[_JBLEN/4];

which looks weird...

Tristan.
Eric Botcazou June 25, 2012, 8:46 a.m. UTC | #6
> Right, I didn't look at the details of the uses of
> DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE but it looks like the ones in
> picochip and stormy16 are redundant too:
>
> For picochip, this port defines DONT_USE_BUILTIN_SETJMP but in the .c
> file (and after all #includes), so it is never exported and not
> otherwise referenced in picochip.c, hence dead:
> config/picochip/picochip.c:#undef DONT_USE_BUILTIN_SETJMP
> config/picochip/picochip.c:#define DONT_USE_BUILTIN_SETJMP 1
>
> stormy16 only undefines DONT_USE_BUILTIN_SETJMP (which is the default
> anyway) so these two lines are redundant:
> config/stormy16/stormy16.h:#undef  DONT_USE_BUILTIN_SETJMP
> config/stormy16/stormy16.h:#define JMP_BUF_SIZE  8

That's my understanding as well.

> That leaves only rl78 and ia64:
> $ egrep "DONT_USE_BUILTIN_SETJMP|JMP_BUF_SIZE" config/* config/*/*
> config/ia64/freebsd.h:#define JMP_BUF_SIZE  76
> config/ia64/hpux.h:#define JMP_BUF_SIZE  (8 * 76)
> config/ia64/ia64.h:#define DONT_USE_BUILTIN_SETJMP
> config/ia64/linux.h:#define JMP_BUF_SIZE  76
> config/ia64/vms.h:#define JMP_BUF_SIZE  (8 * 76)
> config/rl78/rl78.h:/*#define DONT_USE_BUILTIN_SETJMP 1*/
> config/rl78/rl78.h:#undef DONT_USE_BUILTIN_SETJMP
> config/rl78/rl78.h:#define JMP_BUF_SIZE (8*3+8)
>
> The ia64 ones have no useful comments. The 76 is a magic number and it
> seems strange to me that the size of JMP_BUF_SIZE depends on the OS
> (linux and freebsd have 76, hpux has 8*76, and vms copied the hpux
> example with the comment: "Maybe same as HPUX?  Needs to be
> checked."). And anyway, these macros are only relevant for SJLJ
> exceptions - who uses that on ia64??

It looks like DONT_USE_BUILTIN_SETJMP was invented for the IA-64, probably 
because of the Register Stack Engine.  But SPARC also has register windows, 
albeit less sophisticated, and can do without it.  Morever, the Ada compiler 
uses __builtin_setjmp/__builtin_longjmp internally on the host (EH is used for 
error recovery) and is known to work fine on IA-64/Linux, HP-UX and VMS.

In the end, it would appear that DONT_USE_BUILTIN_SETJMP was a quick trick to 
solve specific issues that could very likely have been solved otherwise.  We 
should probably keep it for the sake of IA-64 and get rid of it for all other 
architectures, documenting that it isn't to be used in normal circumstances.
DJ Delorie June 25, 2012, 5:23 p.m. UTC | #7
Let me try again then...

RL78 is confusing and it took a while to get it to work right.  Please
don't change it ;-)
Eric Botcazou June 25, 2012, 8:30 p.m. UTC | #8
> RL78 is confusing and it took a while to get it to work right.  Please
> don't change it ;-)

But we can certainly remove stuff that doesn't do anything; in particular, 
these 3 lines

/*#define DONT_USE_BUILTIN_SETJMP 1*/
#undef DONT_USE_BUILTIN_SETJMP
#define JMP_BUF_SIZE (8*3+8)

can be proved to be equivalent to the empty set.
DJ Delorie June 25, 2012, 9:31 p.m. UTC | #9
> But we can certainly remove stuff that doesn't do anything; in particular, 
> these 3 lines
> 
> /*#define DONT_USE_BUILTIN_SETJMP 1*/
> #undef DONT_USE_BUILTIN_SETJMP
> #define JMP_BUF_SIZE (8*3+8)
> 
> can be proved to be equivalent to the empty set.

If you say so, go for it ;-)
Steven Bosscher June 26, 2012, 8:55 a.m. UTC | #10
On Mon, Jun 25, 2012 at 10:46 AM, Eric Botcazou
<ebotcazou@libertysurf.fr> wrote:
> It looks like DONT_USE_BUILTIN_SETJMP was invented for the IA-64, probably
> because of the Register Stack Engine.  But SPARC also has register windows,
> albeit less sophisticated, and can do without it.  Morever, the Ada compiler
> uses __builtin_setjmp/__builtin_longjmp internally on the host (EH is used for
> error recovery) and is known to work fine on IA-64/Linux, HP-UX and VMS.
>
> In the end, it would appear that DONT_USE_BUILTIN_SETJMP was a quick trick to
> solve specific issues that could very likely have been solved otherwise.  We
> should probably keep it for the sake of IA-64 and get rid of it for all other
> architectures, documenting that it isn't to be used in normal circumstances.

ia64 is now the only remaining architecture that defines
DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE.

If __builtin_setjmp actually does work for ia64, why should we keep
DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?

Ciao!
Steven
Richard Henderson June 26, 2012, 3:38 p.m. UTC | #11
On 06/26/2012 01:55 AM, Steven Bosscher wrote:
> If __builtin_setjmp actually does work for ia64, why should we keep
> DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?

It's an ABI change for anyone using --enable-sjlj-exceptions.
But I don't know who that would be...


r~
John David Anglin June 26, 2012, 5:09 p.m. UTC | #12
On 6/26/2012 11:38 AM, Richard Henderson wrote:
> On 06/26/2012 01:55 AM, Steven Bosscher wrote:
>> If __builtin_setjmp actually does work for ia64, why should we keep
>> DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?
> It's an ABI change for anyone using --enable-sjlj-exceptions.
> But I don't know who that would be...
SJLJ exceptions are forced for hpux10.  It might have been a bug to not 
define
DONT_USE_BUILTIN_SETJMP but nobody complained as far as I know.

Dave
Steven Bosscher June 26, 2012, 5:21 p.m. UTC | #13
On Tue, Jun 26, 2012 at 7:09 PM, John David Anglin <dave.anglin@bell.net> wrote:
> On 6/26/2012 11:38 AM, Richard Henderson wrote:
>>
>> On 06/26/2012 01:55 AM, Steven Bosscher wrote:
>>>
>>> If __builtin_setjmp actually does work for ia64, why should we keep
>>> DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?
>>
>> It's an ABI change for anyone using --enable-sjlj-exceptions.
>> But I don't know who that would be...
>
> SJLJ exceptions are forced for hpux10.  It might have been a bug to not
> define
> DONT_USE_BUILTIN_SETJMP but nobody complained as far as I know.

Did hpux10 even ever run on ia64?

Ciao!
Steven
John David Anglin June 26, 2012, 5:36 p.m. UTC | #14
On 6/26/2012 1:21 PM, Steven Bosscher wrote:
> On Tue, Jun 26, 2012 at 7:09 PM, John David Anglin <dave.anglin@bell.net> wrote:
>> On 6/26/2012 11:38 AM, Richard Henderson wrote:
>>> On 06/26/2012 01:55 AM, Steven Bosscher wrote:
>>>> If __builtin_setjmp actually does work for ia64, why should we keep
>>>> DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?
>>> It's an ABI change for anyone using --enable-sjlj-exceptions.
>>> But I don't know who that would be...
>> SJLJ exceptions are forced for hpux10.  It might have been a bug to not
>> define
>> DONT_USE_BUILTIN_SETJMP but nobody complained as far as I know.
> Did hpux10 even ever run on ia64?
Not that I know of.

I was just thinking that __builtin_setjmp/__builtin_longjmp might not 
interwork with the
libc versions on hpux10.

Dave
Richard Henderson June 26, 2012, 6:52 p.m. UTC | #15
On 06/26/2012 10:36 AM, John David Anglin wrote:
> I was just thinking that __builtin_setjmp/__builtin_longjmp might not interwork with the
> libc versions on hpux10.

They don't need to.


r~
Mike Stump June 26, 2012, 10:06 p.m. UTC | #16
On Jun 26, 2012, at 1:55 AM, Steven Bosscher wrote:
> On Mon, Jun 25, 2012 at 10:46 AM, Eric Botcazou
> <ebotcazou@libertysurf.fr> wrote:
>> It looks like DONT_USE_BUILTIN_SETJMP was invented for the IA-64, probably
>> because of the Register Stack Engine.  But SPARC also has register windows,
>> albeit less sophisticated, and can do without it.  Morever, the Ada compiler
>> uses __builtin_setjmp/__builtin_longjmp internally on the host (EH is used for
>> error recovery) and is known to work fine on IA-64/Linux, HP-UX and VMS.
>> 
>> In the end, it would appear that DONT_USE_BUILTIN_SETJMP was a quick trick to
>> solve specific issues that could very likely have been solved otherwise.  We
>> should probably keep it for the sake of IA-64 and get rid of it for all other
>> architectures, documenting that it isn't to be used in normal circumstances.
> 
> ia64 is now the only remaining architecture that defines
> DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE.
> 
> If __builtin_setjmp actually does work for ia64, why should we keep
> DONT_USE_BUILTIN_SETJMP? Could it break user code somehow?

History is not kind:

http://gcc.gnu.org/ml/gcc-patches/2002-04/msg00124.html

:-)  It seems everyone else has seen the error of their ways and fixed it.  :-)  It got moved to ia64.h, only because a few people mistakingly used it once.  Though, history isn't kinda to me either:

svn log -c13969

:-(  I'd say, remove it, and deprecate the bit and after 2 releases, remove the whole lot.  15 years late...  Sorry for the mess.
diff mbox

Patch

Index: config/pa/pa.h
===================================================================
--- config/pa/pa.h      (revision 188917)
+++ config/pa/pa.h      (working copy)
@@ -1508,9 +1508,6 @@  do {
                         \
      of the return address.  */
         \
   (GEN_INT (-4))

-/* The number of Pmode words for the setjmp buffer.  */
-#define JMP_BUF_SIZE 50
-
 /* We need a libcall to canonicalize function pointers on TARGET_ELF32.  */
 #define CANONICALIZE_FUNCPTR_FOR_COMPARE_LIBCALL \
   "__canonicalize_funcptr_for_compare"
Index: config/sparc/sparc.h
===================================================================
--- config/sparc/sparc.h        (revision 188917)
+++ config/sparc/sparc.h        (working copy)
@@ -1744,9 +1744,6 @@  extern int sparc_indent_opcode;
 #define AS_NIAGARA3_FLAG "d"
 #endif

-/* The number of Pmode words for the setjmp buffer.  */
-#define JMP_BUF_SIZE 12
-
 /* We use gcc _mcount for profiling.  */
 #define NO_PROFILE_COUNTERS 0