Make IR labels more precise

[MaskRay]

Summary

Previous IR labels were defined as "@<func>(". This causes problems because that sequence shows up in calls as well as function definitions. Constrain the labels a bit more by including the define keyword and matching the text before the function name.

[RKSimon]

Is there an existing test file in trunk that you can regenerate to the show the diff?

[greened]

In D68153#1688102, @RKSimon wrote:

Is there an existing test file in trunk that you can regenerate to the show the diff?

Sure. Does this help?

diff --git a/llvm/test/Transforms/BDCE/dead-uses.ll b/llvm/test/Transforms/BDCE/dead-uses.ll
index 34f8cc007f14..42d0f8edca43 100644
--- a/llvm/test/Transforms/BDCE/dead-uses.ll
+++ b/llvm/test/Transforms/BDCE/dead-uses.ll
@@ -8,7 +8,7 @@ declare <2 x i32> @llvm.fshr.v2i32(<2 x i32>, <2 x i32>, <2 x i32>)
 
 ; First fshr operand is dead.
 define i32 @pr39771_fshr_multi_use_instr(i32 %a) {
-; CHECK-LABEL: @pr39771_fshr_multi_use_instr(
+; CHECK-LABEL: define {{[^@]+}}@pr39771_fshr_multi_use_instr(
 ; CHECK-NEXT:    [[X:%.*]] = or i32 [[A:%.*]], 0
 ; CHECK-NEXT:    [[B:%.*]] = tail call i32 @llvm.fshr.i32(i32 0, i32 [[X]], i32 1)
 ; CHECK-NEXT:    [[C:%.*]] = lshr i32 [[B]], 23
@@ -26,7 +26,7 @@ define i32 @pr39771_fshr_multi_use_instr(i32 %a) {
 
 ; First fshr operand is dead (vector variant).
 define <2 x i32> @pr39771_fshr_multi_use_instr_vec(<2 x i32> %a) {
-; CHECK-LABEL: @pr39771_fshr_multi_use_instr_vec(
+; CHECK-LABEL: define {{[^@]+}}@pr39771_fshr_multi_use_instr_vec(
 ; CHECK-NEXT:    [[X:%.*]] = or <2 x i32> [[A:%.*]], zeroinitializer
 ; CHECK-NEXT:    [[B:%.*]] = tail call <2 x i32> @llvm.fshr.v2i32(<2 x i32> zeroinitializer, <2 x i32> [[X]], <2 x i32> <i32 1, i32 1>)                                                                                                                           
 ; CHECK-NEXT:    [[C:%.*]] = lshr <2 x i32> [[B]], <i32 23, i32 23>
@@ -44,7 +44,7 @@ define <2 x i32> @pr39771_fshr_multi_use_instr_vec(<2 x i32> %a) {
 
 ; First fshr operand is dead, but it comes from an argument, not instruction.
 define i32 @pr39771_fshr_multi_use_arg(i32 %a) {
-; CHECK-LABEL: @pr39771_fshr_multi_use_arg(
+; CHECK-LABEL: define {{[^@]+}}@pr39771_fshr_multi_use_arg(
 ; CHECK-NEXT:    [[B:%.*]] = tail call i32 @llvm.fshr.i32(i32 0, i32 [[A:%.*]], i32 1)
 ; CHECK-NEXT:    [[C:%.*]] = lshr i32 [[B]], 23
 ; CHECK-NEXT:    [[D:%.*]] = xor i32 [[C]], [[B]]
@@ -59,7 +59,7 @@ define i32 @pr39771_fshr_multi_use_arg(i32 %a) {
 }
 
 define i32 @pr39771_expanded_fshr_multi_use(i32 %a) {
-; CHECK-LABEL: @pr39771_expanded_fshr_multi_use(
+; CHECK-LABEL: define {{[^@]+}}@pr39771_expanded_fshr_multi_use(
 ; CHECK-NEXT:    [[TMP:%.*]] = lshr i32 [[A:%.*]], 1
 ; CHECK-NEXT:    [[TMP2:%.*]] = shl i32 0, 31
 ; CHECK-NEXT:    [[B:%.*]] = or i32 [[TMP]], [[TMP2]]
@@ -79,7 +79,7 @@ define i32 @pr39771_expanded_fshr_multi_use(i32 %a) {
 
 ; %b operand of %c will be dead initially, but later found live.
 define void @dead_use_invalidation(i32 %a) {
-; CHECK-LABEL: @dead_use_invalidation(
+; CHECK-LABEL: define {{[^@]+}}@dead_use_invalidation(
 ; CHECK-NEXT:    [[B:%.*]] = or i32 [[A:%.*]], 0
 ; CHECK-NEXT:    [[C:%.*]] = shl i32 [[B]], 31
 ; CHECK-NEXT:    [[D:%.*]] = and i32 [[C]], 1

[RKSimon]

Wouldn't this mean that every regeneration would see this change?

[greened]

In D68153#1689791, @RKSimon wrote:

Wouldn't this mean that every regeneration would see this change?

Yes. The label pattern is just wrong as-is because it will match calls in some cases.

[jdoerfert]

In D68153#1698833, @greened wrote:

In D68153#1689791, @RKSimon wrote:

Wouldn't this mean that every regeneration would see this change?

Yes. The label pattern is just wrong as-is because it will match calls in some cases.

I experienced this case myself now. I really like this change and I think breaking things now is better than debugging it over and over again.

[jdoerfert]

I stole this and added it into D68819 as well.

[RKSimon]

@greened Abandon since D68819 landed?

[jdoerfert]

This is already happening under the --function-signature flag. I think we can abandon this patch.

[RKSimon]

D68819 covered this afaict

llvm/utils/UpdateTestChecks/common.py

for checkprefix in checkprefixes:
       # Add space between different check prefixes and also before the first
       # line of code in the test function.
       output_lines.append(comment_marker)
       break
 
 def add_ir_checks(output_lines, comment_marker, prefix_list, func_dict,
                   func_name, preserve_names):
   # Label format is based on IR string.
-  check_label_format = '{} %s-LABEL: @%s('.format(comment_marker)
+  function_def_regex = 'define {{[^@]+}}'
+  check_label_format = '{} %s-LABEL: {}@%s('.format(comment_marker, function_def_regex)
   add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name,
              check_label_format, False, preserve_names)
 
 def add_analyze_checks(output_lines, comment_marker, prefix_list, func_dict, func_name):
   check_label_format = '{} %s-LABEL: \'%s\''.format(comment_marker)
   add_checks(output_lines, comment_marker, prefix_list, func_dict, func_name, check_label_format, False, True)