RFR(S): 8085932: Fixing bugs in detecting memory alignments in SuperWord

[Vladimir Kozlov]

Okay, this is better.

Thanks,
Vladimir

On 6/25/15 2:51 PM, Civlin, Jan wrote:
>
> Vladimir,
>
> Here is the updated patch with trace hidden in a new nested class Trace, that contains all the messages. The Trace class is compiled only in NOT_PRODUCT.
> Looks much simple now (of course more lines but all outside of the algorithmic part).
>
> Thank you,
>
> Jan.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Thursday, June 18, 2015 5:11 PM
> To: Civlin, Jan; hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR(S): 8085932: Fixing bugs in detecting memory alignments in SuperWord
>
> Thank you, Jan
>
> Fixes looks good but it would be nice if you replaced some tracing code
> with functions calls. In some place the execution code is hard to read
> because of big tracing code. For example, in
> SuperWord::memory_alignment() and in SWPointer methods.
>
> The one way to do that is to declare trace methods with empty body in
> product build, for example for SWPointer::scaled_iv_plus_offset() you
> may have new method declaration (not under #ifdef) in  superword.hpp:
>
>     class SWPointer VALUE_OBJ_CLASS_SPEC {
>
>       void trace_1_scaled_iv_plus_offset(...) PRODUCT_RETURN;
>
> and in superword.cpp you will put the method under ifdef:
>
> #ifndef PRODUCT
>     void trace_1_scaled_iv_plus_offset(...) {
>       ....
>     }
> #endif
>
> Then you can simply use it without ifdefs in code:
>
>    bool SWPointer::scaled_iv_plus_offset(Node* n) {
> +  trace_1_scaled_iv_plus_offset(...);
> +
>      if (scaled_iv(n)) {
>
> Note, macro PRODUCT_RETURN is defined as:
>
> #ifdef PRODUCT
> #define PRODUCT_RETURN  {}
> #else
> #define PRODUCT_RETURN  /*next token must be ;*/
> #endif
>
> Thanks,
> Vladimir
>
> On 6/8/15 9:15 AM, Civlin, Jan wrote:
>> Hi All,
>>
>>
>>    We would like to contribute to Fixing bugs in detecting memory
>>    alignments in SuperWord.
>>
>> The contribution Bug ID: 8085932.
>>
>> Please review this patch:
>>
>> Bug-id: https://bugs.openjdk.java.net/browse/JDK-8085932
>>
>> webrev: http://cr.openjdk.java.net/~kvn/8085932/webrev.00/
>>
>>
>>        *Description**: *Fixing bugs in detecting memory alignments in
>>        SuperWord
>>
>> Fixing bugs in detecting memory alignments in SuperWord:
>> SWPointer::scaled_iv_plus_offset (fixing here a bug in detection of
>> "scale"),
>> SWPointer::offset_plus_k (fixing here a bug in detection of "invariant"),
>>
>> Add tracing output to the code that deal with memory alignment. The
>> following routines are traceable:
>>
>> SWPointer::scaled_iv_plus_offset
>> SWPointer::offset_plus_k
>> SWPointer::scaled_iv,
>> WPointer::SWPointer,
>> SuperWord::memory_alignment
>>
>> Tracing is done only for NOT_PRODUCT. Currently tracing is controlled by
>> VectorizeDebug:
>>
>> #ifndef PRODUCT
>>     if (_phase->C->method() != NULL) {
>>          _phase->C->method()->has_option_value("VectorizeDebug",
>> _vector_loop_debug);
>>     }
>> #endif
>>
>> And VectorizeDebug may take any combination (bitwise OR) of the
>> following values:
>> bool is_trace_alignment() { return (_vector_loop_debug & 2) > 0; }
>> bool is_trace_mem_slice() { return (_vector_loop_debug & 4) > 0; }
>> bool is_trace_loop() { return (_vector_loop_debug & 8) > 0; }
>> bool is_trace_adjacent() { return (_vector_loop_debug & 16) > 0; }
>>
>