Skip to content

Conversation

@mtrofin
Copy link
Member

@mtrofin mtrofin commented Jul 29, 2025

See the related issue. We want to set up a build bot where opt runs with -enable-profcheck, which inserts MD_prof before running the rest of the pipeline requested from opt, and then validates resulting profile information (more info in the RFC linked by the aforementioned issue)

In that setup, we will also ignore FileCheck: while the profile info inserted is, currently, equivalent to the profile info a pass would observe via BranchProbabilityInfo/BlockFrequencyInfo, (1) we may want to change that, and (2) some tests are quite sensitive to the output IR, and break if, for instance, extra metadata is present (which it would be due to -enable-profcheck). Since we're just interested in profile consistency on the upcoming bot, ignoring FileCheck is simpler and sufficient. However, this has the effect of passing XFAIL tests. Rather than listing them all, the alternative is to just exclude XFAIL tests.

This PR adds support for that by introducing a --exclude-xfail option to llvm-lit.

Issue #147390

Copy link
Member Author

mtrofin commented Jul 29, 2025

@mtrofin mtrofin changed the title option to wildcard xfail-not [lit] Support wildcard in --xfail-not option Jul 29, 2025
@mtrofin mtrofin requested a review from jdenny-ornl July 29, 2025 19:49
@mtrofin mtrofin marked this pull request as ready for review July 29, 2025 19:49
@llvmbot
Copy link
Member

llvmbot commented Jul 29, 2025

@llvm/pr-subscribers-testing-tools

Author: Mircea Trofin (mtrofin)

Changes

See the related issue. We want to set up a build bot where opt runs with -enable-profcheck, which inserts MD_prof before running the rest of the pipeline requested from opt, and then validates resulting profile information (more info in the RFC linked by the aforementioned issue)

In that setup, we will also ignore FileCheck: while the profile info inserted is, currently, equivalent to the profile info a pass would observe via BranchProbabilityInfo/BlockFrequencyInfo, (1) we may want to change that, and (2) some tests are quite sensitive to the output IR, and break if, for instance, extra metadata is present (which it would be due to -enable-profcheck). Since we're just interested in profile consistency on the upcoming bot, ignoring FileCheck is simpler and sufficient. However, this has the effect of passing XFAIL tests. Rather than listing them all, the alternative is to just ignore passing XFAIL.

This PR adds support for that. It is intentionally unsophisticated - e.g. doesn't support "free range" regex - because the motivating scenario doesn't justify that; we can expand that if needed later.

Issue #147390


Full diff: https://github.com/llvm/llvm-project/pull/151191.diff

2 Files Affected:

  • (modified) llvm/utils/lit/lit/main.py (+5-1)
  • (modified) llvm/utils/lit/tests/xfail-cl.py (+10)
diff --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index 0939838b78ceb..2f22c7ef1d053 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -238,7 +238,11 @@ def mark_xfail(selected_tests, opts):
         test_full_name = t.getFullName()
         if test_file in opts.xfail or test_full_name in opts.xfail:
             t.xfails += "*"
-        if test_file in opts.xfail_not or test_full_name in opts.xfail_not:
+        if (
+            test_file in opts.xfail_not
+            or test_full_name in opts.xfail_not
+            or opts.xfail_not == ["*"]
+        ):
             t.xfail_not = True
 
 
diff --git a/llvm/utils/lit/tests/xfail-cl.py b/llvm/utils/lit/tests/xfail-cl.py
index ef1bb0414cfea..b82fb87847c41 100644
--- a/llvm/utils/lit/tests/xfail-cl.py
+++ b/llvm/utils/lit/tests/xfail-cl.py
@@ -5,11 +5,21 @@
 # RUN:   %{inputs}/xfail-cl \
 # RUN: | FileCheck --check-prefix=CHECK-FILTER %s
 
+# RUN: %{lit} --xfail 'false.txt;false2.txt;top-level-suite :: b :: test.txt' \
+# RUN:   --xfail-not '*' \
+# RUN:   %{inputs}/xfail-cl \
+# RUN: | FileCheck --check-prefix=CHECK-FILTER %s
+
 # RUN: env LIT_XFAIL='false.txt;false2.txt;top-level-suite :: b :: test.txt' \
 # RUN:   LIT_XFAIL_NOT='true-xfail.txt;top-level-suite :: a :: test-xfail.txt' \
 # RUN: %{lit} %{inputs}/xfail-cl \
 # RUN: | FileCheck --check-prefix=CHECK-FILTER %s
 
+# RUN: env LIT_XFAIL='false.txt;false2.txt;top-level-suite :: b :: test.txt' \
+# RUN:   LIT_XFAIL_NOT='*' \
+# RUN: %{lit} %{inputs}/xfail-cl \
+# RUN: | FileCheck --check-prefix=CHECK-FILTER %s
+
 # Check that --xfail-not and LIT_XFAIL_NOT always have precedence.
 
 # RUN: env LIT_XFAIL=true-xfail.txt \

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 1b25068 to 5af21ad Compare July 29, 2025 19:50
@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 5af21ad to b618d13 Compare July 29, 2025 20:13
Copy link
Collaborator

@jdenny-ornl jdenny-ornl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems reasonable to have a way to tell lit you do not care if expected fails become passes, but I'm not sure about this syntax, as noted in my inline comments. Whatever it ends up being, documentation needs to be updated.

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from b618d13 to 20d1c63 Compare July 29, 2025 21:08
Copy link
Member Author

mtrofin commented Jul 29, 2025

ack, I'll update the documentation once we settle on the rest here (because the doc would be 90% that 😄 )

@jdenny-ornl
Copy link
Collaborator

It seems reasonable to have a way to tell lit you do not care if expected fails become passes

I mischaracterized it. It says you expect that expected fails have become passes.

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 20d1c63 to 2d429b3 Compare July 29, 2025 23:29
@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 2d429b3 to 2b67338 Compare July 30, 2025 00:08
@jdenny-ornl
Copy link
Collaborator

Since we're just interested in profile consistency on the upcoming bot, ignoring FileCheck is simpler and sufficient. However, this has the effect of passing XFAIL tests. Rather than listing them all, the alternative is to just ignore passing XFAIL.

So there are no XFAIL tests that are failing because of a non-zero exit status of a command (e.g., opt, diff, grep)? That is, simply avoiding FileCheck invocations is enough to make all XFAIL tests start to pass? If not, then --xfail-not=* will expose those test failures.

@mtrofin
Copy link
Member Author

mtrofin commented Jul 30, 2025

Since we're just interested in profile consistency on the upcoming bot, ignoring FileCheck is simpler and sufficient. However, this has the effect of passing XFAIL tests. Rather than listing them all, the alternative is to just ignore passing XFAIL.

So there are no XFAIL tests that are failing because of a non-zero exit status of a command (e.g., opt, diff, grep)? That is, simply avoiding FileCheck invocations is enough to make all XFAIL tests start to pass? If not, then --xfail-not=* will expose those test failures.

IIUC you're saying it's possible that when turning --xfail-not=* and profcheck, some of the resulting failures aren't due to profcheck (either now or in the future). That's fine - we can piecemeal mark those tests as unsupported when profcheck is enabled.

@jdenny-ornl
Copy link
Collaborator

IIUC you're saying it's possible that when turning --xfail-not=* and profcheck, some of the resulting failures aren't due to profcheck (either now or in the future).

Right. Regardless of profcheck, tests that are already marked XFAIL might be failing places other than FileCheck, so --xfail-not=* will convert them from expected fails to unexpected fails.

That's fine - we can piecemeal mark those tests as unsupported when profcheck is enabled.

For profcheck, is it really worthwhile to run tests that are currently expected to fail? Maybe a more complete solution than --xfail-not=* would be a flag that tells lit to skip tests that are marked xfail. Maybe --skip-xfail.

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 2b67338 to d400000 Compare July 30, 2025 22:52
Copy link
Member Author

mtrofin commented Jul 30, 2025

Makes sense, ptal.

Copy link
Member Author

mtrofin commented Jul 30, 2025

(I realize I need to update documentation and commit message, but wanted first to get agreement on the implementation bit)

@github-actions
Copy link

github-actions bot commented Jul 30, 2025

✅ With the latest revision this PR passed the Python code formatter.

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from d400000 to 5a76e04 Compare July 30, 2025 23:00
Copy link
Collaborator

@jdenny-ornl jdenny-ornl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this.

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 5a76e04 to 776e999 Compare July 31, 2025 17:18
@mtrofin mtrofin changed the title [lit] Support wildcard in --xfail-not option [lit] Optionally exclude xfail tests Jul 31, 2025
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have an XFAIL: foo case where foo is false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like a Inputs/false-xfail.txt (same as true-xfail.txt but RUN: false)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mean a test with, e.g., XFAIL: a-missing-feature. Because a-missing-feature is intentionally a missing feature for the current platform, you don't have to declare it anywhere. In other words, test the issue that originally motivated calling isExpectedToFail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, ptal

@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 776e999 to 9a797be Compare July 31, 2025 17:59
@mtrofin mtrofin force-pushed the users/mtrofin/07-29-option_to_wildcard_xfail-not branch from 9a797be to e97f67f Compare July 31, 2025 19:20
Copy link
Member Author

mtrofin commented Jul 31, 2025

Merge activity

  • Jul 31, 7:48 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 31, 7:50 PM UTC: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin merged commit b383efc into main Jul 31, 2025
10 checks passed
@mtrofin mtrofin deleted the users/mtrofin/07-29-option_to_wildcard_xfail-not branch July 31, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants