[PATCH] D78569: [SVE][CodeGen] Lower SDIV & UDIV to SVE intrinsics

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7670
+                     Mask, Op.getOperand(0), Op.getOperand(1));
+}
+
----------------
sdesmalen wrote:
> efriedma wrote:
> > If we're going to support these operations, we might as well just add isel patterns; that's what we've been doing for other arithmetic operations.
> Just to provide a bit of context to this approach:
> 
> For unpredicated ISD nodes for which there is no predicated instruction, the predicate needs to be generated. For scalable vectors this will be a `ptrue all`, but for fixed-width vectors may take some other predicate such as VL8 for fixed `8` elements.
> 
> Rather than creating new predicated AArch64 ISD nodes for each operation such as `AArch64ISD::UDIV_PRED`, the idea is to reuse the intrinsic layer we already added to support the ACLE - which are predicated and for which we already have the patterns - and map directly onto those.
> 
> By doing the expansion in ISelLowering, the patterns stay simple and we can generalise `getPtrue` method so that it generates the right predicate for any scalable/fixed vector size as done in D71760 avoiding the need to write multiple patterns for different vector lengths.
> 
> This patch was meant as the proof of concept of that idea (as discussed in the sync-up call of Apr 2). 
Using INTRINSIC_WO_CHAIN is a little annoying; it's hard to read in DAG dumps, and it gives weird error messages if we fail in selection.  But there aren't really any other immediate downsides I can think of, vs. doing it the other way (converting the intrinsic to AArch64ISD::UDIV_PRED).

Long-term, we're going to have a target-independent ISD::UDIV_PRED.  We probably want to start using those nodes at some point, to get target-independent optimizations. Not sure if that impacts what we want to do right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78569