From 582d46328ce644d791f4dce31b005fc260d33611 Mon Sep 17 00:00:00 2001 From: Nirav Dave Date: Tue, 26 Feb 2019 15:02:32 +0000 Subject: [PATCH] [DAG] Fix constant store folding to handle non-byte sizes. Avoid crashes from zero-byte values due to sub-byte store sizes. Reviewers: uabelho, courbet, rnk Reviewed By: courbet Subscribers: hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D58626 llvm-svn: 354884 --- llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h | 15 ++++++++------- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp | 16 ++++++++-------- .../CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp | 10 ++++++---- llvm/test/CodeGen/X86/constant-combines.ll | 12 ++++++++++++ 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h b/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h index e934370..deff6dd 100644 --- a/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h +++ b/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h @@ -62,15 +62,16 @@ public: // Returns true if `Other` (with size `OtherSize`) can be proven to be fully // contained in `*this` (with size `Size`). - bool contains(int64_t Size, const BaseIndexOffset &Other, int64_t OtherSize, - const SelectionDAG &DAG) const { - int64_t Offset; - return contains(Size, Other, OtherSize, DAG, Offset); + bool contains(const SelectionDAG &DAG, int64_t BitSize, + const BaseIndexOffset &Other, int64_t OtherBitSize, + int64_t &BitOffset) const; + + bool contains(const SelectionDAG &DAG, int64_t BitSize, + const BaseIndexOffset &Other, int64_t OtherBitSize) const { + int64_t BitOffset; + return contains(DAG, BitSize, Other, OtherBitSize, BitOffset); } - bool contains(int64_t Size, const BaseIndexOffset &Other, int64_t OtherSize, - const SelectionDAG &DAG, int64_t &Offset) const; - // Returns true `BasePtr0` and `BasePtr1` can be proven to alias/not alias, in // which case `IsAlias` is set to true/false. static bool computeAliasing(const BaseIndexOffset &BasePtr0, diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp index 183a1f5..18fe03f 100644 --- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -15427,13 +15427,13 @@ SDValue DAGCombiner::visitSTORE(SDNode *N) { !ST1->getBasePtr().isUndef()) { const BaseIndexOffset STBase = BaseIndexOffset::match(ST, DAG); const BaseIndexOffset ChainBase = BaseIndexOffset::match(ST1, DAG); - unsigned STByteSize = ST->getMemoryVT().getSizeInBits() / 8; - unsigned ChainByteSize = ST1->getMemoryVT().getSizeInBits() / 8; + unsigned STBitSize = ST->getMemoryVT().getSizeInBits(); + unsigned ChainBitSize = ST1->getMemoryVT().getSizeInBits(); // If this is a store who's preceding store to a subset of the current // location and no one other node is chained to that store we can // effectively drop the store. Do not remove stores to undef as they may // be used as data sinks. - if (STBase.contains(STByteSize, ChainBase, ChainByteSize, DAG)) { + if (STBase.contains(DAG, STBitSize, ChainBase, ChainBitSize)) { CombineTo(ST1, ST1->getChain()); return SDValue(); } @@ -15442,17 +15442,17 @@ SDValue DAGCombiner::visitSTORE(SDNode *N) { // able to fold ST's value into the preceding stored value. As we know // the other uses of ST1's chain are unconcerned with ST, this folding // will not affect those nodes. - int64_t Offset; - if (ChainBase.contains(ChainByteSize, STBase, STByteSize, DAG, - Offset)) { + int64_t BitOffset; + if (ChainBase.contains(DAG, ChainBitSize, STBase, STBitSize, + BitOffset)) { SDValue ChainValue = ST1->getValue(); if (auto *C1 = dyn_cast(ChainValue)) { if (auto *C = dyn_cast(Value)) { APInt Val = C1->getAPIntValue(); - APInt InsertVal = C->getAPIntValue().zextOrTrunc(STByteSize * 8); + APInt InsertVal = C->getAPIntValue().zextOrTrunc(STBitSize); // FIXME: Handle Big-endian mode. if (!DAG.getDataLayout().isBigEndian()) { - Val.insertBits(InsertVal, Offset * 8); + Val.insertBits(InsertVal, BitOffset); SDValue NewSDVal = DAG.getConstant(Val, SDLoc(C), ChainValue.getValueType(), C1->isTargetOpcode(), C1->isOpaque()); diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp index b0dcf47..f5adfa5 100644 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp @@ -135,9 +135,10 @@ bool BaseIndexOffset::computeAliasing(const BaseIndexOffset &BasePtr0, return false; // Cannot determine whether the pointers alias. } -bool BaseIndexOffset::contains(int64_t Size, const BaseIndexOffset &Other, - int64_t OtherSize, const SelectionDAG &DAG, - int64_t &Offset) const { +bool BaseIndexOffset::contains(const SelectionDAG &DAG, int64_t BitSize, + const BaseIndexOffset &Other, + int64_t OtherBitSize, int64_t &BitOffset) const { + int64_t Offset; if (!equalBaseIndex(Other, DAG, Offset)) return false; if (Offset >= 0) { @@ -145,7 +146,8 @@ bool BaseIndexOffset::contains(int64_t Size, const BaseIndexOffset &Other, // [-------*this---------] // [---Other--] // ==Offset==> - return Offset + OtherSize <= Size; + BitOffset = 8 * Offset; + return BitOffset + OtherBitSize <= BitSize; } // Other starts strictly before *this, it cannot be fully contained. // [-------*this---------] diff --git a/llvm/test/CodeGen/X86/constant-combines.ll b/llvm/test/CodeGen/X86/constant-combines.ll index 8574168..f3d2df6 100644 --- a/llvm/test/CodeGen/X86/constant-combines.ll +++ b/llvm/test/CodeGen/X86/constant-combines.ll @@ -38,3 +38,15 @@ entry: store float %8, float* %0, align 4 ret void } + + +define void @bitstore_fold() { +; CHECK-LABEL: bitstore_fold: +; CHECK: # %bb.0: # %BB +; CHECK-NEXT: movl $-2, 0 +; CHECK-NEXT: retq +BB: + store i32 -1, i32* null + store i1 false, i1* null + ret void +} -- 2.7.4