On 01/31/2019 10:52, Igor Ignatyev wrote:
Hi Alex,

you have 'if INCLUDE_JVMCI' inside 'ifndef ZERO', although we currently don't 
have zero builds w/ JVMCI (and I don't think such builds would make much 
sense), it's better not to rely on that.  not a blocker from my point of view 
though.

can_pop_frame & can_force_early_return are set only if ZERO is not defined (for zero builds the capabilities are not supported), so I added disabling logic in the same block.

--alex


Thanks,
-- Igor

On Jan 31, 2019, at 10:38 AM, Alex Menkov <alexey.men...@oracle.com> wrote:

Hi guys,

thank you for the feedback.

updated webrev (used the way suggested by David & Igor):
http://cr.openjdk.java.net/~amenkov/tck_red_disable_caps/webrev.02/

--alex

On 01/30/2019 21:31, serguei.spit...@oracle.com wrote:
On 1/30/19 21:24, David Holmes wrote:
On 31/01/2019 3:18 pm, dean.l...@oracle.com wrote:
On 1/30/19 8:59 PM, serguei.spit...@oracle.com wrote:
So, the fix needs to be more like this:
+ // Workaround for 8195635:
+ // disable pop_frame and force_early_return capabilities with Graal
+ #if INCLUDE_JVMCI
+ if (!(EnableJVMCI && UseJVMCICompiler)) {
     jc.can_pop_frame = 1;
     jc.can_force_early_return = 1;
+ } + #endif Not sure, if the check for EnableJVMCI can be removed above.

We still need it to work when INCLUDE_JVMCI is not defined.
How about

JVMCI_ONLY(if (UseJVMCICompiler)) {
...
}

or

if (JVMCI_ONLY(UseJVMCICompiler) NOT_JVMCI(true)) {
...
}

Or just turn them on unconditionally first and turn off explicitly for JVMCI:

  jc.can_pop_frame = 1;
  jc.can_force_early_return = 1;
+ #if INCLUDE_JVMCI
+  // Workaround for 8195635:
+  // disable pop_frame and force_early_return capabilities with Graal
+ if (EnableJVMCI && UseJVMCICompiler) {
+     jc.can_pop_frame = 0;
+     jc.can_force_early_return = 0;
+ }
+ #endif

Oh, Dean is right.
We need these caps initialized even if the macro INCLUDE_JVMCI is undefined.
Then I like variant from David above.
Thanks,
Serguei
David

dl


Reply via email to