[PATCH] D150370: Introduce StructuredData
Nicolai Hähnle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 19 04:57:48 PDT 2023
nhaehnle added a comment.
Thanks for taking a look!
In D150370#4349276 <https://reviews.llvm.org/D150370#4349276>, @Flakebi wrote:
> Why does the symbol map need to be global, shouldn’t it by per LLVMContext (which would also get rid of the locking)?
It's global so that we can cheaply compare an `sdata::Symbol` against an `sdata::RegisterSymbol`. This is used e.g. here: https://github.com/nhaehnle/llvm-project/blob/6bb5923a059c1120e006cb0d0cd63c5ef0806c0e/llvm/lib/IR/Type.cpp#L844
> Needs tests.
What kind of tests do you have in mind? This change doesn't really expose anything, and there are tests in the followup change in the stack.
================
Comment at: llvm/docs/LangRef.rst:3519-3523
+ sdata ::= '{' (sdata_field ',')* sdata_field? '}'
+ sdata_field ::= label sdata_value
+ sdata_value ::= 'type' type
+ ::= integer
+ ::= 'true' | 'false'
----------------
Flakebi wrote:
> Can constants follow the way metadata (and most other things in LLVM) is encoded and prefix the type?
> I.e. `i32 <integer>` and `i1 true/false`
Good question. I've been going back and forth on this, and the current version is as-is mostly because the various !DIxyz metadata doesn't have those prefixes. But that may not be the best example to follow.
I'd be happy to change it to be more in line with metadata. Any other opinions?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150370/new/
https://reviews.llvm.org/D150370
More information about the llvm-commits
mailing list