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

Hi,

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