From 88a2c190a4ecdfdb5be6aef064a668ca8a6096f1 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 31 Jul 2020 14:13:30 -0400 Subject: [PATCH 01/63] Adjust inout to be implemented with a single argument --- compiler/include/flags_list.h | 5 - compiler/passes/normalize.cpp | 34 --- compiler/resolution/AutoDestroyScope.cpp | 16 +- compiler/resolution/ResolutionCandidate.cpp | 9 +- compiler/resolution/fixupExports.cpp | 6 - compiler/resolution/functionResolution.cpp | 5 - compiler/resolution/generics.cpp | 2 - compiler/resolution/resolveFunction.cpp | 34 +-- compiler/resolution/virtualDispatch.cpp | 4 +- compiler/resolution/wrappers.cpp | 257 ++++++++++++-------- 10 files changed, 175 insertions(+), 197 deletions(-) diff --git a/compiler/include/flags_list.h b/compiler/include/flags_list.h index 7477e0b1c3f0..c13520ea5ea7 100644 --- a/compiler/include/flags_list.h +++ b/compiler/include/flags_list.h @@ -145,7 +145,6 @@ symbolFlag( FLAG_FIRST_CLASS_FUNCTION_INVOCATION, npr, "first class function inv symbolFlag( FLAG_FN_RETARG, npr, "fn returns via _retArg", ncm ) symbolFlag( FLAG_FOLLOWER_INDEX, npr, "follower index", "a variable representing a follower loop index" ) symbolFlag( FLAG_FORMAL_TEMP, npr, "formal temp", "a formal temp requiring write-back for an out or inout argument" ) -symbolFlag( FLAG_FORMAL_TEMP_INOUT, npr, "formal temp inout", "a formal temp to back an inout argument" ) symbolFlag( FLAG_FORMAL_TEMP_OUT, npr, "formal temp out", "a formal temp to back an out argument" ) symbolFlag( FLAG_FORWARDING_FN , npr, "forwarding function" , ncm ) symbolFlag( FLAG_FUNCTION_CLASS , npr, "function class" , "first-class function class representation" ) @@ -167,10 +166,6 @@ symbolFlag( FLAG_GLOBAL_VAR_BUILTIN, ypr, "global var builtin", "is accessible t symbolFlag( FLAG_HAS_POSTINIT , ypr, "has postinit" , "type that has a postinit method" ) symbolFlag( FLAG_HAS_RUNTIME_TYPE , ypr, "has runtime type" , "type that has an associated runtime type" ) -// marks a hidden formal that appears after an inout argument. -// the hidden formal covers the "out" part of the "inout". -symbolFlag( FLAG_HIDDEN_FORMAL_INOUT , npr, "hidden formal for inout" , "a separate formal to implement the copy-back part of an inout argument" ) - symbolFlag( FLAG_IGNORE_RUNTIME_TYPE , ypr, "ignore runtime type" , "use the static type only in the return value" ) symbolFlag( FLAG_IGNORE_IN_GLOBAL_ANALYSIS , ypr, "ignore in global analysis" , "ignore this function in global use-before-def analysis" ) symbolFlag( FLAG_RVV, npr, "RVV", "variable is the return value variable" ) diff --git a/compiler/passes/normalize.cpp b/compiler/passes/normalize.cpp index ca6629ad8a56..768489682f58 100644 --- a/compiler/passes/normalize.cpp +++ b/compiler/passes/normalize.cpp @@ -63,8 +63,6 @@ static void makeExportWrapper(FnSymbol* fn); static void fixupArrayFormals(FnSymbol* fn); -static void fixupInoutFormals(FnSymbol* fn); - static bool includesParameterizedPrimitive(FnSymbol* fn); static void replaceFunctionWithInstantiationsOfPrimitive(FnSymbol* fn); static void fixupQueryFormals(FnSymbol* fn); @@ -171,8 +169,6 @@ void normalize() { updateInitMethod(fn); } } - - fixupInoutFormals(fn); } normalizeBase(theProgram, true); @@ -3555,36 +3551,6 @@ static void fixupArrayElementExpr(FnSymbol* fn, } } -/************************************* | ************************************** -* * -* Add a second formal for each inout formal so that later parts of * -* compilation can handle the `in` and `out` parts separately. * -* * -************************************** | *************************************/ -static void fixupInoutFormals(FnSymbol* fn) { - for_formals(formal, fn) { - if (formal->intent == INTENT_INOUT) { - if (fn->hasFlag(FLAG_EXTERN)) { - formal->originalIntent = INTENT_REF; - formal->intent = INTENT_REF; - } else if (formal->variableExpr != NULL) { - USR_FATAL_CONT(formal, "inout varargs not currently supported"); - formal->originalIntent = INTENT_REF; - formal->intent = INTENT_REF; - } else { - // Add a hidden out formal after the inout one. - ArgSymbol* outFormal = formal->copy(); - outFormal->name = astr(outFormal->name, "_out"); - outFormal->originalIntent = INTENT_OUT; - outFormal->intent = INTENT_OUT; - outFormal->addFlag(FLAG_HIDDEN_FORMAL_INOUT); - outFormal->defaultExpr = new BlockStmt(new SymExpr(formal)); - DefExpr* def = new DefExpr(outFormal); - formal->defPoint->insertAfter(def); - } - } - } -} /************************************* | ************************************** * * diff --git a/compiler/resolution/AutoDestroyScope.cpp b/compiler/resolution/AutoDestroyScope.cpp index a336a6120b7a..d91e5fa08c80 100644 --- a/compiler/resolution/AutoDestroyScope.cpp +++ b/compiler/resolution/AutoDestroyScope.cpp @@ -98,16 +98,14 @@ void AutoDestroyScope::addFormalTemps() { if (fn->hasFlag(FLAG_EXTERN)) return; - bool anyOutInout = false; + bool anyOut = false; for_formals(formal, fn) { if (formal->intent == INTENT_OUT || - formal->originalIntent == INTENT_OUT || - formal->intent == INTENT_INOUT || - formal->originalIntent == INTENT_INOUT) { - anyOutInout = true; + formal->originalIntent == INTENT_OUT) { + anyOut = true; } } - if (anyOutInout) { + if (anyOut) { // Go through the function epilogue looking for // write-backs to args from FORMAL_TEMP variables LabelSymbol* epilogue = fn->getEpilogueLabel(); @@ -118,8 +116,7 @@ void AutoDestroyScope::addFormalTemps() { next = cur->next; if (VarSymbol* var = findFormalTempAssignBack(cur)) { CallExpr* call = toCallExpr(cur); - INT_ASSERT(var->hasFlag(FLAG_FORMAL_TEMP_INOUT) || - var->hasFlag(FLAG_FORMAL_TEMP_OUT)); + INT_ASSERT(var->hasFlag(FLAG_FORMAL_TEMP_OUT)); INT_ASSERT(call); mFormalTempActions.push_back(call); call->remove(); // will be added back in just before destroying @@ -437,8 +434,7 @@ void AutoDestroyScope::variablesDestroy(Expr* refStmt, if (var != NULL && var != excludeVar && ignored.count(var) == 0) { if (startingScope->isVariableInitialized(var)) { bool outIntentFormalReturn = forErrorReturn == false && - (var->hasFlag(FLAG_FORMAL_TEMP_OUT) || - var->hasFlag(FLAG_FORMAL_TEMP_INOUT)); + var->hasFlag(FLAG_FORMAL_TEMP_OUT); // No deinit for out formal returns - deinited at call site if (outIntentFormalReturn == false) deinitialize(insertBeforeStmt, NULL, var); diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index e2eeac53b917..043969d1f67f 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -205,8 +205,7 @@ bool ResolutionCandidate::computeAlignment(CallInfo& info) { } if (formalIdxToActual[j] == NULL && - !formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT) && - !formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) { + !formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT)) { match = true; actualIdxToFormal[i] = formal; formalIdxToActual[j] = info.actuals.v[i]; @@ -239,8 +238,7 @@ bool ResolutionCandidate::computeAlignment(CallInfo& info) { // or have a default value. while (formal) { if (formalIdxToActual[j] == NULL && formal->defaultExpr == NULL && - !formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT) && - !formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) { + !formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT)) { failingArgument = formal; reason = RESOLUTION_CANDIDATE_TOO_FEW_ARGUMENTS; return false; @@ -958,8 +956,7 @@ void explainCandidateRejection(CallInfo& info, FnSymbol* fn) { // so no point in trying to find one. } - if (formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT) || - formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) + if (formal->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT)) continue; if (formal->type == dtMethodToken) diff --git a/compiler/resolution/fixupExports.cpp b/compiler/resolution/fixupExports.cpp index fb38b48d0b86..8159cc67c53a 100644 --- a/compiler/resolution/fixupExports.cpp +++ b/compiler/resolution/fixupExports.cpp @@ -244,12 +244,6 @@ static bool needsFixup(Type* t) { static bool validateFormalIntent(FnSymbol* fn, ArgSymbol* as) { Type* t = maybeUnwrapRef(as->type); - // inout intent is not supported at all yet. Since any inout - // argument will trigger an error, skip any error - // for the hidden argument used for the inout intent. - if (as->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) - return false; - // TODO: If we ever add more types to these fixup routines, we really ought // to put these conditions in tables. // diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 1e6817fecfab..54f355d7cd05 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -5900,11 +5900,6 @@ static void lvalueCheckActual(CallExpr* call, Expr* actual, IntentTag intent, Ar nonTaskFnParent->addFlag(FLAG_MODIFIES_CONST_FIELDS); } - // A constness error for an inout argument need only be - // reported for the first of the 2 formals. - if (errorMsg && formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) - errorMsg = false; - if (errorMsg == true) { if (nonTaskFnParent && nonTaskFnParent->hasFlag(FLAG_SUPPRESS_LVALUE_ERRORS)) { diff --git a/compiler/resolution/generics.cpp b/compiler/resolution/generics.cpp index ec5b9c8c9ccb..a98947689f58 100644 --- a/compiler/resolution/generics.cpp +++ b/compiler/resolution/generics.cpp @@ -571,9 +571,7 @@ static void fixupUntypedOutArgRTTs(FnSymbol* fn, // This argument passes the runtime type from the call site // to the function for use when constructing the out value. for_formals(formal, newFn) { - bool inout = formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT); if (formal->typeExpr == NULL && - inout == false && (formal->intent == INTENT_OUT || formal->originalIntent == INTENT_OUT)) { Type* formalType = formal->type->getValType(); diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 18a262e621ac..3c0fbd9f0a74 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -159,10 +159,6 @@ static void resolveFormals(FnSymbol* fn) { // should get propagated to the generated Python files. static void storeDefaultValuesForPython(FnSymbol* fn, ArgSymbol* formal) { - // ignore hidden formals used to implement inout for this purpose - if (formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) - return; - if (fLibraryPython) { Expr* end = formal->defaultExpr->body.tail; @@ -977,11 +973,8 @@ bool AddOutIntentTypeArgs::enterCallExpr(CallExpr* call) { bool outIntent = formal->intent == INTENT_OUT || formal->originalIntent == INTENT_OUT; - bool inout = formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT); - if (outIntent && formal->typeExpr == NULL && - inout == false && formalType->symbol->hasFlag(FLAG_HAS_RUNTIME_TYPE)) { INT_ASSERT(prevFormal && prevActual); @@ -1894,8 +1887,7 @@ void insertFormalTemps(FnSymbol* fn) { SymbolMap formals2vars; for_formals(formal, fn) { - if (formalRequiresTemp(formal, fn) && - !formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) { + if (formalRequiresTemp(formal, fn)) { SET_LINENO(formal); VarSymbol* tmp = newTemp(astr("_formal_tmp_", formal->name)); @@ -1928,11 +1920,9 @@ void insertFormalTemps(FnSymbol* fn) { bool formalRequiresTemp(ArgSymbol* formal, FnSymbol* fn) { return // - // 'out' and 'inout' intents are passed by ref at the C level, so we - // need to make an explicit copy in the codegen'd function */ + // 'out' requires a temp currently // (formal->intent == INTENT_OUT || - formal->intent == INTENT_INOUT || // // 'in' and 'const in' also require a copy, but for simple types // (like ints or class references), we can rely on C's copy when @@ -2102,16 +2092,9 @@ static void addLocalCopiesAndWritebacks(FnSymbol* fn, tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); break; } - case INTENT_INOUT: { - // This formal will be the `in` part. The next formal is the `out` part. - tmp->addFlag(FLAG_NO_COPY); - start->insertBefore(new CallExpr(PRIM_ASSIGN, tmp, formal)); - - tmp->addFlag(FLAG_FORMAL_TEMP); - tmp->addFlag(FLAG_FORMAL_TEMP_INOUT); - tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); + case INTENT_INOUT: + INT_FATAL("Unexpected INTENT case."); break; - } case INTENT_IN: case INTENT_CONST_IN: @@ -2216,14 +2199,7 @@ static void addLocalCopiesAndWritebacks(FnSymbol* fn, if (formal->getValType() != dtNothing) { // For inout or out intent, this assigns the modified value back to the // formal at the end of the function body. - if (formal->intent == INTENT_INOUT) { - // This formal will be the `in` part. The next formal is the `out` part. - DefExpr* nextDef = toDefExpr(formal->defPoint->next); - INT_ASSERT(nextDef && nextDef->sym->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)); - ArgSymbol* outFormal = toArgSymbol(nextDef->sym); - INT_ASSERT(outFormal); - fn->insertIntoEpilogue(new CallExpr(PRIM_ASSIGN, outFormal, tmp)); - } else if (formal->intent == INTENT_OUT) { + if (formal->intent == INTENT_OUT) { fn->insertIntoEpilogue(new CallExpr(PRIM_ASSIGN, formal, tmp)); } } diff --git a/compiler/resolution/virtualDispatch.cpp b/compiler/resolution/virtualDispatch.cpp index 43e5e74cb352..330e080569b1 100644 --- a/compiler/resolution/virtualDispatch.cpp +++ b/compiler/resolution/virtualDispatch.cpp @@ -285,7 +285,7 @@ static int getNumUserFormals(FnSymbol* fn) { int count = 0; for (int i = 3; i <= fnN; i++) { ArgSymbol* fa = fn->getFormal(i); - if (!fa->hasEitherFlag(FLAG_HIDDEN_FORMAL_INOUT, FLAG_TYPE_FORMAL_FOR_OUT)) + if (!fa->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT)) count++; } return count; @@ -298,7 +298,7 @@ static ArgSymbol* getUserFormal(FnSymbol* fn, int idx) { int count = 0; for (int i = 3; i <= fnN; i++) { ArgSymbol* fa = fn->getFormal(i); - if (!fa->hasEitherFlag(FLAG_HIDDEN_FORMAL_INOUT, FLAG_TYPE_FORMAL_FOR_OUT)) + if (!fa->hasFlag(FLAG_TYPE_FORMAL_FOR_OUT)) count++; if (count == idx) return fa; diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index 01d00379f97f..3cb62f6548ec 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -87,14 +87,13 @@ static void handleCoercion(FnSymbol* fn, CallExpr* call, ArgSymbol* formal, SymExpr* actual, SymbolMap& copyMap); -static void fixHiddenInoutArg(FnSymbol *fn, CallExpr* call, - ArgSymbol* formal, SymExpr* actual); - static void handleInIntent(FnSymbol* fn, CallExpr* call, ArgSymbol* formal, SymExpr* actual, - SymbolMap& copyMap); + SymbolMap& copyMap, + SymbolMap& inTmpToActualMap); -static void handleOutIntents(FnSymbol* fn, CallExpr* call); +static void handleOutIntents(FnSymbol* fn, CallExpr* call, + SymbolMap& inTmpToActualMap); bool isPromotionRequired(FnSymbol* fn, CallInfo& info, std::vector& actualIdxToFormal); @@ -193,6 +192,8 @@ FnSymbol* wrapAndCleanUpActuals(FnSymbol* fn, // proc f(a, b=a, c:a.type) // where a formal arguments refer to previous formals SymbolMap copyMap; + // For inout arguments, a map from the in-copy-temp to the real actual + SymbolMap inTmpToActualMap; Expr* currActual = call->get(1); Expr* nextActual = NULL; @@ -217,10 +218,8 @@ FnSymbol* wrapAndCleanUpActuals(FnSymbol* fn, // needs a coercion to the actual type). handleCoercion(retval, call, formal, actual, copyMap); - fixHiddenInoutArg(retval, call, formal, actual); - // adjust for in intent - handleInIntent(retval, call, formal, actual, copyMap); + handleInIntent(retval, call, formal, actual, copyMap, inTmpToActualMap); copyMap.put(formal, actual->symbol()); @@ -232,7 +231,7 @@ FnSymbol* wrapAndCleanUpActuals(FnSymbol* fn, // are not available on the way into the function (only on the way out). // Consider e.g. proc g(out x, y = x) // Here 'y = x' should refer to the value of 'x' on the way in to the fn. - handleOutIntents(retval, call); + handleOutIntents(retval, call, inTmpToActualMap); } return retval; @@ -358,11 +357,6 @@ static void handleDefaultArg(FnSymbol *fn, CallExpr* call, return; } - if (formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT)) { - // pass the same actual a second time - INT_FATAL("hidden inout formal should have already been replaced"); - } - // Create a Block to store the default values // We'll flatten this back out again in a minute. BlockStmt* body = new BlockStmt(BLOCK_SCOPELESS); @@ -738,7 +732,7 @@ static Symbol* createDefaultedActual(FnSymbol* fn, temp->addFlag(FLAG_SUPPRESS_LVALUE_ERRORS); // TODO: do we need to add FLAG_INSERT_AUTO_DESTROY here? - if (formal->intent & INTENT_FLAG_IN) { + if (formal->intent == INTENT_IN || formal->intent == INTENT_CONST_IN) { temp->addFlag(FLAG_NO_AUTO_DESTROY); } @@ -1582,26 +1576,16 @@ static bool checkAnotherFunctionsFormal(FnSymbol* calleeFn, CallExpr* call, return result; } -static void fixHiddenInoutArg(FnSymbol *fn, CallExpr* call, - ArgSymbol* formal, SymExpr* actual) { - if (formal->intent == INTENT_INOUT || - formal->originalIntent == INTENT_INOUT) - { - if (actual && !fn->hasFlag(FLAG_PROMOTION_WRAPPER)) { - SymExpr* nextSe = toSymExpr(actual->next); - INT_ASSERT(nextSe); - // replace dummy actual argument with the real inout actual - nextSe->setSymbol(actual->symbol()); - } - } -} - static void handleInIntent(FnSymbol* fn, CallExpr* call, ArgSymbol* formal, SymExpr* actual, - SymbolMap& copyMap) { + SymbolMap& copyMap, + SymbolMap& inTmpToActualMap) { bool inout = (formal->intent == INTENT_INOUT || - formal->originalIntent == INTENT_INOUT); + formal->originalIntent == INTENT_INOUT) && + fn->hasFlag(FLAG_PROMOTION_WRAPPER) == false; + // don't consider inout in promotion wrapper for this purpose, + // because it's handled within the body of the function. // In intents for initializers called within _new or default init functions // are handled by the _new or default init functions. @@ -1615,46 +1599,86 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, Expr* anchor = call->getStmtExpr(); - { - Symbol* actualSym = actual->symbol(); + Symbol* actualSym = actual->symbol(); - // The result of a default argument for 'in' intent is already owned and - // does not need to be copied. - if (formalRequiresTemp(formal, fn) && - (shouldAddInFormalTempAtCallSite(formal, fn) || inout) && - !checkAnotherFunctionsFormal(fn, call, actualSym) && - actualSym->hasFlag(FLAG_DEFAULT_ACTUAL) == false) { + // The result of a default argument for 'in' intent is already owned and + // does not need to be copied. + if ((formalRequiresTemp(formal, fn) || inout) && + (shouldAddInFormalTempAtCallSite(formal, fn) || inout) && + !checkAnotherFunctionsFormal(fn, call, actualSym) && + actualSym->hasFlag(FLAG_DEFAULT_ACTUAL) == false) { - // Arrays and domains need special handling in order to preserve their - // runtime types. - bool rtt = actualSym->getValType()->symbol->hasFlag(FLAG_HAS_RUNTIME_TYPE); - bool coerceRuntimeTypes = rtt && typeExprReturnsType(formal); + // Arrays and domains need special handling in order to preserve their + // runtime types. + bool rtt = actualSym->getValType()->symbol->hasFlag(FLAG_HAS_RUNTIME_TYPE); + bool coerceRuntimeTypes = rtt && typeExprReturnsType(formal); - // see issue #15628 for explanation and discussion - bool defaultInitAssign = rtt && mustUseRuntimeTypeDefault(formal) && - !coerceRuntimeTypes; + // see issue #15628 for explanation and discussion + bool defaultInitAssign = rtt && mustUseRuntimeTypeDefault(formal) && + !coerceRuntimeTypes; - VarSymbol* runtimeTypeTemp = NULL; - if (coerceRuntimeTypes) { - runtimeTypeTemp = newTemp("_formal_type_tmp"); - runtimeTypeTemp->addFlag(FLAG_TYPE_VARIABLE); - anchor->insertBefore(new DefExpr(runtimeTypeTemp)); + VarSymbol* runtimeTypeTemp = NULL; + if (coerceRuntimeTypes) { + runtimeTypeTemp = newTemp("_formal_type_tmp"); + runtimeTypeTemp->addFlag(FLAG_TYPE_VARIABLE); + anchor->insertBefore(new DefExpr(runtimeTypeTemp)); + + copyFormalTypeExprWrapper(fn, formal, runtimeTypeTemp, + call, anchor, copyMap); + } + + if (defaultInitAssign) { + + insertRuntimeTypeDefaultWrapper(fn, formal, call, actual, copyMap); + + // A copy might be necessary here but might not. + } else if (doesCopyInitializationRequireCopy(actual) || inout) { + // Add a new formal temp at the call site that mimics variable + // initialization from the actual. + VarSymbol* tmp = newTemp(astr("_formal_tmp_in_", formal->name)); + tmp->addFlag(FLAG_EXPR_TEMP); + + // for in intent, + // "move" from call site to called function, so don't destroy + // here. The called function will destroy. + if (!inout) + tmp->addFlag(FLAG_NO_AUTO_DESTROY); + + // for inout intent, need to destroy the temp at the call site + // to allow passing in a single argument (otherwise, when we try + // to do the write-back after the call, the value would be deinited + // already). + if (inout) + tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); - copyFormalTypeExprWrapper(fn, formal, runtimeTypeTemp, - call, anchor, copyMap); + // Does this need to be here? + if (formal->hasFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT)) { + tmp->addFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT); } - if (defaultInitAssign) { + CallExpr* copy = NULL; + if (coerceRuntimeTypes) + copy = new CallExpr(astr_coerceCopy, runtimeTypeTemp, actualSym); + else + copy = new CallExpr(astr_initCopy, actualSym); - insertRuntimeTypeDefaultWrapper(fn, formal, call, actual, copyMap); + CallExpr* move = new CallExpr(PRIM_MOVE, tmp, copy); + anchor->insertBefore(new DefExpr(tmp)); + anchor->insertBefore(move); + + resolveCallAndCallee(copy, false); // false - allow unresolved + resolveCall(move); + + actual->setSymbol(tmp); + } else { + // Is actualSym something that owns its value? + // Is it a call-temp storing the result of a call? + // Then "move" ownership to the called function + // (don't destroy it here, it will be destroyed there). + actualSym->addFlag(FLAG_NO_AUTO_DESTROY); - // A copy might be necessary here but might not. - } else if (doesCopyInitializationRequireCopy(actual)) { - // Add a new formal temp at the call site that mimics variable - // initialization from the actual. + if (coerceRuntimeTypes) { VarSymbol* tmp = newTemp(astr("_formal_tmp_in_", formal->name)); - // "move" from call site to called function, so don't destroy - // here. The called function will destroy. tmp->addFlag(FLAG_NO_AUTO_DESTROY); tmp->addFlag(FLAG_EXPR_TEMP); @@ -1714,9 +1738,20 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, } } } + + if (inout) { + // temporary is required for inout + // the default actual is enough, though. + if (actual->symbol()->hasFlag(FLAG_DEFAULT_ACTUAL) == false && + fn->hasFlag(FLAG_PROMOTION_WRAPPER) == false) { + INT_ASSERT(actual->symbol() != actualSym); + inTmpToActualMap.put(actual->symbol(), actualSym); + } + } } -static void handleOutIntents(FnSymbol* fn, CallExpr* call) { +static void handleOutIntents(FnSymbol* fn, CallExpr* call, + SymbolMap& inTmpToActualMap) { int j = 0; @@ -1735,51 +1770,77 @@ static void handleOutIntents(FnSymbol* fn, CallExpr* call) { SET_LINENO(currActual); nextActual = currActual->next; - if (formal->intent == INTENT_OUT || formal->originalIntent == INTENT_OUT) { + bool out = formal->intent == INTENT_OUT || + formal->originalIntent == INTENT_OUT; + bool inout = formal->intent == INTENT_INOUT || + formal->originalIntent == INTENT_INOUT; + + if (out || inout) { Expr* useExpr = currActual; if (NamedExpr* named = toNamedExpr(currActual)) useExpr = named->actual; - SymExpr* se = toSymExpr(useExpr); - Symbol* actualSym = se->symbol(); - - bool inout = formal->hasFlag(FLAG_HIDDEN_FORMAL_INOUT); + SymExpr* actualSe = toSymExpr(useExpr); + Symbol* assignTo = NULL; + Symbol* assignFrom = NULL; + + if (out) { + // For untyped out formals with runtime types, pass the type + // as the previous argument. + Type* formalType = formal->type->getValType(); + if (formal->typeExpr == NULL && + inout == false && + formalType->symbol->hasFlag(FLAG_HAS_RUNTIME_TYPE)) { + const char* dummyName = astr("_formal_type_tmp_", formal->name); + VarSymbol* typeTmp = newTemp(dummyName, formalType); + typeTmp->addFlag(FLAG_MAYBE_TYPE); + typeTmp->addFlag(FLAG_TYPE_FORMAL_FOR_OUT); + + anchor->insertBefore(new DefExpr(typeTmp)); + + SymExpr* prevActual = toSymExpr(currActual->prev); + INT_ASSERT(prevActual != NULL && j > 0); + prevActual->setSymbol(typeTmp); + } - // For untyped out formals with runtime types, pass the type - // as the previous argument. - Type* formalType = formal->type->getValType(); - if (formal->typeExpr == NULL && - inout == false && - formalType->symbol->hasFlag(FLAG_HAS_RUNTIME_TYPE)) { - const char* dummyName = astr("_formal_type_tmp_", formal->name); - VarSymbol* typeTmp = newTemp(dummyName, formalType); - typeTmp->addFlag(FLAG_MAYBE_TYPE); - typeTmp->addFlag(FLAG_TYPE_FORMAL_FOR_OUT); + VarSymbol* tmp = newTemp(astr("_formal_tmp_out_", formal->name), + formal->getValType()); + tmp->addFlag(FLAG_SUPPRESS_LVALUE_ERRORS); + tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); + tmp->addFlag(FLAG_EXPR_TEMP); - anchor->insertBefore(new DefExpr(typeTmp)); + // Transform f(x) where x is passed with out intent into + // DefExpr tmp + // f(tmp) + // x = tmp -> this might turn into split-init of x + anchor->insertBefore(new DefExpr(tmp)); + currActual->replace(new SymExpr(tmp)); + assignTo = actualSe->symbol(); + assignFrom = tmp; - SymExpr* prevActual = toSymExpr(currActual->prev); - INT_ASSERT(prevActual != NULL && j > 0); - prevActual->setSymbol(typeTmp); + } else { + Symbol* mapSym = inTmpToActualMap.get(actualSe->symbol()); + if (mapSym == NULL) { + // e.g. inout with a defaulted actual + assignTo = actualSe->symbol(); + assignFrom = actualSe->symbol(); + } else { + // we have + // in_tmp = copy(x) + // f(in_tmp) + // + // add assign like + // x = in_tmp + assignTo = mapSym; + assignFrom = actualSe->symbol(); + } } - VarSymbol* tmp = newTemp(astr("_formal_tmp_out_", formal->name), - formal->getValType()); - tmp->addFlag(FLAG_SUPPRESS_LVALUE_ERRORS); - tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); - tmp->addFlag(FLAG_EXPR_TEMP); - - // Transform f(x) where x is passed with out intent into - // DefExpr tmp - // f(tmp) - // x = tmp -> this might turn into split-init of x - anchor->insertBefore(new DefExpr(tmp)); - - CallExpr* assign = new CallExpr("=", actualSym, tmp); - anchorAfter->insertAfter(assign); - anchorAfter = assign; - - currActual->replace(new SymExpr(tmp)); + if (assignTo != assignFrom) { + CallExpr* assign = new CallExpr("=", assignTo, assignFrom); + anchorAfter->insertAfter(assign); + anchorAfter = assign; + } } currActual = nextActual; From 4300acfe6bb3ff49dc6a472ea4bdc41d155b7403 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 31 Jul 2020 14:15:35 -0400 Subject: [PATCH 02/63] Accept behavior change for deinit in fn body --- test/types/records/intents/inout-intent-order.good | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types/records/intents/inout-intent-order.good b/test/types/records/intents/inout-intent-order.good index 7ff1cca527d4..1f909d903ac5 100644 --- a/test/types/records/intents/inout-intent-order.good +++ b/test/types/records/intents/inout-intent-order.good @@ -143,8 +143,8 @@ lhs(2 2) = rhs(20 20) deinit 20 20 lhs(1 1) = rhs(10 10) lhs(2 2) = rhs(20 20) -deinit 20 20 deinit 10 10 +deinit 20 20 (x = 10, ptr = {xx = 10}) (x = 20, ptr = {xx = 20}) deinit 20 20 deinit 10 10 From 3340f4460c5aa8eaadad20b90da7f16fdbf83b1b Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 31 Jul 2020 18:15:56 -0400 Subject: [PATCH 03/63] Start at fixing #16185 All this is trying to get this test ./test/arrays/ferguson/semantic-examples/4-pass-slice-inout.chpl to work --- compiler/include/resolution.h | 11 ++++- compiler/resolution/ResolutionCandidate.cpp | 29 ++++++++++-- compiler/resolution/functionResolution.cpp | 49 ++++++++++++++++++--- compiler/resolution/generics.cpp | 3 +- compiler/resolution/wrappers.cpp | 22 +++++++-- test/functions/vass/varargs-1.good | 32 +++++++++++++- 6 files changed, 130 insertions(+), 16 deletions(-) diff --git a/compiler/include/resolution.h b/compiler/include/resolution.h index d7c17ece5d58..83fedab198b5 100644 --- a/compiler/include/resolution.h +++ b/compiler/include/resolution.h @@ -227,6 +227,11 @@ void getAutoCopyTypeKeys(Vec& keys); FnSymbol* getAutoCopy(Type* t); // returns NULL if there are none FnSymbol* getAutoDestroy(Type* t); // " FnSymbol* getUnalias(Type* t); + +// Some types should decay to another type when assigned into a variable. +// This function returns that type. +Type* getUnaliasTypeDuringResolution(Type* t); + FnSymbol* getCoerceMoveFromCoerceCopy(FnSymbol* coerceCopyFn); const char* getErroneousCopyError(FnSymbol* fn); void markCopyErroneous(FnSymbol* fn, const char* err); @@ -312,7 +317,11 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, Type* formalType, Symbol* formalSym, Expr* ctx, bool allowCoercion=true, - bool implicitBang=false); + bool implicitBang=false, + bool inOrOtherValue=false); + +// in/out/inout +bool inOrOutFormal(ArgSymbol* formal); void resolveIfExprType(CondStmt* stmt); diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index 043969d1f67f..2a396cce7874 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -408,7 +408,8 @@ void ResolutionCandidate::computeSubstitutionForDefaultExpr(ArgSymbol* formal, } else if (Type* type = getInstantiationType(defaultType, NULL, formal->type, NULL, ctx, - true, false)) { + true, false, + inOrOutFormal(formal))) { substitutions.put(formal, type->symbol); } } @@ -473,15 +474,26 @@ Type* getInstantiationType(Symbol* actual, ArgSymbol* formal, Expr* ctx) { bool implicitBang = allowImplicitNilabilityRemoval(actual->type, actual, formal->type, formal); + bool inOrOtherValue = inOrOutFormal(formal); + return getInstantiationType(actual->type, actual, formal->type, formal, ctx, - allowCoercions, implicitBang); + allowCoercions, implicitBang, inOrOtherValue); } Type* getInstantiationType(Type* actualType, Symbol* actualSym, Type* formalType, Symbol* formalSym, Expr* ctx, - bool allowCoercion, bool implicitBang) { + bool allowCoercion, bool implicitBang, + bool inOrOtherValue) { + + // memoize unaliasing for in/inout/out/value return + if (inOrOtherValue) { + if (Type* unalias = getUnaliasTypeDuringResolution(actualType)) { + actualType = unalias; + } + } + Type* ret = getBasicInstantiationType(actualType, actualSym, formalType, formalSym, ctx, allowCoercion, implicitBang); @@ -508,6 +520,17 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, return ret; } +bool inOrOutFormal(ArgSymbol* formal) { + return (formal->intent == INTENT_IN || + formal->originalIntent == INTENT_IN || + formal->intent == INTENT_CONST_IN || + formal->originalIntent == INTENT_CONST_IN || + formal->intent == INTENT_OUT || + formal->originalIntent == INTENT_OUT || + formal->intent == INTENT_INOUT || + formal->originalIntent == INTENT_INOUT); +} + static Type* getBasicInstantiationType(Type* actualType, Symbol* actualSym, Type* formalType, Symbol* formalSym, Expr* ctx, diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 54f355d7cd05..88f9d72306f7 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -177,6 +177,7 @@ static void resolveMove(CallExpr* call); static void resolveNew(CallExpr* call); static void resolveCoerce(CallExpr* call); static void resolveAutoCopyEtc(AggregateType* at); +static void resolveUnalias(AggregateType* at); static Expr* foldTryCond(Expr* expr); @@ -395,6 +396,35 @@ FnSymbol* getUnalias(Type* t) { return unaliasMap.get(t); } +Type* getUnaliasTypeDuringResolution(Type* t) { + if (isSyncType(t) || isSingleType(t)) { + Type* baseType = t->getField("valType")->type; + return baseType; + } + if (t->symbol->hasFlag(FLAG_DOMAIN) || + t->symbol->hasFlag(FLAG_ARRAY)) { + + AggregateType* at = toAggregateType(t); + INT_ASSERT(at); + + // check if it is an array view + // (otherwise we can infinite loop in resolving array functions) + Symbol* instanceField = at->getField("_instance", false); + if (instanceField) { + if (instanceField->type->symbol->hasFlag(FLAG_ALIASING_ARRAY)) { + + // doesn't call resolveAutoCopyEtc b/c of infinite loop + resolveUnalias(at); + if (FnSymbol* unalias = getUnalias(at)) { + return unalias->retType; + } + } + } + } + + return t; +} + FnSymbol* getCoerceMoveFromCoerceCopy(FnSymbol* coerceCopyFn) { return coerceMoveFromCopyMap.get(coerceCopyFn); } @@ -1508,12 +1538,10 @@ bool canCoerce(Type* actualType, return true; } - if (isSyncType(actualType) || isSingleType(actualType)) { - Type* baseType = actualType->getField("valType")->type; - - // sync can't store an array or a param, so no need to - // propagate promotes / paramNarrows - return canDispatch(baseType, NULL, formalType, formalSym, fn); + Type* unaliasType = getUnaliasTypeDuringResolution(actualType); + if (unaliasType != actualType) { + return canDispatch(unaliasType, actualSym, formalType, formalSym, fn, + promotes, paramNarrows); } if (canCoerceTuples(actualType, actualSym, formalType, formalSym, fn)) { @@ -6564,7 +6592,10 @@ void resolveInitVar(CallExpr* call) { // If the target type is generic, compute the appropriate instantiation // type. if (genericTgt) { - Type* inst = getInstantiationType(srcType, NULL, targetType, NULL, call); + Type* inst = getInstantiationType(srcType, NULL, targetType, NULL, call, + /* allowCoercion */ true, + /* implicitBang */ false, + /* inOrOtherValue */ true); // Does not allow initializations of the form: // var x : MyGenericType = ; @@ -9305,6 +9336,10 @@ static void resolveAutoCopyEtc(AggregateType* at) { autoDestroyMap.put(at, fn); } + resolveUnalias(at); +} + +static void resolveUnalias(AggregateType* at) { // resolve unalias // We make the 'unalias' hook available to all user records, // but for now it only applies to array/domain/distribution diff --git a/compiler/resolution/generics.cpp b/compiler/resolution/generics.cpp index a98947689f58..583a2102c650 100644 --- a/compiler/resolution/generics.cpp +++ b/compiler/resolution/generics.cpp @@ -694,7 +694,8 @@ FnSymbol* instantiateFunction(FnSymbol* fn, else if (Type* type = getInstantiationType(defType, NULL, newFormal->type, NULL, call, - true, false)) { + true, false, + inOrOutFormal(formal))) { val = type->symbol; } } diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index 3cb62f6548ec..95eb63e00f94 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1179,6 +1179,12 @@ static bool needToAddCoercion(Type* actualType, if (actualType == dtNil && isClassLikeOrPtr(formalType)) return false; + if (inOrOutFormal(formal)) { + Type* toType = getUnaliasTypeDuringResolution(actualType); + if (toType == formal->getValType()) + return false; // handled by other wrapper code, e.g. handleInIntent + } + // One day, we shouldn't need coercion if canCoerceAsSubtype // returns true. That would cover the above case. However, // the emitted C code doesn't encode the class hierarchy in @@ -1584,6 +1590,15 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, bool inout = (formal->intent == INTENT_INOUT || formal->originalIntent == INTENT_INOUT) && fn->hasFlag(FLAG_PROMOTION_WRAPPER) == false; + + bool in = (formal->intent == INTENT_IN || + formal->originalIntent == INTENT_IN || + formal->intent == INTENT_CONST_IN || + formal->originalIntent == INTENT_CONST_IN); + + if (inout == false && in == false) + return; + // don't consider inout in promotion wrapper for this purpose, // because it's handled within the body of the function. @@ -1599,7 +1614,8 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, Expr* anchor = call->getStmtExpr(); - Symbol* actualSym = actual->symbol(); + Symbol* origActualSym = actual->symbol(); + Symbol* actualSym = origActualSym; // The result of a default argument for 'in' intent is already owned and // does not need to be copied. @@ -1744,8 +1760,8 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, // the default actual is enough, though. if (actual->symbol()->hasFlag(FLAG_DEFAULT_ACTUAL) == false && fn->hasFlag(FLAG_PROMOTION_WRAPPER) == false) { - INT_ASSERT(actual->symbol() != actualSym); - inTmpToActualMap.put(actual->symbol(), actualSym); + INT_ASSERT(actual->symbol() != origActualSym); + inTmpToActualMap.put(actual->symbol(), origActualSym); } } } diff --git a/test/functions/vass/varargs-1.good b/test/functions/vass/varargs-1.good index 8ae163943b4f..7e0220c90cab 100644 --- a/test/functions/vass/varargs-1.good +++ b/test/functions/vass/varargs-1.good @@ -1 +1,31 @@ -varargs-1.chpl:79: error: inout varargs not currently supported +varargs-1.chpl:8: In function 'main': +varargs-1.chpl:9: warning: arg1 +varargs-1.chpl:9: warning: arg2 +varargs-1.chpl:9: warning: arg1 arg2 +varargs-1.chpl:9: warning: 2 +varargs-1.chpl:10: warning: arg1 +varargs-1.chpl:10: warning: arg2 +varargs-1.chpl:10: warning: arg1 arg2 +varargs-1.chpl:10: warning: 2 +varargs-1.chpl:11: warning: arg1 +varargs-1.chpl:11: warning: arg2 +varargs-1.chpl:11: warning: arg1 arg2 +varargs-1.chpl:11: warning: YES!!! +81 : arg1 arg2 +82 : arg1 arg2 +83 : arg1 arg2 +1000 : 101 101 102 +1001 : 101 101 71 +1009 : 105 105 71 +2001 : 105 105 72 +3001 : 105 105 102 +3009 : 301 301 302 +4001 : 105 105 73 +5001 : 0 0 0 +5009 : 501 501 502 +6001 : 501 501 502 +6009 : 601 601 602 +7001 : 601 601 602 +7009 : 701 701 702 +8001 : 701 701 702 +9999 : 701 701 702 From 522400d470d2c0751562ef0bd82d3968e6ee0be2 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 3 Aug 2020 10:24:34 -0400 Subject: [PATCH 04/63] Attempt to fix issue 16185 --- compiler/resolution/callDestructors.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp index 96866b171f89..25d053500b44 100644 --- a/compiler/resolution/callDestructors.cpp +++ b/compiler/resolution/callDestructors.cpp @@ -778,12 +778,18 @@ bool isCallExprTemporary(Expr* initFrom) { } bool doesCopyInitializationRequireCopy(Expr* initFrom) { - if (typeNeedsCopyInitDeinit(initFrom->getValType())) { + Type* fromType = initFrom->getValType(); + if (typeNeedsCopyInitDeinit(fromType)) { // RHS is a reference, need a copy if (initFrom->isRef()) return true; // Past here, it's a value. + // e.g. an array view being copied into an array. + // This copy might be copy-elided later. + if (getUnaliasTypeDuringResolution(fromType) != fromType) + return true; + // Is it the result of a call returning by value? if (isCallExprTemporary(initFrom)) return false; From 6dbe7f5ea9d5fa659b00e1199d3eb819062658ee Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 5 Aug 2020 09:38:03 -0400 Subject: [PATCH 05/63] Add a test of A.domain and copying behavior --- .../ferguson/array-domain-and-copying.chpl | 150 ++++++++++++++++++ .../ferguson/array-domain-and-copying.good | 26 +++ 2 files changed, 176 insertions(+) create mode 100644 test/domains/ferguson/array-domain-and-copying.chpl create mode 100644 test/domains/ferguson/array-domain-and-copying.good diff --git a/test/domains/ferguson/array-domain-and-copying.chpl b/test/domains/ferguson/array-domain-and-copying.chpl new file mode 100644 index 000000000000..a907c361bbe2 --- /dev/null +++ b/test/domains/ferguson/array-domain-and-copying.chpl @@ -0,0 +1,150 @@ +var D = {1..10}; +var A:[D] int; + +proc returnADomain() { + return A.domain; +} +proc test1() { + writeln("test1"); + D = {1..10}; + var dom = returnADomain(); + D = {1..3}; + writeln(dom); +} +test1(); + +proc acceptDomainIn(in dom) { + D = {1..3}; + writeln(dom); +} +proc test2() { + writeln("test2"); + D = {1..10}; + acceptDomainIn(D); +} +test2(); + +proc test3() { + writeln("test3"); + D = {1..10}; + acceptDomainIn(A.domain); +} +test3(); + +proc maybeReturnADomain(option: bool) { + if option { + return A.domain; + } else { + return {1..99}; + } +} +proc test4() { + writeln("test4"); + D = {1..10}; + var dom = maybeReturnADomain(true); + D = {1..3}; + writeln(dom); +} +test4(); +proc test5() { + writeln("test5"); + D = {1..10}; + var dom = maybeReturnADomain(false); + D = {1..3}; + writeln(dom); +} +test5(); + +proc maybeReturnADomainSplit(option: bool) { + var dom: domain(1); + if option { + dom = A.domain; + } else { + dom = {1..99}; + } + return dom; +} +proc test6() { + writeln("test6"); + D = {1..10}; + var dom = maybeReturnADomainSplit(true); + D = {1..3}; + writeln(dom); +} +test6(); +proc test7() { + writeln("test7"); + D = {1..10}; + var dom = maybeReturnADomainSplit(false); + D = {1..3}; + writeln(dom); +} +test7(); + +proc maybeReturnADomainSplitNoType(option: bool) { + var dom; + if option { + dom = A.domain; + } else { + dom = {1..99}; + } + return dom; +} +proc test8() { + writeln("test8"); + D = {1..10}; + var dom = maybeReturnADomainSplit(true); + D = {1..3}; + writeln(dom); +} +test8(); +proc test9() { + writeln("test9"); + D = {1..10}; + var dom = maybeReturnADomainSplit(false); + D = {1..3}; + writeln(dom); +} +test9(); + +proc returnADomainCopyInit() { + var dom: domain(1) = A.domain; + return dom; +} +proc test10() { + writeln("test10"); + D = {1..10}; + var dom = returnADomainCopyInit(); + D = {1..3}; + writeln(dom); +} +test10(); + +proc test11() { + writeln("test11"); + D = {1..10}; + var dom = A.domain; + D = {1..3}; + writeln(dom); +} +test11(); + +proc test12() { + writeln("test12"); + D = {1..10}; + var tmp = A.domain; + var dom = tmp; + D = {1..3}; + writeln(dom); +} +test12(); + +proc test13() { + writeln("test13"); + D = {1..10}; + const ref tmp = A.domain; + var dom = tmp; + D = {1..3}; + writeln(dom); +} +test13(); diff --git a/test/domains/ferguson/array-domain-and-copying.good b/test/domains/ferguson/array-domain-and-copying.good new file mode 100644 index 000000000000..e813c3e0b46a --- /dev/null +++ b/test/domains/ferguson/array-domain-and-copying.good @@ -0,0 +1,26 @@ +test1 +{1..10} +test2 +{1..10} +test3 +{1..10} +test4 +{1..10} +test5 +{1..99} +test6 +{1..10} +test7 +{1..99} +test8 +{1..10} +test9 +{1..99} +test10 +{1..10} +test11 +{1..10} +test12 +{1..10} +test13 +{1..10} From a53bd966f66ca828fb013d88fe64259ea1d59891 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 6 Aug 2020 11:15:49 -0400 Subject: [PATCH 06/63] Change .good for inout being one argument --- test/arrays/ferguson/inout-tuple-ref.good | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/arrays/ferguson/inout-tuple-ref.good b/test/arrays/ferguson/inout-tuple-ref.good index 5ebb76d7c2bd..65d4326cda81 100644 --- a/test/arrays/ferguson/inout-tuple-ref.good +++ b/test/arrays/ferguson/inout-tuple-ref.good @@ -1 +1,2 @@ -inout-tuple-ref.chpl:3: error: inout varargs not currently supported +1 3 4 5 +5 5 5 5 From 604fcb75b7d0f18398e8324980c95303dc08467e Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 6 Aug 2020 11:16:31 -0400 Subject: [PATCH 07/63] Remove chpl__unalias and most chpl__unref. Replace with chpl__initCopy. Some chpl__unref might still be called for tuples; a TODO is to investigate if that is still the case. --- compiler/include/flags_list.h | 1 - compiler/include/resolution.h | 11 +- compiler/passes/splitInit.cpp | 3 +- compiler/resolution/ResolutionCandidate.cpp | 4 +- compiler/resolution/callDestructors.cpp | 120 ++++++++++++-------- compiler/resolution/functionResolution.cpp | 96 +++++++++------- compiler/resolution/resolveFunction.cpp | 25 ++-- compiler/resolution/wrappers.cpp | 2 +- modules/internal/ChapelArray.chpl | 59 +--------- modules/internal/ChapelBase.chpl | 22 ---- modules/minimal/internal/ChapelBase.chpl | 9 -- 11 files changed, 159 insertions(+), 193 deletions(-) diff --git a/compiler/include/flags_list.h b/compiler/include/flags_list.h index c13520ea5ea7..1b34ff390b12 100644 --- a/compiler/include/flags_list.h +++ b/compiler/include/flags_list.h @@ -445,7 +445,6 @@ symbolFlag( FLAG_TYPE_CUSTOM_ASSIGN , npr, "type uses custom =" , "type has user symbolFlag( FLAG_TYPE_FORMAL_FOR_OUT , npr, "type formal for out" , "stores the runtime type for an untyped out argument" ) symbolFlag( FLAG_TYPE_VARIABLE , npr, "type variable" , "contains a type instead of a value" ) -symbolFlag( FLAG_UNALIAS_FN, ypr, "unalias fn" , "function to copy array slices when assigning to a user variable") symbolFlag( FLAG_UNCHECKED_THROWS, ypr, "unchecked throws" , "function throws but handling the errors is not required even in strict mode") symbolFlag( FLAG_UNREF_FN, ypr, "unref fn" , "function to remove reference fields from tuples or copy array slices when returning") symbolFlag( FLAG_UNSAFE, ypr, "unsafe" , "unsafe (disable lifetime and nilability checking)") diff --git a/compiler/include/resolution.h b/compiler/include/resolution.h index 83fedab198b5..a37a5aae1f7c 100644 --- a/compiler/include/resolution.h +++ b/compiler/include/resolution.h @@ -226,11 +226,13 @@ FnSymbol* getAutoCopyForType(Type* type); // requires hasAutoCopyForType()==tr void getAutoCopyTypeKeys(Vec& keys); FnSymbol* getAutoCopy(Type* t); // returns NULL if there are none FnSymbol* getAutoDestroy(Type* t); // " -FnSymbol* getUnalias(Type* t); -// Some types should decay to another type when assigned into a variable. +FnSymbol* getInitCopyDuringResolution(Type* t); + +// Some types should change to another type when assigned into a variable. // This function returns that type. -Type* getUnaliasTypeDuringResolution(Type* t); +// Examples: array view -> array; sync int -> int; iterator -> array +Type* getCopyTypeDuringResolution(Type* t); FnSymbol* getCoerceMoveFromCoerceCopy(FnSymbol* coerceCopyFn); const char* getErroneousCopyError(FnSymbol* fn); @@ -323,6 +325,9 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, // in/out/inout bool inOrOutFormal(ArgSymbol* formal); +bool isCallExprTemporary(Symbol* fromSym); +bool isTemporaryFromNoCopyReturn(Symbol* fromSym); + void resolveIfExprType(CondStmt* stmt); void trimVisibleCandidates(CallInfo& call, diff --git a/compiler/passes/splitInit.cpp b/compiler/passes/splitInit.cpp index 2d32518cac24..dcd7f2413e46 100644 --- a/compiler/passes/splitInit.cpp +++ b/compiler/passes/splitInit.cpp @@ -650,7 +650,8 @@ static bool canCopyElideVar(Symbol* rhs) { static bool canCopyElideCall(CallExpr* call, Symbol* lhs, Symbol* rhs) { return canCopyElideVar(rhs) && rhs->getValType() == lhs->getValType() && - rhs->defPoint->parentSymbol == call->parentSymbol; + rhs->defPoint->parentSymbol == call->parentSymbol && + !(isCallExprTemporary(rhs) && isTemporaryFromNoCopyReturn(rhs)); } // returns true if there was an unconditional return diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index 2a396cce7874..30ac560db7b9 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -489,8 +489,8 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, // memoize unaliasing for in/inout/out/value return if (inOrOtherValue) { - if (Type* unalias = getUnaliasTypeDuringResolution(actualType)) { - actualType = unalias; + if (Type* copyType = getCopyTypeDuringResolution(actualType)) { + actualType = copyType; } } diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp index 25d053500b44..81a8cd9d02d6 100644 --- a/compiler/resolution/callDestructors.cpp +++ b/compiler/resolution/callDestructors.cpp @@ -636,8 +636,6 @@ void ReturnByRef::transformMove(CallExpr* moveExpr) // and using a qualified-type generates yet another temp. Symbol* tmpVar = newTemp("ret_tmp", useLhs->getValType()); - FnSymbol* unaliasFn = NULL; - bool copiesToNoDestroy = false; // Determine if @@ -673,20 +671,25 @@ void ReturnByRef::transformMove(CallExpr* moveExpr) Type* actualType = copiedSe->symbol()->getValType(); Type* returnType = rhsFn->retType->getValType(); - unaliasFn = getUnalias(useLhs->type); - - // Cannot reduce initCopy/autoCopy when types differ - // (unless there is an unaliasFn available) - // Cannot reduce initCopy/autoCopy for sync variables - bool typesOK = unaliasFn != NULL || actualType == returnType; - - if (typesOK == true && + // Skip this transformation if + // * the type differs + // * it is a domain (for A.domain returning unowned) + // * if it's a sync or single variable (different init/auto copy) + if (actualType == returnType && isSyncType(formalType) == false && isSingleType(formalType) == false) { - copyExpr = rhsCall; - if (dstSe->symbol()->hasFlag(FLAG_NO_AUTO_DESTROY)) - copiesToNoDestroy = true; + bool copyFromUnownedDomain = false; + if (actualType->symbol->hasFlag(FLAG_DOMAIN)) { + if (fn->hasFlag(FLAG_NO_COPY_RETURN)) + copyFromUnownedDomain = true; + } + + if (copyFromUnownedDomain == false) { + copyExpr = rhsCall; + if (dstSe->symbol()->hasFlag(FLAG_NO_AUTO_DESTROY)) + copiesToNoDestroy = true; + } } } } @@ -707,30 +710,7 @@ void ReturnByRef::transformMove(CallExpr* moveExpr) // the copyExpr might be a copy added when normalizing initialization // of user variables. *or* it might come from handling `in` intent. if (copyExpr) { - FnSymbol* rhsFn = copyExpr->resolvedFunction(); - - // Use an unalias call if possible - if (rhsFn->hasFlag(FLAG_INIT_COPY_FN) && unaliasFn != NULL) { - // BHARSH: It seems important that there's a temporary to store the - // result of the unaliasFn call. Otherwise we'll move into a variable - // that has multiple uses, which seems to cause a variety of problems. - // - // In particular, I noticed that `changeRetToArgAndClone` generates - // bad AST if I simply did this: - // copyExpr->replace(new CallExpr(unaliasFn, refVar)); - VarSymbol* unaliasTemp = newTemp("unaliasTemp", unaliasFn->retType); - CallExpr* unaliasCall = new CallExpr(unaliasFn, tmpVar); - Expr* anchor = copyExpr->getStmtExpr(); - - // The call needs to be inserted before it is needed but - // after any error handling conditional that might be after callExpr. - anchor->insertBefore(new DefExpr(unaliasTemp)); - anchor->insertBefore(new CallExpr(PRIM_MOVE, unaliasTemp, unaliasCall)); - - copyExpr->replace(new SymExpr(unaliasTemp)); - } else { - copyExpr->replace(copyExpr->get(1)->remove()); - } + copyExpr->replace(copyExpr->get(1)->remove()); if (copiesToNoDestroy) { useLhs->addFlag(FLAG_NO_AUTO_DESTROY); @@ -762,11 +742,7 @@ bool isLocalVariable(Expr* initFrom) { return false; } -static -bool isCallExprTemporary(Expr* initFrom) { - SymExpr* fromSe = toSymExpr(initFrom); - INT_ASSERT(fromSe); - Symbol* fromSym = fromSe->symbol(); +bool isCallExprTemporary(Symbol* fromSym) { if (fromSym->hasFlag(FLAG_EXPR_TEMP) || fromSym->hasFlag(FLAG_INSERT_AUTO_DESTROY_FOR_EXPLICIT_NEW)) { // It's from an auto-destroyed value that is an expression temporary @@ -777,6 +753,33 @@ bool isCallExprTemporary(Expr* initFrom) { return false; } +bool isTemporaryFromNoCopyReturn(Symbol* fromSym) { + + bool anyNoCopyReturn = false; + for_SymbolSymExprs(se, fromSym) { + SymExpr* lhsSe = NULL; + CallExpr* initOrCtor = NULL; + if (CallExpr* call = toCallExpr(se->parentExpr)) { + if (isInitOrReturn(call, lhsSe, initOrCtor)) { + if (lhsSe == se) { + if (initOrCtor != NULL) { + if (FnSymbol* calledFn = initOrCtor->resolvedOrVirtualFunction()) + if (calledFn->hasFlag(FLAG_NO_COPY_RETURN)) + anyNoCopyReturn = true; + } else { + // Track moving from another temp. + SymExpr* rhsSe = toSymExpr(call->get(2)); + if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) + anyNoCopyReturn = true; + } + } + } + } + } + + return anyNoCopyReturn; +} + bool doesCopyInitializationRequireCopy(Expr* initFrom) { Type* fromType = initFrom->getValType(); if (typeNeedsCopyInitDeinit(fromType)) { @@ -787,12 +790,20 @@ bool doesCopyInitializationRequireCopy(Expr* initFrom) { // e.g. an array view being copied into an array. // This copy might be copy-elided later. - if (getUnaliasTypeDuringResolution(fromType) != fromType) + if (getCopyTypeDuringResolution(fromType) != fromType) return true; // Is it the result of a call returning by value? - if (isCallExprTemporary(initFrom)) - return false; + SymExpr* fromSe = toSymExpr(initFrom); + if (isCallExprTemporary(fromSe->symbol())) { + // check for the sub-expression being a function marked with + // pragma "no copy return" + // in that event, the copy is required. + if (isTemporaryFromNoCopyReturn(fromSe->symbol())) + return true; + else + return false; + } // Is it a local variable? Or a global? or an outer? // Need a copy in any of these cases for variable initialization. @@ -809,9 +820,22 @@ bool doesValueReturnRequireCopy(Expr* initFrom) { return true; // Past here, it's a value. + // e.g. an array view being copied into an array. + // This copy might be copy-elided later. + //if (getCopyTypeDuringResolution(fromType) != fromType) + // return true; + // Is it the result of a call returning by value? - if (isCallExprTemporary(initFrom)) - return false; + SymExpr* fromSe = toSymExpr(initFrom); + if (isCallExprTemporary(fromSe->symbol())) { + // check for the sub-expression being a function marked with + // pragma "no copy return" + // in that event, the copy is required. + if (isTemporaryFromNoCopyReturn(fromSe->symbol())) + return true; + else + return false; + } // Is it a local variable? if (isLocalVariable(initFrom)) @@ -1596,7 +1620,6 @@ static void checkForErroneousInitCopies() { if (FnSymbol* callInFn = se->getFunction()) { bool inCopyIsh = callInFn->hasFlag(FLAG_INIT_COPY_FN) || callInFn->hasFlag(FLAG_AUTO_COPY_FN) || - callInFn->hasFlag(FLAG_UNALIAS_FN) || callInFn->hasFlag(FLAG_COERCE_FN) || (callInFn->hasFlag(FLAG_COPY_INIT) && callInFn->hasFlag(FLAG_COMPILER_GENERATED)); @@ -1621,7 +1644,6 @@ static void checkForErroneousInitCopies() { if (FnSymbol* callInFn = se->getFunction()) { bool inCopyIsh = callInFn->hasFlag(FLAG_INIT_COPY_FN) || callInFn->hasFlag(FLAG_AUTO_COPY_FN) || - callInFn->hasFlag(FLAG_UNALIAS_FN) || callInFn->hasFlag(FLAG_COERCE_FN) || (callInFn->hasFlag(FLAG_COPY_INIT) && callInFn->hasFlag(FLAG_COMPILER_GENERATED)); diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 88f9d72306f7..0a807d0af8ff 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -111,10 +111,10 @@ SymbolMap paramMap; Vec callStack; std::map autoCopyMap; +std::map initCopyMap; std::map serializeMap; Map autoDestroyMap; -Map unaliasMap; Map coerceMoveFromCopyMap; Map valueToRuntimeTypeMap; Map iteratorLeaderMap; @@ -177,7 +177,7 @@ static void resolveMove(CallExpr* call); static void resolveNew(CallExpr* call); static void resolveCoerce(CallExpr* call); static void resolveAutoCopyEtc(AggregateType* at); -static void resolveUnalias(AggregateType* at); +static FnSymbol* autoMemoryFunction(AggregateType* at, const char* fnName); static Expr* foldTryCond(Expr* expr); @@ -392,18 +392,8 @@ FnSymbol* getAutoDestroy(Type* t) { return autoDestroyMap.get(t); } -FnSymbol* getUnalias(Type* t) { - return unaliasMap.get(t); -} - -Type* getUnaliasTypeDuringResolution(Type* t) { - if (isSyncType(t) || isSingleType(t)) { - Type* baseType = t->getField("valType")->type; - return baseType; - } - if (t->symbol->hasFlag(FLAG_DOMAIN) || - t->symbol->hasFlag(FLAG_ARRAY)) { - +static bool isAliasingArray(Type* t) { + if (t->symbol->hasFlag(FLAG_ARRAY)) { AggregateType* at = toAggregateType(t); INT_ASSERT(at); @@ -412,19 +402,51 @@ Type* getUnaliasTypeDuringResolution(Type* t) { Symbol* instanceField = at->getField("_instance", false); if (instanceField) { if (instanceField->type->symbol->hasFlag(FLAG_ALIASING_ARRAY)) { - - // doesn't call resolveAutoCopyEtc b/c of infinite loop - resolveUnalias(at); - if (FnSymbol* unalias = getUnalias(at)) { - return unalias->retType; - } + return true; } } } + return false; +} + +Type* getCopyTypeDuringResolution(Type* t) { + if (isSyncType(t) || isSingleType(t)) { + Type* baseType = t->getField("valType")->type; + return baseType; + } + if (isAliasingArray(t) || + t->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { + AggregateType* at = toAggregateType(t); + INT_ASSERT(at); + + FnSymbol* fn = getInitCopyDuringResolution(at); + INT_ASSERT(fn); + return fn->retType; + } + return t; } +FnSymbol* getInitCopyDuringResolution(Type* type) { + std::map::iterator it = initCopyMap.find(type); + + FnSymbol* fn = NULL; + if (it != initCopyMap.end()) + fn = it->second; // can also be NULL + + if (fn != NULL) + return fn; + + if (AggregateType* at = toAggregateType(type)) { + fn = autoMemoryFunction(at, astr_initCopy); + initCopyMap[at] = fn; + } + + return fn; +} + + FnSymbol* getCoerceMoveFromCoerceCopy(FnSymbol* coerceCopyFn) { return coerceMoveFromCopyMap.get(coerceCopyFn); } @@ -1538,10 +1560,15 @@ bool canCoerce(Type* actualType, return true; } - Type* unaliasType = getUnaliasTypeDuringResolution(actualType); - if (unaliasType != actualType) { - return canDispatch(unaliasType, actualSym, formalType, formalSym, fn, - promotes, paramNarrows); + if (formalSym != NULL && + (formalSym->originalIntent == INTENT_IN || + formalSym->originalIntent == INTENT_CONST_IN || + formalSym->originalIntent == INTENT_INOUT)) { + Type* copyType = getCopyTypeDuringResolution(actualType); + if (copyType != actualType) { + return canDispatch(copyType, actualSym, formalType, formalSym, fn, + promotes, paramNarrows); + } } if (canCoerceTuples(actualType, actualSym, formalType, formalSym, fn)) { @@ -3910,7 +3937,6 @@ void resolveNormalCallCompilerWarningStuff(CallExpr* call, if (inFn != NULL) { inCopyIsh = inFn->hasFlag(FLAG_INIT_COPY_FN) || inFn->hasFlag(FLAG_AUTO_COPY_FN) || - inFn->hasFlag(FLAG_UNALIAS_FN) || inFn->name == astrInitEquals; } if (inCopyIsh) { @@ -6690,7 +6716,6 @@ void resolveInitVar(CallExpr* call) { // For example, even though domains can leverage 'init=' for basic // copy-initialization, the compiler only currently knows about calls to // 'chpl__initCopy' and how to turn them into something else when necessary - // (e.g. chpl__unalias). Symbol *definedConst = dst->hasFlag(FLAG_CONST)? gTrue : gFalse; CallExpr* initCopy = new CallExpr(astr_initCopy, srcExpr->remove(), @@ -8661,7 +8686,6 @@ static void resolveExprMaybeIssueError(CallExpr* call) { FnSymbol* fn = frame->getFunction(); bool inCopyIsh = fn->hasFlag(FLAG_INIT_COPY_FN) || fn->hasFlag(FLAG_AUTO_COPY_FN) || - fn->hasFlag(FLAG_UNALIAS_FN) || fn->hasFlag(FLAG_COERCE_FN) || fn->name == astrInitEquals; if (inCopyIsh) { @@ -9269,7 +9293,6 @@ static void resolveDestructors() { ************************************** | *************************************/ static const char* autoCopyFnForType(AggregateType* at); -static FnSymbol* autoMemoryFunction(AggregateType* at, const char* fnName); static void resolveAutoCopies() { for_alive_in_Vec(TypeSymbol, ts, gTypeSymbols) { @@ -9335,20 +9358,6 @@ static void resolveAutoCopyEtc(AggregateType* at) { autoDestroyMap.put(at, fn); } - - resolveUnalias(at); -} - -static void resolveUnalias(AggregateType* at) { - // resolve unalias - // We make the 'unalias' hook available to all user records, - // but for now it only applies to array/domain/distribution - // in order to minimize the changes. - if (unaliasMap.get(at) == NULL && isRecordWrappedType(at) == true) { - FnSymbol* fn = autoMemoryFunction(at, "chpl__unalias"); - - unaliasMap.put(at, fn); - } } // Just use 'chpl__initCopy' instead of 'chpl__autoCopy' @@ -9400,8 +9409,7 @@ static FnSymbol* autoMemoryFunction(AggregateType* at, const char* fnName) { if (FnSymbol* fn = call->resolvedFunction()) { // if it's an initCopy e.g. we should have already marked it as erroneous if (fn->hasFlag(FLAG_INIT_COPY_FN) || - fn->hasFlag(FLAG_AUTO_COPY_FN) || - fn->hasFlag(FLAG_UNALIAS_FN)) + fn->hasFlag(FLAG_AUTO_COPY_FN)) INT_ASSERT(fn->hasFlag(FLAG_ERRONEOUS_COPY)); // Return the resolved function, for storing in the map diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 3c0fbd9f0a74..270d15c88658 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -751,11 +751,18 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { if (CallExpr* call = toCallExpr(se->parentExpr)) { if (call->isPrimitive(PRIM_MOVE) == true && call->get(1) == se) { - Type* rhsType = call->get(2)->typeInfo(); - + Expr* fromExpr = call->get(2); + Type* rhsType = fromExpr->typeInfo(); + + SymExpr* fromSe = toSymExpr(fromExpr); + bool domain = rhsType->symbol->hasFlag(FLAG_DOMAIN) && + fromSe != NULL && + isCallExprTemporary(fromSe->symbol()) && + isTemporaryFromNoCopyReturn(fromSe->symbol()); bool arrayIsh = (rhsType->symbol->hasFlag(FLAG_ARRAY) || rhsType->symbol->hasFlag(FLAG_ITERATOR_RECORD)); + bool handleDomain = skipArray == false && domain; bool handleArray = skipArray == false && arrayIsh; bool handleTuple = skipTuple == false && isTupleContainingAnyReferences(rhsType); @@ -763,13 +770,19 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { // TODO: Should we check if the RHS is a symbol with // 'no auto destroy' on it? If it is, then we'd be copying // the RHS and it would never be destroyed... - if ((handleArray || handleTuple) && !isTypeExpr(call->get(2))) { + if ((handleArray || handleDomain || handleTuple) && + !isTypeExpr(call->get(2))) { + + // Calling this for the side effect of resolving initCopy + getCopyTypeDuringResolution(rhsType); + FnSymbol* initCopyFn = getInitCopyDuringResolution(rhsType); + INT_ASSERT(initCopyFn); SET_LINENO(call); Expr* rhs = call->get(2)->remove(); - VarSymbol* tmp = newTemp("array_unref_ret_tmp", rhsType); + VarSymbol* tmp = newTemp("copy_ret_tmp", rhsType); CallExpr* initTmp = new CallExpr(PRIM_MOVE, tmp, rhs); - CallExpr* unrefCall = new CallExpr("chpl__unref", tmp); + CallExpr* unrefCall = new CallExpr(initCopyFn, tmp); CallExpr* shapeSet = findSetShape(call, ret); FnSymbol* unrefFn = NULL; @@ -815,7 +828,6 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { static bool doNotUnaliasArray(FnSymbol* fn) { return (fn->hasFlag(FLAG_NO_COPY_RETURN) || - fn->hasFlag(FLAG_UNALIAS_FN) || fn->hasFlag(FLAG_RUNTIME_TYPE_INIT_FN) || fn->hasFlag(FLAG_INIT_COPY_FN) || fn->hasFlag(FLAG_AUTO_COPY_FN) || @@ -841,7 +853,6 @@ bool doNotChangeTupleTypeRefLevel(FnSymbol* fn, bool forRet) { fn->hasFlag(FLAG_AUTO_COPY_FN) || // tuple chpl__autoCopy fn->hasFlag(FLAG_COERCE_FN) || // chpl__coerceCopy/Move fn->hasFlag(FLAG_AUTO_DESTROY_FN) || // tuple chpl__autoDestroy - fn->hasFlag(FLAG_UNALIAS_FN) || // tuple chpl__unalias fn->hasFlag(FLAG_ALLOW_REF) || // iteratorIndex (forRet && fn->hasFlag(FLAG_ITERATOR_FN)) // not iterators b/c // * they might return by ref diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index 95eb63e00f94..4c5668e4473c 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1180,7 +1180,7 @@ static bool needToAddCoercion(Type* actualType, return false; if (inOrOutFormal(formal)) { - Type* toType = getUnaliasTypeDuringResolution(actualType); + Type* toType = getCopyTypeDuringResolution(actualType); if (toType == formal->getValType()) return false; // handled by other wrapper code, e.g. handleInIntent } diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 9bb69e14564f..2828aa79817c 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -352,6 +352,8 @@ module ChapelArray { // It would have implications for alias analysis // of arrays. + pragma "no copy return" + pragma "return not owned" proc _getDomain(value) { if _to_unmanaged(value.type) != value.type then compilerError("Domain on borrow created"); @@ -821,6 +823,7 @@ module ChapelArray { return isSparseDom(dom); } + pragma "no copy return" pragma "return not owned" proc chpl__parentDomainFromDomainRuntimeType(type domainType) { pragma "no copy" @@ -859,6 +862,7 @@ module ChapelArray { return _getDistribution(dist._value); } + pragma "no copy return" pragma "return not owned" proc chpl__domainFromArrayRuntimeType(type rtt) { pragma "no copy" @@ -2512,6 +2516,7 @@ module ChapelArray { /* The type of indices used in the array's domain */ proc idxType type return _value.idxType; + pragma "no copy return" pragma "return not owned" proc _dom return _getDomain(_value.dom); @@ -4430,38 +4435,6 @@ module ChapelArray { for x in Xs do yield x; } - // This implementation of arrays and domains can create aliases - // of domains and arrays. Additionally, array aliases are possible - // in the language with the => operator. - // - // A call to the chpl__unalias function is added by the compiler when a user - // variable is initialized from an expression that would normally not require - // a copy. - // - // For example, if we have - // var A:[1..10] int; - // var B = A[1..3]; - // then B is initialized with a slice of A. But since B is a new - // variable, it needs to be a new 3-element array with distinct storage. - // Since the slice is implemented as a function call, without chpl__unalias, - // B would just be initialized to the result of the function call - - // meaning that B would not refer to distinct array elements. - pragma "unalias fn" - inline proc chpl__unalias(x: domain) { - if _to_unmanaged(x._instance.type) != x._instance.type then - compilerError("Domain on borrow created"); - - if x._unowned { - // We could add an autoDestroy here, but it wouldn't do anything for - // an unowned domain. - pragma "no auto destroy" var ret = x; - return ret; - } else { - pragma "no copy" var ret = x; - return ret; - } - } - pragma "init copy fn" proc chpl__initCopy(const ref rhs: domain, definedConst: bool) where isRectangularDom(rhs) { @@ -5047,28 +5020,6 @@ module ChapelArray { } - // see comment on chpl__unalias for domains - pragma "unalias fn" - inline proc chpl__unalias(x: []) { - param isview = (x._value.isSliceArrayView() || - x._value.isRankChangeArrayView() || - x._value.isReindexArrayView()); - - if isview { - // Intended to call chpl__initCopy - pragma "no auto destroy" var ret = x; - // Since chpl__unalias replaces a initCopy(auto/initCopy()) the - // inner value needs to be auto-destroyed. - // TODO: Should this be inserted by the compiler? - chpl__autoDestroy(x); - return ret; - } else { - // Just return a bit-copy/shallow-copy of 'x' - pragma "no copy" var ret = x; - return ret; - } - } - // chpl__initCopy(ir: _iteratorRecord, definedConst: bool) is used to create // an array out of for-expressions, forall-expressions, promoted expressions. // The 'ir' iterator - or its standalone/leader/follower counterpart - is diff --git a/modules/internal/ChapelBase.chpl b/modules/internal/ChapelBase.chpl index 719268096129..c04d2673cbf5 100644 --- a/modules/internal/ChapelBase.chpl +++ b/modules/internal/ChapelBase.chpl @@ -1680,28 +1680,6 @@ module ChapelBase { return chpl__initCopy(x, definedConst); } - pragma "compiler generated" - pragma "last resort" - pragma "unalias fn" - inline proc chpl__unalias(x) { - pragma "no copy" var ret = x; - return ret; - } - - // Returns an array storing the result of the iterator - pragma "unalias fn" - inline proc chpl__unalias(x:_iteratorClass) { - pragma "no copy" var ret = x; - return ret; - } - - // Returns an array storing the result of the iterator - pragma "unalias fn" - inline proc chpl__unalias(const ref x:_iteratorRecord) { - pragma "no copy" var ret = x; - return ret; - } - pragma "compiler generated" pragma "last resort" pragma "auto destroy fn" diff --git a/modules/minimal/internal/ChapelBase.chpl b/modules/minimal/internal/ChapelBase.chpl index 0995f44b0176..26ff48aa60a1 100644 --- a/modules/minimal/internal/ChapelBase.chpl +++ b/modules/minimal/internal/ChapelBase.chpl @@ -37,13 +37,4 @@ module ChapelBase { class _ref { var _val; } - - pragma "compiler generated" - pragma "last resort" - pragma "unalias fn" - inline proc chpl__unalias(x) { - pragma "no copy" var ret = x; - return x; - } - } From 76afaec9b72bc7f1e5f288f7d49ea2952e897f83 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 7 Aug 2020 10:12:22 -0400 Subject: [PATCH 08/63] Fix syncsingle primer --- compiler/resolution/functionResolution.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 0a807d0af8ff..3b1d4abcb01b 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -1560,10 +1560,14 @@ bool canCoerce(Type* actualType, return true; } - if (formalSym != NULL && - (formalSym->originalIntent == INTENT_IN || - formalSym->originalIntent == INTENT_CONST_IN || - formalSym->originalIntent == INTENT_INOUT)) { + // TODO: if we can avoid an inifinite loop, it would be better + // for the below to always call getCopyTypeDuringResolution + // in order to make canCoerce more consistent. + if ((isSyncType(actualType) || isSingleType(actualType)) || + (formalSym != NULL && + (formalSym->originalIntent == INTENT_IN || + formalSym->originalIntent == INTENT_CONST_IN || + formalSym->originalIntent == INTENT_INOUT))) { Type* copyType = getCopyTypeDuringResolution(actualType); if (copyType != actualType) { return canDispatch(copyType, actualSym, formalType, formalSym, fn, From 6cec8b8040e863dc9f5e3e0ba81f1ae1b88cd2d5 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 7 Aug 2020 10:53:36 -0400 Subject: [PATCH 09/63] Fix bugs * better address infinite loop issue * fix problem with instantiation type for type variables See e.g. primers syncsingle.chpl and chplvis4.chpl. --- compiler/resolution/ResolutionCandidate.cpp | 15 ++++++--- compiler/resolution/functionResolution.cpp | 34 +++++++++++++++------ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index 30ac560db7b9..cb978af286ed 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -521,13 +521,18 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, } bool inOrOutFormal(ArgSymbol* formal) { - return (formal->intent == INTENT_IN || - formal->originalIntent == INTENT_IN || - formal->intent == INTENT_CONST_IN || + // Rule out type/param variables + if (formal->hasFlag(FLAG_TYPE_VARIABLE) || + formal->hasFlag(FLAG_PARAM) || + formal->intent == INTENT_TYPE || + formal->originalIntent == INTENT_TYPE || + formal->intent == INTENT_PARAM || + formal->originalIntent == INTENT_PARAM) + return false; + + return (formal->originalIntent == INTENT_IN || formal->originalIntent == INTENT_CONST_IN || - formal->intent == INTENT_OUT || formal->originalIntent == INTENT_OUT || - formal->intent == INTENT_INOUT || formal->originalIntent == INTENT_INOUT); } diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 3b1d4abcb01b..1c258df4cda4 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -428,6 +428,29 @@ Type* getCopyTypeDuringResolution(Type* t) { return t; } +static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, + Type* formalType, ArgSymbol* formalSym, + FnSymbol* fn) { + + Type* copyType = NULL; + + if (isSyncType(actualType) || isSingleType(actualType)) { + copyType = getCopyTypeDuringResolution(actualType); + } else if (isAliasingArray(actualType) || + actualType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { + // Is the formal an array type? If not, coercion to copy + // will not be relevant. + // This check is here to avoid an infinite loop. + if (formalType == dtAny || + formalType->symbol->hasFlag(FLAG_ARRAY)) { + copyType = getCopyTypeDuringResolution(actualType); + } + } + + if (copyType == dtUnknown) copyType = NULL; + return copyType; +} + FnSymbol* getInitCopyDuringResolution(Type* type) { std::map::iterator it = initCopyMap.find(type); @@ -1560,15 +1583,8 @@ bool canCoerce(Type* actualType, return true; } - // TODO: if we can avoid an inifinite loop, it would be better - // for the below to always call getCopyTypeDuringResolution - // in order to make canCoerce more consistent. - if ((isSyncType(actualType) || isSingleType(actualType)) || - (formalSym != NULL && - (formalSym->originalIntent == INTENT_IN || - formalSym->originalIntent == INTENT_CONST_IN || - formalSym->originalIntent == INTENT_INOUT))) { - Type* copyType = getCopyTypeDuringResolution(actualType); + if (Type* copyType = canCoerceToCopyType(actualType, actualSym, + formalType, formalSym, fn)) { if (copyType != actualType) { return canDispatch(copyType, actualSym, formalType, formalSym, fn, promotes, paramNarrows); From c49afde642d907ce33063e52d78539188cc8b09c Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 7 Aug 2020 10:58:48 -0400 Subject: [PATCH 10/63] Fix bug in isTemporaryFromNoCopyReturn See e.g. test/arrays/bradc/arrOfDom/arrOfDom4.chpl --- compiler/resolution/callDestructors.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp index 81a8cd9d02d6..c0e4140ef17e 100644 --- a/compiler/resolution/callDestructors.cpp +++ b/compiler/resolution/callDestructors.cpp @@ -768,9 +768,9 @@ bool isTemporaryFromNoCopyReturn(Symbol* fromSym) { anyNoCopyReturn = true; } else { // Track moving from another temp. - SymExpr* rhsSe = toSymExpr(call->get(2)); - if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) - anyNoCopyReturn = true; + if (SymExpr* rhsSe = toSymExpr(call->get(2))) + if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) + anyNoCopyReturn = true; } } } From 6cd205d49724bca270a9edbaa4b14adfa7521633 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 7 Aug 2020 11:14:52 -0400 Subject: [PATCH 11/63] Improve infinite loop avoidance --- compiler/resolution/functionResolution.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 1c258df4cda4..0d19ce37a1b6 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -440,10 +440,12 @@ static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, actualType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { // Is the formal an array type? If not, coercion to copy // will not be relevant. - // This check is here to avoid an infinite loop. + // The checks here are to avoid infinite loops. if (formalType == dtAny || formalType->symbol->hasFlag(FLAG_ARRAY)) { - copyType = getCopyTypeDuringResolution(actualType); + if (fn == NULL || fn->name != astr_initCopy) { + copyType = getCopyTypeDuringResolution(actualType); + } } } From 217af6f2b0e53343efd8713c42dd29f781b48840 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 20 Aug 2020 11:11:02 -0400 Subject: [PATCH 12/63] Resolve problem with parallel/taskPar/sungeun/private Because of the different way in which the initCopy for an iterator was being resolved, certain functions were not visible. Resolved that by adjusting fixInstantiationPointAndTryResolveBody to consider the case of a type that is not instantiated. Additionally, avoided recursion in resolving initCopy for an iterator by adding two `pragma "no copy"`s. --- compiler/resolution/functionResolution.cpp | 9 ++++++++- modules/internal/ChapelArray.chpl | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 0d19ce37a1b6..ddb01fa4fcfe 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -6955,7 +6955,14 @@ static FnSymbol* fixInstantiationPointAndTryResolveBody(AggregateType* at, CallExpr* call) { if (FnSymbol* fn = call->resolvedFunction()) { - fn->setInstantiationPoint(at->symbol->instantiationPoint); + if (fn->instantiatedFrom != NULL) { + // it is a generic function, so make sure to set instantiationPoint + if (at->symbol->instantiationPoint == NULL) { + fn->setInstantiationPoint(getInstantiationPoint(at->symbol->defPoint)); + } else { + fn->setInstantiationPoint(at->symbol->instantiationPoint); + } + } inTryResolve++; tryResolveStates.push_back(CHECK_BODY_RESOLVES); diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 2828aa79817c..0f6772e81337 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -5221,6 +5221,7 @@ module ChapelArray { _ddata_allocate_postalloc(data, size); // Now construct a DefaultRectangular array using the data + pragma "no copy" var A = D.buildArrayWith(data[0].type, data, size:int); // Normally, the sub-arrays share a domain with the @@ -5243,6 +5244,7 @@ module ChapelArray { if callPostAlloc then _ddata_allocate_postalloc(data, size); + pragma "no copy" var A = D.buildArrayWith(elemType, data, size:int); return A; From e26dbc93293d8f113a4d12e2333837ee23698ac2 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 20 Aug 2020 14:18:35 -0400 Subject: [PATCH 13/63] Fix problems with resolving initCopy(ir) when not needed See e.g. classes/bradc/arrayInClass/genericArrayInClass-otharrs --- compiler/resolution/functionResolution.cpp | 13 ++++++++----- compiler/resolution/resolveFunction.cpp | 2 -- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index ddb01fa4fcfe..21d33c301cc5 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -438,13 +438,16 @@ static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, copyType = getCopyTypeDuringResolution(actualType); } else if (isAliasingArray(actualType) || actualType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { - // Is the formal an array type? If not, coercion to copy - // will not be relevant. - // The checks here are to avoid infinite loops. + // The conditions below avoid infinite loops and problems + // relating to resolving initCopy for iterators when not needed. if (formalType == dtAny || formalType->symbol->hasFlag(FLAG_ARRAY)) { - if (fn == NULL || fn->name != astr_initCopy) { - copyType = getCopyTypeDuringResolution(actualType); + if (fn == NULL || + !(fn->name == astr_initCopy || fn->name == astr_autoCopy)) { + + if (formalSym == NULL || inOrOutFormal(formalSym)) { + copyType = getCopyTypeDuringResolution(actualType); + } } } } diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 270d15c88658..f3625608e382 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -773,8 +773,6 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { if ((handleArray || handleDomain || handleTuple) && !isTypeExpr(call->get(2))) { - // Calling this for the side effect of resolving initCopy - getCopyTypeDuringResolution(rhsType); FnSymbol* initCopyFn = getInitCopyDuringResolution(rhsType); INT_ASSERT(initCopyFn); From 4956fc619cb8b0aa1b32955db35dda7d6dc09f74 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 20 Aug 2020 16:34:33 -0400 Subject: [PATCH 14/63] Don't use value type for 'in' arg in chpl__coerceMove See e.g. test/studies/kmeans/kmeans-blc.chpl --- compiler/include/resolution.h | 4 ++-- compiler/resolution/ResolutionCandidate.cpp | 19 ++++++++++++++----- compiler/resolution/functionResolution.cpp | 5 +++-- compiler/resolution/generics.cpp | 3 ++- compiler/resolution/wrappers.cpp | 2 +- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/compiler/include/resolution.h b/compiler/include/resolution.h index a37a5aae1f7c..3f2859649ad1 100644 --- a/compiler/include/resolution.h +++ b/compiler/include/resolution.h @@ -322,8 +322,8 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, bool implicitBang=false, bool inOrOtherValue=false); -// in/out/inout -bool inOrOutFormal(ArgSymbol* formal); +// in/out/inout but excluding formals to chpl__coerceMove etc +bool inOrOutFormalNeedingCopyType(ArgSymbol* formal); bool isCallExprTemporary(Symbol* fromSym); bool isTemporaryFromNoCopyReturn(Symbol* fromSym); diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index cb978af286ed..d6deafae001d 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -403,13 +403,14 @@ void ResolutionCandidate::computeSubstitutionForDefaultExpr(ArgSymbol* formal, } else if (formal->type->symbol->hasFlag(FLAG_GENERIC) == true) { Type* defaultType = tail->typeInfo(); + bool inOutCopy = inOrOutFormalNeedingCopyType(formal); if (defaultType == dtTypeDefaultToken) { substitutions.put(formal, dtTypeDefaultToken->symbol); } else if (Type* type = getInstantiationType(defaultType, NULL, formal->type, NULL, ctx, true, false, - inOrOutFormal(formal))) { + inOutCopy)) { substitutions.put(formal, type->symbol); } } @@ -465,7 +466,7 @@ void clearCoercibleCache() { actualFormalCoercible.clear(); } -// Uses formalSym and actualSym to compute allowCoercion and implicitBang +// // in a way that is appropriate for uses when resolving arguments static Type* getInstantiationType(Symbol* actual, ArgSymbol* formal, Expr* ctx) { @@ -474,10 +475,10 @@ Type* getInstantiationType(Symbol* actual, ArgSymbol* formal, Expr* ctx) { bool implicitBang = allowImplicitNilabilityRemoval(actual->type, actual, formal->type, formal); - bool inOrOtherValue = inOrOutFormal(formal); + bool inOutCopy = inOrOutFormalNeedingCopyType(formal); return getInstantiationType(actual->type, actual, formal->type, formal, ctx, - allowCoercions, implicitBang, inOrOtherValue); + allowCoercions, implicitBang, inOutCopy); } @@ -520,7 +521,7 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, return ret; } -bool inOrOutFormal(ArgSymbol* formal) { +bool inOrOutFormalNeedingCopyType(ArgSymbol* formal) { // Rule out type/param variables if (formal->hasFlag(FLAG_TYPE_VARIABLE) || formal->hasFlag(FLAG_PARAM) || @@ -530,6 +531,14 @@ bool inOrOutFormal(ArgSymbol* formal) { formal->originalIntent == INTENT_PARAM) return false; + FnSymbol* inFn = formal->defPoint->getFunction(); + INT_ASSERT(inFn); + + // Don't consider 'in' in chpl__coerceMove for this purpose. + if (inFn->name == astr_coerceMove || inFn->name == astr_coerceCopy || + inFn->name == astr_initCopy || inFn->name == astr_autoCopy) + return false; + return (formal->originalIntent == INTENT_IN || formal->originalIntent == INTENT_CONST_IN || formal->originalIntent == INTENT_OUT || diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 21d33c301cc5..c7fb4f528ba3 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -443,9 +443,10 @@ static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, if (formalType == dtAny || formalType->symbol->hasFlag(FLAG_ARRAY)) { if (fn == NULL || - !(fn->name == astr_initCopy || fn->name == astr_autoCopy)) { + !(fn->name == astr_initCopy || fn->name == astr_autoCopy || + fn->name == astr_coerceMove || fn->name == astr_coerceCopy)) { - if (formalSym == NULL || inOrOutFormal(formalSym)) { + if (formalSym == NULL || inOrOutFormalNeedingCopyType(formalSym)) { copyType = getCopyTypeDuringResolution(actualType); } } diff --git a/compiler/resolution/generics.cpp b/compiler/resolution/generics.cpp index 583a2102c650..4bca35154683 100644 --- a/compiler/resolution/generics.cpp +++ b/compiler/resolution/generics.cpp @@ -689,13 +689,14 @@ FnSymbol* instantiateFunction(FnSymbol* fn, } else { Type* defType = tail->typeInfo(); + bool inOutCopy = inOrOutFormalNeedingCopyType(formal); if (defType == dtTypeDefaultToken) val = dtTypeDefaultToken->symbol; else if (Type* type = getInstantiationType(defType, NULL, newFormal->type, NULL, call, true, false, - inOrOutFormal(formal))) { + inOutCopy)) { val = type->symbol; } } diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index 4c5668e4473c..cd030be00ea9 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1179,7 +1179,7 @@ static bool needToAddCoercion(Type* actualType, if (actualType == dtNil && isClassLikeOrPtr(formalType)) return false; - if (inOrOutFormal(formal)) { + if (inOrOutFormalNeedingCopyType(formal)) { Type* toType = getCopyTypeDuringResolution(actualType); if (toType == formal->getValType()) return false; // handled by other wrapper code, e.g. handleInIntent From 247305b5e4fb47a91b2eea22ad4197e9759b3366 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 09:31:41 -0400 Subject: [PATCH 15/63] Just use --memLeaks instead of old workarounds for these tests --- .../ferguson/semantic-examples/CLEANFILES | 1 - .../arrays/ferguson/semantic-examples/EXECOPTS | 2 +- test/arrays/ferguson/semantic-examples/PRECOMP | 2 -- test/arrays/ferguson/semantic-examples/PREDIFF | 18 ------------------ 4 files changed, 1 insertion(+), 22 deletions(-) delete mode 100644 test/arrays/ferguson/semantic-examples/CLEANFILES delete mode 100755 test/arrays/ferguson/semantic-examples/PRECOMP delete mode 100755 test/arrays/ferguson/semantic-examples/PREDIFF diff --git a/test/arrays/ferguson/semantic-examples/CLEANFILES b/test/arrays/ferguson/semantic-examples/CLEANFILES deleted file mode 100644 index fa3f8f7c76e0..000000000000 --- a/test/arrays/ferguson/semantic-examples/CLEANFILES +++ /dev/null @@ -1 +0,0 @@ -memlog diff --git a/test/arrays/ferguson/semantic-examples/EXECOPTS b/test/arrays/ferguson/semantic-examples/EXECOPTS index 728dfc415ab5..3824619db918 100644 --- a/test/arrays/ferguson/semantic-examples/EXECOPTS +++ b/test/arrays/ferguson/semantic-examples/EXECOPTS @@ -1 +1 @@ ---dataParTasksPerLocale=1 --memLeaksLog=memlog +--dataParTasksPerLocale=1 --memLeaks diff --git a/test/arrays/ferguson/semantic-examples/PRECOMP b/test/arrays/ferguson/semantic-examples/PRECOMP deleted file mode 100755 index 97c40494f609..000000000000 --- a/test/arrays/ferguson/semantic-examples/PRECOMP +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/bash -rm -f memlog diff --git a/test/arrays/ferguson/semantic-examples/PREDIFF b/test/arrays/ferguson/semantic-examples/PREDIFF deleted file mode 100755 index a660d9926d1a..000000000000 --- a/test/arrays/ferguson/semantic-examples/PREDIFF +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/bash -cp $2 $2.prediff.tmp -touch $2.prediff.g.tmp -greprc=1 -if [ -f memlog ] -then - grep -A 1000 "Leaked Memory Report" memlog | grep -e '^[0-9]' > $2.prediff.g.tmp - greprc=$? -fi -if [ "$greprc" -eq 0 ] -then - echo "leaks" >> $2.prediff.tmp - cat $2.prediff.g.tmp >> $2.prediff.tmp -fi - -rm $2.prediff.g.tmp -mv $2.prediff.tmp $2 -rm memlog From 294ee90fc1fec45d0c470056c0ca001e8f430ae4 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 20 Aug 2020 16:48:51 -0400 Subject: [PATCH 16/63] Fix problems with returning slices See the included test --- compiler/include/resolution.h | 2 + compiler/passes/normalize.cpp | 16 +---- compiler/resolution/functionResolution.cpp | 16 +++-- compiler/resolution/resolveFunction.cpp | 25 ++++---- .../semantic-examples/9-slice-operations.chpl | 60 +++++++++++++++++++ .../semantic-examples/9-slice-operations.good | 12 ++++ 6 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 test/arrays/ferguson/semantic-examples/9-slice-operations.chpl create mode 100644 test/arrays/ferguson/semantic-examples/9-slice-operations.good diff --git a/compiler/include/resolution.h b/compiler/include/resolution.h index 3f2859649ad1..89595847819d 100644 --- a/compiler/include/resolution.h +++ b/compiler/include/resolution.h @@ -325,6 +325,8 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, // in/out/inout but excluding formals to chpl__coerceMove etc bool inOrOutFormalNeedingCopyType(ArgSymbol* formal); +bool isAliasingArray(Type* t); + bool isCallExprTemporary(Symbol* fromSym); bool isTemporaryFromNoCopyReturn(Symbol* fromSym); diff --git a/compiler/passes/normalize.cpp b/compiler/passes/normalize.cpp index 768489682f58..1e2e4dd240f5 100644 --- a/compiler/passes/normalize.cpp +++ b/compiler/passes/normalize.cpp @@ -3010,8 +3010,7 @@ static void updateVariableAutoDestroy(DefExpr* defExpr) { var->hasFlag(FLAG_PARAM) == false && // Note 1. var->hasFlag(FLAG_REF_VAR) == false && - fn->_this != var && // Note 2. - fn->hasFlag(FLAG_INIT_COPY_FN) == false) { // Note 3. + fn->_this != var) { // Note 2. // Note that if the DefExpr is at module scope, the auto-destroy // for it will end up in the module deinit function. @@ -3027,19 +3026,6 @@ static void updateVariableAutoDestroy(DefExpr* defExpr) { // Note 2: "this" should be passed by reference. Then, no constructor call // is made, and therefore no autodestroy call is needed. -// Note 3: If a record arg to an init copy function is passed by value, -// infinite recursion would ensue. This is an unreachable case (assuming that -// magic conversions from R -> ref R are removed and all existing -// implementations of chpl__initCopy are rewritten using "ref" or "const ref" -// intent on the record argument). - - -// Note 4: These two cases should be regularized. Either the copy constructor -// should *always* be called (and the corresponding destructor always called), -// or we should ensure that the destructor is called only if a constructor is -// called on the same variable. The latter case is an optimization, so the -// simplest implementation calls the copy-constructor in both cases. - /************************************* | ************************************** * * * * diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index c7fb4f528ba3..2b3a54094355 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -392,13 +392,11 @@ FnSymbol* getAutoDestroy(Type* t) { return autoDestroyMap.get(t); } -static bool isAliasingArray(Type* t) { +bool isAliasingArray(Type* t) { if (t->symbol->hasFlag(FLAG_ARRAY)) { AggregateType* at = toAggregateType(t); INT_ASSERT(at); - // check if it is an array view - // (otherwise we can infinite loop in resolving array functions) Symbol* instanceField = at->getField("_instance", false); if (instanceField) { if (instanceField->type->symbol->hasFlag(FLAG_ALIASING_ARRAY)) { @@ -415,7 +413,7 @@ Type* getCopyTypeDuringResolution(Type* t) { Type* baseType = t->getField("valType")->type; return baseType; } - if (isAliasingArray(t) || + if (isAliasingArray(t) || // avoid infinite loop in resolving array functions t->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { AggregateType* at = toAggregateType(t); INT_ASSERT(at); @@ -6743,6 +6741,16 @@ void resolveInitVar(CallExpr* call) { // copy-initialization, the compiler only currently knows about calls to // 'chpl__initCopy' and how to turn them into something else when necessary + // Normally e.g. var y = foo() - where foo returns by value - will not add a + // copy and so the result of foo() need not be auto-destroyed. However, if + // foo() is returning an array slice, then it is copied from and the source + // of the copy does need to be destroyed. + if (SymExpr* rhsSe = toSymExpr(srcExpr)) + if (VarSymbol* rhsVar = toVarSymbol(rhsSe->symbol())) + if (isAliasingArray(rhsVar->getValType())) + if (rhsVar->hasFlag(FLAG_NO_AUTO_DESTROY) == false) + rhsVar->addFlag(FLAG_INSERT_AUTO_DESTROY); + Symbol *definedConst = dst->hasFlag(FLAG_CONST)? gTrue : gFalse; CallExpr* initCopy = new CallExpr(astr_initCopy, srcExpr->remove(), definedConst); diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index f3625608e382..e5e7827c6d8b 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -759,7 +759,8 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { fromSe != NULL && isCallExprTemporary(fromSe->symbol()) && isTemporaryFromNoCopyReturn(fromSe->symbol()); - bool arrayIsh = (rhsType->symbol->hasFlag(FLAG_ARRAY) || + bool array = rhsType->symbol->hasFlag(FLAG_ARRAY); + bool arrayIsh = (array || rhsType->symbol->hasFlag(FLAG_ITERATOR_RECORD)); bool handleDomain = skipArray == false && domain; @@ -767,9 +768,6 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { bool handleTuple = skipTuple == false && isTupleContainingAnyReferences(rhsType); - // TODO: Should we check if the RHS is a symbol with - // 'no auto destroy' on it? If it is, then we'd be copying - // the RHS and it would never be destroyed... if ((handleArray || handleDomain || handleTuple) && !isTypeExpr(call->get(2))) { @@ -782,7 +780,6 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { CallExpr* initTmp = new CallExpr(PRIM_MOVE, tmp, rhs); CallExpr* unrefCall = new CallExpr(initCopyFn, tmp); CallExpr* shapeSet = findSetShape(call, ret); - FnSymbol* unrefFn = NULL; // Used by callDestructors to catch assignment from // a ref to 'tmp' when we know we don't want to copy. @@ -793,13 +790,9 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { call->insertAtTail(unrefCall); - unrefFn = resolveNormalCall(unrefCall); - - resolveFunction(unrefFn); - // Relies on the ArrayView variant having // the 'unref fn' flag in ChapelArray. - if (arrayIsh && unrefFn->hasFlag(FLAG_UNREF_FN) == false) { + if (array && isAliasingArray(rhs->getValType()) == false) { // If the function does not have this flag, this must // be a non-view array. Remove the unref call. unrefCall->replace(rhs->copy()); @@ -812,11 +805,13 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { if (shapeSet) setIteratorRecordShape(shapeSet); - } else if (shapeSet) { - // Set the shape on the array unref temp instead of 'ret'. - shapeSet->get(1)->replace(new SymExpr(tmp)); - call->insertBefore(shapeSet->remove()); - setIteratorRecordShape(shapeSet); + } else { + if (shapeSet) { + // Set the shape on the array unref temp instead of 'ret'. + shapeSet->get(1)->replace(new SymExpr(tmp)); + call->insertBefore(shapeSet->remove()); + setIteratorRecordShape(shapeSet); + } } } } diff --git a/test/arrays/ferguson/semantic-examples/9-slice-operations.chpl b/test/arrays/ferguson/semantic-examples/9-slice-operations.chpl new file mode 100644 index 000000000000..d651971ae44a --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-slice-operations.chpl @@ -0,0 +1,60 @@ +var A:[1..10] int; + +proc foo() { + return A[1..5]; +} + +proc test0() { + writeln("test0"); + ref B = A[1..5]; + B = 1; + writeln(A); +} +A=0; +test0(); + + +proc test1a() { + writeln("test1a"); + var B = A[1..5]; + B = 1; + writeln(A); +} +A=0; +test1a(); + +proc test1b() { + writeln("test1b"); + var B:[1..5] int = A[1..5]; + B = 1; + writeln(A); +} +A=0; +test1b(); + +proc test2a() { + writeln("test2a"); + var B = foo(); + B = 1; + writeln(A); +} +A=0; +test2a(); + +proc test2b() { + writeln("test2b"); + var B:[1..5] int = foo(); + B = 1; + writeln(A); +} +A=0; +test2b(); + +proc test2c() { + writeln("test2c"); + ref B = foo(); + B = 1; + writeln(A); +} +A=0; +test2c(); diff --git a/test/arrays/ferguson/semantic-examples/9-slice-operations.good b/test/arrays/ferguson/semantic-examples/9-slice-operations.good new file mode 100644 index 000000000000..41549b9e4f42 --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-slice-operations.good @@ -0,0 +1,12 @@ +test0 +1 1 1 1 1 0 0 0 0 0 +test1a +0 0 0 0 0 0 0 0 0 0 +test1b +0 0 0 0 0 0 0 0 0 0 +test2a +0 0 0 0 0 0 0 0 0 0 +test2b +0 0 0 0 0 0 0 0 0 0 +test2c +0 0 0 0 0 0 0 0 0 0 From 9b0e795c400b8cb52cec467d0f1dbd39a72755cd Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 10:53:45 -0400 Subject: [PATCH 17/63] Fix canCoerceToCopyType to ignore references See e.g. arrays/ferguson/semantic-examples/4-pass-slice-inout --- compiler/resolution/functionResolution.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 2b3a54094355..bf10599e2ab1 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -432,20 +432,23 @@ static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, Type* copyType = NULL; - if (isSyncType(actualType) || isSingleType(actualType)) { - copyType = getCopyTypeDuringResolution(actualType); - } else if (isAliasingArray(actualType) || - actualType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { + Type* actualValType = actualType->getValType(); + Type* formalValType = formalType->getValType(); + + if (isSyncType(actualValType) || isSingleType(actualValType)) { + copyType = getCopyTypeDuringResolution(actualValType); + } else if (isAliasingArray(actualValType) || + actualValType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { // The conditions below avoid infinite loops and problems // relating to resolving initCopy for iterators when not needed. - if (formalType == dtAny || - formalType->symbol->hasFlag(FLAG_ARRAY)) { + if (formalValType == dtAny || + formalValType->symbol->hasFlag(FLAG_ARRAY)) { if (fn == NULL || !(fn->name == astr_initCopy || fn->name == astr_autoCopy || fn->name == astr_coerceMove || fn->name == astr_coerceCopy)) { if (formalSym == NULL || inOrOutFormalNeedingCopyType(formalSym)) { - copyType = getCopyTypeDuringResolution(actualType); + copyType = getCopyTypeDuringResolution(actualValType); } } } From f5e1b50920ad0570dd94ddc0b81b8cae69be21dc Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 14:19:02 -0400 Subject: [PATCH 18/63] Attempt to resolve issue #16275 So far, only improves the array parts. --- compiler/resolution/functionResolution.cpp | 9 +++ modules/internal/ChapelArray.chpl | 25 ++++++-- test/arrays/ferguson/views-and-copying.chpl | 63 +++++++++++++++++++++ test/arrays/ferguson/views-and-copying.good | 21 +++++++ 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 test/arrays/ferguson/views-and-copying.chpl create mode 100644 test/arrays/ferguson/views-and-copying.good diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index bf10599e2ab1..203c4e989b09 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -420,6 +420,10 @@ Type* getCopyTypeDuringResolution(Type* t) { FnSymbol* fn = getInitCopyDuringResolution(at); INT_ASSERT(fn); + + if (fn->retType == t) + INT_FATAL("Expected different return type for this initCopy"); + return fn->retType; } @@ -6778,6 +6782,11 @@ void resolveInitVar(CallExpr* call) { resolveMove(call); } + if (isAliasingArray(srcExpr->getValType()) || initCopyIter) + if (FnSymbol* initCopyFn = initCopy->resolvedFunction()) + if (initCopyFn->retType == srcExpr->getValType()) + INT_FATAL("Expected different return type for this initCopy"); + } else if (isRecord(targetType->getValType())) { AggregateType* at = toAggregateType(targetType->getValType()); diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 0f6772e81337..9c364fd3c775 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -4469,9 +4469,27 @@ module ChapelArray { pragma "init copy fn" proc chpl__initCopy(const ref rhs: [], definedConst: bool) { - pragma "no copy" - var lhs = chpl__coerceCopy(rhs.type, rhs, definedConst); - return lhs; + + // Reindex and rank change domains should use a non-view domain + // when creating the new array. This is not a problem for slices + // because they already use a non-view domain. + if rhs._value.isRankChangeArrayView() { + type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.upDom), + rhs.eltType); + pragma "no copy" + var lhs = chpl__coerceCopy(t, rhs); + return lhs; + } else if rhs._value.isReindexArrayView() { + type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.updom), + rhs.eltType); + pragma "no copy" + var lhs = chpl__coerceCopy(t, rhs); + return lhs; + } else { + pragma "no copy" + var lhs = chpl__coerceCopy(rhs.type, rhs); + return lhs; + } } pragma "auto copy fn" proc chpl__autoCopy(x: [], definedConst: bool) { @@ -4649,7 +4667,6 @@ module ChapelArray { return lhs; } - pragma "find user line" pragma "coerce fn" proc chpl__coerceCopy(type dstType:_array, rhs:_array, definedConst: bool) { diff --git a/test/arrays/ferguson/views-and-copying.chpl b/test/arrays/ferguson/views-and-copying.chpl new file mode 100644 index 000000000000..e3b71860feb5 --- /dev/null +++ b/test/arrays/ferguson/views-and-copying.chpl @@ -0,0 +1,63 @@ +proc returnIt(arg) { return arg; } + +proc main() { + var A: [1..2] real; + var AA: [1..2, 1..2] real; + + { + writeln(); + writeln("slice"); + const ref rB = A[1..1]; + writeln("to const ref is view: ", rB.isSliceArrayView()); + + var B = A[1..1]; + writeln("to var is view: ", B.isSliceArrayView()); + + const ref rReturned = returnIt(A[1..1]); + writeln("returned is view: ", rReturned.isSliceArrayView()); + + const ref rD = A[1..1].domain; + writeln(".domain to const ref is view: ", rD.isSliceDomainView()); + + var D = A[1..1].domain; + writeln(".domain to var is view: ", D.isSliceDomainView()); + } + + { + writeln(); + writeln("reindex"); + const ref rB = A.reindex(3..4); + writeln("to const ref is view: ", rB.isReindexArrayView()); + + var B = A.reindex(3..4); + writeln("to var is view: ", B.isReindexArrayView()); + + const ref rReturned = returnIt(A.reindex(3..4)); + writeln("returned is view: ", rReturned.isReindexArrayView()); + + const ref rD = A.reindex(3..4).domain; + writeln(".domain to const ref is view: ", rD.isReindexDomainView()); + + var D = A.reindex(3..4).domain; + writeln(".domain to var is view: ", D.isReindexDomainView()); + } + + { + writeln(); + writeln("rank change"); + const ref rB = AA[1, ..]; + writeln("to const ref is view: ", rB.isRankChangeArrayView()); + + var B = AA[1, ..]; + writeln("to var is view: ", B.isRankChangeArrayView()); + + const ref rReturned = returnIt(AA[1, ..]); + writeln("returned is view: ", rReturned.isRankChangeArrayView()); + + const ref rD = AA[1, ..].domain; + writeln(".domain to const ref is view: ", rD.isRankChangeDomainView()); + + var D = AA[1, ..].domain; + writeln(".domain to var is view: ", D.isRankChangeDomainView()); + } +} diff --git a/test/arrays/ferguson/views-and-copying.good b/test/arrays/ferguson/views-and-copying.good new file mode 100644 index 000000000000..557f8439bb9a --- /dev/null +++ b/test/arrays/ferguson/views-and-copying.good @@ -0,0 +1,21 @@ + +slice +to const ref is view: true +to var is view: false +returned is view: false +.domain to const ref is view: false +.domain to var is view: false + +reindex +to const ref is view: true +to var is view: false +returned is view: false +.domain to const ref is view: true +.domain to var is view: true + +rank change +to const ref is view: true +to var is view: false +returned is view: false +.domain to const ref is view: true +.domain to var is view: true From 4d991fdcb58c250dcb1c085cdc6e8fbf01568670 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 14:53:40 -0400 Subject: [PATCH 19/63] Fix rebase issue --- compiler/resolution/wrappers.cpp | 46 +++++--------------------------- 1 file changed, 6 insertions(+), 40 deletions(-) diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index cd030be00ea9..9c5baae1fd7e 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1673,10 +1673,12 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, } CallExpr* copy = NULL; + Symbol *definedConst = formal->hasFlag(FLAG_CONST) ? gTrue : gFalse; if (coerceRuntimeTypes) - copy = new CallExpr(astr_coerceCopy, runtimeTypeTemp, actualSym); + copy = new CallExpr(astr_coerceCopy, runtimeTypeTemp, actualSym, + definedConst); else - copy = new CallExpr(astr_initCopy, actualSym); + copy = new CallExpr(astr_initCopy, actualSym, definedConst); CallExpr* move = new CallExpr(PRIM_MOVE, tmp, copy); anchor->insertBefore(new DefExpr(tmp)); @@ -1703,15 +1705,9 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, tmp->addFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT); } - CallExpr* copy = NULL; Symbol *definedConst = formal->hasFlag(FLAG_CONST) ? gTrue : gFalse; - if (coerceRuntimeTypes) { - copy = new CallExpr(astr_coerceCopy, runtimeTypeTemp, actualSym, - definedConst); - } - else { - copy = new CallExpr(astr_initCopy, actualSym, definedConst); - } + CallExpr* copy = new CallExpr(astr_coerceMove, + runtimeTypeTemp, actualSym, definedConst); CallExpr* move = new CallExpr(PRIM_MOVE, tmp, copy); anchor->insertBefore(new DefExpr(tmp)); @@ -1721,36 +1717,6 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, resolveCall(move); actual->setSymbol(tmp); - } else { - // Is actualSym something that owns its value? - // Is it a call-temp storing the result of a call? - // Then "move" ownership to the called function - // (don't destroy it here, it will be destroyed there). - actualSym->addFlag(FLAG_NO_AUTO_DESTROY); - - if (coerceRuntimeTypes) { - VarSymbol* tmp = newTemp(astr("_formal_tmp_in_", formal->name)); - tmp->addFlag(FLAG_NO_AUTO_DESTROY); - tmp->addFlag(FLAG_EXPR_TEMP); - - // Does this need to be here? - if (formal->hasFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT)) { - tmp->addFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT); - } - - Symbol *definedConst = formal->hasFlag(FLAG_CONST) ? gTrue : gFalse; - CallExpr* copy = new CallExpr(astr_coerceMove, - runtimeTypeTemp, actualSym, definedConst); - - CallExpr* move = new CallExpr(PRIM_MOVE, tmp, copy); - anchor->insertBefore(new DefExpr(tmp)); - anchor->insertBefore(move); - - resolveCallAndCallee(copy, false); // false - allow unresolved - resolveCall(move); - - actual->setSymbol(tmp); - } } } } From 83b6910f84468683512a46361b90ec8fea6fc2fd Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 14:53:57 -0400 Subject: [PATCH 20/63] Remove trailing spaces --- compiler/resolution/wrappers.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index 9c5baae1fd7e..dd35d19d62cb 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1752,9 +1752,9 @@ static void handleOutIntents(FnSymbol* fn, CallExpr* call, SET_LINENO(currActual); nextActual = currActual->next; - bool out = formal->intent == INTENT_OUT || + bool out = formal->intent == INTENT_OUT || formal->originalIntent == INTENT_OUT; - bool inout = formal->intent == INTENT_INOUT || + bool inout = formal->intent == INTENT_INOUT || formal->originalIntent == INTENT_INOUT; if (out || inout) { @@ -1804,8 +1804,8 @@ static void handleOutIntents(FnSymbol* fn, CallExpr* call, Symbol* mapSym = inTmpToActualMap.get(actualSe->symbol()); if (mapSym == NULL) { // e.g. inout with a defaulted actual - assignTo = actualSe->symbol(); - assignFrom = actualSe->symbol(); + assignTo = actualSe->symbol(); + assignFrom = actualSe->symbol(); } else { // we have // in_tmp = copy(x) @@ -1814,7 +1814,7 @@ static void handleOutIntents(FnSymbol* fn, CallExpr* call, // add assign like // x = in_tmp assignTo = mapSym; - assignFrom = actualSe->symbol(); + assignFrom = actualSe->symbol(); } } @@ -2805,7 +2805,7 @@ void buildFastFollowerChecksIfNeeded(CallExpr* checkCall) { // Build "canHaveFastFollowers" check functions -- these don't call DSI // functions. They are called before calling DSI functions and return true for - // arrays, false otherwise. + // arrays, false otherwise. buildFastFollowerCheck(CAN_HAVE_FF, false, wrapFn, ir, requiresPromotion); buildFastFollowerCheck(CAN_HAVE_FF, true, wrapFn, ir, requiresPromotion); From a24e8bbabdcc1329d3a791902e50eb243da9fbb0 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 15:01:21 -0400 Subject: [PATCH 21/63] Fix more rebase errors --- compiler/resolution/resolveFunction.cpp | 3 ++- modules/internal/ChapelArray.chpl | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index e5e7827c6d8b..8be976740b29 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -778,7 +778,8 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { Expr* rhs = call->get(2)->remove(); VarSymbol* tmp = newTemp("copy_ret_tmp", rhsType); CallExpr* initTmp = new CallExpr(PRIM_MOVE, tmp, rhs); - CallExpr* unrefCall = new CallExpr(initCopyFn, tmp); + Symbol* definedConst = gFalse; + CallExpr* unrefCall = new CallExpr(initCopyFn, tmp, definedConst); CallExpr* shapeSet = findSetShape(call, ret); // Used by callDestructors to catch assignment from diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 9c364fd3c775..7b320afe5085 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -4477,17 +4477,17 @@ module ChapelArray { type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.upDom), rhs.eltType); pragma "no copy" - var lhs = chpl__coerceCopy(t, rhs); + var lhs = chpl__coerceCopy(t, rhs, definedConst); return lhs; } else if rhs._value.isReindexArrayView() { type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.updom), rhs.eltType); pragma "no copy" - var lhs = chpl__coerceCopy(t, rhs); + var lhs = chpl__coerceCopy(t, rhs, definedConst); return lhs; } else { pragma "no copy" - var lhs = chpl__coerceCopy(rhs.type, rhs); + var lhs = chpl__coerceCopy(rhs.type, rhs, definedConst); return lhs; } } From c7de3f7551ce43d0f56ba5371f362623df1aaf4e Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Fri, 21 Aug 2020 15:26:28 -0400 Subject: [PATCH 22/63] Address the domain parts of issue #16275 For ./test/arrays/ferguson/views-and-copying.chpl --- modules/internal/ChapelArray.chpl | 25 +++++++++++++++++---- test/arrays/ferguson/views-and-copying.good | 4 ++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 7b320afe5085..5b05c0ba3d18 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -2834,6 +2834,7 @@ module ChapelArray { if boundsChecking then checkRankChange(args); + pragma "no copy" const rcdom = this.domain[(...args)]; // TODO: With additional effort, we could collapse rank changes of @@ -3085,7 +3086,7 @@ module ChapelArray { const redistRec = new _distribution(redist); // redist._free_when_no_doms = true; - pragma "no auto destroy" const newDom = new _domain(redistRec, rank, updom.idxType, updom.stridable, updom.dims()); + pragma "no copy" pragma "no auto destroy" const newDom = new _domain(redistRec, rank, updom.idxType, updom.stridable, updom.dims()); newDom._value._free_when_no_arrs = true; // TODO: With additional effort, we could collapse reindexings of @@ -4439,9 +4440,25 @@ module ChapelArray { proc chpl__initCopy(const ref rhs: domain, definedConst: bool) where isRectangularDom(rhs) { - var lhs = new _domain(rhs.dist, rhs.rank, rhs.idxType, rhs.stridable, - rhs.dims(), definedConst=definedConst); - return lhs; + if isSubtype(rhs.dist._value.type, ArrayViewRankChangeDist) { + // Use the dist the view is over + var lhs = new _domain(_getDistribution(rhs.dist._value.downDist), + rhs.rank, rhs.idxType, rhs.stridable, + rhs.dims(), definedConst=definedConst); + + return lhs; + } else if isSubtype(rhs.dist._value.type, ArrayViewReindexDist) { + // Use the dist the view is over + var lhs = new _domain(_getDistribution(rhs.dist._value.downDist), + rhs.rank, rhs.idxType, rhs.stridable, + rhs.dims(), definedConst=definedConst); + + return lhs; + } else { + var lhs = new _domain(rhs.dist, rhs.rank, rhs.idxType, rhs.stridable, + rhs.dims(), definedConst=definedConst); + return lhs; + } } pragma "init copy fn" diff --git a/test/arrays/ferguson/views-and-copying.good b/test/arrays/ferguson/views-and-copying.good index 557f8439bb9a..0e3f9651aea6 100644 --- a/test/arrays/ferguson/views-and-copying.good +++ b/test/arrays/ferguson/views-and-copying.good @@ -11,11 +11,11 @@ to const ref is view: true to var is view: false returned is view: false .domain to const ref is view: true -.domain to var is view: true +.domain to var is view: false rank change to const ref is view: true to var is view: false returned is view: false .domain to const ref is view: true -.domain to var is view: true +.domain to var is view: false From d64ef2bcf7c0b9010f96617b7ab3b756224aa788 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 24 Aug 2020 10:04:19 -0400 Subject: [PATCH 23/63] Print formal argument AST ids when in developer mode in explainer --- compiler/AST/symbol.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/AST/symbol.cpp b/compiler/AST/symbol.cpp index d5d674bcdda1..8a761f6e7f3d 100644 --- a/compiler/AST/symbol.cpp +++ b/compiler/AST/symbol.cpp @@ -2137,10 +2137,17 @@ const char* toString(ArgSymbol* arg) { case INTENT_TYPE: intent = "type "; break; } + const char* retval = ""; if (arg->getValType() == dtAny || arg->getValType() == dtUnknown) - return astr(intent, arg->name); + retval = astr(intent, arg->name); else - return astr(intent, arg->name, ": ", toString(arg->getValType())); + retval = astr(intent, arg->name, ": ", toString(arg->getValType())); + + if (developer == true) { + retval = astr(retval, " [", istr(arg->id), "]"); + } + + return retval; } const char* toString(VarSymbol* var) { From 9229f5d2a8f5a7e2760ff5faa5f930853cf3e185 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 24 Aug 2020 10:18:15 -0400 Subject: [PATCH 24/63] Use value types in checkResolveFormalsWhereClauses See e.g. test/release/examples/primers/syncsingle.chpl The language design is not supposed to consider `ref` vs not `ref` arguments differently when resolving functions. Of course not all functions can be legally called in all places. --- compiler/resolution/ResolutionCandidate.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index d6deafae001d..1e148d382e36 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -785,16 +785,17 @@ bool ResolutionCandidate::checkResolveFormalsWhereClauses(CallInfo& info) { formal->getValType()); return false; - } else if (canDispatch(actual->type, + } else if (canDispatch(actual->getValType(), actual, - formal->type, + formal->getValType(), formal, fn, &promotes, NULL, formalIsParam) == false) { failingArgument = actual; - reason = classifyTypeMismatch(actual->type, formal->type); + reason = classifyTypeMismatch(actual->getValType(), + formal->getValType()); return false; } else if (isInitThis || isNewTypeArg) { From 96ae1b7d61b984868402d6f34f334911a29f8aa3 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 24 Aug 2020 16:41:43 -0400 Subject: [PATCH 25/63] Improve --print-module-resolution output --- compiler/resolution/functionResolution.cpp | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 203c4e989b09..56d3bb9df29e 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -182,7 +182,7 @@ static FnSymbol* autoMemoryFunction(AggregateType* at, const char* fnName); static Expr* foldTryCond(Expr* expr); static void unmarkDefaultedGenerics(); -static void resolveUses(ModuleSymbol* mod); +static void resolveUses(ModuleSymbol* mod, const char* path); static void resolveSupportForModuleDeinits(); static void resolveExports(); static void resolveEnumTypes(); @@ -8903,10 +8903,10 @@ void resolve() { resolveObviousGlobals(); - resolveUses(ModuleSymbol::mainModule()); + resolveUses(ModuleSymbol::mainModule(), ""); if (printModuleInitModule) - resolveUses(printModuleInitModule); + resolveUses(printModuleInitModule, ""); if (chpl_gen_main) resolveFunction(chpl_gen_main); @@ -9027,30 +9027,32 @@ static void unmarkDefaultedGenerics() { * * ************************************** | *************************************/ -static void resolveUses(ModuleSymbol* mod) { - static Vec initMods; - static int moduleResolutionDepth = 0; +static std::set moduleInitResolved; - if (initMods.set_in(mod) == NULL) { - initMods.set_add(mod); +static void resolveUses(ModuleSymbol* mod, const char* path) { + if (moduleInitResolved.count(mod) == 0) { + moduleInitResolved.insert(mod); - ++moduleResolutionDepth; + if (fPrintModuleResolution == true) { + // update path variable + if (path == NULL || path[0] == '\0') + path = mod->name; + else + path = astr(path, ".", mod->name); + } if (ModuleSymbol* parent = mod->defPoint->getModule()) { if (parent != theProgram && parent != rootModule) { - resolveUses(parent); + resolveUses(parent, path); } } for_vector(ModuleSymbol, usedMod, mod->modUseList) { - resolveUses(usedMod); + resolveUses(usedMod, path); } if (fPrintModuleResolution == true) { - fprintf(stderr, - "%2d Resolving module %30s ...", - moduleResolutionDepth, - mod->name); + fprintf(stderr, "%s\n from %s\n", mod->name, path); } resolveSignatureAndFunction(mod->initFn); @@ -9064,10 +9066,9 @@ static void resolveUses(ModuleSymbol* mod) { mod->accept(&visitor); - fprintf(stderr, " %6d asts\n", visitor.total()); + if (developer) + fprintf(stderr, "%s contains %6d asts\n", mod->name, visitor.total()); } - - --moduleResolutionDepth; } } From 67987c0c83dcaf9faa88ea800a1ed405184293c6 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 24 Aug 2020 17:32:25 -0400 Subject: [PATCH 26/63] Workaround for order of resolution issue with hello --no-local --- compiler/include/flags_list.h | 1 + compiler/resolution/resolveFunction.cpp | 1 + modules/internal/ChapelArray.chpl | 1 + modules/internal/ChapelLocale.chpl | 1 + 4 files changed, 4 insertions(+) diff --git a/compiler/include/flags_list.h b/compiler/include/flags_list.h index 1b34ff390b12..14a884fe3381 100644 --- a/compiler/include/flags_list.h +++ b/compiler/include/flags_list.h @@ -270,6 +270,7 @@ symbolFlag( FLAG_NO_CAPTURE_FOR_TASKING , npr, "no capture for tasking", "does n symbolFlag( FLAG_NO_CODEGEN , ypr, "no codegen" , "do not generate e.g. C code defining this symbol" ) symbolFlag( FLAG_NO_COPY , ypr, "no copy" , "do not apply chpl__initCopy to initialization of a variable" ) symbolFlag( FLAG_NO_COPY_RETURN, ypr, "no copy return", ncm) +symbolFlag( FLAG_NO_COPY_RETURNS_OWNED, ypr, "no copy returns owned", ncm) symbolFlag( FLAG_NO_DEFAULT_FUNCTIONS , ypr, "no default functions" , ncm ) symbolFlag( FLAG_NO_DOC, ypr, "no doc", "do not generate chpldoc documentation for this symbol" ) symbolFlag( FLAG_NO_IMPLICIT_COPY , ypr, "no implicit copy" , "function does not require autoCopy/autoDestroy" ) diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 8be976740b29..2c21ae88cbfa 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -823,6 +823,7 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { static bool doNotUnaliasArray(FnSymbol* fn) { return (fn->hasFlag(FLAG_NO_COPY_RETURN) || fn->hasFlag(FLAG_RUNTIME_TYPE_INIT_FN) || + fn->hasFlag(FLAG_NO_COPY_RETURNS_OWNED) || fn->hasFlag(FLAG_INIT_COPY_FN) || fn->hasFlag(FLAG_AUTO_COPY_FN) || fn->hasFlag(FLAG_COERCE_FN) || diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 5b05c0ba3d18..3d632397a683 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -454,6 +454,7 @@ module ChapelArray { return dom.buildArray(eltType, true); } + pragma "no copy returns owned" // workaround for order of resolution issue proc chpl__convertRuntimeTypeToValue(dom: domain, type eltType, param isNoInit: bool, diff --git a/modules/internal/ChapelLocale.chpl b/modules/internal/ChapelLocale.chpl index 1bfb0afdd44d..25e2035600ba 100644 --- a/modules/internal/ChapelLocale.chpl +++ b/modules/internal/ChapelLocale.chpl @@ -118,6 +118,7 @@ module ChapelLocale { pragma "no doc" var dummyLocale = new locale(localeKind.dummy); + // record locale - defines the locale record - called _locale to aid parsing pragma "no doc" pragma "always RVF" record _locale { From e854637f4ac16a81c10ec1aa284c4dc23759e307 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 11:10:46 -0400 Subject: [PATCH 27/63] Adjust alias output The changed output is not the focus of the tests. --- test/analysis/alias/array-arguments-in.compgood | 1 - test/analysis/alias/array-of-classes-variant.compgood | 5 ----- test/analysis/alias/array-of-classes.compgood | 5 ----- 3 files changed, 11 deletions(-) diff --git a/test/analysis/alias/array-arguments-in.compgood b/test/analysis/alias/array-arguments-in.compgood index b2353d6b6f44..f24a45cbefa8 100644 --- a/test/analysis/alias/array-arguments-in.compgood +++ b/test/analysis/alias/array-arguments-in.compgood @@ -1,6 +1,5 @@ noAliasSets: no-aliases for function main: A no alias B - A no alias ret noAliasSets: no-aliases for function test: a no ref alias b a no ref alias return argument diff --git a/test/analysis/alias/array-of-classes-variant.compgood b/test/analysis/alias/array-of-classes-variant.compgood index ac8990bb7af4..66bd45e1a98d 100644 --- a/test/analysis/alias/array-of-classes-variant.compgood +++ b/test/analysis/alias/array-of-classes-variant.compgood @@ -1,9 +1,6 @@ noAliasSets: no-aliases for function makeArray: noAliasSets: no-aliases for function main: A no alias B - A no alias ret - B no alias ret - ret no alias ret noAliasSets: no-aliases for function deinit: noAliasSets: no-aliases for function _new: noAliasSets: no-aliases for function acceptsArrays: @@ -12,8 +9,6 @@ noAliasSets: no-aliases for function acceptsArrays: no alias noAliasSets: no-aliases for function acceptsClasses: noAliasSets: no-aliases for function acceptsIntRefs: - A may alias ret - B may alias ret LICM: may-alias report for a loop in function acceptsArrays: LICM: may-alias report for a loop in function acceptsClasses: LICM: may-alias report for a loop in function acceptsIntRefs: diff --git a/test/analysis/alias/array-of-classes.compgood b/test/analysis/alias/array-of-classes.compgood index 893285b409f4..719af25de25f 100644 --- a/test/analysis/alias/array-of-classes.compgood +++ b/test/analysis/alias/array-of-classes.compgood @@ -1,8 +1,5 @@ noAliasSets: no-aliases for function main: A no alias B - A no alias ret - B no alias ret - ret no alias ret noAliasSets: no-aliases for function deinit: noAliasSets: no-aliases for function _new: noAliasSets: no-aliases for function acceptsArrays: @@ -11,8 +8,6 @@ noAliasSets: no-aliases for function acceptsArrays: no alias noAliasSets: no-aliases for function acceptsClasses: noAliasSets: no-aliases for function acceptsIntRefs: - A may alias ret - B may alias ret LICM: may-alias report for a loop in function acceptsArrays: LICM: may-alias report for a loop in function acceptsClasses: LICM: may-alias report for a loop in function acceptsIntRefs: From 805446f024043df829aceec3b9a8043595de09ba Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 13:48:33 -0400 Subject: [PATCH 28/63] Fix const checking for inout arguments See e.g. test/functions/deitz/test_inout_const.chpl --- compiler/resolution/functionResolution.cpp | 75 +++++++--------------- compiler/resolution/wrappers.cpp | 4 +- 2 files changed, 25 insertions(+), 54 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 56d3bb9df29e..3a735e61e29d 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -611,38 +611,6 @@ static bool fits_in_uint(int width, Immediate* imm) { return false; } -// Since an `inout` copy will be made at the call site, this function -// looks for the original value before the copy in order to do correct -// const-ness checking for passing to inout arguments. -static SymExpr* findSourceOfInCopy(Symbol* sym) { - if (startsWith(sym->name, "_formal_tmp_in_")) { - for_SymbolSymExprs(se, sym) { - CallExpr* call = toCallExpr(se->getStmtExpr()); - SymExpr* lhsSe = NULL; - CallExpr* initOrCtor = NULL; - if (isInitOrReturn(call, lhsSe, initOrCtor)) { - if (lhsSe->symbol() == sym) { - if (initOrCtor == NULL && call->isPrimitive()) { - // e.g. PRIM_MOVE lhs rhs - SymExpr* rhsSe = toSymExpr(call->get(2)); - INT_ASSERT(rhsSe); - return rhsSe; - } else { - // assumes that the called function is init/coerce: - // the last argument is definedConst - // second to the last argument is the source - Expr* rhs = initOrCtor->get(initOrCtor->numActuals()-1); - SymExpr* rhsSe = toSymExpr(rhs); - INT_ASSERT(rhsSe); - return rhsSe; - } - } - } - } - } - return NULL; -} - // Is this a legal actual argument where an l-value is required? // I.e. for an out/inout/ref formal. // @@ -692,15 +660,6 @@ isLegalLvalueActualArg(ArgSymbol* formal, Expr* actual, if (! (formal && formal->hasFlag(FLAG_ARG_THIS) && calledFn && calledFn->hasFlag(FLAG_REF_TO_CONST_WHEN_CONST_THIS))) { - - if (formal->intent == INTENT_INOUT) { - // check for a formal temp to handle the `in` part; if we have - // it, look at the source of it. - if (SymExpr* sourceSe = findSourceOfInCopy(sym)) { - return isLegalLvalueActualArg(formal, sourceSe, - constnessErrorOut, exprTmpErrorOut); - } - } constnessErrorOut = constnessError; exprTmpErrorOut = exprTmpError; return false; @@ -5903,20 +5862,24 @@ static CallExpr* findOutIntentCallFromAssign(CallExpr* call, Expr** outActual, ArgSymbol** outFormal) { // Call is an assign from a temp - // Find an out argument call setting the temp + // Find an out/inout argument call setting the temp if (call->isNamed("=")) { if (SymExpr* lhs = toSymExpr(call->get(1))) { if (SymExpr* rhs = toSymExpr(call->get(2))) { - if (SymExpr* defSe = rhs->symbol()->getSingleDef()) { - CallExpr* parentCall = toCallExpr(defSe->parentExpr); - if (parentCall->resolvedFunction() != NULL) { - for_formals_actuals(formal, actual, parentCall) { - if (actual == defSe) { - if (formal->intent == INTENT_OUT || - formal->originalIntent == INTENT_OUT) { - *outActual = lhs; - *outFormal = formal; - return parentCall; + if (rhs->symbol()->hasFlag(FLAG_TEMP)) { + for_SymbolDefs(defSe, rhs->symbol()) { + CallExpr* parentCall = toCallExpr(defSe->parentExpr); + if (parentCall->resolvedFunction() != NULL) { + for_formals_actuals(formal, actual, parentCall) { + if (actual == defSe) { + if (formal->intent == INTENT_OUT || + formal->originalIntent == INTENT_OUT || + formal->intent == INTENT_INOUT || + formal->originalIntent == INTENT_INOUT) { + *outActual = lhs; + *outFormal = formal; + return parentCall; + } } } } @@ -6028,7 +5991,13 @@ static void lvalueCheckActual(CallExpr* call, Expr* actual, IntentTag intent, Ar findOutIntentCallFromAssign(call, &outActual, &outFormal); if (outCall != NULL) { - lvalueCheckActual(outCall, outActual, INTENT_OUT, outFormal); + + IntentTag intent = INTENT_OUT; + if (outFormal->intent == INTENT_INOUT || + outFormal->originalIntent == INTENT_INOUT) + intent = INTENT_INOUT; + + lvalueCheckActual(outCall, outActual, intent, outFormal); return; } } diff --git a/compiler/resolution/wrappers.cpp b/compiler/resolution/wrappers.cpp index dd35d19d62cb..ef573cc6e39c 100644 --- a/compiler/resolution/wrappers.cpp +++ b/compiler/resolution/wrappers.cpp @@ -1664,8 +1664,10 @@ static void handleInIntent(FnSymbol* fn, CallExpr* call, // to allow passing in a single argument (otherwise, when we try // to do the write-back after the call, the value would be deinited // already). - if (inout) + if (inout) { tmp->addFlag(FLAG_INSERT_AUTO_DESTROY); + tmp->addFlag(FLAG_SUPPRESS_LVALUE_ERRORS); + } // Does this need to be here? if (formal->hasFlag(FLAG_CONST_DUE_TO_TASK_FORALL_INTENT)) { From 3d7f3395a5e809e4239572ab3808d5cc10b41703 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 14:23:52 -0400 Subject: [PATCH 29/63] Accept difference in warning output for 2 scan tests See issue #16298 --- test/scan/scanViews.good | 2 +- test/scan/scanViewsBlock.good | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/scan/scanViews.good b/test/scan/scanViews.good index 3b0ec5f9526e..7702ab227d19 100644 --- a/test/scan/scanViews.good +++ b/test/scan/scanViews.good @@ -6,7 +6,7 @@ scanViews.chpl:4: warning: scan has been serialized (see issue #12482) scanViews.chpl:11: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged domain(1,int(64),false),int(64),unmanaged ArrayViewReindexDist(unmanaged DefaultDist,unmanaged domain(1,int(64),false),int(64),unmanaged domain(1,int(64),false)))] int(64)) scanViews.chpl:3: In function 'scanArr': scanViews.chpl:4: warning: scan has been serialized (see issue #12482) -scanViews.chpl:12: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged domain(1,int(64),false),int(64),unmanaged ArrayViewReindexDist(unmanaged DefaultDist,unmanaged domain(1,int(64),false),int(64),unmanaged domain(1,int(64),false)))] int(64)) +scanViews.chpl:12: Function 'scanArr' instantiated as: scanArr(X: [domain(1,int(64),false)] int(64)) scanViews.chpl:3: In function 'scanArr': scanViews.chpl:4: warning: scan has been serialized (see issue #12482) scanViews.chpl:16: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] int(64)) diff --git a/test/scan/scanViewsBlock.good b/test/scan/scanViewsBlock.good index 11cc2052ff09..ec5935577d45 100644 --- a/test/scan/scanViewsBlock.good +++ b/test/scan/scanViewsBlock.good @@ -6,7 +6,7 @@ scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) scanViewsBlock.chpl:13: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist),int(64),unmanaged ArrayViewReindexDist(unmanaged Block(1,int(64),unmanaged DefaultDist),unmanaged domain(1,int(64),false),int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist)))] int(64)) scanViewsBlock.chpl:3: In function 'scanArr': scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) -scanViewsBlock.chpl:14: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist),int(64),unmanaged ArrayViewReindexDist(unmanaged Block(1,int(64),unmanaged DefaultDist),unmanaged domain(1,int(64),false),int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist)))] int(64)) +scanViewsBlock.chpl:14: Function 'scanArr' instantiated as: scanArr(X: [BlockDom(1,int(64),false,unmanaged DefaultDist)] int(64)) scanViewsBlock.chpl:3: In function 'scanArr': scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) scanViewsBlock.chpl:18: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] int(64)) From c2d3aa497984f7806e9473f287f553e8c181ec89 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 14:29:07 -0400 Subject: [PATCH 30/63] Adjust for minor differences in lifetime checker tests --- test/classes/delete-free/lifetimes/arr-dom.chpl | 2 +- test/classes/delete-free/lifetimes/ref-escapes.good | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/classes/delete-free/lifetimes/arr-dom.chpl b/test/classes/delete-free/lifetimes/arr-dom.chpl index ca44b1b63b93..d1a6038a62b4 100644 --- a/test/classes/delete-free/lifetimes/arr-dom.chpl +++ b/test/classes/delete-free/lifetimes/arr-dom.chpl @@ -1,6 +1,6 @@ pragma "safe" module arrdom { - +pragma "no copy return" proc badReturnBorrowedArrayDom() { var A:[1..10] int; diff --git a/test/classes/delete-free/lifetimes/ref-escapes.good b/test/classes/delete-free/lifetimes/ref-escapes.good index 7cec904d9d61..cf0ed46340e2 100644 --- a/test/classes/delete-free/lifetimes/ref-escapes.good +++ b/test/classes/delete-free/lifetimes/ref-escapes.good @@ -3,6 +3,7 @@ ref-escapes.chpl:8: error: Reference to scoped variable cannot be returned ref-escapes.chpl:7: note: consider scope of x ref-escapes.chpl:11: In function 'badder': ref-escapes.chpl:12: error: Reference to scoped variable cannot be returned +ref-escapes.chpl:11: note: consider scope of x ref-escapes.chpl:20: In function 'baddest': ref-escapes.chpl:22: error: Reference to scoped variable cannot be returned ref-escapes.chpl:21: note: consider scope of r From 69f72f62b26f064a9d7df1f75a02f2520221956b Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 15:21:52 -0400 Subject: [PATCH 31/63] Adjust override-intents.good for just 1 inout argument --- test/classes/ferguson/override-intents.good | 42 +++++++++------------ 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/test/classes/ferguson/override-intents.good b/test/classes/ferguson/override-intents.good index 24165ce1a7e8..052e0d07cfef 100644 --- a/test/classes/ferguson/override-intents.good +++ b/test/classes/ferguson/override-intents.good @@ -7,12 +7,24 @@ override-intents.chpl:83: error: overriding method uses 'const in' intent override-intents.chpl:86: error: Child.in_blank conflicting intent for argument 'arg' in overriding method override-intents.chpl:12: error: base method uses 'in' intent override-intents.chpl:86: error: overriding method uses default intent +override-intents.chpl:90: error: Child.out_inout conflicting intent for argument 'arg' in overriding method +override-intents.chpl:16: error: base method uses 'out' intent +override-intents.chpl:90: error: overriding method uses 'inout' intent override-intents.chpl:93: error: Child.out_ref conflicting intent for argument 'arg' in overriding method override-intents.chpl:19: error: base method uses 'out' intent override-intents.chpl:93: error: overriding method uses 'ref' intent override-intents.chpl:94: error: Child.out_const_ref conflicting intent for argument 'arg' in overriding method override-intents.chpl:20: error: base method uses 'out' intent override-intents.chpl:94: error: overriding method uses 'const ref' intent +override-intents.chpl:98: error: Child.inout_out conflicting intent for argument 'arg' in overriding method +override-intents.chpl:24: error: base method uses 'inout' intent +override-intents.chpl:98: error: overriding method uses 'out' intent +override-intents.chpl:102: error: Child.inout_ref conflicting intent for argument 'arg' in overriding method +override-intents.chpl:28: error: base method uses 'inout' intent +override-intents.chpl:102: error: overriding method uses 'ref' intent +override-intents.chpl:103: error: Child.inout_const_ref conflicting intent for argument 'arg' in overriding method +override-intents.chpl:29: error: base method uses 'inout' intent +override-intents.chpl:103: error: overriding method uses 'const ref' intent override-intents.chpl:106: error: Child.const_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:32: error: base method uses 'const' intent override-intents.chpl:106: error: overriding method uses 'in' intent @@ -34,12 +46,18 @@ override-intents.chpl:122: error: overriding method uses default intent override-intents.chpl:125: error: Child.ref_out conflicting intent for argument 'arg' in overriding method override-intents.chpl:51: error: base method uses 'ref' intent override-intents.chpl:125: error: overriding method uses 'out' intent +override-intents.chpl:126: error: Child.ref_inout conflicting intent for argument 'arg' in overriding method +override-intents.chpl:52: error: base method uses 'ref' intent +override-intents.chpl:126: error: overriding method uses 'inout' intent override-intents.chpl:130: error: Child.ref_const_ref conflicting intent for argument 'arg' in overriding method override-intents.chpl:56: error: base method uses 'ref' intent override-intents.chpl:130: error: overriding method uses 'const ref' intent override-intents.chpl:134: error: Child.const_ref_out conflicting intent for argument 'arg' in overriding method override-intents.chpl:60: error: base method uses 'const ref' intent override-intents.chpl:134: error: overriding method uses 'out' intent +override-intents.chpl:135: error: Child.const_ref_inout conflicting intent for argument 'arg' in overriding method +override-intents.chpl:61: error: base method uses 'const ref' intent +override-intents.chpl:135: error: overriding method uses 'inout' intent override-intents.chpl:138: error: Child.const_ref_ref conflicting intent for argument 'arg' in overriding method override-intents.chpl:64: error: base method uses 'const ref' intent override-intents.chpl:138: error: overriding method uses 'ref' intent @@ -72,10 +90,6 @@ override-intents.chpl:88: error: Child.out_in override keyword present but no su override-intents.chpl:88: error: Child.out_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:14: error: base method uses 'out' intent override-intents.chpl:88: error: overriding method uses 'in' intent -override-intents.chpl:90: error: Child.out_inout override keyword present but no superclass method matches signature to override -override-intents.chpl:90: error: Child.out_inout conflicting intent for argument 'arg' in overriding method -override-intents.chpl:16: error: base method uses 'out' intent -override-intents.chpl:90: error: overriding method uses 'inout' intent override-intents.chpl:91: error: Child.out_const override keyword present but no superclass method matches signature to override override-intents.chpl:91: error: Child.out_const conflicting intent for argument 'arg' in overriding method override-intents.chpl:17: error: base method uses 'out' intent @@ -92,10 +106,6 @@ override-intents.chpl:97: error: Child.inout_in override keyword present but no override-intents.chpl:97: error: Child.inout_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:23: error: base method uses 'inout' intent override-intents.chpl:97: error: overriding method uses 'in' intent -override-intents.chpl:98: error: Child.inout_out override keyword present but no superclass method matches signature to override -override-intents.chpl:98: error: Child.inout_out conflicting intent for argument 'arg' in overriding method -override-intents.chpl:24: error: base method uses 'inout' intent -override-intents.chpl:98: error: overriding method uses 'out' intent override-intents.chpl:100: error: Child.inout_const override keyword present but no superclass method matches signature to override override-intents.chpl:100: error: Child.inout_const conflicting intent for argument 'arg' in overriding method override-intents.chpl:26: error: base method uses 'inout' intent @@ -104,14 +114,6 @@ override-intents.chpl:101: error: Child.inout_const_in override keyword present override-intents.chpl:101: error: Child.inout_const_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:27: error: base method uses 'inout' intent override-intents.chpl:101: error: overriding method uses 'const in' intent -override-intents.chpl:102: error: Child.inout_ref override keyword present but no superclass method matches signature to override -override-intents.chpl:102: error: Child.inout_ref conflicting intent for argument 'arg' in overriding method -override-intents.chpl:28: error: base method uses 'inout' intent -override-intents.chpl:102: error: overriding method uses 'ref' intent -override-intents.chpl:103: error: Child.inout_const_ref override keyword present but no superclass method matches signature to override -override-intents.chpl:103: error: Child.inout_const_ref conflicting intent for argument 'arg' in overriding method -override-intents.chpl:29: error: base method uses 'inout' intent -override-intents.chpl:103: error: overriding method uses 'const ref' intent override-intents.chpl:104: error: Child.inout_blank override keyword present but no superclass method matches signature to override override-intents.chpl:104: error: Child.inout_blank conflicting intent for argument 'arg' in overriding method override-intents.chpl:30: error: base method uses 'inout' intent @@ -152,10 +154,6 @@ override-intents.chpl:124: error: Child.ref_in override keyword present but no s override-intents.chpl:124: error: Child.ref_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:50: error: base method uses 'ref' intent override-intents.chpl:124: error: overriding method uses 'in' intent -override-intents.chpl:126: error: Child.ref_inout override keyword present but no superclass method matches signature to override -override-intents.chpl:126: error: Child.ref_inout conflicting intent for argument 'arg' in overriding method -override-intents.chpl:52: error: base method uses 'ref' intent -override-intents.chpl:126: error: overriding method uses 'inout' intent override-intents.chpl:127: error: Child.ref_const override keyword present but no superclass method matches signature to override override-intents.chpl:127: error: Child.ref_const conflicting intent for argument 'arg' in overriding method override-intents.chpl:53: error: base method uses 'ref' intent @@ -172,10 +170,6 @@ override-intents.chpl:133: error: Child.const_ref_in override keyword present bu override-intents.chpl:133: error: Child.const_ref_in conflicting intent for argument 'arg' in overriding method override-intents.chpl:59: error: base method uses 'const ref' intent override-intents.chpl:133: error: overriding method uses 'in' intent -override-intents.chpl:135: error: Child.const_ref_inout override keyword present but no superclass method matches signature to override -override-intents.chpl:135: error: Child.const_ref_inout conflicting intent for argument 'arg' in overriding method -override-intents.chpl:61: error: base method uses 'const ref' intent -override-intents.chpl:135: error: overriding method uses 'inout' intent override-intents.chpl:136: error: Child.const_ref_const override keyword present but no superclass method matches signature to override override-intents.chpl:136: error: Child.const_ref_const conflicting intent for argument 'arg' in overriding method override-intents.chpl:62: error: base method uses 'const ref' intent From 5423b4147e7abb3666c09611a05e287abdc65de1 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 15:48:48 -0400 Subject: [PATCH 32/63] Fix problem with creating array of owned from forall expr See e.g. test/classes/delete-free/owned/owned-init.chpl --- modules/internal/ChapelArray.chpl | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 3d632397a683..cc1f500f18b7 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -5092,6 +5092,7 @@ module ChapelArray { return chpl__initCopy_shapeHelp(shape, ir); } + pragma "no copy returns owned" pragma "ignore transfer errors" proc chpl__initCopy_shapeHelp(shape: domain, ir: _iteratorRecord) { From fc803e4e33563715f87697ed3d290bc076946a69 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 16:19:21 -0400 Subject: [PATCH 33/63] Update tmacd-promotion One of the cases stopped printing out a rank change array (due to the resolution of issue #16275). See issue #16300 which asks questions about the current output. --- modules/internal/ChapelArray.chpl | 1 + test/arrays/shapes/tmacd-promotion.chpl | 54 ++++++++++++++++++++----- test/arrays/shapes/tmacd-promotion.good | 15 +++++-- 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index cc1f500f18b7..efd91794c2aa 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -5070,6 +5070,7 @@ module ChapelArray { // instances // TODO: can we make this domain constant by passing an argument to the // initializer? + // MPPF: Should this shape be un-view'd? var shape = new _domain(ir._shape_); // Important: ir._shape_ points to a domain class for a domain diff --git a/test/arrays/shapes/tmacd-promotion.chpl b/test/arrays/shapes/tmacd-promotion.chpl index 693a6c0511a2..14287e256bf9 100644 --- a/test/arrays/shapes/tmacd-promotion.chpl +++ b/test/arrays/shapes/tmacd-promotion.chpl @@ -1,12 +1,48 @@ config const n = 2; var b: [0..n+1, 0..n+1, 0..1] int; -const N = b[0..n-1, 0..n-1, 0]; -writeln("====== N ======"); -writeln(N); -writeln(N.type:string); - -const NN = b[0..n-1, 0..n-1, 0] + b[0..n-1, 2..n+1, 0]; -writeln("====== NN ====="); -writeln(NN); -writeln(NN.type:string); +proc initb() { + for i in 0..n+1 { + for j in 0..n+1 { + for k in 0..1 { + b[i,j,k] = 10000*i + 100*j + k; + } + } + } +} + +{ + initb(); + const N = b[0..n-1, 0..n-1, 0]; + b = 0; + writeln("====== const N ======"); + writeln(N); + writeln(N.type:string); +} + +{ + initb(); + const NN = b[0..n-1, 0..n-1, 0] + b[0..n-1, 2..n+1, 0]; + b = 0; + writeln("====== const NN ====="); + writeln(NN); + writeln(NN.type:string); +} + +{ + initb(); + const ref N = b[0..n-1, 0..n-1, 0]; + b = 0; + writeln("====== const ref N ======"); + writeln(N); + writeln(N.type:string); +} + +{ + initb(); + const ref NN = b[0..n-1, 0..n-1, 0] + b[0..n-1, 2..n+1, 0]; + b = 0; + writeln("====== const ref NN ====="); + writeln(NN); + writeln(NN.type:string); +} diff --git a/test/arrays/shapes/tmacd-promotion.good b/test/arrays/shapes/tmacd-promotion.good index d559e0139d9d..c0cb1c9bcaca 100644 --- a/test/arrays/shapes/tmacd-promotion.good +++ b/test/arrays/shapes/tmacd-promotion.good @@ -1,8 +1,15 @@ -====== N ====== -0 0 -0 0 +====== const N ====== +0 100 +10000 10100 +[domain(2,int(64),false)] int(64) +====== const NN ===== +200 400 +20200 20400 [ArrayViewRankChangeDom(2,int(64),false,3*bool,3*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,3*bool,3*int(64)))] int(64) -====== NN ===== +====== const ref N ====== 0 0 0 0 [ArrayViewRankChangeDom(2,int(64),false,3*bool,3*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,3*bool,3*int(64)))] int(64) +====== const ref NN ===== +0 0 0 0 +iterator From 1c96ffa93a1666cc9bb56f9fe560ec01abefb48c Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 16:52:56 -0400 Subject: [PATCH 34/63] Add test for 16301 and adjust test of const checking domain task intent --- .../checks/forall-inout-domain-bug-16301.chpl | 14 ++++++++++++++ .../checks/forall-inout-domain-bug-16301.good | 3 +++ .../taskPar/taskIntents/03-error-in-fun.good | 12 ++++++++++++ 3 files changed, 29 insertions(+) create mode 100644 test/parallel/forall/checks/forall-inout-domain-bug-16301.chpl create mode 100644 test/parallel/forall/checks/forall-inout-domain-bug-16301.good diff --git a/test/parallel/forall/checks/forall-inout-domain-bug-16301.chpl b/test/parallel/forall/checks/forall-inout-domain-bug-16301.chpl new file mode 100644 index 000000000000..6937c2503a3b --- /dev/null +++ b/test/parallel/forall/checks/forall-inout-domain-bug-16301.chpl @@ -0,0 +1,14 @@ +config const n = 1; + +proc main() { + var D = {1..3}; + + foo(D); +} + +proc foo(inout D: domain(1)) { + + forall i in 1..n { // task intent required? + D = {2..4}; // race if n > 1 + } +} diff --git a/test/parallel/forall/checks/forall-inout-domain-bug-16301.good b/test/parallel/forall/checks/forall-inout-domain-bug-16301.good new file mode 100644 index 000000000000..ed4792bb4eba --- /dev/null +++ b/test/parallel/forall/checks/forall-inout-domain-bug-16301.good @@ -0,0 +1,3 @@ +forall-inout-domain-bug-16301.chpl:9: In function 'foo': +forall-inout-domain-bug-16301.chpl:12: error: cannot assign to const variable +forall-inout-domain-bug-16301.chpl:11: note: The shadow variable 'D' is constant due to forall intents in this loop diff --git a/test/parallel/taskPar/taskIntents/03-error-in-fun.good b/test/parallel/taskPar/taskIntents/03-error-in-fun.good index 9031dc65f026..6fae3265533b 100644 --- a/test/parallel/taskPar/taskIntents/03-error-in-fun.good +++ b/test/parallel/taskPar/taskIntents/03-error-in-fun.good @@ -360,6 +360,10 @@ 03-error-in-fun.chpl:785: note: The shadow variable 'enm' is constant due to task intents in this begin statement 03-error-in-fun.chpl:810: error: cannot assign to const variable 03-error-in-fun.chpl:785: note: The shadow variable 'cls' is constant due to task intents in this begin statement +03-error-in-fun.chpl:812: error: cannot assign to const variable +03-error-in-fun.chpl:785: note: The shadow variable 'dom1' is constant due to task intents in this begin statement +03-error-in-fun.chpl:813: error: cannot assign to const variable +03-error-in-fun.chpl:785: note: The shadow variable 'dom2' is constant due to task intents in this begin statement 03-error-in-fun.chpl:819: error: cannot assign to const variable 03-error-in-fun.chpl:817: note: The shadow variable 'b0' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:820: error: cannot assign to const variable @@ -402,6 +406,10 @@ 03-error-in-fun.chpl:817: note: The shadow variable 'enm' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:842: error: cannot assign to const variable 03-error-in-fun.chpl:817: note: The shadow variable 'cls' is constant due to task intents in this parallel statement +03-error-in-fun.chpl:844: error: cannot assign to const variable +03-error-in-fun.chpl:817: note: The shadow variable 'dom1' is constant due to task intents in this parallel statement +03-error-in-fun.chpl:845: error: cannot assign to const variable +03-error-in-fun.chpl:817: note: The shadow variable 'dom2' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:851: error: cannot assign to const variable 03-error-in-fun.chpl:849: note: The shadow variable 'b0' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:852: error: cannot assign to const variable @@ -444,6 +452,10 @@ 03-error-in-fun.chpl:849: note: The shadow variable 'enm' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:874: error: cannot assign to const variable 03-error-in-fun.chpl:849: note: The shadow variable 'cls' is constant due to task intents in this parallel statement +03-error-in-fun.chpl:876: error: cannot assign to const variable +03-error-in-fun.chpl:849: note: The shadow variable 'dom1' is constant due to task intents in this parallel statement +03-error-in-fun.chpl:877: error: cannot assign to const variable +03-error-in-fun.chpl:849: note: The shadow variable 'dom2' is constant due to task intents in this parallel statement 03-error-in-fun.chpl:921: In function 'funOut': 03-error-in-fun.chpl:957: error: cannot assign to const variable 03-error-in-fun.chpl:955: note: The shadow variable 'b0' is constant due to task intents in this begin statement From 0fab450fbed81df84d195b3adca94f58530c3d9b Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 26 Aug 2020 17:38:37 -0400 Subject: [PATCH 35/63] Make int/uint <<= and >>= operators use integral shift amount Resolves a current problem with release/examples/primers/associative but I will also resolve that a different way. --- modules/internal/ChapelBase.chpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/internal/ChapelBase.chpl b/modules/internal/ChapelBase.chpl index c04d2673cbf5..cacb3df3b453 100644 --- a/modules/internal/ChapelBase.chpl +++ b/modules/internal/ChapelBase.chpl @@ -1915,20 +1915,20 @@ module ChapelBase { lhs = lhs ^ rhs; } - inline proc >>=(ref lhs:int(?w), rhs:int(w)) { + inline proc >>=(ref lhs:int(?w), rhs:integral) { __primitive(">>=", lhs, rhs); } - inline proc >>=(ref lhs:uint(?w), rhs:uint(w)) { + inline proc >>=(ref lhs:uint(?w), rhs:integral) { __primitive(">>=", lhs, rhs); } inline proc >>=(ref lhs, rhs) { lhs = lhs >> rhs; } - inline proc <<=(ref lhs:int(?w), rhs:int(w)) { + inline proc <<=(ref lhs:int(?w), rhs:integral) { __primitive("<<=", lhs, rhs); } - inline proc <<=(ref lhs:uint(?w), rhs:uint(w)) { + inline proc <<=(ref lhs:uint(?w), rhs:integral) { __primitive("<<=", lhs, rhs); } inline proc <<=(ref lhs, rhs) { From 293d6d901667cb4adc1bfe9c5f1c4d014dbedab0 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 09:46:11 -0400 Subject: [PATCH 36/63] Have _check_tuple_var_decl accept arguments with const ref intent To make compilation of this function slightly more stable to other changes. --- modules/internal/ChapelTuple.chpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/internal/ChapelTuple.chpl b/modules/internal/ChapelTuple.chpl index 0028ac352498..3d3e71fb3925 100644 --- a/modules/internal/ChapelTuple.chpl +++ b/modules/internal/ChapelTuple.chpl @@ -161,7 +161,7 @@ module ChapelTuple { return __primitive("is star tuple type", x); pragma "no doc" - proc _check_tuple_var_decl(x: _tuple, param p) param { + proc _check_tuple_var_decl(const ref x: _tuple, param p) param { if p == x.size { return true; } else { @@ -170,7 +170,7 @@ module ChapelTuple { } } pragma "no doc" - proc _check_tuple_var_decl(x, param p) param { + proc _check_tuple_var_decl(const ref x, param p) param { compilerError("illegal tuple variable declaration with non-tuple initializer"); return false; } From 25d0b028b65fb2eb6a0fdf905a156b965d633b31 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 09:46:52 -0400 Subject: [PATCH 37/63] Revert "Use value types in checkResolveFormalsWhereClauses" This reverts commit 8b6987d11ed8e6f8aca0179c32c302d9d072b341. This commit was causing problems so I will try solving the problem a different way. --- compiler/resolution/ResolutionCandidate.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index 1e148d382e36..d6deafae001d 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -785,17 +785,16 @@ bool ResolutionCandidate::checkResolveFormalsWhereClauses(CallInfo& info) { formal->getValType()); return false; - } else if (canDispatch(actual->getValType(), + } else if (canDispatch(actual->type, actual, - formal->getValType(), + formal->type, formal, fn, &promotes, NULL, formalIsParam) == false) { failingArgument = actual; - reason = classifyTypeMismatch(actual->getValType(), - formal->getValType()); + reason = classifyTypeMismatch(actual->type, formal->type); return false; } else if (isInitThis || isNewTypeArg) { From 35ac623da46649cf9d6b49f1967619180dd6f9b6 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 09:51:17 -0400 Subject: [PATCH 38/63] Add comments about TODOs related to ref types --- compiler/resolution/ResolutionCandidate.cpp | 7 +++++++ compiler/resolution/functionResolution.cpp | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index d6deafae001d..a6e3f70a7af5 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -785,6 +785,10 @@ bool ResolutionCandidate::checkResolveFormalsWhereClauses(CallInfo& info) { formal->getValType()); return false; + + // MPF TODO: one day, this should use actual/formal getValType, + // and canCoerce should be adjusted to consider intents, + // rather than depending on ref types at this stage in compilation. } else if (canDispatch(actual->type, actual, formal->type, @@ -871,6 +875,9 @@ bool ResolutionCandidate::checkGenericFormals(Expr* ctx) { bool formalIsParam = formal->hasFlag(FLAG_INSTANTIATED_PARAM) || formal->intent == INTENT_PARAM; + // MPF TODO: one day, this should use actual/formal getValType, + // and canCoerce should be adjusted to consider intents, + // rather than depending on ref types at this stage in compilation. if (canDispatch(actual->type, actual, formal->type, diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 3a735e61e29d..bae0ed9e4afa 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -1539,6 +1539,11 @@ bool canCoerce(Type* actualType, bool* paramNarrows) { bool tmpPromotes = false; bool tmpParamNarrows = false; + + // MPF TODO: use the intent instead of whether or not `formalType` + // has a `ref` type in order to rule out coercions that would not + // be allowed (e.g. passing an `int(8)` argument to a `ref x: int(64)`). + if (canParamCoerce(actualType, actualSym, formalType, &tmpParamNarrows)) { if (paramNarrows) *paramNarrows = tmpParamNarrows; return true; From 897c58ff8f0022e32813deec00488aaffb1a291c Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 09:57:09 -0400 Subject: [PATCH 39/63] Work around issue with `const` sync arguments --- modules/internal/ChapelSyncvar.chpl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/internal/ChapelSyncvar.chpl b/modules/internal/ChapelSyncvar.chpl index 49ce3bd7d5ae..e1027ac9532b 100644 --- a/modules/internal/ChapelSyncvar.chpl +++ b/modules/internal/ChapelSyncvar.chpl @@ -137,13 +137,13 @@ module ChapelSyncvar { // ``a`` needs to be a ``valType``, not a sync. // pragma "dont disable remote value forwarding" - proc init(const other : _syncvar) { + proc init(const ref other : _syncvar) { this.valType = other.valType; this.wrapped = other.wrapped; this.isOwned = false; } - proc init=(const other : _syncvar) { + proc init=(const ref other : _syncvar) { // Allow initialization from compatible sync variables, e.g.: // var x : sync int = 5; // var y : sync real = x; @@ -679,13 +679,13 @@ module ChapelSyncvar { // ``a`` needs to be a ``valType``, not a single. // pragma "dont disable remote value forwarding" - proc init(const other : _singlevar) { + proc init(const ref other : _singlevar) { this.valType = other.valType; wrapped = other.wrapped; isOwned = false; } - proc init=(const other : _singlevar) { + proc init=(const ref other : _singlevar) { // Allow initialization from compatible single variables, e.g.: // var x : single int = 5; // var y : single real = x; From 0252bf3f28b7e324c6d4343e588df4ba14a67260 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 10:05:54 -0400 Subject: [PATCH 40/63] Un-future / Un-no-test tests as #16290 is resolved --- .../initializers/compilerGenerated/copyDomainFormal.bad | 6 ------ .../compilerGenerated/copyDomainFormal.future | 9 --------- .../modifyDomain-array-created-by-init.notest | 0 3 files changed, 15 deletions(-) delete mode 100644 test/classes/initializers/compilerGenerated/copyDomainFormal.bad delete mode 100644 test/classes/initializers/compilerGenerated/copyDomainFormal.future delete mode 100644 test/classes/initializers/compilerGenerated/modifyDomain-array-created-by-init.notest diff --git a/test/classes/initializers/compilerGenerated/copyDomainFormal.bad b/test/classes/initializers/compilerGenerated/copyDomainFormal.bad deleted file mode 100644 index 9b214da44414..000000000000 --- a/test/classes/initializers/compilerGenerated/copyDomainFormal.bad +++ /dev/null @@ -1,6 +0,0 @@ -These should be different: -{1..5} -{1..5} -These should be different: -{1..5} -{1..5} diff --git a/test/classes/initializers/compilerGenerated/copyDomainFormal.future b/test/classes/initializers/compilerGenerated/copyDomainFormal.future deleted file mode 100644 index 9a2e04769db1..000000000000 --- a/test/classes/initializers/compilerGenerated/copyDomainFormal.future +++ /dev/null @@ -1,9 +0,0 @@ -Compiler-generated init doesn't copy domain formals -#16290 - -With this future, we are notest'ing - - test/classes/initializers/compilerGenerated/modifyDomain-array-created-by-init - -because it causes invalid writes. Once this bug is fixed, that notest should be -removed diff --git a/test/classes/initializers/compilerGenerated/modifyDomain-array-created-by-init.notest b/test/classes/initializers/compilerGenerated/modifyDomain-array-created-by-init.notest deleted file mode 100644 index e69de29bb2d1..000000000000 From 51f807296f7e2cd6b48bbe3d4b63c908ca2f089e Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 11:03:16 -0400 Subject: [PATCH 41/63] Move check for coercion to copy type later in canCoerce For release/examples/primers/chplvis/chplvis4 --- compiler/resolution/functionResolution.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index bae0ed9e4afa..6d87b4abb134 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -1558,14 +1558,6 @@ bool canCoerce(Type* actualType, return true; } - if (Type* copyType = canCoerceToCopyType(actualType, actualSym, - formalType, formalSym, fn)) { - if (copyType != actualType) { - return canDispatch(copyType, actualSym, formalType, formalSym, fn, - promotes, paramNarrows); - } - } - if (canCoerceTuples(actualType, actualSym, formalType, formalSym, fn)) { return true; } @@ -1614,6 +1606,14 @@ bool canCoerce(Type* actualType, } } + if (Type* copyType = canCoerceToCopyType(actualType, actualSym, + formalType, formalSym, fn)) { + if (copyType != actualType) { + return canDispatch(copyType, actualSym, formalType, formalSym, fn, + promotes, paramNarrows); + } + } + return false; } From 7b841aaa700624aaee43f8b1ed1cca9d5b81f965 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 11:03:44 -0400 Subject: [PATCH 42/63] Compute copy tuple including un-viewing arrays For release/examples/primers/slices with --verify --- compiler/resolution/tuples.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/resolution/tuples.cpp b/compiler/resolution/tuples.cpp index 95e99e4fd671..4606f6bf2249 100644 --- a/compiler/resolution/tuples.cpp +++ b/compiler/resolution/tuples.cpp @@ -799,9 +799,7 @@ instantiate_tuple_initCopy_or_autoCopy(FnSymbol* fn, AggregateType* ct = origCt; - if (valueOnly) { - ct = computeCopyTuple(origCt, valueOnly, copy_fun, fn->body); - } + ct = computeCopyTuple(origCt, valueOnly, copy_fun, fn->body); BlockStmt* block = new BlockStmt(BLOCK_SCOPELESS); From e5e536cceb0adf0e720314234eefe13abb68d195 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Mon, 31 Aug 2020 16:03:11 -0400 Subject: [PATCH 43/63] Adjust tuple autoCopy See e.g. test/release/examples/primers/reductions.chpl Replaces no-longer-used borrowConvert argument with forCopy and uses it to avoid adding a reference element in the return type for autoCopy when none was present in the argument. --- compiler/resolution/tuples.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/compiler/resolution/tuples.cpp b/compiler/resolution/tuples.cpp index 4606f6bf2249..2d280567e6f9 100644 --- a/compiler/resolution/tuples.cpp +++ b/compiler/resolution/tuples.cpp @@ -937,7 +937,7 @@ static AggregateType* do_computeTupleWithIntent(bool valueOnly, AggregateType* at, const char* copyWith, BlockStmt* testBlock, - bool borrowConvert) { + bool forCopy) { INT_ASSERT(at->symbol->hasFlag(FLAG_TUPLE)); // Construct tuple that would be used for a particular argument intent. @@ -951,10 +951,16 @@ static AggregateType* do_computeTupleWithIntent(bool valueOnly, if (i != 0) { // skip size field Type* useType = field->type->getValType(); + bool allowReference = true; + if (valueOnly) + allowReference = false; + else if (forCopy) + allowReference = isReferenceType(field->type); + // Compute the result type of copying // (but don't apply this to references if !valueOnly) if (copyWith && typeNeedsCopyInitDeinit(useType) && - (valueOnly || !isReferenceType(field->type))) { + allowReference==false) { VarSymbol* var = newTemp("test_copy", useType); CallExpr* copy = new CallExpr(copyWith, var, gFalse); testBlock->insertAtTail(copy); @@ -969,9 +975,9 @@ static AggregateType* do_computeTupleWithIntent(bool valueOnly, INT_ASSERT(useAt); useType = do_computeTupleWithIntent(valueOnly, intent, useAt, - copyWith, testBlock, borrowConvert); + copyWith, testBlock, forCopy); - if (valueOnly == false) { + if (allowReference) { if (intent == INTENT_BLANK || intent == INTENT_CONST) { IntentTag concrete = concreteIntent(intent, useType); if ((concrete & INTENT_FLAG_REF) != 0) { @@ -982,14 +988,7 @@ static AggregateType* do_computeTupleWithIntent(bool valueOnly, } } else if (shouldChangeTupleType(useType) == true) { - // Argument instantiated from any that is a tuple of owned - // -> tuple of borrow - if (isManagedPtrType(useType) && borrowConvert) { - if (intent == INTENT_CONST || intent == INTENT_BLANK) - useType = getManagedPtrBorrowType(useType); - } - - if (valueOnly == false) { + if (allowReference) { // If the tuple is passed with blank intent // *and* the concrete intent for the element type // of the tuple is a type where blank-intent-means-ref, @@ -1047,7 +1046,7 @@ AggregateType* computeNonRefTuple(AggregateType* t) AggregateType* computeCopyTuple(AggregateType* t, bool valueOnly, const char* copyName, BlockStmt* testBlock) { - return do_computeTupleWithIntent(valueOnly, INTENT_BLANK, t, copyName, testBlock, false); + return do_computeTupleWithIntent(valueOnly, INTENT_BLANK, t, copyName, testBlock, true); } From 12404f29df6e8e9f46081dc9eb15694ba78ef697 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 14:24:25 -0400 Subject: [PATCH 44/63] Add isAliasingArrayType/isAliasingArrayImplType --- compiler/AST/type.cpp | 18 ++++++++++++++++ compiler/include/resolution.h | 2 -- compiler/include/type.h | 3 +++ compiler/optimizations/noAliasSets.cpp | 11 +--------- compiler/resolution/functionResolution.cpp | 24 ++++------------------ compiler/resolution/resolveFunction.cpp | 4 +--- 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/compiler/AST/type.cpp b/compiler/AST/type.cpp index 0d372361cabb..ac0d4dc3023c 100644 --- a/compiler/AST/type.cpp +++ b/compiler/AST/type.cpp @@ -1236,6 +1236,24 @@ bool isDistImplType(Type* type) return isDerivedType(type, FLAG_BASE_DIST); } +bool isAliasingArrayImplType(Type* t) { + return t->symbol->hasFlag(FLAG_ALIASING_ARRAY); +} + +bool isAliasingArrayType(Type* t) { + if (t->symbol->hasFlag(FLAG_ARRAY)) { + AggregateType* at = toAggregateType(t); + INT_ASSERT(at); + + Symbol* instanceField = at->getField("_instance", false); + if (instanceField) { + return isAliasingArrayImplType(instanceField->type); + } + } + + return false; +} + static bool isDerivedType(Type* type, Flag flag) { AggregateType* at = NULL; diff --git a/compiler/include/resolution.h b/compiler/include/resolution.h index 89595847819d..3f2859649ad1 100644 --- a/compiler/include/resolution.h +++ b/compiler/include/resolution.h @@ -325,8 +325,6 @@ Type* getInstantiationType(Type* actualType, Symbol* actualSym, // in/out/inout but excluding formals to chpl__coerceMove etc bool inOrOutFormalNeedingCopyType(ArgSymbol* formal); -bool isAliasingArray(Type* t); - bool isCallExprTemporary(Symbol* fromSym); bool isTemporaryFromNoCopyReturn(Symbol* fromSym); diff --git a/compiler/include/type.h b/compiler/include/type.h index 1c9ea25f686e..589802fce397 100644 --- a/compiler/include/type.h +++ b/compiler/include/type.h @@ -471,6 +471,9 @@ bool isRecordWrappedType(const Type* t); bool isDomImplType(Type* t); bool isArrayImplType(Type* t); bool isDistImplType(Type* t); +bool isAliasingArrayImplType(Type* t); +bool isAliasingArrayType(Type* t); + bool isManagedPtrType(const Type* t); Type* getManagedPtrBorrowType(const Type* t); AggregateType* getManagedPtrManagerType(Type* t); diff --git a/compiler/optimizations/noAliasSets.cpp b/compiler/optimizations/noAliasSets.cpp index c076968c07d3..2ac8faa32b9c 100644 --- a/compiler/optimizations/noAliasSets.cpp +++ b/compiler/optimizations/noAliasSets.cpp @@ -96,16 +96,7 @@ void addNoAliasSetForFormal(ArgSymbol* arg, static bool isNonAliasingArrayImplType(Type* t) { - // Array views are marked with this flag because they - // can alias other arrays. - if (t->symbol->hasFlag(FLAG_ALIASING_ARRAY)) - return false; - - // Non-array view array classes - if (isArrayImplType(t)) - return true; - - return false; + return isArrayImplType(t) && !isAliasingArrayImplType(t); } static diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index 6d87b4abb134..f604a8977729 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -392,28 +392,12 @@ FnSymbol* getAutoDestroy(Type* t) { return autoDestroyMap.get(t); } -bool isAliasingArray(Type* t) { - if (t->symbol->hasFlag(FLAG_ARRAY)) { - AggregateType* at = toAggregateType(t); - INT_ASSERT(at); - - Symbol* instanceField = at->getField("_instance", false); - if (instanceField) { - if (instanceField->type->symbol->hasFlag(FLAG_ALIASING_ARRAY)) { - return true; - } - } - } - - return false; -} - Type* getCopyTypeDuringResolution(Type* t) { if (isSyncType(t) || isSingleType(t)) { Type* baseType = t->getField("valType")->type; return baseType; } - if (isAliasingArray(t) || // avoid infinite loop in resolving array functions + if (isAliasingArrayType(t) || // avoid inf. loop in resolving array functions t->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { AggregateType* at = toAggregateType(t); INT_ASSERT(at); @@ -441,7 +425,7 @@ static Type* canCoerceToCopyType(Type* actualType, Symbol* actualSym, if (isSyncType(actualValType) || isSingleType(actualValType)) { copyType = getCopyTypeDuringResolution(actualValType); - } else if (isAliasingArray(actualValType) || + } else if (isAliasingArrayType(actualValType) || actualValType->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { // The conditions below avoid infinite loops and problems // relating to resolving initCopy for iterators when not needed. @@ -6728,7 +6712,7 @@ void resolveInitVar(CallExpr* call) { // of the copy does need to be destroyed. if (SymExpr* rhsSe = toSymExpr(srcExpr)) if (VarSymbol* rhsVar = toVarSymbol(rhsSe->symbol())) - if (isAliasingArray(rhsVar->getValType())) + if (isAliasingArrayType(rhsVar->getValType())) if (rhsVar->hasFlag(FLAG_NO_AUTO_DESTROY) == false) rhsVar->addFlag(FLAG_INSERT_AUTO_DESTROY); @@ -6756,7 +6740,7 @@ void resolveInitVar(CallExpr* call) { resolveMove(call); } - if (isAliasingArray(srcExpr->getValType()) || initCopyIter) + if (isAliasingArrayType(srcExpr->getValType()) || initCopyIter) if (FnSymbol* initCopyFn = initCopy->resolvedFunction()) if (initCopyFn->retType == srcExpr->getValType()) INT_FATAL("Expected different return type for this initCopy"); diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 2c21ae88cbfa..5507ba19366a 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -791,9 +791,7 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { call->insertAtTail(unrefCall); - // Relies on the ArrayView variant having - // the 'unref fn' flag in ChapelArray. - if (array && isAliasingArray(rhs->getValType()) == false) { + if (array && isAliasingArrayType(rhs->getValType()) == false) { // If the function does not have this flag, this must // be a non-view array. Remove the unref call. unrefCall->replace(rhs->copy()); From 63cdca27f691a7d21adcac5f8b2c6f87801f082c Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 14:43:22 -0400 Subject: [PATCH 45/63] Change ownsArrInstance in rank change/reindex to param Per discussion in issue #16275 Also adds the test variants from #16275 to views-and-copying.chpl --- modules/internal/ArrayViewRankChange.chpl | 6 +- modules/internal/ArrayViewReindex.chpl | 6 +- modules/internal/ChapelArray.chpl | 24 +---- test/arrays/ferguson/views-and-copying.chpl | 111 +++++++++++++++++++- test/arrays/ferguson/views-and-copying.good | 32 ++++++ 5 files changed, 148 insertions(+), 31 deletions(-) diff --git a/modules/internal/ArrayViewRankChange.chpl b/modules/internal/ArrayViewRankChange.chpl index fc9b9bba906c..8e91d2425b6d 100644 --- a/modules/internal/ArrayViewRankChange.chpl +++ b/modules/internal/ArrayViewRankChange.chpl @@ -476,12 +476,12 @@ module ArrayViewRankChange { // through the array field above. const indexCache; - const ownsArrInstance; + param ownsArrInstance; proc init(type eltType, const _DomPid, const dom, const _ArrPid, const _ArrInstance, const collapsedDim, const idx, - const ownsArrInstance : bool = false) { + param ownsArrInstance : bool = false) { super.init(eltType = eltType); this._DomPid = _DomPid; this.dom = dom; @@ -527,7 +527,7 @@ module ArrayViewRankChange { // methods like this... // override proc isRankChangeArrayView() param { - return true; + return !ownsArrInstance; } diff --git a/modules/internal/ArrayViewReindex.chpl b/modules/internal/ArrayViewReindex.chpl index 5de27e652ca8..6513379eb652 100644 --- a/modules/internal/ArrayViewReindex.chpl +++ b/modules/internal/ArrayViewReindex.chpl @@ -382,11 +382,11 @@ module ArrayViewReindex { // through the array field above. const indexCache; - const ownsArrInstance; + param ownsArrInstance; proc init(type eltType, const _DomPid, const dom, const _ArrPid, const _ArrInstance, - const ownsArrInstance : bool = false) { + param ownsArrInstance : bool = false) { super.init(eltType = eltType); this._DomPid = _DomPid; this.dom = dom; @@ -436,7 +436,7 @@ module ArrayViewReindex { // methods like this... // override proc isReindexArrayView() param { - return true; + return !ownsArrInstance; } diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index efd91794c2aa..ef9beddb24df 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -4487,27 +4487,9 @@ module ChapelArray { pragma "init copy fn" proc chpl__initCopy(const ref rhs: [], definedConst: bool) { - - // Reindex and rank change domains should use a non-view domain - // when creating the new array. This is not a problem for slices - // because they already use a non-view domain. - if rhs._value.isRankChangeArrayView() { - type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.upDom), - rhs.eltType); - pragma "no copy" - var lhs = chpl__coerceCopy(t, rhs, definedConst); - return lhs; - } else if rhs._value.isReindexArrayView() { - type t = chpl__buildArrayRuntimeType(_getDomain(rhs.domain.updom), - rhs.eltType); - pragma "no copy" - var lhs = chpl__coerceCopy(t, rhs, definedConst); - return lhs; - } else { - pragma "no copy" - var lhs = chpl__coerceCopy(rhs.type, rhs, definedConst); - return lhs; - } + pragma "no copy" + var lhs = chpl__coerceCopy(rhs.type, rhs, definedConst); + return lhs; } pragma "auto copy fn" proc chpl__autoCopy(x: [], definedConst: bool) { diff --git a/test/arrays/ferguson/views-and-copying.chpl b/test/arrays/ferguson/views-and-copying.chpl index e3b71860feb5..e924c2b5161e 100644 --- a/test/arrays/ferguson/views-and-copying.chpl +++ b/test/arrays/ferguson/views-and-copying.chpl @@ -1,6 +1,10 @@ proc returnIt(arg) { return arg; } -proc main() { +proc test1() { + writeln(); + writeln(); + writeln("### test1"); + var A: [1..2] real; var AA: [1..2, 1..2] real; @@ -12,7 +16,7 @@ proc main() { var B = A[1..1]; writeln("to var is view: ", B.isSliceArrayView()); - + const ref rReturned = returnIt(A[1..1]); writeln("returned is view: ", rReturned.isSliceArrayView()); @@ -31,7 +35,7 @@ proc main() { var B = A.reindex(3..4); writeln("to var is view: ", B.isReindexArrayView()); - + const ref rReturned = returnIt(A.reindex(3..4)); writeln("returned is view: ", rReturned.isReindexArrayView()); @@ -47,7 +51,7 @@ proc main() { writeln("rank change"); const ref rB = AA[1, ..]; writeln("to const ref is view: ", rB.isRankChangeArrayView()); - + var B = AA[1, ..]; writeln("to var is view: ", B.isRankChangeArrayView()); @@ -61,3 +65,102 @@ proc main() { writeln(".domain to var is view: ", D.isRankChangeDomainView()); } } +test1(); + +proc test2() +{ + writeln(); + writeln(); + writeln("### test2"); + + var A: [1..2] real; + var AA: [1..2, 1..2] real; + + { + writeln(); + writeln("reindex"); + var B = A.reindex(3..4); + writeln("to var is view: ", B.isReindexArrayView()); + + const ref rD = B.domain; + writeln("var.domain is view: ", rD.isReindexDomainView()); + } + + { + writeln(); + writeln("rank change"); + var B = AA[1, ..]; + writeln("to var is view: ", B.isRankChangeArrayView()); + + const ref rD = B.domain; + writeln("var.domain is view: ", rD.isRankChangeDomainView()); + } +} +test2(); + +proc test3() { + writeln(); + writeln(); + writeln("### test3"); + + var A: [1..2] real; + var AA: [1..2, 1..2] real; + + { + writeln(); + writeln("slice"); + A = 0; + const ref rB = A[1..1]; + A = 1; + writeln("to const ref behaves view: ", A[1]); + + A = 0; + var B = A[1..1]; + A = 1; + writeln("to var behaves view: ", B[1]); + + A = 0; + const ref rReturned = returnIt(A[1..1]); + A = 1; + writeln("returned behaves view: ", rReturned[1]); + } + + { + writeln(); + writeln("reindex"); + A = 0; + const ref rB = A.reindex(3..4); + A = 1; + writeln("to const ref behaves view: ", rB[3]); + + A = 0; + var B = A.reindex(3..4); + A = 1; + writeln("to var behaves view: ", B[3]); + + A = 0; + const ref rReturned = returnIt(A.reindex(3..4)); + A = 1; + writeln("returned behaves view: ", rReturned[3]); + } + + { + writeln(); + writeln("rank change"); + AA = 0; + const ref rB = AA[1, ..]; + AA = 1; + writeln("to const ref behaves view: ", rB[1]); + + AA = 0; + var B = AA[1, ..]; + AA = 1; + writeln("to var behaves view: ", B[1]); + + AA = 0; + const ref rReturned = returnIt(AA[1, ..]); + AA = 1; + writeln("returned behaves view: ", rReturned[1]); + } +} +test3(); diff --git a/test/arrays/ferguson/views-and-copying.good b/test/arrays/ferguson/views-and-copying.good index 0e3f9651aea6..e26effeb05bd 100644 --- a/test/arrays/ferguson/views-and-copying.good +++ b/test/arrays/ferguson/views-and-copying.good @@ -1,4 +1,7 @@ + +### test1 + slice to const ref is view: true to var is view: false @@ -19,3 +22,32 @@ to var is view: false returned is view: false .domain to const ref is view: true .domain to var is view: false + + +### test2 + +reindex +to var is view: false +var.domain is view: true + +rank change +to var is view: false +var.domain is view: true + + +### test3 + +slice +to const ref behaves view: 1.0 +to var behaves view: 0.0 +returned behaves view: 0.0 + +reindex +to const ref behaves view: 1.0 +to var behaves view: 0.0 +returned behaves view: 0.0 + +rank change +to const ref behaves view: 1.0 +to var behaves view: 0.0 +returned behaves view: 0.0 From beaa34b67c40b7c0a29d6df9349a82c85702e6b3 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 15:54:57 -0400 Subject: [PATCH 46/63] Add new alias test of reindex/rank change value arrays --- .../array-arguments-reindex-rank-change.chpl | 27 +++++++++++++++++++ ...ray-arguments-reindex-rank-change.compgood | 12 +++++++++ ...ray-arguments-reindex-rank-change.execgood | 0 3 files changed, 39 insertions(+) create mode 100644 test/analysis/alias/array-arguments-reindex-rank-change.chpl create mode 100644 test/analysis/alias/array-arguments-reindex-rank-change.compgood create mode 100644 test/analysis/alias/array-arguments-reindex-rank-change.execgood diff --git a/test/analysis/alias/array-arguments-reindex-rank-change.chpl b/test/analysis/alias/array-arguments-reindex-rank-change.chpl new file mode 100644 index 000000000000..0e8aece5d2a9 --- /dev/null +++ b/test/analysis/alias/array-arguments-reindex-rank-change.chpl @@ -0,0 +1,27 @@ +config const n = 10; + +proc main() { + var A: [1..n] real; + var AA: [1..n, 1..n] real; + + const ref reindexA = A.reindex(0..n-1); + var reindexVarA = reindexA; + + const ref rankChangeA = AA[1, ..]; + var rankChangeVarA = rankChangeA; + + testA1(A, reindexA); // aliases + testA2(AA, rankChangeA); // aliases + + testN3(A, reindexVarA); // does not alias + testN4(AA, reindexVarA); // does not alias + testN5(AA, rankChangeVarA); // does not alias + testN6(A, rankChangeVarA); // does not alias +} + +proc testA1(a, b) { } +proc testA2(a, b) { } +proc testN3(a, b) { } +proc testN4(a, b) { } +proc testN5(a, b) { } +proc testN6(a, b) { } diff --git a/test/analysis/alias/array-arguments-reindex-rank-change.compgood b/test/analysis/alias/array-arguments-reindex-rank-change.compgood new file mode 100644 index 000000000000..5837d3a577ab --- /dev/null +++ b/test/analysis/alias/array-arguments-reindex-rank-change.compgood @@ -0,0 +1,12 @@ +noAliasSets: no-aliases for function main: + A no alias AA +noAliasSets: no-aliases for function testA1: +noAliasSets: no-aliases for function testA2: +noAliasSets: no-aliases for function testN3: + a no ref alias b +noAliasSets: no-aliases for function testN4: + a no ref alias b +noAliasSets: no-aliases for function testN5: + a no ref alias b +noAliasSets: no-aliases for function testN6: + a no ref alias b diff --git a/test/analysis/alias/array-arguments-reindex-rank-change.execgood b/test/analysis/alias/array-arguments-reindex-rank-change.execgood new file mode 100644 index 000000000000..e69de29bb2d1 From cc0cf4f367dca0c0e08e52628e0184b4878136be Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 15:55:11 -0400 Subject: [PATCH 47/63] Remove old dead code in postFold --- compiler/resolution/postFold.cpp | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/compiler/resolution/postFold.cpp b/compiler/resolution/postFold.cpp index 479f5c9ea405..76a7819b8040 100644 --- a/compiler/resolution/postFold.cpp +++ b/compiler/resolution/postFold.cpp @@ -213,7 +213,6 @@ static Expr* postFoldNormal(CallExpr* call) { * * ************************************** | *************************************/ -static void insertValueTemp(Expr* insertPoint, Expr* actual); static bool isSameTypeOrInstantiation(Type* sub, Type* super, Expr* ctx); static Expr* postFoldPrimop(CallExpr* call) { @@ -585,12 +584,6 @@ static Expr* postFoldPrimop(CallExpr* call) { call->replace(retval); - } else if (strncmp(call->primitive->name, "_fscan", 6) == 0 || - strcmp (call->primitive->name, "_readToEndOfLine") == 0 || - strcmp (call->primitive->name, "_now_timer") == 0) { - for_actuals(actual, call) { - insertValueTemp(call->getStmtExpr(), actual); - } } return retval; @@ -646,24 +639,6 @@ bool isCoercibleOrInstantiation(Type* sub, Type* super, Expr* ctx) { return dispatch && !promotes; } - -static void insertValueTemp(Expr* insertPoint, Expr* actual) { - if (SymExpr* se = toSymExpr(actual)) { - if (se->symbol()->type->refType == NULL) { - VarSymbol* tmp = newTemp("_value_tmp_", se->symbol()->getValType()); - - insertPoint->insertBefore(new DefExpr(tmp)); - - insertPoint->insertBefore(new CallExpr(PRIM_MOVE, - tmp, - new CallExpr(PRIM_DEREF, - se->symbol()))); - - se->setSymbol(tmp); - } - } -} - /************************************* | ************************************** * * * * From ef93a43bfdbc4f5570d727ea067bd4bfd274db59 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 15:55:21 -0400 Subject: [PATCH 48/63] Set aliasing array flag for benefit of no-alias analysis The analysis in noAliasSets.cpp uses the flag FLAG_ALIASING_ARRAY to know when one array might alias another. Update rank change/reindex array types that do not alias to remove that flag. --- compiler/AST/primitive.cpp | 2 ++ compiler/include/primitive_list.h | 2 ++ compiler/resolution/preFold.cpp | 15 +++++++++++++++ modules/internal/ArrayViewRankChange.chpl | 2 ++ modules/internal/ArrayViewReindex.chpl | 2 ++ 5 files changed, 23 insertions(+) diff --git a/compiler/AST/primitive.cpp b/compiler/AST/primitive.cpp index f6c521de1f1d..2ca8550bc9a7 100644 --- a/compiler/AST/primitive.cpp +++ b/compiler/AST/primitive.cpp @@ -1085,6 +1085,8 @@ initPrimitive() { prim_def(PRIM_TO_NILABLE_CLASS, "to nilable class", returnInfoToNilable, false, false); prim_def(PRIM_TO_NON_NILABLE_CLASS, "to non nilable class", returnInfoToNonNilable, false, false); + prim_def(PRIM_SET_ALIASING_ARRAY_ON_TYPE, "set aliasing array on type", returnInfoVoid, false, false); + prim_def(PRIM_NEEDS_AUTO_DESTROY, "needs auto destroy", returnInfoBool, false, false); // if the argument is a value, mark it with "no auto destroy" diff --git a/compiler/include/primitive_list.h b/compiler/include/primitive_list.h index 752ec731fd49..f08f4acba45b 100644 --- a/compiler/include/primitive_list.h +++ b/compiler/include/primitive_list.h @@ -308,6 +308,8 @@ PRIMITIVE_R(PRIM_TO_NILABLE_CLASS) PRIMITIVE_R(PRIM_TO_NON_NILABLE_CLASS) + PRIMITIVE_R(PRIM_SET_ALIASING_ARRAY_ON_TYPE) + PRIMITIVE_G(PRIM_INVARIANT_START) PRIMITIVE_G(PRIM_NO_ALIAS_SET) PRIMITIVE_G(PRIM_COPIES_NO_ALIAS_SET) diff --git a/compiler/resolution/preFold.cpp b/compiler/resolution/preFold.cpp index 4d3ba1c8b669..043d3a531721 100644 --- a/compiler/resolution/preFold.cpp +++ b/compiler/resolution/preFold.cpp @@ -1734,6 +1734,21 @@ static Expr* preFoldPrimOp(CallExpr* call) { break; } + case PRIM_SET_ALIASING_ARRAY_ON_TYPE: { + Expr* typeArg = call->get(1); + SymExpr* valArg = toSymExpr(call->get(2)); + + Type* t = canonicalClassType(typeArg->getValType()); + if (valArg->symbol() == gTrue) + t->symbol->addFlag(FLAG_ALIASING_ARRAY); + else if (valArg->symbol() == gFalse) + t->symbol->removeFlag(FLAG_ALIASING_ARRAY); + + retval = new CallExpr(PRIM_NOOP); + call->replace(retval); + break; + } + default: break; diff --git a/modules/internal/ArrayViewRankChange.chpl b/modules/internal/ArrayViewRankChange.chpl index 8e91d2425b6d..e2881a2d9581 100644 --- a/modules/internal/ArrayViewRankChange.chpl +++ b/modules/internal/ArrayViewRankChange.chpl @@ -491,6 +491,8 @@ module ArrayViewRankChange { this.idx = idx; this.indexCache = buildIndexCacheHelper(_ArrInstance, dom, collapsedDim, idx); this.ownsArrInstance = ownsArrInstance; + this.complete(); + __primitive("set aliasing array on type", this.type, !ownsArrInstance); } // Forward all unhandled methods to underlying privatized array diff --git a/modules/internal/ArrayViewReindex.chpl b/modules/internal/ArrayViewReindex.chpl index 6513379eb652..bce1bf9db8d6 100644 --- a/modules/internal/ArrayViewReindex.chpl +++ b/modules/internal/ArrayViewReindex.chpl @@ -394,6 +394,8 @@ module ArrayViewReindex { this._ArrInstance = _ArrInstance; this.indexCache = buildIndexCacheHelper(_ArrInstance, dom); this.ownsArrInstance = ownsArrInstance; + this.complete(); + __primitive("set aliasing array on type", this.type, !ownsArrInstance); } forwarding arr except these, From d609c87b8704aaa3a656d788d57fe4049c0152f3 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 16:13:09 -0400 Subject: [PATCH 49/63] Storing reindex domain into var is still a reindex domain --- modules/internal/ChapelArray.chpl | 22 +++------------------ test/arrays/ferguson/views-and-copying.good | 4 ++-- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index ef9beddb24df..306c9963dcd4 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -4441,25 +4441,9 @@ module ChapelArray { proc chpl__initCopy(const ref rhs: domain, definedConst: bool) where isRectangularDom(rhs) { - if isSubtype(rhs.dist._value.type, ArrayViewRankChangeDist) { - // Use the dist the view is over - var lhs = new _domain(_getDistribution(rhs.dist._value.downDist), - rhs.rank, rhs.idxType, rhs.stridable, - rhs.dims(), definedConst=definedConst); - - return lhs; - } else if isSubtype(rhs.dist._value.type, ArrayViewReindexDist) { - // Use the dist the view is over - var lhs = new _domain(_getDistribution(rhs.dist._value.downDist), - rhs.rank, rhs.idxType, rhs.stridable, - rhs.dims(), definedConst=definedConst); - - return lhs; - } else { - var lhs = new _domain(rhs.dist, rhs.rank, rhs.idxType, rhs.stridable, - rhs.dims(), definedConst=definedConst); - return lhs; - } + var lhs = new _domain(rhs.dist, rhs.rank, rhs.idxType, rhs.stridable, + rhs.dims(), definedConst=definedConst); + return lhs; } pragma "init copy fn" diff --git a/test/arrays/ferguson/views-and-copying.good b/test/arrays/ferguson/views-and-copying.good index e26effeb05bd..5924d7e1d965 100644 --- a/test/arrays/ferguson/views-and-copying.good +++ b/test/arrays/ferguson/views-and-copying.good @@ -14,14 +14,14 @@ to const ref is view: true to var is view: false returned is view: false .domain to const ref is view: true -.domain to var is view: false +.domain to var is view: true rank change to const ref is view: true to var is view: false returned is view: false .domain to const ref is view: true -.domain to var is view: false +.domain to var is view: true ### test2 From 7a3640543b01d80ff6f31130e6be9be1ddb008f6 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 16:56:44 -0400 Subject: [PATCH 50/63] Domain of view array is still view --- test/arrays/shapes/tmacd-promotion.good | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/arrays/shapes/tmacd-promotion.good b/test/arrays/shapes/tmacd-promotion.good index c0cb1c9bcaca..0f88185de97e 100644 --- a/test/arrays/shapes/tmacd-promotion.good +++ b/test/arrays/shapes/tmacd-promotion.good @@ -1,7 +1,7 @@ ====== const N ====== 0 100 10000 10100 -[domain(2,int(64),false)] int(64) +[ArrayViewRankChangeDom(2,int(64),false,3*bool,3*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,3*bool,3*int(64)))] int(64) ====== const NN ===== 200 400 20200 20400 From f0476e7c16c2f072b7460e6fe60affacab2efc44 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 16:57:07 -0400 Subject: [PATCH 51/63] Adjust print-module-resolution test for more detailed output --- .../ferguson/print-module-resolution.good | 177 +++++++++++++++++- .../ferguson/print-module-resolution.prediff | 5 +- 2 files changed, 178 insertions(+), 4 deletions(-) diff --git a/test/compflags/ferguson/print-module-resolution.good b/test/compflags/ferguson/print-module-resolution.good index bb135a4cee82..dc9abc49a777 100644 --- a/test/compflags/ferguson/print-module-resolution.good +++ b/test/compflags/ferguson/print-module-resolution.good @@ -1 +1,176 @@ -nnnn Resolving module print-module-resolution ... nnnn asts +SysCTypes + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.SysCTypes +ByteBufferHelpers + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.ByteBufferHelpers +NVStringFactory + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.BytesStringCommon.NVStringFactory +SysBasic + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.BytesStringCommon.SysBasic +BytesCasts + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.BytesStringCommon.Bytes.BytesCasts +Bytes + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.BytesStringCommon.Bytes +BytesStringCommon + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.BytesStringCommon +CString + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.CString +StringCasts + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String.StringCasts +String + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv.String +ChapelEnv + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.ChapelEnv +HaltWrappers + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase.HaltWrappers +ChapelBase + from print-module-resolution.ChapelStandard.startInitCommDiags.ChapelBase +Reflection + from print-module-resolution.ChapelStandard.startInitCommDiags.CommDiagnostics.Reflection +CommDiagnostics + from print-module-resolution.ChapelStandard.startInitCommDiags.CommDiagnostics +startInitCommDiags + from print-module-resolution.ChapelStandard.startInitCommDiags +OwnedObject + from print-module-resolution.ChapelStandard.OwnedObject +MemConsistency + from print-module-resolution.ChapelStandard.SharedObject.ChapelError.ChapelLocks.Atomics.MemConsistency +Atomics + from print-module-resolution.ChapelStandard.SharedObject.ChapelError.ChapelLocks.Atomics +ChapelLocks + from print-module-resolution.ChapelStandard.SharedObject.ChapelError.ChapelLocks +ChapelError + from print-module-resolution.ChapelStandard.SharedObject.ChapelError +SharedObject + from print-module-resolution.ChapelStandard.SharedObject +NetworkAtomics + from print-module-resolution.ChapelStandard.NetworkAtomics +NetworkAtomicTypes + from print-module-resolution.ChapelStandard.NetworkAtomicTypes +AtomicsCommon + from print-module-resolution.ChapelStandard.AtomicsCommon +ChapelIteratorSupport + from print-module-resolution.ChapelStandard.ChapelIteratorSupport +ChapelThreads + from print-module-resolution.ChapelStandard.ChapelThreads +DSIUtil + from print-module-resolution.ChapelStandard.ChapelTuple.DSIUtil +ChapelTuple + from print-module-resolution.ChapelStandard.ChapelTuple +Math + from print-module-resolution.ChapelStandard.ChapelRange.Math +ChapelRange + from print-module-resolution.ChapelStandard.ChapelRange +ChapelReduce + from print-module-resolution.ChapelStandard.ChapelReduce +AlignedTSupport + from print-module-resolution.ChapelStandard.ChapelSyncvar.AlignedTSupport +SyncVarRuntimeSupport + from print-module-resolution.ChapelStandard.ChapelSyncvar.SyncVarRuntimeSupport +ChapelSyncvar + from print-module-resolution.ChapelStandard.ChapelSyncvar +ChapelTaskDataHelp + from print-module-resolution.ChapelStandard.ChapelTaskDataHelp +ChapelLocale + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.ChapelLocale +ArrayViewSlice + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ArrayViewSlice +ArrayViewRankChange + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ArrayViewRankChange +ArrayViewReindex + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ArrayViewReindex +SysError + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ChapelDebugPrint.IO.SysError +Regexp + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ChapelDebugPrint.IO.FormattedIO.Regexp +FormattedIO + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ChapelDebugPrint.IO.FormattedIO +IO + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ChapelDebugPrint.IO +ChapelDebugPrint + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.ChapelDebugPrint +List + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.List +RadixSortHelp + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.MSBRadixSort.RadixSortHelp +ShellSort + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.MSBRadixSort.ShellSort +MSBRadixSort + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.MSBRadixSort +ShallowCopy + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.QuickSort.ShallowCopy +InsertionSort + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.QuickSort.InsertionSort +QuickSort + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort.QuickSort +Sort + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray.Sort +ChapelArray + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelArray +ChapelHashtable + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelDistribution.ChapelHashtable +ChapelDistribution + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ChapelDistribution +RangeChunk + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.DefaultSparse.RangeChunk +Search + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.DefaultSparse.Search +DefaultSparse + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.DefaultSparse +ChapelHashing + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.DefaultAssociative.ChapelHashing +DefaultAssociative + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.DefaultAssociative +ExternalArray + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular.ExternalArray +DefaultRectangular + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.DefaultRectangular +ChapelNumLocales + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.ChapelNumLocales +Sys + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup.Sys +LocaleModelHelpSetup + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpSetup +LocaleModelHelpRuntime + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat.LocaleModelHelpRuntime +LocaleModelHelpFlat + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpFlat +LocaleModelHelpMem + from print-module-resolution.ChapelStandard.LocaleModel.LocaleModelHelpMem +LocaleModel + from print-module-resolution.ChapelStandard.LocaleModel +LocalesArray + from print-module-resolution.ChapelStandard.LocalesArray +ChapelAutoLocalAccess + from print-module-resolution.ChapelStandard.ChapelAutoLocalAccess +ChapelIO + from print-module-resolution.ChapelStandard.ChapelIO +LocaleTree + from print-module-resolution.ChapelStandard.LocaleTree +ChapelTaskID + from print-module-resolution.ChapelStandard.ChapelTaskID +ChapelTaskTable + from print-module-resolution.ChapelStandard.ChapelTaskTable +MemTracking + from print-module-resolution.ChapelStandard.MemTracking +ChapelUtil + from print-module-resolution.ChapelStandard.ChapelUtil +ChapelTaskData + from print-module-resolution.ChapelStandard.ChapelTaskData +CPtr + from print-module-resolution.ChapelStandard.ChapelSerializedBroadcast.CPtr +ChapelSerializedBroadcast + from print-module-resolution.ChapelStandard.ChapelSerializedBroadcast +ExportWrappers + from print-module-resolution.ChapelStandard.ExportWrappers +Builtins + from print-module-resolution.ChapelStandard.Builtins +Types + from print-module-resolution.ChapelStandard.Types +stopInitCommDiags + from print-module-resolution.ChapelStandard.stopInitCommDiags +ChapelStandard + from print-module-resolution.ChapelStandard +print-module-resolution + from print-module-resolution +PrintModuleInitOrder + from PrintModuleInitOrder diff --git a/test/compflags/ferguson/print-module-resolution.prediff b/test/compflags/ferguson/print-module-resolution.prediff index 977407c8a332..a7d2ef13fcde 100755 --- a/test/compflags/ferguson/print-module-resolution.prediff +++ b/test/compflags/ferguson/print-module-resolution.prediff @@ -9,6 +9,5 @@ MYLOG="" LAST="" -MYLOG=`grep print-module-resolution $LOG | sed 's/[0-9][0-9]*/nnnn/g'` - -echo $MYLOG > $LOG +grep -v contains $LOG > $LOG.tmp +mv $LOG.tmp $LOG From 49ba12ced7589313f584c54238a21d55fad9b760 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Wed, 2 Sep 2020 17:21:57 -0400 Subject: [PATCH 52/63] Use instantiation point from iterator when resolving auto-copy See e.g. test/classes/initializers/promotion/test_promote_initializer_owned.chpl --- compiler/resolution/functionResolution.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/compiler/resolution/functionResolution.cpp b/compiler/resolution/functionResolution.cpp index f604a8977729..4152058984fb 100644 --- a/compiler/resolution/functionResolution.cpp +++ b/compiler/resolution/functionResolution.cpp @@ -6939,11 +6939,25 @@ static FnSymbol* fixInstantiationPointAndTryResolveBody(AggregateType* at, if (FnSymbol* fn = call->resolvedFunction()) { if (fn->instantiatedFrom != NULL) { // it is a generic function, so make sure to set instantiationPoint + BlockStmt* point = NULL; if (at->symbol->instantiationPoint == NULL) { - fn->setInstantiationPoint(getInstantiationPoint(at->symbol->defPoint)); + // If the type doesn't have an instantiation point, use its defPoint + point = getInstantiationPoint(at->symbol->defPoint); + // Unless it is an iterator record - in that case use the + // instantiation point for the iterator if there is one. + if (at->symbol->hasFlag(FLAG_ITERATOR_RECORD)) { + IteratorInfo* ii = at->iteratorInfo; + if (ii != NULL && ii->iterator != NULL) { + BlockStmt* iterPt = ii->iterator->instantiationPoint(); + if (iterPt != NULL) + point = iterPt; + } + } } else { - fn->setInstantiationPoint(at->symbol->instantiationPoint); + point = at->symbol->instantiationPoint; } + INT_ASSERT(point != NULL); + fn->setInstantiationPoint(point); } inTryResolve++; From c8e4062235e8963b00b8104e18e8ee47f7479c17 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 10:51:51 -0400 Subject: [PATCH 53/63] Do not initCopy tuple elements containing references For test/expressions/loop-expr/forall-over-zip-over-arrays.chpl --- compiler/resolution/tuples.cpp | 6 ++++-- test/expressions/loop-expr/forall-over-zip-over-arrays.chpl | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/resolution/tuples.cpp b/compiler/resolution/tuples.cpp index 2d280567e6f9..79c4f073fafb 100644 --- a/compiler/resolution/tuples.cpp +++ b/compiler/resolution/tuples.cpp @@ -199,8 +199,10 @@ FnSymbol* makeConstructTuple(std::vector& args, // or less the same now. Symbol* element = NULL; - if (isReferenceType(args[i]->type)) { - // If it is a reference, pass it through + if (isReferenceType(args[i]->type) || + isTupleContainingAnyReferences(args[i]->type)) { + // If it is a reference, pass it through. + // insertCasts will handle reference level adjustments. element = arg; } else { // Otherwise, copy it diff --git a/test/expressions/loop-expr/forall-over-zip-over-arrays.chpl b/test/expressions/loop-expr/forall-over-zip-over-arrays.chpl index 5800dcf5ebc4..5faf5d51ea9e 100644 --- a/test/expressions/loop-expr/forall-over-zip-over-arrays.chpl +++ b/test/expressions/loop-expr/forall-over-zip-over-arrays.chpl @@ -3,17 +3,20 @@ var A, B: [1..4] int; var R: [1..4] real; var S: [1..4] string; +A = 0; B = 0; R = 0.0; S = ""; forall xy in ([ab in zip(A,B)] ab) do xy = (1,5); // update A and B's elements writeln(A); writeln(B); +A = 0; B = 0; R = 0.0; S = ""; forall xyz in zip([ab in zip(B,R)] ab, S) do xyz = ((2,6.6),"hi"); writeln(B); writeln(R); writeln(S); +A = 0; B = 0; R = 0.0; S = ""; forall zyx in zip(S, [ab in zip(R,A)] ab) do zyx = ("bye", (3.1,4)); writeln(A); From dd9f0c5e3a4092d2c1746bb631c52388ab6e530c Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 11:01:14 -0400 Subject: [PATCH 54/63] Revert "Accept difference in warning output for 2 scan tests" This reverts commit af113b398e80507e44628b7e318cb01ca2231133. --- test/scan/scanViews.good | 2 +- test/scan/scanViewsBlock.good | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/scan/scanViews.good b/test/scan/scanViews.good index 7702ab227d19..3b0ec5f9526e 100644 --- a/test/scan/scanViews.good +++ b/test/scan/scanViews.good @@ -6,7 +6,7 @@ scanViews.chpl:4: warning: scan has been serialized (see issue #12482) scanViews.chpl:11: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged domain(1,int(64),false),int(64),unmanaged ArrayViewReindexDist(unmanaged DefaultDist,unmanaged domain(1,int(64),false),int(64),unmanaged domain(1,int(64),false)))] int(64)) scanViews.chpl:3: In function 'scanArr': scanViews.chpl:4: warning: scan has been serialized (see issue #12482) -scanViews.chpl:12: Function 'scanArr' instantiated as: scanArr(X: [domain(1,int(64),false)] int(64)) +scanViews.chpl:12: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged domain(1,int(64),false),int(64),unmanaged ArrayViewReindexDist(unmanaged DefaultDist,unmanaged domain(1,int(64),false),int(64),unmanaged domain(1,int(64),false)))] int(64)) scanViews.chpl:3: In function 'scanArr': scanViews.chpl:4: warning: scan has been serialized (see issue #12482) scanViews.chpl:16: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] int(64)) diff --git a/test/scan/scanViewsBlock.good b/test/scan/scanViewsBlock.good index ec5935577d45..11cc2052ff09 100644 --- a/test/scan/scanViewsBlock.good +++ b/test/scan/scanViewsBlock.good @@ -6,7 +6,7 @@ scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) scanViewsBlock.chpl:13: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist),int(64),unmanaged ArrayViewReindexDist(unmanaged Block(1,int(64),unmanaged DefaultDist),unmanaged domain(1,int(64),false),int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist)))] int(64)) scanViewsBlock.chpl:3: In function 'scanArr': scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) -scanViewsBlock.chpl:14: Function 'scanArr' instantiated as: scanArr(X: [BlockDom(1,int(64),false,unmanaged DefaultDist)] int(64)) +scanViewsBlock.chpl:14: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewReindexDom(1,int(64),false,int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist),int(64),unmanaged ArrayViewReindexDist(unmanaged Block(1,int(64),unmanaged DefaultDist),unmanaged domain(1,int(64),false),int(64),unmanaged BlockDom(1,int(64),false,unmanaged DefaultDist)))] int(64)) scanViewsBlock.chpl:3: In function 'scanArr': scanViewsBlock.chpl:4: warning: scan has been serialized (see issue #12482) scanViewsBlock.chpl:18: Function 'scanArr' instantiated as: scanArr(X: [ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] int(64)) From 16e143314cccb44547e513f2c012b16457f42507 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 11:38:55 -0400 Subject: [PATCH 55/63] Fix bugs in uts tests I created issue #16329 to ask if we should allow inout/out intents for extern/export. --- test/studies/uts/sha1_rng.chpl | 16 +++------------- test/studies/uts/uts_rec.chpl | 2 +- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/test/studies/uts/sha1_rng.chpl b/test/studies/uts/sha1_rng.chpl index dbfc899a6a7d..6cfb3d1adbf6 100644 --- a/test/studies/uts/sha1_rng.chpl +++ b/test/studies/uts/sha1_rng.chpl @@ -8,28 +8,18 @@ module sha1_rng { // SHA1 Message Digest: 20 Bytes -- this is splittable and transferrable - // The message digest (hash) is implemented in Chapel as an array of 20 - // uint(8)s. C pointers to these hashes are passed into the external - // functions declared below. The way the pointer is generated from Chapel is - // currently a workaround because calling out is not fully implemented. - // To generate the pointer we declare the argument in the extern to be an - // inout uint(8) and at the call site we pass in hash[1]. In the current - // Chapel spec, copy-in/out is the responsibility of the function being - // called, so (voila!) this causes the compiler to give a pointer to the - // first element in the hash to the extern function. - // Create a fresh RNG_state context from a given seed -extern proc rng_init(inout state: uint(8), seed: sha_int); +extern proc rng_init(ref state: uint(8), seed: sha_int); // Spawn the i'th child's hash given the parent's hash // Parallel Note: Either all the spawns must be done by the same // processor, or i must be shipped along with the child -extern proc rng_spawn(inout mystate: uint(8), inout newstate: uint(8), spawnNumber: sha_int); +extern proc rng_spawn(ref mystate: uint(8), ref newstate: uint(8), spawnNumber: sha_int); // Return the current random 32 bit number from an RNG_state -extern proc rng_rand(inout mystate: uint(8)): sha_int; +extern proc rng_rand(ref mystate: uint(8)): sha_int; // Scale 32 bit positive int onto the interval [0, 1) diff --git a/test/studies/uts/uts_rec.chpl b/test/studies/uts/uts_rec.chpl index 9412ac6fc4d8..5386a66835c4 100644 --- a/test/studies/uts/uts_rec.chpl +++ b/test/studies/uts/uts_rec.chpl @@ -275,7 +275,7 @@ proc create_tree(parent: unmanaged TreeNode, wasParallel: bool = false): int { var count: int = parent.genChildren(); for i in parent.childDom do - count += create_tree(parent.children[i], false); + count += create_tree(parent.children[i]!, false); return count; } } From 08c52eef54ac33bc1c6d3a7b6a2db5c20a536e01 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 13:38:44 -0400 Subject: [PATCH 56/63] Workaround issue #16233 / #15973 For e.g. test/library/standard/DataFrames/psahabu/AddSeries.chpl --- .../standard/DataFrames/DataFrames.chpl | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/library/standard/DataFrames/DataFrames.chpl b/test/library/standard/DataFrames/DataFrames.chpl index 82e134e1271b..6326b203bc7d 100644 --- a/test/library/standard/DataFrames/DataFrames.chpl +++ b/test/library/standard/DataFrames/DataFrames.chpl @@ -495,33 +495,33 @@ module DataFrames { } iter items_fast() { - for t in zip(ords, data) do - yield t; + for (o,d) in zip(ords, data) do + yield (o,d); } iter items_fast(type idxType) { if idx { - for t in zip(idx!.these(idxType), data) do - yield t; + for (i,d) in zip(idx!.these(idxType), data) do + yield (i,d); } } // yields tuples where the first value is the valid bit pragma "no doc" iter _these() { - for t in zip(valid_bits, data) do - yield t; + for (v,d) in zip(valid_bits, data) do + yield (v,d); } pragma "no doc" iter _items() { - for t in zip(valid_bits, this.items_fast()) do - yield t; + for (v,d) in zip(valid_bits, this.items_fast()) do + yield (v,d); } pragma "no doc" iter _items(type idxType) { - for t in zip(valid_bits, this.items_fast(idxType)) do - yield t; + for (v,d) in zip(valid_bits, this.items_fast(idxType)) do + yield (v,d); } /* From 67fb6e4c59e64892320c2b7a12ea9360ac8b2ec7 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 15:17:48 -0400 Subject: [PATCH 57/63] Un-future some tests and adjust .bad files --- .../ferguson/inout-tuple-ref-wait-array.bad | 1 - .../inout-tuple-ref-wait-array.future | 2 -- .../ferguson/inout-tuple-ref-wait-int.bad | 1 - .../ferguson/inout-tuple-ref-wait-int.future | 2 -- test/arrays/views/initArrFieldWithSlice.bad | 4 +-- .../owned-transfer-from-nonnil-tuple.bad | 4 +-- test/users/ferguson/bulkerr.bad | 1 - test/users/ferguson/bulkerr.future | 2 -- .../users/npadmana/twopt/do_smu_soa-error.bad | 3 -- test/users/shetag/tensor.bad | 6 ---- test/users/shetag/tensor.chpl | 3 -- test/users/shetag/tensor.future | 14 -------- test/users/thom/arrArgIn-promote.bad | 4 --- test/users/thom/arrArgIn-promote.future | 35 ------------------- 14 files changed, 3 insertions(+), 79 deletions(-) delete mode 100644 test/arrays/ferguson/inout-tuple-ref-wait-array.bad delete mode 100644 test/arrays/ferguson/inout-tuple-ref-wait-array.future delete mode 100644 test/arrays/ferguson/inout-tuple-ref-wait-int.bad delete mode 100644 test/arrays/ferguson/inout-tuple-ref-wait-int.future delete mode 100644 test/users/ferguson/bulkerr.bad delete mode 100644 test/users/ferguson/bulkerr.future delete mode 100644 test/users/shetag/tensor.bad delete mode 100644 test/users/shetag/tensor.future delete mode 100644 test/users/thom/arrArgIn-promote.bad delete mode 100644 test/users/thom/arrArgIn-promote.future diff --git a/test/arrays/ferguson/inout-tuple-ref-wait-array.bad b/test/arrays/ferguson/inout-tuple-ref-wait-array.bad deleted file mode 100644 index 9ca54117845a..000000000000 --- a/test/arrays/ferguson/inout-tuple-ref-wait-array.bad +++ /dev/null @@ -1 +0,0 @@ -inout-tuple-ref-wait-array.chpl:6: error: inout varargs not currently supported diff --git a/test/arrays/ferguson/inout-tuple-ref-wait-array.future b/test/arrays/ferguson/inout-tuple-ref-wait-array.future deleted file mode 100644 index df55ed69c0b6..000000000000 --- a/test/arrays/ferguson/inout-tuple-ref-wait-array.future +++ /dev/null @@ -1,2 +0,0 @@ -bug: inout varargs not implemented correctly -#16148 diff --git a/test/arrays/ferguson/inout-tuple-ref-wait-int.bad b/test/arrays/ferguson/inout-tuple-ref-wait-int.bad deleted file mode 100644 index 0d6cb021baad..000000000000 --- a/test/arrays/ferguson/inout-tuple-ref-wait-int.bad +++ /dev/null @@ -1 +0,0 @@ -inout-tuple-ref-wait-int.chpl:6: error: inout varargs not currently supported diff --git a/test/arrays/ferguson/inout-tuple-ref-wait-int.future b/test/arrays/ferguson/inout-tuple-ref-wait-int.future deleted file mode 100644 index df55ed69c0b6..000000000000 --- a/test/arrays/ferguson/inout-tuple-ref-wait-int.future +++ /dev/null @@ -1,2 +0,0 @@ -bug: inout varargs not implemented correctly -#16148 diff --git a/test/arrays/views/initArrFieldWithSlice.bad b/test/arrays/views/initArrFieldWithSlice.bad index afb421065842..02be944cff8d 100644 --- a/test/arrays/views/initArrFieldWithSlice.bad +++ b/test/arrays/views/initArrFieldWithSlice.bad @@ -1,4 +1,4 @@ -initArrFieldWithSlice.chpl:30: error: unresolved call 'C.init([domain(1,int(64),false)] real(64))' +initArrFieldWithSlice.chpl:31: error: unresolved call 'C.init([ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] real(64))' initArrFieldWithSlice.chpl:1: note: this candidate did not match: C.init(X: [domain(1,int(64),false)] real(64)) -initArrFieldWithSlice.chpl:30: note: because call actual argument #1 with type [domain(1,int(64),false)] real(64) +initArrFieldWithSlice.chpl:31: note: because call actual argument #1 with type [ArrayViewRankChangeDom(1,int(64),false,2*bool,2*int(64),int(64),unmanaged ArrayViewRankChangeDist(unmanaged DefaultDist,2*bool,2*int(64)))] real(64) initArrFieldWithSlice.chpl:2: note: is passed to formal 'in X: [domain(1,int(64),false)] real(64)' diff --git a/test/classes/delete-free/owned/owned-transfer-from-nonnil-tuple.bad b/test/classes/delete-free/owned/owned-transfer-from-nonnil-tuple.bad index 2a72e5654231..d385423d9b77 100644 --- a/test/classes/delete-free/owned/owned-transfer-from-nonnil-tuple.bad +++ b/test/classes/delete-free/owned/owned-transfer-from-nonnil-tuple.bad @@ -1,3 +1 @@ -owned-transfer-from-nonnil-tuple.chpl:3: In function 'test': -owned-transfer-from-nonnil-tuple.chpl:5: error: Cannot transfer ownership from this non-nilable reference variable -owned-transfer-from-nonnil-tuple.chpl:5: error: Cannot transfer ownership from this non-nilable reference variable +owned-transfer-from-nonnil-tuple.chpl:5: error: halt reached - argument to ! is nil diff --git a/test/users/ferguson/bulkerr.bad b/test/users/ferguson/bulkerr.bad deleted file mode 100644 index 2acfd40494c7..000000000000 --- a/test/users/ferguson/bulkerr.bad +++ /dev/null @@ -1 +0,0 @@ -bulkerr.chpl:15: error: inout varargs not currently supported diff --git a/test/users/ferguson/bulkerr.future b/test/users/ferguson/bulkerr.future deleted file mode 100644 index df55ed69c0b6..000000000000 --- a/test/users/ferguson/bulkerr.future +++ /dev/null @@ -1,2 +0,0 @@ -bug: inout varargs not implemented correctly -#16148 diff --git a/test/users/npadmana/twopt/do_smu_soa-error.bad b/test/users/npadmana/twopt/do_smu_soa-error.bad index 472c5ee52bca..50a765d971cf 100644 --- a/test/users/npadmana/twopt/do_smu_soa-error.bad +++ b/test/users/npadmana/twopt/do_smu_soa-error.bad @@ -1,5 +1,2 @@ do_smu_soa-error.chpl:294: In function 'doPairs': do_smu_soa-error.chpl:299: error: Cannot transfer ownership from this non-nilable reference variable -do_smu_soa-error.chpl:301: In function 'initialPP12': -do_smu_soa-error.chpl:303: error: Cannot transfer ownership from this non-nilable reference variable -do_smu_soa-error.chpl:303: error: Cannot transfer ownership from this non-nilable reference variable diff --git a/test/users/shetag/tensor.bad b/test/users/shetag/tensor.bad deleted file mode 100644 index 78cb3412e103..000000000000 --- a/test/users/shetag/tensor.bad +++ /dev/null @@ -1,6 +0,0 @@ -tensor.chpl:67: In function 'getslice': -tensor.chpl:69: error: unresolved call 'Vector.init(int(64), [domain(1,int(64),false)] real(64))' -tensor.chpl:17: note: this candidate did not match: Vector.init(n: int(64), a: [domain(1,int(64),false)] real(64)) -tensor.chpl:69: note: because call actual argument #2 with type [domain(1,int(64),false)] real(64) -tensor.chpl:19: note: is passed to formal 'in a: [domain(1,int(64),false)] real(64)' -tensor.chpl:220: Function 'getslice' instantiated as: getslice(lo: int(64), hi: int(64)) diff --git a/test/users/shetag/tensor.chpl b/test/users/shetag/tensor.chpl index be774a3cf320..41b213af1019 100644 --- a/test/users/shetag/tensor.chpl +++ b/test/users/shetag/tensor.chpl @@ -250,9 +250,6 @@ proc main() { writeln("m4.mul(v3) = ", v5); writeln("m4.rmul(v4) = ", v6); - delete m1; - delete m4; - delete v6; delete v5; delete v4; diff --git a/test/users/shetag/tensor.future b/test/users/shetag/tensor.future deleted file mode 100644 index 49cca856a724..000000000000 --- a/test/users/shetag/tensor.future +++ /dev/null @@ -1,14 +0,0 @@ -bug: Compiler-generated initializers are surprisingly non-generic for array fields - -With the change from closed-form representation of arrays to array -views, this test no longer works due to the fact that the -compiler-generated initializer requires the actual to be a default -rectangular. As tensor-workaround.chpl and the codes in the issue -show, this isn't the case with user-defined initializer as well. - -Once the compiler-generated initializer is similarly generic, this -test should work again. Once it does, we should remove the -tensor-workaround.chpl variation of it. We should also be able to -remove a similar manually-introduced initializer for CandidateDomain -in test/studies/amr/lib/amr/BergerRigoutsosClustering.chpl. - diff --git a/test/users/thom/arrArgIn-promote.bad b/test/users/thom/arrArgIn-promote.bad deleted file mode 100644 index 956c3a9fac2b..000000000000 --- a/test/users/thom/arrArgIn-promote.bad +++ /dev/null @@ -1,4 +0,0 @@ -arrArgIn-promote.chpl:8: error: unresolved call 'writeArr(promoted expression)' -arrArgIn-promote.chpl:1: note: this candidate did not match: writeArr(in X: [] real) -arrArgIn-promote.chpl:8: note: because call actual argument #1 with type iterator -arrArgIn-promote.chpl:1: note: is passed to formal 'in X: []' diff --git a/test/users/thom/arrArgIn-promote.future b/test/users/thom/arrArgIn-promote.future deleted file mode 100644 index 2b1f7cf3d8d9..000000000000 --- a/test/users/thom/arrArgIn-promote.future +++ /dev/null @@ -1,35 +0,0 @@ -semantic: array arguments, promotions, and return values - -This series of 6 tests suggests to me that we may still -have questions in our semantics or implementation about -array arguments, promotions, and return types. In -particular, how should a non-lvalue "array" expression -like A+B be interpreted? I tend to think of it as a -virtual array (over A.domain) with no l-value. For -this reason, I would expect it to be illegal to be -passed to an array argument with blank intent, legal -to pass to an array argument with in intent, and -(my preference, though I realize there may be implementation -challenges) legal to pass to an array argument with -const intent. - -Interestingly, the compiler is sometimes more permissive -with functions that return arrays (e.g., return A+B). -In our "compiler should not insert array temps" thinking, -I would think that this would be equivalent to the above -(and would like to see it implemented by transforming the -function to accept the RHS array, if any, as an optional -argument and doing the assignment at the return statement). -This would similarly make it illegal for the blank intent -(no RHS array to assign to/lvalue for the function return type), -legal for the in intent (use the "copy in temporary array" -as the RHS array), but this might suggest it should be -illegal for the array of const intent (darn... I'm not sure -how to explain this -- it seems desireable). - -Surprisingly, arrArgBlank-function.chpl doesn't seem to -have any problems, making me think we may be inserting -a temporary array that I wouldn't have expected? - -Mostly, this whole exercise confused me more than I was -comfortable with... From 198dfd6904dfb53f711e475252f69b3b5f80a988 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 15:30:25 -0400 Subject: [PATCH 58/63] Add test for issue #16185 --- .../ferguson/semantic-examples/9-issue-16185.chpl | 15 +++++++++++++++ .../ferguson/semantic-examples/9-issue-16185.good | 4 ++++ 2 files changed, 19 insertions(+) create mode 100644 test/arrays/ferguson/semantic-examples/9-issue-16185.chpl create mode 100644 test/arrays/ferguson/semantic-examples/9-issue-16185.good diff --git a/test/arrays/ferguson/semantic-examples/9-issue-16185.chpl b/test/arrays/ferguson/semantic-examples/9-issue-16185.chpl new file mode 100644 index 000000000000..e59b54ff1908 --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-issue-16185.chpl @@ -0,0 +1,15 @@ +proc run() { + var A:[1..10] int; + + foo(A[1..3]); // accepts argument with `in` and sets element 1 + writeln("after foo, A[1] is ", A[1]); // is A[1] still 0? +} + +proc foo(in A) { + writeln("in foo, chpl__isArrayView(A) is ", chpl__isArrayView(A)); + writeln("in foo, A[1] is ", A[1]); + writeln("in foo, setting A[1] = 1"); + A[1] = 1; +} + +run(); diff --git a/test/arrays/ferguson/semantic-examples/9-issue-16185.good b/test/arrays/ferguson/semantic-examples/9-issue-16185.good new file mode 100644 index 000000000000..47f45852fed1 --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-issue-16185.good @@ -0,0 +1,4 @@ +in foo, chpl__isArrayView(A) is false +in foo, A[1] is 0 +in foo, setting A[1] = 1 +after foo, A[1] is 0 From 20f06e714c2ea9a08e2c04526d20850ba109132a Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 15:33:44 -0400 Subject: [PATCH 59/63] Add test from issue #16195 --- .../semantic-examples/9-issue-16195.chpl | 16 ++++++++++++++++ .../semantic-examples/9-issue-16195.good | 6 ++++++ 2 files changed, 22 insertions(+) create mode 100644 test/arrays/ferguson/semantic-examples/9-issue-16195.chpl create mode 100644 test/arrays/ferguson/semantic-examples/9-issue-16195.good diff --git a/test/arrays/ferguson/semantic-examples/9-issue-16195.chpl b/test/arrays/ferguson/semantic-examples/9-issue-16195.chpl new file mode 100644 index 000000000000..31f3898e65ed --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-issue-16195.chpl @@ -0,0 +1,16 @@ +var D = {1..10}; +var A:[D] int; + +proc f(in dom) { + D = {1..3}; + writeln("D is ", D); + writeln("dom is ", dom); +} + +writeln("passing D"); +D = {1..10}; +f(D); + +writeln("passing A.domain"); +D = {1..10}; +f(A.domain); diff --git a/test/arrays/ferguson/semantic-examples/9-issue-16195.good b/test/arrays/ferguson/semantic-examples/9-issue-16195.good new file mode 100644 index 000000000000..a873c1002d97 --- /dev/null +++ b/test/arrays/ferguson/semantic-examples/9-issue-16195.good @@ -0,0 +1,6 @@ +passing D +D is {1..3} +dom is {1..10} +passing A.domain +D is {1..3} +dom is {1..10} From 619d27be3386c7d4fe798b35a0b8d29a13876da8 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 15:43:53 -0400 Subject: [PATCH 60/63] Add variant of issue #16007 using in intent --- test/arrays/ferguson/iterator-to-in-issue-16007.chpl | 7 +++++++ test/arrays/ferguson/iterator-to-in-issue-16007.good | 1 + 2 files changed, 8 insertions(+) create mode 100644 test/arrays/ferguson/iterator-to-in-issue-16007.chpl create mode 100644 test/arrays/ferguson/iterator-to-in-issue-16007.good diff --git a/test/arrays/ferguson/iterator-to-in-issue-16007.chpl b/test/arrays/ferguson/iterator-to-in-issue-16007.chpl new file mode 100644 index 000000000000..9cbd572ac519 --- /dev/null +++ b/test/arrays/ferguson/iterator-to-in-issue-16007.chpl @@ -0,0 +1,7 @@ +proc foo(in a : [] int) { + writeln(a); +} + +var a : [1..10] int = 1; +var b : [1..10] int = 2; +foo(a+b); diff --git a/test/arrays/ferguson/iterator-to-in-issue-16007.good b/test/arrays/ferguson/iterator-to-in-issue-16007.good new file mode 100644 index 000000000000..e5d720454a0c --- /dev/null +++ b/test/arrays/ferguson/iterator-to-in-issue-16007.good @@ -0,0 +1 @@ +3 3 3 3 3 3 3 3 3 3 From 32d4bfdeac1780d0e7ba3a0cc8be12fb26e38848 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Thu, 3 Sep 2020 17:16:27 -0400 Subject: [PATCH 61/63] Remove outdated comments --- compiler/resolution/callDestructors.cpp | 5 ----- modules/internal/ChapelArray.chpl | 1 - 2 files changed, 6 deletions(-) diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp index c0e4140ef17e..69da9d429d3b 100644 --- a/compiler/resolution/callDestructors.cpp +++ b/compiler/resolution/callDestructors.cpp @@ -820,11 +820,6 @@ bool doesValueReturnRequireCopy(Expr* initFrom) { return true; // Past here, it's a value. - // e.g. an array view being copied into an array. - // This copy might be copy-elided later. - //if (getCopyTypeDuringResolution(fromType) != fromType) - // return true; - // Is it the result of a call returning by value? SymExpr* fromSe = toSymExpr(initFrom); if (isCallExprTemporary(fromSe->symbol())) { diff --git a/modules/internal/ChapelArray.chpl b/modules/internal/ChapelArray.chpl index 306c9963dcd4..5486c7b27c9c 100644 --- a/modules/internal/ChapelArray.chpl +++ b/modules/internal/ChapelArray.chpl @@ -5036,7 +5036,6 @@ module ChapelArray { // instances // TODO: can we make this domain constant by passing an argument to the // initializer? - // MPPF: Should this shape be un-view'd? var shape = new _domain(ir._shape_); // Important: ir._shape_ points to a domain class for a domain From 0d30c64f81da61dbf5c939c5c1efdf39fd9a7142 Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Tue, 8 Sep 2020 10:17:13 -0400 Subject: [PATCH 62/63] Respond to review feedback --- compiler/resolution/ResolutionCandidate.cpp | 4 ++-- compiler/resolution/callDestructors.cpp | 13 +++++++++---- compiler/resolution/resolveFunction.cpp | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/compiler/resolution/ResolutionCandidate.cpp b/compiler/resolution/ResolutionCandidate.cpp index a6e3f70a7af5..ab58bd04c911 100644 --- a/compiler/resolution/ResolutionCandidate.cpp +++ b/compiler/resolution/ResolutionCandidate.cpp @@ -466,8 +466,8 @@ void clearCoercibleCache() { actualFormalCoercible.clear(); } -// -// in a way that is appropriate for uses when resolving arguments +// Uses formalSym and actualSym to compute allowCoercion, implicitBang, and +// inOutCopy in a way that is appropriate for uses when resolving arguments static Type* getInstantiationType(Symbol* actual, ArgSymbol* formal, Expr* ctx) { bool allowCoercions = shouldAllowCoercions(actual, formal); diff --git a/compiler/resolution/callDestructors.cpp b/compiler/resolution/callDestructors.cpp index 69da9d429d3b..9b74990559be 100644 --- a/compiler/resolution/callDestructors.cpp +++ b/compiler/resolution/callDestructors.cpp @@ -763,14 +763,18 @@ bool isTemporaryFromNoCopyReturn(Symbol* fromSym) { if (isInitOrReturn(call, lhsSe, initOrCtor)) { if (lhsSe == se) { if (initOrCtor != NULL) { - if (FnSymbol* calledFn = initOrCtor->resolvedOrVirtualFunction()) - if (calledFn->hasFlag(FLAG_NO_COPY_RETURN)) + if (FnSymbol* calledFn = initOrCtor->resolvedOrVirtualFunction()) { + if (calledFn->hasFlag(FLAG_NO_COPY_RETURN)) { anyNoCopyReturn = true; + } + } } else { // Track moving from another temp. - if (SymExpr* rhsSe = toSymExpr(call->get(2))) - if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) + if (SymExpr* rhsSe = toSymExpr(call->get(2))) { + if (isTemporaryFromNoCopyReturn(rhsSe->symbol())) { anyNoCopyReturn = true; + } + } } } } @@ -795,6 +799,7 @@ bool doesCopyInitializationRequireCopy(Expr* initFrom) { // Is it the result of a call returning by value? SymExpr* fromSe = toSymExpr(initFrom); + INT_ASSERT(fromSe != NULL); // assuming normalized AST if (isCallExprTemporary(fromSe->symbol())) { // check for the sub-expression being a function marked with // pragma "no copy return" diff --git a/compiler/resolution/resolveFunction.cpp b/compiler/resolution/resolveFunction.cpp index 5507ba19366a..80a2217c1b2d 100644 --- a/compiler/resolution/resolveFunction.cpp +++ b/compiler/resolution/resolveFunction.cpp @@ -769,13 +769,13 @@ static void insertUnrefForArrayOrTupleReturn(FnSymbol* fn) { isTupleContainingAnyReferences(rhsType); if ((handleArray || handleDomain || handleTuple) && - !isTypeExpr(call->get(2))) { + !isTypeExpr(fromExpr)) { FnSymbol* initCopyFn = getInitCopyDuringResolution(rhsType); INT_ASSERT(initCopyFn); SET_LINENO(call); - Expr* rhs = call->get(2)->remove(); + Expr* rhs = fromExpr->remove(); VarSymbol* tmp = newTemp("copy_ret_tmp", rhsType); CallExpr* initTmp = new CallExpr(PRIM_MOVE, tmp, rhs); Symbol* definedConst = gFalse; From 1b403db69db2fef0c42a3bf6c6fc2825c3e8e14a Mon Sep 17 00:00:00 2001 From: Michael Ferguson Date: Tue, 8 Sep 2020 10:35:44 -0400 Subject: [PATCH 63/63] Add test of domains and if-exprs --- .../ferguson/array-domain-and-ifexprs.chpl | 53 +++++++++++++++++++ .../ferguson/array-domain-and-ifexprs.good | 16 ++++++ 2 files changed, 69 insertions(+) create mode 100644 test/domains/ferguson/array-domain-and-ifexprs.chpl create mode 100644 test/domains/ferguson/array-domain-and-ifexprs.good diff --git a/test/domains/ferguson/array-domain-and-ifexprs.chpl b/test/domains/ferguson/array-domain-and-ifexprs.chpl new file mode 100644 index 000000000000..76687f9db6e4 --- /dev/null +++ b/test/domains/ferguson/array-domain-and-ifexprs.chpl @@ -0,0 +1,53 @@ +config const option = true; +var A:[1..10] int; + +proc makeDomain() { + return {1..2}; +} + +proc test1() { + writeln("test1"); + var D = if option then A.domain else makeDomain(); + writeln("D is ", D); + D = {2..2}; + writeln("D is now ", D); + writeln("A.domain is now ", A.domain); +} +test1(); + +proc test2() { + writeln("test2"); + var D = if option then makeDomain() else A.domain; + writeln("D is ", D); + D = {2..2}; + writeln("D is now ", D); + writeln("A.domain is now ", A.domain); +} +test2(); + +proc retTestA() { + return if option then A.domain else makeDomain(); +} +proc retTestB() { + return if option then makeDomain() else A.domain; +} + +proc test3() { + writeln("test3"); + var D = retTestA(); + writeln("D is ", D); + D = {2..2}; + writeln("D is now ", D); + writeln("A.domain is now ", A.domain); +} +test3(); + +proc test4() { + writeln("test4"); + var D = retTestB(); + writeln("D is ", D); + D = {2..2}; + writeln("D is now ", D); + writeln("A.domain is now ", A.domain); +} +test4(); diff --git a/test/domains/ferguson/array-domain-and-ifexprs.good b/test/domains/ferguson/array-domain-and-ifexprs.good new file mode 100644 index 000000000000..dadfd6552fbd --- /dev/null +++ b/test/domains/ferguson/array-domain-and-ifexprs.good @@ -0,0 +1,16 @@ +test1 +D is {1..10} +D is now {2..2} +A.domain is now {1..10} +test2 +D is {1..2} +D is now {2..2} +A.domain is now {1..10} +test3 +D is {1..10} +D is now {2..2} +A.domain is now {1..10} +test4 +D is {1..2} +D is now {2..2} +A.domain is now {1..10}