<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 30, 2014 at 3:12 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, May 30, 2014 at 3:03 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>

> On Fri, May 30, 2014 at 3:00 PM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> wrote:<br>
>><br>
>> On Fri, May 30, 2014 at 12:45 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>
>> wrote:<br>
>>><br>
>>> On Fri, May 30, 2014 at 10:26 AM, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
>>> ><br>
>>> >> On May 29, 2014, at 10:03 PM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>><br>
>>> >> wrote:<br>
>>> >><br>
>>> >> Yeah, well, we can add "FrameTeardown" machine instruction flag in<br>
>>> >> addition to "FrameSetup", and annotate machine instructions added in frame<br>
>>> >> lowering code. But do we expect many users of this flag (assuming that this<br>
>>> >> usage is also somewhat questionable)?<br>
>>> ><br>
>>> > The FrameSetup flag is also only used by the debug info, so at least it<br>
>>> > wouldn’t be any more questionable than that :-)<br>
>><br>
>><br>
>> OK. Why don't we land the patch as-is now (don't terminate the register at<br>
>> the end of MBB if this register is only modified in frame setup and the last<br>
>> MBB - it should be safe), and then replace "the last MBB" condition with<br>
>> checking a "FrameTeardown" flag, attached to certain machine instructions?<br>
>> In this way we won't have to special-case certain registers and make<br>
>> assumptions about their constant-ness.<br>
><br>
> Given the limited nature of this fix (both in the fix's possible scope<br>
> and intent/purpose) I'm personally sort of inclined to do this by<br>
> whitelisting certain registers instead, if that were/is possible...<br>
> seems simpler and still sufficient for the required purpose here.<br>
><br>
> Can we easily just test whether a register is the stack/frame pointer?<br>
> (I actually know so little about this that the difference between<br>
> those two things isn't straight in my head and I'm not sure which, or<br>
> both, we need for the common/intended case here)<br>
><br>
<br>
</div></div>If we need to stack realign we might not be able to do this as easily.<br>
I'm not a huge fan of either method, but I think that the assumption<br>
that Alexey has might make more sense than whitelisting. My logic:<br>
Whitelisting involves assuming that various registers won't be used in<br>
ways that surprise us and I can think of a few ways where it could<br>
happen: stack realignment, inline asm, a smarter stack coloring<br>
algorithm.<br></blockquote><div><br></div><div>Stack realignment usually happens in function prologue / frame setup, so I don't</div><div>immediately see the way it would hurt us. But the inline asm case is indeed interesting.</div>
<div>I know too little of stack coloring to make a judgement. I though it just minimizes</div><div>the number of stack slots so that we can re-use the same stack slot for different</div><div>independent variables, but the actual stack pointer modifications are not performed</div>
<div>in the middle of the function, but probably this could change over time:</div><div><br></div><div>void foo() {</div><div>  {</div><div>    int a[100];</div><div>  }</div><div>  {</div><div>    int b;</div><div>    // "a" is inaccessible, why not save the stack space used to store "a"</div>
<div>    // before we call "bar" ?</div><div>    bar();</div><div>  }</div><div>}</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
>><br>
>>><br>
>>> ><br>
>>><br>
>>> :)<br>
>>><br>
>>> >> I can also make this patch even less general and just special-case the<br>
>>> >> frame pointer and the stack pointer registers, so that the code wouldn't<br>
>>> >> pretend to solve a general problem we're facing.<br>
>>> ><br>
>>> > The frame pointer should be stable over the entire function, special<br>
>>> > casing it seems to be appropriate. I don’t know whether, e.g, stack coloring<br>
>>> > would cause the stack pointer to be modified in the middle of a function.<br>
>>> ><br>
>>><br>
>>> FWIW it may not _now_ but there's nothing from stopping it either. :\<br>
>>><br>
>>> -eric<br>
>><br>
>><br>
>> --<br>
>> Alexey Samsonov<br>
>> <a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div>
</div></div>