Index: lib/Transforms/Scalar/LICM.cpp =================================================================== --- lib/Transforms/Scalar/LICM.cpp +++ lib/Transforms/Scalar/LICM.cpp @@ -687,35 +687,56 @@ if (match(CI, m_Intrinsic())) // Assumes don't actually alias anything or throw return true; - + // Handle simple cases by querying alias analysis. FunctionModRefBehavior Behavior = AA->getModRefBehavior(CI); if (Behavior == FMRB_DoesNotAccessMemory) return true; - if (AliasAnalysis::onlyReadsMemory(Behavior)) { - // A readonly argmemonly function only reads from memory pointed to by - // it's arguments with arbitrary offsets. If we can prove there are no - // writes to this memory in the loop, we can hoist or sink. - if (AliasAnalysis::onlyAccessesArgPointees(Behavior)) { - // TODO: expand to writeable arguments - for (Value *Op : CI->arg_operands()) - if (Op->getType()->isPointerTy() && - pointerInvalidatedByLoop( - MemoryLocation(Op, MemoryLocation::UnknownSize, AAMDNodes()), - CurAST, CurLoop, AA)) + if (AliasAnalysis::onlyAccessesArgPointees(Behavior)) { + CallSite CS(CI); + ModRefInfo CallMask = createModRefInfo(Behavior); + + // Some intrinsics are marked as modifying memory for control flow + // modelling purposes, but don't actually modify any specific memory + // location. + using namespace PatternMatch; + if (CI->use_empty() && + match(CI, m_Intrinsic())) + CallMask = clearMod(CallMask); + + for (auto AI = CS.arg_begin(), AE = CS.arg_end(); AI != AE; ++AI) { + const Value *Arg = *AI; + if (!Arg->getType()->isPointerTy()) + continue; + unsigned ArgIdx = std::distance(CS.arg_begin(), AI); + MemoryLocation ArgLoc = MemoryLocation::getForArgument(CS, ArgIdx, + nullptr); + ModRefInfo ArgMask = AA->getArgModRefInfo(CS, ArgIdx); + ArgMask = intersectModRef(CallMask, ArgMask); + if (isRefSet(ArgMask) && + pointerInvalidatedByLoop(ArgLoc, CurAST, CurLoop, AA)) + return false; + if (isModSet(ArgMask)) { + auto &AS = CurAST->getAliasSetFor(ArgLoc); + if (AS.isRef() || !AS.isMustAlias()) + // Quick exit test, handled by the full path below as well. return false; - return true; + auto *UniqueI = AS.getUniqueInstruction(); + if (!UniqueI) + // other memory op, give up + return false; + assert(UniqueI == CI && "AS must contain SI"); + // falthrough, success + } } - - // If this call only reads from memory and there are no writes to memory - // in the loop, we can hoist or sink the call as appropriate. - if (isReadOnly(CurAST)) - return true; + return true; } - - // FIXME: This should use mod/ref information to see if we can hoist or - // sink the call. - + + // If this call only reads from memory and there are no writes to memory + // in the loop, we can hoist or sink the call as appropriate. + if (AliasAnalysis::onlyReadsMemory(Behavior) && isReadOnly(CurAST)) + return true; + // TODO: Handle a WriteOnly call analogously to a fence return false; } else if (auto *FI = dyn_cast(&I)) { // Fences alias (most) everything to provide ordering. For the moment,