Page MenuHomePhabricator

plotfi (Puyan Lotfi)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 30 2017, 10:52 AM (260 w, 6 d)

Recent Activity

Tue, Nov 1

plotfi accepted D137155: [MIRVRegNamer] Avoid opcode hash collision.

LGTM, Nice work John.

Tue, Nov 1, 12:10 PM · Restricted Project, Restricted Project

Oct 11 2022

plotfi added a reviewer for D128249: Adding clone_attrs attribute.: nuriamari.
Oct 11 2022, 1:02 PM · Restricted Project, Restricted Project
plotfi updated subscribers of D128249: Adding clone_attrs attribute..
Oct 11 2022, 1:01 PM · Restricted Project, Restricted Project

Oct 6 2022

plotfi added a comment to D135091: Fix assert in generated `direct` property getter/setters due to removal of `_cmd` parameter..

LGTM but waiting on Akira would be nice imho.

Oct 6 2022, 2:49 PM · Restricted Project, Restricted Project
plotfi added a comment to D135091: Fix assert in generated `direct` property getter/setters due to removal of `_cmd` parameter..

@ahatanak how does this diff look to you?

Oct 6 2022, 2:42 PM · Restricted Project, Restricted Project

Sep 28 2022

plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@ahatanak I can revive some of what I was working on from https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for the checks as @rjmccall mentioned.

I believe the generated direct methods already handle the null checks and class init in CGObjCCommonMac::GenerateDirectMethodPrologue, meaning the thunks aren't strictly necessary for the callee to handle them.

Could the thunks instead allow us to have publicly-visible mangled names (something akin to the new selector stubs _objc_msgSend$selectorName but for _objc_direct$Class_selectorName) while leaving the actual impl name alone, letting the stack traces see normal ObjC symbol names?

I think the square brackets are still problematic for linking, so is LLVM's handling of \01 (I believe).

Sep 28 2022, 10:18 AM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@ahatanak I can revive some of what I was working on from https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for the checks as @rjmccall mentioned.

I believe the generated direct methods already handle the null checks and class init in CGObjCCommonMac::GenerateDirectMethodPrologue, meaning the thunks aren't strictly necessary for the callee to handle them.

Could the thunks instead allow us to have publicly-visible mangled names (something akin to the new selector stubs _objc_msgSend$selectorName but for _objc_direct$Class_selectorName) while leaving the actual impl name alone, letting the stack traces see normal ObjC symbol names?

Sep 28 2022, 8:57 AM · Restricted Project, Restricted Project

Sep 27 2022

plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@ahatanak I can revive some of what I was working on from https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for the checks as @rjmccall mentioned.

Sep 27 2022, 12:59 PM · Restricted Project, Restricted Project
plotfi updated subscribers of D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@dmaclach @ahatanak @mwyman How do things look from here? Do you want something for properties as well or would it be ok if we did this in a later commit?

Huh, for some reason I thought when I'd last poked at using the visibility attribute it wasn't allowed on ObjC methods, which is why I'd suggested adding the enum on objc_direct, but as that no longer appears to be the case yes I like this approach better.

@dmaclach @mwyman I am also very happy with the fact that we can just reuse the regular visibility attribute. In the future we can decide on revised behavior for hasMethodVisibilityDefault.

@ahatanak Do you have feedback on this?

The visibility attribute changes look good to me.

Do we have the answers to the last two questions John raised in https://reviews.llvm.org/D86049#2255738? I think we should get it right the first time since, once we make the direct methods visible, it'd be hard to change where the null check or class initialization is done since that would be an ABI change. Should we run experiments to measure the impact on code size?

Sep 27 2022, 12:56 PM · Restricted Project, Restricted Project
plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 27 2022, 12:41 PM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@dmaclach @ahatanak @mwyman How do things look from here? Do you want something for properties as well or would it be ok if we did this in a later commit?

Huh, for some reason I thought when I'd last poked at using the visibility attribute it wasn't allowed on ObjC methods, which is why I'd suggested adding the enum on objc_direct, but as that no longer appears to be the case yes I like this approach better.

Sep 27 2022, 10:55 AM · Restricted Project, Restricted Project

Sep 26 2022

plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 26 2022, 6:03 PM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@dmaclach @ahatanak @mwyman How do things look from here? Do you want something for properties as well or would it be ok if we did this in a later commit?

Sep 26 2022, 12:49 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 26 2022, 12:47 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Updated based on @ahatanak's feedback.

Sep 26 2022, 9:36 AM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
  1. Do we change the existing visibility behavior of objc methods? Yes / No

I don't think we want to change the existing visibility behavior of non-direct objc methods. Is there a use reason or use case for making them visible outside the linkage unit?

  1. If we leave hidden as the default do we change the behavior for objc_direct? Yes / No

I think direct methods shouldn't be hidden by default (i.e., they should get the default visibility). But it's not clear to me whether we should make that change right away as I've heard concerns from people internally. I think I need more time to understand what exactly their concerns are.

  1. If we leave objc_direct as hidden by default do we expand the existing objc_direct attr to have the enum as you said so __attribute__((objc_direct("visible"))) or do we add a new attr as I have done so far?

I wasn't sure why it wasn't possible to use the existing __attribute__((visibility("default"))) attribute. Is it not possible to make only the direct methods get the default visibility?

This is not possible because default visibility is implicit (as far as I understand). It can not be checked if __attribute__((visibility("default"))) is set because it is always set unless -fvisibility=hidden is passed on the command line. So either we treat direct methods like everything else, and hide them when __attribute__((visibility("hidden"))) or the command line to hide everything by default is used, or we need a new attr or a new enum on the existing objc_direct attr.

Does this make sense or am I missing some details?

But there are ways to check whether the user explicitly added a visibility attribute (e.g., __attribute__((visibility("default")))), right? For example, NamedDecl::getExplicitVisibility.

I'm just not sure whether we want to add support for a new attribute like __attribute__((objc_direct("default"))) since it seems equivalent to __attribute__((objc_direct, visibility("default"))).

Sep 26 2022, 6:44 AM · Restricted Project, Restricted Project

Sep 23 2022

plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 23 2022, 6:48 PM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
  1. Do we change the existing visibility behavior of objc methods? Yes / No

I don't think we want to change the existing visibility behavior of non-direct objc methods. Is there a use reason or use case for making them visible outside the linkage unit?

  1. If we leave hidden as the default do we change the behavior for objc_direct? Yes / No

I think direct methods shouldn't be hidden by default (i.e., they should get the default visibility). But it's not clear to me whether we should make that change right away as I've heard concerns from people internally. I think I need more time to understand what exactly their concerns are.

  1. If we leave objc_direct as hidden by default do we expand the existing objc_direct attr to have the enum as you said so __attribute__((objc_direct("visible"))) or do we add a new attr as I have done so far?

I wasn't sure why it wasn't possible to use the existing __attribute__((visibility("default"))) attribute. Is it not possible to make only the direct methods get the default visibility?

Sep 23 2022, 12:25 PM · Restricted Project, Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
  1. Do we change the existing visibility behavior of objc methods? Yes / No

I don't think we want to change the existing visibility behavior of non-direct objc methods. Is there a use reason or use case for making them visible outside the linkage unit?

  1. If we leave hidden as the default do we change the behavior for objc_direct? Yes / No

I think direct methods shouldn't be hidden by default (i.e., they should get the default visibility). But it's not clear to me whether we should make that change right away as I've heard concerns from people internally. I think I need more time to understand what exactly their concerns are.

  1. If we leave objc_direct as hidden by default do we expand the existing objc_direct attr to have the enum as you said so __attribute__((objc_direct("visible"))) or do we add a new attr as I have done so far?

I wasn't sure why it wasn't possible to use the existing __attribute__((visibility("default"))) attribute. Is it not possible to make only the direct methods get the default visibility?

Sep 23 2022, 12:23 PM · Restricted Project, Restricted Project

Sep 21 2022

plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

I think the decision tree goes

Sep 21 2022, 1:45 PM · Restricted Project, Restricted Project
plotfi updated subscribers of D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

@ahatanak @mwyman @rjmccall @dmaclach I am going to work on a version of this patch where the visibility can be encoded into the objc_direct attribute. Does that seem reasonable to do from here?

Sep 21 2022, 1:35 PM · Restricted Project, Restricted Project

Sep 20 2022

plotfi added a comment to D131424: Remove the unused/undefined _cmd parameter to objc_direct methods..

Hi Akira,

I'd reached out to John offline and he'd mentioned you might be able to help on some of these objc_direct reviews; if so, that would be wonderful!

-Michael

Sep 20 2022, 1:40 PM · Restricted Project, Restricted Project

Sep 13 2022

plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 13 2022, 12:29 PM · Restricted Project, Restricted Project

Sep 10 2022

plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 10 2022, 12:38 AM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Adding a test case to cover @protocol methods not being allowed to contain direct

Sep 10 2022, 12:38 AM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Update test.

Sep 10 2022, 12:26 AM · Restricted Project, Restricted Project
plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 10 2022, 12:25 AM · Restricted Project, Restricted Project

Sep 9 2022

plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 9 2022, 11:10 PM · Restricted Project, Restricted Project
plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 9 2022, 11:09 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Updated to use mangleObjCMethodName in clang/lib/AST/Mangle.cpp to set the _+<a: > direct method name.

Sep 9 2022, 11:06 PM · Restricted Project, Restricted Project
plotfi added inline comments to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..
Sep 9 2022, 10:46 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Updated with some of @mwyman's feedback.

Sep 9 2022, 10:42 PM · Restricted Project, Restricted Project

Aug 8 2022

plotfi added a comment to D131424: Remove the unused/undefined _cmd parameter to objc_direct methods..

Updated D86049.

Aug 8 2022, 10:26 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Updating implementation to use an objc_direct_visible attr to explicitly mark when we want objc_direct to be exposed outside of the link unit.

Aug 8 2022, 10:10 PM · Restricted Project, Restricted Project
plotfi added inline comments to D131424: Remove the unused/undefined _cmd parameter to objc_direct methods..
Aug 8 2022, 6:58 PM · Restricted Project, Restricted Project
plotfi added a comment to D131424: Remove the unused/undefined _cmd parameter to objc_direct methods..

I tried running the following on some example code and got a stacktrace:

Aug 8 2022, 5:57 PM · Restricted Project, Restricted Project

Jul 19 2022

plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Hi, I work with @dmaclach. I know this has been sitting around without much activity for two years, but we believe there is a solid use-case for this on our end and I'd like to help get the ABI nailed down and land this change (or one accomplishing the same goal). Do you still have interest in this?

Jul 19 2022, 1:33 PM · Restricted Project, Restricted Project

Jun 29 2022

plotfi added a comment to D128688: [llvm-objcopy] Remove support for legacy .zdebug sections.

@plotfi Do you have zlib-gnu usage?

Hi all thanks for reaching out. I will confirm with some folks who I wrote this zlib-gnu support for to confirm it is not needed.

We don't need zlib-gnu anymore.

Jun 29 2022, 10:39 AM · Restricted Project, Restricted Project
plotfi added a comment to D128688: [llvm-objcopy] Remove support for legacy .zdebug sections.

@plotfi Do you have zlib-gnu usage?

Jun 29 2022, 10:13 AM · Restricted Project, Restricted Project

Jun 21 2022

plotfi added a comment to D128249: Adding clone_attrs attribute..

Thanks for the feedback on corner cases @aaron.ballman, this will give me more concrete things to think about here.

Jun 21 2022, 10:19 AM · Restricted Project, Restricted Project
plotfi added a comment to D128249: Adding clone_attrs attribute..

This is pretty incomplete. Please ignore for now.

Jun 21 2022, 1:14 AM · Restricted Project, Restricted Project
plotfi requested review of D128249: Adding clone_attrs attribute..
Jun 21 2022, 1:14 AM · Restricted Project, Restricted Project

Apr 5 2022

plotfi added a reviewer for D122691: [clang][Sema] Add flag to LookupName to force C/ObjC codepath: rjmccall.
Apr 5 2022, 2:20 PM · Restricted Project, Restricted Project
plotfi updated subscribers of D122691: [clang][Sema] Add flag to LookupName to force C/ObjC codepath.

@rjmccall would you be able to review this patch? What do you think of this approach? This change is to support C++-Interop on the Swift side.

Apr 5 2022, 2:20 PM · Restricted Project, Restricted Project

Jan 25 2022

plotfi committed rG227d18b3a87a: [lld][macho][NFC] Make MachO/start-end.s test less britle by checking for _main: (authored by plotfi).
[lld][macho][NFC] Make MachO/start-end.s test less britle by checking for _main:
Jan 25 2022, 7:24 PM

Oct 15 2021

plotfi committed rGaa80034ab986: [DebugInfo] retainedTypes should not have subprograms (authored by ellis).
[DebugInfo] retainedTypes should not have subprograms
Oct 15 2021, 9:43 AM
plotfi closed D111593: [DebugInfo] retainedTypes should not have subprograms.
Oct 15 2021, 9:42 AM · Restricted Project, debug-info

Jun 28 2021

plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

Has anything happened with this at all or did it get dropped?

Jun 28 2021, 10:42 AM · Restricted Project, Restricted Project

Jun 10 2021

plotfi added inline comments to D87302: [IRSim][IROutliner] Adding DebugInfo handling for IR outlined functions..
Jun 10 2021, 2:43 PM · debug-info, Restricted Project

May 3 2021

plotfi accepted D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..

LGTM once you've addressed @phosek's nits.

May 3 2021, 11:30 PM · Restricted Project

Mar 31 2021

plotfi added inline comments to D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..
Mar 31 2021, 5:16 PM · Restricted Project
plotfi added inline comments to D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..
Mar 31 2021, 5:07 PM · Restricted Project
plotfi added inline comments to D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..
Mar 31 2021, 4:13 PM · Restricted Project

Mar 30 2021

plotfi added inline comments to D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..
Mar 30 2021, 6:02 PM · Restricted Project

Mar 25 2021

plotfi added a comment to D99399: [elfabi] Prepare llvm-elfabi for elfabi/ifs merging..

I will take another look over this commit in the morning. Overall looks good.

Mar 25 2021, 10:37 PM · Restricted Project

Mar 3 2021

plotfi added a comment to D97878: [DirectoryWatcher] Increase timeout to make test less flaky.

@jkorous thoughts?

Mar 3 2021, 7:18 PM · Restricted Project
plotfi accepted D97878: [DirectoryWatcher] Increase timeout to make test less flaky.

This makes sense to me. I approve. Can we move the 3/60 seconds number to a const int value set somewhere higher up in the file as a global with a comment explaining this as well?

Mar 3 2021, 7:17 PM · Restricted Project

Feb 16 2021

plotfi accepted D94461: [llvm-ifs] Add option to use InterfaceStub library.

If @compnerd has no gripes go ahead and land. LGTM.
Before landing, please make sure to run the check-clang tests as well because the interface stubs generation on the clang driver side invokes llvm-ifs.

Feb 16 2021, 12:52 PM · Restricted Project

Feb 1 2021

plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.

Ping? Haven't seen any chatter in a while. Anything new here? I don't want to hold you folks up.

Feb 1 2021, 1:52 PM · Restricted Project

Jan 24 2021

plotfi added a comment to D95321: [NFC] Fixing build warning with llvm-mca.

@wolfgangp I get this warning on macOS. I noticed it wasn't fixed in https://github.com/llvm/llvm-project/commit/c6e8f81410a2942b5abd112aa6e468268e01d946#diff-67f7be4aff998787c9c4efcb95e5d0dc225e1dcd37b9b005a3db5493f02efdc3 so I posted this small diff.

Jan 24 2021, 4:13 PM · Restricted Project
plotfi requested review of D95321: [NFC] Fixing build warning with llvm-mca.
Jan 24 2021, 4:12 PM · Restricted Project

Jan 20 2021

plotfi accepted D92532: [IRSimilarity] Don't copy the Mapper for createCandidatesFromSuffixTree.

Is this still open? Seems good to go.

Jan 20 2021, 4:42 PM · Restricted Project

Jan 12 2021

plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.

You mean use armv7---elf as a triple? While that is possible, keep in mind that it might change the ABI of the generated interfaces. While it is unlikely to matter as the ifso cannot be used at runtime, if the linker attempts to do something like symbolic resolution of ABI dependent specialization (e.g. strcmp.eabi vs strcmp where the latter is generic), that could matter. However, adding support to explicitly override the encoded triple based on the user's input is reasonable (though might deserve a warning that the triple is being overridden).

Jan 12 2021, 4:25 PM · Restricted Project
plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.

The full triple is needed in some cases: armv7-unknown-linux-gnueabi vs armv7-unknown-linux-gnueabihf vs armv7-unknown-linux-eabihf vs armv7-unknown-linux-eabi are all different. Additionally, there is nothing that prevents you from doing something like: armv7-unknown-linux-gnueabihf-coff or armv7-unknown-linux-gnueabihf-macho to generate COFF or MachO object files that are reading in additional bits of ABI information and the libc from the target triple. I really think that preserving the target triple is best.

Jan 12 2021, 4:16 PM · Restricted Project
plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.
  1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

I meant to add, as part of llvm-ifs being invoked by clang the idea was that clang would just forward its triple and llvm-ifs would decide what to do with it because we wanted the ifs format to be kinda platform agnostic but we wanted llvm-ifs to potentially be able to generate stubs for multiple platforms in the future (like MS .libs and tbds etc too).

Would you need a full triple in that case? Other binary tools like linkers doesn't use a full triple either, they use just e_machine and e_ident. Would that be sufficient?

Jan 12 2021, 10:37 AM · Restricted Project

Jan 11 2021

plotfi added inline comments to D94461: [llvm-ifs] Add option to use InterfaceStub library.
Jan 11 2021, 7:57 PM · Restricted Project
plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.
  1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?

The triple-ness was added because originally IFS was intended to be a tool that the clang driver invokes. The idea was that you build some project with IFS enabled and you automatically get the stubs for shipping or for CI purposes to track what new exposure was done in recent builds of said project. I think it would be ok to add a flag to override the triple if that isn't there already. @compnerd
thoughts?

Jan 11 2021, 7:45 PM · Restricted Project
plotfi added a comment to D94461: [llvm-ifs] Add option to use InterfaceStub library.
  1. I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?
Jan 11 2021, 7:43 PM · Restricted Project
plotfi added a reviewer for D94461: [llvm-ifs] Add option to use InterfaceStub library: compnerd.
Jan 11 2021, 7:34 PM · Restricted Project

Oct 27 2020

plotfi added inline comments to D88180: [RFC] StableHashTree Implementation..
Oct 27 2020, 1:25 PM · Restricted Project
plotfi retitled D88180: [RFC] StableHashTree Implementation. from [RFC] HashTree and MachineOutliner HashTree Serialization for cross module data sharing. to [RFC] StableHashTree Implementation..
Oct 27 2020, 1:17 PM · Restricted Project
plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..

Updated based on @paquette's feedback. This only includes the StableHashTree data structure and a unit test.

Oct 27 2020, 1:17 PM · Restricted Project

Oct 23 2020

plotfi added a comment to D89432: [llvm-elfabi] Emit ELF .dynsym and .dynamic sections.

Nice. Will you folks be updating llvm-ifs to use this support instead of yaml2obj soon? If so add me to the review, I will be happy to take a look.

Oct 23 2020, 10:46 AM · Restricted Project

Oct 20 2020

plotfi added a comment to D89764: [llvm] Fix ODRViolations for VersionTuple YAML specializations NFC.

Thanks for fixing this @cishida!

Oct 20 2020, 4:51 PM · Restricted Project
plotfi added a comment to D89764: [llvm] Fix ODRViolations for VersionTuple YAML specializations NFC.

I think I am overall ok with this for now until the TBE folks post some more patches to integrate ifs and tbe.

Oct 20 2020, 1:02 PM · Restricted Project

Oct 7 2020

plotfi added a comment to D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting.

@vsk any update on this? Is there anything we can do to help in landing this patch in llvm-project/main?

Oct 7 2020, 2:08 PM · Restricted Project, Restricted Project

Oct 1 2020

plotfi added a comment to D88666: DirectoryWatcher: add an implementation for Windows.

Overall looks good.

Oct 1 2020, 9:41 AM · Restricted Project

Sep 24 2020

plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..

comments added

Sep 24 2020, 10:28 PM · Restricted Project
plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..

clang-tidy

Sep 24 2020, 10:15 PM · Restricted Project
plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..

spelling and grammar

Sep 24 2020, 6:32 PM · Restricted Project
plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..

Cleaning up patch to be easier to understand.

Sep 24 2020, 6:20 PM · Restricted Project
plotfi updated the diff for D88180: [RFC] StableHashTree Implementation..
Sep 24 2020, 4:34 PM · Restricted Project
plotfi added a comment to D76570: [AArch64] Homogeneous Prolog and Epilog for Size Optimization.

@sdesmalen @efriedma @dmgreen Hi Sander, Eli, Dave. Would any of you have some time to help review this Prolog Epilog Size optimization patch? Much appreciated if you do. Me and @kyulee would be available to chat on IRC or discord about it for more info.

Sep 24 2020, 12:54 PM · Restricted Project
plotfi added reviewers for D76570: [AArch64] Homogeneous Prolog and Epilog for Size Optimization: sdesmalen, efriedma.
Sep 24 2020, 12:51 PM · Restricted Project

Sep 23 2020

plotfi added a comment to D88180: [RFC] StableHashTree Implementation..

@thegameg: @paquette tells be you might have some ideas on a better format than this json business going on here (based on work you've done on remarks). What do you think?

Sep 23 2020, 1:26 PM · Restricted Project
plotfi requested review of D88180: [RFC] StableHashTree Implementation..
Sep 23 2020, 1:20 PM · Restricted Project

Sep 18 2020

plotfi added inline comments to D86977: [IRSim][IROutliner] Limit to extracting regions that only require inputs.
Sep 18 2020, 11:28 AM · Restricted Project
plotfi added inline comments to D86977: [IRSim][IROutliner] Limit to extracting regions that only require inputs.
Sep 18 2020, 10:50 AM · Restricted Project
plotfi added a reviewer for D86977: [IRSim][IROutliner] Limit to extracting regions that only require inputs: plotfi.
Sep 18 2020, 10:44 AM · Restricted Project

Sep 15 2020

plotfi added inline comments to D86968: [IRSim] Adding IR Instruction Mapper.
Sep 15 2020, 11:48 AM · Restricted Project
plotfi added inline comments to D86968: [IRSim] Adding IR Instruction Mapper.
Sep 15 2020, 11:41 AM · Restricted Project
plotfi added inline comments to D86968: [IRSim] Adding IR Instruction Mapper.
Sep 15 2020, 10:51 AM · Restricted Project
plotfi added inline comments to D86968: [IRSim] Adding IR Instruction Mapper.
Sep 15 2020, 10:31 AM · Restricted Project

Sep 14 2020

plotfi added a reviewer for D86968: [IRSim] Adding IR Instruction Mapper: plotfi.
Sep 14 2020, 9:48 AM · Restricted Project

Sep 8 2020

plotfi committed rGefc17c4bc668: [NFC] Fixing a gcc compiler warning. (authored by plotfi).
[NFC] Fixing a gcc compiler warning.
Sep 8 2020, 4:45 PM

Sep 3 2020

plotfi added a comment to D66029: llvm-canon.

@mpaszkowski ping

Sep 3 2020, 11:42 PM · Restricted Project
plotfi added a comment to D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods..

I've updated the diff to reflect the alternate non-wrapper exposure. This way requires the sanitizing of the exported objc_direct method name.

Sep 3 2020, 7:39 PM · Restricted Project, Restricted Project
plotfi updated the diff for D86952: [MIRVRegNamer] MachineInstr StableHashing..
Sep 3 2020, 10:48 AM · Restricted Project
plotfi added a comment to D86952: [MIRVRegNamer] MachineInstr StableHashing..

Oh wait the tests already test what I was concerned about. LGTM with minor nits.

(although decoupling the tests from mir-canon would be nice, since this is supposed to be standalone)

Sep 3 2020, 10:26 AM · Restricted Project
plotfi added a comment to D86952: [MIRVRegNamer] MachineInstr StableHashing..

Is there any way to test the hash itself on certain instructions?

e.g. verify that two instructions with different address spaces get a different hash?

Sep 3 2020, 10:06 AM · Restricted Project