| View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
| 0003059 | VTK | (No Category) | public | 2006-03-29 16:47 | 2016-08-12 09:54 | ||||
| Reporter | Sean McBride | ||||||||
| Assigned To | Sebastien Barre | ||||||||
| Priority | normal | Severity | minor | Reproducibility | always | ||||
| Status | closed | Resolution | moved | ||||||
| Platform | OS | OS Version | |||||||
| Product Version | |||||||||
| Target Version | Fixed in Version | ||||||||
| Summary | 0003059: Remove assumption that build and target machine are the same (ie support Mac OS Universal Binaries) | ||||||||
| Description | Apple is now selling Macs that use 2 kinds of CPUs: PPC and Intel. Both kinds will be out there for years to come. They have created something called a 'universal binary' which is basically a single executable file that has both PowerPC and Intel object code, and so runs natively on both types of Mac. Both PPC and Intel Macs can compile both PPC and Intel code. Building something as universal is pretty easy, at least in the sense that it only takes a few parameters to gcc. Recent versions of cmake now also include functionality to build universal binaries. On the other hand, building something as universal is pretty hard in the sense that you must be sure to write your code in a portable way. Specifically, one must not assume that the target platform is the same as the build platform. vtk makes this assumption in many places. This bug is requesting that it stop. :) I realise that this is no small task. It will take time, but it is something that can be done bit by bit. The first step, I believe, is to stop making this assumption in all new vtk code. Perhaps: a) adding it to the coding standards here <http://www.vtk.org/Wiki/VTK_Coding_Standards>? [^] b) informing all vtk developers (at kitware at least) The next step is to find where the assumption is already made and get rid of it. One problem is CMake's TRY_RUN functionality, which vtk uses for at least two things. Ideally, all TRY_RUN steps should be avoided, because by their nature they assume the build and target machines are of the same type. vtk uses TRY_RUN for: 1) endianess The PPC is big endian, the Intel is little. Building vtk tests the build machine's endianess and potentially defines VTK_WORDS_BIGENDIAN. This is no good for building universal binaries, as it will always be wrong half the time. I'm not sure what the solution is here. Some ideas: a) there are POSIX APIs to swap bytes: ntohl, htonl, ntohs, and htons. But do they exist on windows? I guess not. b) Apple's gcc and Intel's mac compiler automatically #define __BIG_ENDIAN__ or __LITTLE_ENDIAN__. I've read that 'most modern Unix systems define the byte order in the sys/param.h include file'. Does Windows have a similar header? If so, VTK_WORDS_BIGENDIAN could be defined by comparing to existing defines in system headers. c) endianess could be determined at runtime, but this may not be acceptable for performance-critical code. 2) Type sizes building vtk checks the size of various types such as char, short, int, long, float, and double on the build machine. For the particular case of Mac Universal Binaries, the sizes of these types do not change between PPC and Intel. But one day they might, as 64 bit CPUs become more popular. One type that does change size between ppc and intel is 'bool', it is 8 bit on one and 32 bit on the other, but I do not see any VTK_SIZEOF_BOOL or similar. Lots of what vtk does with this info looks like this: // Template ID's must be 32-bits. See .cxx file for more information. #if VTK_SIZEOF_SHORT == 4 typedef unsigned short TemplateIDType; #elif VTK_SIZEOF_INT == 4 typedef unsigned int TemplateIDType; #elif VTK_SIZEOF_LONG == 4 typedef unsigned long TemplateIDType; #endif This kind of thing could be easily replaced using the standard integer types in stdint.h, These types are part of the C99 standard, 7 years old now. Like so: typedef uint32_t TemplateIDType; Then TemplateIDType will be 32bits no matter which platform it's built on. For info on these types, see: <http://www.opengroup.org/onlinepubs/000095399/basedefs/stdint.h.html> [^] Or are there compilers that vtk must support that still do not support stdint.h? If so, they could be special-cased. 3) Other. :) I'm sure there are other issues as well, but I think the above is a starting point. And I want to file this bug sooner than later. | ||||||||
| Tags | No tags attached. | ||||||||
| Project | |||||||||
| Type | |||||||||
| Attached Files |  vtk_patch.txt [^] (1,171 bytes) 1969-12-31 19:00 [Show Content]
| ||||||||
| Relationships | |
| Relationships |
| Notes | |
|
(0004176) Sean McBride (developer) 2006-06-08 13:37 |
I am willing to spend some time and effort writing and testing patches to vtk, but we need to get a discussion going in this bug on what direction kitware wants to take, if any. I believe we should start with the endianess issue. My preference is for option 'b'. Thoughts? |
|
(0004642) Sean McBride (developer) 2006-08-14 14:08 |
Regarding #1, Bill Hoffman suggested to me in an email: "in the configured header file it can check for __BIG_ENDIAN__ or __LITTLE_ENDIAN__ and change the value found by cmake to the correct one". This sounds like a great starting point to me! But I'm not sure how to proceed. How/where can I change vtkConfigure.h after cmake creates it? On my PPC Mac, the file starts like so: /* Byte order. */ #define VTK_WORDS_BIGENDIAN I'd like to change it to: /* Byte order. */ #if defined(__LITTLE_ENDIAN__) #undef VTK_WORDS_BIGENDIAN #elif defined(__BIG_ENDIAN__) #define VTK_WORDS_BIGENDIAN #endif |
|
(0005651) Sean McBride (developer) 2006-11-02 16:36 |
I've attached a patch to vtkConfigure.h.in that uses __BIG_ENDIAN__ and __LITTLE_ENDIAN__, if defined, to define VTK_WORDS_BIGENDIAN. Specifically, I changed: #cmakedefine VTK_WORDS_BIGENDIAN to #if defined(__BIG_ENDIAN__) # define VTK_WORDS_BIGENDIAN #elif defined(__LITTLE_ENDIAN__) # undef VTK_WORDS_BIGENDIAN #else #cmakedefine VTK_WORDS_BIGENDIAN #endif This should be safe for all OSes and compilers. Would someone care to commit it? Thanks. |
|
(0006297) 2007-01-31 10:01 |
I just now checked in a partial fix. It does as Bill suggested wrt to endianess. Also does similar type sizes. |
|
(0006298) Sean McBride (developer) 2007-01-31 10:02 |
I just now checked in a partial fix. It does as Bill suggested wrt to endianess. Also does similar for type sizes. |
|
(0006512) Sean McBride (developer) 2007-02-21 11:58 |
I'm happy to say that as of the current code in cvs, things seems to be working fairly well! I am able to build a Universal VTK app on i386, and it runs properly on both i386 and ppc. It think it would be premature to close this bug though... |
|
(0010412) Sebastien Barre (developer) 2008-02-07 10:12 |
Fixed in the CVS then. |
|
(0010430) Sean McBride (developer) 2008-02-08 11:05 |
After rereading my most recent comment (before Sebastien closed this bug as fixed), I realise I was not sufficiently clear. I do not consider this bug fixed. OTOH, I also don't think the remaining work is vital for 5.2. VTK currently works OK when built Universal, but only because of several hack jobs. What I hope to see changed: 1) stop using TRY_RUN for endianness detection. 2) stop using TRY_RUN for checking type sizes. 3) stop using TRY_RUN everywhere. This would allow VTK to be cross-compiled. I realise this is a lot of work, but it can be done bit by bit. |
|
(0010453) Sebastien Barre (developer) 2008-02-11 13:40 |
Sean, We are trying to hunt VTK bugs lately, so it would help if we can resolve/close this one. Here is some feedback from Alex Neundorf, I quote: > Ideally, all TRY_RUN steps should be avoided, That's right. > vtk uses TRY_RUN for: > 1) endianess I think that's not right with cmake cvs. At least the test for endianess coming with cmake cvs uses only a try_compile: # on mac, if there are universal binaries built both will be true # return the result depending on the machine on which cmake runs IF(CMAKE_TEST_ENDIANESS_STRINGS_BE AND CMAKE_TEST_ENDIANESS_STRINGS_LE) IF(CMAKE_SYSTEM_PROCESSOR MATCHES powerpc) SET(CMAKE_TEST_ENDIANESS_STRINGS_BE TRUE) SET(CMAKE_TEST_ENDIANESS_STRINGS_LE FALSE) ELSE(CMAKE_SYSTEM_PROCESSOR MATCHES powerpc) SET(CMAKE_TEST_ENDIANESS_STRINGS_BE FALSE) SET(CMAKE_TEST_ENDIANESS_STRINGS_LE TRUE) ENDIF(CMAKE_SYSTEM_PROCESSOR MATCHES powerpc) MESSAGE(STATUS "TEST_BIG_ENDIAN found different results, consider setting CMAKE_OSX_ARCHITECTURES or CMAKE_TRY_COMPILE_OSX_ARCHITECTURES to one or no architecture !") ENDIF(CMAKE_TEST_ENDIANESS_STRINGS_BE AND CMAKE_TEST_ENDIANESS_STRINGS_LE) The same is in CheckTypeSize.cmake: IF(NOT "${CMAKE_CHECKTYPESIZE_FIRST_RESULT}" STREQUAL "${${VARIABLE}}") MESSAGE(SEND_ERROR "CHECK_TYPE_SIZE found different results, consider setting CMAKE_OSX_ARCHITECTURES or CMAKE_TRY_COMPILE_OSX_ARCHITECTURES to one or no architecture !") ENDIF(NOT "${CMAKE_CHECKTYPESIZE_FIRST_RESULT}" STREQUAL "${${VARIABLE}}") So developers get a message if two different results are found. Does this help a bit ? |
|
(0010455) Sean McBride (developer) 2008-02-11 14:18 |
Sebastien, I'm afraid I do not follow... I'm actually not very familiar with CMake's syntax. This is how understand your snippit: if I set CMAKE_OSX_ARCHITECTURES to have 2 or more values, say ppc and i386, and I then use CMake's built-in endianness detection, I get an error message. Is that correct? |
|
(0010456) Alex Neundorf (reporter) 2008-02-11 14:54 |
Yes. If you are using cmake cvs, the test for the endianess is done using a try_compile(). If cmake detects that the created file is both big and little endian (i.e. a universal binary), it will set the result depending on what the current platform is and print an information message. This means it gives the same result as with cmake 2.4.x but informs the user that he should do something. And I think there was somewhere an example in a config-header how to check for endianess at compile time (i.e. not at cmake time). Was it in vtk or paraview ? Sebastian, can you check ? There were some nested ifdefs I think. For the type size, cmake also detects if there are multiple results (i.e. a universal binary), but only prints the message if the results actually differ. About "don't use try_run() at all": yes, this would be the goal, but hasn't been reached yet. Alex |
|
(0010457) Sean McBride (developer) 2008-02-11 16:27 |
I see. So, I agree that CMake 2.6 behaves better than 2.4. 2.4 builds half your executable wrong (it picks one endianness and uses it for both), 2.6 still does that but at least tells you. :) Maybe this is the best CMake can do? I guess it is the responsibility of a project that uses CMake, like VTK, to _not use_ CMake's built-in endianness check. If you look at vtkConfigure.h.in you'll see at the top: #if !defined(__APPLE__) #cmakedefine VTK_WORDS_BIGENDIAN #elif defined(__BIG_ENDIAN__) # define VTK_WORDS_BIGENDIAN #endif The check for __APPLE__ was added by me as a workaround for this bug. Basically, on Apple, we ignore CMake's built-in endianness test and instead check the __BIG_ENDIAN__ #define that the compiler provides. On the ppc pass of a universal build __BIG_ENDIAN__ is defined, and on the intel pass it is not. Really, there isn't a need for a try_run/try_compile for endianness at all. After all, a compiler is something that changes human readable code to assembly instructions. Since it's generating the assembly, it knows all about the CPU, including the endianness! What we are missing is a portable way to know at compile time. The __BIG_ENDIAN__ define is a gcc thing. But maybe every compiler has some sort of equivalent define? |
|
(0036848) Kitware Robot (administrator) 2016-08-12 09:54 |
Resolving issue as `moved`. This issue tracker is no longer used. Further discussion of this issue may take place in the current VTK Issues page linked in the banner at the top of this page. |
| Notes |
| Issue History | |||
| Date Modified | Username | Field | Change |
| 2008-02-06 09:58 | Jeff Baumes | Assigned To | Will Schroeder => Sebastien Barre |
| 2008-02-06 09:58 | Jeff Baumes | Severity | major => minor |
| 2008-02-07 10:05 | Sean McBride | Description Updated | |
| 2008-02-07 10:12 | Sebastien Barre | Status | tabled => closed |
| 2008-02-07 10:12 | Sebastien Barre | Note Added: 0010412 | |
| 2008-02-07 10:12 | Sebastien Barre | Resolution | open => fixed |
| 2008-02-08 11:05 | Sean McBride | Status | closed => @20@ |
| 2008-02-08 11:05 | Sean McBride | Resolution | fixed => reopened |
| 2008-02-08 11:05 | Sean McBride | Note Added: 0010430 | |
| 2008-02-11 13:40 | Sebastien Barre | Note Added: 0010453 | |
| 2008-02-11 14:18 | Sean McBride | Note Added: 0010455 | |
| 2008-02-11 14:54 | Alex Neundorf | Note Added: 0010456 | |
| 2008-02-11 16:27 | Sean McBride | Note Added: 0010457 | |
| 2011-06-16 13:11 | Zack Galbreath | Category | => (No Category) |
| 2016-08-12 09:54 | Kitware Robot | Note Added: 0036848 | |
| 2016-08-12 09:54 | Kitware Robot | Status | expired => closed |
| 2016-08-12 09:54 | Kitware Robot | Resolution | reopened => moved |
| Issue History |
| Copyright © 2000 - 2018 MantisBT Team |