Skip to content

PR for llvm/llvm-project#64289 #556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,12 @@ bool TailRecursionEliminator::eliminateCall(CallInst *CI) {
for (unsigned I = 0, E = CI->arg_size(); I != E; ++I) {
if (CI->isByValArgument(I)) {
copyLocalTempOfByValueOperandIntoArguments(CI, I);
// When eliminating a tail call, we modify the values of the arguments.
// Therefore, if the byval parameter has a readonly attribute, we have to
// remove it. It is safe because, from the perspective of a caller, the
// byval parameter is always treated as "readonly," even if the readonly
// attribute is removed.
F.removeParamAttr(I, Attribute::ReadOnly);
ArgumentPHIs[I]->addIncoming(F.getArg(I), BB);
} else
ArgumentPHIs[I]->addIncoming(CI->getArgOperand(I), BB);
Expand Down
27 changes: 27 additions & 0 deletions llvm/test/Transforms/PhaseOrdering/pr64289-tce.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -O3 < %s | FileCheck %s

; A miscompilation found on https://github.com/llvm/llvm-project/issues/64289.
; 1. PostOrderFunctionAttrsPass added readonly to the parameter.
; 2. TailCallElimPass modified the parameter but kept readonly.
; 3. LICMPass incorrectly hoisted the load instruction.

define void @pr64289(ptr noalias byval(i64) %x) {
; CHECK-LABEL: @pr64289(
; CHECK-NEXT: start:
; CHECK-NEXT: ret void
;
start:
%new_x = alloca i64, align 8
%x_val = load i64, ptr %x, align 8
%is_zero = icmp eq i64 %x_val, 0
br i1 %is_zero, label %end, label %recurse

recurse:
store i64 0, ptr %new_x, align 8
call void @pr64289(ptr %new_x)
br label %end

end:
ret void
}
17 changes: 9 additions & 8 deletions llvm/test/Transforms/TailCallElim/tre-byval-parameter-2.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
; RUN: opt < %s -passes=tailcallelim -verify-dom-info -S | FileCheck %s

; the test was generated from the following C++ source:
Expand All @@ -24,23 +24,24 @@

; Function Attrs: noinline nounwind uwtable
define dso_local void @_Z7dostuff1AS_i(ptr nocapture byval(%struct.A) align 8 %a, ptr nocapture readonly byval(%struct.A) align 8 %b, i32 %i) local_unnamed_addr #0 {
; CHECK-LABEL: @_Z7dostuff1AS_i(
; CHECK-LABEL: define {{[^@]+}}@_Z7dostuff1AS_i
; CHECK-SAME: (ptr nocapture byval([[STRUCT_A:%.*]]) align 8 [[A:%.*]], ptr nocapture byval([[STRUCT_A]]) align 8 [[B:%.*]], i32 [[I:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[AGG_TMP52:%.*]] = alloca [[STRUCT_A:%.*]], align 8
; CHECK-NEXT: [[AGG_TMP52:%.*]] = alloca [[STRUCT_A]], align 8
; CHECK-NEXT: [[AGG_TMP1:%.*]] = alloca [[STRUCT_A]], align 8
; CHECK-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_A]], align 8
; CHECK-NEXT: [[AGG_TMP5:%.*]] = alloca [[STRUCT_A]], align 8
; CHECK-NEXT: br label [[TAILRECURSE:%.*]]
; CHECK: tailrecurse:
; CHECK-NEXT: [[I_TR:%.*]] = phi i32 [ [[I:%.*]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[IF_END:%.*]] ]
; CHECK-NEXT: [[I_TR:%.*]] = phi i32 [ [[I]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[IF_END:%.*]] ]
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[I_TR]], 10
; CHECK-NEXT: br i1 [[CMP]], label [[RETURN:%.*]], label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [[STRUCT_A]], ptr [[A:%.*]], i64 0, i32 0, i64 5
; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [[STRUCT_A]], ptr [[A]], i64 0, i32 0, i64 5
; CHECK-NEXT: [[TMP0:%.*]] = load i64, ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[INC:%.*]] = add nsw i64 [[TMP0]], 1
; CHECK-NEXT: store i64 [[INC]], ptr [[ARRAYIDX]], align 8
; CHECK-NEXT: [[ARRAYIDX4:%.*]] = getelementptr inbounds [[STRUCT_A]], ptr [[B:%.*]], i64 0, i32 0, i64 5
; CHECK-NEXT: [[ARRAYIDX4:%.*]] = getelementptr inbounds [[STRUCT_A]], ptr [[B]], i64 0, i32 0, i64 5
; CHECK-NEXT: [[TMP1:%.*]] = load i64, ptr [[ARRAYIDX4]], align 8
; CHECK-NEXT: [[CALL:%.*]] = tail call i32 (ptr, ...) @printf(ptr nonnull dereferenceable(1) @.str, i64 [[INC]], i64 [[TMP1]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 80, ptr nonnull [[AGG_TMP]])
Expand Down Expand Up @@ -101,13 +102,13 @@ declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture) #2

; Function Attrs: noinline norecurse nounwind optnone uwtable
define dso_local i32 @main() local_unnamed_addr #3 {
; CHECK-LABEL: @main(
; CHECK-LABEL: define {{[^@]+}}@main() local_unnamed_addr {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_A:%.*]], align 8
; CHECK-NEXT: [[AGG_TMP1:%.*]] = alloca [[STRUCT_A]], align 8
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[AGG_TMP]], ptr align 8 @global, i64 80, i1 false)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[AGG_TMP1]], ptr align 8 @global, i64 80, i1 false)
; CHECK-NEXT: tail call void @_Z7dostuff1AS_i(ptr byval(%struct.A) align 8 [[AGG_TMP]], ptr byval(%struct.A) align 8 [[AGG_TMP1]], i32 0)
; CHECK-NEXT: tail call void @_Z7dostuff1AS_i(ptr byval([[STRUCT_A]]) align 8 [[AGG_TMP]], ptr byval([[STRUCT_A]]) align 8 [[AGG_TMP1]], i32 0)
; CHECK-NEXT: ret i32 0
;
entry:
Expand Down
13 changes: 7 additions & 6 deletions llvm/test/Transforms/TailCallElim/tre-byval-parameter.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
; RUN: opt < %s -passes=tailcallelim -verify-dom-info -S | FileCheck %s

; the test was generated from the following C++ source:
Expand All @@ -24,21 +24,22 @@

; Function Attrs: uwtable
define dso_local i32 @_Z3fooi1S(i32 %count, ptr nocapture readonly byval(%struct.S) align 8 %p1) local_unnamed_addr #0 {
; CHECK-LABEL: @_Z3fooi1S(
; CHECK-LABEL: define {{[^@]+}}@_Z3fooi1S
; CHECK-SAME: (i32 [[COUNT:%.*]], ptr nocapture byval([[STRUCT_S:%.*]]) align 8 [[P1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[AGG_TMP_I1:%.*]] = alloca [[STRUCT_S:%.*]], align 8
; CHECK-NEXT: [[AGG_TMP_I1:%.*]] = alloca [[STRUCT_S]], align 8
; CHECK-NEXT: [[AGG_TMP_I:%.*]] = alloca [[STRUCT_S]], align 8
; CHECK-NEXT: [[AGG_TMP14:%.*]] = alloca [[STRUCT_S]], align 8
; CHECK-NEXT: [[AGG_TMP:%.*]] = alloca [[STRUCT_S]], align 8
; CHECK-NEXT: [[AGG_TMP1:%.*]] = alloca [[STRUCT_S]], align 8
; CHECK-NEXT: br label [[TAILRECURSE:%.*]]
; CHECK: tailrecurse:
; CHECK-NEXT: [[COUNT_TR:%.*]] = phi i32 [ [[COUNT:%.*]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[IF_END:%.*]] ]
; CHECK-NEXT: [[COUNT_TR:%.*]] = phi i32 [ [[COUNT]], [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[IF_END:%.*]] ]
; CHECK-NEXT: [[CMP:%.*]] = icmp sgt i32 [[COUNT_TR]], 10
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END]]
; CHECK: if.then:
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr nonnull align 8 dereferenceable(20) [[AGG_TMP]], ptr nonnull align 8 dereferenceable(20) [[P1:%.*]], i64 20, i1 false)
; CHECK-NEXT: [[CALL:%.*]] = tail call i32 @_Z3zoo1S(ptr nonnull byval(%struct.S) align 8 [[AGG_TMP]])
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr nonnull align 8 dereferenceable(20) [[AGG_TMP]], ptr nonnull align 8 dereferenceable(20) [[P1]], i64 20, i1 false)
; CHECK-NEXT: [[CALL:%.*]] = tail call i32 @_Z3zoo1S(ptr nonnull byval([[STRUCT_S]]) align 8 [[AGG_TMP]])
; CHECK-NEXT: br label [[RETURN:%.*]]
; CHECK: if.end:
; CHECK-NEXT: [[ADD]] = add nsw i32 [[COUNT_TR]], 1
Expand Down