[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

Wed Oct 23 06:44:31 PDT 2019

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

In D68877#1718729 <https://reviews.llvm.org/D68877#1718729>, @kmclaughlin wrote:

> There is not yet support for vector selects, so for this patch the intention was that any passthru which is not all zero or undef would result in a selection failure.
>  Do you think it would acceptable to handle different passthrus in a future patch which also implements vector selects for SVE?

Yeah OK. That makes sense. I remember the same chicken and egg from MVE. We ended up doing them the other way around there. Can you try and add a FIXME comment somewhere?

Otherwise LGTM.

================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:4753
+  let hasSideEffects = 1, hasNoSchedulingInfo = 1, mayLoad = 1 in {
+  def "" : Pseudo<(outs listty:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, simm4s1:$imm4), []>,
+           PseudoInstExpansion<(!cast<Instruction>(NAME # _REAL) listty:$Zt, PPR3bAny:$Pg, GPR64sp:$Rn, simm4s1:$imm4)>;
----------------
kmclaughlin wrote:
> dmgreen wrote:
> > Can you explain why is this pseudo is needed, exactly? I feel that using pseudos is often the wrong solution to a problem (it may be required here, I'm just sure why exactly).
> > 
> > We currently seem to generate ld1b (for example), over ldnf1b. Is there ever a time that we expect to generate a nf load?
> The pseudo was a workaround that was added downstream for non-faulting loads, but it is not needed here.
Righteo. We know where to find it if it turns out we do need them.

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

https://reviews.llvm.org/D68877