Page MenuHomePhabricator

alexbdv (Alex Borcan)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 29 2018, 2:13 PM (231 w, 4 h)

Recent Activity

Jun 4 2020

alexbdv accepted D81187: Add cl::ZeroOrMore to get around build system issues.

Thanks for fixing this !

Jun 4 2020, 12:42 PM · Restricted Project

May 14 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@dexonsmith - Are you OK with that ?

May 14 2020, 11:57 AM · Restricted Project, Restricted Project

May 13 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

Could I please get a review on this ? Thanks :) !

May 13 2020, 2:09 PM · Restricted Project, Restricted Project

May 6 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@dexonsmith @erik.pilkington The change is final now, could we get this in ?

May 6 2020, 2:44 PM · Restricted Project, Restricted Project

May 5 2020

alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
May 5 2020, 6:54 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
May 5 2020, 5:49 PM · Restricted Project, Restricted Project

May 4 2020

alexbdv retitled D74813: Function block naming - add hash of parameter type to end of block name from [RFC] Add hash of block contents to function block names to Function block naming - add hash of parameter type to end of block name.
May 4 2020, 6:18 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.

Updating tests.

May 4 2020, 6:18 PM · Restricted Project, Restricted Project

Apr 29 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@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 incrememntal) number, so changing it shouldn't make a difference.
Did I miss anything ?

Apr 29 2020, 2:00 PM · Restricted Project, Restricted Project
alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@erik.pilkington would the hash-based numbering be OK for now ?

Apr 29 2020, 12:21 PM · Restricted Project, Restricted Project

Apr 23 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@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 update tests).

Apr 23 2020, 1:00 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
Apr 23 2020, 1:00 PM · Restricted Project, Restricted Project

Apr 22 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

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.

Apr 22 2020, 11:57 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
Apr 22 2020, 11:25 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
Apr 22 2020, 12:29 AM · Restricted Project, Restricted Project
alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@dexonsmith, @erik.pilkington - how about this ?

Apr 22 2020, 12:29 AM · Restricted Project, Restricted Project

Apr 15 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@dexonsmith - I think that should work - like this ?

Apr 15 2020, 1:02 AM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
Apr 15 2020, 1:02 AM · Restricted Project, Restricted Project

Apr 14 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

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

Apr 14 2020, 3:12 PM · Restricted Project, Restricted Project
alexbdv updated the diff for D74813: Function block naming - add hash of parameter type to end of block name.
Apr 14 2020, 3:12 PM · Restricted Project, Restricted Project

Apr 13 2020

alexbdv added a comment to D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .

@rjmccall I see, I'm not that familiar with non-LTO workflow, I'll look into adding what needs to be added to have this also work for non-LTO. I was imagining It would be lot more integration.
Any other thoughts on this ?

Apr 13 2020, 9:10 PM · Restricted Project
alexbdv added a comment to D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .

@rjmccall - Yes, by "extra logic" I was referring to having to add integration with the non-LTO scenario. All the fundamental "logic" for computing offsets should already be here.
The pass is LTO-specific because this is the only scenario I'm trying to address right now - no other reason. Are there any cons to leaving it as a TODO for the future ?

Apr 13 2020, 4:52 PM · Restricted Project
alexbdv added a comment to D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .

@rjmccall - It can be implemented, sure but it would require extra logic that is not included here.
How common is this scenario ? I was targeting LTO only for now. Would it be OK to leave this as a TODO ?

Apr 13 2020, 3:15 PM · Restricted Project

Apr 11 2020

alexbdv added a comment to D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .

@rjmccall - It does not. This would only work if all interface implementations were inside the same translation unit. But since that is not a common scenario it was not implemented to support this.

Apr 11 2020, 1:34 AM · Restricted Project

Apr 9 2020

alexbdv updated the summary of D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .
Apr 9 2020, 5:07 PM · Restricted Project
alexbdv created D77830: [RFC] Generate more constant iVar Offsets via global LTO analysis .
Apr 9 2020, 2:43 PM · Restricted Project

Apr 8 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@dexonsmith - any suggestions to move forward on this ?

Apr 8 2020, 1:02 PM · Restricted Project, Restricted Project
alexbdv added inline comments to D74813: Function block naming - add hash of parameter type to end of block name.
Apr 8 2020, 12:30 PM · Restricted Project, Restricted Project

Mar 4 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

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

Mar 4 2020, 2:05 PM · Restricted Project, Restricted Project

Feb 22 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

@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 repeated runs) - so seems the hashing is insignificant compile-time-wise even in worst case scenario (where most of the code is in function blocks).
And here is the actual text that is being hashed - the textual representation of the AST - this is not llvm IR with metadata: https://paste.ofcode.org/bfMzhbvHz9HRT7mVMe48Mx

Feb 22 2020, 5:43 PM · Restricted Project, Restricted Project

Feb 19 2020

alexbdv added a comment to D74813: Function block naming - add hash of parameter type to end of block name.

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

Feb 19 2020, 5:29 PM · Restricted Project, Restricted Project
alexbdv updated subscribers of D74813: Function block naming - add hash of parameter type to end of block name.

@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. Also, unexpectedly (by default) changing the name of the function blocks might have a negative impact on some existing workflows.

Feb 19 2020, 11:27 AM · Restricted Project, Restricted Project

Feb 18 2020

alexbdv created D74813: Function block naming - add hash of parameter type to end of block name.
Feb 18 2020, 11:10 PM · Restricted Project, Restricted Project

Sep 4 2018

alexbdv added a comment to D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

I don't have commit access. steven_wu can you please commit this for me ?

Sep 4 2018, 3:33 PM
alexbdv updated the diff for D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.
Sep 4 2018, 3:23 PM
alexbdv updated the diff for D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.
Sep 4 2018, 3:14 PM
alexbdv added a comment to D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.

Indeed it can, that is another fix. I was just doing the "obvious" fix since I assume there is a reason why the ::codegen(Module &TheModule) function exists.
Should we just remove the function altogether ? As-is it is not thread-safe = results in crashes.
Also, a clone of TMBuilder is not expensive considering the context.

Sep 4 2018, 1:15 PM
alexbdv added a reviewer for D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified: llvm-commits.
Sep 4 2018, 12:14 PM
alexbdv created D51651: [ThinLTO] Fix memory corruption in ThinLTOCodeGenerator when CodeGenOnly was specified.
Sep 4 2018, 12:10 PM