Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented May 16, 2025

Closes #140215.

@dtcxzyw dtcxzyw requested a review from nikic as a code owner May 16, 2025 13:25
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels May 16, 2025
@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Closes #140215.


Full diff: https://github.com/llvm/llvm-project/pull/140250.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+6-3)
  • (modified) llvm/test/Transforms/InstCombine/vararg.ll (+41-11)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 3d35bf753c40e..12e08c09ea67d 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -816,9 +816,12 @@ removeTriviallyEmptyRange(IntrinsicInst &EndI, InstCombinerImpl &IC,
 }
 
 Instruction *InstCombinerImpl::visitVAEndInst(VAEndInst &I) {
-  removeTriviallyEmptyRange(I, *this, [](const IntrinsicInst &I) {
-    return I.getIntrinsicID() == Intrinsic::vastart ||
-           I.getIntrinsicID() == Intrinsic::vacopy;
+  removeTriviallyEmptyRange(I, *this, [&I](const IntrinsicInst &II) {
+    // Bail out on the case where the source va_list of a va_copy is destroyed
+    // immediately by a follow-up va_end.
+    return II.getIntrinsicID() == Intrinsic::vastart ||
+           (II.getIntrinsicID() == Intrinsic::vacopy &&
+            I.getArgOperand(0) != II.getArgOperand(1));
   });
   return nullptr;
 }
diff --git a/llvm/test/Transforms/InstCombine/vararg.ll b/llvm/test/Transforms/InstCombine/vararg.ll
index ef75b0c91ce27..eb24256cfa9bc 100644
--- a/llvm/test/Transforms/InstCombine/vararg.ll
+++ b/llvm/test/Transforms/InstCombine/vararg.ll
@@ -1,17 +1,14 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
 
 %struct.__va_list = type { ptr, ptr, ptr, i32, i32 }
 
-declare void @llvm.lifetime.start.p0(i64, ptr nocapture)
-declare void @llvm.lifetime.end.p0(i64, ptr nocapture)
-declare void @llvm.va_start(ptr)
-declare void @llvm.va_end(ptr)
-declare void @llvm.va_copy(ptr, ptr)
-
-define i32 @func(ptr nocapture readnone %fmt, ...) {
-; CHECK-LABEL: @func(
-; CHECK: entry:
-; CHECK-NEXT: ret i32 0
+define void @func(ptr nocapture readnone %fmt, ...) {
+; CHECK-LABEL: define void @func(
+; CHECK-SAME: ptr readnone captures(none) [[FMT:%.*]], ...) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    ret void
+;
 entry:
   %va0 = alloca %struct.__va_list, align 8
   %va1 = alloca %struct.__va_list, align 8
@@ -23,6 +20,39 @@ entry:
   call void @llvm.lifetime.end.p0(i64 32, ptr %va1)
   call void @llvm.va_end(ptr %va0)
   call void @llvm.lifetime.end.p0(i64 32, ptr %va0)
-  ret i32 0
+  ret void
 }
 
+declare void @callee(ptr)
+
+define void @func_destroy_copy_src(ptr nocapture readnone %fmt, ...) {
+; CHECK-LABEL: define void @func_destroy_copy_src(
+; CHECK-SAME: ptr readnone captures(none) [[FMT:%.*]], ...) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[VA0:%.*]] = alloca [[STRUCT___VA_LIST:%.*]], align 8
+; CHECK-NEXT:    [[VA1:%.*]] = alloca [[STRUCT___VA_LIST]], align 8
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 32, ptr nonnull [[VA0]])
+; CHECK-NEXT:    call void @llvm.lifetime.start.p0(i64 32, ptr nonnull [[VA1]])
+; CHECK-NEXT:    call void @llvm.va_start.p0(ptr nonnull [[VA0]])
+; CHECK-NEXT:    call void @llvm.va_copy.p0(ptr nonnull [[VA1]], ptr nonnull [[VA0]])
+; CHECK-NEXT:    call void @llvm.va_end.p0(ptr [[VA0]])
+; CHECK-NEXT:    call void @callee(ptr nonnull [[VA1]])
+; CHECK-NEXT:    call void @llvm.va_end.p0(ptr [[VA1]])
+; CHECK-NEXT:    call void @llvm.lifetime.end.p0(i64 32, ptr nonnull [[VA1]])
+; CHECK-NEXT:    call void @llvm.lifetime.end.p0(i64 32, ptr nonnull [[VA0]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  %va0 = alloca %struct.__va_list, align 8
+  %va1 = alloca %struct.__va_list, align 8
+  call void @llvm.lifetime.start.p0(i64 32, ptr %va0)
+  call void @llvm.lifetime.start.p0(i64 32, ptr %va1)
+  call void @llvm.va_start(ptr %va0)
+  call void @llvm.va_copy(ptr %va1, ptr %va0)
+  call void @llvm.va_end(ptr %va0)
+  call void @callee(ptr %va1)
+  call void @llvm.va_end(ptr %va1)
+  call void @llvm.lifetime.end.p0(i64 32, ptr %va1)
+  call void @llvm.lifetime.end.p0(i64 32, ptr %va0)
+  ret void
+}

@dtcxzyw dtcxzyw merged commit 8d0ebd8 into llvm:main May 17, 2025
14 checks passed
@dtcxzyw dtcxzyw deleted the fix-140215 branch May 17, 2025 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InstCombine pass wrongly removes va_start/va_end

3 participants