<div dir="auto">Ack, I will update with the test from <a href="https://reviews.llvm.org/D67296">https://reviews.llvm.org/D67296</a> on Monday.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 6, 2019, 11:08 PM George Burgess IV via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">george.burgess.iv added a comment.<br>
<br>
Thanks for this!<br>
<br>
+1 for a simple -disable-basicaa test please.<br>
<br>
<br>
<br>
================<br>
Comment at: lib/Analysis/MemorySSA.cpp:287<br>
     case Intrinsic::assume:<br>
+    case Intrinsic::dbg_addr:<br>
+    case Intrinsic::dbg_declare:<br>
----------------<br>
this should be dynamically dead code, no? `II` is sourced from `MD`, so there has to be an existing `Def` for these. if we're special-casing elsewhere to ensure these `Def`s are never created, ...<br>
<br>
(if you'd rather make these new cases lead to `assert(false && "debuginfo shouldn't have associated defs!");` or similar, i'm equally content with that)<br>
<br>
<br>
================<br>
Comment at: lib/Analysis/MemorySSA.cpp:1734<br>
   // FIXME: Replace this special casing with a more accurate modelling of<br>
   // assume's control dependency.<br>
   if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I))<br>
----------------<br>
nit: please include a note saying something like debuginfo intrinsics may be considered clobbers when we have a nonstandard AA pipeline, so we special-case them<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D67307/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D67307/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D67307" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D67307</a><br>
<br>
<br>
<br>
</blockquote></div>