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

On Sep 17, 2012, at 12:01 PM, Duncan Sands <baldrick at free.fr> wrote:
> 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.
I am also CC-ing LLVM-dev. 

Currently we have the flag "ProtectFromEscapedAllocas" which decides
if we want to detect obvious violations of the lifetime zone and disable the
coloring for that specific zone.

I agree with what you are saying and I intend to move forward with the plan we
agreed upon, which is, (1) disable the flag and find out what breaks, (2) fix
the broken user code or compiler passes, (3) repeat.

We also have three more things that we need to do:
1. We need to remove the inliner hack. I found that the StackColoring recovers
99% of the memory that is saved by the hack, but there are a few programs which
still lose (allocate more stack space) when removing the hack.
2. We need to add a clang flag for disabling stack coloring.
3. We need to modify clang to generate more lifetime markers, not only when
inlining.

> 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!

Thanks,
Nadav