--- old/src/share/vm/compiler/directivesParser.hpp 2015-10-26 15:10:53.250985193 +0100 +++ new/src/share/vm/compiler/directivesParser.hpp 2015-10-26 15:10:53.126985189 +0100 @@ -33,6 +33,7 @@ intxFlag, doubleFlag, ccstrFlag, + ccstrlistFlag, UnknownFlagType }; @@ -41,6 +42,7 @@ "int", "double", "string", + "string list", "unknown" }; --- old/src/share/vm/compiler/compilerDirectives.hpp 2015-10-26 15:10:53.230985192 +0100 +++ new/src/share/vm/compiler/compilerDirectives.hpp 2015-10-26 15:10:53.106985188 +0100 @@ -47,7 +47,7 @@ cflags(DumpReplay, bool, false, DumpReplay) \ cflags(DumpInline, bool, false, DumpInline) \ cflags(CompilerDirectivesIgnoreCompileCommands, bool, CompilerDirectivesIgnoreCompileCommands, X) \ - cflags(DisableIntrinsic, ccstr, DisableIntrinsic, DisableIntrinsic) + cflags(DisableIntrinsic, ccstrlist, DisableIntrinsic, DisableIntrinsic) #ifdef COMPILER1 #define compilerdirectives_c1_flags(cflags) @@ -100,6 +100,8 @@ InlineMatcher* _inlinematchers; CompilerDirectives* _directive; + static ccstrlist canonicalize_disableintrinsic(ccstrlist option_value); + public: DirectiveSet(CompilerDirectives* directive); ~DirectiveSet(); @@ -141,6 +143,7 @@ void print_bool(outputStream* st, ccstr n, bool v, bool mod) { if (mod) { st->print("%s:%s ", n, v ? "true" : "false"); } } void print_double(outputStream* st, ccstr n, double v, bool mod) { if (mod) { st->print("%s:%f ", n, v); } } void print_ccstr(outputStream* st, ccstr n, ccstr v, bool mod) { if (mod) { st->print("%s:%s ", n, v); } } + void print_ccstrlist(outputStream* st, ccstr n, ccstr v, bool mod) { print_ccstr(st, n, v, mod); } void print(outputStream* st) { print_inline(st); --- old/src/share/vm/compiler/compilerDirectives.cpp 2015-10-26 15:10:53.254985193 +0100 +++ new/src/share/vm/compiler/compilerDirectives.cpp 2015-10-26 15:10:53.114985189 +0100 @@ -175,12 +175,38 @@ return NULL; } +// In the list of disabled intrinsics, the ID of the disabled intrinsics can separated: +// - by ',' (if -XX:DisableIntrinsic is used once when invoking the VM) or +// - by '\n' (if -XX:DisableIntrinsic is used multiple times when invoking the VM) or +// - by ' ' (if DisableIntrinsic is used on a per-method level, e.g., with CompileCommand). +// +// To simplify the processing of the list, the canonicalize_disableintrinsic() method +// returns a new copy of the list in which '\n' and ' ' is replaced with ','. +ccstrlist DirectiveSet::canonicalize_disableintrinsic(ccstrlist option_value) { + char* canonicalized_list = NEW_C_HEAP_ARRAY(char, strlen(option_value) + 1, mtCompiler); + int i = 0; + char current; + while ((current = option_value[i]) != '\0') { + if (current == '\n' || current == ' ') { + canonicalized_list[i] = ','; + } else { + canonicalized_list[i] = current; + } + i++; + } + canonicalized_list[i] = '\0'; + return canonicalized_list; +} + DirectiveSet::DirectiveSet(CompilerDirectives* d) :_inlinematchers(NULL), _directive(d) { #define init_defaults_definition(name, type, dvalue, compiler) this->name##Option = dvalue; compilerdirectives_common_flags(init_defaults_definition) compilerdirectives_c2_flags(init_defaults_definition) compilerdirectives_c1_flags(init_defaults_definition) memset(_modified, 0, sizeof _modified); + + // Canonicalize DisableIntrinsic to contain only ',' as a separator. + this->DisableIntrinsicOption = canonicalize_disableintrinsic(DisableIntrinsic); } DirectiveSet::~DirectiveSet() { @@ -192,12 +218,11 @@ tmp = next; } - // Free if modified, otherwise it just points to the global vm flag value - // or to the Compile command option - if (_modified[DisableIntrinsicIndex]) { - assert(this->DisableIntrinsicOption != NULL, ""); - FREE_C_HEAP_ARRAY(char, (void *)this->DisableIntrinsicOption); - } + // When constructed, DirectiveSet canonicalizes the DisableIntrinsic flag + // into a new string. Therefore, that string is deallocated when + // the DirectiveSet is destroyed. + assert(this->DisableIntrinsicOption != NULL, ""); + FREE_C_HEAP_ARRAY(char, (void *)this->DisableIntrinsicOption); } // Backward compatibility for CompileCommands @@ -253,6 +278,14 @@ compilerdirectives_c2_flags(init_default_cc) compilerdirectives_c1_flags(init_default_cc) + // Canonicalize DisableIntrinsic to contain only ',' as a separator. + ccstrlist option_value; + if (!_modified[DisableIntrinsicIndex] && + CompilerOracle::has_option_value(method, "DisableIntrinsic", option_value)) { + set->DisableIntrinsicOption = canonicalize_disableintrinsic(option_value); + } + + if (!changed) { // We didn't actually update anything, discard. delete set; @@ -352,8 +385,26 @@ vmIntrinsics::ID id = method->intrinsic_id(); assert(id != vmIntrinsics::_none, "must be a VM intrinsic"); - ccstr disable_intr = DisableIntrinsicOption; - return ((disable_intr != '\0') && strstr(disable_intr, vmIntrinsics::name_at(id)) != NULL); + ResourceMark rm; + + // Create a copy of the string that contains the list of disabled + // intrinsics. The copy is created because the string + // will be modified by strtok(). Then, the list is tokenized with + // ',' as a separator. + size_t length = strlen(DisableIntrinsicOption); + char* local_list = NEW_RESOURCE_ARRAY(char, length + 1); + strncpy(local_list, DisableIntrinsicOption, length + 1); + + char* token = strtok(local_list, ","); + while (token != NULL) { + if (strcmp(token, vmIntrinsics::name_at(id)) == 0) { + return true; + } else { + token = strtok(NULL, ","); + } + } + + return false; } DirectiveSet* DirectiveSet::clone(DirectiveSet const* src) { @@ -371,15 +422,13 @@ compilerdirectives_c2_flags(copy_members_definition) compilerdirectives_c1_flags(copy_members_definition) - // Must duplicate ccstr option if it was modified, otherwise it is global. - if (src->_modified[DisableIntrinsicIndex]) { - assert(src->DisableIntrinsicOption != NULL, ""); - size_t len = strlen(src->DisableIntrinsicOption) + 1; - char* s = NEW_C_HEAP_ARRAY(char, len, mtCompiler); - strncpy(s, src->DisableIntrinsicOption, len); - assert(s[len-1] == '\0', ""); - set->DisableIntrinsicOption = s; - } + // Create a local copy of the DisableIntrinsicOption. + assert(src->DisableIntrinsicOption != NULL, ""); + size_t len = strlen(src->DisableIntrinsicOption) + 1; + char* s = NEW_C_HEAP_ARRAY(char, len, mtCompiler); + strncpy(s, src->DisableIntrinsicOption, len); + assert(s[len-1] == '\0', ""); + set->DisableIntrinsicOption = s; return set; } --- old/src/share/vm/compiler/directivesParser.cpp 2015-10-26 15:10:53.306985194 +0100 +++ new/src/share/vm/compiler/directivesParser.cpp 2015-10-26 15:10:53.178985190 +0100 @@ -288,7 +288,7 @@ break; case JSON_STRING: - if (option_key->flag_type != ccstrFlag) { + if (option_key->flag_type != ccstrFlag && option_key->flag_type != ccstrlistFlag) { error(VALUE_ERROR, "Cannot use string value for a %s flag", flag_type_names[option_key->flag_type]); return false; } else { --- /dev/null 2015-10-21 14:54:53.661377003 +0200 +++ new/test/compiler/intrinsics/IntrinsicDisabledTest.java 2015-10-26 15:10:53.246985192 +0100 @@ -0,0 +1,211 @@ +/* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8138651 + * @library /testlibrary /../../test/lib + * @build IntrinsicDisabledTest + * @run main ClassFileInstaller sun.hotspot.WhiteBox + * sun.hotspot.WhiteBox$WhiteBoxPermission + * @run main/othervm -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -XX:DisableIntrinsic=_putCharVolatile,_putInt + * -XX:DisableIntrinsic=_putIntVolatile + * -XX:CompileCommand=option,sun.misc.Unsafe::putChar,ccstrlist,DisableIntrinsic,_getCharVolatile,_getInt + * -XX:CompileCommand=option,sun.misc.Unsafe::putCharVolatile,ccstrlist,DisableIntrinsic,_getIntVolatile + * IntrinsicDisabledTest + */ + +import java.lang.reflect.Executable; +import java.util.Objects; + +import sun.hotspot.WhiteBox; + +import jdk.test.lib.Platform; + +public class IntrinsicDisabledTest { + + private static final WhiteBox wb = WhiteBox.getWhiteBox(); + + /* Compilation level corresponding to C1. */ + private static final int COMP_LEVEL_SIMPLE = 1; + + /* Compilation level corresponding to C2. */ + private static final int COMP_LEVEL_FULL_OPTIMIZATION = 4; + + /* Determine if tiered compilation is enabled. */ + private static boolean isTieredCompilationEnabled() { + return Boolean.valueOf(Objects.toString(wb.getVMFlag("TieredCompilation"))); + } + + /* This test uses several methods from sun.misc.Unsafe. The method + * getMethod() returns a different Executable for each different + * combination of its input parameters. There are eight possible + * combinations, getMethod can return an Executable representing + * the following methods: putChar, putCharVolatile, getChar, + * getCharVolatile, putInt, putIntVolatile, getInt, + * getIntVolatile. These methods were selected because they can + * be intrinsified by both the C1 and the C2 compiler. + */ + static Executable getMethod(boolean isChar, boolean isPut, boolean isVolatile) throws RuntimeException { + Executable aMethod; + String methodTypeName = isChar ? "Char" : "Int"; + + try { + Class aClass = Class.forName("sun.misc.Unsafe"); + if (isPut) { + aMethod = aClass.getDeclaredMethod("put" + methodTypeName + (isVolatile ? "Volatile" : ""), + Object.class, + long.class, + isChar ? char.class : int.class); + } else { + aMethod = aClass.getDeclaredMethod("get" + methodTypeName + (isVolatile ? "Volatile" : ""), + Object.class, + long.class); + } + } catch (NoSuchMethodException e) { + throw new RuntimeException("Test bug, method is unavailable. " + e); + } catch (ClassNotFoundException e) { + throw new RuntimeException("Test bug, class is unavailable. " + e); + } + + return aMethod; + } + + public static void test(int compLevel) { + + Executable putChar = getMethod(true, /*isPut =*/ true, /*isVolatile = */ false); + Executable getChar = getMethod(true, /*isPut =*/ false, /*isVolatile = */ false); + Executable putCharVolatile = getMethod(true, /*isPut =*/ true, /*isVolatile = */ true); + Executable getCharVolatile = getMethod(true, /*isPut =*/ false, /*isVolatile = */ true); + Executable putInt = getMethod(false, /*isPut =*/ true, /*isVolatile = */ false); + Executable getInt = getMethod(false, /*isPut =*/ false, /*isVolatile = */ false); + Executable putIntVolatile = getMethod(false, /*isPut =*/ true, /*isVolatile = */ true); + Executable getIntVolatile = getMethod(false, /*isPut =*/ false, /*isVolatile = */ true); + + /* Test if globally disabling intrinsics works. */ + if (!wb.isIntrinsicAvailable(putChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putChar.toGenericString() + + "] is not available globally although it should be."); + } + + if (wb.isIntrinsicAvailable(putCharVolatile, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putCharVolatile.toGenericString() + + "] is available globally although it should not be."); + } + + if (wb.isIntrinsicAvailable(putInt, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putInt.toGenericString() + + "] is available globally although it should not be."); + } + + if (wb.isIntrinsicAvailable(putIntVolatile, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putIntVolatile.toGenericString() + + "] is available globally although it should not be."); + } + + /* Test if disabling intrinsics on a per-method level + * works. The method for which intrinsics are + * disabled (the compilation context) is putChar. */ + if (!wb.isIntrinsicAvailable(getChar, putChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getChar.toGenericString() + + "] is not available for intrinsification in [" + + putChar.toGenericString() + "] although it should be."); + } + + if (wb.isIntrinsicAvailable(getCharVolatile, putChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getCharVolatile.toGenericString() + + "] is available for intrinsification in [" + + putChar.toGenericString() + "] although it should not be."); + } + + if (wb.isIntrinsicAvailable(getInt, putChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getInt.toGenericString() + + "] is available for intrinsification in [" + + putChar.toGenericString() + "] although it should not be."); + } + + if (wb.isIntrinsicAvailable(getIntVolatile, putCharVolatile, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getIntVolatile.toGenericString() + + "] is available for intrinsification in [" + + putCharVolatile.toGenericString() + "] although it should not be."); + } + + /* Test if disabling intrinsics on a per-method level + * leaves those intrinsics enabled globally. */ + if (!wb.isIntrinsicAvailable(getCharVolatile, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getCharVolatile.toGenericString() + + "] is not available globally although it should be."); + } + + if (!wb.isIntrinsicAvailable(getInt, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getInt.toGenericString() + + "] is not available globally although it should be."); + } + + + if (!wb.isIntrinsicAvailable(getIntVolatile, compLevel)) { + throw new RuntimeException("Intrinsic for [" + getIntVolatile.toGenericString() + + "] is not available globally although it should be."); + } + + /* Test if disabling an intrinsic globally disables it on a + * per-method level as well. */ + if (!wb.isIntrinsicAvailable(putChar, getChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putChar.toGenericString() + + "] is not available for intrinsification in [" + + getChar.toGenericString() + "] although it should be."); + } + + if (wb.isIntrinsicAvailable(putCharVolatile, getChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putCharVolatile.toGenericString() + + "] is available for intrinsification in [" + + getChar.toGenericString() + "] although it should not be."); + } + + if (wb.isIntrinsicAvailable(putInt, getChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putInt.toGenericString() + + "] is available for intrinsification in [" + + getChar.toGenericString() + "] although it should not be."); + } + + if (wb.isIntrinsicAvailable(putIntVolatile, getChar, compLevel)) { + throw new RuntimeException("Intrinsic for [" + putIntVolatile.toGenericString() + + "] is available for intrinsification in [" + + getChar.toGenericString() + "] although it should not be."); + } + } + + public static void main(String args[]) { + if (Platform.isServer()) { + if (isTieredCompilationEnabled()) { + test(COMP_LEVEL_SIMPLE); + } + test(COMP_LEVEL_FULL_OPTIMIZATION); + } else { + test(COMP_LEVEL_SIMPLE); + } + } +}