[LLVM][Autoupgrade] Delete invalid lifetime.start/lifetime.end intrinsic decls and calls - #206769
Conversation
dda4382 to
40ee722
Compare
LLVM's AutoUpgrade used to create invalid and unused `lifetime.start.i64` and `lifetime.end.i64` intrinsics. This was fixed with llvm#204601. However, existing bitcode generated prior to this fix might still have these unused and invalid declarations in them. Adopt Autoupgrade to handle them. Invalid declaration of these intrinsics will be deleted, and if there exists calls to these invalid intrinsics (not expected to be generated by AutoUpgrade prior to the bug fix, but handling this case just for completeness) these calls will be deleted.
40ee722 to
c2fe088
Compare
|
@llvm/pr-subscribers-llvm-ir Author: Rahul Joshi (jurahul) ChangesAutoUpgrade used to create invalid and unused Invalid declaration of these intrinsics will be deleted, and if there exists calls to these invalid intrinsics (not expected to be generated by AutoUpgrade prior to the bug fix, but handling this case just for completeness) these calls will be deleted as well. Also tested with Full diff: https://github.com/llvm/llvm-project/pull/206769.diff 2 Files Affected:
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 3a823f906b012..463088d2b283c 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -1552,21 +1552,34 @@ static bool upgradeIntrinsicFunction1(Function *F, Function *&NewFn,
return true;
}
break;
- case 'l':
- if ((Name.starts_with("lifetime.start") ||
- Name.starts_with("lifetime.end")) &&
- F->arg_size() == 2) {
- Intrinsic::ID IID = Name.starts_with("lifetime.start")
- ? Intrinsic::lifetime_start
- : Intrinsic::lifetime_end;
- rename(F);
- // Old 2 argument form of these intrinsics have [Size, Ptr] as arguments.
- // Use the Ptr argument to create new declaration.
- NewFn = Intrinsic::getOrInsertDeclaration(F->getParent(), IID,
- F->getArg(1)->getType());
- return true;
+ case 'l': {
+ bool IsLifetimeStart = Name.consume_front("lifetime.start");
+ bool IsLifetimeEnd = !IsLifetimeStart && Name.consume_front("lifetime.end");
+ if (IsLifetimeStart || IsLifetimeEnd) {
+ if (F->arg_size() == 2) {
+ Intrinsic::ID IID = IsLifetimeStart ? Intrinsic::lifetime_start
+ : Intrinsic::lifetime_end;
+ rename(F);
+ // Old 2 argument form of these intrinsics have [Size, Ptr] as
+ // arguments. Use the Ptr argument to create new declaration.
+ NewFn = Intrinsic::getOrInsertDeclaration(F->getParent(), IID,
+ F->getArg(1)->getType());
+ return true;
+ } else if (F->arg_size() == 1 && Name == ".i64") {
+ // Matches @llvm.lifetime.{start/end}.i64 which used to be created by
+ // Autoupgrade prior to
+ // https://github.com/llvm/llvm-project/pull/204601. This is an invalid
+ // intrinsic with no expected calls. To allow auto-upgrade process to
+ // delete such invalid intrinsic declaration, set NewFn = nullptr
+ // and return true here. If there are actual calls to this intrinsics
+ // (which is not expected), they will be deleted in
+ // UpgradeIntrinsicCall.
+ NewFn = nullptr;
+ return true;
+ }
}
break;
+ }
case 'm': {
// Updating the memory intrinsics (memcpy/memmove/memset) that have an
// alignment parameter to embedding the alignment as an attribute of
@@ -5125,6 +5138,9 @@ void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) {
Rep = upgradeVectorSplice(CI, Builder);
} else if (Name.consume_front("convert.")) {
Rep = upgradeConvertIntrinsicCall(Name, CI, F, Builder);
+ } else if (Name == "lifetime.start.i64" || Name == "lifetime.end.i64") {
+ // Delete calls to invalid @llvm.lifetime.{start,end}.i64 intrinsics.
+ Rep = nullptr;
} else {
llvm_unreachable("Unknown function for CallBase upgrade.");
}
diff --git a/llvm/test/Assembler/autoupgrade-lifetime-intrinsics-invalid.ll b/llvm/test/Assembler/autoupgrade-lifetime-intrinsics-invalid.ll
new file mode 100644
index 0000000000000..2555962e45599
--- /dev/null
+++ b/llvm/test/Assembler/autoupgrade-lifetime-intrinsics-invalid.ll
@@ -0,0 +1,30 @@
+; RUN: split-file %s %t
+
+;--- test1.ll
+; Verify that auto-upgrade eliminate the unused invalid declaration of lifetime
+; start and end intrinsics.
+; RUN: llvm-as < %t/test1.ll | llvm-dis | FileCheck %t/test1.ll
+; CHECK-NOT: @llvm.lifetime.start.i64(i64)
+; CHECK-NOT: @llvm.lifetime.end.i64(i64)
+
+declare void @llvm.lifetime.start.i64(i64)
+declare void @llvm.lifetime.end.i64(i64)
+
+;--- test2.ll
+; Verify that if there is an actual call to the invalid lifetime intrinsics,
+; the calls and the declaration will be deleted.
+; RUN: llvm-as < %t/test2.ll | llvm-dis | FileCheck %t/test2.ll
+
+; CHECK-NOT: @llvm.lifetime.start.i64(i64)
+; CHECK-NOT: @llvm.lifetime.end.i64(i64)
+; CHECK-NOT: call void @llvm.lifetime.start.i64(i64 0)
+; CHECK-NOT: call void @llvm.lifetime.end.i64(i64 0)
+
+declare void @llvm.lifetime.start.i64(i64)
+declare void @llvm.lifetime.end.i64(i64)
+
+define void @foo() {
+ call void @llvm.lifetime.start.i64(i64 0)
+ call void @llvm.lifetime.end.i64(i64 0)
+ ret void
+}
|
|
Looking at https://ci.swift.org/job/llvm.org/job/test-suite-verify-machineinstrs-x86_64-O3/job/main/ I still see the CI failing even after my fix. However, locally if I build clang (on Linux) and run the same/similar command I don't see any failure: @cofibrant any idea why the CI might still be failing? |
|
Latest build is green so presumably unrelated |
…sic decls and calls (llvm#206769) AutoUpgrade used to create invalid and unused `lifetime.start.i64` and `lifetime.end.i64` intrinsics. This was fixed with llvm#204601. However, existing bitcode generated prior to this fix might still have these unused and invalid declarations in them, which now fail IR verification. Adopt Autoupgrade to clean them up. Invalid declaration of these intrinsics will be deleted, and if there exists calls to these invalid intrinsics (not expected to be generated by AutoUpgrade prior to the bug fix, but handling this case just for completeness) these calls will be deleted as well. Also tested with `sap_fct_splitting.bc` reported in llvm#204478 (comment) (though not committing that as a unit test).
AutoUpgrade used to create invalid and unused
lifetime.start.i64andlifetime.end.i64intrinsics. This was fixed with #204601. However, existing bitcode generated prior to this fix might still have these unused and invalid declarations in them, which now fail IR verification. Adopt Autoupgrade to clean them up.Invalid declaration of these intrinsics will be deleted, and if there exists calls to these invalid intrinsics (not expected to be generated by AutoUpgrade prior to the bug fix, but handling this case just for completeness) these calls will be deleted as well.
Also tested with
sap_fct_splitting.bcreported in #204478 (comment) (though not committing that as a unit test).