From 906c4d5f809120d84670b689417c1de07bda3891 Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Mon, 20 Mar 2017 16:05:32 +0100
Subject: [PATCH] Fix: temporary shareable should not be 'value' of explicit
RPromise
---
.../truffle/r/nodes/function/PromiseHelperNode.java | 11 +++++++++++
.../com/oracle/truffle/r/test/ExpectedTestOutput.test | 8 ++++++++
.../r/test/library/base/TestPromiseOptimizations.java | 8 +++++++-
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/PromiseHelperNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/PromiseHelperNode.java
index 5dfa524184..aa95117cb5 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/PromiseHelperNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/PromiseHelperNode.java
@@ -50,6 +50,7 @@ import com.oracle.truffle.r.runtime.data.RPromise;
import com.oracle.truffle.r.runtime.data.RPromise.EagerPromise;
import com.oracle.truffle.r.runtime.data.RPromise.EagerPromiseBase;
import com.oracle.truffle.r.runtime.data.RPromise.PromiseState;
+import com.oracle.truffle.r.runtime.data.RShareable;
import com.oracle.truffle.r.runtime.nodes.RBaseNode;
/**
@@ -237,6 +238,16 @@ public class PromiseHelperNode extends RBaseNode {
return promise.getValue();
}
Object obj = generateValueDefaultSlowPath(frame, promise);
+ // if the value is temporary, we increment the reference count. The reason is that
+ // temporary values are considered available to be reused and altered (e.g. as a
+ // result of arithmetic operation), which is what we do not want to happen to a
+ // value that we are saving as the promise result.
+ if (obj instanceof RShareable) {
+ RShareable shareable = (RShareable) obj;
+ if (shareable.isTemporary()) {
+ shareable.incRefCount();
+ }
+ }
promise.setValue(obj);
return obj;
}
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index 83b6bca2e5..914b2734b3 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -77177,6 +77177,10 @@ Error in get("x") : object 'x' not found
#{ x <- function(){3} ; gg <- function() { assign("x", 4) ; g <- function() { if (FALSE) { x <- 2 } ; f <- function() { h <- function() { x() } ; h() } ; f() } ; g() } ; gg() }
[1] 3
+##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
+#{ delayedAssign('x', c(1,2,3)); x/180; x }
+[1] 1 2 3
+
##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
#{ f <- function(x) { assign('e', function() {x}, parent.frame()) } ; a <- 1 ; f( a ) ; a <- 10 ; e() }
[1] 10
@@ -77217,6 +77221,10 @@ Error in get("x") : object 'x' not found
#{ f <- function(x) { sys.frame(sys.nframe()) } ; a <- 1 ; e <- f( a ) ; a <- 10 ; e$x }
[1] 10
+##com.oracle.truffle.r.test.library.base.TestPromiseOptimizations.testDeoptimization#
+#{ pi/180; pi }
+[1] 3.141593
+
##com.oracle.truffle.r.test.library.base.TestSimpleArithmetic.testArithmeticUpdate#
#{ x <- 3 ; f <- function(z) { if (z) { x <- 1 } ; x <- 1L + x ; x } ; f(FALSE) }
[1] 4
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestPromiseOptimizations.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestPromiseOptimizations.java
index 07a77a76c6..b15e3ead2c 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestPromiseOptimizations.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/base/TestPromiseOptimizations.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, 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
@@ -61,5 +61,11 @@ public class TestPromiseOptimizations extends TestBase {
// "delayedAssign" RFunction: (Special case of but handled by: delayedAssign)
assertEval("{ f <- function(x) { delayedAssign('b', function() {x}, sys.frame(sys.nframe()), parent.frame()); } ; a <- 1 ; f( a ) ; a <- 10 ; b() }");
+
+ // the value of 'pi' promise may look like temporary vector, but the arithmetic operation
+ // must not re-use it for the result
+ assertEval("{ pi/180; pi }");
+ // similar situation as above
+ assertEval("{ delayedAssign('x', c(1,2,3)); x/180; x }");
}
}
--
GitLab