[llvm] r273703 - Revert "InstCombine rule to fold trunc when value available"
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 24 11:42:58 PDT 2016
Author: rnk
Date: Fri Jun 24 13:42:58 2016
New Revision: 273703
URL: http://llvm.org/viewvc/llvm-project?rev=273703&view=rev
Log:
Revert "InstCombine rule to fold trunc when value available"
This reverts commit r273608.
Broke building code with sanitizers, where apparently these kinds of
loads, casts, and truncations are common:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24502
http://crbug.com/623099
Modified:
llvm/trunk/include/llvm/Analysis/Loads.h
llvm/trunk/lib/Analysis/Loads.cpp
llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
llvm/trunk/test/Transforms/InstCombine/trunc.ll
Modified: llvm/trunk/include/llvm/Analysis/Loads.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Loads.h?rev=273703&r1=273702&r2=273703&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/Loads.h (original)
+++ llvm/trunk/include/llvm/Analysis/Loads.h Fri Jun 24 13:42:58 2016
@@ -61,38 +61,6 @@ bool isSafeToLoadUnconditionally(Value *
/// to scan in the block, used by FindAvailableLoadedValue().
extern cl::opt<unsigned> DefMaxInstsToScan;
-/// Scan the ScanBB block backwards checking to see if we have the value at
-/// the memory address \p Ptr of type \p AccessTy locally available within a
-/// small number of instructions. If the value is available, return it.
-///
-/// If not, return the iterator for the last validated instruction that the
-/// value would be live through. If we scanned the entire block and didn't
-/// find something that invalidates *Ptr or provides it, ScanFrom would be
-/// left at begin() and this returns null. ScanFrom could also be left
-///
-/// MaxInstsToScan specifies the maximum instructions to scan in the block.
-/// If it is set to 0, it will scan the whole block. You can also optionally
-/// specify an alias analysis implementation, which makes this more precise.
-///
-/// If AATags is non-null and a load or store is found, the AA tags from the
-/// load or store are recorded there. If there are no AA tags or if no access
-/// is found, it is left unmodified.
-///
-/// IsAtomicMemOp specifies the atomicity of the memory operation that accesses
-/// \p *Ptr. We verify atomicity constraints are satisfied when value forwarding
-/// from another memory operation that has value \p *Ptr available.
-///
-/// Note that we assume the \p *Ptr is accessed through a non-volatile but
-/// potentially atomic load. Any other constraints should be verified at the
-/// caller.
-Value *FindAvailableLoadedValue(Value *Ptr, Type *AccessTy, bool IsAtomicMemOp,
- BasicBlock *ScanBB,
- BasicBlock::iterator &ScanFrom,
- unsigned MaxInstsToScan,
- AliasAnalysis *AA = nullptr,
- AAMDNodes *AATags = nullptr,
- bool *IsLoadCSE = nullptr);
-
/// FindAvailableLoadedValue - Scan the ScanBB block backwards (starting at
/// the instruction before ScanFrom) checking to see if we have the value at
/// the memory address *Ptr locally available within a small number of
Modified: llvm/trunk/lib/Analysis/Loads.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Loads.cpp?rev=273703&r1=273702&r2=273703&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/Loads.cpp (original)
+++ llvm/trunk/lib/Analysis/Loads.cpp Fri Jun 24 13:42:58 2016
@@ -300,9 +300,8 @@ llvm::DefMaxInstsToScan("available-load-
"to scan backward from a given instruction, when searching for "
"available loaded value"));
-/// Scan the ScanBB block backwards checking to see if we have the value at
-/// the memory address \p Ptr of type \p AccessTy locally available within a
-/// small number of instructions. If the value is available, return it.
+/// \brief Scan the ScanBB block backwards to see if we have the value at the
+/// memory address *Ptr locally available within a small number of instructions.
///
/// The scan starts from \c ScanFrom. \c MaxInstsToScan specifies the maximum
/// instructions to scan in the block. If it is set to \c 0, it will scan the whole
@@ -319,25 +318,26 @@ llvm::DefMaxInstsToScan("available-load-
///
/// If \c AATags is non-null and a load or store is found, the AA tags from the
/// load or store are recorded there. If there are no AA tags or if no access is
-/// is found, it is left unmodified.
-///
-/// IsAtomicMemOp specifies the atomicity of the memory operation that accesses
-/// \p *Ptr. We verify atomicity constraints are satisfied when value forwarding
-/// from another memory operation that has value \p *Ptr available.
-///
-/// Note that we assume the \p *Ptr is accessed through a non-volatile but
-/// potentially atomic load. Any other constraints should be verified at the
-/// caller.
-Value *llvm::FindAvailableLoadedValue(Value *Ptr, Type *AccessTy, bool IsAtomicMemOp,
- BasicBlock *ScanBB,
+/// found, it is left unmodified.
+Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan,
AliasAnalysis *AA, AAMDNodes *AATags,
bool *IsLoadCSE) {
-
if (MaxInstsToScan == 0)
MaxInstsToScan = ~0U;
+ Value *Ptr = Load->getPointerOperand();
+ Type *AccessTy = Load->getType();
+
+ // We can never remove a volatile load
+ if (Load->isVolatile())
+ return nullptr;
+
+ // Anything stronger than unordered is currently unimplemented.
+ if (!Load->isUnordered())
+ return nullptr;
+
const DataLayout &DL = ScanBB->getModule()->getDataLayout();
// Try to get the store size for the type.
@@ -363,14 +363,14 @@ Value *llvm::FindAvailableLoadedValue(Va
// If this is a load of Ptr, the loaded value is available.
// (This is true even if the load is volatile or atomic, although
// those cases are unlikely.)
- if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
- Value *LoadPtr = LI->getPointerOperand()->stripPointerCasts();
- if (AreEquivalentAddressValues(LoadPtr, StrippedPtr) &&
+ if (LoadInst *LI = dyn_cast<LoadInst>(Inst))
+ if (AreEquivalentAddressValues(
+ LI->getPointerOperand()->stripPointerCasts(), StrippedPtr) &&
CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {
// We can value forward from an atomic to a non-atomic, but not the
// other way around.
- if (LI->isAtomic() < IsAtomicMemOp)
+ if (LI->isAtomic() < Load->isAtomic())
return nullptr;
if (AATags)
@@ -380,8 +380,6 @@ Value *llvm::FindAvailableLoadedValue(Va
return LI;
}
- }
-
if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
Value *StorePtr = SI->getPointerOperand()->stripPointerCasts();
// If this is a store through Ptr, the value is available!
@@ -393,7 +391,7 @@ Value *llvm::FindAvailableLoadedValue(Va
// We can value forward from an atomic to a non-atomic, but not the
// other way around.
- if (SI->isAtomic() < IsAtomicMemOp)
+ if (SI->isAtomic() < Load->isAtomic())
return nullptr;
if (AATags)
@@ -436,44 +434,4 @@ Value *llvm::FindAvailableLoadedValue(Va
// Got to the start of the block, we didn't find it, but are done for this
// block.
return nullptr;
-
-}
-
-/// \brief Scan the ScanBB block backwards to see if we have the value at the
-/// memory address *Ptr locally available within a small number of instructions.
-///
-/// The scan starts from \c ScanFrom. \c MaxInstsToScan specifies the maximum
-/// instructions to scan in the block. If it is set to \c 0, it will scan the whole
-/// block.
-///
-/// If the value is available, this function returns it. If not, it returns the
-/// iterator for the last validated instruction that the value would be live
-/// through. If we scanned the entire block and didn't find something that
-/// invalidates \c *Ptr or provides it, \c ScanFrom is left at the last
-/// instruction processed and this returns null.
-///
-/// You can also optionally specify an alias analysis implementation, which
-/// makes this more precise.
-///
-/// If \c AATags is non-null and a load or store is found, the AA tags from the
-/// load or store are recorded there. If there are no AA tags or if no access is
-/// is found, it is left unmodified.
-Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,
- BasicBlock::iterator &ScanFrom,
- unsigned MaxInstsToScan,
- AliasAnalysis *AA, AAMDNodes *AATags,
- bool *IsLoadCSE) {
-
- // We can never remove a volatile load
- if (Load->isVolatile())
- return nullptr;
-
- // Anything stronger than unordered is currently unimplemented.
- if (!Load->isUnordered())
- return nullptr;
-
- // Return the full value of the load if available.
- return FindAvailableLoadedValue(Load->getPointerOperand(), Load->getType(),
- Load->isAtomic(), ScanBB, ScanFrom,
- MaxInstsToScan, AA, AATags, IsLoadCSE);
}
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=273703&r1=273702&r2=273703&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Fri Jun 24 13:42:58 2016
@@ -14,10 +14,9 @@
#include "InstCombineInternal.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/Analysis/ConstantFolding.h"
-#include "llvm/Analysis/Loads.h"
-#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/PatternMatch.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
using namespace llvm;
using namespace PatternMatch;
@@ -577,24 +576,6 @@ Instruction *InstCombiner::visitTrunc(Tr
if (Instruction *I = foldVecTruncToExtElt(CI, *this, DL))
return I;
- // When trunc operand is a widened load, see if we can get the value from a
- // previous store/load
- if (auto *LI = dyn_cast<LoadInst>(Src)) {
- BasicBlock::iterator BBI(*LI);
-
- // Scan a few instructions up from LI and if we find a partial load/store
- // of Type DestTy that feeds into LI, we can replace all uses of the trunc
- // with the load/store value.
- // This replacement can be done only in the case of non-volatile loads. If
- // the load is atomic, its only use should be the trunc instruction. We
- // don't want to allow other users of LI to see a value that is out of sync
- // with the value we're folding the trunc to (in case of a race).
- if (!LI->isVolatile() && (!LI->isAtomic() || LI->hasOneUse()))
- if (Value *AvailableVal = FindAvailableLoadedValue(
- LI->getPointerOperand(), DestTy, LI->isAtomic(), LI->getParent(),
- BBI, DefMaxInstsToScan))
- return replaceInstUsesWith(CI, AvailableVal);
- }
return nullptr;
}
Modified: llvm/trunk/test/Transforms/InstCombine/trunc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/trunc.ll?rev=273703&r1=273702&r2=273703&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/trunc.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/trunc.ll Fri Jun 24 13:42:58 2016
@@ -181,60 +181,3 @@ bb1:
bb2:
unreachable
}
-
-declare void @consume(i8) readonly
-define i1 @trunc_load_store(i8* align 2 %a) {
- store i8 0, i8 *%a, align 2
- %bca = bitcast i8* %a to i16*
- %wide.load = load i16, i16* %bca, align 2
- %lowhalf.1 = trunc i16 %wide.load to i8
- call void @consume(i8 %lowhalf.1)
- %cmp.2 = icmp ult i16 %wide.load, 256
- ret i1 %cmp.2
-; CHECK-LABEL: @trunc_load_store
-; CHECK-NOT: trunc
-; CHECK: call void @consume(i8 0)
-}
-
-
-; The trunc can be replaced with the load value.
-define i1 @trunc_load_load(i8* align 2 %a) {
- %pload = load i8, i8* %a, align 2
- %bca = bitcast i8* %a to i16*
- %wide.load = load i16, i16* %bca, align 2
- %lowhalf = trunc i16 %wide.load to i8
- call void @consume(i8 %lowhalf)
- call void @consume(i8 %pload)
- %cmp.2 = icmp ult i16 %wide.load, 256
- ret i1 %cmp.2
-; CHECK-LABEL: @trunc_load_load
-; CHECK-NOT: trunc
-}
-
-; trunc should not be replaced since atomic load %wide.load has more than one use.
-; different values can be seen by the uses of %wide.load in case of race.
-define i1 @trunc_atomic_loads(i8* align 2 %a) {
- %pload = load atomic i8, i8* %a unordered, align 2
- %bca = bitcast i8* %a to i16*
- %wide.load = load atomic i16, i16* %bca unordered, align 2
- %lowhalf = trunc i16 %wide.load to i8
- call void @consume(i8 %lowhalf)
- call void @consume(i8 %pload)
- %cmp.2 = icmp ult i16 %wide.load, 256
- ret i1 %cmp.2
-; CHECK-LABEL: @trunc_atomic_loads
-; CHECK: trunc
-}
-
-; trunc cannot be replaced since store size is not trunc result size
-define i1 @trunc_different_size_load(i16 * align 2 %a) {
- store i16 0, i16 *%a, align 2
- %bca = bitcast i16* %a to i32*
- %wide.load = load i32, i32* %bca, align 2
- %lowhalf = trunc i32 %wide.load to i8
- call void @consume(i8 %lowhalf)
- %cmp.2 = icmp ult i32 %wide.load, 256
- ret i1 %cmp.2
-; CHECK-LABEL: @trunc_different_size_load
-; CHECK: %lowhalf = trunc i32 %wide.load to i8
-}
More information about the llvm-commits
mailing list