[PATCH]: add support for FreeBSD

Roman_Divacky · October 7, 2008, 4:00pm

hi

the attached patch add support for FreeBSD to clang. if you
consider it ok, please commit

roman

David_Chisnall1 · October 8, 2008, 9:24am

You are hard-coding the version to a FreeBSD release that currently corresponds to -CURRENT. This would be better done by calling uname() and setting it to the release field of the returned structure, or getting it from some other source if building a cross-compiler.

David

Roman_Divacky · October 8, 2008, 10:04am

this number is hardcoded in gcc as well. It's not exported outside gcc
in any header file or something.

do you want me to check for existence of

/usr/src/gnu/usr.bin/cc/cc_tools/freebsd-native.h

and including it in a case it exists and otherwise putting there some dummy
number?

I might ignite a discussion on the fbsd mailing list if we can change
the sources to export these two numbers to be exported in some header file.

thnx! roman

Chris_Lattner · October 9, 2008, 6:40pm

hi

the attached patch add support for FreeBSD to clang. if you
consider it ok, please commit

+ Define(Defs, "__FreeBSD__", "8");
+ Define(Defs, "__FreeBSD_cc_version", "800001");

You are hard-coding the version to a FreeBSD release that currently
corresponds to -CURRENT. This would be better done by calling uname()
and setting it to the release field of the returned structure, or
getting it from some other source if building a cross-compiler.

this number is hardcoded in gcc as well. It's not exported outside gcc
in any header file or something.

do you want me to check for existence of

/usr/src/gnu/usr.bin/cc/cc_tools/freebsd-native.h

and including it in a case it exists and otherwise putting there some dummy
number?

I think that wrapping the configuration logic with #ifdef __freebsd__ (or whatever is appropriate) would make sense. Follow the lead of the similar darwin code.

I might ignite a discussion on the fbsd mailing list if we can change
the sources to export these two numbers to be exported in some header file.

On darwin, we autodetect using the "approved system interfaces". The idea is that we want to be able to build a binary on darwin9 and have it run find on darwin10. The idea is that we want the compiler to default to generating code for the current OS, but that requires detecting it (since the build may have been done on a different one).

Make sense?

-Chris

Roman_Divacky · October 11, 2008, 9:08am

>>>I might ignite a discussion on the fbsd mailing list if we can
>>>change
>>>the sources to export these two numbers to be exported in some
>>>header file.
>>
>>On darwin, we autodetect using the "approved system interfaces". The
>>idea is that we want to be able to build a binary on darwin9 and have
>>it run find on darwin10. The idea is that we want the compiler to
>>default to generating code for the current OS, but that requires
>>detecting it (since the build may have been done on a different one).
>>
>>Make sense?
>
>well.. I could parse output of "touch dummy_file.c ; gcc -E -dM
>dummy_file.c"
>to get the two defines. there's no other way to get this on runtime,
>is
>this an acceptable solution?

I'm confused about why that would be useful. Worst case you could
parse the output of 'system("uname -a")' or something, no? Presumably
uname is implemented in terms of some library/system call, and that
could just be used instead?

there's some confusion here...

we are talking about:
+ Define(Defs, "__FreeBSD__", "8");
+ Define(Defs, "__FreeBSD_cc_version", "800001");

right?

the problem is that the second number is defined only in an internal freebsd
header file or can be obtained via "gcc -E -dM dummy_file.c", the __FreeBSD_cc_version
does not correspond to anything in uname.

so here's the plan. I'll convince people in the freebsd land to include this number
in some public header file and

#ifdef __FreeBSD__
#include <that_header_file.h>
#endif

to get the numbers, ok?

hope it's clear enough now

roman

p.s. keep me CCed as I am not subscribed to the ML

ddunbar · October 11, 2008, 7:58pm

Ok, I understand that issue now.

However, the proposed solution still doesn’t solve the issue that the value will be compiled in instead of determined at run time. Presumably that is what gcc is doing so this is acceptable (the difference is that clang is less likely to be shipped “bound” to some particular OS version).

If its only being determined at compile time then it isn’t unreasonable to obtain the number from “gcc -dM”; chances are we will end up wanting to do this for other configuration values eventually. Of course, getting the value from a header is certainly preferable if you can get the changes into FreeBSD (I’m still confused as to where gcc is getting the number from).

Daniel

Peter_O_Gorman · October 11, 2008, 9:40pm

Daniel Dunbar wrote:

changes into FreeBSD (I'm still confused as to where gcc is getting the
number from).

To answer where it gets the number from:

http://www.freebsd.org/cgi/cvsweb.cgi/src/gnu/usr.bin/cc/cc_tools/freebsd-native.h?rev=1.29;content-type=text%2Fplain
http://www.freebsd.org/cgi/cvsweb.cgi/src/contrib/gcc/config/freebsd-spec.h?rev=1.25;content-type=text%2Fplain

I don't believe __FreeBSD_cc_version gets defined by any of the
compilers in the ports collection, only by /usr/bin/gcc. It looks like a
vendor local patch to me. The __FreeBSD__ number can be gotten from uname().

Peter

Chris_Lattner · October 12, 2008, 5:28pm

I'm confused about why that would be useful. Worst case you could
parse the output of 'system("uname -a")' or something, no? Presumably
uname is implemented in terms of some library/system call, and that
could just be used instead?

there's some confusion here...

we are talking about:
+ Define(Defs, "__FreeBSD__", "8");

As Daniel pointed out, this one can be obtained from a dynamic check.

+ Define(Defs, "__FreeBSD_cc_version", "800001");

right?

the problem is that the second number is defined only in an internal freebsd
header file or can be obtained via "gcc -E -dM dummy_file.c", the __FreeBSD_cc_version
does not correspond to anything in uname.

Ok, but what *is* that number and how does it get used? For example, we always currently hardcode these values:

   DefineBuiltinMacro(Buf, "__GNUC_MINOR__=2");
   DefineBuiltinMacro(Buf, "__GNUC_PATCHLEVEL__=1");
   DefineBuiltinMacro(Buf, "__GNUC__=4");

this is us claiming to be 4.2.1. In my mind, this is ok because code checking these is really almost always doing a feature check on some thing that 4.2.1 supports, and we plan to eventually support most of these.

Is __FreeBSD_cc_version like this? When does it change? What other values has it been over time? Is it always just '__FreeBSD__ + "00001"'?

-Chris

Roman_Divacky · October 14, 2008, 7:20am

>+ Define(Defs, "__FreeBSD_cc_version", "800001");
>
>right?
>
>the problem is that the second number is defined only in an internal
>freebsd
>header file or can be obtained via "gcc -E -dM dummy_file.c", the
>__FreeBSD_cc_version
>does not correspond to anything in uname.

Ok, but what *is* that number and how does it get used? For example,
we always currently hardcode these values:

  DefineBuiltinMacro(Buf, "__GNUC_MINOR__=2");
  DefineBuiltinMacro(Buf, "__GNUC_PATCHLEVEL__=1");
  DefineBuiltinMacro(Buf, "__GNUC__=4");

this is us claiming to be 4.2.1. In my mind, this is ok because code
checking these is really almost always doing a feature check on some
thing that 4.2.1 supports, and we plan to eventually support most of
these.

Is __FreeBSD_cc_version like this? When does it change? What other
values has it been over time? Is it always just '__FreeBSD__ +
"00001"'?

I think you are right.... the __FreeBSD_cc_version is used to denote
what gcc is used on a given FreeBSD. I believe we can easily pretend
it's always X00001 where X is the FreeBSD major number. do you like this
patch? I could not test it but it should be ok...

Index: lib/Basic/Targets.cpp

Chris_Lattner · October 15, 2008, 5:51am

The patch looks great to me, can you please resend as an attachment? It is coming through inline and I can't copy/paste it successfully, thanks!

-Chris

Roman_Divacky · October 15, 2008, 6:35am

here it is.

clang.patch (3.19 KB)

Chris_Lattner · October 15, 2008, 3:14pm

Ah, I'm sorry, one more change please.

The code to detect the OS should go into the llvm::sys::osVersion() function in libsystem. This causes clang to set the target triple in CreateTargetTriple() in clang.cpp. You will want it to set a target triple like "freebsd8-..." etc.

Based on the target triple, you want getFreeBSDDefines to set the "release". For an example, look at how getDarwinDefines extracts the "darwin8-..." into some logic that eventually sets __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__.

Splitting the patch into these two pieces is important because you want clang to *default* to the current dynamic system, but you want to be able to override/cross compile with the -target=... option. Also, calling uname in Targets.cpp doesn't work because not all hosts have it (which is why it needs to be in libsystem).

One trick here is that osName is not very well factored. Daniel, were you planning to make the interface be "return a target triple"?

-Chris

Roman_Divacky · October 16, 2008, 11:14am

this should do the trick

clang.patch (3.05 KB)

Chris_Lattner · October 16, 2008, 5:05pm

Applied, thanks!
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20081013/008213.html

-Chris

ddunbar · October 16, 2008, 9:19pm

Yes, however this is low on the priority list. I’ll try and factor in my changes to whatever platforms we support once I have it.

Daniel