Skip to content

Commit f7fe241

Browse files
eregoneregon
committed
Use InlinedConditionProfile in MergedObjectTypeModuleGetAttributeInnerNode
* So we profile both cases, whether we ever or never saw a descriptor, and same for value, to avoid extra control flow in PE compilation.
1 parent ab61d77 commit f7fe241

File tree

5 files changed

+21
-30
lines changed

5 files changed

+21
-30
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/module/ModuleBuiltins.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import com.oracle.graal.python.nodes.ErrorMessages;
8787
import com.oracle.graal.python.nodes.PNodeWithContext;
8888
import com.oracle.graal.python.nodes.PRaiseNode;
89+
import com.oracle.graal.python.nodes.attributes.MergedObjectTypeModuleGetAttributeNode;
8990
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromModuleNode;
9091
import com.oracle.graal.python.nodes.attributes.ReadAttributeFromObjectNode;
9192
import com.oracle.graal.python.nodes.attributes.WriteAttributeToObjectNode;
@@ -237,6 +238,9 @@ static Object doError(Object self, @SuppressWarnings("unused") Object dict,
237238
@Slot(value = SlotKind.tp_getattro, isComplex = true)
238239
@GenerateNodeFactory
239240
public abstract static class ModuleGetattributeNode extends GetAttrBuiltinNode {
241+
/**
242+
* Keep in sync with {@link MergedObjectTypeModuleGetAttributeNode}
243+
*/
240244
@Specialization
241245
static Object getattribute(VirtualFrame frame, PythonModule self, Object keyObj,
242246
@Bind Node inliningTarget,

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/object/ObjectBuiltins.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -523,20 +523,19 @@ Object doIt(VirtualFrame frame, Object object, Object keyObj,
523523
@Cached GetObjectSlotsNode getDescrSlotsNode,
524524
@Cached LookupAttributeInMRONode.Dynamic lookup,
525525
@Cached CastToTruffleStringChecked1Node castToString,
526-
@Cached InlinedBranchProfile hasDescProfile,
526+
@Cached InlinedConditionProfile hasDescProfile,
527527
@Cached InlinedConditionProfile hasDescrGetProfile,
528-
@Cached InlinedBranchProfile hasValueProfile,
528+
@Cached InlinedConditionProfile hasValueProfile,
529529
@Cached PRaiseNode raiseNode) {
530530
TruffleString key = castToString.cast(inliningTarget, keyObj, ErrorMessages.ATTR_NAME_MUST_BE_STRING, keyObj);
531531

532532
Object type = getClassNode.execute(inliningTarget, object);
533533
Object descr = lookup.execute(type, key);
534-
boolean hasDescr = descr != PNone.NO_VALUE;
534+
boolean hasDescr = hasDescProfile.profile(inliningTarget, descr != PNone.NO_VALUE);
535535

536536
TpSlot get = null;
537537
boolean hasDescrGet = false;
538538
if (hasDescr) {
539-
hasDescProfile.enter(inliningTarget);
540539
var descrSlots = getDescrSlotsNode.execute(inliningTarget, descr);
541540
get = descrSlots.tp_descr_get();
542541
hasDescrGet = hasDescrGetProfile.profile(inliningTarget, get != null);
@@ -547,13 +546,11 @@ Object doIt(VirtualFrame frame, Object object, Object keyObj,
547546

548547
// The only difference between all 3 nodes
549548
Object value = readAttributeOfObject(object, key);
550-
if (value != PNone.NO_VALUE) {
551-
hasValueProfile.enter(inliningTarget);
549+
if (hasValueProfile.profile(inliningTarget, value != PNone.NO_VALUE)) {
552550
return value;
553551
}
554552

555553
if (hasDescr) {
556-
hasDescProfile.enter(inliningTarget);
557554
if (!hasDescrGet) {
558555
return descr;
559556
} else {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/thread/ThreadLocalBuiltins.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@
9191
import com.oracle.truffle.api.dsl.Specialization;
9292
import com.oracle.truffle.api.frame.VirtualFrame;
9393
import com.oracle.truffle.api.nodes.Node;
94-
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
9594
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
9695
import com.oracle.truffle.api.strings.TruffleString;
9796

@@ -154,7 +153,7 @@ Object doIt(VirtualFrame frame, PThreadLocal object, Object keyObj,
154153
@Cached GetObjectSlotsNode getDescrSlotsNode,
155154
@Cached CastToTruffleStringChecked1Node castToString,
156155
@Cached HashingStorageGetItem getDictStorageItem,
157-
@Cached InlinedBranchProfile hasDescProfile,
156+
@Cached InlinedConditionProfile hasDescProfile,
158157
@Cached InlinedConditionProfile hasDescrGetProfile,
159158
@Cached InlinedConditionProfile hasValueProfile,
160159
@Cached PRaiseNode raiseNode) {
@@ -165,12 +164,11 @@ Object doIt(VirtualFrame frame, PThreadLocal object, Object keyObj,
165164

166165
Object type = getClassNode.execute(inliningTarget, object);
167166
Object descr = lookup.execute(type, key);
168-
boolean hasDescr = descr != PNone.NO_VALUE;
167+
boolean hasDescr = hasDescProfile.profile(inliningTarget, descr != PNone.NO_VALUE);
169168

170169
TpSlot get = null;
171170
boolean hasDescrGet = false;
172171
if (hasDescr) {
173-
hasDescProfile.enter(inliningTarget);
174172
var descrSlots = getDescrSlotsNode.execute(inliningTarget, descr);
175173
get = descrSlots.tp_descr_get();
176174
hasDescrGet = hasDescrGetProfile.profile(inliningTarget, get != null);
@@ -186,7 +184,6 @@ Object doIt(VirtualFrame frame, PThreadLocal object, Object keyObj,
186184
}
187185

188186
if (hasDescr) {
189-
hasDescProfile.enter(inliningTarget);
190187
if (!hasDescrGet) {
191188
return descr;
192189
} else {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/type/TypeBuiltins.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -539,21 +539,20 @@ protected Object doIt(VirtualFrame frame, Object object, Object keyObj,
539539
@Cached GetObjectSlotsNode getValueSlotsNode,
540540
@Cached LookupAttributeInMRONode.Dynamic lookup,
541541
@Cached CastToTruffleStringChecked1Node castToString,
542-
@Cached InlinedBranchProfile hasDescProfile,
542+
@Cached InlinedConditionProfile hasDescProfile,
543543
@Cached InlinedConditionProfile hasDescrGetProfile,
544-
@Cached InlinedBranchProfile hasValueProfile,
544+
@Cached InlinedConditionProfile hasValueProfile,
545545
@Cached InlinedBranchProfile hasNonDescriptorValueProfile,
546546
@Cached PRaiseNode raiseNode) {
547547
TruffleString key = castToString.cast(inliningTarget, keyObj, ErrorMessages.ATTR_NAME_MUST_BE_STRING, keyObj);
548548

549549
Object type = getClassNode.execute(inliningTarget, object);
550550
Object descr = lookup.execute(type, key);
551-
boolean hasDescr = descr != PNone.NO_VALUE;
551+
boolean hasDescr = hasDescProfile.profile(inliningTarget, descr != PNone.NO_VALUE);
552552

553553
TpSlot get = null;
554554
boolean hasDescrGet = false;
555555
if (hasDescr) {
556-
hasDescProfile.enter(inliningTarget);
557556
var descrSlots = getDescrSlotsNode.execute(inliningTarget, descr);
558557
get = descrSlots.tp_descr_get();
559558
hasDescrGet = hasDescrGetProfile.profile(inliningTarget, get != null);
@@ -564,8 +563,7 @@ protected Object doIt(VirtualFrame frame, Object object, Object keyObj,
564563

565564
// The only difference between all 3 nodes
566565
Object value = readAttributeOfClass(object, key);
567-
if (value != PNone.NO_VALUE) {
568-
hasValueProfile.enter(inliningTarget);
566+
if (hasValueProfile.profile(inliningTarget, value != PNone.NO_VALUE)) {
569567
var valueGet = getValueSlotsNode.execute(inliningTarget, value).tp_descr_get();
570568
if (valueGet == null) {
571569
hasNonDescriptorValueProfile.enter(inliningTarget);
@@ -576,7 +574,6 @@ protected Object doIt(VirtualFrame frame, Object object, Object keyObj,
576574
}
577575

578576
if (hasDescr) {
579-
hasDescProfile.enter(inliningTarget);
580577
if (!hasDescrGet) {
581578
return descr;
582579
} else {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/attributes/MergedObjectTypeModuleGetAttributeNode.java‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,18 @@ abstract class MergedObjectTypeModuleGetAttributeInnerNode extends PNodeWithCont
110110

111111
/**
112112
* Keep in sync with {@link ObjectBuiltins.GetAttributeNode} and
113-
* {@link TypeBuiltins.GetattributeNode} and {@link ModuleBuiltins.ModuleGetattributeNode} and
114-
* {@link ThreadLocalBuiltins.GetAttributeNode}
113+
* {@link TypeBuiltins.GetattributeNode} and {@link ThreadLocalBuiltins.GetAttributeNode} and
114+
* {@link ModuleBuiltins.ModuleGetattributeNode}
115115
*/
116116
@Specialization(guards = {"slots.tp_getattro() == cachedSlot", "isObjectTypeModuleGetAttribute(cachedSlot)"}, limit = "1")
117117
static Object doIt(VirtualFrame frame, Node inliningTarget, Object object, TruffleString key, Object type, @SuppressWarnings("unused") TpSlots slots,
118118
@Cached("slots.tp_getattro()") TpSlot cachedSlot,
119119
// Common
120120
@Cached GetObjectSlotsNode getDescrSlotsNode,
121121
@Cached LookupAttributeInMRONode.Dynamic lookup,
122-
@Cached InlinedBranchProfile hasDescProfile,
122+
@Cached InlinedConditionProfile hasDescProfile,
123123
@Cached InlinedConditionProfile hasDescrGetProfile,
124-
@Cached InlinedBranchProfile hasValueProfile,
124+
@Cached InlinedConditionProfile hasValueProfile,
125125
@Cached PRaiseNode raiseNode,
126126
@Cached CallSlotDescrGet.Lazy callSlotDescrGet,
127127
// Specific to a given tp_getattro, some should probably be lazy
@@ -134,12 +134,11 @@ static Object doIt(VirtualFrame frame, Node inliningTarget, Object object, Truff
134134
assert hasNoGetAttr(object);
135135
try {
136136
Object descr = lookup.execute(type, key);
137-
boolean hasDescr = descr != PNone.NO_VALUE;
137+
boolean hasDescr = hasDescProfile.profile(inliningTarget, descr != PNone.NO_VALUE);
138138

139139
TpSlot get = null;
140140
boolean hasDescrGet = false;
141141
if (hasDescr) {
142-
hasDescProfile.enter(inliningTarget);
143142
var descrSlots = getDescrSlotsNode.execute(inliningTarget, descr);
144143
get = descrSlots.tp_descr_get();
145144
hasDescrGet = hasDescrGetProfile.profile(inliningTarget, get != null);
@@ -153,15 +152,13 @@ static Object doIt(VirtualFrame frame, Node inliningTarget, Object object, Truff
153152
if (cachedSlot != TypeBuiltins.SLOTS.tp_getattro()) {
154153
// ObjectBuiltins.SLOTS.tp_getattro() || ModuleBuiltins.SLOTS.tp_getattro()
155154
value = readAttributeOfObjectNode.execute(object, key);
156-
if (value != PNone.NO_VALUE) {
157-
hasValueProfile.enter(inliningTarget);
155+
if (hasValueProfile.profile(inliningTarget, value != PNone.NO_VALUE)) {
158156
return value;
159157
}
160158
} else {
161159
// TypeBuiltins.SLOTS.tp_getattro()
162160
value = readAttributeOfClassNode.execute(object, key);
163-
if (value != PNone.NO_VALUE) {
164-
hasValueProfile.enter(inliningTarget);
161+
if (hasValueProfile.profile(inliningTarget, value != PNone.NO_VALUE)) {
165162
var valueGet = getValueSlotsNode.execute(inliningTarget, value).tp_descr_get();
166163
if (valueGet == null) {
167164
hasNonDescriptorValueProfile.enter(inliningTarget);
@@ -173,7 +170,6 @@ static Object doIt(VirtualFrame frame, Node inliningTarget, Object object, Truff
173170
}
174171

175172
if (hasDescr) {
176-
hasDescProfile.enter(inliningTarget);
177173
if (!hasDescrGet) {
178174
return descr;
179175
} else {

0 commit comments

Comments
 (0)