[PATCH] D147697: [IR] Add TargetExtTypeClass

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D147697#4435720 <https://reviews.llvm.org/D147697#4435720>, @nhaehnle wrote:

>> Could you please add a test where a target type is first used and later type info for it is provided? I'd like to make sure this is an error.
>
> Added the conflicting-typeinfos3.ll subtest which cover this (if I understood you correctly).

Yes, that's what I had in mind. Thanks!



================
Comment at: llvm/docs/LangRef.rst:3837
+        hasZeroInit: i1 true,
+      }
 
----------------
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?


================
Comment at: llvm/include/llvm/Bitcode/LLVMBitCodes.h:738
+  SDATA_TYPE = 1,
+  SDATA_SYMBOL = 2,
+  SDATA_INT_BASE = 15, // add the bit size
----------------
nhaehnle wrote:
> nikic wrote:
> > What is SDATA_SYMBOL used for, in the context of this patch? It doesn't look like the corresponding IR handling in D150370 supports this.
> It isn't used yet. Symbols-as-values is something that I added when exploring how to put enums into metadata, and I didn't remove it here because it does seem useful.
In that case, please drop it until it is used for something (and thus testable).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147697