[llvm-commits] [llvm] r163678 - in /llvm/trunk: lib/CodeGen/StackColoring.cpp test/CodeGen/X86/StackColoring.ll

[Duncan Sands]

Hi,

On 17/09/12 05:58, Daniel Berlin wrote:
> On Sat, Sep 15, 2012 at 9:03 AM, Nadav Rotem <nrotem at apple.com> wrote:
>>
>>
>> On Sep 15, 2012, at 14:24, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>>
>>> Wait, doesn't this mean that you don't need the lifetime markers at all??
>>> If you remove the markers that you cannot prove that are ok, then it's equivalent to not having this information at all, and then use only the computed liveness intervals.
>>>
>>
>> We use the lifetime markers to calculate the liveness intervals and throw away the intervals which are found to be broken. We need the markers to calculate the intervals.
>
> As Duncan asked, shouldn't you just fix the producers of such broken
> intervals (be they compilers or user code)?
> For example, if someone returns a reference to a local variable,
> beside being undefined, the code will often get broken by other
> optimizations anyway.
>
> This won't be a surprise either, since both clang and GCC warn about
> it by default in the case of returning a reference to a local.
>
> Besides Duncan's concern about cost, note that there are always
> wonderfully exciting ways people will come up with broken/illegal code
> that "breaks" a particular pass, so if you go down this route, you may
> find other things you have to check for as well.  As they say, it's
> impossible to make anything foolproof, because fools are so damn
> ingenious.

what's more, it is possible to have a memory access outside the lifetime
markers for correct code, when that code is unreachable.

If the point of view (taken by the rest of the compiler) is that IR is
correct and that that fact should be exploited maximally, then when you
see a "broken" memory access you would say: "great, this is telling me
that this memory access is unreachable due to some complicated reasons
that were beyond our power to analyse previously, let me replace it with
unreachable for a nice code size reduction and potential speedup".

Instead you are penalizing correct IR (by not doing this optimization) and
instead you're trying to sweep wrong IR under the carpet where no one will
notice.  That's a losing game in my opinion.

Indeed, replacing such memory accesses by unreachable is also a good thing
for incorrect IR: codegen will turn that into a "trap" instruction, and
users (and compiler writers) will quickly discover the mistake in their code
and fix it [*].  Or if they aren't willing to fix it then at least they will
quickly find out what the problem is and turn off stack colouring via the
appropriate gcc/clang option.

Ciao, Duncan.

[*] You can even market this as a feature, since using temporaries beyond
their lifetimes is a common and very nasty mistake - but now clang will
tell you about it by causing a trap to occur!