[PATCH] D36113: [Loop Vectorize] Vectorize Loops with Backward Dependence

[DIVYA SHANMUGHAN via Phabricator via llvm-commits]

DIVYA added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:131
+bool LoopInstructionsReordering::checkDepAndReorder(StoreInst *StInst,
+                                                    Instruction *LdInst) {
+  std::queue<Instruction *> InstQueue;
----------------
hfinkel wrote:
> Is what you're doing here sufficient? What happens if you have memory-access instructions , function calls, etc. that need to be moved because of (potential) memory dependencies?
If there are any memory-access instructions, or function calls , on which the load instruction depends, then the MemorySSA  analysis will find that out, and the control will never reach this function.



================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:187
+      if (MSSA->isLiveOnEntryDef(D))
+        Reordered |= checkDepAndReorder(St, Ld);
+      else if (Instruction *DefInst = dyn_cast<Instruction>(D)) {
----------------
hfinkel wrote:
> I'm somewhat concerned that a number of these cases are actually handling loop-invariant values, meaning that we're doing a suboptimal handling of something that LICM should handle more fully. The problem is that dealing with these after vectorization could be more difficult than before, and if we have a phase-ordering problem such that we're missing these cases, we might just end up with suboptimal code. Are you seeing these cases in practice?
> 

I think the loop invariant codes will be already hoisted outside the loop before this pass.The Defining access will be liveOnEntry for  the loads  and stores from pointer arguments .
For example in the below code,  the defining access for the load Instruction for a[i+1], is Live on Entry .However the load instruction is not loop invariant.

For the case
int  foo1(int n,int * restrict a, int * restrict b, int *restrict m){
  int i;
  for (i = 0; i < n; i++){
    a[i] = b[i];
    m[i] = a[i+1];
  }
}




================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:200
+          if (DefPhi) {
+            if (OI.dominates(St, Ld))
+              Reordered |= checkDepAndReorder(St, Ld);
----------------
hfinkel wrote:
> Are you assuming here that the Phi must be the loop backedge? What if there's control flow in the loop (note that the vectorizer can do if conversion).
Even if the Phi is not from loop backedge , rather from control flow, still if the store Instruction dominates the load, the load can be moved above the store Instruction.Since the store Instruction remains below the defining access(which is memory phi)


https://reviews.llvm.org/D36113