<div dir="ltr">Thanks for the explanation Daniel. If it's the case we can occasionally expect invalid IR then I'd like to move forward with reviewing this patch to fix D34135. What is the process for me to follow?</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 6, 2018 at 4:38 PM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Tue, Mar 6, 2018 at 2:18 PM, Brian Rzycki via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">brzycki added a comment.<br>
<br>
I have analyzed this failure and have what I believe (in @dberlin 's words) to be the smallest hammer fix:<br>
<br>
  $ git diff lib/Analysis/LazyValueInfo.cpp<br>
  diff --git a/llvm/lib/Analysis/LazyValueI<wbr>nfo.cpp b/llvm/lib/Analysis/LazyValueI<wbr>nfo.cpp<br>
  index 65fd007dc0b..36e66f7f6ef 100644<br>
  --- a/llvm/lib/Analysis/LazyValueI<wbr>nfo.cpp<br>
  +++ b/llvm/lib/Analysis/LazyValueI<wbr>nfo.cpp<br>
  @@ -1145,9 +1145,17 @@ getValueFromConditionImpl(Valu<wbr>e *Val, Value *Cond, bool isTrueDest,<br>
                (!isTrueDest && BO->getOpcode() != BinaryOperator::Or))<br>
       return ValueLatticeElement::getOverde<wbr>fined();<br>
<br>
  -  auto RHS = getValueFromCondition(Val, BO->getOperand(0), isTrueDest, Visited);<br>
  -  auto LHS = getValueFromCondition(Val, BO->getOperand(1), isTrueDest, Visited);<br>
  -  return intersect(RHS, LHS);<br>
  +  // Prevent infinite recursion if Cond references itself as in this example:<br>
  +  //  Cond: "%tmp4 = and i1 %tmp4, undef"<br>
  +  //    BL: "%tmp4 = and i1 %tmp4, undef"<br>
  +  //    BR: "i1 undef"<br>
  +  Value *BL = BO->getOperand(0);<br>
  +  Value *BR = BO->getOperand(1);<br>
  +  if (BL == Cond || BR == Cond)<br>
  +    return ValueLatticeElement::getOverde<wbr>fined();<br>
  +<br>
  +  return intersect(getValueFromConditio<wbr>n(Val, BL, isTrueDest, Visited),<br>
  +                   getValueFromCondition(Val, BR, isTrueDest, Visited));<br>
   }<br>
<br>
The infinite recursion happens because `Visited` is not altered or updated until `getValueFromConditionImpl()` returns a value. In this case `getValueFromConditionImpl()` calls `getValueFromCondition()` which calls back to `getValueFromConditionImpl()` in an attempt to resolve the LHS and RHS of a valid `BinaryOperator`.<br>
<br>
In our case this specific instruction we have `%tmp4 = and i1 %tmp4, undef` which I don't quite understand. Shouldn't this be invalid SSA form?<br></blockquote><div><br></div><div><br></div></div></div><div>We allow somewhat illegal IR in unreachable blocks.</div><div>There are some mailing list discussions around this.</div><div><br></div><div>This is under the guise of it  being easier than knowing when the block is dead and removing it.<br></div><div>In my experience, it generates more bugs trying to avoid illegal IR than it is worth, and most passes end up trying to find a way to at least avoid visiting dead blocks.</div><div>(<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Some try to be resilient and visit dead blocks, but they rarely survive it over time, and you will see a whole host of bugzilla bugs in this category :P)</span></div><div><br></div><div>Handling this is much easier in non-lazy passes.</div><div><br></div></div></div></div>
</blockquote></div><br></div>