[PATCH] Add support for symbolic large constant entries inside stackmaps
undingen at gmail.com
Sun Apr 26 13:51:10 PDT 2015
Thanks for the comments and sorry for not having written any documentation because I thought the default behavior of "RTDyldMemoryManager::getSymbolAddress" which will resolve the symbols makes documenting the change unnecessary but I can see that this may be unexpected and needs documentation.
I reused the existing stackmap format in order to not break existing clients. That's why I choose to put them in the constant value table but this means it's impossible for clients to retrieve the number of large constants which don't need symbol resolving and which do. (They will just be at the end of the table but there is no way to know how many there are at the end)
If changing the stackmap format is not a big problem than maybe it's best to make a new section, which would make the resulting stackmap and the code to generate it inside llvm more obvious.
The MVT::i64 check is my attempt at making sure that the symbol addresses are exactly 64bit long like the entries in the stackmap constant table. But this may should get changed to allow smaller values.
Thanks again for the comments, I will wait with changing the patch until we all agreed on how to proceed.
In http://reviews.llvm.org/D9176#161717, @reames wrote:
> Comments inline, but you are still missing the most important part of the patch. Where is the documentation which describes the output format? I have inferred some of it from the code, but it will save you time if we can settle on the output before hashing through every bit of code. We may need to change the code based on the result of the format discussion.
> One question to consider: As a consumer of the generated stackmap, how do I tell a given entry is a actual constant vs a symbol that needs resolved? Maybe I'm missing the obvious, but I didn't see this in the code. Or are you assuming that the consumer is only parsing the finalized/full relocated version of the binary section?
More information about the llvm-commits