[PATCH] D79357: [SveEmitter] Add builtins for svdup and svindex

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:8047
+    Value *PFalse = Constant::getNullValue(PTrue->getType());
+    Value *Sel = Builder.CreateSelect(CmpNE, PTrue, PFalse);
+    return EmitSVEPredicateCast(Sel, cast<llvm::ScalableVectorType>(Ty));
----------------
sdesmalen wrote:
> efriedma wrote:
> > The select might not be the best approach. I mean, it's semantically correct, but the backend currently doesn't know how to lower a select like that.  And even if it did, it's probably better to avoid generating it for now.
> > 
> > Maybe this should use whilelo directly, since that's what the backend would eventually use anyway.
> > but the backend currently doesn't know how to lower a select like that
> Yes I noticed, that was the reason for D79356 (added as parent revision)
> 
> > And even if it did, it's probably better to avoid generating it for now.
> Is that because there are common optimisations on the select? I thought it would be safe to use the 'full vector' form of `select` where the predicate is a scalar (and both operands are splats)
Yes, the idea would just be to avoid common optimizations.  Maybe there aren't that many vector optimizations on a select with a scalar condition, though.  If it isn't blowing up, I guess it's fine.


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

https://reviews.llvm.org/D79357