[PATCH] D80384: [SVE] Add flag to specify SVE register size, using this to calculate legal vector types.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 17 12:56:35 PDT 2020


efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

Okay, it sounds like we're on the same page.  LGTM



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:3500
+  if (VT.getVectorElementType() == MVT::i1)
+    return false;
+
----------------
paulwalker-arm wrote:
> efriedma wrote:
> > paulwalker-arm wrote:
> > > efriedma wrote:
> > > > Restricting i1 types like this seems a little weird to me.  We don't need to pretend to be NEON. We want these types to be legal for similar reasons we want the other fixed vector types to be legal.  I guess we could leave the check for now if it makes it easier to get simple testcases working, but I don't think this is what we want long-term.
> > > > 
> > > > For non-i1 types, should we check the element type is legal?  integer_fixedlen_vector_valuetypes() and fp_fixedlen_vector_valuetypes() can return some exotic types.
> > > Part of this is me not looking for trouble at this stage and wanting to wait to see if there's a performance reason to make them legal.  I think the majority of the cases can be handled after the conversion to scalable vector types, which just leaves the cross-block scenarios.
> > > 
> > > However, from a functional point of view I'm trying not to make any ABI related changes. Currently the handling of i1 based vectors (well all fixed length vectors) has already been decided for NEON and so I cannot just change it in isolation without breaking compatibility with existing libraries (which don't know about SVE).
> > > 
> > > In the future we'll need a function attribute that corresponds to an as yet undefined ABI. When that is in place we can relax the i1 restriction.  For now though my focus is purely on block level code generation.
> > > 
> > We can mess with the ABI separately if we need to say the values are sign-extended or whatever.
> > 
> > I'm concerned that if we don't make this legal now, we're going to end up writing a bunch of code to work around the fact that we can't produce i1 vectors after legalization.
> Do you have any specific examples.  I ask because my experience is the opposite. By keeping fixed length i1 vectors as illegal I am able to remove a lot of work that just isn't necessary to achieve my objective of supporting larger fixed length vectors.
> 
> I'll admit that here I'm potentially trading a bit on performance but there's nothing binding here, it's just the order of implementation I prefer.
> 
> None of this affects scalable vectors so it doesn't affect the post operation legalisation side of things.
Say we have something like `(a==b|c==d)? e : f`.  In NEON, we do something with blending in vector registers.  In SVE, we do not want the condition to ever be moved from predicate registers to vector registers.  So now you're pattern-matching some combination of logic operators and blends to try to recover the obvious SVE code.

If you aren't really doing that sort of optimization in the first iteration, transitioning later might not be a big deal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80384





More information about the llvm-commits mailing list