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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/include/clang/Basic/arm_sve.td:1050
 def SVCOMPACT    : SInst<"svcompact[_{d}]",   "dPd",  "ilUiUlfd",        MergeNone, "aarch64_sve_compact">;
-//  SVDUP_LANE    (to land in D78750)
+def SVDUP_LANE   : SInst<"svdup_lane[_{d}]",  "ddL",  "csilUcUsUiUlhfd", MergeNone, "aarch64_sve_tbl">; // Implemented using tbl
 def SVDUPQ_LANE  : SInst<"svdupq_lane[_{d}]", "ddn",  "csilUcUsUiUlhfd", MergeNone, "aarch64_sve_dupq_lane">;
----------------
"Implemented using tbl" isn't really a helpful comment.

I guess you're doing it this way so the lane doesn't have to be a constant?  (And then I guess you can optimize it in the backend.)


================
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));
----------------
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.


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

https://reviews.llvm.org/D79357