From ecdc695b227a3bdcf8e50c9c1b404dfa6592f208 Mon Sep 17 00:00:00 2001 From: Ilia Filippov Date: Mon, 24 Mar 2014 16:20:23 +0400 Subject: [PATCH] Changing overload rules to match C++ behavior: Emit a warning when the best overload match has some number of no-best matching parameters. --- docs/ispc.rst | 4 +- expr.cpp | 57 ++++++++++++++++----- expr.h | 3 +- tests/array-mixed-unif-vary-indexing-2.ispc | 2 +- tests/array-mixed-unif-vary-indexing.ispc | 2 +- tests/array-multidim-gather-scatter.ispc | 2 +- tests/func-overload-max.ispc | 2 +- tests/funcptr-uniform-10.ispc | 2 +- tests/goto-4.ispc | 2 +- tests/max-float-1.ispc | 2 +- tests/max-float-2.ispc | 2 +- tests/min-float-1.ispc | 2 +- tests/min-float-2.ispc | 2 +- tests/typedef-2.ispc | 2 +- 14 files changed, 59 insertions(+), 27 deletions(-) diff --git a/docs/ispc.rst b/docs/ispc.rst index dc65ee2f..e9d248f4 100644 --- a/docs/ispc.rst +++ b/docs/ispc.rst @@ -2935,7 +2935,9 @@ of a function, ``ispc`` uses the following model to choose the best function: each conversion of two types has its cost. ``ispc`` tries to find conversion with the smallest cost. When ``ispc`` can't find any conversion it means that this function is not suitable. Then ``ispc`` sums costs for all arguments and -chooses the function with the smallest final cost. +chooses the function with the smallest final cost. If the chosen function +has some arguments which costs are bigger than their costs in other function +this treats as ambiguous. Costs of type conversions placed from small to big: 1. Parameter types match exactly. diff --git a/expr.cpp b/expr.cpp index b7d6e657..4a1a16db 100644 --- a/expr.cpp +++ b/expr.cpp @@ -8188,13 +8188,15 @@ int FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, const std::vector &argTypes, const std::vector *argCouldBeNULL, - const std::vector *argIsConstant) { + const std::vector *argIsConstant, + int * cost) { int costSum = 0; // In computing the cost function, we only worry about the actual // argument types--using function default parameter values is free for // the purposes here... for (int i = 0; i < (int)argTypes.size(); ++i) { + cost[i] = 0; // The cost imposed by this argument will be a multiple of // costScale, which has a value set so that for each of the cost // buckets, even if all of the function arguments undergo the next @@ -8208,12 +8210,12 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, if (Type::Equal(callType, fargType)) // Perfect match: no cost // Step "1" from documentation - costSum += 0; + cost[i] += 0; else if (argCouldBeNULL && (*argCouldBeNULL)[i] && lArgIsPointerType(fargType)) // Passing NULL to a pointer-typed parameter is also a no-cost operation // Step "1" from documentation - costSum += 0; + cost[i] += 0; else { // If the argument is a compile-time constant, we'd like to // count the cost of various conversions as much lower than the @@ -8232,7 +8234,7 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, // It is possible to pass (vf -> cvfr) // but it is worse than (vf -> vfr) or (cvf -> cvfr) // Step "3" from documentation - costSum += 2 * costScale; + cost[i] += 2 * costScale; } if (!Type::Equal(callType->GetReferenceTarget()->GetAsNonConstType(), fargType->GetReferenceTarget()->GetAsNonConstType())) { @@ -8242,7 +8244,7 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, } // penalty for equal types under reference (vf -> vfr is worse than vf -> vf) // Step "2" from documentation - costSum += 2 * costScale; + cost[i] += 2 * costScale; continue; } const Type *callTypeNP = callType; @@ -8250,7 +8252,7 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, callTypeNP = callType->GetReferenceTarget(); // we can treat vfr as vf for callType with some penalty // Step "5" from documentation - costSum += 2 * costScale; + cost[i] += 2 * costScale; } // Now we deal with references, so we can normalize to non-const types @@ -8263,13 +8265,13 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, if (Type::Equal(callTypeNC, fargTypeNC)) { // The best case: vf -> vf. // Step "4" from documentation - costSum += 1 * costScale; + cost[i] += 1 * costScale; continue; } if (lIsMatchWithTypeWidening(callTypeNC, fargTypeNC)) { // A little bit worse case: vf -> vd. // Step "6" from documentation - costSum += 8 * costScale; + cost[i] += 8 * costScale; continue; } if (fargType->IsVaryingType() && callType->IsUniformType()) { @@ -8278,31 +8280,34 @@ FunctionSymbolExpr::computeOverloadCost(const FunctionType *ftype, if (Type::Equal(callTypeNC->GetAsVaryingType(), fargTypeNC)) { // uf -> vf is better than uf -> ui or uf -> ud // Step "7" from documentation - costSum += 16 * costScale; + cost[i] += 16 * costScale; continue; } if (lIsMatchWithTypeWidening(callTypeNC->GetAsVaryingType(), fargTypeNC)) { // uf -> vd is better than uf -> vi (128 < 128 + 64) // but worse than uf -> ui (128 > 64) // Step "9" from documentation - costSum += 128 * costScale; + cost[i] += 128 * costScale; continue; } // 128 + 64 is the max. uf -> vi is the worst case. // Step "10" from documentation - costSum += 128 * costScale; + cost[i] += 128 * costScale; } if (CanConvertTypes(callTypeNC, fargTypeNC)) // two cases: the worst is 128 + 64: uf -> vi and // the only 64: (64 < 128) uf -> ui worse than uf -> vd // Step "8" from documentation - costSum += 64 * costScale; + cost[i] += 64 * costScale; else // Failure--no type conversion possible... return -1; } } + for (int i = 0; i < (int)argTypes.size(); ++i) { + costSum = costSum + cost[i]; + } return costSum; } @@ -8337,6 +8342,7 @@ FunctionSymbolExpr::ResolveOverloads(SourcePos argPos, int bestMatchCost = 1<<30; std::vector matches; std::vector candidateCosts; + std::vector candidateExpandCosts; if (actualCandidates.size() == 0) goto failure; @@ -8346,9 +8352,12 @@ FunctionSymbolExpr::ResolveOverloads(SourcePos argPos, const FunctionType *ft = CastType(actualCandidates[i]->type); AssertPos(pos, ft != NULL); + int * cost = new int[argTypes.size()]; candidateCosts.push_back(computeOverloadCost(ft, argTypes, argCouldBeNULL, - argIsConstant)); + argIsConstant, + cost)); + candidateExpandCosts.push_back(cost); } // Find the best cost, and then the candidate or candidates that have @@ -8361,8 +8370,28 @@ FunctionSymbolExpr::ResolveOverloads(SourcePos argPos, if (bestMatchCost == (1<<30)) goto failure; for (int i = 0; i < (int)candidateCosts.size(); ++i) { - if (candidateCosts[i] == bestMatchCost) + if (candidateCosts[i] == bestMatchCost) { + for (int j = 0; j < (int)candidateCosts.size(); ++j) { + for (int k = 0; k < argTypes.size(); k++) { + if (candidateCosts[j] != -1 && + candidateExpandCosts[j][k] < candidateExpandCosts[i][k]) { + std::vector temp; + temp.push_back(actualCandidates[i]); + temp.push_back(actualCandidates[j]); + std::string candidateMessage = + lGetOverloadCandidateMessage(temp, argTypes, argCouldBeNULL); + Warning(pos, "call to \"%s\" is ambiguous. " + "This warning will be turned into error in the next ispc release.\n" + "Please add explicit cast to arguments to have unambiguous match." + "\n%s", funName, candidateMessage.c_str()); + } + } + } matches.push_back(actualCandidates[i]); + } + } + for (int i = 0; i < (int)candidateExpandCosts.size(); ++i) { + delete [] candidateExpandCosts[i]; } if (matches.size() == 1) { diff --git a/expr.h b/expr.h index b539ff1b..7d209bba 100644 --- a/expr.h +++ b/expr.h @@ -635,7 +635,8 @@ private: static int computeOverloadCost(const FunctionType *ftype, const std::vector &argTypes, const std::vector *argCouldBeNULL, - const std::vector *argIsConstant); + const std::vector *argIsConstant, + int * cost); /** Name of the function that is being called. */ std::string name; diff --git a/tests/array-mixed-unif-vary-indexing-2.ispc b/tests/array-mixed-unif-vary-indexing-2.ispc index 8143ca29..a885a136 100644 --- a/tests/array-mixed-unif-vary-indexing-2.ispc +++ b/tests/array-mixed-unif-vary-indexing-2.ispc @@ -16,7 +16,7 @@ export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { else x[a][b-1] = 1; - a = min(a, 46); + a = min(a, 46.f); RET[programIndex] = x[3][a]; } diff --git a/tests/array-mixed-unif-vary-indexing.ispc b/tests/array-mixed-unif-vary-indexing.ispc index 96fc0870..bd3e6919 100644 --- a/tests/array-mixed-unif-vary-indexing.ispc +++ b/tests/array-mixed-unif-vary-indexing.ispc @@ -10,7 +10,7 @@ export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { for (uniform int j = 0; j < 47; ++j) x[i][j] = 2+b-5; - a = min(a,46); + a = min(a,46.f); x[a][b-1] = 0; RET[programIndex] = x[2][a]; } diff --git a/tests/array-multidim-gather-scatter.ispc b/tests/array-multidim-gather-scatter.ispc index 1528b070..4fc59f52 100644 --- a/tests/array-multidim-gather-scatter.ispc +++ b/tests/array-multidim-gather-scatter.ispc @@ -11,7 +11,7 @@ export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { uniform int index[4] = { 0, 1, 2, 4 }; float v = index[programIndex & 0x3]; - x[min(a,39)][v] = 0; + x[min(a,39.f)][v] = 0; RET[programIndex] = x[v+1][v]; } diff --git a/tests/func-overload-max.ispc b/tests/func-overload-max.ispc index 37360030..44b20db7 100644 --- a/tests/func-overload-max.ispc +++ b/tests/func-overload-max.ispc @@ -4,7 +4,7 @@ export uniform int width() { return programCount; } export void f_f(uniform float RET[], uniform float aFOO[]) { float a = 1. / aFOO[programIndex]; - RET[programIndex] = max(0, a); + RET[programIndex] = max(0.f, a); } export void result(uniform float RET[]) { diff --git a/tests/funcptr-uniform-10.ispc b/tests/funcptr-uniform-10.ispc index 75fcbfe9..8490778f 100644 --- a/tests/funcptr-uniform-10.ispc +++ b/tests/funcptr-uniform-10.ispc @@ -16,7 +16,7 @@ export void f_f(uniform float RET[], uniform float aFOO[]) { uniform FuncType func[] = { bar, foo }; float a = aFOO[programIndex]; float b = aFOO[0]-1; // == 0 - func[min(a, 0)] = foo; + func[min(a, 0.f)] = foo; RET[programIndex] = func[0](a, b); } diff --git a/tests/goto-4.ispc b/tests/goto-4.ispc index 97c373ef..9d5bf630 100644 --- a/tests/goto-4.ispc +++ b/tests/goto-4.ispc @@ -9,7 +9,7 @@ export void f_f(uniform float RET[], uniform float aFOO[]) { encore: ++RET[programIndex]; if (any(a != 0)) { - a = max(a-1, 0); + a = max(a-1, 0.f); goto encore; } } diff --git a/tests/max-float-1.ispc b/tests/max-float-1.ispc index 24b9822d..5674a7bf 100644 --- a/tests/max-float-1.ispc +++ b/tests/max-float-1.ispc @@ -6,7 +6,7 @@ export uniform int width() { return programCount; } export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { float a = aFOO[programIndex]; RET[programIndex] = max(3 * a, 10.f); - RET[width()-1] = max(b, 100); + RET[width()-1] = max(b, 100.f); } diff --git a/tests/max-float-2.ispc b/tests/max-float-2.ispc index f990b102..16937b42 100644 --- a/tests/max-float-2.ispc +++ b/tests/max-float-2.ispc @@ -6,7 +6,7 @@ export uniform int width() { return programCount; } export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { float a = aFOO[programIndex]; RET[programIndex] = max(-10 * (a-3), .1f); - RET[width() - 1] = max(-10 * b, 2); + RET[width() - 1] = max(-10 * b, 2.f); } export void result(uniform float RET[]) { diff --git a/tests/min-float-1.ispc b/tests/min-float-1.ispc index 5b62c5c5..3a5ad5f3 100644 --- a/tests/min-float-1.ispc +++ b/tests/min-float-1.ispc @@ -6,7 +6,7 @@ export uniform int width() { return programCount; } export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { float a = aFOO[programIndex]; RET[programIndex] = min(3 * a, 10.f); - RET[width()-1] = min(b, 100); + RET[width()-1] = min(b, 100.f); } diff --git a/tests/min-float-2.ispc b/tests/min-float-2.ispc index 85c226ca..e099d64b 100644 --- a/tests/min-float-2.ispc +++ b/tests/min-float-2.ispc @@ -6,7 +6,7 @@ export uniform int width() { return programCount; } export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { float a = aFOO[programIndex]; RET[programIndex] = min(-10 * (a-3), .1f); - RET[width() - 1] = min(-10 * b, 2); + RET[width() - 1] = min(-10 * b, 2.f); } export void result(uniform float RET[]) { diff --git a/tests/typedef-2.ispc b/tests/typedef-2.ispc index e8117dd4..af71dc6e 100644 --- a/tests/typedef-2.ispc +++ b/tests/typedef-2.ispc @@ -19,7 +19,7 @@ export void f_fu(uniform float RET[], uniform float aFOO[], uniform float b) { for (uniform int i = 0; i < 16; ++i) for (uniform int j = 0; j < 16; ++j) bar.foo[i].x[j] = b; - RET[programIndex] = bar.foo[min(15, a-1)].x[min(15, a-1)]; + RET[programIndex] = bar.foo[min(15.f, a-1)].x[min(15.f, a-1)]; } export void result(uniform float RET[]) { RET[programIndex] = 5; }