[PATCH] D68414: [SROA] Enhance AggLoadStoreRewriter to rewrite integer load/store if it covers multi fields in original aggregate
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 22 23:04:42 PDT 2020
efriedma added a comment.
> For LLVM testsuite function presplitOverlappedSlices returns true for 114 times, I think most of them are from basictest.ll.
I mean the LLVM testsuite, not the regression tests. (http://llvm.org/docs/TestSuiteGuide.html). I'm trying to get an idea how wide the impact is on general C/C++ code. I'm not expecting this to trigger very frequently, but I want to make sure my intuition is correct.
================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4164
+ if (!S1.isSplittable())
+ continue;
+ for (Slice &S2 : P) {
----------------
Carrot wrote:
> efriedma wrote:
> > Is it possible that we don't end up actually splitting a partition after we pre-split? Could we try harder to avoid that?
> This function only presplits slices, it doesn't split partitions. The partitions are only constructed on demand when we request an iterator from AllocaSlices like:
> for (auto &P : AS.partitions())
I wasn't trying to ask about the partitions datastructure.
I meant, are there cases where we do the transform, but then SROA can't do any further transforms? So instead of a single load, we have two loads and some bit manipulation that doesn't simplify? I'd like to avoid splitting the load/store instruction if it''s not actually going to help.
================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4141
+// Limit the number of times presplitOverlappedSlices is called.
+#define MAX_PRESPLIT_ITERATIONS 128
+
----------------
`static const int MaxPresplitIterations = 128;`
================
Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:4356
+
+ // Remove the killed slices that have ben pre-split.
+ AS.erase(llvm::remove_if(AS, [](const Slice &S) { return S.isDead(); }),
----------------
"been"
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68414/new/
https://reviews.llvm.org/D68414
More information about the llvm-commits
mailing list