[PATCH] D74813: [RFC] Add hash of block contents to function block names

Alex Borcan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 19 17:29:07 PST 2020


alexbdv added a comment.

@vsk - about breaking existing workflows - I was referring only to if / when this gets shipped out as the default - all the names for the function blocks will change and this might cause issue with tooling that relies on symbol names being consistent across builds.

@dexonsmith

- Just to make sure - this isn't dumping LLVM IR but a textual representation of the AST. Does this have the same issues with metadata / metadata numbering that LLVM IR has, or you were referring to the AST text dump, not llvm IR ? Also I don't think that clang would be a good test case here - it doesn't have many block functions.
- Stability wise - I'm still not sure if this has the metadata numbering issue (since it's AST text representation), perhaps you can tell me how to check.
- Correct, the hash might change if the block contents changes - this is kind of the intended use for this. As the flag mentions, it is hash-based.

Moving to a per-function index-based approach seems like the correct default approach. I would still like to have the hash version under a flag though. Moving to per-function index-based should be as simple as not adding the  discriminator at all. Since function blocks contain the parent's function's name and given that llvm auto-renames duplicate symbols by adding an incremental number - the end result is that function blocks will be incrementally named based on the order that they have in the parent function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813





More information about the cfe-commits mailing list