[PATCH] D25155: [Polly] Build and run isl_test as part of check-polly

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

Thanks for the lit implementation!

I tried and got:

  $ ninja check-polly-isl
  [1/1] Running isl unit tests only
  /root/build/llvm/release/tools/polly/test/../test
  FAIL: Polly - isl unit tests :: isl_test.sh (1 of 1)
  ******************** TEST 'Polly - isl unit tests :: isl_test.sh' FAILED ********************
  Script:
  --
  isl_test
  --
  Exit Code: 127
  
  Command Output (stderr):
  --
  /root/build/llvm/release/tools/polly/test/Output/isl_test.sh.script: line 1: isl_test: command not found
  
  --
  
  ********************
  Testing Time: 1.17s
  ********************
  Failing Tests (1):
      Polly - isl unit tests :: isl_test.sh
  
    Unexpected Failures: 1
  FAILED: cd /root/build/llvm/release/tools/polly/test && /usr/bin/python2.7 /root/src/llvm/utils/lit/lit.py -sv --param polly_site_config=/root/build/llvm/release/tools/polly/test/UnitIsl/lit.site.cfg /root/build/llvm/release/tools/polly/test/UnitIsl
  ninja: build stopped: subcommand failed.

It also runs with `ninja polly-check-tests`, presumbly because lit searches for `lit.site.cfg`'s in all subdirectories. But, if you allow the question, if you don't want to run this by the default `ninja check-polly`, why implementing this in the first place?



> CMakeLists.txt:77
>                    --param polly_unit_site_config=${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
> +                  --param polly_unit_site_config=${CMAKE_CURRENT_BINARY_DIR}/Unit/lit.site.cfg
>                    --param build_config=${CMAKE_CFG_INTDIR}

Why is this line duplicated?

> isl_test.sh:1
> +; RUN: isl_test

Wouldn't the `unittests` folder be better suited?

> lit.cfg:109-111
> +    opt_cmd = subprocess.Popen([os.path.join(llvm_tools_dir, 'opt'), '-version'],
> +                           stdout = subprocess.PIPE,
> +                           env=config.environment)

Given that PollyISL doesn't need anything from LLVM (here: `opt`), can we remove a lot of such lines?

https://reviews.llvm.org/D25155