[llvm] [verify][swift] Allow passing swifterror to phi instructions (PR #138598)
Ellis Hoag via llvm-commits
llvm-commits at lists.llvm.org
Mon May 5 16:04:59 PDT 2025
https://github.com/ellishg updated https://github.com/llvm/llvm-project/pull/138598
>From da187ea19130796243ee92921a6638e26725dde2 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellishoag at meta.com>
Date: Mon, 5 May 2025 14:48:53 -0700
Subject: [PATCH 1/2] [verify][swift] Allow passing swifterror to phi
instructions
---
llvm/lib/IR/Verifier.cpp | 56 ++++++++++++++++++++++++--------
llvm/test/Verifier/swifterror.ll | 34 ++++++++++++++++++-
2 files changed, 76 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index a798808d79656..2e5ff4b5df8dd 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3653,18 +3653,36 @@ void Verifier::visitCallBase(CallBase &Call) {
// well.
for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
if (Call.paramHasAttr(i, Attribute::SwiftError)) {
- Value *SwiftErrorArg = Call.getArgOperand(i);
- if (auto AI = dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
- Check(AI->isSwiftError(),
- "swifterror argument for call has mismatched alloca", AI, Call);
- continue;
+ SetVector<const Value *> Args;
+ Args.insert(Call.getArgOperand(i));
+ bool DidChange;
+ do {
+ DidChange = false;
+ // Inherit the incoming values of phi instructions to capture all
+ // values.
+ for (const Value *Arg : Args)
+ if (auto *PhiI = dyn_cast<PHINode>(Arg))
+ for (const auto &Op : PhiI->incoming_values())
+ DidChange |= Args.insert(Op.get());
+ } while (DidChange);
+
+ for (const Value *SwiftErrorArg : Args) {
+ if (isa<PHINode>(SwiftErrorArg))
+ continue;
+ if (auto AI =
+ dyn_cast<AllocaInst>(SwiftErrorArg->stripInBoundsOffsets())) {
+ Check(AI->isSwiftError(),
+ "swifterror argument for call has mismatched alloca", AI, Call);
+ continue;
+ }
+ auto ArgI = dyn_cast<Argument>(SwiftErrorArg);
+ Check(ArgI,
+ "swifterror argument should come from an alloca or parameter",
+ SwiftErrorArg, Call);
+ Check(ArgI->hasSwiftErrorAttr(),
+ "swifterror argument for call has mismatched parameter", ArgI,
+ Call);
}
- auto ArgI = dyn_cast<Argument>(SwiftErrorArg);
- Check(ArgI, "swifterror argument should come from an alloca or parameter",
- SwiftErrorArg, Call);
- Check(ArgI->hasSwiftErrorAttr(),
- "swifterror argument for call has mismatched parameter", ArgI,
- Call);
}
if (Attrs.hasParamAttr(i, Attribute::ImmArg)) {
@@ -4356,11 +4374,23 @@ void Verifier::verifySwiftErrorCall(CallBase &Call,
}
void Verifier::verifySwiftErrorValue(const Value *SwiftErrorVal) {
+ SetVector<const User *> Users(SwiftErrorVal->users().begin(),
+ SwiftErrorVal->users().end());
+ bool DidChange;
+ do {
+ DidChange = false;
+ // Inherit the users of phi instructions to capture all users.
+ for (const User *U : Users)
+ if (auto PhiI = dyn_cast<PHINode>(U))
+ for (const User *U : PhiI->users())
+ DidChange |= Users.insert(U);
+ } while (DidChange);
+
// Check that swifterror value is only used by loads, stores, or as
// a swifterror argument.
- for (const User *U : SwiftErrorVal->users()) {
+ for (const User *U : Users) {
Check(isa<LoadInst>(U) || isa<StoreInst>(U) || isa<CallInst>(U) ||
- isa<InvokeInst>(U),
+ isa<InvokeInst>(U) || isa<PHINode>(U),
"swifterror value can only be loaded and stored from, or "
"as a swifterror argument!",
SwiftErrorVal, U);
diff --git a/llvm/test/Verifier/swifterror.ll b/llvm/test/Verifier/swifterror.ll
index d27b43234cadc..ae340d24068fb 100644
--- a/llvm/test/Verifier/swifterror.ll
+++ b/llvm/test/Verifier/swifterror.ll
@@ -1,4 +1,4 @@
-; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=swifterror
%swift_error = type {i64, i8}
@@ -10,6 +10,36 @@ define float @foo(ptr swifterror %error_ptr_ref) {
ret float 1.0
}
+; CHECK: swifterror value can only be loaded and stored from, or as a swifterror argument!
+; CHECK: %ptr0 = alloca swifterror ptr, align 8
+; CHECK: %t = getelementptr inbounds ptr, ptr %err, i64 1
+; CHECK: swifterror value can only be loaded and stored from, or as a swifterror argument!
+; CHECK: %ptr1 = alloca swifterror ptr, align 8
+; CHECK: %t = getelementptr inbounds ptr, ptr %err, i64 1
+define float @phi(i1 %a) {
+entry:
+ %ptr0 = alloca swifterror ptr, align 8
+ %ptr1 = alloca swifterror ptr, align 8
+ %ptr2 = alloca ptr, align 8
+ br i1 %a, label %body, label %body2
+
+body:
+ %err = phi ptr [ %ptr0, %entry ], [ %ptr1, %body ]
+ %t = getelementptr inbounds ptr, ptr %err, i64 1
+ br label %body
+
+; CHECK: swifterror argument for call has mismatched alloca
+; CHECK: %ptr2 = alloca ptr, align 8
+; CHECK: %call = call float @foo(ptr swifterror %err_bad)
+body2:
+ %err_bad = phi ptr [ %ptr0, %entry ], [ %ptr2, %body2 ]
+ %call = call float @foo(ptr swifterror %err_bad)
+ br label %body2
+
+end:
+ ret float 1.0
+}
+
; CHECK: swifterror argument for call has mismatched alloca
; CHECK: %error_ptr_ref = alloca ptr
; CHECK: %call = call float @foo(ptr swifterror %error_ptr_ref)
@@ -22,12 +52,14 @@ entry:
}
; CHECK: swifterror alloca must have pointer type
+; CHECK: %a = alloca swifterror i128, align 8
define void @swifterror_alloca_invalid_type() {
%a = alloca swifterror i128
ret void
}
; CHECK: swifterror alloca must not be array allocation
+; CHECK: %a = alloca swifterror ptr, i64 2, align 8
define void @swifterror_alloca_array() {
%a = alloca swifterror ptr, i64 2
ret void
>From e04291818f11610b47f9df99eee7de1e78c437a2 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellishoag at meta.com>
Date: Mon, 5 May 2025 16:04:48 -0700
Subject: [PATCH 2/2] use worklist
---
llvm/lib/IR/Verifier.cpp | 55 ++++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 2e5ff4b5df8dd..b9fa2a540f706 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3653,20 +3653,22 @@ void Verifier::visitCallBase(CallBase &Call) {
// well.
for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
if (Call.paramHasAttr(i, Attribute::SwiftError)) {
- SetVector<const Value *> Args;
- Args.insert(Call.getArgOperand(i));
- bool DidChange;
- do {
- DidChange = false;
- // Inherit the incoming values of phi instructions to capture all
- // values.
- for (const Value *Arg : Args)
- if (auto *PhiI = dyn_cast<PHINode>(Arg))
- for (const auto &Op : PhiI->incoming_values())
- DidChange |= Args.insert(Op.get());
- } while (DidChange);
-
- for (const Value *SwiftErrorArg : Args) {
+ const Value *Arg = Call.getArgOperand(i);
+ SetVector<const Value *> Values;
+ Values.insert(Arg);
+ SmallVector<const PHINode *> PHIWorkList;
+ if (auto *PhiI = dyn_cast<PHINode>(Arg))
+ PHIWorkList.push_back(PhiI);
+
+ while (!PHIWorkList.empty()) {
+ const auto *PhiI = PHIWorkList.pop_back_val();
+ for (const auto &Op : PhiI->incoming_values()) {
+ const auto *NextPhiI = dyn_cast<PHINode>(Op.get());
+ if (Values.insert(Op.get()) && NextPhiI)
+ PHIWorkList.push_back(NextPhiI);
+ }
+ }
+ for (const Value *SwiftErrorArg : Values) {
if (isa<PHINode>(SwiftErrorArg))
continue;
if (auto AI =
@@ -4376,21 +4378,26 @@ void Verifier::verifySwiftErrorCall(CallBase &Call,
void Verifier::verifySwiftErrorValue(const Value *SwiftErrorVal) {
SetVector<const User *> Users(SwiftErrorVal->users().begin(),
SwiftErrorVal->users().end());
- bool DidChange;
- do {
- DidChange = false;
- // Inherit the users of phi instructions to capture all users.
- for (const User *U : Users)
- if (auto PhiI = dyn_cast<PHINode>(U))
- for (const User *U : PhiI->users())
- DidChange |= Users.insert(U);
- } while (DidChange);
+ SmallVector<const PHINode *> PHIWorkList;
+ for (const User *U : Users)
+ if (auto *PhiI = dyn_cast<PHINode>(U))
+ PHIWorkList.push_back(PhiI);
+ while (!PHIWorkList.empty()) {
+ const auto *PhiI = PHIWorkList.pop_back_val();
+ for (const User *U : PhiI->users()) {
+ const auto *NextPhiI = dyn_cast<PHINode>(U);
+ if (Users.insert(U) && NextPhiI)
+ PHIWorkList.push_back(NextPhiI);
+ }
+ }
// Check that swifterror value is only used by loads, stores, or as
// a swifterror argument.
for (const User *U : Users) {
+ if (isa<PHINode>(U))
+ continue;
Check(isa<LoadInst>(U) || isa<StoreInst>(U) || isa<CallInst>(U) ||
- isa<InvokeInst>(U) || isa<PHINode>(U),
+ isa<InvokeInst>(U),
"swifterror value can only be loaded and stored from, or "
"as a swifterror argument!",
SwiftErrorVal, U);
More information about the llvm-commits
mailing list