On Mon, 6 Feb 2023 09:42:43 GMT, Axel Boldt-Christmas <aboldtch@openjdk.org> wrote:
Roberto Castañeda Lozano has updated the pull request incrementally with two additional commits since the last revision:
- Add missing include - Fix include order
test/hotspot/jtreg/compiler/gcbarriers/TestZGCBarrierElision.java line 329:
327: 328: @Test 329: // The second atomic access barrier should be elided, but is not.
Is it worth explicitly documenting the behaviour and the wanted/expected behaviour in annotations? E.g.
@Test @IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "2" }, phase = CompilePhase.FINAL_CODE) @IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, ELIDED, "0" }, phase = CompilePhase.FINAL_CODE) // The second atomic access barrier should be elided, but is not. // @IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, REMAINING, "1" }, phase = CompilePhase.FINAL_CODE) // @IR(counts = { IRNode.Z_GET_AND_SET_P_WITH_BARRIER_FLAG, ELIDED, "1" }, phase = CompilePhase.FINAL_CODE) static void testAtomicThenAtomic(Outer o, Inner i) { field1VarHandle.getAndSet(o, i); field1VarHandle.getAndSet(o, i); }
Same goes for all the other test cases.
IR checks in compiler tests are commonly interpreted as documentation for expected behavior only. I fear adding IR checks for the current (unexpected) behavior would risk misleading inattentive readers, so I would rather leave it as-is if you don't mind. ------------- PR: https://git.openjdk.org/zgc/pull/12