Translate VP intrinsics to VP_* SDNodes. The tests check whether a matching vp_* SDNode is emitted.
Can OC here be OPC? That's probably a more common abbreviation for opcode.
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.
That's upstream code - i'll rename to HANDLE_VP_TO_OPC in an NFC patch and rebase.
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.
Missing ( somewhere?
Mismatch between EVLPOS and VLENPOS
I'm thinking of these as scopes as the documentation describes them, and it strikes me that the "INTRINSIC" and "SDNODE" scopes overlap here when using BEGIN_REGISTER_VP and END_REGISTER_VP:
BEGIN_REGISTER_VP_INTRINSIC BEGIN_REGISTER_VP_SDNODE END_REGISTER_VP_INTRINSIC END_REGISTER_VP_SDNODE
Is there a reason for this, or should it be balanced?
BEGIN_REGISTER_VP_INTRINSIC BEGIN_REGISTER_VP_SDNODE END_REGISTER_VP_SDNODE END_REGISTER_VP_INTRINSIC
parameter named VPIntrin like in the function def?
One advantage of improper nesting is that you can check easily whether scopes overlap just using macros. Eg, you can ask what is the canonical SDNode of this VPIntrinsic:
BEGIN_VP_INTRINSIC BEGIN_VP_SDNODE ; <-- this SDNode belongs to the VP intrinsic of the outer scope [..] END_VP_INTRINSIC
At the same time, you can go the opposite direction and look up the VPIntrinsic that has this SDNode as its canonical form:
BEGIN_VP_SDNODE [..] END_VP_INTRINSIC ; <-- this intrinsic belongs to the VP SDNode of the outer scope END_VP_SDNODE
(not that we were exploiting that atm..).. on the other hand nesting scopes is more intuitive but doesn't seem to have anything else speaking for it.
Should this use EVLPOS instead of VLENPOS?
I think ISDOPC or something might be better than SDID. I don't think we tend to call them ids for SelectionDAG. In SelectionDAG terms, id would tend to mean the NodeID field in SDNode.
This ever expected to return an Optional with no value set? It looks like the only caller just calls getValue(). Should we just have an llvm_unreachable in the switch instead?
Can we use "not llc" instead of "|| true"?
Thanks for your feedback so far!
- Tests: use not --crash instead of piping to true. (Hexagon tests do the same thing).
- Say VPIntrin instead of VPI.
- Say SDOPC instead of SDID.
- Added llvm_unreachable() if no SDNode opcode is available for a given vp intrinsic.
- Fixed various cases of can(n)onical i missed in the last update.
I noticed these new tests won't work on Release mode, so I disabled them. Please update them later.
This test won't work on Release mode since error messages don't contain temporal register name under Release mode. I disabled all tests under Release mode. Please update them later.