Skip to content

Commit

Permalink
Simplify convert-uast's handling of SymExprs and similar (#25946)
Browse files Browse the repository at this point in the history
This PR makes incremental progress towards having convert-uast handle
more of the fully resolved data.

It does the following:
* instead of using various global variables, use a single `Converter`
for all modules and do fixups at the end. To accomplish this, added a
wrapper class `UastConverter`, which contains a pointer to the real
`Converter` in a way that allows the implementation details of
`Converter` to be private to `convert-uast.cpp`. This involved adjusting
`parseAndConvert.cpp` which calls the functionality in
`convert-uast.cpp`; now it creates a `UastConverter` and passes it to a
few functions, calling methods on it to convert uAst to AST.
* Remove the complicated handling of fixups in `ConvertedSymbolsMap`. A
fixup is needed when the process of converting uAST to AST encounters
something that refers to some uAST that is not converted yet. Previous
to this PR, fixups were processed at the end of whatever syntactic
construct contains both the SymExpr and the target Symbol, but since
this could not be relied on in all cases, there was also a global fixups
table. The problems with this approach are 1) it makes this code
significantly more complicated 2) it can't apply to Types or handle
point-of-instantiation in any reasonable way and 3) it does not offer
significant performance improvement (as measured with
`--dyno-scope-bundled`). So, this PR removes `ConvertedSymbolsMap` and
folds the relevant structures into `Converter` so that there is only one
mapping of how a symbol/type was converted and only one list of fixups.
(There are different fixups lists for different types of things as they
are processed differently; but there is no longer a chain of
`ConvertedSymbolsMap` each containing fixups lists).
* Prepares for type fixups by adding `TemporaryConversionType` which can
be used when a type is known but the type has not yet been converted.
This is similar to `TemporaryConversionSymbol` but for types.
* Adjust `wellknown.cpp` / `wellknown.h` to consider `dtCPointer` and
`dtCPointerConst` to be well-known types that need to be handled in a
manner similar to `string` and `bytes` records. These will have a dummy
`AggregateType` created early in compilation and the details will be
filled in when processing the relevant declarations from the module
code. `dtCPointer` and `dtCPointerConst` are used in `convert-uast.cpp`
when converting C pointer types because the type system in `frontend/`
has its own representation for these apart from these classes in the
modules.
 
For Cray/chapel-private#6261

Reviewed by @DanilaFe - thanks!

- [x] full comm=none testing
  • Loading branch information
mppf authored Sep 24, 2024
2 parents 12b8da9 + 9bb63b3 commit 827e02e
Show file tree
Hide file tree
Showing 11 changed files with 493 additions and 457 deletions.
5 changes: 3 additions & 2 deletions compiler/AST/baseAST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ void printStatistics(const char* pass) {
kTemporaryConversionThunk;
int nSymbol = nModuleSymbol+nVarSymbol+nArgSymbol+nShadowVarSymbol+nTypeSymbol+nFnSymbol+nInterfaceSymbol+nEnumSymbol+nLabelSymbol+nTemporaryConversionSymbol;
int kSymbol = kModuleSymbol+kVarSymbol+kArgSymbol+kShadowVarSymbol+kTypeSymbol+kFnSymbol+kInterfaceSymbol+kEnumSymbol+kLabelSymbol+kTemporaryConversionSymbol;
int nType = nPrimitiveType+nConstrainedType+nEnumType+nAggregateType+nFunctionType+nDecoratedClassType;
int kType = kPrimitiveType+kConstrainedType+kEnumType+kAggregateType+kFunctionType+kDecoratedClassType;
int nType = nPrimitiveType+nConstrainedType+nEnumType+nAggregateType+nFunctionType+nTemporaryConversionType+nDecoratedClassType;
int kType = kPrimitiveType+kConstrainedType+kEnumType+kAggregateType+kFunctionType+kTemporaryConversionType+kDecoratedClassType;

fprintf(stderr, "%7d asts (%6dK) %s\n", nStmt+nExpr+nSymbol+nType, kStmt+kExpr+kSymbol+kType, pass);

Expand Down Expand Up @@ -481,6 +481,7 @@ Type* BaseAST::getWideRefType() {
const char* BaseAST::astTagAsString() const {
switch (astTag) {
case E_TemporaryConversionThunk: return "TemporaryConversionThunk";
case E_TemporaryConversionType: return "TemporaryConversionType";
case E_PrimitiveType: return "PrimitiveType";
case E_ConstrainedType: return "ConstrainedType";
case E_EnumType: return "EnumType";
Expand Down
36 changes: 36 additions & 0 deletions compiler/AST/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "global-ast-vecs.h"

#include "chpl/framework/compiler-configuration.h"
#include "chpl/types/QualifiedType.h"

#include <cmath>

Expand Down Expand Up @@ -1010,6 +1011,41 @@ bool FunctionType::isGeneric() const {
return false;
}

/************************************* | **************************************
* *
* *
* *
************************************** | *************************************/

TemporaryConversionType::TemporaryConversionType(chpl::types::QualifiedType qt)
: Type(E_TemporaryConversionType, nullptr), qt(qt)
{
this->symbol = dtUnknown->symbol;
gTemporaryConversionTypes.add(this);
}

TemporaryConversionType*
TemporaryConversionType::copyInner(SymbolMap* map) {
INT_FATAL(this, "unexpected call to TemporaryConversionType::copyInner");
return nullptr;
}

void TemporaryConversionType::replaceChild(BaseAST* old_ast, BaseAST* new_ast) {
INT_FATAL(this, "Unexpected case in TemporaryConversionType::replaceChild");
}

void TemporaryConversionType::verify() {
Type::verify();
if (astTag != E_TemporaryConversionType) {
INT_FATAL(this, "Bad TemporaryConversionType::astTag");
}
}

void TemporaryConversionType::accept(AstVisitor* visitor) {
INT_FATAL(this, "Unexpected case in TemporaryConversionType::accept");
}


/************************************* | **************************************
* *
* *
Expand Down
25 changes: 15 additions & 10 deletions compiler/AST/wellknown.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ AggregateType* dtString;
AggregateType* dtTaskBundleRecord;
AggregateType* dtTuple;

// these are only used when the dyno resolver is active
AggregateType* dtCPointer;
AggregateType* dtCPointerConst;

Type* dt_c_int;
Type* dt_c_uint;
Type* dt_c_long;
Expand Down Expand Up @@ -186,18 +190,19 @@ struct WellKnownAggregateTypeNeededEarly
// These types are a required part of the compiler/module interface.
static WellKnownAggregateTypeNeededEarly sWellKnownAggregateTypesNeededEarly[]=
{
// name userFacingName global isClass
{ "_bytes", "bytes", &dtBytes, false },
{ "_locale", "locale", &dtLocale, false },
{ "_object", "RootClass", &dtObject, true },
{ "_owned", nullptr, &dtOwned, false },
{ "_range", "range", &dtRange, false },
{ "_shared", nullptr, &dtShared, false },
{ "_string", "string", &dtString, false },
{ "_tuple", nullptr, &dtTuple, false },
// name userFacingName global isClass
{ "_bytes", "bytes", &dtBytes, false },
{ "_locale", "locale", &dtLocale, false },
{ "_object", "RootClass", &dtObject, true },
{ "_owned", nullptr, &dtOwned, false },
{ "_range", "range", &dtRange, false },
{ "_shared", nullptr, &dtShared, false },
{ "_string", "string", &dtString, false },
{ "_tuple", nullptr, &dtTuple, false },
{ "c_ptr", "c_ptr", &dtCPointer, true },
{ "c_ptrConst", "c_ptrConst", &dtCPointerConst, true },
};


struct WellKnownType
{
const char* name;
Expand Down
8 changes: 7 additions & 1 deletion compiler/include/baseAST.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
macro(EnumType) sep \
macro(AggregateType) sep \
macro(FunctionType) sep \
macro(TemporaryConversionType) sep \
macro(DecoratedClassType) sep \
\
macro(ModuleSymbol) sep \
Expand Down Expand Up @@ -100,6 +101,7 @@ class Expr;
class GenRet;
class LcnSymbol;
class Symbol;
class TemporaryConversionType;
class Type;

class BlockStmt;
Expand Down Expand Up @@ -189,7 +191,8 @@ enum AstTag {
E_EnumType,
E_AggregateType,
E_FunctionType,
E_DecoratedClassType
E_TemporaryConversionType,
E_DecoratedClassType,
};

static inline bool isExpr(AstTag tag)
Expand Down Expand Up @@ -360,6 +363,7 @@ def_is_ast(FunctionType)
def_is_ast(ConstrainedType)
def_is_ast(EnumType)
def_is_ast(AggregateType)
def_is_ast(TemporaryConversionType)
def_is_ast(DecoratedClassType)
#undef def_is_ast

Expand Down Expand Up @@ -419,6 +423,7 @@ def_to_ast(FunctionType)
def_to_ast(ConstrainedType)
def_to_ast(EnumType)
def_to_ast(AggregateType)
def_to_ast(TemporaryConversionType)
def_to_ast(DecoratedClassType)
def_to_ast(Type)

Expand Down Expand Up @@ -481,6 +486,7 @@ def_less_ast(PrimitiveType)
def_less_ast(ConstrainedType)
def_less_ast(EnumType)
def_less_ast(AggregateType)
def_less_ast(TemporaryConversionType)
def_less_ast(DecoratedClassType)
def_less_ast(Type)

Expand Down
36 changes: 24 additions & 12 deletions compiler/include/convert-uast.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,35 @@
#include "alist.h"
#include "baseAST.h"
#include "ModuleSymbol.h"

#include "chpl/framework/Context.h"
#include "chpl/framework/ID.h"
#include "chpl/uast/BuilderResult.h"
#include "chpl/uast/Module.h"

// when converting, only convert modules in this set
// TODO: switch to converting a module-at-a-time (including submodules)
extern std::set<chpl::ID> gConvertFilterModuleIds;
class Converter;

class UastConverter {
private:
std::unique_ptr<Converter> converter_;

public:
UastConverter(chpl::Context* context);
~UastConverter();

// these help to know if submodules should be handled.
// when converting, only convert modules that were added to this set.
void clearModulesToConvert();
void addModuleToConvert(chpl::ID id);

ModuleSymbol*
convertToplevelModule(const chpl::uast::Module* mod,
ModTag modTag);

ModuleSymbol*
convertToplevelModule(chpl::Context* context,
const chpl::uast::Module* mod,
ModTag modTag);
// apply fixups to fix SymExprs to refer to Symbols that
// might have been created in a different order.
void postConvertApplyFixups();
};

// apply fixups to fix SymExprs to refer to Symbols that
// might have been created in a different order.
// TODO: in the future, this should be a method on Converter,
// and there should be 1 Converter to convert a module and its dependencies.
void postConvertApplyFixups(chpl::Context* context);

#endif
4 changes: 2 additions & 2 deletions compiler/include/symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -671,8 +671,8 @@ class TemporaryConversionSymbol final : public Symbol {
chpl::ID symId;
const chpl::resolution::TypedFnSignature* sig;

TemporaryConversionSymbol(chpl::ID symId);
TemporaryConversionSymbol(const chpl::resolution::TypedFnSignature* sig);
explicit TemporaryConversionSymbol(chpl::ID symId);
explicit TemporaryConversionSymbol(const chpl::resolution::TypedFnSignature* sig);

void verify() override;
void accept(AstVisitor* visitor) override;
Expand Down
22 changes: 22 additions & 0 deletions compiler/include/type.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,28 @@ class FunctionType final : public Type {
static const char* retTagMnemonicMangled(RetTag tag);
};

/************************************* | **************************************
* *
* *
* *
************************************** | *************************************/


// Similar to TemporaryConversionSymbol and works with it for
// temporarily representing a Type.
class TemporaryConversionType final : public Type {
public:
chpl::types::QualifiedType qt;
explicit TemporaryConversionType(chpl::types::QualifiedType qt);
void verify() override;
void accept(AstVisitor* visitor) override;
DECLARE_COPY(TemporaryConversionType);
TemporaryConversionType* copyInner(SymbolMap* map) override;
void replaceChild(BaseAST* old_ast, BaseAST* new_ast) override;
};



/************************************* | **************************************
* *
* *
Expand Down
4 changes: 4 additions & 0 deletions compiler/include/wellknown.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ extern AggregateType* dtString;
extern AggregateType* dtTaskBundleRecord;
extern AggregateType* dtTuple;

// these are only used when the dyno resolver is active
extern AggregateType* dtCPointer;
extern AggregateType* dtCPointerConst;

extern Type* dt_c_int;
extern Type* dt_c_uint;
extern Type* dt_c_long;
Expand Down
Loading

0 comments on commit 827e02e

Please sign in to comment.