vx-lang / vx-lang/Vx

Test runner silently ignores --check-prefixes (plural), so those RUN lines check nothing

Open
#338 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

devex testing
Dominant language
Rust
Stars
14
Forks
2
Avg merge
12h 42m
Merged PRs (30d)
61

Description

The compile-test runner parses only --check-prefix= (singular). A RUN line using --check-prefixes=A,B falls back to the default prefix, and if the file has no // CHECK: lines that RUN line contributes zero checks and passes.

tests/backend/pass/cpu_fusion_overhead.vx has three RUN lines:

// RUN: vxc %s --emit-mlir -O0 | FileCheck %s --check-prefixes=MLIR,MLIR-O0
// RUN: vxc %s --emit-mlir -O3 | FileCheck %s --check-prefixes=MLIR,MLIR-O3
// RUN: vxc %s --emit-llvm -O3 | FileCheck %s --check-prefix=LLVM

The first two check nothing at all. npu_fusion_overhead.vx is the same shape.

How it surfaced

The runner understands only CHECK: and CHECK-NOT: as ordered substrings; anything else contributes no checks. There is a guard that refuses a file using the other directives, since passing-without-checking is indistinguishable from passing for the right reason -- but the guard only looked for CHECK-*. A file that declares its own prefix and then writes MYPREFIX-SAME: walked straight past it.

That guard now derives the prefixes from the file's own RUN lines, and immediately found // LLVM-LABEL: module { in both files above. Converting it to an enforceable // LLVM: module { then failed -- the output that RUN line produces does not contain that text. So the directive had been asserting something untrue for as long as it has existed, invisibly. Removed, with a note in place so it is not reinstated as a harmless-looking label.

--check-prefixes is the same class of hole one level up, and is left as-is here because fixing it will expose whatever those two RUN lines were meant to assert, which is a separate question from the change that found it.

Worth deciding

Either support the directives (real FileCheck semantics), or make the runner reject what it cannot enforce -- including plural prefixes and any prefix with no matching check lines. The second is cheaper and is the same principle already applied to CHECK-DAG/NEXT/SAME: a check the runner cannot perform should fail the file rather than be skipped.

A RUN line that produces no checks is the specific thing to catch. That is a one-line condition and it would have caught all four cases here.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in the compile-test runner's handling of FileCheck RUN lines and its existing guard for unsupported directives. Reproduce the cases in tests/backend/pass/cpu_fusion_overhead.vx and npu_fusion_overhead.vx, then add coverage for plural prefixes and RUN lines that produce no checks. Done means unsupported or unchecked RUN lines are rejected, or plural-prefix semantics are fully enforced.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
testing-qa, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.