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.
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 >>>>
