Index: clang/test/CodeGen/arm-cmse-attr.c =================================================================== --- clang/test/CodeGen/arm-cmse-attr.c +++ clang/test/CodeGen/arm-cmse-attr.c @@ -29,9 +29,9 @@ { } -// CHECK: define{{.*}} void @f1(void ()* nocapture %fptr) {{[^#]*}}#0 { +// CHECK: define{{.*}} void @f1(void ()* nocapture readonly %fptr) {{[^#]*}}#0 { // CHECK: call void %fptr() #2 -// CHECK: define{{.*}} void @f2(void ()* nocapture %fptr) {{[^#]*}}#0 { +// CHECK: define{{.*}} void @f2(void ()* nocapture readonly %fptr) {{[^#]*}}#0 { // CHECK: call void %fptr() #2 // CHECK: define{{.*}} void @f3() {{[^#]*}}#1 { // CHECK: define{{.*}} void @f4() {{[^#]*}}#1 { Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp =================================================================== --- llvm/lib/Transforms/IPO/FunctionAttrs.cpp +++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp @@ -707,62 +707,56 @@ continue; } - Function *F = CB.getCalledFunction(); - if (!F) { - if (CB.onlyReadsMemory()) { - IsRead = true; - AddUsersToWorklistIfCapturing(); - continue; - } else if (!CB.isCallee(U) && CB.hasFnAttr(Attribute::WriteOnly)) { - IsWrite = true; - AddUsersToWorklistIfCapturing(); - continue; - } - return Attribute::None; + if (CB.isCallee(U)) { + // Indirect call of an argument is a reading, noncapturing use + IsRead = true; + Captures = false; + AddUsersToWorklistIfCapturing(); + continue; } + // U cannot be the callee operand use as we explicitly checked for that. // Note: the callee and the two successor blocks *follow* the argument // operands. This means there is no need to adjust UseIndex to account // for these. - - unsigned UseIndex = std::distance(CB.arg_begin(), U); - - // U cannot be the callee operand use: since we're exploring the - // transitive uses of an Argument, having such a use be a callee would - // imply the call site is an indirect call or invoke; and we'd take the - // early exit above. + const unsigned UseIndex = std::distance(CB.arg_begin(), U); assert(UseIndex < CB.data_operands_size() && "Data operand use expected!"); - bool IsOperandBundleUse = UseIndex >= CB.arg_size(); - - if (UseIndex >= F->arg_size() && !IsOperandBundleUse) { - assert(F->isVarArg() && "More params than args in non-varargs call"); - return Attribute::None; - } - Captures &= !CB.doesNotCapture(UseIndex); - // Since the optimizer (by design) cannot see the data flow corresponding - // to a operand bundle use, these cannot participate in the optimistic SCC - // analysis. Instead, we model the operand bundle uses as arguments in - // call to a function external to the SCC. - if (IsOperandBundleUse || - !SCCNodes.count(&*std::next(F->arg_begin(), UseIndex))) { - - // The accessors used on call site here do the right thing for calls and - // invokes with operand bundles. - - if (CB.doesNotAccessMemory(UseIndex)) { /* nop */ } - else if (CB.onlyReadsMemory() || CB.onlyReadsMemory(UseIndex)) - IsRead = true; - else if (CB.hasFnAttr(Attribute::WriteOnly) || - CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) - IsWrite = true; - else - return Attribute::None; + auto canSpeculate = [&]() { + if (UseIndex >= CB.arg_size()) + // Since the optimizer (by design) cannot see the data flow + // corresponding to a operand bundle use, these cannot participate in + // the optimistic SCC analysis. Instead, we model the operand bundle + // uses as arguments in call to a function external to the SCC. + return false; + auto *F = CB.getCalledFunction(); + if (!F) + return false; + if (UseIndex >= F->arg_size()) { + assert(F->isVarArg()); + return false; + } + Argument *A = &*std::next(F->arg_begin(), UseIndex); + return 0 != SCCNodes.count(A); + }; + if (canSpeculate()) { + // Speculatively ignore this use for now. + AddUsersToWorklistIfCapturing(); + continue; } + if (CB.doesNotAccessMemory(UseIndex)) { /* nop */ } + else if (CB.onlyReadsMemory() || CB.onlyReadsMemory(UseIndex)) + IsRead = true; + else if (CB.hasFnAttr(Attribute::WriteOnly) || + CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) + IsWrite = true; + else + return Attribute::None; + AddUsersToWorklistIfCapturing(); break; } Index: llvm/test/Transforms/FunctionAttrs/nocapture.ll =================================================================== --- llvm/test/Transforms/FunctionAttrs/nocapture.ll +++ llvm/test/Transforms/FunctionAttrs/nocapture.ll @@ -128,7 +128,7 @@ } -; FNATTR: define void @nc3(void ()* nocapture %p) +; FNATTR: define void @nc3(void ()* nocapture readonly %p) define void @nc3(void ()* %p) { call void %p() ret void @@ -141,7 +141,7 @@ ret void } -; FNATTR: define void @nc5(void (i8*)* nocapture %f, i8* nocapture %p) +; FNATTR: define void @nc5(void (i8*)* nocapture readonly %f, i8* nocapture %p) define void @nc5(void (i8*)* %f, i8* %p) { call void %f(i8* %p) readonly nounwind call void %f(i8* nocapture %p) Index: llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll =================================================================== --- llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll +++ llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll @@ -1,14 +1,8 @@ ; RUN: opt -function-attrs -S < %s | FileCheck %s ; RUN: opt -passes=function-attrs -S < %s | FileCheck %s -; This checks for an iterator wraparound bug in FunctionAttrs. The previous -; "incorrect" behavior was inferring readonly for the %x argument in @caller. -; Inferring readonly for %x *is* actually correct, since @va_func is marked -; readonly, but FunctionAttrs was inferring readonly for the wrong reasons (and -; we _need_ the readonly on @va_func to trigger the problematic code path). It -; is possible that in the future FunctionAttrs becomes smart enough to infer -; readonly for %x for the right reasons, and at that point this test will have -; to be marked invalid. +; This checks for a previously existing iterator wraparound bug in +; FunctionAttrs, and in the process covers corner cases with varargs. declare void @llvm.va_start(i8*) declare void @llvm.va_end(i8*) @@ -24,8 +18,26 @@ } define i32 @caller(i32* %x) { -; CHECK-LABEL: define i32 @caller(i32* nocapture %x) +; CHECK-LABEL: define i32 @caller(i32* nocapture readonly %x) entry: call void(i32*,...) @va_func(i32* null, i32 0, i32 0, i32 0, i32* %x) ret i32 42 } + +define void @va_func2(i32* readonly %b, ...) { +; CHECK-LABEL: define void @va_func2(i32* nocapture readonly %b, ...) + entry: + %valist = alloca i8 + call void @llvm.va_start(i8* %valist) + call void @llvm.va_end(i8* %valist) + %x = call i32 @caller(i32* %b) + ret void +} + +define i32 @caller2(i32* %x, i32* %y) { +; CHECK-LABEL: define i32 @caller2(i32* nocapture readonly %x, i32* %y) + entry: + call void(i32*,...) @va_func2(i32* %x, i32 0, i32 0, i32 0, i32* %y) + ret i32 42 +} + Index: llvm/test/Transforms/FunctionAttrs/writeonly.ll =================================================================== --- llvm/test/Transforms/FunctionAttrs/writeonly.ll +++ llvm/test/Transforms/FunctionAttrs/writeonly.ll @@ -83,19 +83,19 @@ ret void } -; CHECK: define void @fptr_test1(i8* %p, void (i8*)* nocapture %f) +; CHECK: define void @fptr_test1(i8* %p, void (i8*)* nocapture readonly %f) define void @fptr_test1(i8* %p, void (i8*)* %f) { call void %f(i8* %p) ret void } -; CHECK: define void @fptr_test2(i8* %p, void (i8*)* nocapture %f) +; CHECK: define void @fptr_test2(i8* writeonly %p, void (i8*)* nocapture readonly %f) define void @fptr_test2(i8* %p, void (i8*)* %f) { call void %f(i8* writeonly %p) ret void } -; CHECK: define void @fptr_test3(i8* writeonly %p, void (i8*)* nocapture %f) +; CHECK: define void @fptr_test3(i8* writeonly %p, void (i8*)* nocapture readonly %f) define void @fptr_test3(i8* %p, void (i8*)* %f) { call void %f(i8* %p) writeonly ret void