[update_llc_test_checks] Handle mixed asm and ISel debug output

[arichardson]

Summary

I noticed a potentially uninitialized variable warning after D119368.
While this is a false(-ish) positive warning, it did highlight that the
output_type variable was being set based on the last RUN line, so mixed
ISel and ASM checks will not work. This commit allows mixed ISel and ASM
output and also fixes the IntelliJ warning. I have deleted the
add_checks() function from isel.py as it is identical to the one in asm.py
and restored the old name (which fixes the formatting too).

[ningxinr]

Thank you so much for the improvements, Alexander! It would be nice to support isel and asm at the same time. Although I do have a few questions and one concern with regard to an ongoing change of mine.

I noticed a potentially uninitialized variable warning after D119368.

Would you mind pointing me to that warning? Is it complaining about output_type? And after your change the warning is gone? That does sound quite false positive-ish...

I have deleted the add_checks() function from isel.py as it is identical to the one in asm.py

Please don't delete that add_checks() function yet. It is only identical to the one in asm.py because we are still working on it... I have a review D122824 in progress (Sorry this one took a bit too long for me to get back to it, but I do plan to get back to it in the next few days.)

restored the old name (which fixes the formatting too)

Did the warning complained about the naming as well? I am quite confused here. Is it the fact that this line was using output_type that caused the warning or the function name add_checks() has cause the warning? What is the motivation for changing back the name here? Is it because the add_checks() in isel.py is removed so we prefer the old name?

Thanks for the improvement of the feature and thanks in advance for your patience with all my questions. :)

[arichardson]

In D125303#3506720, @ningxinr wrote:

Thank you so much for the improvements, Alexander! It would be nice to support isel and asm at the same time. Although I do have a few questions and one concern with regard to an ongoing change of mine.

I noticed a potentially uninitialized variable warning after D119368.

Would you mind pointing me to that warning? Is it complaining about output_type? And after your change the warning is gone? That does sound quite false positive-ish...

I have deleted the add_checks() function from isel.py as it is identical to the one in asm.py

Please don't delete that add_checks() function yet. It is only identical to the one in asm.py because we are still working on it... I have a review D122824 in progress (Sorry this one took a bit too long for me to get back to it, but I do plan to get back to it in the next few days.)

Ah I didn't see that one. For mixed ISel and non-ISel output this will need a different approach though since the correct prefix depends on the current RUN: line.

restored the old name (which fixes the formatting too)

Did the warning complained about the naming as well? I am quite confused here. Is it the fact that this line was using output_type that caused the warning or the function name add_checks() has cause the warning? What is the motivation for changing back the name here? Is it because the add_checks() in isel.py is removed so we prefer the old name?

No warning here, I just noticed that the function call line continuations were no longer aligned after you renamed the function. I'll make a suggestion on that review.

Thanks for the improvement of the feature and thanks in advance for your patience with all my questions. :)

[arichardson]

rebase

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/x86_isel.ll

 ; RUN: llc -mtriple=x86_64 -stop-after=finalize-isel -debug-only=isel -o /dev/null %s 2>&1 | FileCheck %s --check-prefix=PIC
 ; RUN: llc -mtriple=x86_64-windows -stop-after=finalize-isel -debug-only=isel -o /dev/null %s 2>&1 | FileCheck %s --check-prefix=WIN
+; RUN: llc -mtriple=x86_64 < %s | FileCheck %s --check-prefix=ASM
 
 define i64 @i64_test(i64 %i) nounwind readnone {
   %loc = alloca i64
   %j = load i64, i64 * %loc
   %r = add i64 %i, %j
   ret i64 %r
 }
 

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/x86_isel.ll.expected

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64 -stop-after=finalize-isel -debug-only=isel -o /dev/null %s 2>&1 | FileCheck %s --check-prefix=PIC
 ; RUN: llc -mtriple=x86_64-windows -stop-after=finalize-isel -debug-only=isel -o /dev/null %s 2>&1 | FileCheck %s --check-prefix=WIN
+; RUN: llc -mtriple=x86_64 < %s | FileCheck %s --check-prefix=ASM
 
 define i64 @i64_test(i64 %i) nounwind readnone {
 ; PIC-LABEL: i64_test:
 ; PIC:       SelectionDAG has 12 nodes:
 ; PIC-NEXT:    t0: ch = EntryToken
 ; PIC-NEXT:    t2: i64,ch = CopyFromReg t0, Register:i64 %0
 ; PIC-NEXT:    t7: i64,i32,ch = ADD64rm<Mem:(dereferenceable load (s64) from %ir.loc)> t2, TargetFrameIndex:i64<0>, TargetConstant:i8<1>, Register:i64 $noreg, TargetConstant:i32<0>, Register:i16 $noreg, t0
 ; PIC-NEXT:    t10: ch,glue = CopyToReg t0, Register:i64 $rax, t7
 ; PIC-NEXT:    t11: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t10, t10:1
 ; PIC-EMPTY:
 ;
 ; WIN-LABEL: i64_test:
 ; WIN:       SelectionDAG has 12 nodes:
 ; WIN-NEXT:    t0: ch = EntryToken
 ; WIN-NEXT:    t2: i64,ch = CopyFromReg t0, Register:i64 %0
 ; WIN-NEXT:    t7: i64,i32,ch = ADD64rm<Mem:(dereferenceable load (s64) from %ir.loc)> t2, TargetFrameIndex:i64<0>, TargetConstant:i8<1>, Register:i64 $noreg, TargetConstant:i32<0>, Register:i16 $noreg, t0
 ; WIN-NEXT:    t10: ch,glue = CopyToReg t0, Register:i64 $rax, t7
 ; WIN-NEXT:    t11: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t10, t10:1
 ; WIN-EMPTY:
+;
+; ASM-LABEL: i64_test:
+; ASM:       # %bb.0:
+; ASM-NEXT:    movq %rdi, %rax
+; ASM-NEXT:    addq -{{[0-9]+}}(%rsp), %rax
+; ASM-NEXT:    retq
   %loc = alloca i64
   %j = load i64, i64 * %loc
   %r = add i64 %i, %j
   ret i64 %r
 }
 
 define i64 @i32_test(i32 %i) nounwind readnone {
 ; PIC-LABEL: i32_test:

 ; WIN:       SelectionDAG has 15 nodes:
 ; WIN-NEXT:    t0: ch = EntryToken
 ; WIN-NEXT:    t2: i32,ch = CopyFromReg t0, Register:i32 %0
 ; WIN-NEXT:    t7: i32,i32,ch = ADD32rm<Mem:(dereferenceable load (s32) from %ir.loc)> t2, TargetFrameIndex:i64<0>, TargetConstant:i8<1>, Register:i64 $noreg, TargetConstant:i32<0>, Register:i16 $noreg, t0
 ; WIN-NEXT:    t8: i64 = SUBREG_TO_REG TargetConstant:i64<0>, t7, TargetConstant:i32<6>
 ; WIN-NEXT:    t11: ch,glue = CopyToReg t0, Register:i64 $rax, t8
 ; WIN-NEXT:    t12: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t11, t11:1
 ; WIN-EMPTY:
+;
+; ASM-LABEL: i32_test:
+; ASM:       # %bb.0:
+; ASM-NEXT:    movl %edi, %eax
+; ASM-NEXT:    addl -{{[0-9]+}}(%rsp), %eax
+; ASM-NEXT:    retq
   %loc = alloca i32
   %j = load i32, i32 * %loc
   %r = add i32 %i, %j
   %ext = zext i32 %r to i64
   ret i64 %ext
 }
 
 define i64 @i16_test(i16 %i) nounwind readnone {

 ; WIN-NEXT:    t0: ch = EntryToken
 ; WIN-NEXT:    t2: i16,ch = CopyFromReg t0, Register:i16 %0
 ; WIN-NEXT:    t7: i16,i32,ch = ADD16rm<Mem:(dereferenceable load (s16) from %ir.loc)> t2, TargetFrameIndex:i64<0>, TargetConstant:i8<1>, Register:i64 $noreg, TargetConstant:i32<0>, Register:i16 $noreg, t0
 ; WIN-NEXT:    t14: i32 = MOVZX32rr16 t7
 ; WIN-NEXT:    t8: i64 = SUBREG_TO_REG TargetConstant:i64<0>, t14, TargetConstant:i32<6>
 ; WIN-NEXT:    t11: ch,glue = CopyToReg t0, Register:i64 $rax, t8
 ; WIN-NEXT:    t12: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t11, t11:1
 ; WIN-EMPTY:
+;
+; ASM-LABEL: i16_test:
+; ASM:       # %bb.0:
+; ASM-NEXT:    addw -{{[0-9]+}}(%rsp), %di
+; ASM-NEXT:    movzwl %di, %eax
+; ASM-NEXT:    retq
   %loc = alloca i16
   %j = load i16, i16 * %loc
   %r = add i16 %i, %j
   %ext = zext i16 %r to i64
   ret i64 %ext
 }
 
 define i64 @i8_test(i8 %i) nounwind readnone {

 ; WIN-NEXT:    t0: ch = EntryToken
 ; WIN-NEXT:    t2: i8,ch = CopyFromReg t0, Register:i8 %0
 ; WIN-NEXT:    t7: i8,i32,ch = ADD8rm<Mem:(dereferenceable load (s8) from %ir.loc)> t2, TargetFrameIndex:i64<0>, TargetConstant:i8<1>, Register:i64 $noreg, TargetConstant:i32<0>, Register:i16 $noreg, t0
 ; WIN-NEXT:    t14: i32 = MOVZX32rr8 t7
 ; WIN-NEXT:    t8: i64 = SUBREG_TO_REG TargetConstant:i64<0>, t14, TargetConstant:i32<6>
 ; WIN-NEXT:    t11: ch,glue = CopyToReg t0, Register:i64 $rax, t8
 ; WIN-NEXT:    t12: ch = RET TargetConstant:i32<0>, Register:i64 $rax, t11, t11:1
 ; WIN-EMPTY:
+;
+; ASM-LABEL: i8_test:
+; ASM:       # %bb.0:
+; ASM-NEXT:    addb -{{[0-9]+}}(%rsp), %dil
+; ASM-NEXT:    movzbl %dil, %eax
+; ASM-NEXT:    retq
   %loc = alloca i8
   %j = load i8, i8 * %loc
   %r = add i8 %i, %j
   %ext = zext i8 %r to i64
   ret i64 %ext
 }

llvm/utils/update_llc_test_checks.py

for l in ti.run_lines:
       if m:
         triple_in_cmd = m.groups()[0]
 
       march_in_cmd = None
       m = common.MARCH_ARG_RE.search(llc_cmd)
       if m:
         march_in_cmd = m.groups()[0]
 
-      m = common.DEBUG_ONLY_ARG_RE.search(llc_cmd)
-      if m and m.groups()[0] == 'isel':
-        from UpdateTestChecks import isel as output_type
-      else:
-        from UpdateTestChecks import asm as output_type
-
       common.verify_filecheck_prefixes(filecheck_cmd)
       if llc_tool not in LLC_LIKE_TOOLS:
         common.warn('Skipping non-llc RUN line: ' + l)
         continue
 
       if not filecheck_cmd.startswith('FileCheck '):
         common.warn('Skipping non-FileChecked RUN line: ' + l)
         continue
for prefixes, llc_tool, llc_args, preprocess_cmd, triple_in_cmd, march_in_cmd in run_list:
 
       raw_tool_output = common.invoke_tool(ti.args.llc_binary or llc_tool,
                                            llc_args, ti.path, preprocess_cmd,
                                            verbose=ti.args.verbose)
       triple = triple_in_cmd or triple_in_ir
       if not triple:
         triple = common.get_triple_from_march(march_in_cmd)
 
+      m = common.DEBUG_ONLY_ARG_RE.search(llc_args)
+      if m and m.groups()[0] == 'isel':
+        from UpdateTestChecks import isel as output_type
+      else:
+        from UpdateTestChecks import asm as output_type
       scrubber, function_re = output_type.get_run_handler(triple)
       builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True)
       builder.processed_prefixes(prefixes)
 
     func_dict = builder.finish_and_get_func_dict()
     global_vars_seen_dict = {}
 
     is_in_function = False