Fix #5459 #6220

Keno · 2014-03-19T23:00:51Z

This combines the .o emission code from #5787 (without the .o loading code that required LLVM3.5) with some error checking code for cpuid mismatch. When releasing a binary, set JULIA_CPU_TARGET to "core2" (we discussed that this is a reasonable minimum) in your Make.user and everything should work just fine.

@JeffBezanson
@vtjnash (if you want to look it over)

This combines the .o emission code from #5787 with some error checking code for cpuid mismatch. When releasing a binary, set JULIA_CPU_TARGET to "core2" (we discussed that this is a reasonable minimum) in your Make.user and everything should work just fine.

vtjnash · 2014-03-20T01:47:24Z

src/dump.c

@@ -108,6 +110,24 @@ static void jl_load_sysimg_so(char *fname)
    if (sysimg_handle != 0) {
        sysimg_gvars = (jl_value_t***)jl_dlsym(sysimg_handle, "jl_sysimg_gvars");
        globalUnique = *(size_t*)jl_dlsym(sysimg_handle, "jl_globalUnique");
+        const char *cpu_target = (const char*)jl_dlsym(sysimg_handle, "jl_sysimg_cpu_target");
+        if (strcmp(cpu_target,jl_cpu_string) != 0)
+            jl_error("Julia and the system image were compiled for different architectures.");


"Please delete or regenerate sys.{so,dll,dylib}."

vtjnash · 2014-03-20T01:50:27Z

Yeah, let's go ahead and do this. I didn't realize llc was corrupting the backtrace information just as badly. We should probably just disable sys.dll until LDC finishes adding support for SEH in LLVM.

vtjnash · 2014-03-20T01:53:20Z

Is there a reason we want to load the .o file instead of the .dll? We should make sure it uses less memory, and/of is faster before we change since the .o format wasn't really designed for efficiency (I hear that LLVM is planning on introducing their own .o format which can be linked much faster, and in parallel by using a better data layout)

Keno · 2014-03-20T02:14:57Z

Loading the .o file has the advantage of not requiring a linker to be present (so we can create it at install time). Other than that there is no difference.

tkelman · 2014-03-20T02:20:18Z

This is pretty cool! But the LLVM header TargetLibraryInfo.h results in some redefinition errors due to some Windows-only defines in support/dtypes.h https://gist.github.com/tkelman/9655957

Hm, not Julia's fault, it's MinGW-w64's fault, see https://github.com/mirror/mingw-w64/blob/master/trunk/mingw-w64-headers/crt/sys/stat.h#L253-L261 and https://github.com/mirror/mingw-w64/blob/master/trunk/mingw-w64-headers/crt/stdio.h#L466-L483 - #define considered harmful?

… messages

These should really go in CPPFLAGS, but apparently on mingw, there's defines in the stdio headers that rename the function (rather than doing it on a symbol lelve), so we can't use this when we include LLVM headers. Anyway, as long as we don't call the IO functions from C++ we should be fine.

tkelman · 2014-03-20T03:26:10Z

Probably causes other breakage, but adding

#undef stat
#undef fstat
#undef fseeko
#undef ftello

right after the Windows-only #include <sys/stat.h> in src/support/dtypes.h fixes the redefinition errors.

Using cpuid in dump.c and cgutils.cpp requires (see sys.c L416)

#ifdef _OS_WINDOWS_
#define cpuid    __cpuid
#endif

Keno · 2014-03-20T03:27:41Z

I just moved the -D_FILE_OFFSET_BITS back to CFLAGS. Not correct but should work in this case.

Keno · 2014-03-20T03:28:49Z

The cpuid thing seems stupid. I'll just rename it to jl_cpuid, which should be ok on all platforms (thanks for doing this BTW).

tkelman · 2014-03-20T03:44:05Z

-D_FILE_OFFSET_BITS is much better.

Not so sure about ok on all platforms though?

codegen.do: In function `jl_gen_llvm_gv_array':
/home/Tony/julia/src/cgutils.cpp:256: undefined reference to `jl_cpuid'
dump.do: In function `jl_load_sysimg_so':
/home/Tony/julia/src/dump.c:118: undefined reference to `jl_cpuid'
collect2: error: ld returned 1 exit status
Makefile:82: recipe for target '/home/Tony/julia/usr/bin/libjulia-debug.dll' failed

Keno · 2014-03-20T03:49:16Z

Try DLLEXPORT in front of the jl_cpuid in sys.c and report back?

tkelman · 2014-03-20T03:57:19Z

Nope. Still undefined reference.

Moving #ifdef __SSE__ to after the definition of void jl_cpuid fixed it though. Slightly odd, since this machine is a little old but not ancient - 2630QM Sandy Bridge.

The inline assembly may give MSVC trouble, but that might just require putting __cpuid back for _MSC_VER.

vtjnash · 2014-03-20T04:14:03Z

gcc -dM -E - -march=native < /dev/null

Strange. I see __SSE__ defined on my 64-bit boxes (as expected), and not on my 32-bit boxes (also as expected), unless I pass -march=native (also as expected).

tkelman · 2014-03-20T04:25:53Z

I was trying both i686-w64-mingw32 and x86_64-w64-mingw32. Didn't realize __SSE__ was 64-bit (or 32 bit if -march=native is added) only, but now that you mention it, the undefined reference was only happening for 32-bit. We want the function defined either way, right?

http://gist.github.com/9657225

http://i.imgur.com/lz45tQf.png (ignore the duplicate leaf stuff, it's a recent Cygwin binutils bug, should be harmless - oh and I was deleting sys.dll myself in the script I ran, so don't be alarmed that it's not there)

Keno · 2014-03-21T00:35:58Z

Alright, I moved jl_cpuid out of the __SSE. I'll merge later tonight.

Fix #5459

JeffBezanson · 2014-03-21T03:07:01Z

src/dump.c

@@ -108,6 +110,26 @@ static void jl_load_sysimg_so(char *fname)
    if (sysimg_handle != 0) {
        sysimg_gvars = (jl_value_t***)jl_dlsym(sysimg_handle, "jl_sysimg_gvars");
        globalUnique = *(size_t*)jl_dlsym(sysimg_handle, "jl_globalUnique");
+        const char *cpu_target = (const char*)jl_dlsym(sysimg_handle, "jl_sysimg_cpu_target");
+        if (strcmp(cpu_target,jl_cpu_string) != 0)
+            jl_error("Julia and the system image were compiled for different architectures.\n"


"Julia and the System Image" is our new band name.

Our first album cover:

mbauman · 2014-03-22T16:00:26Z

Unfortunately, this breaks compatibility with llvm-svn:

$ make
    CC src/codegen.o
codegen.cpp:314:20: error: no matching constructor for initialization of 'llvm::raw_fd_ostream'
    raw_fd_ostream OS(fname, err);
                   ^  ~~~~~~~~~~
    (…)
codegen.cpp:322:5: error: implicit instantiation of undefined template 'llvm::OwningPtr<llvm::TargetMachine>'
    TM(jl_TargetMachine->getTarget().createTargetMachine(
    ^
/Users/mbauman/Code/git/julia/usr/include/llvm/Support/MemoryBuffer.h:27:25: note: template is declared here
template<class T> class OwningPtr;
                        ^
codegen.cpp:338:12: error: cannot initialize a parameter of type 'llvm::Pass *' with an rvalue of type 'llvm::DataLayout *'
    PM.add(new DataLayout(*jl_ExecutionEngine->getDataLayout()));
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/mbauman/Code/git/julia/usr/include/llvm/IR/LegacyPassManager.h:58:18: note: passing argument to parameter 'P' here
  void add(Pass *P) override;
                 ^
codegen.cpp:4141:39: error: use of undeclared identifier 'mattr'
    std::vector<std::string> attrvec (mattr, mattr+2);
                                      ^
4 errors generated.

Fixing the most of these is trivial (some of the same API changes as in #5984), but my C++ template-foo isn't up to the task of properly fixing the second error. Here's a patch set (I'm not submitting this as a PR since it punts on creating an OwningPtr and uses a std::unique_ptr instead — but shouldn't they be equivalent now?):

diff --git a/src/codegen.cpp b/src/codegen.cpp
index 8f7c1ab..2221c16 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -311,14 +311,22 @@ extern "C"
 void jl_dump_objfile(char* fname, int jit_model)
 {
     std::string err;
+#ifdef LLVM35
+    raw_fd_ostream OS(fname, err, sys::fs::F_None);
+#else
     raw_fd_ostream OS(fname, err);
+#endif
     formatted_raw_ostream FOS(OS);
     jl_gen_llvm_gv_array();

     // We don't want to use MCJIT's target machine because
     // it uses the large code model and we may potentially
     // want less optimizations there.
+#ifdef LLVM35
+    std::unique_ptr<TargetMachine>
+#else
     OwningPtr<TargetMachine>
+#endif
     TM(jl_TargetMachine->getTarget().createTargetMachine(
         jl_TargetMachine->getTargetTriple(),
         jl_TargetMachine->getTargetCPU(),
@@ -335,7 +343,11 @@ void jl_dump_objfile(char* fname, int jit_model)

     PassManager PM;
     PM.add(new TargetLibraryInfo(Triple(jl_TargetMachine->getTargetTriple())));
+#ifdef LLVM35
+    PM.add(new DataLayoutPass(*jl_ExecutionEngine->getDataLayout()));
+#else
     PM.add(new DataLayout(*jl_ExecutionEngine->getDataLayout()));
+#endif
     if (TM->addPassesToEmitFile(PM, FOS, TargetMachine::CGFT_ObjectFile, false)) {
         jl_error("Could not generate obj file for this target");
     }
@@ -4134,6 +4146,7 @@ extern "C" void jl_init_codegen(void)
 #endif
 #ifdef USE_MCJIT
     jl_mcjmm = new SectionMemoryManager();
+    const char *mattr[] = {};
 #else
     // Temporarily disable Haswell BMI2 features due to LLVM bug.
     const char *mattr[] = {"-bmi2", "-avx2"};

ihnorton · 2014-03-22T22:38:49Z

@mbauman, fixed in d988b01

mbauman · 2014-03-23T20:17:59Z

Thanks @ihnorton !

ihnorton · 2014-04-27T15:27:52Z

@loladiro could we switch to llvm::Host instead of our own cpuid in assembly? llvm::Host has a bunch of logic already to deal with various arm flavors...

Keno · 2014-04-27T16:57:14Z

I tried to make that work for the longest time, but couldn't. If you have working code, I'm all in favor.

JeffBezanson · 2014-04-27T17:04:34Z

We still need our own cpuid in sys.c so the runtime can potentially be used without LLVM.

ihnorton · 2014-04-28T05:19:39Z

We can't access the necessary registers on ARM in user mode (so, effectively never AFAICT). It appears that the way everyone (llvm, android) does this is by reading /proc/cpuinfo, so I guess that's the way to go too. Might not need to even parse it - hash the 512-byte feature string and just save the hash?

Fix #5459

c90c0af

This combines the .o emission code from #5787 with some error checking code for cpuid mismatch. When releasing a binary, set JULIA_CPU_TARGET to "core2" (we discussed that this is a reasonable minimum) in your Make.user and everything should work just fine.

vtjnash reviewed Mar 20, 2014
View reviewed changes

Keno added 3 commits March 19, 2014 23:16

Add Please delete or regenerate sys.{so,dll,dylib}'' to all the error…

509b419

… messages

Add newlines to error messages

a6476e0

cpuid -> jl_cpuid

aa76e58

Move jl_cpuid

5f4611f

Keno added a commit that referenced this pull request Mar 21, 2014

Merge pull request #6220 from JuliaLang/kf/cpuid

2966837

Fix #5459

Keno merged commit 2966837 into master Mar 21, 2014

JeffBezanson reviewed Mar 21, 2014
View reviewed changes

mbauman added a commit to mbauman/julia that referenced this pull request Mar 22, 2014

hacks to JuliaLang#6220 to work with llvm3.5

6bafd7a

tkelman mentioned this pull request Mar 23, 2014

libopenlibm.a Makefile rule is missing #6241

Closed

Keno deleted the kf/cpuid branch August 10, 2014 18:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #5459 #6220

Fix #5459 #6220

Keno commented Mar 19, 2014

vtjnash Mar 20, 2014

vtjnash commented Mar 20, 2014

vtjnash commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

vtjnash commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 21, 2014

JeffBezanson Mar 21, 2014

StefanKarpinski Mar 21, 2014

mbauman commented Mar 22, 2014

ihnorton commented Mar 22, 2014

mbauman commented Mar 23, 2014

ihnorton commented Apr 27, 2014

Keno commented Apr 27, 2014

JeffBezanson commented Apr 27, 2014

ihnorton commented Apr 28, 2014

Fix #5459 #6220

Fix #5459 #6220

Conversation

Keno commented Mar 19, 2014

vtjnash Mar 20, 2014

Choose a reason for hiding this comment

vtjnash commented Mar 20, 2014

vtjnash commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 20, 2014

tkelman commented Mar 20, 2014

vtjnash commented Mar 20, 2014

tkelman commented Mar 20, 2014

Keno commented Mar 21, 2014

JeffBezanson Mar 21, 2014

Choose a reason for hiding this comment

StefanKarpinski Mar 21, 2014

Choose a reason for hiding this comment

mbauman commented Mar 22, 2014

ihnorton commented Mar 22, 2014

mbauman commented Mar 23, 2014

ihnorton commented Apr 27, 2014

Keno commented Apr 27, 2014

JeffBezanson commented Apr 27, 2014

ihnorton commented Apr 28, 2014