RFR (M) CR 8050147: StoreLoad barrier interferes with stack usages

[Aleksey Shipilev]

Thanks, Vladimir.

Coming back to this...

On 08/12/2014 02:30 AM, Vladimir Kozlov wrote:
> You may need to modify Compile::bang_size_in_bytes() and
> LIR_Assembler::bang_size_in_bytes() to bang correct number of pages. Try
> to test with small stacks and see
> test/compiler/uncommontrap/StackOverflowGuardPagesOff.java.
> 
> Related problems fixed recently:
> https://bugs.openjdk.java.net/browse/JDK-8026775
> https://bugs.openjdk.java.net/browse/JDK-8032410

Ugh, what a mess. I think I understand the essence for those two issues.
The test you mention there passes on current and patched builds, but
that tells me nothing.

To fix this properly, I need some guidance. If we are shooting for "lock
addl (%esp, -OFFSET)" for a StoreLoad barrier, does it mean we should
bang for (frame_size + OFFSET) bytes? If so, is this the addition we are
shooting for?

diff -r 184aac46be1f src/share/vm/c1/c1_LIRAssembler.cpp
--- a/src/share/vm/c1/c1_LIRAssembler.cpp	Sun Aug 10 19:38:53 2014 -0700
+++ b/src/share/vm/c1/c1_LIRAssembler.cpp	Tue Aug 19 14:18:12 2014 +0400
@@ -169,7 +169,7 @@
 // removes the need to bang the stack in the deoptimization blob which
 // in turn simplifies stack overflow handling.
 int LIR_Assembler::bang_size_in_bytes() const {
-  return MAX2(initial_frame_size_in_bytes(),
_compilation->interpreter_frame_size());
+  return MAX2(initial_frame_size_in_bytes() +
VM_Version::L1_line_size(), _compilation->interpreter_frame_size());
 }

 void LIR_Assembler::emit_exception_entries(ExceptionInfoList* info_list) {
diff -r 184aac46be1f src/share/vm/opto/compile.cpp
--- a/src/share/vm/opto/compile.cpp	Sun Aug 10 19:38:53 2014 -0700
+++ b/src/share/vm/opto/compile.cpp	Tue Aug 19 14:18:12 2014 +0400
@@ -430,7 +430,7 @@
 // removes the need to bang the stack in the deoptimization blob which
 // in turn simplifies stack overflow handling.
 int Compile::bang_size_in_bytes() const {
-  return MAX2(_interpreter_frame_size, frame_size_in_bytes());
+  return MAX2(_interpreter_frame_size, frame_size_in_bytes() +
VM_Version::L1_line_size());
 }

This makes me uneasy for two reasons:

 a) We only need to do this for x86. ifdef'ing the generic
LIR_Assembler/Compile code seems odd. Can we pro-actively bang for an
additional cache line for all platforms?

 b) The cache line size here does seem like a magic number, and I would
like to move the StoreLoad SP offset calculation somewhere -- but where
to? VM_Version seems to be too generic to handle the x86-specific case.
Putting it in assembler(_x86) seems to introduce the artificial
dependency between the machine-neutral and machine-dependent parts of
the compiler.

Thanks,
-Aleksey.