[PATCH] D21723: [RFC] Enhance synchscope representation

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D21723#700067, @kzhuravl wrote:

> > I think I mentioned in the past using target specific strings instead of integer in textual and bitcode serialization, why was this ruled out? This was intended to addressed such issues (and make the textual IR more readable)
>
> I do not recall original reasons we did not go with target specific strings, but just thinking from the top of my head:
>
> - Existing synchronization scope field is represented as 32 bits unsigned field in the bitcode


Not a problem. Just redirect through a table.

> - Introducing new synchronization scopes is naturally simple - minimal impact across the board

Can be made as easy with strings.

> - I think changing the synchronization scope representation to strings will impose unnecessary complexity
>   - On maintaining backwards compatibility for the bitcode (in addition, strings might be of arbitrary length)

I don't understand the problem: add a mapping string <-> int in the bitcode once, keep an int to encode the syncscope, but use the mapping on read when the syncscope id is `> current_max_number`.

> - On using existing synchronization scopes (CrossThread, SingleThread) within the llvm. Do we want to represent those as strings as well?

Just in the textual format.

> - Few size-sensitive structures (e.g. MMO) have synchronization scope as a member, so changing syncscope to string will increase their size

I don't see why you're think that the internals of LLVM have to match the textual or bitcode representation. We routinely pool strings in the context.

> - +additional burden for carrying through strings throughout the compilation (compared to ints)

It seems to me that you're raising concerns that would only apply to a straight naive translation of strings in the IR to strings everywhere in the internals of LLVM. I didn't see anything that can't be addressed through a string pool in the context (and in the bitcode FWIW).


https://reviews.llvm.org/D21723