From 8a4b31ddf4a6afe9bcb0271483aa49f8179bc139 Mon Sep 17 00:00:00 2001 From: Christian Humer <christian.humer@oracle.com> Date: Thu, 11 Jan 2024 17:50:54 +0100 Subject: [PATCH] Fix corner case where cached fields should not be generated for createCachedFields to avoid compiler warnings. --- .../truffle/api/dsl/test/GR51148Test.java | 126 ++++++++++++++++++ .../generator/FlatNodeGenFactory.java | 28 ++-- 2 files changed, 144 insertions(+), 10 deletions(-) create mode 100644 truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/GR51148Test.java diff --git a/truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/GR51148Test.java b/truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/GR51148Test.java new file mode 100644 index 000000000000..05924ea4af73 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/GR51148Test.java @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.dsl.test; + +import com.oracle.truffle.api.dsl.Bind; +import com.oracle.truffle.api.dsl.Cached; +import com.oracle.truffle.api.dsl.Cached.Exclusive; +import com.oracle.truffle.api.dsl.Cached.Shared; +import com.oracle.truffle.api.dsl.Specialization; +import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.profiles.ConditionProfile; +import com.oracle.truffle.api.profiles.InlinedConditionProfile; + +/* + * This test fails by emitting a warning in the generated code. + * It passes if no warning is emitted. + */ +public class GR51148Test { + + @SuppressWarnings({"truffle", "unused"}) + abstract static class TestNode extends Node { + + abstract Object execute(Object arg); + + @Specialization(guards = "arg0 == 0") + Object s0(int arg0, + @Bind("this") Node node, + @Cached @Shared InlinedConditionProfile profile0, + @Cached @Exclusive InlinedConditionProfile cachedProfile0, + // use a number of cached profiles to force trigger a specialization data + // class + @Cached @Exclusive ConditionProfile cachedProfile1, + @Cached @Exclusive ConditionProfile cachedProfile2, + @Cached @Exclusive ConditionProfile cachedProfile3, + @Cached @Exclusive ConditionProfile cachedProfile4, + @Cached @Exclusive ConditionProfile cachedProfile5, + @Cached @Exclusive ConditionProfile cachedProfile6, + @Cached @Exclusive ConditionProfile cachedProfile7, + @Cached @Exclusive ConditionProfile cachedProfile8, + @Cached @Exclusive ConditionProfile cachedProfile9, + @Cached @Exclusive ConditionProfile cachedProfile10, + @Cached @Exclusive ConditionProfile cachedProfile11) { + profile0.profile(this, arg0 == 0); + return arg0; + } + + /* + * This specialization triggers a data class, before the fix this was emitting an error + * requiring @Bind("this") to bind the inlining target. But in this case "this" can safely + * be used. + */ + @SuppressWarnings("truffle-interpreted-performance") + @Specialization(guards = "arg0 == 1") + Object s1(int arg0, + @Bind("this") Node node, + @Cached @Shared InlinedConditionProfile profile0, + @Cached @Exclusive InlinedConditionProfile cachedProfile0, + @Cached @Exclusive ConditionProfile cachedProfile1, + @Cached @Exclusive ConditionProfile cachedProfile2, + @Cached @Exclusive ConditionProfile cachedProfile3, + @Cached @Exclusive ConditionProfile cachedProfile4, + @Cached @Exclusive ConditionProfile cachedProfile5) { + profile0.profile(this, arg0 == 1); + return arg0; + } + + /* + * This specialization triggers a data class, before the fix this was emitting an error + * requiring @Bind("this") to bind the inlining target. But in this case "this" can safely + * be used. + */ + @SuppressWarnings("truffle-interpreted-performance") + @Specialization(guards = "arg0 == 2") + Object s2(int arg0, + @Bind("this") Node node, + @Cached @Shared InlinedConditionProfile profile0, + @Cached @Exclusive InlinedConditionProfile cachedProfile0, + @Cached @Exclusive ConditionProfile cachedProfile1, + @Cached @Exclusive ConditionProfile cachedProfile2, + @Cached @Exclusive ConditionProfile cachedProfile3, + @Cached @Exclusive ConditionProfile cachedProfile4, + @Cached @Exclusive ConditionProfile cachedProfile5) { + profile0.profile(this, arg0 == 1); + return arg0; + } + } + +} diff --git a/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/FlatNodeGenFactory.java b/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/FlatNodeGenFactory.java index 3a3d4972a374..db0f9dc56f34 100644 --- a/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/FlatNodeGenFactory.java +++ b/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/generator/FlatNodeGenFactory.java @@ -1883,7 +1883,6 @@ private List<Element> createCachedFields(CodeTypeElement baseType) { continue; } expressions.add(fieldName); - createCachedFieldsImpl(nodeElements, nodeElements, null, null, cache, true); } } @@ -2180,6 +2179,11 @@ private void createCachedFieldsImpl( } CacheExpression sharedCache = lookupSharedCacheKey(cache); InlinedNodeData inline = sharedCache.getInlinedNode(); + /* + * Handles corner case where we try to avoid generating shared cached fields if we are + * always using parent access cache for a shared cache. + */ + boolean generateCachedFields = specialization != null || !hasCacheParentAccess(cache) || hasSharedCacheDirectAccess(lookupSharedCacheKey(cache)); if (inline != null) { Parameter parameter = cache.getParameter(); @@ -2194,14 +2198,16 @@ private void createCachedFieldsImpl( for (InlineFieldData field : inline.getFields()) { builder.startGroup(); if (field.isState()) { - BitSet specializationBitSet = findInlinedState(specializationState, field); - CodeVariableElement updaterField = createStateUpdaterField(specialization, specializationState, field, specializationClassElements); - String updaterFieldName = updaterField.getName(); - builder.startCall(updaterFieldName, "subUpdater"); - BitRange range = specializationBitSet.getStates().queryRange(StateQuery.create(InlinedNodeState.class, field)); - builder.string(String.valueOf(range.offset)); - builder.string(String.valueOf(range.length)); - builder.end(); + if (generateCachedFields) { + BitSet specializationBitSet = findInlinedState(specializationState, field); + CodeVariableElement updaterField = createStateUpdaterField(specialization, specializationState, field, specializationClassElements); + String updaterFieldName = updaterField.getName(); + builder.startCall(updaterFieldName, "subUpdater"); + BitRange range = specializationBitSet.getStates().queryRange(StateQuery.create(InlinedNodeState.class, field)); + builder.string(String.valueOf(range.offset)); + builder.string(String.valueOf(range.length)); + builder.end(); + } } else { /* * All other fields need fields to get inlined. We do not support specialization @@ -2265,7 +2271,9 @@ private void createCachedFieldsImpl( addSourceDoc(javadoc, specialization, cache, null); javadoc.end(); - nodeElements.add(cachedField); + if (generateCachedFields) { + nodeElements.add(cachedField); + } } else { Parameter parameter = cache.getParameter(); String fieldName = createFieldName(specialization, cache);