[PATCH] D65884: [ARM] MVE Tail Predication

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Target/ARM/MVETailPredication.cpp:364
+  // Search for Elems in the following SCEV:
+  // (1 + ((-VF + (VF * (((VF - 1) + %Elems) /u VF))<nuw>) /u VF))<nuw><nsw>
+  const SCEV *Elems = nullptr;
----------------
samparker wrote:
> SjoerdMeijer wrote:
> > samparker wrote:
> > > SjoerdMeijer wrote:
> > > > What we are doing here, is looking at generated code to get `%Elems`. But the vectorizer has generated this code, so somewhere this knowledge is present. The only question is, I guess, how easily accessible. Can we query SCEV or something else to avoid this pattern matching?
> > > I've had a play with some other parts of SCEV, the helpers for delinearization, to see if it could answer the size of the array accessed by the loop. I can't seem to get this to work with the GEPs in the tests as they're not be represented by clean AddRecExprs. I presume it's not unreasonable for SCEV to not be that useful //after// vectorization..? It does looks like it could be used though, but I don't think I have the knowledge to do it safely! I'll add a TODO / FIXME.
> > Alright, fair enough.
> > Last question then, last crazy idea, could this information be attached to the loop as metadata?
> That doesn't seem to be the way metadata is used. It would require it to be kept up-to-date by any transform, which is unreasonable. Loop properties are provided by analysis passes, so this could live in one of them... but it seems a bit niche and should probably be done in a better way :)
Okay, that's also fair enough.
I really cannot admit I am a big fan of this pattern matching here, which is a bit of understatement, but I guess it is what it is for now. I.e., your TODO captures it well for me. The pattern matching is extremely 'focused', and looking into SCEV and friends could be a project on itself...and so while that is considered, this is okay'ish.

I will now continue reviewing the remaining bits I haven't looked at.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65884/new/

https://reviews.llvm.org/D65884