[llvm] r254950 - [EarlyCSE] Simplify and invert ParseMemoryInst [NFCI]
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 7 13:40:58 PST 2015
Cancel that. I made a silly mistake. I was only building x86. Will
revert and then investigate and reapply.
Philip
On 12/07/2015 01:35 PM, Philip Reames via llvm-commits wrote:
> Not on my platform. Can you copy the error message so I can
> investigate? Or point to a buildbot?
>
> On 12/07/2015 01:34 PM, Rafael Espíndola wrote:
>> Transforms/EarlyCSE/AArch64/intrinsics.ll is now faling
>>
>> On 7 December 2015 at 16:27, Philip Reames via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Author: reames
>>> Date: Mon Dec 7 15:27:15 2015
>>> New Revision: 254950
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=254950&view=rev
>>> Log:
>>> [EarlyCSE] Simplify and invert ParseMemoryInst [NFCI]
>>>
>>> Restructure ParseMemoryInst - which was introduced to abstract over
>>> target specific load and stores instructions - to just query the
>>> underlying instructions. In theory, this could be slightly slower
>>> than caching the results, but in practice, it's very unlikely to be
>>> measurable.
>>>
>>> The simple query scheme makes it far easier to understand, and much
>>> easier to extend with new queries. Given I'm about to need to add
>>> new query types, doing the cleanup first seemed worthwhile.
>>>
>>> Do we still believe the target specific intrinsic handling is
>>> worthwhile in EarlyCSE? It adds quite a bit of complexity and makes
>>> the code harder to read. Being able to delete the abstraction
>>> entirely would be wonderful.
>>>
>>>
>>>
>>> Modified:
>>> llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp
>>>
>>> Modified: llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp?rev=254950&r1=254949&r2=254950&view=diff
>>> ==============================================================================
>>>
>>> --- llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/Scalar/EarlyCSE.cpp Mon Dec 7
>>> 15:27:15 2015
>>> @@ -388,57 +388,58 @@ private:
>>> class ParseMemoryInst {
>>> public:
>>> ParseMemoryInst(Instruction *Inst, const TargetTransformInfo
>>> &TTI)
>>> - : Load(false), Store(false), IsSimple(true),
>>> MayReadFromMemory(false),
>>> - MayWriteToMemory(false), MatchingId(-1), Ptr(nullptr) {
>>> - MayReadFromMemory = Inst->mayReadFromMemory();
>>> - MayWriteToMemory = Inst->mayWriteToMemory();
>>> - if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
>>> - MemIntrinsicInfo Info;
>>> - if (!TTI.getTgtMemIntrinsic(II, Info))
>>> - return;
>>> - if (Info.NumMemRefs == 1) {
>>> - Store = Info.WriteMem;
>>> - Load = Info.ReadMem;
>>> - MatchingId = Info.MatchingId;
>>> - MayReadFromMemory = Info.ReadMem;
>>> - MayWriteToMemory = Info.WriteMem;
>>> - IsSimple = Info.IsSimple;
>>> - Ptr = Info.PtrVal;
>>> - }
>>> - } else if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>>> - Load = true;
>>> - IsSimple = LI->isSimple();
>>> - Ptr = LI->getPointerOperand();
>>> + : IsTargetMemInst(false), Inst(Inst) {
>>> + if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst))
>>> + if (TTI.getTgtMemIntrinsic(II, Info) && Info.NumMemRefs == 1)
>>> + IsTargetMemInst = true;
>>> + }
>>> + bool isLoad() const {
>>> + if (IsTargetMemInst) return Info.ReadMem;
>>> + return isa<LoadInst>(Inst);
>>> + }
>>> + bool isStore() const {
>>> + if (IsTargetMemInst) return Info.WriteMem;
>>> + return isa<StoreInst>(Inst);
>>> + }
>>> + bool isSimple() const {
>>> + if (IsTargetMemInst) return Info.IsSimple;
>>> + if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>>> + return LI->isSimple();
>>> } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>>> - Store = true;
>>> - IsSimple = SI->isSimple();
>>> - Ptr = SI->getPointerOperand();
>>> + return SI->isSimple();
>>> }
>>> + return Inst->isAtomic();
>>> }
>>> - bool isLoad() const { return Load; }
>>> - bool isStore() const { return Store; }
>>> - bool isSimple() const { return IsSimple; }
>>> bool isMatchingMemLoc(const ParseMemoryInst &Inst) const {
>>> - return Ptr == Inst.Ptr && MatchingId == Inst.MatchingId;
>>> + return (getPointerOperand() == Inst.getPointerOperand() &&
>>> + getMatchingId() == Inst.getMatchingId());
>>> }
>>> - bool isValid() const { return Ptr != nullptr; }
>>> - int getMatchingId() const { return MatchingId; }
>>> - Value *getPtr() const { return Ptr; }
>>> - bool mayReadFromMemory() const { return MayReadFromMemory; }
>>> - bool mayWriteToMemory() const { return MayWriteToMemory; }
>>> + bool isValid() const { return getPointerOperand() != nullptr; }
>>>
>>> - private:
>>> - bool Load;
>>> - bool Store;
>>> - bool IsSimple;
>>> - bool MayReadFromMemory;
>>> - bool MayWriteToMemory;
>>> // For regular (non-intrinsic) loads/stores, this is set to
>>> -1. For
>>> // intrinsic loads/stores, the id is retrieved from the
>>> corresponding
>>> // field in the MemIntrinsicInfo structure. That field contains
>>> // non-negative values only.
>>> - int MatchingId;
>>> - Value *Ptr;
>>> + int getMatchingId() const {
>>> + if (IsTargetMemInst) return Info.MatchingId;
>>> + return -1;
>>> + }
>>> + Value *getPointerOperand() const {
>>> + if (IsTargetMemInst) return Info.PtrVal;
>>> + if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
>>> + return LI->getPointerOperand();
>>> + } else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
>>> + return SI->getPointerOperand();
>>> + }
>>> + return nullptr;
>>> + }
>>> + bool mayReadFromMemory() const { return
>>> Inst->mayReadFromMemory(); }
>>> + bool mayWriteToMemory() const { return Inst->mayWriteToMemory(); }
>>> +
>>> + private:
>>> + bool IsTargetMemInst;
>>> + MemIntrinsicInfo Info;
>>> + Instruction *Inst;
>>> };
>>>
>>> bool processNode(DomTreeNode *Node);
>>> @@ -565,7 +566,7 @@ bool EarlyCSE::processNode(DomTreeNode *
>>>
>>> // If we have an available version of this load, and if it
>>> is the right
>>> // generation, replace this instruction.
>>> - LoadValue InVal = AvailableLoads.lookup(MemInst.getPtr());
>>> + LoadValue InVal =
>>> AvailableLoads.lookup(MemInst.getPointerOperand());
>>> if (InVal.Data != nullptr && InVal.Generation ==
>>> CurrentGeneration &&
>>> InVal.MatchingId == MemInst.getMatchingId()) {
>>> Value *Op = getOrCreateResult(InVal.Data, Inst->getType());
>>> @@ -583,7 +584,7 @@ bool EarlyCSE::processNode(DomTreeNode *
>>>
>>> // Otherwise, remember that we have this instruction.
>>> AvailableLoads.insert(
>>> - MemInst.getPtr(),
>>> + MemInst.getPointerOperand(),
>>> LoadValue(Inst, CurrentGeneration,
>>> MemInst.getMatchingId()));
>>> LastStore = nullptr;
>>> continue;
>>> @@ -659,7 +660,7 @@ bool EarlyCSE::processNode(DomTreeNode *
>>> // to non-volatile loads, so we don't have to check for
>>> volatility of
>>> // the store.
>>> AvailableLoads.insert(
>>> - MemInst.getPtr(),
>>> + MemInst.getPointerOperand(),
>>> LoadValue(Inst, CurrentGeneration,
>>> MemInst.getMatchingId()));
>>>
>>> // Remember that this was the last normal store we saw for
>>> DSE.
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list