JDK-7032963 : StoreCM shouldn't participate in store elimination
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: hs21
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: solaris_10
  • CPU: x86
  • Submitted: 2011-03-31
  • Updated: 2011-07-29
  • Resolved: 2011-05-10
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 Other
7Fixed hs21Fixed
Description
While looking at the code we generate for a simple program I noticed that we were eliminating card marks with CMS in a illegal fashion.  For this simple program:

public class test {
   static Object a1;
   static Object a2;
   static Object a3;
   public static void main(String[] args) {
       a1 = args;
       a2 = args;
       a3 = args;
   }
}

we generate this:

000   B1: #     N1 <- BLOCK HEAD IS JUNK   Freq: 1
000     SAVE   R_SP,-72,R_SP
004     SET    precise klass test: 0x00843a50:Constant:exact *,R_L0     !ptr
00c +   SET    0xf92d4000,R_L2  !ptr
010 +   STW    R_I0,[R_L0 + #352]       ! ptr ! Field test.a3
014 +   STW    R_I0,[R_L0 + #348]       ! ptr ! Field test.a2
018 +   SRL    R_L0,#9,R_L1     ! Cast ptr R_L0 to int and shift
01c     STW    R_I0,[R_L0 + #344]       ! ptr ! Field test.a1
020 +   STB    #0,[R_L2 + R_L1] ! CMS card-mark byte 0
024     SETHI  #PollAddr,L0     ! Load Polling address
       LDUW   [L0],G0  !Poll for Safepointing
       RET
       RESTORE
034 +   ! return
034

which happens to be ok because the STB happens last but we don't actually have enough dependences to ensure that we get this schedule.  We emit 3 separate StoreCMs for each field references and each one has a dependence on the store that it covers. What's going wrong is that we're allowing StoreCM to participate in the the store elimination in StoreNode::Ideal so we end up with this:

45     StoreP  ===  5  7  44  10  [[ 16  50 ]]  @precise klass test: 0x00843a50:Constant:exact+352 *, name=a3, idx=6; Memory: @precise klass test: 0
x00843a50:Constant:exact+352 *, name=a3, idx=6; !jvms: test::main @ bci:9
36     StoreP  ===  5  7  35  10  [[ 16 ]]  @precise klass test: 0x00843a50:Constant:exact+348 *, name=a2, idx=5;  Memory: @precise klass test: 0x008
43a50:Constant:exact+348 *, name=a2, idx=5; !jvms: test::main @ bci:5
25     StoreP  ===  5  7  24  10  [[ 16 ]]  @precise klass test: 0x00843a50:Constant:exact+344 *, name=a1, idx=4;  Memory: @precise klass test: 0x008
43a50:Constant:exact+344 *, name=a1, idx=4; !jvms: test::main @ bci:1
50     StoreCM ===  5  7  31  23  45  [[ 16 ]]  @rawptr:BotPTR, idx=Raw;  Memory: @rawptr:BotPTR, idx=Raw; !jvms: test::main @ bci:9

The store to a3 has a StoreCM and it has killed the StoreCMs for a1 and a2 but it doesn't have a dependence on it.  Since the slices are independent the a3 operations could be scheduled before the store to a1 and a2.  It may be that this is rare in practice and/or the local schedule tends to put the StoreCM last but it's clearly wrong.  G1 is safe from this because the StoreCMs are never close enough to be eliminated.

It's easy to fix but it might hurt CMS performance a bit.  It could also be done safely if the StoreCM could have dependences on multiple stores but since it's not using normal precedence edges I'm not sure how this would be implemented.

Does this sound like any issues that have been seen in the past?

Comments
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/e6beb62de02d
23-04-2011

EVALUATION 7032963: StoreCM shouldn't participate in store elimination Reviewed-by: kvn StoreCM shouldn't participate in redundant store elimination since that could violate the requirement that a StoreCM must be strictly after a field update. This results in a large number of redundant StoreCMs being emitted for blocks of fields updates, so I added an optimization to fold them up safely. Previously the extra dependence was converted into a precedence edge just before register allocation but I moved this logic into final_graph_reshape. I then added logic to search through chains of StoreCMs to eliminate earlier redundant ones and transfer their precedence edges to the one that is kept. This ensures that they are scheduled properly. This actually eliminates duplicates that were previously missed so the code quality is slightly better. Tested by inspecting code generation with script to identify duplicates. Also ran CTW with -XX:+UseCondCardMark and -XX:+UseG1GC.
06-04-2011

EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/e6beb62de02d
06-04-2011