sdesmalen added inline comments.
================ Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:7670 + Mask, Op.getOperand(0), Op.getOperand(1)); +} + ---------------- efriedma wrote: > 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. I agree that using INTRINSIC_WO_CHAIN will be a bit annoying for more complicated patterns. The reuse of the intrinsics was merely for convenience because we already have the patterns, and was not a critical part of the design. It shouldn't be a big effort to create AArch64ISD nodes and use these for the intrinsics as well. If we use AArch64-specific nodes we can implement what's needed now for SVE codegen and I expect we can easily migrate to target-independent nodes when they get added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78569/new/ https://reviews.llvm.org/D78569 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
