<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 01/04/2015 05:32 PM, Chandler
Carruth wrote:<br>
</div>
<blockquote
cite="mid:CAGCO0Khq+tGipYBwGR-MCYouZyLu42ce5KEZh0TgBVccWZGcOw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Sun, Jan 4, 2015 at 4:56 PM,
Philip Reames <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:listmail@philipreames.com" target="_blank">listmail@philipreames.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Chandler,
this is large enough it really should have gone for
review. At minimum, a chance for everyone involved to
process the ownership changes. I'm not asking you to
revert, just making the point.</blockquote>
</div>
<br>
I really disagree.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">First off, this change is not actually
large in substance. Almost all of the diff is entirely
mechanical.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">For the changes to the core analysis, I
actually did discuss them very briefly with Hal on IRC, but
they didn't seem controversial enough to warrant deep review.
There were two substantive parts to it:</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">- The change to have a separate
analysis API from the pass was already discussed numerous
times in the context of the new pass manager. I don't think
its reasonable to re-hash that discussion with each analysis.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">- The change to use a vector instead of
a set of the assume intrinsics, and weak value handles rather
than self-removing callback handles in that collection. I
wouldn't have made this change at the same time at all except
that as I started to do the mechanical bit, it became obvious
that this would be a significant simplification. I would have
had to do much more work to separate them than I had to do
land them at the same time.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">I do regret the fact that a design
change only became apparent half way through a large,
mechanical refactoring to separate out the cache from the
pass. But it really didn't seem worth going through the very
significant amount of work to separate them in this instance,
and the value of separating them just didn't seem that high --
this is not a complex or subtle piece of code, and the changes
just didn't seem that interesting.</div>
</div>
</blockquote>
Chandler, I'm not going to argue with you on this one. I disagree
with your conclusions, but understand your reasoning. I think the
largest point we differ on is the effort/value trade off of
separating two distinct changes. Its not worth debating further. <br>
<blockquote
cite="mid:CAGCO0Khq+tGipYBwGR-MCYouZyLu42ce5KEZh0TgBVccWZGcOw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">I thought I had built enough trust
within the community to make this kind of change without
breaking thing significantly or causing problems, </div>
</div>
</blockquote>
This is a red herring and you know it. :) This isn't about me
trusting you at all. It's about having code which is easy to review
and understand what is changing. <br>
<blockquote
cite="mid:CAGCO0Khq+tGipYBwGR-MCYouZyLu42ce5KEZh0TgBVccWZGcOw@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="gmail_extra">and the nature of the change seems
obvious to me. You clearly disagree, but I'd like some
confirmation from others on this front. I'm going to be make
many similar changes to this one as part of porting every pass
to work with both pass managers. If they all need pre-commit
review, then frankly, it's not going to happen. I'd have
better luck creating a branch and doing it there.</div>
</div>
</blockquote>
I think you may have misunderstood my point (which probably means I
didn't make it well.) The changes to split the analysis pass has
been previously discussed and was uninteresting. That's precisely
why I would have liked you to separate the ownership change. :) I
wanted to be able to review (pre or post is a separate debate) those
changes in isolation without having to wade through the
restructuring changes. <br>
<br>
For the record, given it sounds like you had previously discussed
the interesting part of this change with Hal, I will formally
withdraw any objection.<br>
<br>
p.s. Please do not hold future analysis restructuring for review. <br>
<br>
Philip<br>
<br>
<br>
</body>
</html>