From ad6b5762b64f6706cb4d1bcad383c373736b5af6 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Tue, 20 Dec 2022 08:18:50 -0500 Subject: [PATCH] SCCP: Don't assert on constantexpr casts of function uses This includes 2 different, related fixes: 1. Fix asserting on direct assume-like intrinsic uses of a function address 2. Fix asserting on constant expression casts used by assume-like intrinsics. By default hasAddressTaken permits assume-like intrinsic uses, which ignores assume-like calls and pointer casts of the address used by assume-like calls. Fixes #59602, but there are additional issues I encountered when debugging this. For instance, the original failing bitcast expression was really unused. Clang tentatively created it for the function type, but was unnecessary after applyGlobalValReplacements. That did not clean up the now dead ConstantExpr which hung around oun the user list, so this assert only reproduced when running clang from the original testcase, and didn't just running opt -passes=ipsccp. I don't know who is responsible for cleaning up unused ConstantExprs, but I've run into similar issues several times recently. Additionally, I found a few assertions with llvm.ssa.copy with functions and casts of functions as the argument. Another issue theoretically exists if hasAddressTaken chooses to respect nocapture when passed function addresses. The search here would need to do additional work to look at the users of the constant cast to see if any call sites need returned to be stripped. --- llvm/lib/Transforms/IPO/SCCP.cpp | 20 +++++-- .../SCCP/issue59602-assume-like-call-users.ll | 62 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 llvm/test/Transforms/SCCP/issue59602-assume-like-call-users.ll diff --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp index 19659f4..341550a 100644 --- a/llvm/lib/Transforms/IPO/SCCP.cpp +++ b/llvm/lib/Transforms/IPO/SCCP.cpp @@ -83,6 +83,14 @@ static void findReturnsToZap(Function &F, return !isOverdefined(LV); }); } + + // We don't consider assume-like intrinsics to be actual address + // captures. + if (auto *II = dyn_cast(U)) { + if (II->isAssumeLikeIntrinsic()) + return true; + } + return !isOverdefined(Solver.getLatticeValueFor(U)); }) && "We can only zap functions where all live users have a concrete value"); @@ -336,10 +344,16 @@ static bool runIPSCCP( for (Argument &A : F->args()) F->removeParamAttr(A.getArgNo(), Attribute::Returned); for (Use &U : F->uses()) { - // Skip over blockaddr users. - if (isa(U.getUser())) + CallBase *CB = dyn_cast(U.getUser()); + if (!CB) { + assert(isa(U.getUser()) || + (isa(U.getUser()) && + all_of(U.getUser()->users(), [](const User *UserUser) { + return cast(UserUser)->isAssumeLikeIntrinsic(); + }))); continue; - CallBase *CB = cast(U.getUser()); + } + for (Use &Arg : CB->args()) CB->removeParamAttr(CB->getArgOperandNo(&Arg), Attribute::Returned); } diff --git a/llvm/test/Transforms/SCCP/issue59602-assume-like-call-users.ll b/llvm/test/Transforms/SCCP/issue59602-assume-like-call-users.ll new file mode 100644 index 0000000..352eb21 --- /dev/null +++ b/llvm/test/Transforms/SCCP/issue59602-assume-like-call-users.ll @@ -0,0 +1,62 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature +; RUN: opt -S -passes=ipsccp < %s | FileCheck %s + +@extern = global i32 0 + +define i32 @call_assume_self_user() { +; CHECK-LABEL: define {{[^@]+}}@call_assume_self_user() { +; CHECK-NEXT: [[CALL:%.*]] = call i32 @assume_self_user() +; CHECK-NEXT: ret i32 0 +; + %call = call i32 @assume_self_user() + ret i32 0 +} + +; Function address direct user is assume-like intrinsic. +; +; Make sure this doesn't hit "We can only zap functions where all live +; users have a concrete value" +define internal i32 @assume_self_user() { +; CHECK-LABEL: define {{[^@]+}}@assume_self_user() { +; CHECK-NEXT: [[OBJSIZE:%.*]] = call i32 @llvm.objectsize.i32.p0(ptr @assume_self_user, i1 false, i1 false, i1 false) +; CHECK-NEXT: store i32 [[OBJSIZE]], ptr @extern, align 4 +; CHECK-NEXT: ret i32 undef +; + %objsize = call i32 @llvm.objectsize.i32.p0(ptr @assume_self_user, i1 false, i1 false, i1 false) + store i32 %objsize, ptr @extern + ret i32 0 +} + + +; The objectsize call is an "assume like intrinsic" that can be +; ignored for address taken purposes. The user of @constexpr_self_user +; is a constant expression cast, so that the use call site does not +; directly appear as a user. + +define i32 @callsite_with_returned() { +; CHECK-LABEL: define {{[^@]+}}@callsite_with_returned() { +; CHECK-NEXT: [[CALL:%.*]] = call addrspace(1) i32 @constexpr_self_user(i32 123) +; CHECK-NEXT: ret i32 0 +; + %call = call addrspace(1) i32 @constexpr_self_user(i32 returned 123) + ret i32 0 +} + +; Has constexpr cast user in an assume-like intrinsic. Make sure we +; don't assert on a cast for the user, and still remove +; returned from the caller. +define internal i32 @constexpr_self_user(i32 %arg0) addrspace(1) { +; CHECK-LABEL: define {{[^@]+}}@constexpr_self_user +; CHECK-SAME: (i32 [[ARG0:%.*]]) addrspace(1) { +; CHECK-NEXT: [[OBJSIZE:%.*]] = call i32 @llvm.objectsize.i32.p0(ptr addrspacecast (ptr addrspace(1) @constexpr_self_user to ptr), i1 false, i1 false, i1 false) +; CHECK-NEXT: store i32 [[OBJSIZE]], ptr @extern, align 4 +; CHECK-NEXT: ret i32 undef +; + %objsize = call i32 @llvm.objectsize.i32.p0(ptr addrspacecast (ptr addrspace(1) @constexpr_self_user to ptr), i1 false, i1 false, i1 false) + store i32 %objsize, ptr @extern + ret i32 %arg0 +} + +declare i32 @llvm.objectsize.i32.p0(ptr, i1 immarg, i1 immarg, i1 immarg) #0 + +attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) } -- 2.7.4