Skip to content

Commit bb22513

Browse files
committed
Fix: ensure pointer identity for small int/long wrappers in compiled code.
1 parent a9254c6 commit bb22513

File tree

2 files changed

+48
-9
lines changed

2 files changed

+48
-9
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/CExtNodes.java

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,25 @@ static PrimitiveNativeWrapper doIntegerSmall(@SuppressWarnings("unused") CExtCon
351351
return PrimitiveNativeWrapper.createInt(i);
352352
}
353353

354+
/**
355+
* Generic integer conversion.<br/>
356+
* In the interpreter: we just lookup the cached primitive native wrapper and use this
357+
* instance. In compiled code, we do the same but create a fresh copy such that the compiler
358+
* always sees a fresh instance. This avoids a phi and certainly a real allocation. Note: it
359+
* is important to copy the existing cached wrapper because otherwise pointer equality is
360+
* not ensured (see also:
361+
* {@link AsPythonObjectBaseNode#mayUsePrimitive(IsPointerNode, PrimitiveNativeWrapper)}).
362+
*/
354363
@Specialization(replaces = "doIntegerSmall")
355364
static PrimitiveNativeWrapper doInteger(@SuppressWarnings("unused") CExtContext cextContext, int i,
356365
@Shared("contextRef") @CachedContext(PythonLanguage.class) ContextReference<PythonContext> contextRef) {
357-
if (CompilerDirectives.inInterpreter() && CApiGuards.isSmallInteger(i)) {
358-
return doIntegerSmall(cextContext, i, contextRef);
366+
if (CApiGuards.isSmallInteger(i)) {
367+
if (CompilerDirectives.inInterpreter()) {
368+
return doIntegerSmall(cextContext, i, contextRef);
369+
} else {
370+
// for explanation: see method doc
371+
return doIntegerSmall(cextContext, i, contextRef).copy();
372+
}
359373
}
360374
return PrimitiveNativeWrapper.createInt(i);
361375
}
@@ -373,8 +387,13 @@ static PrimitiveNativeWrapper doLongSmall(@SuppressWarnings("unused") CExtContex
373387
@Specialization(replaces = "doLongSmall")
374388
static PrimitiveNativeWrapper doLong(@SuppressWarnings("unused") CExtContext cextContext, long l,
375389
@Shared("contextRef") @CachedContext(PythonLanguage.class) ContextReference<PythonContext> contextRef) {
376-
if (CompilerDirectives.inInterpreter() && CApiGuards.isSmallLong(l)) {
377-
return doLongSmall(cextContext, l, contextRef);
390+
if (CApiGuards.isSmallLong(l)) {
391+
// for explanation of this construct: see 'ToSulongNode.doInteger'
392+
if (CompilerDirectives.inInterpreter()) {
393+
return doLongSmall(cextContext, l, contextRef);
394+
} else {
395+
return doLongSmall(cextContext, l, contextRef).copy();
396+
}
378397
}
379398
return PrimitiveNativeWrapper.createLong(l);
380399
}
@@ -602,17 +621,27 @@ static PrimitiveNativeWrapper doLongSmall(@SuppressWarnings("unused") CExtContex
602621
@Specialization(replaces = "doIntegerSmall")
603622
static PrimitiveNativeWrapper doInteger(CExtContext cextContext, int i,
604623
@Shared("contextRef") @CachedContext(PythonLanguage.class) ContextReference<PythonContext> contextRef) {
605-
if (CompilerDirectives.inInterpreter() && CApiGuards.isSmallInteger(i)) {
606-
return doIntegerSmall(cextContext, i, contextRef);
624+
if (CApiGuards.isSmallInteger(i)) {
625+
// for explanation of this construct: see 'ToSulongNode.doInteger'
626+
if (CompilerDirectives.inInterpreter()) {
627+
return doIntegerSmall(cextContext, i, contextRef);
628+
} else {
629+
return doIntegerSmall(cextContext, i, contextRef).copy();
630+
}
607631
}
608632
return PrimitiveNativeWrapper.createInt(i);
609633
}
610634

611635
@Specialization(replaces = "doLongSmall")
612636
static PrimitiveNativeWrapper doLong(CExtContext cextContext, long l,
613637
@Shared("contextRef") @CachedContext(PythonLanguage.class) ContextReference<PythonContext> contextRef) {
614-
if (CompilerDirectives.inInterpreter() && CApiGuards.isSmallLong(l)) {
615-
return doLongSmall(cextContext, l, contextRef);
638+
if (CApiGuards.isSmallLong(l)) {
639+
// for explanation of this construct: see 'ToSulongNode.doInteger'
640+
if (CompilerDirectives.inInterpreter()) {
641+
return doLongSmall(cextContext, l, contextRef);
642+
} else {
643+
return doLongSmall(cextContext, l, contextRef).copy();
644+
}
616645
}
617646
return PrimitiveNativeWrapper.createLong(l);
618647
}

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/DynamicObjectNativeWrapper.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,12 @@ private PrimitiveNativeWrapper(double dvalue) {
14741474
this.dvalue = dvalue;
14751475
}
14761476

1477+
private PrimitiveNativeWrapper(byte state, long value, double dvalue) {
1478+
this.state = state;
1479+
this.value = value;
1480+
this.dvalue = dvalue;
1481+
}
1482+
14771483
public byte getState() {
14781484
return state;
14791485
}
@@ -1553,7 +1559,11 @@ public boolean equals(Object obj) {
15531559

15541560
@Override
15551561
public int hashCode() {
1556-
return (int) (value ^ Double.doubleToRawLongBits(dvalue) ^ state);
1562+
return (Long.hashCode(value) ^ Long.hashCode(Double.doubleToRawLongBits(dvalue)) ^ state);
1563+
}
1564+
1565+
PrimitiveNativeWrapper copy() {
1566+
return new PrimitiveNativeWrapper(state, value, dvalue);
15571567
}
15581568

15591569
@Override

0 commit comments

Comments
 (0)