[PATCH] D48442: [SPECCPU2017] Add addition platform options and missing flags.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: External/SPEC/SpecCPU2017.cmake:117-147
     if(ARCH STREQUAL "x86" AND TARGET_OS STREQUAL "Linux")
       add_definitions(-DSPEC_LINUX) # 526.blender_r
       add_definitions(-DSPEC_AUTO_BYTEORDER=0x12345678)
       if (CMAKE_SIZEOF_VOID_P EQUAL 8)
         # Linux x86_64
         add_definitions(-DSPEC_LP64)
         add_definitions(-DSPEC_LINUX_X64) # perlbench
----------------
MatzeB wrote:
> This feels like some SPEC flags (SPEC_LP64/SPEC_ILP32,SPEC_AUTO_BYTEORDER) should only depend on CMAKE_SIZEOF_VOID_P (and not the TARGET_OS) or IS_BIGENDIAN rather than getting shoved into per OS/per architecture parts.
I didn't know about `IS_BIGENDIAN`. I will use that.

SPEC_LP64/SPEC_ILP32/SPEC_LLP64 is a but more complicated, it definitely does not only depend on the address width. 64 bit msvc uses LLP64, clang on Windows is LLP64 (`WindowsX86_64TargetInfo`), mingw on Windows is LP64. I could use cmake's `CheckTypeSize` to check the exact type model.

Unfortunately there are flags such as `SPEC_LINUX_X64`, so we don't get around checks for specific platforms.



================
Comment at: External/SPEC/SpecCPU2017.cmake:137
+    elseif (ARCH STREQUAL "x86" AND TARGET_OS STREQUAL "Windows")
+      add_definitions(-DSPEC_AUTO_BYTEORDER=0x12345678) # Big endian
+      if (CMAKE_SIZEOF_VOID_P EQUAL 8)
----------------
MatzeB wrote:
> comment typo?
Yes, indeed.


https://reviews.llvm.org/D48442