[PATCH] D100257: [VPlan] Add VPUserID to distinguish between recipes and others.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 20 08:27:48 PDT 2021


fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:685
+  static inline bool classof(const VPUser *U) {
+    return U->getVPUserID() == VPUser::VPUserID::Recipe;
+  }
----------------
a.elovikov wrote:
> Can we make `getVPUserID()` protected instead of friendship above?
Unfortunately I don't think so. If it's protected, we can access it from inside a VPRecipeBase object, but here we need to access it from a `VPUser` pointer directly.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:276
 
+static_assert(sizeof(VPUser) <= 48, "size of VPUser changed");
+
----------------
a.elovikov wrote:
> 1) Why is that important? Do we rely on it or is it simply to prevent uncontrolled growth?
> 2) Why do we expect it to be the same on 32/64-bit platforms? Don't we have any pointers inside the object? I'd expect the vtable itself would make it different...
> Why is that important? Do we rely on it or is it simply to prevent uncontrolled growth?

It's just to prevent accidental growth, but it is not strictly necessary (and we don't have static asserts for other sizes yet), but it is a common pattern, e.g. in various key classes in Clang to prevent unnoticed size growth.

> Why do we expect it to be the same on 32/64-bit platforms?

I think `48` should be the upper bound on 64 bit platforms (unless there are differences caused by different 64 bit operating systems) and on 32-bit platforms it should be less than or equal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100257



More information about the llvm-commits mailing list