<div dir="ltr"><div>1 week ping!</div><div><br></div><div>+Eric Christopher, I'd also appreciate a review.</div><div><br></div>Hal, do you have any further review comments? (Does it look good to you, but you don't feel like you have sufficient knowledge to OK a commit?)<div>
<br></div><div>I think that between the three of us, Hal, Eric and myself, that would be sufficient eyes to approve the patch for commit.</div><div><br></div><div>As a note to everybody else, this is likely to break out of tree targets when it lands. My current plan is to send an email to llvm-dev shortly before committing this patch, after I get the OK to commit.</div>
<div><br></div><div>Nick</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 24 February 2014 17:19, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">+cc Evan Cheng<div><br></div><div>Evan, I'm not sure whether you're still the right person to review this. I consider it a substantive change to how the backend works, even though the line count of the patch is small. If nothing else, I figure you want to be aware of this patch before it lands.<span class="HOEnZb"><font color="#888888"><br>
<div class="gmail_extra"><br></div></font></span><div class="gmail_extra"><span class="HOEnZb"><font color="#888888">Nick</font></span><div><div class="h5"><br><br><div class="gmail_quote">On 24 February 2014 17:01, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On 24 February 2014 16:45, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>----- Original Message -----<br>
> From: "Nick Lewycky" <<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>><br>
</div><div>> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> Cc: "Commit Messages and Patches for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br>
> Sent: Monday, February 24, 2014 6:31:34 PM<br>
> Subject: Re: patch: remove PseudoSourceValue from Value.<br>
><br>
><br>
><br>
><br>
> On 24 February 2014 16:20, Hal Finkel < <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> > wrote:<br>
><br>
><br>
> What is the underlying motivation for doing this?<br>
><br>
><br>
><br>
> The proximate cause is that I'm trying to remove the vtable from<br>
> llvm::Value, and in order to do that I'm going to write dispatch<br>
> functions that switch on the ValueID and call the appropriate<br>
> function from the most-derived-type. If I do that, then we have<br>
> lib/IR placing calls to lib/CodeGen, which is a layering violation.<br>
><br>
><br>
> At a higher level, PSV does not belong there. It is not part of the<br>
> IR. No IR passes use or understand it. It's not documented in the<br>
> LangRef. Frankly, it's a hack -- and it's a great hack that and I<br>
> understand its utility for lib/CodeGen -- but that doesn't change<br>
> the fact that it's not part of llvm's IR. I've just never been<br>
> motivated to do something about it until now.<br>
<br>
</div>Thanks for explaining!<br>
<br>
AliasAnalysis::AliasResult AAResult = AA->alias(<br>
- AliasAnalysis::Location(MMOa->getValue(), Overlapa,<br>
- UseTBAA ? MMOa->getTBAAInfo() : 0),<br>
- AliasAnalysis::Location(MMOb->getValue(), Overlapb,<br>
- UseTBAA ? MMOb->getTBAAInfo() : 0));<br>
+ AliasAnalysis::Location(MMOa->getValue(), Overlapa,<br>
+ UseTBAA ? MMOa->getTBAAInfo() : 0),<br>
+ AliasAnalysis::Location(MMOb->getValue(), Overlapb,<br>
+ UseTBAA ? MMOb->getTBAAInfo() : 0));<br>
<br>
This is purely an indentation change, can you please commit this separately?<br></blockquote><div><br></div></div></div><div>Done.</div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- MapVector<const Value *, std::vector<SUnit *> >::iterator IE =<br>
+ MapVector<PointerUnion<const Value *, const PseudoSourceValue *>,<br>
+ std::vector<SUnit *> >::iterator IE =<br>
<br>
These are starting to get really ugly. Can you please do something so that we don't have to use these really long local types in many places (like make a local typedef)?<br></blockquote><div><br></div></div><div>Done. Updated patch attached.</div>
<span><font color="#888888">
<div><br></div><div>Nick</div><div><br></div></font></span></div><br></div></div>
</blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div>