[PATCH] D147697: [IR] Add TargetExtTypeClass

[Joshua Cranmer via Phabricator via llvm-commits]

jcranmer-intel added inline comments.


================
Comment at: llvm/docs/LangRef.rst:3837
+        hasZeroInit: i1 true,
+      }
 
----------------
nikic wrote:
> nhaehnle wrote:
> > nikic wrote:
> > > This could use some clarification on how the properties for target types with different arguments relate, if at all. If you define properties for `target("mytype")`, do those also apply to `target("mytype", i32)`?
> > > 
> > > Do you need to repeat the properties for every combination of arguments in the IR, or is there some inheritance?
> > They don't relate and there is no inheritance, every target type is distinct and has its own properties. I did briefly consider the possibility of inheritance, but it seemed to quickly get very complex and not that useful. I'm going to try to clarify that.
> I see. I think this point could use some input from people who are using target types, in particular @jcranmer-intel.
> 
> The way I originally expected this to work is that the target type properties are independent of the type arguments entirely, so that `target("mytype")` and all `target("mytype", Ti, Ni)` share the same properties.
> 
> I'm not familiar enough with how these types are used to say whether independent properties for each argument combination makes sense or not.
All of the use cases I have had have the property that properties are based entirely on the string property. Indeed, one property that could be useful to check is the number of integer/type arguments!

That said, there are some cases where varying type parameters might change the properties of the type--suppose something like `target("complex", float)` versus `target("complex", double)` or `target("decfp", 64)` versus `target("decfp", 32)`. Although for many of those types, I do question if they should be target extension types in the first place.


================
Comment at: llvm/include/llvm/IR/TargetExtType.h:51-59
+  /// Given a partially initialized type, return the layout type (defaults to
+  /// void, indicating that the type can't be laid out in memory).
+  using GetLayoutTypeFn = Type *(TargetExtType *T);
+  GetLayoutTypeFn *GetLayoutType = nullptr;
+
+  /// Given a partially initialized type, return the type properties (defaults
+  /// to 0).
----------------
I question the need for these to be function pointers rather than instance properties. (The verifier function does need to be its own function).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147697