[UTC] CHECK-EMPTY instead of skipping blank lines

[Camsyn]

Previously, any blank lines in IR were ignored by UTC, leading to more fragile CHECKs being generated.

This change lets UTC, 1) emit CHECK-EMPTY to check blank lines, and 2) generate more CHECK-NEXTs, landing the discussion #165419 (comment).

Moreover, this change also aligns the behavior of IR check-gen to ASM check-gen, which has been emitting CHECK-EMPTY since a8a89c7.

[llvmbot]

@llvm/pr-subscribers-testing-tools

Author: Kunqiu Chen (Camsyn)

Changes

Previously, any blank lines in IR were ignored by UTC, leading to more fragile CHECKs being generated.

This change lets UTC, 1) emit CHECK-EMPTY to check blank lines, and 2) generate more CHECK-NEXTs, landing the discussion #165419 (comment).

Moreover, this change also aligns the behavior of IR check-gen to ASM check-gen, which has been emitting CHECK-EMPTY since a8a89c7.

---

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

4 Files Affected:

• (added) llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll (+29)
• (added) llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll.expected (+57)
• (added) llvm/test/tools/UpdateTestChecks/update_test_checks/check_empty.test (+3)
• (modified) llvm/utils/UpdateTestChecks/common.py (+9-27)

diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll new file mode 100644 index 0000000000000..abaa0ce2dd3b4 --- /dev/null +++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll @@ -0,0 +1,29 @@ +; RUN: opt < %s -S | FileCheck %s + +; Test whether the UTC check empty lines instead of skipping them. +define i32 @test(i32 %x) { +entry: + br label %block1 + +block1: + %cmp = icmp eq i32 %x, 0 + br i1 %cmp, label %block2, label %exit1 + +block2: + br i1 %cmp, label %block3, label %exit2 + +block3: + br i1 %cmp, label %exit3, label %exit4 + +exit1: + ret i32 0 + +exit2: + ret i32 %x + +exit3: + ret i32 %x + +exit4: + ret i32 %x +} diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll.expected b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll.expected new file mode 100644 index 0000000000000..dc2a37907039e --- /dev/null +++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/check_empty.ll.expected @@ -0,0 +1,57 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6 +; RUN: opt < %s -S | FileCheck %s + +; Test whether the UTC check empty lines instead of skipping them. +define i32 @test(i32 %x) { +; CHECK-LABEL: define i32 @test( +; CHECK-SAME: i32 [[X:%.*]]) { +; CHECK-NEXT: [[ENTRY:.*:]] +; CHECK-NEXT: br label %[[BLOCK1:.*]] +; CHECK-EMPTY: +; CHECK-NEXT: [[BLOCK1]]: +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[X]], 0 +; CHECK-NEXT: br i1 [[CMP]], label %[[BLOCK2:.*]], label %[[EXIT1:.*]] +; CHECK-EMPTY: +; CHECK-NEXT: [[BLOCK2]]: +; CHECK-NEXT: br i1 [[CMP]], label %[[BLOCK3:.*]], label %[[EXIT2:.*]] +; CHECK-EMPTY: +; CHECK-NEXT: [[BLOCK3]]: +; CHECK-NEXT: br i1 [[CMP]], label %[[EXIT3:.*]], label %[[EXIT4:.*]] +; CHECK-EMPTY: +; CHECK-NEXT: [[EXIT1]]: +; CHECK-NEXT: ret i32 0 +; CHECK-EMPTY: +; CHECK-NEXT: [[EXIT2]]: +; CHECK-NEXT: ret i32 [[X]] +; CHECK-EMPTY: +; CHECK-NEXT: [[EXIT3]]: +; CHECK-NEXT: ret i32 [[X]] +; CHECK-EMPTY: +; CHECK-NEXT: [[EXIT4]]: +; CHECK-NEXT: ret i32 [[X]] +; +entry: + br label %block1 + +block1: + %cmp = icmp eq i32 %x, 0 + br i1 %cmp, label %block2, label %exit1 + +block2: + br i1 %cmp, label %block3, label %exit2 + +block3: + br i1 %cmp, label %exit3, label %exit4 + +exit1: + ret i32 0 + +exit2: + ret i32 %x + +exit3: + ret i32 %x + +exit4: + ret i32 %x +} diff --git a/llvm/test/tools/UpdateTestChecks/update_test_checks/check_empty.test b/llvm/test/tools/UpdateTestChecks/update_test_checks/check_empty.test new file mode 100644 index 0000000000000..61e85152db951 --- /dev/null +++ b/llvm/test/tools/UpdateTestChecks/update_test_checks/check_empty.test @@ -0,0 +1,3 @@ +## test whether the UTC generates CHECK-EMPTY for blank lines +# RUN: cp -f %S/Inputs/check_empty.ll %t.ll && %update_test_checks %t.ll +# RUN: diff -u %t.ll %S/Inputs/check_empty.ll.expected diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py index 8cd200c93a482..d9ef660ebdca8 100644 --- a/llvm/utils/UpdateTestChecks/common.py +++ b/llvm/utils/UpdateTestChecks/common.py @@ -2282,41 +2282,23 @@ def add_checks( original_check_lines=original_check_lines.get(checkprefix), ) - # This could be selectively enabled with an optional invocation argument. - # Disabled for now: better to check everything. Be safe rather than sorry. - - # Handle the first line of the function body as a special case because - # it's often just noise (a useless asm comment or entry label). - # if func_body[0].startswith("#") or func_body[0].startswith("entry:"): - # is_blank_line = True - # else: - # output_lines.append('%s %s: %s' % (comment_marker, checkprefix, func_body[0])) - # is_blank_line = False - - is_blank_line = False for func_line in func_body: if func_line.strip() == "": - is_blank_line = True + # Instead of skipping blank lines, using CHECK_EMPTY is more defensive. + output_lines.append( + "{} {}-EMPTY:".format(comment_marker, checkprefix) + ) continue # Do not waste time checking IR comments. func_line = SCRUB_IR_COMMENT_RE.sub(r"", func_line) - # Skip blank lines instead of checking them. - if is_blank_line: - output_lines.append( - "{} {}: {}".format( - comment_marker, checkprefix, func_line - ) - ) - else: - check_suffix = "-NEXT" if not is_filtered else "" - output_lines.append( - "{} {}{}: {}".format( - comment_marker, checkprefix, check_suffix, func_line - ) + check_suffix = "-NEXT" if not is_filtered else "" + output_lines.append( + "{} {}{}: {}".format( + comment_marker, checkprefix, check_suffix, func_line ) - is_blank_line = False + ) # Add space between different check prefixes and also before the first # line of code in the test function. 

[Camsyn]

Should this change be gated by a new version?

[arichardson]

I think this could potentially cause a lot of test churn when regenerating, so probably worth making it a new version. But I'll defer that decision to @nikic

[nikic]

Yes, this definitely needs to be versioned, as it's going to cause a massive amount of churn. You can add it to version 7 instead of creation a new one, as it's not the DEFAULT_VERSION yet.

[github-actions]

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

[arichardson]

Suggested change

	blank_line_indices = {
	i for i, line in enumerate(func_body) if line.strip() == ""
	} if ginfo.get_version() >= 7 else set()
	blank_line_indices = set()
	if ginfo.get_version() >= 7:
	blank_line_indices = {i for i, line in enumerate(func_body) if line.strip() == ""}

code formatter is not happy with the current one, maybe this here works?

[arichardson]

race condition :)

[nikic]

Suggested change

	; Test whether the UTC check empty lines instead of skipping them.
	; Test whether UTC checks empty lines instead of skipping them.