From 1b77a8f491300c31096f7cc00de9f16c6ce6d8f7 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 5 Nov 2016 18:05:57 +0100 Subject: [PATCH] - fixed: Tacking on a return statement should only be done if the function has branches that actually reach the end. Otherwise it may interfere with return type deduction. - used the return check to optimize out unneeded jumps at the end of an if statement's first block. --- src/scripting/codegeneration/codegen.cpp | 52 +++++++++++++++++++++++- src/scripting/codegeneration/codegen.h | 5 +++ src/scripting/vm/vmbuilder.cpp | 7 ++++ src/scripting/zscript/zcc_compile.cpp | 6 +-- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index 4ece6ad4f..5ab66c4c2 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -6879,6 +6879,18 @@ FxExpression *FxSequence::Resolve(FCompileContext &ctx) return this; } +//========================================================================== +// +// FxSequence :: CheckReturn +// +//========================================================================== + +bool FxSequence::CheckReturn() +{ + // a sequence always returns when its last element returns. + return Expressions.Size() > 0 && Expressions.Last()->CheckReturn(); +} + //========================================================================== // // FxSequence :: Emit @@ -7194,6 +7206,24 @@ ExpEmit FxSwitchStatement::Emit(VMFunctionBuilder *build) return ExpEmit(); } +//========================================================================== +// +// FxSequence :: CheckReturn +// +//========================================================================== + +bool FxSwitchStatement::CheckReturn() +{ + //A switch statement returns when it contains no breaks and ends with a return + for (auto line : *Content) + { + if (line->ExprType == EFX_JumpStatement) + { + return false; // Break means that the end of the statement will be reached, Continue cannot happen in the last statement of the last block. + } + } + return Content->Size() > 0 && Content->Last()->CheckReturn(); +} //========================================================================== // @@ -7355,17 +7385,35 @@ ExpEmit FxIfStatement::Emit(VMFunctionBuilder *build) v.Free(build); if (path2 != nullptr) { - size_t path1jump = build->Emit(OP_JMP, 0); + size_t path1jump; + + // if the branch ends with a return we do not need a terminating jmp. + if (!path1->CheckReturn()) path1jump = build->Emit(OP_JMP, 0); + else path1jump = 0xffffffff; // Evaluate second path build->BackpatchToHere(jumpspot); v = path2->Emit(build); v.Free(build); jumpspot = path1jump; } - build->BackpatchToHere(jumpspot); + if (jumpspot != 0xffffffff) build->BackpatchToHere(jumpspot); return ExpEmit(); } + +//========================================================================== +// +// FxIfStatement :: CheckReturn +// +//========================================================================== + +bool FxIfStatement::CheckReturn() +{ + //An if statement returns if both branches return, if present. + return (WhenTrue == nullptr || WhenTrue->CheckReturn()) && + (WhenFalse == nullptr || WhenFalse->CheckReturn()); +} + //========================================================================== // // FxLoopStatement :: Resolve diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index a244e519e..f4426da90 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -297,6 +297,7 @@ public: virtual bool RequestAddress(bool *writable); virtual PPrototype *ReturnProto(); virtual VMFunction *GetDirectFunction(); + virtual bool CheckReturn() { return false; } bool IsNumeric() const { return ValueType != TypeName && ValueType->GetRegCount() == 1 && (ValueType->GetRegType() == REGT_INT || ValueType->GetRegType() == REGT_FLOAT); } bool IsFloat() const { return ValueType->GetRegType() == REGT_FLOAT && ValueType->GetRegCount() == 1; } bool IsInteger() const { return ValueType != TypeName && (ValueType->GetRegType() == REGT_INT); } @@ -1390,6 +1391,7 @@ public: ExpEmit Emit(VMFunctionBuilder *build); void Add(FxExpression *expr) { if (expr != NULL) Expressions.Push(expr); expr->NeedResult = false; } VMFunction *GetDirectFunction(); + bool CheckReturn(); }; //========================================================================== @@ -1438,6 +1440,7 @@ public: ~FxSwitchStatement(); FxExpression *Resolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); + bool CheckReturn(); }; //========================================================================== @@ -1476,6 +1479,7 @@ public: ~FxIfStatement(); FxExpression *Resolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); + bool CheckReturn(); }; //========================================================================== @@ -1589,6 +1593,7 @@ public: FxExpression *Resolve(FCompileContext&); ExpEmit Emit(VMFunctionBuilder *build); VMFunction *GetDirectFunction(); + bool CheckReturn() { return true; } }; //========================================================================== diff --git a/src/scripting/vm/vmbuilder.cpp b/src/scripting/vm/vmbuilder.cpp index 5b792e7cc..f8451e62c 100644 --- a/src/scripting/vm/vmbuilder.cpp +++ b/src/scripting/vm/vmbuilder.cpp @@ -715,6 +715,13 @@ void FFunctionBuildList::Build() FScriptPosition::StrictErrors = !item.FromDecorate; item.Code = item.Code->Resolve(ctx); + if (!item.Code->CheckReturn()) + { + auto newcmpd = new FxCompoundStatement(item.Code->ScriptPosition); + newcmpd->Add(item.Code); + newcmpd->Add(new FxReturnStatement(nullptr, item.Code->ScriptPosition)); + item.Code = newcmpd->Resolve(ctx); + } item.Proto = ctx.ReturnProto; // Make sure resolving it didn't obliterate it. diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index d6bf1d7f3..44b2f489c 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -2464,15 +2464,15 @@ FxExpression *ZCCCompiler::ConvertAST(PClass *cls, ZCC_TreeNode *ast) // This must be done here so that we can check for a trailing return statement. auto x = new FxCompoundStatement(*ast); auto compound = static_cast(ast); - bool isreturn = false; + //bool isreturn = false; auto node = compound->Content; if (node != nullptr) do { x->Add(ConvertNode(node)); - isreturn = node->NodeType == AST_ReturnStmt; + //isreturn = node->NodeType == AST_ReturnStmt; node = static_cast(node->SiblingNext); } while (node != compound->Content); - if (!isreturn) x->Add(new FxReturnStatement(nullptr, *ast)); + //if (!isreturn) x->Add(new FxReturnStatement(nullptr, *ast)); return x; } }