From 16e1c6aede2b31b7459a2be5d9aae6e1489e52bb Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Fri, 3 Aug 2018 11:19:51 +0200
Subject: [PATCH] Fix PE of FrameHelper

---
 .../r/nodes/builtin/base/FrameFunctions.java  | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java
index e617025e13..a217681484 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/FrameFunctions.java
@@ -125,18 +125,33 @@ public class FrameFunctions {
             this.ignoreDotInternal = ignoreDotInternal;
         }
 
-        protected Frame getFrame(VirtualFrame frame, int n) {
+        // Note: the helper methods either return MaterializedFrame or they read some specific
+        // argument from the target frame, but without returning it, in which case we may avoid
+        // frame materialization in some cases. If a method returns both virtual and materialized
+        // frame disguised under common Frame, the compiler may have some issue in finding out that
+        // the virtual frame does not escape.
+
+        protected MaterializedFrame getMaterializedFrame(VirtualFrame frame, int n) {
+            assert access == FrameAccess.MATERIALIZE;
             RCaller c = RArguments.getCall(frame);
             int actualFrame = decodeFrameNumber(c, n);
-            Frame numberedFrame = getNumberedFrame(frame, actualFrame);
-            Frame ret = RInternalError.guaranteeNonNull(numberedFrame);
-            return ret;
+            MaterializedFrame numberedFrame = (MaterializedFrame) getNumberedFrame(frame, actualFrame, true);
+            return RInternalError.guaranteeNonNull(numberedFrame);
+        }
+
+        protected RFunction getFunction(VirtualFrame frame, int n) {
+            assert access == FrameAccess.READ_ONLY;
+            RCaller currentCall = RArguments.getCall(frame);
+            int actualFrame = decodeFrameNumber(currentCall, n);
+            Frame targetFrame = getNumberedFrame(frame, actualFrame, false);
+            return RArguments.getFunction(targetFrame);
         }
 
         protected RCaller getCall(VirtualFrame frame, int n) {
+            assert access == FrameAccess.READ_ONLY;
             RCaller currentCall = RArguments.getCall(frame);
             int actualFrame = decodeFrameNumber(currentCall, n);
-            Frame targetFrame = getNumberedFrame(frame, actualFrame);
+            Frame targetFrame = getNumberedFrame(frame, actualFrame, false);
             return RArguments.getCall(targetFrame);
         }
 
@@ -169,9 +184,9 @@ public class FrameFunctions {
         private static final int ITERATE_LEVELS = 2;
 
         @ExplodeLoop
-        protected Frame getNumberedFrame(VirtualFrame frame, int actualFrame) {
+        protected Frame getNumberedFrame(VirtualFrame frame, int actualFrame, boolean materialize) {
             if (currentFrameProfile.profile(RArguments.getDepth(frame) == actualFrame)) {
-                return frame;
+                return materialize ? frame.materialize() : frame;
             } else {
                 if (RArguments.getDepth(frame) - actualFrame <= ITERATE_LEVELS) {
                     Frame current = frame;
@@ -182,11 +197,12 @@ public class FrameFunctions {
                         }
                     }
                 }
-                return Utils.getStackFrame(access, actualFrame);
+                Frame result = Utils.getStackFrame(access, actualFrame);
+                return materialize ? (MaterializedFrame) result : result;
             }
         }
 
-        private static Frame getCallerFrame(Frame current) {
+        private static MaterializedFrame getCallerFrame(Frame current) {
             Object callerFrame = RArguments.getCallerFrame(current);
             if (callerFrame instanceof CallerFrameClosure) {
                 CallerFrameClosure closure = (CallerFrameClosure) callerFrame;
@@ -194,7 +210,7 @@ public class FrameFunctions {
                 return closure.getMaterializedCallerFrame();
             }
             assert callerFrame == null || callerFrame instanceof Frame;
-            return (Frame) callerFrame;
+            return (MaterializedFrame) callerFrame;
         }
     }
 
@@ -465,8 +481,8 @@ public class FrameFunctions {
             if (zeroProfile.profile(which == 0)) {
                 result = REnvironment.globalEnv();
             } else {
-                Frame callerFrame = getFrameHelper().getFrame(frame, which);
-                result = REnvironment.frameToEnvironment(callerFrame.materialize());
+                MaterializedFrame callerFrame = getFrameHelper().getMaterializedFrame(frame, which);
+                result = REnvironment.frameToEnvironment(callerFrame);
             }
 
             // Deoptimize every promise which is now in this frame, as it might leave it's stack
@@ -498,7 +514,7 @@ public class FrameFunctions {
                 RPairList result = RDataFactory.createPairList();
                 RPairList next = result;
                 for (int i = 1; i < depth; i++) {
-                    MaterializedFrame mf = helper.getNumberedFrame(frame, i).materialize();
+                    MaterializedFrame mf = (MaterializedFrame) helper.getNumberedFrame(frame, i, true);
                     deoptFrameNode.deoptimizeFrame(RArguments.getArguments(mf));
                     next.setCar(REnvironment.frameToEnvironment(mf));
                     if (i != depth - 1) {
@@ -604,8 +620,7 @@ public class FrameFunctions {
         @Specialization
         protected Object sysFunction(VirtualFrame frame, int which) {
             // N.B. Despite the spec, n==0 is treated as the current function
-            Frame callerFrame = helper.getFrame(frame, which);
-            RFunction func = RArguments.getFunction(callerFrame);
+            RFunction func = helper.getFunction(frame, which);
 
             if (func == null) {
                 return RNull.instance;
@@ -732,7 +747,7 @@ public class FrameFunctions {
                 // If the parent frame is the caller frame, we can use more efficient code:
                 return REnvironment.frameToEnvironment(getCallerFrameNode().execute(frame));
             }
-            return REnvironment.frameToEnvironment(getFrameHelper().getNumberedFrame(frame, call.getDepth()).materialize());
+            return REnvironment.frameToEnvironment((MaterializedFrame) getFrameHelper().getNumberedFrame(frame, call.getDepth(), true));
         }
 
         private FrameHelper getFrameHelper() {
-- 
GitLab