[PATCH] D91441: [VP] Build VP SDNodes

[Simon Moll via Phabricator via llvm-commits]

simoll added inline comments.


================
Comment at: llvm/include/llvm/IR/VPIntrinsics.def:72
+
+#define END_REGISTER_VP(INTRIN, SDID) \
+END_REGISTER_VP_INTRINSIC(INTRIN) \
----------------
frasercrmck wrote:
> 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
> ```
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91441