Hi Mark, On Thu, Nov 29, 2018 at 12:57:03PM +0100, Mark Wielaard wrote: > On Wed, 2018-11-28 at 15:00 -0600, Segher Boessenkool wrote: > > On Tue, Nov 27, 2018 at 08:54:07PM +0100, Mark Wielaard wrote: > > > On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote: > > > > > diff --git a/gcc/config/rs6000/sysv4.h > > > > > b/gcc/config/rs6000/sysv4.h > > > > > index 0c67634..9330acf 100644 > > > > > --- a/gcc/config/rs6000/sysv4.h > > > > > +++ b/gcc/config/rs6000/sysv4.h > > > > > @@ -972,6 +972,11 @@ ncrtn.o%s" > > > > > /* Generate entries in .fixup for relocatable addresses. */ > > > > > #define RELOCATABLE_NEEDS_FIXUP 1 > > > > > > > > > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD) > > > > > + #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \ > > > > > + || DEFAULT_ABI == > > > > > ABI_ELFv2) > > > > > +#endif > > > > > > > > I don't think this belongs in sysv4.h . > > > > > > I might have gotten lost in the tree of defines/macros. > > > > > > There are two sysv4.h files gcc/config/rs6000/sysv4.h and > > > gcc/config/powerpcspe/sysv4.h > > > > Forget about powerpcspe please, I am talking about rs6000 only. > > I don't know the differences between the config backends, but it looks > like at least some of the configs were just copy/pasted which might > explain why they both define things the same (in sysv4.h). And they
Yes, things are copied. My point is, my review is for rs6000/ only :-) > > You want linux.h and freebsd.h, maybe the "64" versions of those separately. > > Or put this in rs6000.h. sysv4.h is a random header for this, it doesn't > > belong there. > > The reason I added it to sysv4.h is because it matches where the > TARGET_ASM_FILE_END hook is setup. It might make sense to have Yeah, but it does not make sense to put it there, since you are handling other ABIs as well. > So if I understand correctly you would like to have: > > rs6000/linux.h and rs6000/freebsd.h: > #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES 1 > > rs6000/linux64.h and rs6000/freebsd64.h: > #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES (DEFAULT_ABI == > ABI_ELFv2) Something like that yes. Or in rs6000.h, maybe. Some location that makes sense :-) Segher