[Openmp-commits] [PATCH] D78566: [OpenMP] Add scaffolding for negative runtime tests

Tue Apr 21 11:52:38 PDT 2020

jdenny added a comment.

In D78566#1994835 <https://reviews.llvm.org/D78566#1994835>, @Hahnfeld wrote:

> I see no `config.substitutions` for the other test suites, so I guess it will try to use `not` from `PATH`?

Ah, thanks for pointing that out.  I wasn't being careful with those test suites because I'm not actually adding tests to them yet.  No, `not` is not guaranteed to be in `PATH`.

> IIRC LLVM and Clang add a substitution from `not` to the full path (via `use_default_substitutions`, but that's probably not available for standalone builds). Doing so is a bit tricky because "not" may be part of the checked strings.

What do you mean by "checked strings" here?  Lit substitutions only happen on RUN lines, right?

> LLVM does some funny `\| \bnot\b` regex (which I have yet to understand), so I leave it up to you if you want to dig into that.

I don't understand that either.  That makes sense only when `not` stdin is a pipe.  Grepping for `RUN: not` shows that's not always true.  That's probably why `not` is also in `PATH`, but that makes the substitution seem redundant.  When I remove the substitution (from `lit/llvm/config.py`), check-llvm behaves.  I haven't yet run check-clang, etc.

> Alternatively, we can either provide `%not` (which makes it unambiguous)

My only argument against that solution is that no other LLVM test suite does this.  I grepped for `RUN:.*%not` to verify.

`\bnot\b` works in a quick example I just tried.

> or just not wire it up for the other suites and adapt the commit message.

Maybe.

I just noticed another issue: ompt tests are skipped if FileCheck is not available.  This is configured in `openmp/runtime/test/lit.cfg`.  However, other openmp test suites that use FileCheck don't bother.  Why is ompt different?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78566