<div dir="ltr">Ah, thanks for the pointer - I was looking for the usual "invalidates 'end'" problem and the fact that the code was saving end and not recomputing it lead me to think that wasn't an issue, then. I guess this range has a blessed/universal/constant end that doesn't need to be recomputed? Ah, yep, I see it (defusechain_iterator) does, it goes to null.<br><br>Thanks for explaining/catching that!</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Mar 17, 2017 at 1:09 PM Daniel Berlin <<a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg">No.<div class="gmail_msg">This code as written is safe against the *current* use changing, but the auto version is not.</div><div class="gmail_msg">The range loop is essentially:</div><div class="gmail_msg"><code style="color:rgb(0,0,0);font-size:12.8px;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">{</b></code><br style="color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em;color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">auto && __range =</b></code> <span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">range_expression</span> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">;</b></code> <br class="gmail_msg"></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">for (auto __begin =</b></code> <i class="gmail_msg">begin_expr</i><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">,</b></code> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">__end =</b></code> <i class="gmail_msg">end_expr</i><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">;</b></code> <br class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">__begin != __end; ++__begin) {</b></code> <br class="gmail_msg"></dd></dl></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">range_declaration</span> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">= *__begin;</b></code> <br class="gmail_msg"></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">loop_statement</span> <br class="gmail_msg"></dd></dl></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">} </b></code><br class="gmail_msg"></dd></dl><p style="margin:0.4em 0px 0.5em;line-height:1.1em;color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">} </b></code></p></div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">If loop statement destroys begin, you can't move to the next thing properly (at least, for uses).</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">This would work if it was:</div><div class="gmail_msg"><div class="gmail_msg"><code style="color:rgb(0,0,0);font-size:12.8px;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">{</b></code><br style="color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em;color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">auto && __range =</b></code> <span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">range_expression</span> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">;</b></code> <br class="gmail_msg"></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">for (auto __begin =</b></code> <i class="gmail_msg">begin_expr</i><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">,</b></code> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">__end =</b></code> <i class="gmail_msg">end_expr</i><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">;</b></code> <br class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><dl style="margin-top:0.2em;margin-bottom:0.5em" class="gmail_msg"><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">__begin != __end;) {</b></code> <br class="gmail_msg"></dd></dl></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">range_declaration</span> <code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">= *__begin++;</b></code> <br class="gmail_msg"></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><span class="m_-7873747863377219159gmail-t-spar gmail_msg" style="color:rgb(128,128,128);font-style:italic;line-height:1.1em">loop_statement</span> <br class="gmail_msg"></dd></dl></dd><dd style="line-height:1.1em;margin-left:1.6em;margin-bottom:0.1em;margin-right:0px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">} </b></code><br class="gmail_msg"></dd></dl><p style="margin:0.4em 0px 0.5em;line-height:1.1em;color:rgb(0,0,0);font-family:dejavusans,"dejavu sans",arial,sans-serif;font-size:12.8px" class="gmail_msg"><code style="background-color:transparent;font-family:dejavusansmono,"dejavu sans mono",courier,monospace" class="gmail_msg"><b class="gmail_msg">} </b></code></p></div></div><div class="gmail_msg"><br class="gmail_msg"></div></div><div class="gmail_extra gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"></div></div><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg">On Fri, Mar 17, 2017 at 10:01 AM, David Blaikie via llvm-commits <span dir="ltr" class="gmail_msg"><<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br class="gmail_msg"></div></div><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><span class="gmail_msg"><div dir="ltr" class="gmail_msg">On Fri, Mar 17, 2017 at 9:57 AM Andrew Ng via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">andrewng added a comment.<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
Thanks for the review.<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
================<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:567-568<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
+ unsigned LastVReg = Last.getOperand(0).getReg();<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
+ for (auto UI = MRI->use_nodbg_begin(LastVReg),<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
+ UE = MRI->use_nodbg_end();<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
UI != UE;) {<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
----------------<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
dblaikie wrote:<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
> I think you can use:<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
><br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
> for (MachineOperand &MO : MRI->use_nodbg_operands(LastVReg))<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
><br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
> here, probably?<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
Yes, at first I thought so too, but inside the loop there is:<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
```<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
MO.setReg(First.getOperand(0).getReg());<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
```<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
which could affect the use iterator. So the original code which increments the use iterator before any changes is safer.<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg"></blockquote></span><div class="gmail_msg"><br class="gmail_msg">Not quite sure how one could be safer than the other - they're the same code, aren't they? (range-for and the manually written for loop look like they have identical semantics)<br class="gmail_msg"> </div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<a href="https://reviews.llvm.org/D30835" rel="noreferrer" class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg" target="_blank">https://reviews.llvm.org/D30835</a><br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
<br class="m_-7873747863377219159m_5139403081024907060gmail_msg gmail_msg">
</blockquote></div></div>
<br class="gmail_msg"></blockquote></div></div><div class="gmail_extra gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">_______________________________________________<br class="gmail_msg">
llvm-commits mailing list<br class="gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
<br class="gmail_msg"></blockquote></div></div></blockquote></div>