clang patches for OpenBSD

Hello,

These patches are adapted from
http://www.openbsd.org/cgi-bin/cvsweb/ports/devel/llvm/patches/

Please review them and give some feedback as I hope they make it to
llvm/clang 2.9 before your lock in a few weeks.
Matthew Dempsky helped me to compile C++ using clang++, so that's
resolved and clang & clang++ work fine on OpenBSD amd64 -current with
a couple of patches for its gcc 4.2.1.

IMHO, the absolute deal breaker to compiling on OpenBSD amd64 platform
is the llvm/Makefile.rules. The "local: *;" string when generated in
the four exports.map files breaks compilation with gcc 4.2.1 (refer to
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-November/011899.html).

There's a extra include of -lgcc and a deliberately commented out
-lpthread in llvm/tools/clang/lib/Driver/Tools.cpp.
I am unsure about -pthread or -lpthread but Matthew referred to this
particular diff in a email here
http://marc.info/?l=openbsd-ports&m=129780043311893&w=2

Thanks for your time

---------------------------llvm.diff---------------------------------------------------------

Index: lib/Support/Unix/Path.inc

clang.diff (1.82 KB)

llvm.diff (1.25 KB)

These patches are adapted from
http://www.openbsd.org/cgi-bin/cvsweb/ports/devel/llvm/patches/

Please review them and give some feedback as I hope they make it to
llvm/clang 2.9 before your lock in a few weeks.
Matthew Dempsky helped me to compile C++ using clang++, so that's
resolved and clang & clang++ work fine on OpenBSD amd64 -current with
a couple of patches for its gcc 4.2.1.

Hi Amit,

Sounds great, I'd love to get good OpenBSD support for LLVM 2.9 (which branches on March 6th). I applied the Path.inc, SemaDeclAttr.cpp, and AttributeList.cpp hunks of this to mainline, leaving the three hunks below unresolved.

+++ Makefile.rules (working copy)
@@ -971,7 +971,6 @@
  $(Verb) echo "{" > $@
  $(Verb) grep -q "\<" $< && echo " global:" >> $@ || :
  $(Verb) sed -e 's/$$/;/' -e 's/^/ /' < $< >> $@
- $(Verb) echo " local: *;" >> $@
  $(Verb) echo "};" >> $@
clean-local::
  -$(Verb) $(RM) -f $(NativeExportsFile)

IMHO, the absolute deal breaker to compiling on OpenBSD amd64 platform
is the llvm/Makefile.rules. The "local: *;" string when generated in
the four exports.map files breaks compilation with gcc 4.2.1 (refer to
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-November/011899.html).

Ok, but this is not an acceptable patch, because it will break other targets. If you look at the makefile, it is already set up for several different export map configurations. You could try undefining HAVE_LINK_VERSION_SCRIPT for example (see line 401 as an example of a host-specific makefile chunk that does this) or add an ifndef openbsd if none of the existing options work.

+++ lib/Driver/Tools.cpp (working copy)
@@ -3080,9 +3080,9 @@

    if (Args.hasArg(options::OPT_pthread))
      CmdArgs.push_back("-pthread");
+ //CmdArgs.push_back("-lpthread");
    if (!Args.hasArg(options::OPT_shared))
      CmdArgs.push_back("-lc");
- CmdArgs.push_back("-lgcc");
  }

  if (!Args.hasArg(options::OPT_nostdlib) &&

There's a extra include of -lgcc and a deliberately commented out
-lpthread in llvm/tools/clang/lib/Driver/Tools.cpp.
I am unsure about -pthread or -lpthread but Matthew referred to this
particular diff in a email here
http://marc.info/?l=openbsd-ports&m=129780043311893&w=2

I don't understand the issue here, but this has the same problem. We need this conditionalized to only apply to OpenBSD. I'm not familiar with the driver, so I don't know what the best way to go for that is.

+++ lib/Lex/Lexer.cpp (working copy)
@@ -1516,6 +1516,7 @@
  return true;
}

+#undef __SSE2__
#ifdef __SSE2__
#include <emmintrin.h>
#elif __ALTIVEC__

What is the issue with this hunk? You're just turning off the vectorization of the lexer, but you don't indicate why. Like the other patches above, we can't take this into mainline as-is.

-Chris

+#undef __SSE2__
#ifdef __SSE2__
#include <emmintrin.h>

I don't think this will be accepted.

#elif __ALTIVEC__
Index: lib/Driver/Tools.cpp

--- lib/Driver/Tools.cpp (revision 125833)
+++ lib/Driver/Tools.cpp (working copy)
@@ -3080,9 +3080,9 @@

if \(Args\.hasArg\(options::OPT\_pthread\)\)
  CmdArgs\.push\_back\(&quot;\-pthread&quot;\);

+ //CmdArgs.push_back("-lpthread");

And this as well.

This part is in the OpenBSD specific linker call.

Joerg

Nope, you're off by about two months. Ports lock was about a week ago.

However, there's a high chance that this will get fixed long before 5.0.

Chris and Anton,

I am sorry that I sent out a bad Makefile.rules patch without thinking
about the other OS. I tested the Makefile.rules and confirmed that
this below patch worked on OpenBSD, and should work on others. I
verified twice with a clean rm -rf bindir.
http://www.openbsd.org/cgi-bin/cvsweb/ports/devel/llvm/patches/patch-Makefile_rules
doesn't work at all.

Index: Makefile.rules

Try actually reading the post. The patches are for the LLVM tree, not OpenBSD's ports tree.

> +++ lib/Driver/Tools.cpp (working copy)
> @@ -3080,9 +3080,9 @@
>
> if (Args.hasArg(options::OPT_pthread))
> CmdArgs.push_back("-pthread");
> + //CmdArgs.push_back("-lpthread");
> if (!Args.hasArg(options::OPT_shared))
> CmdArgs.push_back("-lc");
> - CmdArgs.push_back("-lgcc");
> }
>
> if (!Args.hasArg(options::OPT_nostdlib) &&

> There's a extra include of -lgcc and a deliberately commented out
> -lpthread in llvm/tools/clang/lib/Driver/Tools.cpp.
> I am unsure about -pthread or -lpthread but Matthew referred to this
> particular diff in a email here
> http://marc.info/?l=openbsd-ports&m=129780043311893&w=2

Please note that Amit's diff here is off slightly; the actual diff
that I would suggest for inclusion in LLVM/clang is attached below.

I don't understand the issue here, but this has the same problem.
We need this conditionalized to only apply to OpenBSD. I'm not
familiar with the driver, so I don't know what the best way to go for
that is.

The issue is that when the "clang" driver is invoked using the
-pthread option, it needs to pass -lpthread to ld. E.g., the freebsd
link job construction code already correctly does this, but the
openbsd implementation erroneously passes -pthread instead, which
causes a link error because ld doesn't understand this option.

Thanks!

(I don't have anything useful to add about the -lgcc arguments though;
they do seem redundant, but it's consistent with gcc's behavior so I'd
suggest leaving it.)

--- tools/clang/lib/Driver/Tools.cpp.orig Thu Sep 2 16:59:25 2010
+++ tools/clang/lib/Driver/Tools.cpp Tue Feb 15 10:25:35 2011
@@ -2787,7 +2787,7 @@ void openbsd::link::ConstructJob(Compilation &C, const
     CmdArgs.push_back("-lgcc");

     if (Args.hasArg(options::OPT_pthread))
- CmdArgs.push_back("-pthread");
+ CmdArgs.push_back("-lpthread");
     if (!Args.hasArg(options::OPT_shared))
       CmdArgs.push_back("-lc");
     CmdArgs.push_back("-lgcc");

I am forwarding on behalf of Matthew Dempsky.

Ok, sounds great! Applied in r126133, thanks!

-Chris

Applied in r126135, thanks! Please let me know if there are any other openbsd patches outstanding, I think they're all in now.

-Chris