[aarch64-port-dev ] Assertion failures in mauve

[Andrew Haley]

On 08/09/2013 10:10 AM, Edward Nevill wrote:
> On Tue, 2013-08-06 at 11:21 +0100, Andrew Haley wrote:
>> On 08/06/2013 11:03 AM, Edward Nevill wrote:
>>>
>>> #  Internal Error (/home/ed/work/ssl/jdk8/hotspot/src/cpu/aarch64/vm/assembler_aarch64.cpp:1439), pid=16692, tid=139736898889472
>>> #  assert(sets_flags || uimm || Rd != Rn) failed: insn is a NOP
>>>
>>> The failing tests are
>>>
>>> java.awt.Label.PaintTest
> 
> [17 more failing tests deleted]
> 
>> That depends on why we're generating the NOP.  Can you have a look,
>> please?
> 
> All 18 test cases fail when trying to compile the same method
> 
>   37808 1980   !b        java.awt.FlowLayout::preferredLayoutSize (274 bytes)
> 
> Here is the relevant section of code from jdk/src/share/classes/java/awt/FlowLayout.java ([...] indicates irrelevant sections of code which have been elided).
> 
>     public Dimension preferredLayoutSize(Container target) {
> [...]
>         int maxAscent = 0;
>         int maxDescent = 0;
> [...]
>                     int baseline = m.getBaseline(d.width, d.height);
>                     if (baseline >= 0) {
>                         maxAscent = Math.max(maxAscent, baseline);
>                         maxDescent = Math.max(maxDescent, d.height - baseline);
>                     }
> [...]
>             dim.height = Math.max(maxAscent + maxDescent, dim.height);
> [...]
>     }
> 
> Now, m.getBaseline is inlined and either returns -1, or throws an exception. Therefore the code following is dead code eliminated. maxAscent and maxDescent are then constant propagated.
> 
> The following is the LIR generated.
> 
> +PrintIRWithLIR
> 
>   192  1    i102   i24 + i24
>      move [int:0|I] [R156|I]
>      add [R156|I] [int:0|I] [R157|I]
> 
> which after register allocation becomes
> 
> +PrintLIR
> 
>  222 move [int:0|I] [c_rarg0|I]
>  224 add [c_rarg0|I] [int:0|I] [c_rarg0|I]
> 
> which causes the assert failure above.
> 
> My own feeling is that we should fix this by silently suppressing the generation of the NOP in the back end. Trying to fix the in the LIR is going to lead to non trivial changes in shared code.

I don't think we can do that in the assembler, because I think that

   add	w1, w1, 0

is not a NOP: it sets the top bits of X1.

Fixed thusly.

Andrew.


changeset:   4817:62c56934d893
tag:         tip
parent:      4814:074ae524f3e0
user:        aph
date:        Fri Aug 09 14:41:12 2013 +0100
summary:     C1: don't generate code for add rN, rN, #0.

diff -r 074ae524f3e0 -r 62c56934d893 src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Wed Aug 07 17:22:00 2013 +0100
+++ b/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp	Fri Aug 09 14:41:12 2013 +0100
@@ -1661,6 +1661,11 @@
       // cpu register - constant
       jint c = right->as_constant_ptr()->as_jint();

+      assert(code == lir_add || code == lir_sub, "mismatched arithmetic op");
+      if (c == 0 && dreg == lreg) {
+	COMMENT("effective nop elided");
+	return;
+      }
       switch(left->type()) {
       case T_INT:
 	switch (code) {
@@ -1685,12 +1690,10 @@

   } else if (left->is_double_cpu()) {
     Register lreg_lo = left->as_register_lo();
-    Register lreg_hi = left->as_register_hi();

     if (right->is_double_cpu()) {
       // cpu register - cpu register
       Register rreg_lo = right->as_register_lo();
-      Register rreg_hi = right->as_register_hi();
       switch (code) {
       case lir_add: __ add (dest->as_register_lo(), lreg_lo, rreg_lo); break;
       case lir_sub: __ sub (dest->as_register_lo(), lreg_lo, rreg_lo); break;
@@ -1703,9 +1706,15 @@

     } else if (right->is_constant()) {
       jlong c = right->as_constant_ptr()->as_jlong_bits();
+      Register dreg = as_reg(dest);
+      assert(code == lir_add || code == lir_sub, "mismatched arithmetic op");
+      if (c == 0 && dreg == lreg_lo) {
+	COMMENT("effective nop elided");
+	return;
+      }
       switch (code) {
-        case lir_add: __ add(dest->as_register_lo(), lreg_lo, c); break;
-        case lir_sub: __ sub(dest->as_register_lo(), lreg_lo, c); break;
+        case lir_add: __ add(dreg, lreg_lo, c); break;
+        case lir_sub: __ sub(dreg, lreg_lo, c); break;
         default:
           ShouldNotReachHere();
       }