<div dir="ltr">The RAUW you added doesn't insert the bitcast you would need to do this transform. This ended up crashing while building Chromium:<div><a href="https://bugs.chromium.org/p/chromium/issues/detail?id=623099">https://bugs.chromium.org/p/chromium/issues/detail?id=623099</a></div><div><br></div><div>I will prepare a revert. I have an unreduced test case, would you like it, or do you think you can sort it out?</div><div><br></div><div>This was the bad RAUW assertion:</div><div><div>Assertion failed: New->getType() == getType() && "replaceAllUses of value with n</div><div>ew value of different type!", file ..\lib\IR\Value.cpp, line 375</div><div><br></div><div>Load being replaced:</div><div> %138 = load float, float* %m_x.i527, align 4, !dbg !447</div><div>Trunc being inserted:</div><div> %155 = trunc i64 %140 to i32, !dbg !470</div></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 23, 2016 at 1:22 PM, Anna Thomas via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: annat<br>
Date: Thu Jun 23 15:22:22 2016<br>
New Revision: 273608<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=273608&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=273608&view=rev</a><br>
Log:<br>
InstCombine rule to fold trunc when value available<br>
<br>
Summary:<br>
This instcombine rule folds away trunc operations that have value available from a prior load or store.<br>
This kind of code can be generated as a result of GVN widening the load or from source code as well.<br>
<br>
Reviewers: reames, majnemer, sanjoy<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D21246" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21246</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Analysis/Loads.h<br>
llvm/trunk/lib/Analysis/Loads.cpp<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
llvm/trunk/test/Transforms/InstCombine/trunc.ll<br>
<br>
Modified: llvm/trunk/include/llvm/Analysis/Loads.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Loads.h?rev=273608&r1=273607&r2=273608&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Loads.h?rev=273608&r1=273607&r2=273608&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Analysis/Loads.h (original)<br>
+++ llvm/trunk/include/llvm/Analysis/Loads.h Thu Jun 23 15:22:22 2016<br>
@@ -61,6 +61,38 @@ bool isSafeToLoadUnconditionally(Value *<br>
/// to scan in the block, used by FindAvailableLoadedValue().<br>
extern cl::opt<unsigned> DefMaxInstsToScan;<br>
<br>
+/// Scan the ScanBB block backwards checking to see if we have the value at<br>
+/// the memory address \p Ptr of type \p AccessTy locally available within a<br>
+/// small number of instructions. If the value is available, return it.<br>
+///<br>
+/// If not, return the iterator for the last validated instruction that the<br>
+/// value would be live through. If we scanned the entire block and didn't<br>
+/// find something that invalidates *Ptr or provides it, ScanFrom would be<br>
+/// left at begin() and this returns null. ScanFrom could also be left<br>
+///<br>
+/// MaxInstsToScan specifies the maximum instructions to scan in the block.<br>
+/// If it is set to 0, it will scan the whole block. You can also optionally<br>
+/// specify an alias analysis implementation, which makes this more precise.<br>
+///<br>
+/// If AATags is non-null and a load or store is found, the AA tags from the<br>
+/// load or store are recorded there. If there are no AA tags or if no access<br>
+/// is found, it is left unmodified.<br>
+///<br>
+/// IsAtomicMemOp specifies the atomicity of the memory operation that accesses<br>
+/// \p *Ptr. We verify atomicity constraints are satisfied when value forwarding<br>
+/// from another memory operation that has value \p *Ptr available.<br>
+///<br>
+/// Note that we assume the \p *Ptr is accessed through a non-volatile but<br>
+/// potentially atomic load. Any other constraints should be verified at the<br>
+/// caller.<br>
+Value *FindAvailableLoadedValue(Value *Ptr, Type *AccessTy, bool IsAtomicMemOp,<br>
+ BasicBlock *ScanBB,<br>
+ BasicBlock::iterator &ScanFrom,<br>
+ unsigned MaxInstsToScan,<br>
+ AliasAnalysis *AA = nullptr,<br>
+ AAMDNodes *AATags = nullptr,<br>
+ bool *IsLoadCSE = nullptr);<br>
+<br>
/// FindAvailableLoadedValue - Scan the ScanBB block backwards (starting at<br>
/// the instruction before ScanFrom) checking to see if we have the value at<br>
/// the memory address *Ptr locally available within a small number of<br>
<br>
Modified: llvm/trunk/lib/Analysis/Loads.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Loads.cpp?rev=273608&r1=273607&r2=273608&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Loads.cpp?rev=273608&r1=273607&r2=273608&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Analysis/Loads.cpp (original)<br>
+++ llvm/trunk/lib/Analysis/Loads.cpp Thu Jun 23 15:22:22 2016<br>
@@ -300,8 +300,9 @@ llvm::DefMaxInstsToScan("available-load-<br>
"to scan backward from a given instruction, when searching for "<br>
"available loaded value"));<br>
<br>
-/// \brief Scan the ScanBB block backwards to see if we have the value at the<br>
-/// memory address *Ptr locally available within a small number of instructions.<br>
+/// Scan the ScanBB block backwards checking to see if we have the value at<br>
+/// the memory address \p Ptr of type \p AccessTy locally available within a<br>
+/// small number of instructions. If the value is available, return it.<br>
///<br>
/// The scan starts from \c ScanFrom. \c MaxInstsToScan specifies the maximum<br>
/// instructions to scan in the block. If it is set to \c 0, it will scan the whole<br>
@@ -318,26 +319,25 @@ llvm::DefMaxInstsToScan("available-load-<br>
///<br>
/// If \c AATags is non-null and a load or store is found, the AA tags from the<br>
/// load or store are recorded there. If there are no AA tags or if no access is<br>
-/// found, it is left unmodified.<br>
-Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,<br>
+/// is found, it is left unmodified.<br>
+///<br>
+/// IsAtomicMemOp specifies the atomicity of the memory operation that accesses<br>
+/// \p *Ptr. We verify atomicity constraints are satisfied when value forwarding<br>
+/// from another memory operation that has value \p *Ptr available.<br>
+///<br>
+/// Note that we assume the \p *Ptr is accessed through a non-volatile but<br>
+/// potentially atomic load. Any other constraints should be verified at the<br>
+/// caller.<br>
+Value *llvm::FindAvailableLoadedValue(Value *Ptr, Type *AccessTy, bool IsAtomicMemOp,<br>
+ BasicBlock *ScanBB,<br>
BasicBlock::iterator &ScanFrom,<br>
unsigned MaxInstsToScan,<br>
AliasAnalysis *AA, AAMDNodes *AATags,<br>
bool *IsLoadCSE) {<br>
+<br>
if (MaxInstsToScan == 0)<br>
MaxInstsToScan = ~0U;<br>
<br>
- Value *Ptr = Load->getPointerOperand();<br>
- Type *AccessTy = Load->getType();<br>
-<br>
- // We can never remove a volatile load<br>
- if (Load->isVolatile())<br>
- return nullptr;<br>
-<br>
- // Anything stronger than unordered is currently unimplemented.<br>
- if (!Load->isUnordered())<br>
- return nullptr;<br>
-<br>
const DataLayout &DL = ScanBB->getModule()->getDataLayout();<br>
<br>
// Try to get the store size for the type.<br>
@@ -363,14 +363,14 @@ Value *llvm::FindAvailableLoadedValue(Lo<br>
// If this is a load of Ptr, the loaded value is available.<br>
// (This is true even if the load is volatile or atomic, although<br>
// those cases are unlikely.)<br>
- if (LoadInst *LI = dyn_cast<LoadInst>(Inst))<br>
- if (AreEquivalentAddressValues(<br>
- LI->getPointerOperand()->stripPointerCasts(), StrippedPtr) &&<br>
+ if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {<br>
+ Value *LoadPtr = LI->getPointerOperand()->stripPointerCasts();<br>
+ if (AreEquivalentAddressValues(LoadPtr, StrippedPtr) &&<br>
CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)) {<br>
<br>
// We can value forward from an atomic to a non-atomic, but not the<br>
// other way around.<br>
- if (LI->isAtomic() < Load->isAtomic())<br>
+ if (LI->isAtomic() < IsAtomicMemOp)<br>
return nullptr;<br>
<br>
if (AATags)<br>
@@ -380,6 +380,8 @@ Value *llvm::FindAvailableLoadedValue(Lo<br>
return LI;<br>
}<br>
<br>
+ }<br>
+<br>
if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {<br>
Value *StorePtr = SI->getPointerOperand()->stripPointerCasts();<br>
// If this is a store through Ptr, the value is available!<br>
@@ -391,7 +393,7 @@ Value *llvm::FindAvailableLoadedValue(Lo<br>
<br>
// We can value forward from an atomic to a non-atomic, but not the<br>
// other way around.<br>
- if (SI->isAtomic() < Load->isAtomic())<br>
+ if (SI->isAtomic() < IsAtomicMemOp)<br>
return nullptr;<br>
<br>
if (AATags)<br>
@@ -434,4 +436,44 @@ Value *llvm::FindAvailableLoadedValue(Lo<br>
// Got to the start of the block, we didn't find it, but are done for this<br>
// block.<br>
return nullptr;<br>
+<br>
+}<br>
+<br>
+/// \brief Scan the ScanBB block backwards to see if we have the value at the<br>
+/// memory address *Ptr locally available within a small number of instructions.<br>
+///<br>
+/// The scan starts from \c ScanFrom. \c MaxInstsToScan specifies the maximum<br>
+/// instructions to scan in the block. If it is set to \c 0, it will scan the whole<br>
+/// block.<br>
+///<br>
+/// If the value is available, this function returns it. If not, it returns the<br>
+/// iterator for the last validated instruction that the value would be live<br>
+/// through. If we scanned the entire block and didn't find something that<br>
+/// invalidates \c *Ptr or provides it, \c ScanFrom is left at the last<br>
+/// instruction processed and this returns null.<br>
+///<br>
+/// You can also optionally specify an alias analysis implementation, which<br>
+/// makes this more precise.<br>
+///<br>
+/// If \c AATags is non-null and a load or store is found, the AA tags from the<br>
+/// load or store are recorded there. If there are no AA tags or if no access is<br>
+/// is found, it is left unmodified.<br>
+Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,<br>
+ BasicBlock::iterator &ScanFrom,<br>
+ unsigned MaxInstsToScan,<br>
+ AliasAnalysis *AA, AAMDNodes *AATags,<br>
+ bool *IsLoadCSE) {<br>
+<br>
+ // We can never remove a volatile load<br>
+ if (Load->isVolatile())<br>
+ return nullptr;<br>
+<br>
+ // Anything stronger than unordered is currently unimplemented.<br>
+ if (!Load->isUnordered())<br>
+ return nullptr;<br>
+<br>
+ // Return the full value of the load if available.<br>
+ return FindAvailableLoadedValue(Load->getPointerOperand(), Load->getType(),<br>
+ Load->isAtomic(), ScanBB, ScanFrom,<br>
+ MaxInstsToScan, AA, AATags, IsLoadCSE);<br>
}<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=273608&r1=273607&r2=273608&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=273608&r1=273607&r2=273608&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Thu Jun 23 15:22:22 2016<br>
@@ -14,9 +14,10 @@<br>
#include "InstCombineInternal.h"<br>
#include "llvm/ADT/SetVector.h"<br>
#include "llvm/Analysis/ConstantFolding.h"<br>
+#include "llvm/Analysis/Loads.h"<br>
+#include "llvm/Analysis/TargetLibraryInfo.h"<br>
#include "llvm/IR/DataLayout.h"<br>
#include "llvm/IR/PatternMatch.h"<br>
-#include "llvm/Analysis/TargetLibraryInfo.h"<br>
using namespace llvm;<br>
using namespace PatternMatch;<br>
<br>
@@ -576,6 +577,24 @@ Instruction *InstCombiner::visitTrunc(Tr<br>
if (Instruction *I = foldVecTruncToExtElt(CI, *this, DL))<br>
return I;<br>
<br>
+ // When trunc operand is a widened load, see if we can get the value from a<br>
+ // previous store/load<br>
+ if (auto *LI = dyn_cast<LoadInst>(Src)) {<br>
+ BasicBlock::iterator BBI(*LI);<br>
+<br>
+ // Scan a few instructions up from LI and if we find a partial load/store<br>
+ // of Type DestTy that feeds into LI, we can replace all uses of the trunc<br>
+ // with the load/store value.<br>
+ // This replacement can be done only in the case of non-volatile loads. If<br>
+ // the load is atomic, its only use should be the trunc instruction. We<br>
+ // don't want to allow other users of LI to see a value that is out of sync<br>
+ // with the value we're folding the trunc to (in case of a race).<br>
+ if (!LI->isVolatile() && (!LI->isAtomic() || LI->hasOneUse()))<br>
+ if (Value *AvailableVal = FindAvailableLoadedValue(<br>
+ LI->getPointerOperand(), DestTy, LI->isAtomic(), LI->getParent(),<br>
+ BBI, DefMaxInstsToScan))<br>
+ return replaceInstUsesWith(CI, AvailableVal);<br>
+ }<br>
return nullptr;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/test/Transforms/InstCombine/trunc.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/trunc.ll?rev=273608&r1=273607&r2=273608&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/trunc.ll?rev=273608&r1=273607&r2=273608&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/trunc.ll (original)<br>
+++ llvm/trunk/test/Transforms/InstCombine/trunc.ll Thu Jun 23 15:22:22 2016<br>
@@ -181,3 +181,60 @@ bb1:<br>
bb2:<br>
unreachable<br>
}<br>
+<br>
+declare void @consume(i8) readonly<br>
+define i1 @trunc_load_store(i8* align 2 %a) {<br>
+ store i8 0, i8 *%a, align 2<br>
+ %bca = bitcast i8* %a to i16*<br>
+ %wide.load = load i16, i16* %bca, align 2<br>
+ %lowhalf.1 = trunc i16 %wide.load to i8<br>
+ call void @consume(i8 %lowhalf.1)<br>
+ %cmp.2 = icmp ult i16 %wide.load, 256<br>
+ ret i1 %cmp.2<br>
+; CHECK-LABEL: @trunc_load_store<br>
+; CHECK-NOT: trunc<br>
+; CHECK: call void @consume(i8 0)<br>
+}<br>
+<br>
+<br>
+; The trunc can be replaced with the load value.<br>
+define i1 @trunc_load_load(i8* align 2 %a) {<br>
+ %pload = load i8, i8* %a, align 2<br>
+ %bca = bitcast i8* %a to i16*<br>
+ %wide.load = load i16, i16* %bca, align 2<br>
+ %lowhalf = trunc i16 %wide.load to i8<br>
+ call void @consume(i8 %lowhalf)<br>
+ call void @consume(i8 %pload)<br>
+ %cmp.2 = icmp ult i16 %wide.load, 256<br>
+ ret i1 %cmp.2<br>
+; CHECK-LABEL: @trunc_load_load<br>
+; CHECK-NOT: trunc<br>
+}<br>
+<br>
+; trunc should not be replaced since atomic load %wide.load has more than one use.<br>
+; different values can be seen by the uses of %wide.load in case of race.<br>
+define i1 @trunc_atomic_loads(i8* align 2 %a) {<br>
+ %pload = load atomic i8, i8* %a unordered, align 2<br>
+ %bca = bitcast i8* %a to i16*<br>
+ %wide.load = load atomic i16, i16* %bca unordered, align 2<br>
+ %lowhalf = trunc i16 %wide.load to i8<br>
+ call void @consume(i8 %lowhalf)<br>
+ call void @consume(i8 %pload)<br>
+ %cmp.2 = icmp ult i16 %wide.load, 256<br>
+ ret i1 %cmp.2<br>
+; CHECK-LABEL: @trunc_atomic_loads<br>
+; CHECK: trunc<br>
+}<br>
+<br>
+; trunc cannot be replaced since store size is not trunc result size<br>
+define i1 @trunc_different_size_load(i16 * align 2 %a) {<br>
+ store i16 0, i16 *%a, align 2<br>
+ %bca = bitcast i16* %a to i32*<br>
+ %wide.load = load i32, i32* %bca, align 2<br>
+ %lowhalf = trunc i32 %wide.load to i8<br>
+ call void @consume(i8 %lowhalf)<br>
+ %cmp.2 = icmp ult i32 %wide.load, 256<br>
+ ret i1 %cmp.2<br>
+; CHECK-LABEL: @trunc_different_size_load<br>
+; CHECK: %lowhalf = trunc i32 %wide.load to i8<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>