From 5e6f06cf59e508e801cc0fa094938707a66c956f Mon Sep 17 00:00:00 2001 From: Aaron Gutierrez Date: Thu, 11 May 2017 15:42:11 -0400 Subject: [PATCH] Fixed issue with aliasing local variables ISPC++ now produces valid code, or an appropriate error message, for all of my test cases. --- expr.cpp | 29 ++++++++++++++++++++++ expr.h | 1 + func.cpp | 52 ++++++++++++++++++++++++++++++--------- func.h | 2 +- stmt.cpp | 9 ++++++- sym.cpp | 12 +++++---- sym.h | 2 +- tests_ispcpp/Makefile | 2 +- tests_ispcpp/hello.cpp | 2 +- tests_ispcpp/varying.cpp | 27 ++++++++++++++++++++ tests_ispcpp/varying.ispc | 14 +++++++++++ type.cpp | 8 +++--- 12 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 tests_ispcpp/varying.cpp create mode 100644 tests_ispcpp/varying.ispc diff --git a/expr.cpp b/expr.cpp index 8fe870c1..4d04a298 100644 --- a/expr.cpp +++ b/expr.cpp @@ -8099,6 +8099,11 @@ SymbolExpr::ReplacePolyType(const PolyType *from, const Type *to) { if (!symbol) return NULL; + Symbol *tmp = m->symbolTable->LookupVariable(symbol->name.c_str()); + if (tmp) { + tmp->parentFunction = symbol->parentFunction; + symbol = tmp; + } if (Type::EqualForReplacement(symbol->type->GetBaseType(), from)) { symbol->type = PolyType::ReplaceType(symbol->type, to); @@ -8176,6 +8181,14 @@ FunctionSymbolExpr::Optimize() { return this; } +Expr * +FunctionSymbolExpr::ReplacePolyType(const PolyType *from, const Type *to) { + // force re-evaluation of overloaded type + this->triedToResolve = false; + + return this; +} + int FunctionSymbolExpr::EstimateCost() const { @@ -8422,6 +8435,16 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, cost[i] += 8 * costScale; continue; } + if (callTypeNC->IsPolymorphicType()) { + const PolyType *callTypeP = + CastType(callTypeNC->GetBaseType()); + if (callTypeP->CanBeType(fargTypeNC->GetBaseType()) && + callTypeNC->IsArrayType() == fargTypeNC->IsArrayType() && + callTypeNC->IsPointerType() == fargTypeNC->IsPointerType()){ + cost[i] += 8 * costScale; + continue; + } + } if (fargType->IsVaryingType() && callType->IsUniformType()) { // Here we deal with brodcasting uniform to varying. // callType - varying and fargType - uniform is forbidden. @@ -8548,6 +8571,12 @@ FunctionSymbolExpr::ResolveOverloads(SourcePos argPos, return true; } else if (matches.size() > 1) { + for (size_t i=0; iIsPolymorphicType()) { + matchingFunc = matches[0]; + return true; + } + } // Multiple matches: ambiguous std::string candidateMessage = lGetOverloadCandidateMessage(matches, argTypes, argCouldBeNULL); diff --git a/expr.h b/expr.h index e80926e9..0fd44158 100644 --- a/expr.h +++ b/expr.h @@ -761,6 +761,7 @@ public: Symbol *GetBaseSymbol() const; Expr *TypeCheck(); Expr *Optimize(); + Expr *ReplacePolyType(const PolyType *from, const Type *to); void Print() const; int EstimateCost() const; llvm::Constant *GetConstant(const Type *type) const; diff --git a/func.cpp b/func.cpp index 9dca2b34..6d6fdbb9 100644 --- a/func.cpp +++ b/func.cpp @@ -90,7 +90,7 @@ #endif #include -Function::Function(Symbol *s, Stmt *c) { +Function::Function(Symbol *s, Stmt *c, bool typecheck) { sym = s; code = c; @@ -98,13 +98,15 @@ Function::Function(Symbol *s, Stmt *c) { Assert(maskSymbol != NULL); if (code != NULL) { - code = TypeCheck(code); + if (typecheck) { + code = TypeCheck(code); - if (code != NULL && g->debugPrint) { - printf("After typechecking function \"%s\":\n", - sym->name.c_str()); - code->Print(0); - printf("---------------------\n"); + if (code != NULL && g->debugPrint) { + printf("After typechecking function \"%s\":\n", + sym->name.c_str()); + code->Print(0); + printf("---------------------\n"); + } } if (code != NULL) { @@ -641,7 +643,6 @@ Function::IsPolyFunction() const { return false; } - std::vector * Function::ExpandPolyArguments(SymbolTable *symbolTable) const { Assert(symbolTable != NULL); @@ -659,30 +660,57 @@ Function::ExpandPolyArguments(SymbolTable *symbolTable) const { } const FunctionType *ft = CastType(versions[i]->type); + symbolTable->PushScope(); + + Symbol *s = symbolTable->LookupFunction(versions[i]->name.c_str(), ft); Stmt *ncode = (Stmt*)CopyAST(code); + Function *f = new Function(s, ncode, false); + + for (size_t j=0; jargs[j] = new Symbol(*args[j]); + symbolTable->AddVariable(f->args[j], false); + } + for (int j=0; jGetNumParameters(); j++) { if (func->GetParameterType(j)->IsPolymorphicType()) { const PolyType *from = CastType( func->GetParameterType(j)->GetBaseType()); - ncode = (Stmt*)TranslatePoly(ncode, from, + f->code = (Stmt*)TranslatePoly(f->code, from, ft->GetParameterType(j)->GetBaseType()); if (g->debugPrint) { printf("%s after replacing %s with %s:\n\n", sym->name.c_str(), from->GetString().c_str(), ft->GetParameterType(j)->GetBaseType()->GetString().c_str()); - ncode->Print(0); + f->code->Print(0); printf("------------------------------------------\n\n"); } } } - Symbol *s = symbolTable->LookupFunction(versions[i]->name.c_str(), ft); + // we didn't typecheck before, now we can + f->code = TypeCheck(f->code); - expanded->push_back(new Function(s, ncode)); + f->code = Optimize(f->code); + + if (g->debugPrint) { + printf("After optimizing expanded function \"%s\":\n", + f->sym->name.c_str()); + f->code->Print(0); + printf("---------------------\n"); + } + + + + symbolTable->PopScope(); + + + + + expanded->push_back(f); } return expanded; diff --git a/func.h b/func.h index 86d801f4..87013e2b 100644 --- a/func.h +++ b/func.h @@ -44,7 +44,7 @@ class Function { public: - Function(Symbol *sym, Stmt *code); + Function(Symbol *sym, Stmt *code, bool typecheck=true); const Type *GetReturnType() const; const FunctionType *GetType() const; diff --git a/stmt.cpp b/stmt.cpp index 0d84c704..bab93ecc 100644 --- a/stmt.cpp +++ b/stmt.cpp @@ -559,7 +559,8 @@ DeclStmt::TypeCheck() { // an int as the constValue later... const Type *type = vars[i].sym->type; if (CastType(type) != NULL || - CastType(type) != NULL) { + CastType(type) != NULL || + CastType(type) != NULL) { // If it's an expr list with an atomic type, we'll later issue // an error. Need to leave vars[i].init as is in that case so // it is in fact caught later, though. @@ -577,9 +578,15 @@ DeclStmt::TypeCheck() { Stmt * DeclStmt::ReplacePolyType(const PolyType *from, const Type *to) { for (size_t i = 0; i < vars.size(); i++) { + vars[i].sym = new Symbol(*vars[i].sym); + m->symbolTable->AddVariable(vars[i].sym, false); Symbol *s = vars[i].sym; if (Type::EqualForReplacement(s->type->GetBaseType(), from)) { s->type = PolyType::ReplaceType(s->type, to); + + // this typecast *should* be valid after typechecking + vars[i].init = TypeConvertExpr(vars[i].init, s->type, + "initializer"); } } diff --git a/sym.cpp b/sym.cpp index 48ee06f7..796f5f02 100644 --- a/sym.cpp +++ b/sym.cpp @@ -95,14 +95,14 @@ SymbolTable::PopScope() { bool -SymbolTable::AddVariable(Symbol *symbol) { +SymbolTable::AddVariable(Symbol *symbol, bool issueScopeWarning) { Assert(symbol != NULL); // Check to see if a symbol of the same name has already been declared. for (int i = (int)variables.size() - 1; i >= 0; --i) { SymbolMapType &sm = *(variables[i]); if (sm.find(symbol->name) != sm.end()) { - if (i == (int)variables.size()-1) { + if (i == (int)variables.size()-1 && issueScopeWarning) { // If a symbol of the same name was declared in the // same scope, it's an error. Error(symbol->pos, "Ignoring redeclaration of symbol \"%s\".", @@ -112,9 +112,11 @@ SymbolTable::AddVariable(Symbol *symbol) { else { // Otherwise it's just shadowing something else, which // is legal but dangerous.. - Warning(symbol->pos, - "Symbol \"%s\" shadows symbol declared in outer scope.", - symbol->name.c_str()); + if (issueScopeWarning) { + Warning(symbol->pos, + "Symbol \"%s\" shadows symbol declared in outer scope.", + symbol->name.c_str()); + } (*variables.back())[symbol->name] = symbol; return true; } diff --git a/sym.h b/sym.h index 41973c72..620fb172 100644 --- a/sym.h +++ b/sym.h @@ -141,7 +141,7 @@ public: with a symbol defined at the same scope. (Symbols may shaodow symbols in outer scopes; a warning is issued in this case, but this method still returns true.) */ - bool AddVariable(Symbol *symbol); + bool AddVariable(Symbol *symbol, bool issueScopeWarning=true); /** Looks for a variable with the given name in the symbol table. This method searches outward from the innermost scope to the outermost, diff --git a/tests_ispcpp/Makefile b/tests_ispcpp/Makefile index e30bfc2d..7881a592 100644 --- a/tests_ispcpp/Makefile +++ b/tests_ispcpp/Makefile @@ -1,5 +1,5 @@ CXX=g++ -CXXFLAGS=-std=c++11 +CXXFLAGS=-std=c++11 -O2 ISPC=../ispc ISPCFLAGS=--target=sse4-x2 -O2 --arch=x86-64 diff --git a/tests_ispcpp/hello.cpp b/tests_ispcpp/hello.cpp index 219c7da8..06e0cd51 100644 --- a/tests_ispcpp/hello.cpp +++ b/tests_ispcpp/hello.cpp @@ -1,7 +1,7 @@ #include #include -#include "hello.ispc.h" +#include "hello.h" int main() { float A[100]; diff --git a/tests_ispcpp/varying.cpp b/tests_ispcpp/varying.cpp new file mode 100644 index 00000000..d56159a6 --- /dev/null +++ b/tests_ispcpp/varying.cpp @@ -0,0 +1,27 @@ +#include +#include + +#include "varying.h" + +int main() { + float A[256]; + double B[256]; + double outA[256]; + double outB[256]; + + + for (int i=0; i<256; i++) { + A[i] = 1. / (i+1); + B[i] = 1. / (i+1); + } + + ispc::square(256, (float*)&A, (double*)&outA); + + ispc::square(256, (double*)&B, (double*)&outB); + + for (int i=0; i<256; i++) { + printf("float: %.16f\tdouble: %.16f\n", outA[i], outB[i]); + } + + return 0; +} diff --git a/tests_ispcpp/varying.ispc b/tests_ispcpp/varying.ispc new file mode 100644 index 00000000..3657fee6 --- /dev/null +++ b/tests_ispcpp/varying.ispc @@ -0,0 +1,14 @@ +floating foo(const uniform int a, floating b) { + floating out = b; + for (int i = 1; iIsVaryingType()) t = t->GetAsVaryingType(); - fprintf(stderr, "Replacing type \"%s\" with \"%s\"\n", - from->GetString().c_str(), - t->GetString().c_str()); + if (g->debugPrint) { + fprintf(stderr, "Replacing type \"%s\" with \"%s\"\n", + from->GetString().c_str(), + t->GetString().c_str()); + } return t; }