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

[Civlin, Jan]

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; }
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: webrev.062515.tar.bz2
Type: application/octet-stream
Size: 1043695 bytes
Desc: webrev.062515.tar.bz2
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150625/db1e26c5/webrev.062515.tar-0001.bz2>