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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.
This revision is now accepted and ready to land.

I don't have a copy of Spec2017 yet so I cannot test any of it.

The changes look sensible though, with some suggestions below:



================
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
----------------
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.


================
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)
----------------
comment typo?


https://reviews.llvm.org/D48442