[utils] Reflow asm check generation to tolerate blank lines

[atanasyan]

Summary

This change introduces two fixes. The second fix allows to generate a test to check the first fix.

• Output CHECK-EMPTY prefix for an empty line in ASM output. Before that fix update_llc_test_checks.py incorrectly emits CHECK-NEXT: <space> prefix.
• Fix the ASM_FUNCTION_MIPS_RE regex to stop on a real function epilogue not on an inline assembler prologue and include inline assembler code into a test.

[MaskRay]

utils/UpdateTestChecks/common.py

195–196

if not is_asm:

218

if not is_asm:

[sdardis]

Address review comments.

[sdardis]

Ping.

[lebedev.ri]

This change enables the update_llc_test_checks.py utility to correctly generate
CHECK lines for inline assembly, where the llc can insert blank lines before
and after an inline assembly string.

Was that not possible before? Perhaps you can showcase that here?

[sdardis]

In D48006, you can see the change for the MIPS specific assembly scrubbing to accept inline assembly. I have also included some example output of the changes of that revision. Since the blank lines around the inline assembly are not scrubbed, FileCheck fails the test case with:

/Users/sdardis/dev/llvm/llvm/test/CodeGen/Mips/cconv/arguments-varargs.ll:33:16: error: found empty check string with prefix 'O32-BE:'
; O32-BE-NEXT: 
               ^
LLVM ERROR: IO failure on output stream: Broken pipe

With D48006 applied, then applying this diff and regenerating test/CodeGen/Mips/cconv/arguments-varargs.ll we can see the following set of changes:

diff --git a/test/CodeGen/Mips/cconv/arguments-varargs.ll b/test/CodeGen/Mips/cconv/arguments-varargs.ll
index 7739ca2..3277a6e 100644
--- a/test/CodeGen/Mips/cconv/arguments-varargs.ll
+++ b/test/CodeGen/Mips/cconv/arguments-varargs.ll
@@ -30,10 +30,8 @@ define void @fn_i16_dotdotdot_i16(i16 %a, ...) {
 ; O32-BE-NEXT:    .set at
 ; O32-BE-NEXT:    .set macro
 ; O32-BE-NEXT:    .set reorder
-; O32-BE-NEXT:  
-; O32-BE-NEXT:    teqi $zero, 1
-; O32-BE-NEXT:  
-; O32-BE-NEXT:    .set pop
+; O32-BE:         teqi $zero, 1
+; O32-BE:         .set pop
 ; O32-BE-NEXT:    #NO_APP
 ; O32-BE-NEXT:    lw $1, 0($sp)
 ; O32-BE-NEXT:    addiu $2, $1, 4

(There are more occurrences like this.) By scrubbing out the blank lines and restarting the LABEL/LABEL-NEXT checks as appropriate, the test case is correctly regenerated and includes all the inline assembly.

[sdardis]

Rebase to master.

[sdardis]

Ping.

[sdardis]

Ping.

[atanasyan]

Does the following patch uses new CHECK-EMPTY directive solves the problem?

--- a/utils/UpdateTestChecks/common.py
+++ b/utils/UpdateTestChecks/common.py
@@ -215,7 +215,10 @@ def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
       if is_asm == True:
         output_lines.append('%s %s:       %s' % (comment_marker, checkprefix, func_body[0]))
         for func_line in func_body[1:]:
-          output_lines.append('%s %s-NEXT:  %s' % (comment_marker, checkprefix, func_line))
+          if func_line.strip() == '':
+            output_lines.append('%s %s-EMPTY:' % (comment_marker, checkprefix))
+          else:
+            output_lines.append('%s %s-NEXT:  %s' % (comment_marker, checkprefix, func_line))
         break

       # For IR output, change all defs to FileCheck variables, so we're immune

[mstojanovic]

The tests were fixed in rL343730.

[RKSimon]

I'm not sure if its a good idea to start skipping lines like this - would a fix to make FileCheck more tolerant of blank lines be any better?

[lebedev.ri]

There's [[ https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-empty-directive | CHECK-EMPTY ]]

[RKSimon]

In D47192#1653876, @lebedev.ri wrote:

There's [[ https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-empty-directive | CHECK-EMPTY ]]

Even better - thanks!

[atanasyan]

What do you think about teaching UpdateTestChecks to generate CHECK-EMPTY when a line is empty?

In D47192#1157635, @atanasyan wrote:

Does the following patch uses new CHECK-EMPTY directive solves the problem?

--- a/utils/UpdateTestChecks/common.py
+++ b/utils/UpdateTestChecks/common.py
@@ -215,7 +215,10 @@ def add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
       if is_asm == True:
         output_lines.append('%s %s:       %s' % (comment_marker, checkprefix, func_body[0]))
         for func_line in func_body[1:]:
-          output_lines.append('%s %s-NEXT:  %s' % (comment_marker, checkprefix, func_line))
+          if func_line.strip() == '':
+            output_lines.append('%s %s-EMPTY:' % (comment_marker, checkprefix))
+          else:
+            output_lines.append('%s %s-NEXT:  %s' % (comment_marker, checkprefix, func_line))
         break

       # For IR output, change all defs to FileCheck variables, so we're immune

[RKSimon]

In D47192#1653904, @atanasyan wrote:

What do you think about teaching UpdateTestChecks to generate CHECK-EMPTY when a line is empty?

Yes - sorry I blanked that snippet for some reason. Using CHECK-EMPTY and including a test change in this patch to show the diff would be good.

[RKSimon]

reverse ping

[atanasyan]

• Emit CHECK-EMPTY prefix for an empty line in ASM output
• Add MIPS related fix and update test case to check the changes.

[RKSimon]

LGTM - cheers

utils/UpdateTestChecks/common.py

for checkprefix in checkprefixes:
         break
       # TODO func_dict[checkprefix] may be None, '' or not exist.
       # Fix the call sites.
       if func_name not in func_dict[checkprefix] or not func_dict[checkprefix][func_name]:
         continue
 
       # Add some space between different check prefixes, but not after the last
       # check line (before the test code).
-      if is_asm == True:
+      if not is_asm:
         if len(printed_prefixes) != 0:
           output_lines.append(comment_marker)
 
       printed_prefixes.append(checkprefix)
       output_lines.append(check_label_format % (checkprefix, func_name))
       func_body = func_dict[checkprefix][func_name].splitlines()
 
-      # For ASM output, just emit the check lines.
-      if is_asm == True:
-        output_lines.append('%s %s:       %s' % (comment_marker, checkprefix, func_body[0]))
-        for func_line in func_body[1:]:
-          output_lines.append('%s %s-NEXT:  %s' % (comment_marker, checkprefix, func_line))
-        break
-
       # For IR output, change all defs to FileCheck variables, so we're immune
       # to variable naming fashions.
+      if is_asm == False:
         func_body = genericize_check_lines(func_body, is_analyze)
 
       # 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
+      # In the case of assembly there may be scrubbed lines between the label
+      # of a function and the start of the lines we're checking.
+      is_blank_line = is_asm
 
       for func_line in func_body:
         if func_line.strip() == '':
           is_blank_line = True
           continue
+
+        if not is_asm:
           # 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 == True:
           output_lines.append('{} {}:       {}'.format(
               comment_marker, checkprefix, func_line))
         else:
           output_lines.append('{} {}-NEXT:  {}'.format(
               comment_marker, checkprefix, func_line))