misc. patches

Hi,

here are some minor patches that for various reasons I've not submitted yet - I'm just trying to clear my list of differences before christmas...

First of all the clear.patch file contains a patch that enables the JIT to drop all global mappings. I need this because when I have N threads I compile N different versions of my functions using different memory areas for global variables - it's also useful in other dynamic recompilation scenarios.

Next the warnings.patch file contains some minor modification to make Visual Studio shut up (and possibly to make LLVM more 64bit friendly). I chose to typedef a size_type on the LLVM containers that try to mimic the std:: containers, while in the other case I just made a simple cast.

Finally the leaks.patch file contains a destructor for the JITMemoryManager that frees system memory and a minor modification to how one particular singleton object gets instantiated in CommandLine.cpp to stop the VS leak detector complaining.

m.

leaks.patch (1.94 KB)

warnings.patch (2.31 KB)

clear.patch (908 Bytes)

VS has a 64-bit portability mode, where it will complain when it sees non-portable code. I haven't tried it yet on LLVM, but in my experience it will generate a *lot* of warnings. Every time a size_t or ptrdiff_t is assigned to an int or even a long it will complain (Microsoft defines long as 32-bits, even in win64). On the other hand, gcc defines long as 64-bits on 64-bit Unix (so what does it do on cygwin or mingw?). The portability warnings are still useful, but it does mean that "long" must be banished from the code.

Morten Ofstad wrote:

Morten,

Patches applied! Thanks.

I'm not sure that the clear.patch has general applicability but I
applied it anyway since you seem to need this convenience function.

Reid.

VS has a 64-bit portability mode, where it will complain when it sees non-portable code. I haven't tried it yet on LLVM, but in my experience it will generate a *lot* of warnings. Every time a size_t or ptrdiff_t is assigned to an int or even a long it will complain (Microsoft defines long as 32-bits, even in win64). On the other hand, gcc defines long as 64-bits on 64-bit Unix (so what does it do on cygwin or mingw?). The portability warnings are still useful, but it does mean that "long" must be banished from the code.

I think that the 'bad' habit that occurs in LLVM code is assuming that 'int' is 32-bits and 'long long' is 64-bit. I don't think that these assumptions fail on any machine that we currently care about, though if desired, these could be cleaned up. I don't think we use 'long' extensively (it varies in sizes on hosts we already support well, so it can't be trusted ;-). As you're noticing, we do assume that 'unsigned' can hold various STL size_types though.

-Chris

Next the warnings.patch file contains some minor modification to make Visual Studio shut up (and possibly to make LLVM more 64bit friendly).

_______________________________________________
LLVM Developers mailing list
LLVMdev@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://mail.cs.uiuc.edu/mailman/listinfo/llvmdev

-Chris

Jeff Cohen wrote:

VS has a 64-bit portability mode, where it will complain when it sees non-portable code. I haven't tried it yet on LLVM, but in my experience it will generate a *lot* of warnings. Every time a size_t or ptrdiff_t is assigned to an int or even a long it will complain (Microsoft defines long as 32-bits, even in win64). On the other hand, gcc defines long as 64-bits on 64-bit Unix (so what does it do on cygwin or mingw?). The portability warnings are still useful, but it does mean that "long" must be banished from the code.

I'm not trying to eliminate the warnings when compiling LLVM -- I'm trying to eliminate the warnings in applications _using_ LLVM which might be (as in our case) using the 64-bit portability mode. So I only want to patch some very commonly included header files in order to compile our application cleanly. I am absolutely not suggesting to banish "long" from the code.

m.

Morten,

The leaks.patch file introduced a static destructor ordering problem
which lead to garbled output. The comment above those lines of code
indicates why it needs to be the way it is. My bad for committing it in
the first place. Please be careful in the future.

Reid.

Morten Ofstad wrote:

Jeff Cohen wrote:

VS has a 64-bit portability mode, where it will complain when it sees non-portable code. I haven't tried it yet on LLVM, but in my experience it will generate a *lot* of warnings. Every time a size_t or ptrdiff_t is assigned to an int or even a long it will complain (Microsoft defines long as 32-bits, even in win64). On the other hand, gcc defines long as 64-bits on 64-bit Unix (so what does it do on cygwin or mingw?). The portability warnings are still useful, but it does mean that "long" must be banished from the code.

I'm not trying to eliminate the warnings when compiling LLVM -- I'm trying to eliminate the warnings in applications _using_ LLVM which might be (as in our case) using the 64-bit portability mode. So I only want to patch some very commonly included header files in order to compile our application cleanly. I am absolutely not suggesting to banish "long" from the code.

m.

Didn't mean to imply that you did; however, *I* am suggesting it :slight_smile:

But apparently it's not a problem. 64-bit portability warnings were already turned on in the project files you created, and precisely zero warnings are generated. LLVM is already 64-bit clean as far as VC++ is concerned. At least the part that gets built with VC++.

Morten,

The leaks.patch file introduced a static destructor ordering problem
which lead to garbled output. The comment above those lines of code
indicates why it needs to be the way it is. My bad for committing it in
the first place. Please be careful in the future.

More specifically, the lib/Support/Timer.cpp change was the problem, and was reverted:
http://mail.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20041213/022289.html

-Chris

I haven't tried the VC project files, but out of curiousity, what pieces are missing?

-Chris

I don't know what pieces are missing, but the build log I just mailed you shows what's not missing.

Chris Lattner wrote:

It was too good to be true. While the 64-bit portability warnings were turned on, the specific warnings themselves were being explicitly suppressed by different project settings. Not sure what the point of that was. Anyway, about 1000 warnings are generated.

Jeff Cohen wrote: