[PATCH] D91441: [VP] Build VP SDNodes

Simon Moll via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 00:37:15 PST 2020


simoll added inline comments.


================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:85
+// \p OC  The standard IR opcode.
 #ifndef HANDLE_VP_TO_OC
+#define HANDLE_VP_TO_OC(OC)
----------------
craig.topper wrote:
> Can OC here be OPC?  That's probably a more common abbreviation for opcode.
That's upstream code - i'll rename to `HANDLE_VP_TO_OPC` in an NFC patch and rebase.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6140
     return;
+#define BEGIN_REGISTER_VP_INTRINSIC(VPINTRIN, ...) case Intrinsic::VPINTRIN:
+#include "llvm/IR/VPIntrinsics.def"
----------------
craig.topper wrote:
> Instead of adding all of the enumerated values to the switch, I wonder if it might sense to use a dyn_cast<VPIntrinsic> in the default case? Same for constrained fp.
I can only speak for this patch - this was born out of laziness and the vague idea that having one "uberswitch" over all intrinsics gives you better/faster code.
If we want to change this, we should probably do so in a separate patch that uses `dyn_cast<XIntrinsic>` where possible.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91441



More information about the llvm-commits mailing list