[PATCH] D96104: [test-suite] SPEC2017 CPU Fotonik3d floating point tests.

Tue Feb 9 10:48:38 PST 2021

Meinersbur added inline comments.

================
Comment at: External/SPEC/CFP2017rate/549.fotonik3d_r/CMakeLists.txt:63
+            "${TEST_SUITE_SPEC2017_ROOT}/bin/specxz"
+    COMMAND "${TEST_SUITE_SPEC2017_ROOT}/bin/specxz"
+            -dc "${INPUT_${_run_type}_DIR}/OBJ.dat.xz"
----------------
Meinersbur wrote:
> naromero77 wrote:
> > Meinersbur wrote:
> > > naromero77 wrote:
> > > > Meinersbur wrote:
> > > > > Files in this dir requires to run the install script, where I up to now just extracted the archive. (`specxz` is in `\tools\bin\$platform`). In addition, the platform on which cmake runs, llvm-lit is invoked, and the tests executables are running (`TEST_SUITE_REMOTE_HOST`) might not be the same. For which of these architecture is `specxz` supposed to be?
> > > > > 
> > > > > Is there any possibility to not require installation?
> > > > I assume that SPEC CPU 2017 is installed by the user. The `object.pm` for fotonik3d includes a `sub generate_inputs` section which calls `specxz`. So, if you just run the SPEC CPU 2017 installer it will not create the needed `OBJ.dat`, it is created on the fly.
> > > > 
> > > > The `specxz` that is pointed to in `object.pm`is
> > > > `'command'   => ::jp($ENV{SPEC}, "bin", "specxz"),`
> > > > 
> > > > Your point is that that I might be using the wrong `specxz`? This is called while the tests are being built (after cmake, during make, but before the call to llvm-lit), which is normally the host? Is `${TEST_SUITE_SPEC2017_ROOT}/bin/specxz` not for the host?
> > > > 
> > > From how I understand from `install.sh`, `${TEST_SUITE_SPEC2017_ROOT}/bin/specxz` contains the copy of the file that corresponds to the architecture as returned by `uname`.
> > > 
> > > I see that this is the behavior of SPEC itself, but I liked the convenience of just dumping the spec2017 files into one directory, e.g. an NFS location to be used from multiple platforms.
> > > 
> > > Possibilities to avoid the installation:
> > > 1. Use `xz` as installed on the host.
> > > 2. Autodetect the architecture in the CMakeLists.txt
> > > 3. Try out all specxz executables, stop with the one that worked
> > > 4. Let cmake compile 557.xz_r (`spec_xz.c`) and use that executable.
> > > 
> > > I am not sure whether this would be worth the effort, but it would be nice.
> > I think it terms of simplicity and portability my initial thought would be to go with option 4 -- I am assuming that `spec_xz.c` hasn't been modified in some adverse way.
> > 
> > I think that option 1 would be OK for Linux users, but `xz` doesn't seem to be part of the default Mac OS installation. It is part of homebrew which most Mac OS developers would normally have installed. This path also adds another **external** dependency, though a pretty easy one which is a bit of minus.
> > 
> > With regards to option 2, this also sounds straightforward but still requires installation of SPEC itself right which you want to avoid.
> > 
> > How does option 4 sound to you?
> > 
> > The other possibility is to wait until it becomes a serious issue for someone and then we can fix. It never occurred to me NOT to run the installation script for SPEC CPU 2017. Do you know what is the convention for the other older SPEC test suites?
> > 
> > I will wait to hear back from you before moving forward.
> > 
> Option 2. could be as simple as using the directory `${TEST_SUITE_SPEC2017_ROOT}/tools/bin/${CMAKE_HOST_SYSTEM_NAME}-${CMAKE_HOST_SYSTEM_PROCESSOR}/specxz`. (CMAKE_HOST_SYSTEM_NAME has to be lower cased). Maybe only as a fallback if `${TEST_SUITE_SPEC2017_ROOT}/specxz` is not available.
> 
> There might also be no precompiled toolset available for the target system, e.g. SPEC OpenMP 2012 has none for 64 bit ARM. How to build the tools is described here: https://www.spec.org/cpu2017/Docs/tools-build.html .
> 
> I have not required installation for the SPEC2006 or SPEC2000 Externals.
> 
> However, if it turns out too be too much work, I'd compromise to accept this patch that prints a helpful message if no installed `specxz` is found, and add this functionality later if needed.
The [[ https://www.spec.org/cpu2017/Docs/tools-build.html | tools-build documentation ]] refers to the `install_archives` folder that I actually don't have. However, xz is open source and could be added directly: https://tukaani.org/xz/

Repository:
  rT test-suite

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96104/new/

https://reviews.llvm.org/D96104