Skip to content

Commit

Permalink
Always prune unused 'uninstantiated' variables before codegen (chapel…
Browse files Browse the repository at this point in the history
…-lang#24491)

Fix nightly test failures running under `-compopts --baseline
-futures-mode 3` (run with `--baseline` and only include futures with
SKIPIFs).

Adjust cleanup before code generation to remove symbols with type
`uninstantiated` that have zero uses. This is required because
currently, we do not run `--dead-code-elimination` when under
`--baseline`.

Note that 5 futures still fail when running a plain-old `--baseline`
paratest. Fixing them is future work.

In the future, perhaps we should consider always running dead code
elimination, and even disabling its command-line flag altogether. This
is because the current code generator expects dead code elimination to
have occurred in order to function properly (as evidenced by this
patch). Currently, we have a bunch of adhoc "code elimination" that
occurs even under `--baseline`, but the algorithm for
`--dead-code-elimination` is comprehensive and requires no additional
developer effort.

TESTING

- [x] `linux64`, `standard`
- [x] `linux64`, `--baseline -futures-mode 3`

Reviewed by @benharsh. Thanks!
  • Loading branch information
dlongnecke-cray authored Feb 27, 2024
2 parents 27ec48c + 9b99379 commit 7d5d14c
Showing 1 changed file with 32 additions and 8 deletions.
40 changes: 32 additions & 8 deletions compiler/AST/astutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1172,19 +1172,43 @@ static void pruneUnusedRefs(Vec<TypeSymbol*>& types)
}
}

static bool shouldRemoveSymBeforeCodeGeneration(Symbol* sym) {
if (!sym || !sym->inTree() || !sym->type) return false;

void cleanupAfterTypeRemoval()
{
// Do not remove symbols with at least one use.
if (sym->firstSymExpr()) return false;

// Do not remove formals or functions.
if (isArgSymbol(sym) || isFnSymbol(sym)) return false;

// Do not remove fields.
if (isTypeSymbol(sym->defPoint->parentSymbol)) return false;

if (sym->type == dtUninstantiated &&
sym != dtUninstantiated->symbol) {
return true;
}
return false;
}

void cleanupAfterTypeRemoval() {
//
// change symbols with dead types to void (important for baseline)
//
forv_Vec(DefExpr, def, gDefExprs) {
if (def->inTree() &&
def->sym->type != NULL &&
isAggregateType(def->sym->type) == true &&
isTypeSymbol(def->sym) == false &&
def->sym->type->symbol->inTree() == false)
def->sym->type = dtNothing;
auto sym = def->sym;
if (!def->inTree() || !sym || !sym->type) continue;

if (!sym->type->symbol->inTree() && isAggregateType(sym->type) &&
!isTypeSymbol(sym)) {
sym->type = dtNothing;
}

// Some types must never reach code generation, as they have no runtime
// representation. E.g., 'uninstantiated' is one of these types.
// Temporaries can be introduced with this type when resolving methods
// over any partially-instantiated type.
if (shouldRemoveSymBeforeCodeGeneration(sym)) def->remove();
}

// Clear out any uses of removed types in Type::substitutionsPostResolve
Expand Down

0 comments on commit 7d5d14c

Please sign in to comment.