[PATCH] D32422: LoopVectorizer: let target prefer scalar addressing computations (+ minor improvements in SystemZTTI)

[Elena Demikhovsky via Phabricator via llvm-commits]

delena added a comment.

In general, I think that one-level-up scalarization is good for all targets. Anyway, attempt to scalarize the Ptr should be done if the taken decision is "Scalarize".



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7301
+
+  // Start with all scalar pointer uses.
+  SmallPtrSet<Instruction *, 8> AddrDefs;
----------------
But interleave loads are also here. You don't need to scalarize their pointers.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7305
+    for (Instruction &I : *BB) {
+      Instruction *PtrDef = nullptr;
+      if (auto *LoadI = dyn_cast<LoadInst>(&I))
----------------
You will find getPointerOperand(Inst) utility in this file. It retrieves a pointer from load and store.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7322
+      if (auto *InstOp = dyn_cast<Instruction>(Op))
+        if (TheLoop->contains(InstOp) && !isa<PHINode>(InstOp) &&
+            AddrDefs.insert(InstOp).second == true)
----------------
In your examples you show only one level up. I don't believe that we have too many real cases of multiple address redirections ,  but theoretically, it may not be so profitable if you go many levels up.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7322
+      if (auto *InstOp = dyn_cast<Instruction>(Op))
+        if (TheLoop->contains(InstOp) && !isa<PHINode>(InstOp) &&
+            AddrDefs.insert(InstOp).second == true)
----------------
delena wrote:
> In your examples you show only one level up. I don't believe that we have too many real cases of multiple address redirections ,  but theoretically, it may not be so profitable if you go many levels up.
The scalarized intsruction should belong to the same  Basic block and , probably, have only one user.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7331
+        // Scalarize a load of address
+        setWideningDecision(I, VF, CM_Scalarize,
+                            (VF * getMemoryInstructionCost(I, 1)));
----------------
At this point you change widening decision of already analyzed Load inst. Make sure that this inst is not a part of interleave group.


https://reviews.llvm.org/D32422