Address CR feedback

MikeHolman · MikeHolman · commit 75f5e8f298b9 · 2017-09-12T17:54:22.000-07:00
diff --git a/lib/Backend/BackendOpCodeAttrAsmJs.cpp b/lib/Backend/BackendOpCodeAttrAsmJs.cpp
@@ -13,8 +13,7 @@ namespace OpCodeAttrAsmJs
         OpNoFallThrough = 1 << 0, // Opcode doesn't fallthrough in flow  and its always jump to the return from this opcode.
         OpHasMultiSizeLayout = 1 << 1,
         OpHasProfiled = 1 << 2,
-        OpProfiled = 1 << 3,
-        OpByteCodeOnly = 1 << 4
+        OpProfiled = 1 << 3
     };
 
     static const int OpcodeAttributesAsmJs[] =
diff --git a/lib/Backend/Sym.h b/lib/Backend/Sym.h
@@ -176,7 +176,6 @@ class StackSym: public Sym
     bool            IsFromByteCodeConstantTable() const { return m_isFromByteCodeConstantTable; }
     void            SetIsFromByteCodeConstantTable() { this->m_isFromByteCodeConstantTable = true; }
     Js::ArgSlot     GetArgSlotNum() const { Assert(HasArgSlotNum()); return m_slotNum; }
-    void            SetArgSlotNum(Js::ArgSlot newNum) { m_slotNum = newNum; }
     bool            HasArgSlotNum() const { return !!(m_isArgSlotSym | m_isArgSlotRegSym); }
     void            IncrementArgSlotNum();
     void            DecrementArgSlotNum();
diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -4948,15 +4948,15 @@ namespace Js
             DynamicProfileInfoList * profileInfoList = GetScriptContext()->GetProfileInfoList();
             if (profileInfoList)
             {
-                for (auto iter = profileInfoList->GetEditingIterator(); iter.IsValid(); iter.Next())
+                FOREACH_SLISTBASE_ENTRY_EDITING(Field(DynamicProfileInfo*), info, profileInfoList, iter)
                 {
-                    DynamicProfileInfo * info = iter.Data();
                     if (info->HasFunctionBody() && info->GetFunctionBody() == this)
                     {
                         iter.UnlinkCurrent();
                         break;
                     }
                 }
+                NEXT_SLISTBASE_ENTRY_EDITING;
             }
 #endif
             this->dynamicProfileInfo = nullptr;
diff --git a/lib/Runtime/ByteCode/OpCodesAsmJs.h b/lib/Runtime/ByteCode/OpCodesAsmJs.h
@@ -21,7 +21,7 @@
 
 #define MACRO_WMS_PROFILED( opcode, layout, attr) \
     MACRO_WMS(opcode, layout, OpHasProfiled|attr) \
-    MACRO_WMS(Profiled##opcode, Profiled##layout, OpByteCodeOnly|OpProfiled|attr) \
+    MACRO_WMS(Profiled##opcode, Profiled##layout, OpProfiled|attr) \
 
 
 //              (   OpCodeAsmJs              , LayoutAsmJs     , OpCodeAttrAsmJs )
diff --git a/lib/Runtime/Language/DynamicProfileInfo.cpp b/lib/Runtime/Language/DynamicProfileInfo.cpp
@@ -217,7 +217,11 @@ namespace Js
     bool DynamicProfileInfo::IsEnabled(const FunctionBody *const functionBody)
     {
         Assert(functionBody);
-        return IsEnabled_OptionalFunctionBody(functionBody, functionBody->GetScriptContext());
+        return (IsEnabled_OptionalFunctionBody(functionBody, functionBody->GetScriptContext())
+#ifdef ENABLE_WASM
+            && !(PHASE_TRACE1(Js::WasmInOutPhase) && functionBody->IsWasmFunction())
+#endif
+            );
     }
 
     bool DynamicProfileInfo::IsEnabled_OptionalFunctionBody(const FunctionBody *const functionBody, const ScriptContext *const scriptContext)
@@ -248,7 +252,11 @@ namespace Js
     bool DynamicProfileInfo::IsEnabled(const Js::Phase phase, const FunctionBody *const functionBody)
     {
         Assert(functionBody);
-        return IsEnabled_OptionalFunctionBody(phase, functionBody, functionBody->GetScriptContext());
+        return (IsEnabled_OptionalFunctionBody(phase, functionBody, functionBody->GetScriptContext())
+#ifdef ENABLE_WASM
+            && !(PHASE_TRACE1(Js::WasmInOutPhase) && functionBody->IsWasmFunction())
+#endif
+            );
     }
 
     bool DynamicProfileInfo::IsEnabled_OptionalFunctionBody(
@@ -414,7 +422,7 @@ namespace Js
     {
         if (!callerBody || !callerBody->GetIsAsmjsMode() || !calleeBody || !calleeBody->GetIsAsmjsMode())
         {
-            AssertMsg(UNREACHED, "Call to RecordAsmJsCallSiteInfo without 2 wasm function body");
+            AssertMsg(UNREACHED, "Call to RecordAsmJsCallSiteInfo without two asm.js/wasm FunctionBody");
             return;
         }
         
diff --git a/lib/WasmReader/WasmByteCodeGenerator.cpp b/lib/WasmReader/WasmByteCodeGenerator.cpp
@@ -965,7 +965,7 @@ EmitInfo WasmBytecodeGenerator::EmitCall()
         calleeSignature = calleeInfo->GetSignature();
         // currently only handle inlining internal function calls
         // in future we can expand to all calls by adding checks in inliner and falling back to call in case ScriptFunction doesn't match
-        if (GetReader()->m_currentNode.call.funcType == FunctionIndexTypes::Function)
+        if (GetReader()->m_currentNode.call.funcType == FunctionIndexTypes::Function && !PHASE_TRACE1(Js::WasmInOutPhase))
         {
             profileId = GetNextProfileId();
         }
@@ -1135,10 +1135,7 @@ EmitInfo WasmBytecodeGenerator::EmitCall()
             default:
                 throw WasmCompilationException(_u("Unknown call return type %u"), retInfo.type);
             }
-            if (retInfo.type != WasmTypes::Void)
-            {
-                retInfo.location = GetRegisterSpace(retInfo.type)->AcquireTmpRegister();
-            }
+            retInfo.location = GetRegisterSpace(retInfo.type)->AcquireTmpRegister();
             m_writer->AsmReg2(convertOp, retInfo.location, varRetReg);
         }
     }
diff --git a/test/wasm/inlining.baseline b/test/wasm/inlining.baseline
@@ -0,0 +1,2 @@
+-1970372561
+-1970372561
diff --git a/test/wasm/rlexe.xml b/test/wasm/rlexe.xml
@@ -158,6 +158,13 @@
     <tags>exclude_jshost,exclude_win7</tags>
   </default>
 </test>
+<test>
+  <default>
+    <files>inlining.js</files>
+    <baseline>inlining.baseline</baseline>
+    <tags>exclude_jshost,exclude_win7,exclude_xplat</tags>
+  </default>
+</test>
 <test>
   <default>
     <files>params.js</files>

Original file line number	Diff line number	Diff line change
`@@ -4948,15 +4948,15 @@ namespace Js`
`4948`	`4948`	`DynamicProfileInfoList * profileInfoList = GetScriptContext()->GetProfileInfoList();`
`4949`	`4949`	`if (profileInfoList)`
`4950`	`4950`	`{`
`4951`		`- for (auto iter = profileInfoList->GetEditingIterator(); iter.IsValid(); iter.Next())`
	`4951`	`+ FOREACH_SLISTBASE_ENTRY_EDITING(Field(DynamicProfileInfo*), info, profileInfoList, iter)`
`4952`	`4952`	`{`
`4953`		`- DynamicProfileInfo * info = iter.Data();`
`4954`	`4953`	`if (info->HasFunctionBody() && info->GetFunctionBody() == this)`
`4955`	`4954`	`{`
`4956`	`4955`	`iter.UnlinkCurrent();`
`4957`	`4956`	`break;`
`4958`	`4957`	`}`
`4959`	`4958`	`}`
	`4959`	`+ NEXT_SLISTBASE_ENTRY_EDITING;`
`4960`	`4960`	`}`
`4961`	`4961`	`#endif`
`4962`	`4962`	`this->dynamicProfileInfo = nullptr;`
Original file line number	Diff line number	Diff line change
`@@ -217,7 +217,11 @@ namespace Js`
`217`	`217`	`bool DynamicProfileInfo::IsEnabled(const FunctionBody *const functionBody)`
`218`	`218`	`{`
`219`	`219`	`Assert(functionBody);`
`220`		`- return IsEnabled_OptionalFunctionBody(functionBody, functionBody->GetScriptContext());`
	`220`	`+ return (IsEnabled_OptionalFunctionBody(functionBody, functionBody->GetScriptContext())`
	`221`	`+#ifdef ENABLE_WASM`
	`222`	`+ && !(PHASE_TRACE1(Js::WasmInOutPhase) && functionBody->IsWasmFunction())`
	`223`	`+#endif`
	`224`	`+ );`
`221`	`225`	`}`
`222`	`226`
`223`	`227`	`bool DynamicProfileInfo::IsEnabled_OptionalFunctionBody(const FunctionBody const functionBody, const ScriptContext const scriptContext)`
`@@ -248,7 +252,11 @@ namespace Js`
`248`	`252`	`bool DynamicProfileInfo::IsEnabled(const Js::Phase phase, const FunctionBody *const functionBody)`
`249`	`253`	`{`
`250`	`254`	`Assert(functionBody);`
`251`		`- return IsEnabled_OptionalFunctionBody(phase, functionBody, functionBody->GetScriptContext());`
	`255`	`+ return (IsEnabled_OptionalFunctionBody(phase, functionBody, functionBody->GetScriptContext())`
	`256`	`+#ifdef ENABLE_WASM`
	`257`	`+ && !(PHASE_TRACE1(Js::WasmInOutPhase) && functionBody->IsWasmFunction())`
	`258`	`+#endif`
	`259`	`+ );`
`252`	`260`	`}`
`253`	`261`
`254`	`262`	`bool DynamicProfileInfo::IsEnabled_OptionalFunctionBody(`
`@@ -414,7 +422,7 @@ namespace Js`
`414`	`422`	`{`
`415`	`423`	`if (!callerBody \|\| !callerBody->GetIsAsmjsMode() \|\| !calleeBody \|\| !calleeBody->GetIsAsmjsMode())`
`416`	`424`	`{`
`417`		`- AssertMsg(UNREACHED, "Call to RecordAsmJsCallSiteInfo without 2 wasm function body");`
	`425`	`+ AssertMsg(UNREACHED, "Call to RecordAsmJsCallSiteInfo without two asm.js/wasm FunctionBody");`
`418`	`426`	`return;`
`419`	`427`	`}`
`420`	`428`
Original file line number	Diff line number	Diff line change
`@@ -965,7 +965,7 @@ EmitInfo WasmBytecodeGenerator::EmitCall()`
`965`	`965`	`calleeSignature = calleeInfo->GetSignature();`
`966`	`966`	`// currently only handle inlining internal function calls`
`967`	`967`	`// in future we can expand to all calls by adding checks in inliner and falling back to call in case ScriptFunction doesn't match`
`968`		`- if (GetReader()->m_currentNode.call.funcType == FunctionIndexTypes::Function)`
	`968`	`+ if (GetReader()->m_currentNode.call.funcType == FunctionIndexTypes::Function && !PHASE_TRACE1(Js::WasmInOutPhase))`
`969`	`969`	`{`
`970`	`970`	`profileId = GetNextProfileId();`
`971`	`971`	`}`
`@@ -1135,10 +1135,7 @@ EmitInfo WasmBytecodeGenerator::EmitCall()`
`1135`	`1135`	`default:`
`1136`	`1136`	`throw WasmCompilationException(_u("Unknown call return type %u"), retInfo.type);`
`1137`	`1137`	`}`
`1138`		`- if (retInfo.type != WasmTypes::Void)`
`1139`		`- {`
`1140`		`- retInfo.location = GetRegisterSpace(retInfo.type)->AcquireTmpRegister();`
`1141`		`- }`
	`1138`	`+ retInfo.location = GetRegisterSpace(retInfo.type)->AcquireTmpRegister();`
`1142`	`1139`	`m_writer->AsmReg2(convertOp, retInfo.location, varRetReg);`
`1143`	`1140`	`}`
`1144`	`1141`	`}`