[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv added a comment. @dexonsmith - Are you OK with that ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv added a comment. Could I please get a review on this ? Thanks :) ! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv added a comment. @dexonsmith @erik.pilkington The change is final now, could we get this in ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv updated this revision to Diff 262281. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/lib/AST/Mangle.cpp clang/test/CXX/expr/expr.prim/expr.prim.lambda/blocks-irgen.mm clang/test/CodeGen/block-with-perdefinedexpr.cpp c

[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv updated this revision to Diff 262272. alexbdv marked an inline comment as done. alexbdv edited the summary of this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/lib/AST/Mang

[PATCH] D74813: Function block naming - add hash of parameter type to end of block name

alexbdv updated this revision to Diff 261978. alexbdv added a comment. Herald added subscribers: kerbowa, nhaehnle, jvesely. Updating tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/li

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

alexbdv added a comment. @dexonsmith what regression are you referring to ? What this change is effectively doing now is changing the numbering of the blocks from incremental to hash-based. So the demangler functionality remains the same (i think) - I saw that it is ignoring the (currently incr

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

alexbdv added a comment. @erik.pilkington would the hash-based numbering be OK for now ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing

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

alexbdv added a comment. @erik.pilkington How about just adding numeric hash of block parameters for now ? This way we don't have to change the mangling / demangling scheme at all. The mangling / demangling changes bring me into parts of LLVM that I'm not familiar with. (PS - I still have to upd

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

alexbdv updated this revision to Diff 259684. alexbdv marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/lib/AST/Mangle.cpp Index: clang/lib/AST/Mangle.cpp

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

alexbdv marked 6 inline comments as done. alexbdv added a comment. For tests - the existing block tests should be enough, just need to update them. I updated a few - want to make sure changes are final before updating all the tests to not have to update tests again. Comment at

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

alexbdv updated this revision to Diff 259487. Herald added a reviewer: jhenderson. Herald added a project: libc++abi. Herald added a subscriber: libcxx-commits. Herald added a reviewer: libc++abi. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/ne

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

alexbdv updated this revision to Diff 259181. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/lib/AST/Mangle.cpp llvm/include/llvm/Demangle/ItaniumDemangle.h Index: llvm/include/llvm/Demangle/ItaniumDemangle.h ==

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

alexbdv added a comment. @dexonsmith, @erik.pilkington - how about this ? Z7my_mainv_block_invoke_ZTSi_ZTSj_ZTSPVK3sss demangled as: invocation function for block with params '(int, unsigned int, sss const volatile*)' in my_main() CHANGES SINCE LAST ACTION https://reviews.llvm.org/D748

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

alexbdv added a comment. @dexonsmith - I think that should work - like this ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing list cfe-c

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

alexbdv updated this revision to Diff 257616. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/lib/AST/Mangle.cpp Index: clang/lib/AST/Mangle.cpp ==

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

alexbdv added a comment. @erik.pilkington / @vsk / @dexonsmith - how block name = hash(parent_function_name + block_type) ? So any blocks that are inside the same parent function + have the same type will get an incremental number to de-duplicate names. CHANGES SINCE LAST ACTION https://rev

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

alexbdv updated this revision to Diff 257508. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 Files: clang/include/clang/AST/Mangle.h clang/lib/AST/Mangle.cpp clang/lib/CodeGen/CGBlocks.cpp clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/

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

alexbdv added a comment. @dexonsmith - any suggestions to move forward on this ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74813/new/ https://reviews.llvm.org/D74813 ___ cfe-commits mailing list cf

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

alexbdv marked an inline comment as done. alexbdv added inline comments. Comment at: clang/lib/AST/Mangle.cpp:56 + + // Strip out addresses + char *ptr = &strStmtBuff[1]; manmanren wrote: > Is this needed to have deterministic behavior? Correct. Repository:

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

alexbdv added a comment. @vsk / @dexonsmith - I've added some more info in latest comments. Let me know if I can provide more info / context on this to be able to reach a conclusion. Or if you think it is clear at this point that the hash-based approach is a no-go. Repository: rG LLVM Gith

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

alexbdv added a comment. @dexonsmith I did a benchmark with the worst case example I can come up with - 1000 regular functions and 1000 blocks : https://paste.ofcode.org/CFU6b46nuAA6ymxUEpkzka I didn't manage to measure any performance decrease (the performance decrease was within the noise of

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

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. @de

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

alexbdv added a subscriber: vsk. alexbdv added a comment. @vsk - sure will add tests when removing from RFC. As for making it default - would rather have this under a flag as hashing the block contents does have some overhead and I imagine this feature wouldn't be beneficial in most scenarios.

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

alexbdv created this revision. alexbdv added reviewers: MaskRay, vsk, JonasToth, ruiu. Herald added a project: clang. Herald added a subscriber: cfe-commits. Function blocks don't have a name specified in source code. Currently their symbol name is based on the parent function's name and an index