Re: [PATCH] GNU/KNetBSD support (with ChangeLog)

Re: [PATCH] GNU/KNetBSD support (with ChangeLog)

[Jim Wilson]

Robert Millan wrote:
>>	* config/i386/knetbsd-gnu.h: New. i386-*-knetbsd-gnu definitions.
>>	* config.gcc: Add *-*-knetbsd*-gnu and i[34567]86-*-knetbsd*-gnu.

It looks like you took the existing config/i386/netbsd-elf.h file and 
then modified it to suit your needs.

However, I see that you are now including both netbsd-elf.h and 
knetbsd-gnu.h which means you are defining a lot of macros twice. 
Unless there is a good reason for this, you should delete all of the 
duplicate definitions out of knetbsd-gnu.h.  Only define the things you 
need to override netbsd-elf.h.  That will result in a smaller hopefully 
easier to maintain file.  This is more like what the existing 
kfreebsdgnu.h file does.

I see that the kfreebsd port created a config/kfreebsdgnu.h file to 
contain definitions that apply to all kfreebsd targets.  You might want 
to do the same for knetbsdgnu.  The config/kfreebsdgnu.h file holds a 
TARGET_OS_CPP_BUILTINS macro which is the right place to put this.  It 
also has a TARGET_CPU_CPP_BUILTINS macro which obviously belongs in the 
i386 directory, so don't copy that mistake.

The only other issue I have is with copyright assignments.  We need one 
for gcc on file for you before we can accept a patch from you.  I can't 
verify this myself at the moment.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

Re: [PATCH] GNU/KNetBSD support (with ChangeLog)

[Robert Millan]

On Thu, Dec 04, 2003 at 11:15:35PM -0800, Jim Wilson wrote:
> Robert Millan wrote:
> >>	* config/i386/knetbsd-gnu.h: New. i386-*-knetbsd-gnu definitions.
> >>	* config.gcc: Add *-*-knetbsd*-gnu and i[34567]86-*-knetbsd*-gnu.
> 
> It looks like you took the existing config/i386/netbsd-elf.h file and 
> then modified it to suit your needs.
> 
> However, I see that you are now including both netbsd-elf.h and 
> knetbsd-gnu.h which means you are defining a lot of macros twice. 
> Unless there is a good reason for this, you should delete all of the 
> duplicate definitions out of knetbsd-gnu.h.  Only define the things you 
> need to override netbsd-elf.h.  That will result in a smaller hopefully 
> easier to maintain file.  This is more like what the existing 
> kfreebsdgnu.h file does.
> 
> I see that the kfreebsd port created a config/kfreebsdgnu.h file to 
> contain definitions that apply to all kfreebsd targets.  You might want 
> to do the same for knetbsdgnu.  The config/kfreebsdgnu.h file holds a 
> TARGET_OS_CPP_BUILTINS macro which is the right place to put this.  It 
> also has a TARGET_CPU_CPP_BUILTINS macro which obviously belongs in the 
> i386 directory, so don't copy that mistake.

I wasn't fully aware of this, since the initial patch was made by Momchil
Velikov. My task has been merely that of updating it for current CVS.

I'm CCing him, so he can comment on this. Momchil, it'd be nice if you could
solve Jim's concerns, I'm not well versed in gcc/config internals and it'd
take me a while to figure out what to do here.

> The only other issue I have is with copyright assignments.  We need one 
> for gcc on file for you before we can accept a patch from you.

My copyright is assigned for gcc. I asked Momchil to do the same some time
ago but I don't remember more about that now.

> I can't verify this myself at the moment.

I know the problem. Maybe you could ask the copyright clerk to verify it
for you?

-- 
Robert Millan

"[..] but the delight and pride of Aule is in the deed of making, and in the
thing made, and neither in possession nor in his own mastery; wherefore he
gives and hoards not, and is free from care, passing ever on to some new work."

 -- J.R.R.T, Ainulindale (Silmarillion)