[PATCH] D88962: [SVE] Add support for scalable vectors in vectorize_width loop attribute

Thu Nov 5 08:59:44 PST 2020

sdesmalen added inline comments.

================
Comment at: llvm/test/Transforms/LoopVectorize/metadata-width.ll:55
+!3 = !{!"llvm.loop.vectorize.width", !4}
+!4 = !{i32 8, i32 0}
----------------
david-arm wrote:
> fhahn wrote:
> > david-arm wrote:
> > > david-arm wrote:
> > > > sdesmalen wrote:
> > > > > nit: `s/i32 0/i1 false/`
> > > > If I make this change the test fails. We will still always print out "i32 0". Is there a way to control the format generated?
> > > Sorry I just realised this is an input to the test, rather than testing the output! Please ignore my comment.
> > also needs a test which actually sets `isScalable` to `true`?
> Unfortunately I can't do that at the moment because the vectorizer crashes when trying to perform scalable vectorisation. This is why I only tested the 'false' case. In theory I could create a negative test that tests the vectoriser crashes, which we could remove later once we support vectorisation. Any thoughts @fhahn or @sdesmalen ?
The proof of concept for scalable loop vectorization (D90343) depends on this patch and shows the `isScalable = true` case does something sensible with this information. Hopefully that is sufficient for now while we work on actually implementing scalable auto-vec support in the LoopVectorizer.

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

https://reviews.llvm.org/D88962