Page MenuHomePhabricator

Make IR labels more precise
Needs ReviewPublic

Authored by greened on Sep 27 2019, 12:02 PM.

Details

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.

Diff Detail

Event Timeline

greened created this revision.Sep 27 2019, 12:02 PM
greened edited the summary of this revision. (Show Details)Sep 30 2019, 6:47 AM

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

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

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

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.

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.

I stole this and added it into D68819 as well.

@greened Abandon since D68819 landed?

xbolva00 resigned from this revision.Sat, Nov 30, 5:55 AM