RFR(M): Tiered: incorrect results in VM tests stringconcat with -Xcomp -XX:+DeoptimizeALot on solaris-amd64

[Christian Thalinger]

src/share/vm/opto/stringopts.cpp:

+   // No interesing paths

Typo.

Otherwise this looks good.

On Oct 10, 2013, at 2:56 PM, Igor Veresov <igor.veresov at oracle.com> wrote:

> Thanks for the review!
> 
> On Oct 9, 2013, at 10:57 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
>> +   // Find if any path from start_mem to any node is end_set has side effects.
>>                                          typo    ^ is -> in
>> 
> yup
> 
>> Can you use set() method instead visited <<= start_mem->_idx? It is the same but more easy to understand.
>> 
> 
> ok, and I've also added remove() to VectorSet for symmetry. 
> 
>> General note. Instead of checking memory between all possible combinations of append calls can you check only between final call toString and first call to StringBuffer? It should cover all.
> 
> ok
> 
>> 
>> I think in product VM you can bailout immideately when you see store and loadstore.
>> 
> 
> That's a good point, added that. I moved the path printing for debugging in that branch as well - I'm not quite comfortable having different logic in debug in product. So the path printout is going to terminate with the found side effect node (store or loadstore).
> 
>> Instead of next_edge + path you can use Node_Stack class specifically designed for such cases.
>> See example in PhaseCFG::schedule_early().
>> 
> 
> That's really useful! I haven't noticed that data structure. Refactored the code to use it.
> 
>> I think you need to assert assert for nodes you skip - closing } should be } else { assert() } to make sure you did not miss some nodes which affect memory.
>> 
> 
> ok
> 
> I've updated the webrev in-place: http://cr.openjdk.java.net/~iveresov/8009303/webrev/
> 
> 
> igor
> 
>> thanks,
>> Vladimir
>> 
>> On 10/9/13 7:39 PM, Igor Veresov wrote:
>>> I've update the code a bit. I decided to share the arrays I use for traversal between the calls to path_has_side_effects().
>>> 
>>> On Oct 9, 2013, at 3:11 PM, Igor Veresov <igor.veresov at oracle.com> wrote:
>>> 
>>>> String concat optimization optimization collapses the pattern
>>>> 
>>>> StringBuffer x = new StringBuffer();
>>>> x.append(y);
>>>> x.append(z);
>>>> x.toString();
>>>> 
>>>> into a single allocation of a string and forming the result directly. All possible deopts that may happen in the optimized code restart this pattern from the beginning (starting from the StringBuffer allocation). That means that the whole pattern must me side-effect free.  Existing code verifies that the pattern is sound from the control flow point of view, but omits the memory flow verification. This leaves a possibility for a computation on an argument of append() to have side effects. The fix adds memory flow analysis to find the possibly offending stores. We iterate up the memory graph (doing DFS) finding all possible simple paths starting from an append call, until we see the next append call, or a StringBuffer constructor.  Then we analyze the path to see if it contains side effects. The analysis is exponential, so we limit the number of iterations.
>>>> 
>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8009303/webrev/
>>>> 
>>>> Testing: eyeballing the graph, the failing tests, jprt
>>>> 
>>>> igor
>>> 
>