[PATCH] D26967: Put opt-viewer critical items in parallel

Mon Jan 23 08:29:08 PST 2017

anemet added a comment.

HI Brian!

In https://reviews.llvm.org/D26967#653334, @bcain wrote:

> If the .key property of Remark would be better off by having a self.Function element, should I just add it?

I think we should just go with your patch as is and incrementally improve the situation from there.  As I said the original version is also incorrect so we're technically not regressing anything here.

I have been holding off commits to avoid causing merge conflicts for you but the queue is getting pretty long; I'd like to get this in and go from there.

> Also: note that this parallel implementation spawns processes, not threads.  This means that globally visible changes like e.g. class-members have to be manually delivered back to the parent.  This is why the Remark.max_hotness is not updated in get_remarks().   Changing this implementation to be multithreaded is simple and only requires slightly different imports.
> 
>> If you want to see an example of this, diff _Users_adam_proj_org_llvm_build-rel_bin_.._include_c++_v1___functional_base.html between html-orig and html-par from the tarball I sent you.   On the very first line we annotate (line 63), there are licm remarks with different inlining contexts on the parallel version but not on the serial one.
> 
> I'll see if I can make a minimal reproducer, sorting through all of these differences is difficult to reason about IMO.

I wouldn't worry about this.  Why don't you just post the latest version of the patch and then we improve/fix thing further in tree.

Thanks again for your work.

Adam

Repository:
  rL LLVM

https://reviews.llvm.org/D26967