[PATCH] D100763: [LoopVectorize] Don't create unnecessary vscale intrinsic calls

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1116
+      ConstantInt::get(Step->getType(), StepVal * VF.getKnownMinValue());
+  return VF.isScalable() ? B.CreateVScale(NewStep) : NewStep;
 }
----------------
david-arm wrote:
> frasercrmck wrote:
> > frasercrmck wrote:
> > > Not that this is wrong, but would it be inappropriate to make `CreateVScale` detect a zero step and return zero?
> > Sorry I just realised that's what you've done. Do we need to make this change, then?
> It's not really necessary as the end result is the same - I was just trying to avoid the compiler doing more work for the Step=0 case, i.e. the creation of a ConstantInt, etc. I noticed there are a few places in the vectoriser where we call this function with a Step of 0 - it happens when widening a PHI instruction or vectorising induction variables.
> 
> If you prefer I can just revert this code?
I don't feel particularly strongly. I suppose my initial comment was trying to avoid repeating ourselves and duplicating logic. But I also realise that this is how compile-time regressions creep in. I don't know how impactful the creation of a zero ConstantInt is? Presumably it's cached more often than not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100763