Skip to content

Commit a19096d

Browse files
dianqkcuviper
authored andcommitted
[TailCallElim] Remove the readonly attribute of byval.
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. Fixes llvm#64289. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D156793 (cherry picked from commit c3f227e)
1 parent 922c350 commit a19096d

File tree

4 files changed

+35
-2
lines changed

4 files changed

+35
-2
lines changed

llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,12 @@ bool TailRecursionEliminator::eliminateCall(CallInst *CI) {
675675
for (unsigned I = 0, E = CI->arg_size(); I != E; ++I) {
676676
if (CI->isByValArgument(I)) {
677677
copyLocalTempOfByValueOperandIntoArguments(CI, I);
678+
// When eliminating a tail call, we modify the values of the arguments.
679+
// Therefore, if the byval parameter has a readonly attribute, we have to
680+
// remove it. It is safe because, from the perspective of a caller, the
681+
// byval parameter is always treated as "readonly," even if the readonly
682+
// attribute is removed.
683+
F.removeParamAttr(I, Attribute::ReadOnly);
678684
ArgumentPHIs[I]->addIncoming(F.getArg(I), BB);
679685
} else
680686
ArgumentPHIs[I]->addIncoming(CI->getArgOperand(I), BB);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
2+
; RUN: opt -S -O3 < %s | FileCheck %s
3+
4+
; A miscompilation found on https://github.com/llvm/llvm-project/issues/64289.
5+
; 1. PostOrderFunctionAttrsPass added readonly to the parameter.
6+
; 2. TailCallElimPass modified the parameter but kept readonly.
7+
; 3. LICMPass incorrectly hoisted the load instruction.
8+
9+
define void @pr64289(ptr noalias byval(i64) %x) {
10+
; CHECK-LABEL: @pr64289(
11+
; CHECK-NEXT: start:
12+
; CHECK-NEXT: ret void
13+
;
14+
start:
15+
%new_x = alloca i64, align 8
16+
%x_val = load i64, ptr %x, align 8
17+
%is_zero = icmp eq i64 %x_val, 0
18+
br i1 %is_zero, label %end, label %recurse
19+
20+
recurse:
21+
store i64 0, ptr %new_x, align 8
22+
call void @pr64289(ptr %new_x)
23+
br label %end
24+
25+
end:
26+
ret void
27+
}

llvm/test/Transforms/TailCallElim/tre-byval-parameter-2.ll

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
; Function Attrs: noinline nounwind uwtable
2626
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 {
2727
; CHECK-LABEL: define {{[^@]+}}@_Z7dostuff1AS_i
28-
; CHECK-SAME: (ptr nocapture byval([[STRUCT_A:%.*]]) align 8 [[A:%.*]], ptr nocapture readonly byval([[STRUCT_A]]) align 8 [[B:%.*]], i32 [[I:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
28+
; 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]+]] {
2929
; CHECK-NEXT: entry:
3030
; CHECK-NEXT: [[AGG_TMP52:%.*]] = alloca [[STRUCT_A]], align 8
3131
; CHECK-NEXT: [[AGG_TMP1:%.*]] = alloca [[STRUCT_A]], align 8

llvm/test/Transforms/TailCallElim/tre-byval-parameter.ll

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
; Function Attrs: uwtable
2626
define dso_local i32 @_Z3fooi1S(i32 %count, ptr nocapture readonly byval(%struct.S) align 8 %p1) local_unnamed_addr #0 {
2727
; CHECK-LABEL: define {{[^@]+}}@_Z3fooi1S
28-
; CHECK-SAME: (i32 [[COUNT:%.*]], ptr nocapture readonly byval([[STRUCT_S:%.*]]) align 8 [[P1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
28+
; CHECK-SAME: (i32 [[COUNT:%.*]], ptr nocapture byval([[STRUCT_S:%.*]]) align 8 [[P1:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
2929
; CHECK-NEXT: entry:
3030
; CHECK-NEXT: [[AGG_TMP_I1:%.*]] = alloca [[STRUCT_S]], align 8
3131
; CHECK-NEXT: [[AGG_TMP_I:%.*]] = alloca [[STRUCT_S]], align 8

0 commit comments

Comments
 (0)