Index: llvm/include/llvm/IR/InstrTypes.h =================================================================== --- llvm/include/llvm/IR/InstrTypes.h +++ llvm/include/llvm/IR/InstrTypes.h @@ -1848,7 +1848,14 @@ bool isNoInline() const { return hasFnAttr(Attribute::NoInline); } void setIsNoInline() { addFnAttr(Attribute::NoInline); } /// Determine if the call does not access memory. - bool doesNotAccessMemory() const { return hasFnAttr(Attribute::ReadNone); } + bool doesNotAccessMemory() const { + return hasFnAttr(Attribute::ReadNone) && + // If the call lives in presplit coroutine, we couldn't assume the + // call wouldn't access memory even if it has readnone attribute. + // Since readnone could be used for thread identification and + // coroutines might resume in different threads. + (!getFunction() || !getFunction()->isPresplitCoroutine()); + } void setDoesNotAccessMemory() { addFnAttr(Attribute::ReadNone); } /// Determine if the call does not access or only reads memory. @@ -1860,7 +1867,10 @@ /// Determine if the call does not access or only writes memory. bool onlyWritesMemory() const { - return hasImpliedFnAttr(Attribute::WriteOnly); + return hasImpliedFnAttr(Attribute::WriteOnly) && + // See the comments in doesNotAccessMemory. Due to readnone implies + // writeonly. + (!getFunction() || !getFunction()->isPresplitCoroutine()); } void setOnlyWritesMemory() { addFnAttr(Attribute::WriteOnly); } Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp =================================================================== --- llvm/lib/Analysis/BasicAliasAnalysis.cpp +++ llvm/lib/Analysis/BasicAliasAnalysis.cpp @@ -771,6 +771,14 @@ // Can't do better than this. return FMRB_DoesNotAccessMemory; + // readnone meant to not access memory. However, it was used accidently for + // thread identification. It was fine before we introduced coroutines. Since + // coroutine may resume in different threads. So we decide to treat readnone + // attribute in presplit coroutine as readonly. + if (Call->hasFnAttr(Attribute::ReadNone) && + Call->getFunction()->isPresplitCoroutine()) + return FMRB_OnlyReadsMemory; + FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior; // If the callsite knows it only reads memory, don't return worse Index: llvm/lib/Analysis/GlobalsModRef.cpp =================================================================== --- llvm/lib/Analysis/GlobalsModRef.cpp +++ llvm/lib/Analysis/GlobalsModRef.cpp @@ -263,7 +263,11 @@ if (!Call->hasOperandBundles()) if (const Function *F = Call->getCalledFunction()) if (FunctionInfo *FI = getFunctionInfo(F)) { - if (!isModOrRefSet(FI->getModRefInfo())) + if (!isModOrRefSet(FI->getModRefInfo()) && + // When the call lives in presplit coroutine, we couldn't assume it + // to not access memory even if the callee is marked as readnone. + // Since a coroutine might resume in different threads. + !Call->getFunction()->isPresplitCoroutine()) Min = FMRB_DoesNotAccessMemory; else if (!isModSet(FI->getModRefInfo())) Min = FMRB_OnlyReadsMemory; Index: llvm/test/Transforms/Coroutines/coro-readnone-01.ll =================================================================== --- /dev/null +++ llvm/test/Transforms/Coroutines/coro-readnone-01.ll @@ -0,0 +1,89 @@ +; Tests that the readnone function which cross suspend points wouldn't be misoptimized. +; RUN: opt < %s -S -passes='default' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_SPLITTED +; RUN: opt < %s -S -passes='early-cse' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_UNSPLITTED +; RUN: opt < %s -S -passes='gvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_UNSPLITTED +; RUN: opt < %s -S -passes='newgvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_UNSPLITTED + +define ptr @f() "coroutine.presplit" { +entry: + %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null) + %size = call i32 @llvm.coro.size.i32() + %alloc = call i8* @malloc(i32 %size) + %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc) + %j = call i32 @readnone_func() readnone + %sus_result = call i8 @llvm.coro.suspend(token none, i1 false) + switch i8 %sus_result, label %suspend [i8 0, label %resume + i8 1, label %cleanup] +resume: + %i = call i32 @readnone_func() readnone + %cmp = icmp eq i32 %i, %j + br i1 %cmp, label %same, label %diff + +same: + call void @print_same() + br label %cleanup + +diff: + call void @print_diff() + br label %cleanup + +cleanup: + %mem = call i8* @llvm.coro.free(token %id, i8* %hdl) + call void @free(i8* %mem) + br label %suspend + +suspend: + call i1 @llvm.coro.end(i8* %hdl, i1 0) + ret i8* %hdl +} + +; Tests that normal functions wouldn't be affected. +define i1 @normal_function() { +entry: + %i = call i32 @readnone_func() readnone + %j = call i32 @readnone_func() readnone + %cmp = icmp eq i32 %i, %j + br i1 %cmp, label %same, label %diff + +same: + call void @print_same() + ret i1 true + +diff: + call void @print_diff() + ret i1 false +} + +; CHECK_SPLITTED-LABEL: normal_function( +; CHECK_SPLITTED-NEXT: entry +; CHECK_SPLITTED-NEXT: call i32 @readnone_func() +; CHECK_SPLITTED-NEXT: call void @print_same() +; CHECK_SPLITTED-NEXT: ret i1 true +; +; CHECK_SPLITTED-LABEL: f.resume( +; CHECK_UNSPLITTED-LABEL: @f( +; CHECK: br i1 %cmp, label %same, label %diff +; CHECK-EMPTY: +; CHECK-NEXT: same: +; CHECK-NEXT: call void @print_same() +; CHECK-NEXT: br label +; CHECK-EMPTY: +; CHECK-NEXT: diff: +; CHECK-NEXT: call void @print_diff() +; CHECK-NEXT: br label + +declare i32 @readnone_func() readnone + +declare void @print_same() +declare void @print_diff() +declare i8* @llvm.coro.free(token, i8*) +declare i32 @llvm.coro.size.i32() +declare i8 @llvm.coro.suspend(token, i1) + +declare token @llvm.coro.id(i32, i8*, i8*, i8*) +declare i1 @llvm.coro.alloc(token) +declare i8* @llvm.coro.begin(token, i8*) +declare i1 @llvm.coro.end(i8*, i1) + +declare noalias i8* @malloc(i32) +declare void @free(i8*) Index: llvm/test/Transforms/Coroutines/coro-readnone-02.ll =================================================================== --- /dev/null +++ llvm/test/Transforms/Coroutines/coro-readnone-02.ll @@ -0,0 +1,81 @@ +; Tests that the readnone function which don't cross suspend points could be optimized expectly after split. +; +; RUN: opt < %s -S -passes='default' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_SPLITTED +; RUN: opt < %s -S -passes='coro-split,early-cse,simplifycfg' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_SPLITTED +; RUN: opt < %s -S -passes='coro-split,gvn,simplifycfg' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_SPLITTED +; RUN: opt < %s -S -passes='coro-split,newgvn,simplifycfg' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_SPLITTED +; RUN: opt < %s -S -passes='early-cse' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_UNSPLITTED +; RUN: opt < %s -S -passes='gvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_UNSPLITTED +; RUN: opt < %s -S -passes='newgvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK_UNSPLITTED + +define ptr @f() "coroutine.presplit" { +entry: + %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null) + %size = call i32 @llvm.coro.size.i32() + %alloc = call i8* @malloc(i32 %size) + %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc) + %sus_result = call i8 @llvm.coro.suspend(token none, i1 false) + switch i8 %sus_result, label %suspend [i8 0, label %resume + i8 1, label %cleanup] +resume: + %i = call i32 @readnone_func() readnone + ; noop call to break optimization to combine two consecutive readonly calls. + call void @nop() + %j = call i32 @readnone_func() readnone + %cmp = icmp eq i32 %i, %j + br i1 %cmp, label %same, label %diff + +same: + call void @print_same() + br label %cleanup + +diff: + call void @print_diff() + br label %cleanup + +cleanup: + %mem = call i8* @llvm.coro.free(token %id, i8* %hdl) + call void @free(i8* %mem) + br label %suspend + +suspend: + call i1 @llvm.coro.end(i8* %hdl, i1 0) + ret i8* %hdl +} + +; +; CHECK_SPLITTED-LABEL: f.resume( +; CHECK_SPLITTED-NEXT: : +; CHECK_SPLITTED-NEXT: call i32 @readnone_func() #[[ATTR_NUM:[0-9]+]] +; CHECK_SPLITTED-NEXT: call void @nop() +; CHECK_SPLITTED-NEXT: call void @print_same() +; +; CHECK_SPLITTED: attributes #[[ATTR_NUM]] = { readnone } +; +; CHECK_UNSPLITTED-LABEL: @f( +; CHECK_UNSPLITTED: br i1 %cmp, label %same, label %diff +; CHECK_UNSPLITTED-EMPTY: +; CHECK_UNSPLITTED-NEXT: same: +; CHECK_UNSPLITTED-NEXT: call void @print_same() +; CHECK_UNSPLITTED-NEXT: br label +; CHECK_UNSPLITTED-EMPTY: +; CHECK_UNSPLITTED-NEXT: diff: +; CHECK_UNSPLITTED-NEXT: call void @print_diff() +; CHECK_UNSPLITTED-NEXT: br label + +declare i32 @readnone_func() readnone +declare void @nop() + +declare void @print_same() +declare void @print_diff() +declare i8* @llvm.coro.free(token, i8*) +declare i32 @llvm.coro.size.i32() +declare i8 @llvm.coro.suspend(token, i1) + +declare token @llvm.coro.id(i32, i8*, i8*, i8*) +declare i1 @llvm.coro.alloc(token) +declare i8* @llvm.coro.begin(token, i8*) +declare i1 @llvm.coro.end(i8*, i1) + +declare noalias i8* @malloc(i32) +declare void @free(i8*) Index: llvm/unittests/Analysis/AliasAnalysisTest.cpp =================================================================== --- llvm/unittests/Analysis/AliasAnalysisTest.cpp +++ llvm/unittests/Analysis/AliasAnalysisTest.cpp @@ -366,6 +366,33 @@ EXPECT_EQ(AR, AliasResult::PartialAlias); EXPECT_EQ(-1, AR.getOffset()); } + +TEST_F(AliasAnalysisTest, AAInCoroutines) { + LLVMContext C; + SMDiagnostic Err; + std::unique_ptr M = parseAssemblyString(R"( + define void @f() "coroutine.presplit" { + entry: + %ReadNoneCall = call i32 @readnone_func() readnone + ret void + } + + declare i32 @readnone_func() readnone + )", + Err, C); + + ASSERT_TRUE(M); + Function *F = M->getFunction("f"); + CallInst *ReadNoneCall = + cast(getInstructionByName(*F, "ReadNoneCall")); + + auto &AA = getAAResults(*F); + EXPECT_FALSE(AA.doesNotAccessMemory(ReadNoneCall)); + EXPECT_TRUE(AA.onlyReadsMemory(ReadNoneCall)); + + EXPECT_EQ(FMRB_OnlyReadsMemory, AA.getModRefBehavior(ReadNoneCall)); +} + class AAPassInfraTest : public testing::Test { protected: LLVMContext C; Index: llvm/unittests/Analysis/GlobalsModRefTest.cpp =================================================================== --- llvm/unittests/Analysis/GlobalsModRefTest.cpp +++ llvm/unittests/Analysis/GlobalsModRefTest.cpp @@ -11,6 +11,8 @@ #include "llvm/Analysis/CallGraph.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/AsmParser/Parser.h" +#include "llvm/IR/InstIterator.h" +#include "llvm/IR/Instructions.h" #include "llvm/Support/SourceMgr.h" #include "gtest/gtest.h" @@ -56,3 +58,40 @@ EXPECT_EQ(FMRB_DoesNotAccessMemory, AAR.getModRefBehavior(&F2)); EXPECT_EQ(FMRB_OnlyReadsMemory, AAR.getModRefBehavior(&F3)); } + +static Instruction *getInstructionByName(Function &F, StringRef Name) { + for (auto &I : instructions(F)) + if (I.getName() == Name) + return &I; + llvm_unreachable("Expected to find instruction!"); +} + +TEST(GlobalsModRef, ReadNoneInCoroutines) { + LLVMContext C; + SMDiagnostic Err; + std::unique_ptr M = parseAssemblyString(R"( + define void @f() "coroutine.presplit" { + entry: + %ReadNoneCall = call i32 @readnone_func() readnone + ret void + } + + declare i32 @readnone_func() readnone + )", + Err, C); + + ASSERT_TRUE(M); + Function *F = M->getFunction("f"); + + Triple Trip(M->getTargetTriple()); + TargetLibraryInfoImpl TLII(Trip); + TargetLibraryInfo TLI(TLII); + auto GetTLI = [&TLI](Function &F) -> TargetLibraryInfo & { return TLI; }; + llvm::CallGraph CG(*M); + + auto AAR = GlobalsAAResult::analyzeModule(*M, GetTLI, CG); + CallInst *ReadNoneCall = + cast(getInstructionByName(*F, "ReadNoneCall")); + + EXPECT_EQ(FMRB_OnlyReadsMemory, AAR.getModRefBehavior(ReadNoneCall)); +} Index: llvm/unittests/IR/InstructionsTest.cpp =================================================================== --- llvm/unittests/IR/InstructionsTest.cpp +++ llvm/unittests/IR/InstructionsTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/IR/Instructions.h" +#include "llvm-c/Core.h" #include "llvm/ADT/CombinationGenerator.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Analysis/ValueTracking.h" @@ -20,13 +21,13 @@ #include "llvm/IR/FPEnv.h" #include "llvm/IR/Function.h" #include "llvm/IR/IRBuilder.h" +#include "llvm/IR/InstIterator.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/MDBuilder.h" #include "llvm/IR/Module.h" #include "llvm/IR/NoFolder.h" #include "llvm/IR/Operator.h" #include "llvm/Support/SourceMgr.h" -#include "llvm-c/Core.h" #include "gmock/gmock-matchers.h" #include "gtest/gtest.h" #include @@ -1695,5 +1696,40 @@ EXPECT_EQ(H.getAllocationSizeInBits(DL), TypeSize::getFixed(160)); } +static Instruction *getInstructionByName(Function &F, StringRef Name) { + for (auto &I : instructions(F)) + if (I.getName() == Name) + return &I; + llvm_unreachable("Expected to find instruction!"); +} + +TEST(InstructionsTest, CallInstInPresplitCoroutine) { + LLVMContext Ctx; + std::unique_ptr M = parseIR(Ctx, R"( + define void @f() "coroutine.presplit" { + entry: + %ReadNoneCall = call i32 @readnone_func() readnone + %WriteOnlyCall = call i32 @writeonly_func() writeonly + ret void + } + + declare i32 @readnone_func() readnone + declare i32 @writeonly_func() writeonly + )"); + + ASSERT_TRUE(M); + Function *F = M->getFunction("f"); + CallInst *ReadNoneCall = + cast(getInstructionByName(*F, "ReadNoneCall")); + CallInst *WriteOnlyCall = + cast(getInstructionByName(*F, "WriteOnlyCall")); + + EXPECT_FALSE(ReadNoneCall->doesNotAccessMemory()); + EXPECT_FALSE(ReadNoneCall->onlyWritesMemory()); + EXPECT_TRUE(ReadNoneCall->onlyReadsMemory()); + + EXPECT_FALSE(WriteOnlyCall->onlyWritesMemory()); +} + } // end anonymous namespace } // end namespace llvm