[PATCH] D89798: [SVE][AArch64] Fix TypeSize warning in loop vectorization legality

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:1020
+        continue;
+      if (isDereferenceableAndAlignedInLoop(LI, TheLoop, SE, *DT))
         SafePointers.insert(LI->getPointerOperand());
----------------
peterwaller-arm wrote:
> sdesmalen wrote:
> > peterwaller-arm wrote:
> > > I'd be tempted to make this if (!foo()) continue, so that the "passing" case is in the body of the loop, thus making it straightforward to add or remove conditions from the loop.
> > nit: yet now the diff is bigger, it seems simpler to just add one extra line:
> > ```
> >       if (LI && !mustSuppressSpeculation(*LI) &&
> > +         !LI->getType()->isVectorType() &&
> >           isDereferenceableAndAlignedInLoop(LI, TheLoop, SE, *DT))```
> On this occasion he was following https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
> 
> Is there an etiquette to minimize size-of-diff?
I guess that would be covered by the introduction [0] to the guide. It explicitly says that large scale refactoring isn't wanted, but that if you're changing it anyway then update it. My thinking is that that this sort of small patch it would be legitimate to update the style in the surrounding to follow the guide with respect to the early exit recommendation.

So what do others think on balance: legitimate in this case or not?

[0] https://llvm.org/docs/CodingStandards.html#introduction


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89798