<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from rtf -->
<style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<font face="Calibri" size="2"><span style="font-size:11pt;">
<div> </div>
<div> </div>
<div>  </div>
<div> </div>
<div>Vladimir.</div>
<div> </div>
<div>It is a good explanation, thank you.</div>
<div> </div>
<div>I clearly remember I was debugging this code (attached) and it was hitting lines with _invar != NULL, but now I changed something in java example<font color="#1F497D"> </font>source and the _invar is just always NULL in the debugger... </div>
<div> </div>
<div>I’ll try to figure out how it was before and if I find the java code on which _invar was not NULL, I’ll send it to you.<font color="#1F497D"> </font>Otherwise I will remove my change.</div>
<div> </div>
<div>Please notice that this java example is good to demonstrate that this change is important for the vectorization (though I probably should remove the comment):</div>
<div><font face="Consolas" size="1" color="blue"><span style="font-size:8pt;"><b>if</b><font color="black"> (</font><font color="navy">tmp</font><font color="black">.</font><font color="navy">_invar</font><font color="black"> == </font><font color="navy">NULL</font><font color="black">
) || </font><font color="navy">_slp</font><font color="black">-></font><font color="navy">do_vector_loop</font><font color="black">()) { </font><font color="green">//I do not know, why tmp._invar == NULL was here at first hand</font></span></font></div>
<div> </div>
<div> </div>
<div>Here is the code where <font color="#1F497D">I</font> used to see that JVM is not recognizing the invariants (but as I said I have already modified it and now _invar != NULL does not occurs)<font color="#1F497D"> </font></div>
<div>Here in method <font face="Courier New" size="1"><span style="font-size:8pt;">aYb</span></font>  the expressions <font face="Courier New" size="1"><span style="font-size:8pt;">i*cols</span></font> and <font face="Courier New" size="1"><span style="font-size:8pt;">j*cols
</span></font>are actually invariants and set in the caller method  <font face="Courier New" size="1"><span style="font-size:8pt;">multiply_transpose</span></font>:</div>
<div> </div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">    static double aYb(double[] left, double[] right, int cols, int i, int j) {</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        double sum = 0;</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        for (int k = 0; k < cols; k++) {</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">            sum += left[k + i * cols] * right[k + j * cols];</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        }</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        return sum;</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">    }</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;"> </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">When it was called from </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">    static void multiply_transpose(double[] result, double[] left, double[] right, int cols, double[] T) {</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        assert (left.length == right.length);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        assert (left.length == cols * cols);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        assert (result.length == cols * cols);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        transpose(right, T, cols);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;"> </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //IntStream.range(0, cols * cols).parallel().forEach(id -> {  </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //    int i = id / cols;  </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //    int j = id % cols;  </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //    double sum = aYb(left, T, cols, i, j);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //    result[i * cols + j] = sum;  </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        //}); </span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        for (int id = 0; id < cols * cols; id++) {</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">            int i = id / cols;</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">            int j = id % cols;</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">            double sum = aYb(left, T, cols, i, j);</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">            result[i * cols + j] = sum;</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">        }</span></font></div>
<div><font face="Courier New" size="1"><span style="font-size:8pt;">    }</span></font></div>
<div> </div>
<div> </div>
<div>Thanks,</div>
<div> </div>
<div>Jan.</div>
<div> </div>
<div> </div>
<div> </div>
<div>-----Original Message-----</div>
<div>From: Vladimir Kozlov [<a href="mailto:vladimir.kozlov@oracle.com"><font color="#0563C1"><u>mailto:vladimir.kozlov@oracle.com</u></font></a>] </div>
<div>Sent: Thursday, June 18, 2015 5:52 PM</div>
<div>To: <a href="mailto:hotspot-compiler-dev@openjdk.java.net"><font color="#0563C1"><u>hotspot-compiler-dev@openjdk.java.net</u></font></a></div>
<div>Cc: Civlin, Jan</div>
<div>Subject: Re: RFR(S): 8085932: Fixing bugs in detecting memory alignments in SuperWord</div>
<div> </div>
<div>Jan,</div>
<div> </div>
<div>Here is why next code return false:</div>
<div> </div>
<div>   if (_scale != 0) {</div>
<div>     return false;  // already found a scale</div>
<div> </div>
<div>   if (_invar != NULL) return false; // already have an invariant</div>
<div> </div>
<div> </div>
<div>SWPointer() method tries to set _scale, _offset, _invar values.</div>
<div>But, for example, simple array access address uses 2 AddP nodes and each of them has offsets but different offsets. Usually one have invariant offset and another - scaled index:</div>
<div> </div>
<div>   AddP (base, base, iv*scale + offset)</div>
<div>   AddP (base, addp, invar)</div>
<div> </div>
<div>SWPointer() iterates over all AddP:</div>
<div> </div>
<div>   for (int i = 0; i < 3; i++) {</div>
<div>     if (!scaled_iv_plus_offset(adr->in(AddPNode::Offset))) {</div>
<div>       assert(!valid(), "too complex");</div>
<div>       return;</div>
<div>     }</div>
<div>     adr = adr->in(AddPNode::Address);</div>
<div>     if (base == adr || !adr->is_AddP()) {</div>
<div>       break; // stop looking at addp's</div>
<div>     }</div>
<div>   }</div>
<div> </div>
<div>And this code assumes only one of AddP can set those fields (_scale, _offset, _invar). If second AddP tries to set a field which is set by previous AddP it is considered complex address expression, for example:</div>
<div> </div>
<div>   AddP (base, base, iv*scale + offset_con + invar1)</div>
<div>   AddP (base, addp, invar2)</div>
<div> </div>
<div>and such cases are skipped.</div>
<div> </div>
<div>Please, show your case for which you want to return 'true'.</div>
<div> </div>
<div>Thanks,</div>
<div>Vladimir</div>
<div> </div>
<div>On 6/18/15 5:10 PM, Vladimir Kozlov wrote:</div>
<div>> Thank you, Jan</div>
<div>></div>
<div>> Fixes looks good but it would be nice if you replaced some tracing </div>
<div>> code with functions calls. In some place the execution code is hard to </div>
<div>> read because of big tracing code. For example, in</div>
<div>> SuperWord::memory_alignment() and in SWPointer methods.</div>
<div>></div>
<div>> The one way to do that is to declare trace methods with empty body in </div>
<div>> product build, for example for SWPointer::scaled_iv_plus_offset() you </div>
<div>> may have new method declaration (not under #ifdef) in  superword.hpp:</div>
<div>></div>
<div>>    class SWPointer VALUE_OBJ_CLASS_SPEC {</div>
<div>></div>
<div>>      void trace_1_scaled_iv_plus_offset(...) PRODUCT_RETURN;</div>
<div>></div>
<div>> and in superword.cpp you will put the method under ifdef:</div>
<div>></div>
<div>> #ifndef PRODUCT</div>
<div>>    void trace_1_scaled_iv_plus_offset(...) {</div>
<div>>      ....</div>
<div>>    }</div>
<div>> #endif</div>
<div>></div>
<div>> Then you can simply use it without ifdefs in code:</div>
<div>></div>
<div>>   bool SWPointer::scaled_iv_plus_offset(Node* n) {</div>
<div>> +  trace_1_scaled_iv_plus_offset(...);</div>
<div>> +</div>
<div>>     if (scaled_iv(n)) {</div>
<div>></div>
<div>> Note, macro PRODUCT_RETURN is defined as:</div>
<div>></div>
<div>> #ifdef PRODUCT</div>
<div>> #define PRODUCT_RETURN  {}</div>
<div>> #else</div>
<div>> #define PRODUCT_RETURN  /*next token must be ;*/ #endif</div>
<div>></div>
<div>> Thanks,</div>
<div>> Vladimir</div>
<div>></div>
<div>> On 6/8/15 9:15 AM, Civlin, Jan wrote:</div>
<div>>> Hi All,</div>
<div>>></div>
<div>>></div>
<div>>>   We would like to contribute to Fixing bugs in detecting memory</div>
<div>>>   alignments in SuperWord.</div>
<div>>></div>
<div>>> The contribution Bug ID: 8085932.</div>
<div>>></div>
<div>>> Please review this patch:</div>
<div>>></div>
<div>>> Bug-id: <a href="https://bugs.openjdk.java.net/browse/JDK-8085932"><font color="#0563C1"><u>https://bugs.openjdk.java.net/browse/JDK-8085932</u></font></a></div>
<div>>></div>
<div>>> webrev: <a href="http://cr.openjdk.java.net/~kvn/8085932/webrev.00/"><font color="#0563C1"><u>http://cr.openjdk.java.net/~kvn/8085932/webrev.00/</u></font></a></div>
<div>>></div>
<div>>></div>
<div>>>       *Description**: *Fixing bugs in detecting memory alignments in</div>
<div>>>       SuperWord</div>
<div>>></div>
<div>>> Fixing bugs in detecting memory alignments in SuperWord:</div>
<div>>> SWPointer::scaled_iv_plus_offset (fixing here a bug in detection of </div>
<div>>> "scale"), SWPointer::offset_plus_k (fixing here a bug in detection of </div>
<div>>> "invariant"),</div>
<div>>></div>
<div>>> Add tracing output to the code that deal with memory alignment. The </div>
<div>>> following routines are traceable:</div>
<div>>></div>
<div>>> SWPointer::scaled_iv_plus_offset</div>
<div>>> SWPointer::offset_plus_k</div>
<div>>> SWPointer::scaled_iv,</div>
<div>>> WPointer::SWPointer,</div>
<div>>> SuperWord::memory_alignment</div>
<div>>></div>
<div>>> Tracing is done only for NOT_PRODUCT. Currently tracing is controlled </div>
<div>>> by</div>
<div>>> VectorizeDebug:</div>
<div>>></div>
<div>>> #ifndef PRODUCT</div>
<div>>>    if (_phase->C->method() != NULL) {</div>
<div>>>         _phase->C->method()->has_option_value("VectorizeDebug",</div>
<div>>> _vector_loop_debug);</div>
<div>>>    }</div>
<div>>> #endif</div>
<div>>></div>
<div>>> And VectorizeDebug may take any combination (bitwise OR) of the </div>
<div>>> following values:</div>
<div>>> bool is_trace_alignment() { return (_vector_loop_debug & 2) > 0; } </div>
<div>>> bool is_trace_mem_slice() { return (_vector_loop_debug & 4) > 0; } </div>
<div>>> bool is_trace_loop() { return (_vector_loop_debug & 8) > 0; } bool </div>
<div>>> is_trace_adjacent() { return (_vector_loop_debug & 16) > 0; }</div>
<div>>>  </div>
<div> </div>
</span></font>
</body>
</html>