<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Mon, May 15, 2017 at 8:03 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_1059760155374107994gmail-">On Mon, May 15, 2017 at 6:35 PM, Sanjay Patel <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>> wrote:<br>
> This isn't "code"; it's test coverage. In any case, I won't be able revert<br>
> this myself until tomorrow morning (mountain time zone). Since you obviously<br>
> object to this commit, can you revert it for me?<br>
><br>
> note that in addition to a potential transform, these tests add coverage for<br>
> a potential infinite loop from conflicting InstCombines. Having encountered<br>
> more than my fair share of those, I'd like to see far more of these negative<br>
> regression tests, so I hit them during development rather than via bug<br>
> report.<br>
><br>
<br>
</span>There's no rush reverting this, as it's not breaking things.<br>
I'm fine with leaving the tests for the infinite loop, but please add<br>
a comment explaining that.<br></blockquote><div><br><br>Patch posted for review:<br><a href="https://reviews.llvm.org/D33242" target="_blank">https://reviews.llvm.org/<wbr>D33242</a><br><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I'd appreciate if we can continue the discussion on whether this<br>
transformation should live on phabricator before we go forward with<br>
this.<br></blockquote><div><br></div><div>Sure - it wasn't my intention to imply that we are adding InstCombine folds based on these tests, but I can see how that might be assumed. I'll add more commentary with similar commits. In many cases like this, I have tried to improve the LLVM regression test directory cultural norms by using/requesting meaningful names for files and tests and/or adding comments, but recent commits show that "foo" and "poo" without any explanation are still accepted.<br><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
--<br>
Davide<br>
</blockquote></div><br></div></div>