[PATCH] D42759: [CGP] Split large data structres to sink more GEPs

Haicheng Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 27 11:01:55 PDT 2018


haicheng added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:3757
           return true;
+      } else if (EnableGEPOffsetSplit && isa<GetElementPtrInst>(AddrInst) &&
+                 TLI.shouldConsiderGEPOffsetSplit() && Depth == 0 &&
----------------
skatkov wrote:
> Why do you need the check isa<GetElementPtrInst>(AddrInst)?
> You are in "case Instruction::GetElementPtr" basing on "switch (Opcode) {" where Opcode is actually AddrInst->getOpcode().
> So to me it is redundant...
AddrInst is a user.  It can be a GetElementPtrConstantExpr which is not supported by CGP yet.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4873
+                   compareGEPOffset);
+    LargeOffsetGEPs.erase(
+        std::unique(LargeOffsetGEPs.begin(), LargeOffsetGEPs.end()),
----------------
skatkov wrote:
> I wonder if you sort the an array anyway why not to use set like data-structure to keep them and avoid erase of unique elements?
> Is there any hidden sense for that?
I need a data structure that can be iterated in a sorted order (ascending by offsets) and all elements (pointers to GEPs) are unique.  I think the data in the set are unique but I cannot access them in the order that I want If I use a set.


Repository:
  rL LLVM

https://reviews.llvm.org/D42759





More information about the llvm-commits mailing list