diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp index 4c05f42fe0e..c83ba24018f 100644 --- a/lib/Transforms/Scalar/LICM.cpp +++ b/lib/Transforms/Scalar/LICM.cpp @@ -904,23 +904,24 @@ bool llvm::promoteLoopAccessesToScalars( // is not safe, because *P may only be valid to access if 'c' is true. // // The safety property divides into two parts: - // 1) The memory may not be dereferenceable on entry to the loop. In this + // p1) The memory may not be dereferenceable on entry to the loop. In this // case, we can't insert the required load in the preheader. - // 2) The memory model does not allow us to insert a store along any dynamic + // p2) The memory model does not allow us to insert a store along any dynamic // path which did not originally have one. // - // It is safe to promote P if all uses are direct load/stores and if at - // least one is guaranteed to be executed. - bool GuaranteedToExecute = false; - - // It is also safe to promote P if we can prove that speculating a load into - // the preheader is safe (i.e. proving dereferenceability on all - // paths through the loop), and that the memory can be proven thread local - // (so that the memory model requirement doesn't apply.) We first establish - // the former, and then run a capture analysis below to establish the later. - // We can use any access within the alias set to prove dereferenceability + // If at least one store is guaranteed to execute, both properties are + // satisfied, and promotion is legal. + // This, however, is not a necessary condition. Even if no store/load is + // guaranteed to execute, we can still establish these properties: + // (p1) by proving that hoisting the load into the preheader is + // safe (i.e. proving dereferenceability on all paths through the loop). We + // can use any access within the alias set to prove dereferenceability, // since they're all must alias. - bool CanSpeculateLoad = false; + // (p2) by proving the memory is thread-local, so the memory model + // requirement does not apply, and stores are safe to insert. + + bool DereferenceableInPH = false; + bool SafeToInsertStore = false; SmallVector LoopUses; SmallPtrSet PointerMustAliases; @@ -969,8 +970,8 @@ bool llvm::promoteLoopAccessesToScalars( if (!Load->isSimple()) return Changed; - if (!GuaranteedToExecute && !CanSpeculateLoad) - CanSpeculateLoad = isSafeToExecuteUnconditionally( + if (!DereferenceableInPH) + DereferenceableInPH = isSafeToExecuteUnconditionally( *Load, DT, CurLoop, SafetyInfo, Preheader->getTerminator()); } else if (const StoreInst *Store = dyn_cast(UI)) { // Stores *of* the pointer are not interesting, only stores *to* the @@ -981,28 +982,29 @@ bool llvm::promoteLoopAccessesToScalars( if (!Store->isSimple()) return Changed; - // Note that we only check GuaranteedToExecute inside the store case - // so that we do not introduce stores where they did not exist before - // (which would break the LLVM concurrency model). - - // If the alignment of this instruction allows us to specify a more - // restrictive (and performant) alignment and if we are sure this - // instruction will be executed, update the alignment. - // Larger is better, with the exception of 0 being the best alignment. + // If the store is guaranteed to execute, both properties are satisfied. + // We may want to check if a store is guaranteed to execute even if we + // already know that promotion is safe, since it may have higher + // alignment than any other guaranteed stores, in which case we can + // raise the alignment on the promoted store. unsigned InstAlignment = Store->getAlignment(); - if ((InstAlignment > Alignment || InstAlignment == 0) && - Alignment != 0) { + if (!InstAlignment) + InstAlignment = + MDL.getABITypeAlignment(Store->getValueOperand()->getType()); + + if (!DereferenceableInPH || !SafeToInsertStore || + (InstAlignment > Alignment)) { if (isGuaranteedToExecute(*UI, DT, CurLoop, SafetyInfo)) { - GuaranteedToExecute = true; - Alignment = InstAlignment; + DereferenceableInPH = true; + SafeToInsertStore = true; + Alignment = std::max(Alignment, InstAlignment); } - } else if (!GuaranteedToExecute) { - GuaranteedToExecute = - isGuaranteedToExecute(*UI, DT, CurLoop, SafetyInfo); } - if (!GuaranteedToExecute && !CanSpeculateLoad) { - CanSpeculateLoad = isDereferenceableAndAlignedPointer( + // If the store is not guaranteed to execute, we may still get + // deref info through it. + if (!DereferenceableInPH) { + DereferenceableInPH = isDereferenceableAndAlignedPointer( Store->getPointerOperand(), Store->getAlignment(), MDL, Preheader->getTerminator(), DT); } @@ -1021,18 +1023,23 @@ bool llvm::promoteLoopAccessesToScalars( } } - // Check legality per comment above. Otherwise, we can't promote. - bool PromotionIsLegal = GuaranteedToExecute; - if (!PromotionIsLegal && CanSpeculateLoad) { - // If this is a thread local location, then we can insert stores along - // paths which originally didn't have them without violating the memory - // model. + + // If we couldn't prove we can hoist the load, bail. + if (!DereferenceableInPH) + return Changed; + + // We know we can hoist the load, but don't have a guaranteed store. + // Check whether the location is thread-local. If it is, then we can insert + // stores along paths which originally didn't have them without violating the + // memory model. + if (!SafeToInsertStore) { Value *Object = GetUnderlyingObject(SomePtr, MDL); - PromotionIsLegal = + SafeToInsertStore = isAllocLikeFn(Object, TLI) && !PointerMayBeCaptured(Object, true, true); } - if (!PromotionIsLegal) + // If we've still failed to prove we can sink the store, give up. + if (!SafeToInsertStore) return Changed; // Otherwise, this is safe to promote, lets do it!