From 2cc48ec378e438eaba6f79a78be90b2cc1585482 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Thu, 17 Nov 2016 16:44:41 +0100 Subject: [PATCH] - implemented code generation for stack variables. - fixed code generation for using local variables as array index. This must use a different register for the array element offset because the original register may not be overwritten. --- src/p_mobj.cpp | 7 + src/scripting/codegeneration/codegen.cpp | 292 ++++++++++++++++++----- src/scripting/codegeneration/codegen.h | 35 ++- src/scripting/vm/vm.h | 2 +- src/scripting/vm/vmbuilder.cpp | 7 + src/scripting/vm/vmbuilder.h | 16 ++ src/scripting/vm/vmframe.cpp | 6 +- src/scripting/zscript/zcc_compile.cpp | 40 ++-- wadsrc/static/zscript/actor.txt | 1 + 9 files changed, 310 insertions(+), 96 deletions(-) diff --git a/src/p_mobj.cpp b/src/p_mobj.cpp index 02c2a3578..8029e24c0 100644 --- a/src/p_mobj.cpp +++ b/src/p_mobj.cpp @@ -6829,6 +6829,13 @@ const char *AActor::GetTag(const char *def) const } } +DEFINE_ACTION_FUNCTION(AActor, GetTag) +{ + PARAM_SELF_PROLOGUE(AActor); + PARAM_STRING(def); + ACTION_RETURN_STRING(self->GetTag(def.Len() == 0? nullptr : def.GetChars())); +} + void AActor::SetTag(const char *def) { if (def == NULL || *def == 0) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index 44d7cda88..c522b2145 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -5084,9 +5084,18 @@ FxExpression *FxIdentifier::Resolve(FCompileContext& ctx) FxLocalVariableDeclaration *local = ctx.FindLocalVariable(Identifier); if (local != nullptr) { - auto x = new FxLocalVariable(local, ScriptPosition); - delete this; - return x->Resolve(ctx); + if (local->ValueType->GetRegType() != REGT_NIL) + { + auto x = new FxLocalVariable(local, ScriptPosition); + delete this; + return x->Resolve(ctx); + } + else + { + auto x = new FxStackVariable(local->ValueType, local->StackOffset, ScriptPosition); + delete this; + return x->Resolve(ctx); + } } if (Identifier == NAME_Default) @@ -5584,6 +5593,106 @@ ExpEmit FxGlobalVariable::Emit(VMFunctionBuilder *build) } +//========================================================================== +// +// +// +//========================================================================== + +FxStackVariable::FxStackVariable(PType *type, int offset, const FScriptPosition &pos) + : FxExpression(EFX_StackVariable, pos) +{ + membervar = new PField(NAME_None, type, 0, offset); + AddressRequested = false; + AddressWritable = true; // must be true unless classx tells us otherwise if requested. +} + +//========================================================================== +// +// force delete the PField because we know we won't need it anymore +// and it won't get GC'd until the compiler finishes. +// +//========================================================================== + +FxStackVariable::~FxStackVariable() +{ + // Q: Is this good or bad? Needs testing if this is fine or better left to the GC anyway. DObject's destructor is anything but cheap. + membervar->ObjectFlags |= OF_YesReallyDelete; + delete membervar; +} + +//========================================================================== +// +// +//========================================================================== + +void FxStackVariable::ReplaceField(PField *newfield) +{ + membervar->ObjectFlags |= OF_YesReallyDelete; + delete membervar; + membervar = newfield; +} + +//========================================================================== +// +// +// +//========================================================================== + +bool FxStackVariable::RequestAddress(FCompileContext &ctx, bool *writable) +{ + AddressRequested = true; + if (writable != nullptr) *writable = AddressWritable && !ctx.CheckReadOnly(membervar->Flags); + return true; +} + +//========================================================================== +// +// +// +//========================================================================== + +FxExpression *FxStackVariable::Resolve(FCompileContext &ctx) +{ + CHECKRESOLVED(); + ValueType = membervar->Type; + return this; +} + +ExpEmit FxStackVariable::Emit(VMFunctionBuilder *build) +{ + int offsetreg = -1; + + if (membervar->Offset != 0) offsetreg = build->GetConstantInt((int)membervar->Offset); + + if (AddressRequested) + { + ExpEmit obj(build, REGT_POINTER); + if (offsetreg >= 0) + { + build->Emit(OP_ADDA_RK, obj.RegNum, build->FramePointer.RegNum, offsetreg); + return obj; + } + else return build->FramePointer; + } + ExpEmit loc(build, membervar->Type->GetRegType(), membervar->Type->GetRegCount()); + + if (membervar->BitValue == -1) + { + if (offsetreg == -1) offsetreg = build->GetConstantInt(0); + build->Emit(membervar->Type->GetLoadOp(), loc.RegNum, build->FramePointer.RegNum, offsetreg); + } + else + { + ExpEmit obj(build, REGT_POINTER); + if (offsetreg >= 0) build->Emit(OP_ADDA_RK, obj.RegNum, build->FramePointer.RegNum, offsetreg); + obj.Free(build); + build->Emit(OP_LBIT, loc.RegNum, obj.RegNum, 1 << membervar->BitValue); + } + return loc; +} + + //========================================================================== // // @@ -5684,6 +5793,16 @@ FxExpression *FxStructMember::Resolve(FCompileContext &ctx) classx = nullptr; return x; } + else if (classx->ExprType == EFX_StackVariable) + { + auto parentfield = static_cast(classx)->membervar; + auto newfield = new PField(membervar->SymbolName, membervar->Type, membervar->Flags | parentfield->Flags, membervar->Offset + parentfield->Offset, membervar->BitValue); + static_cast(classx)->ReplaceField(newfield); + classx->isresolved = false; // re-resolve the parent so it can also check if it can be optimized away. + auto x = classx->Resolve(ctx); + classx = nullptr; + return x; + } else if (classx->ExprType == EFX_LocalVariable && classx->IsVector()) // vectors are a special case because they are held in registers { // since this is a vector, all potential things that may get here are single float or an xy-vector. @@ -5831,7 +5950,7 @@ FxExpression *FxArrayElement::Resolve(FCompileContext &ctx) return nullptr; } } - if (index->ValueType->GetRegType() != REGT_INT && index->ValueType != TypeName) + if (!index->IsInteger()) { ScriptPosition.Message(MSG_ERROR, "Array index must be integer"); delete this; @@ -5854,16 +5973,10 @@ FxExpression *FxArrayElement::Resolve(FCompileContext &ctx) delete this; return nullptr; } + // Todo: optimize out the array. } ValueType = arraytype->ElementType; - if (ValueType->GetRegType() != REGT_INT && ValueType->GetRegType() != REGT_FLOAT) - { - // int arrays only for now - ScriptPosition.Message(MSG_ERROR, "Only numeric arrays are supported."); - delete this; - return nullptr; - } if (!Array->RequestAddress(ctx, &AddressWritable)) { ScriptPosition.Message(MSG_ERROR, "Unable to dereference array."); @@ -5902,7 +6015,18 @@ ExpEmit FxArrayElement::Emit(VMFunctionBuilder *build) { if (indexval != 0) { - build->Emit(OP_ADDA_RK, start.RegNum, start.RegNum, build->GetConstantInt(indexval)); + if (!start.Fixed) + { + build->Emit(OP_ADDA_RK, start.RegNum, start.RegNum, build->GetConstantInt(indexval)); + } + else + { + // do not clobber local variables. + ExpEmit temp(build, start.RegType); + build->Emit(OP_ADDA_RK, temp.RegNum, start.RegNum, build->GetConstantInt(indexval)); + start.Free(build); + start = temp; + } } } else @@ -5915,21 +6039,40 @@ ExpEmit FxArrayElement::Emit(VMFunctionBuilder *build) { ExpEmit indexv(index->Emit(build)); ExpEmit indexwork = indexv.Fixed ? ExpEmit(build, indexv.RegType) : indexv; + build->Emit(OP_BOUND, indexv.RegNum, arraytype->ElementCount); + int shiftbits = 0; while (1u << shiftbits < arraytype->ElementSize) { shiftbits++; } - assert(1u << shiftbits == arraytype->ElementSize && "Element sizes other than power of 2 are not implemented"); - build->Emit(OP_BOUND, indexv.RegNum, arraytype->ElementCount); - if (shiftbits > 0) + if (1u << shiftbits == arraytype->ElementSize) { - build->Emit(OP_SLL_RI, indexwork.RegNum, indexv.RegNum, shiftbits); + if (shiftbits > 0) + { + build->Emit(OP_SLL_RI, indexwork.RegNum, indexv.RegNum, shiftbits); + } + } + else + { + // A shift won't do, so use a multiplication + build->Emit(OP_MUL_RK, indexwork.RegNum, indexv.RegNum, build->GetConstantInt(arraytype->ElementSize)); } if (AddressRequested) { - build->Emit(OP_ADDA_RR, start.RegNum, start.RegNum, indexwork.RegNum); + if (!start.Fixed) + { + build->Emit(OP_ADDA_RR, start.RegNum, start.RegNum, indexwork.RegNum); + } + else + { + // do not clobber local variables. + ExpEmit temp(build, start.RegType); + build->Emit(OP_ADDA_RR, temp.RegNum, start.RegNum, indexwork.RegNum); + start.Free(build); + start = temp; + } } else { @@ -8413,7 +8556,7 @@ FxLocalVariableDeclaration::FxLocalVariableDeclaration(PType *type, FName name, VarFlags = varflags; Name = name; RegCount = type == TypeVector2 ? 2 : type == TypeVector3 ? 3 : 1; - Init = initval == nullptr? nullptr : new FxTypeCast(initval, type, false); + Init = initval; } FxLocalVariableDeclaration::~FxLocalVariableDeclaration() @@ -8424,75 +8567,96 @@ FxLocalVariableDeclaration::~FxLocalVariableDeclaration() FxExpression *FxLocalVariableDeclaration::Resolve(FCompileContext &ctx) { CHECKRESOLVED(); - SAFE_RESOLVE_OPT(Init, ctx); if (ctx.Block == nullptr) { ScriptPosition.Message(MSG_ERROR, "Variable declaration outside compound statement"); delete this; return nullptr; } + if (ValueType->RegType == REGT_NIL) + { + auto sfunc = static_cast(ctx.Function->Variants[0].Implementation); + StackOffset = sfunc->AllocExtraStack(ValueType); + // Todo: Process the compound initializer once implemented. + } + else + { + if (Init) Init = new FxTypeCast(Init, ValueType, false); + SAFE_RESOLVE_OPT(Init, ctx); + } ctx.Block->LocalVars.Push(this); return this; } ExpEmit FxLocalVariableDeclaration::Emit(VMFunctionBuilder *build) { - if (Init == nullptr) + if (ValueType->RegType != REGT_NIL) { - RegNum = build->Registers[ValueType->GetRegType()].Get(RegCount); - } - else - { - ExpEmit emitval = Init->Emit(build); - - int regtype = emitval.RegType; - if (regtype < REGT_INT || regtype > REGT_TYPE) + if (Init == nullptr) { - ScriptPosition.Message(MSG_ERROR, "Attempted to assign a non-value"); - return ExpEmit(); - } - if (emitval.Konst) - { - auto constval = static_cast(Init); - RegNum = build->Registers[regtype].Get(1); - switch (regtype) - { - default: - case REGT_INT: - build->Emit(OP_LK, RegNum, build->GetConstantInt(constval->GetValue().GetInt())); - break; - - case REGT_FLOAT: - build->Emit(OP_LKF, RegNum, build->GetConstantFloat(constval->GetValue().GetFloat())); - break; - - case REGT_POINTER: - build->Emit(OP_LKP, RegNum, build->GetConstantAddress(constval->GetValue().GetPointer(), ATAG_GENERIC)); - break; - - case REGT_STRING: - build->Emit(OP_LKS, RegNum, build->GetConstantString(constval->GetValue().GetString())); - } - emitval.Free(build); - } - else if (Init->ExprType != EFX_LocalVariable) - { - // take over the register that got allocated while emitting the Init expression. - RegNum = emitval.RegNum; + RegNum = build->Registers[ValueType->GetRegType()].Get(RegCount); } else { - ExpEmit out(build, emitval.RegType, emitval.RegCount); - build->Emit(ValueType->GetMoveOp(), out.RegNum, emitval.RegNum); - RegNum = out.RegNum; + ExpEmit emitval = Init->Emit(build); + + int regtype = emitval.RegType; + if (regtype < REGT_INT || regtype > REGT_TYPE) + { + ScriptPosition.Message(MSG_ERROR, "Attempted to assign a non-value"); + return ExpEmit(); + } + if (emitval.Konst) + { + auto constval = static_cast(Init); + RegNum = build->Registers[regtype].Get(1); + switch (regtype) + { + default: + case REGT_INT: + build->Emit(OP_LK, RegNum, build->GetConstantInt(constval->GetValue().GetInt())); + break; + + case REGT_FLOAT: + build->Emit(OP_LKF, RegNum, build->GetConstantFloat(constval->GetValue().GetFloat())); + break; + + case REGT_POINTER: + build->Emit(OP_LKP, RegNum, build->GetConstantAddress(constval->GetValue().GetPointer(), ATAG_GENERIC)); + break; + + case REGT_STRING: + build->Emit(OP_LKS, RegNum, build->GetConstantString(constval->GetValue().GetString())); + } + emitval.Free(build); + } + else if (Init->ExprType != EFX_LocalVariable) + { + // take over the register that got allocated while emitting the Init expression. + RegNum = emitval.RegNum; + } + else + { + ExpEmit out(build, emitval.RegType, emitval.RegCount); + build->Emit(ValueType->GetMoveOp(), out.RegNum, emitval.RegNum); + RegNum = out.RegNum; + } } } + else + { + // Init arrays and structs. + } return ExpEmit(); } void FxLocalVariableDeclaration::Release(VMFunctionBuilder *build) { // Release the register after the containing block gets closed - assert(RegNum != -1); - build->Registers[ValueType->GetRegType()].Return(RegNum, RegCount); + if(RegNum != -1) + { + build->Registers[ValueType->GetRegType()].Return(RegNum, RegCount); + } + // Stack space will not be released because that would make controlled destruction impossible. + // For that all local stack variables need to live for the entire execution of a function. } diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index f1778512f..84ddfcdd8 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -44,6 +44,7 @@ #include "sc_man.h" #include "s_sound.h" #include "actor.h" +#include "vmbuilder.h" #define CHECKRESOLVED() if (isresolved) return this; isresolved=true; @@ -202,17 +203,6 @@ struct ExpVal } }; -struct ExpEmit -{ - ExpEmit() : RegNum(0), RegType(REGT_NIL), RegCount(1), Konst(false), Fixed(false), Final(false), Target(false) {} - ExpEmit(int reg, int type, bool konst = false, bool fixed = false) : RegNum(reg), RegType(type), RegCount(1), Konst(konst), Fixed(fixed), Final(false), Target(false) {} - ExpEmit(VMFunctionBuilder *build, int type, int count = 1); - void Free(VMFunctionBuilder *build); - void Reuse(VMFunctionBuilder *build); - - BYTE RegNum, RegType, RegCount, Konst:1, Fixed:1, Final:1, Target:1; -}; - enum EFxType { EFX_Expression, @@ -282,6 +272,7 @@ enum EFxType EFX_DynamicCast, EFX_GlobalVariable, EFX_Super, + EFX_StackVariable, EFX_COUNT }; @@ -1234,6 +1225,27 @@ public: ExpEmit Emit(VMFunctionBuilder *build); }; +//========================================================================== +// +// FxLocalVariable +// +//========================================================================== + +class FxStackVariable : public FxExpression +{ +public: + PField *membervar; + bool AddressRequested; + bool AddressWritable; + + FxStackVariable(PType *type, int offset, const FScriptPosition&); + ~FxStackVariable(); + void ReplaceField(PField *newfield); + FxExpression *Resolve(FCompileContext&); + bool RequestAddress(FCompileContext &ctx, bool *writable); + ExpEmit Emit(VMFunctionBuilder *build); +}; + //========================================================================== // // FxSelf @@ -1746,6 +1758,7 @@ class FxLocalVariableDeclaration : public FxExpression int VarFlags; int RegCount; public: + int StackOffset = -1; int RegNum = -1; FxLocalVariableDeclaration(PType *type, FName name, FxExpression *initval, int varflags, const FScriptPosition &p); diff --git a/src/scripting/vm/vm.h b/src/scripting/vm/vm.h index d82411d51..c9f75f214 100644 --- a/src/scripting/vm/vm.h +++ b/src/scripting/vm/vm.h @@ -818,7 +818,6 @@ public: void InitExtra(void *addr); void DestroyExtra(void *addr); - void SetExtraSpecial(PType *type, unsigned offset); int AllocExtraStack(PType *type); }; @@ -1047,6 +1046,7 @@ void CallAction(VMFrameStack *stack, VMFunction *vmfunc, AActor *self); #define ACTION_RETURN_VEC3(v) do { DVector3 u = v; if (numret > 0) { assert(ret != nullptr); ret[0].SetVector(u); return 1; } return 0; } while(0) #define ACTION_RETURN_INT(v) do { int u = v; if (numret > 0) { assert(ret != NULL); ret->SetInt(u); return 1; } return 0; } while(0) #define ACTION_RETURN_BOOL(v) ACTION_RETURN_INT(v) +#define ACTION_RETURN_STRING(v) do { FString u = v; if (numret > 0) { assert(ret != NULL); ret->SetString(u); return 1; } return 0; } while(0) // Checks to see what called the current action function #define ACTION_CALL_FROM_ACTOR() (stateinfo == nullptr || stateinfo->mStateType == STATE_Actor) diff --git a/src/scripting/vm/vmbuilder.cpp b/src/scripting/vm/vmbuilder.cpp index 19d89fc6f..721ff5fab 100644 --- a/src/scripting/vm/vmbuilder.cpp +++ b/src/scripting/vm/vmbuilder.cpp @@ -718,6 +718,13 @@ void FFunctionBuildList::Build() FScriptPosition::StrictErrors = !item.FromDecorate; item.Code = item.Code->Resolve(ctx); + // If we need extra space, load the frame pointer into a register so that we do not have to call the wasteful LFP instruction more than once. + if (item.Function->ExtraSpace > 0) + { + buildit.FramePointer = ExpEmit(&buildit, REGT_POINTER); + buildit.FramePointer.Fixed = true; + buildit.Emit(OP_LFP, buildit.FramePointer.RegNum); + } // Make sure resolving it didn't obliterate it. if (item.Code != nullptr) diff --git a/src/scripting/vm/vmbuilder.h b/src/scripting/vm/vmbuilder.h index a5013dc57..d69c97707 100644 --- a/src/scripting/vm/vmbuilder.h +++ b/src/scripting/vm/vmbuilder.h @@ -3,6 +3,19 @@ #include "dobject.h" +class VMFunctionBuilder; + +struct ExpEmit +{ + ExpEmit() : RegNum(0), RegType(REGT_NIL), RegCount(1), Konst(false), Fixed(false), Final(false), Target(false) {} + ExpEmit(int reg, int type, bool konst = false, bool fixed = false) : RegNum(reg), RegType(type), RegCount(1), Konst(konst), Fixed(fixed), Final(false), Target(false) {} + ExpEmit(VMFunctionBuilder *build, int type, int count = 1); + void Free(VMFunctionBuilder *build); + void Reuse(VMFunctionBuilder *build); + + BYTE RegNum, RegType, RegCount, Konst:1, Fixed : 1, Final : 1, Target : 1; +}; + class VMFunctionBuilder { public: @@ -63,6 +76,9 @@ public: // amount of implicit parameters so that proper code can be emitted for method calls int NumImplicits; + // keep the frame pointer, if needed, in a register because the LFP opcode is hideously inefficient, requiring more than 20 instructions on x64. + ExpEmit FramePointer; + private: struct AddrKonst { diff --git a/src/scripting/vm/vmframe.cpp b/src/scripting/vm/vmframe.cpp index e5a302606..b20c8ac89 100644 --- a/src/scripting/vm/vmframe.cpp +++ b/src/scripting/vm/vmframe.cpp @@ -182,15 +182,11 @@ void VMScriptFunction::DestroyExtra(void *addr) } } -void VMScriptFunction::SetExtraSpecial(PType *type, unsigned offset) -{ - type->SetDefaultValue(nullptr, offset, &SpecialInits); -} - int VMScriptFunction::AllocExtraStack(PType *type) { int address = ((ExtraSpace + type->Align - 1) / type->Align) * type->Align; ExtraSpace = address + type->Size; + type->SetDefaultValue(nullptr, address, &SpecialInits); return address; } diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index aec8f5853..596b7d8c4 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -2970,29 +2970,39 @@ FxExpression *ZCCCompiler::ConvertNode(ZCC_TreeNode *ast) auto loc = static_cast(ast); auto node = loc->Vars; FxSequence *list = new FxSequence(*ast); + + PType *ztype = DetermineType(ConvertClass, node, node->Name, loc->Type, true, false); + + if (loc->Type->ArraySize != nullptr) + { + ztype = ResolveArraySize(ztype, loc->Type->ArraySize, &ConvertClass->Symbols); + } + do { - // Type determination must be done for each field to properly handle array definitions. - PType *type = DetermineType(ConvertClass, node, node->Name, loc->Type, true, false); - if (type->IsKindOf(RUNTIME_CLASS(PArray))) + PType *type; + + if (node->ArraySize != nullptr) { - Error(loc, "Local array variables not implemented yet."); + type = ResolveArraySize(ztype, node->ArraySize, &ConvertClass->Symbols); } else { - FxExpression *val; - if (node->InitIsArray) - { - Error(node, "Tried to initialize %s with an array", FName(node->Name).GetChars()); - val = nullptr; - } - else - { - val = node->Init ? ConvertNode(node->Init) : nullptr; - } - list->Add(new FxLocalVariableDeclaration(type, node->Name, val, 0, *node)); // todo: Handle flags in the grammar. + type = ztype; } + FxExpression *val; + if (node->InitIsArray) + { + Error(node, "Compound initializer not implemented yet"); + val = nullptr; + } + else + { + val = node->Init ? ConvertNode(node->Init) : nullptr; + } + list->Add(new FxLocalVariableDeclaration(type, node->Name, val, 0, *node)); // todo: Handle flags in the grammar. + node = static_cast(node->SiblingNext); } while (node != loc->Vars); return list; diff --git a/wadsrc/static/zscript/actor.txt b/wadsrc/static/zscript/actor.txt index f20a0b19e..043ab3ed4 100644 --- a/wadsrc/static/zscript/actor.txt +++ b/wadsrc/static/zscript/actor.txt @@ -66,6 +66,7 @@ class Actor : Thinker native native static float absangle(float ang1, float ang2); native static float GetDefaultSpeed(class type); native void RemoveFromHash(); + native string GetTag(string defstr = ""); native float GetBobOffset(float frac = 0); native void SetDamage(int dmg); native static bool isDehState(state st);