Skip to content

Implicit read/write of extern instances #5343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Implicit read/write of extern instances #5343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backends/p4test/midend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ limitations under the License.
#include "midend/flattenUnions.h"
#include "midend/global_copyprop.h"
#include "midend/hsIndexSimplify.h"
#include "midend/implicitReadWrite.h"
#include "midend/local_copyprop.h"
#include "midend/midEndLast.h"
#include "midend/nestedStructs.h"
Expand Down Expand Up @@ -90,6 +91,7 @@ MidEnd::MidEnd(P4TestOptions &options, std::ostream *outStream) {

addPasses(
{new P4::DumpPipe("MidEnd start"),
new P4::ImplicitReadWrite(&typeMap),
options.ndebug ? new P4::RemoveAssertAssume(&typeMap) : nullptr,
new P4::RemoveMiss(&typeMap),
new P4::EliminateNewtype(&typeMap),
Expand Down
30 changes: 29 additions & 1 deletion frontends/p4/simplifyDefUse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,10 @@ class FindUninitialized : public Inspector {
typeMap->setType(src_member, ftype);
processHeadersInAssignment(dst_member, src_member, ftype, ftype);
}
} else if (typeMap->externImplicitReadType(src_type)) {
for (const auto *s : headerDefs->getStorageLocation(dst)) {
headerDefs->setValueToStorage(s, TernaryBool::Yes);
}
} else {
BUG("%1%: unexpected expression on RHS", src);
}
Expand Down Expand Up @@ -1469,6 +1473,30 @@ class RemoveUnused : public Transform {
ReferenceMap *refMap;
TypeMap *typeMap;

// FIXME -- this is very messy -- should be a better way to detect this
// Perhaps via P4::SideEffects
bool isExternType(const IR::Expression *e) {
auto *type = typeMap ? typeMap->getType(e, false) : nullptr;
if (!type) type = e->type;
if (auto *tsc = type->to<IR::Type_SpecializedCanonical>()) type = tsc->substituted;
if (auto *ts = type->to<IR::Type_Specialized>()) type = ts->baseType;
return type->is<IR::Type_Extern>();
}

bool implicitExernAssign(const IR::Expression *exp) {
while (!isExternType(exp)) {
if (auto *sl = exp->to<IR::AbstractSlice>())
exp = sl->e0;
else if (auto *m = exp->to<IR::Member>())
exp = m->expr;
else if (auto *ai = exp->to<IR::ArrayIndex>())
exp = ai->left;
else
return false;
}
return true;
}

public:
explicit RemoveUnused(const HasUses &hasUses, ReferenceMap *refMap, TypeMap *typeMap)
: hasUses(hasUses), refMap(refMap), typeMap(typeMap) {
Expand All @@ -1477,7 +1505,7 @@ class RemoveUnused : public Transform {
setName("RemoveUnused");
}
const IR::Node *postorder(IR::BaseAssignmentStatement *statement) override {
if (!hasUses.hasUses(getOriginal())) {
if (!hasUses.hasUses(getOriginal()) && !implicitExernAssign(statement->left)) {
Log::TempIndent indent;
LOG3("Removing statement " << getOriginal() << " " << statement << indent);
SideEffects se(typeMap);
Expand Down
62 changes: 40 additions & 22 deletions frontends/p4/typeChecking/typeCheckExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1313,19 +1313,29 @@ const IR::Node *TypeInferenceBase::postorder(const IR::PathExpression *expressio
} else if (decl->is<IR::Declaration_Variable>()) {
setLeftValue(expression);
setLeftValue(getOriginal<IR::Expression>());
} else if (decl->is<IR::Parameter>()) {
auto paramDecl = decl->to<IR::Parameter>();
} else if (auto paramDecl = decl->to<IR::Parameter>()) {
if (paramDecl->direction == IR::Direction::InOut ||
paramDecl->direction == IR::Direction::Out) {
setLeftValue(expression);
setLeftValue(getOriginal<IR::Expression>());
} else if (paramDecl->direction == IR::Direction::None) {
setCompileTimeConstant(expression);
setCompileTimeConstant(getOriginal<IR::Expression>());
if (typeMap->externImplicitAssignType(paramDecl->type, true)) {
setLeftValue(expression);
setLeftValue(getOriginal<IR::Expression>());
}
}
} else if (decl->is<IR::Declaration_Constant>() || decl->is<IR::Declaration_Instance>()) {
} else if (decl->is<IR::Declaration_Constant>()) {
setCompileTimeConstant(expression);
setCompileTimeConstant(getOriginal<IR::Expression>());
} else if (auto di = decl->to<IR::Declaration_Instance>()) {
setCompileTimeConstant(expression);
setCompileTimeConstant(getOriginal<IR::Expression>());
if (typeMap->externImplicitAssignType(di->type, true)) {
setLeftValue(expression);
setLeftValue(getOriginal<IR::Expression>());
}
} else if (decl->is<IR::Method>() || decl->is<IR::Function>()) {
type = getType(decl->getNode());
// Each method invocation uses fresh type variables
Expand Down Expand Up @@ -1598,28 +1608,36 @@ const IR::Node *TypeInferenceBase::postorder(const IR::Member *expression) {
if (auto ts = type->to<IR::Type_SpecializedCanonical>()) type = ts->substituted;

if (auto *ext = type->to<IR::Type_Extern>()) {
auto call = findContext<IR::MethodCallExpression>();
if (call == nullptr) {
typeError("%1%: Methods can only be called", expression);
setCompileTimeConstant(expression);
setCompileTimeConstant(getOriginal<IR::Expression>());
if (auto call = getParent<IR::MethodCallExpression>()) {
auto method = ext->lookupMethod(expression->member, call->arguments);
if (method == nullptr) {
typeError("%1%: extern %2% does not have method matching this call", expression,
ext->name);
return expression;
}

const IR::Type *methodType = getType(method);
if (methodType == nullptr) return expression;
// Each method invocation uses fresh type variables
methodType = cloneWithFreshTypeVariables(methodType->to<IR::IMayBeGenericType>());

setType(getOriginal(), methodType);
setType(expression, methodType);
return expression;
}
auto method = ext->lookupMethod(expression->member, call->arguments);
if (method == nullptr) {
typeError("%1%: extern %2% does not have method matching this call", expression,
ext->name);
} else if (auto cvt = typeMap->externImplicitReadType(ext)) {
if (cvt->is<IR::Type_StructLike>()) {
type = cvt;
// assume we'll convert to that and fall through
} else {
typeError("%1%: Methods can only be called", expression);
return expression;
}
} else {
typeError("%1%: Methods can only be called", expression);
return expression;
}

const IR::Type *methodType = getType(method);
if (methodType == nullptr) return expression;
// Each method invocation uses fresh type variables
methodType = cloneWithFreshTypeVariables(methodType->to<IR::IMayBeGenericType>());

setType(getOriginal(), methodType);
setType(expression, methodType);
setCompileTimeConstant(expression);
setCompileTimeConstant(getOriginal<IR::Expression>());
return expression;
}

bool inMethod = getParent<IR::MethodCallExpression>() != nullptr;
Expand Down
5 changes: 5 additions & 0 deletions frontends/p4/typeChecking/typeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ const IR::Type *TypeInferenceBase::specialize(const IR::IMayBeGenericType *type,
}

// May return nullptr if a type error occurs.
// FIXME -- this seems to be broken in that it sometimes creates duplicate struct
// types (new IR::Struct objects tht are identical to the existing type). This will
// break any code that assumens types can be compared by pointer (which much code does)
const IR::Type *TypeInferenceBase::canonicalize(const IR::Type *type) {
if (type == nullptr) return nullptr;

Expand Down Expand Up @@ -548,6 +551,8 @@ const IR::Expression *TypeInferenceBase::assignment(const IR::Node *errorPositio
if (destType->is<IR::Type_Dontcare>()) return sourceExpression;
const IR::Type *initType = getType(sourceExpression);
if (initType == nullptr) return sourceExpression;
if (auto implicit = typeMap->externImplicitAssignType(destType)) destType = implicit;
if (auto implicit = typeMap->externImplicitReadType(initType)) initType = implicit;

auto tvs = unifyCast(errorPosition, destType, initType,
"Source expression '%1%' produces a result of type '%2%' which cannot be "
Expand Down
20 changes: 18 additions & 2 deletions frontends/p4/typeChecking/typeUnification.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ namespace P4 {

using namespace literals;

const IR::Type *unspecialize(const IR::Type *type) {
if (auto tsc = type->to<IR::Type_SpecializedCanonical>()) type = tsc->substituted;
if (auto ts = type->to<IR::Type_Specialized>()) type = ts->baseType;
return type;
}

/// Unifies a call with a prototype.
bool TypeUnification::unifyCall(const BinaryConstraint *constraint) {
// These are canonical types.
Expand Down Expand Up @@ -91,7 +97,8 @@ bool TypeUnification::unifyCall(const BinaryConstraint *constraint) {
return constraint->reportError(
constraints->getCurrentSubstitution(),
"%1%: Read-only value used for out/inout parameter '%2%'", arg->srcInfo, param);
else if (param->direction == IR::Direction::None && !arg->compileTimeConstant)
else if (param->direction == IR::Direction::None && !arg->compileTimeConstant &&
!unspecialize(arg->type)->is<IR::Type_Extern>())
return constraint->reportError(constraints->getCurrentSubstitution(),
"%1%: argument used for directionless parameter '%2%' "
"must be a compile-time constant",
Expand Down Expand Up @@ -416,8 +423,13 @@ bool TypeUnification::unify(const BinaryConstraint *constraint) {
constraints->add(new EqualityConstraint(dotsField->type, partial, constraint));
}
return true;
} else if (auto implicit = typeMap->externImplicitReadType(src)) {
// FIXME -- we're supposed to be able to compare types for equality by
// just comparing pointers, but it seems like typechecking internally
// creates duplicate struct types. This seems like a bug waiting to
// happen, since if any such type leaks, later passes may fail mysteriously
if (implicit->equiv(*dest)) return true;
}

return constraint->reportError(constraints->getCurrentSubstitution());
} else if (dest->is<IR::Type_Base>()) {
if (dest->is<IR::Type_Bits>() && src->is<IR::Type_InfInt>()) {
Expand All @@ -430,6 +442,10 @@ bool TypeUnification::unify(const BinaryConstraint *constraint) {
constraints->add(constraint->create(dest, src));
return true;
}
if (auto implicit = typeMap->externImplicitReadType(src)) {
if (*implicit == *dest) return true;
if (implicit->is<IR::Type_SerEnum>()) src = implicit;
}
if (auto senum = src->to<IR::Type_SerEnum>()) {
if (constraint->is<P4::CanBeImplicitlyCastConstraint>()) {
if (dest->is<IR::Type_Bits>()) {
Expand Down
43 changes: 43 additions & 0 deletions frontends/p4/typeMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ limitations under the License.

namespace P4 {

using namespace literals;

bool TypeMap::typeIsEmpty(const IR::Type *type) const {
if (auto bt = type->to<IR::Type_Bits>()) {
return bt->size == 0;
Expand Down Expand Up @@ -401,4 +403,45 @@ int TypeMap::widthBits(const IR::Type *type, const IR::Node *errorPosition, bool
return -1;
}

const IR::Type *TypeMap::externImplicitAssignType(const IR::Type *type, bool skipIndex) const {
if (auto mappedType = getType(type)) type = mappedType;
if (auto typeType = type->to<IR::Type_Type>()) type = typeType->type;
if (skipIndex)
while (auto arrType = type->to<IR::Type_Array>()) type = arrType->elementType;
if (auto tsc = type->to<IR::Type_SpecializedCanonical>()) type = tsc->substituted;
if (auto ts = type->to<IR::Type_Specialized>()) type = ts->baseType;
if (auto et = type->to<IR::Type_Extern>()) {
for (auto m : et->methods) {
if (m->hasAnnotation("implicit"_cs) && m->type->returnType->is<IR::Type_Void>() &&
m->getParameters()->size() == 1) {
type = m->getParameters()->getParameter(0)->type;
if (auto mappedType = getType(type)) type = mappedType;
if (auto typeType = type->to<IR::Type_Type>()) type = typeType->type;
return type;
}
}
}
return nullptr;
}

const IR::Type *TypeMap::externImplicitReadType(const IR::Type *type, bool skipIndex) const {
if (auto mappedType = getType(type)) type = mappedType;
if (auto typeType = type->to<IR::Type_Type>()) type = typeType->type;
if (skipIndex)
while (auto arrType = type->to<IR::Type_Array>()) type = arrType->elementType;
if (auto tsc = type->to<IR::Type_SpecializedCanonical>()) type = tsc->substituted;
if (auto ts = type->to<IR::Type_Specialized>()) type = ts->baseType;
if (auto et = type->to<IR::Type_Extern>()) {
for (auto m : et->methods) {
if (m->hasAnnotation("implicit"_cs) && m->getParameters()->size() == 0) {
type = m->type->returnType;
if (auto mappedType = getType(type)) type = mappedType;
if (auto typeType = type->to<IR::Type_Type>()) type = typeType->type;
return type;
}
}
}
return nullptr;
}

} // namespace P4
7 changes: 7 additions & 0 deletions frontends/p4/typeMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ class TypeMap final : public ProgramMap {

/// True is type occupies no storage.
bool typeIsEmpty(const IR::Type *type) const;

// helper functions for implicitly assignable/readable externs, needed by typeChecking
// and typeUnification
// FIXME -- we currently only allow one implicit assign and one implicit read per
// extern, as unification can't deal with resolving more than one (overload resolution)
const IR::Type *externImplicitAssignType(const IR::Type *, bool skipIndex = false) const;
const IR::Type *externImplicitReadType(const IR::Type *, bool skipIndex = false) const;
};
} // namespace P4

Expand Down
2 changes: 2 additions & 0 deletions midend/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ set (MIDEND_SRCS
flattenLogMsg.cpp
flattenUnions.cpp
hsIndexSimplify.cpp
implicitReadWrite.cpp
interpreter.cpp
global_copyprop.cpp
local_copyprop.cpp
Expand Down Expand Up @@ -93,6 +94,7 @@ set (MIDEND_HDRS
flattenInterfaceStructs.h
flattenUnions.h
has_side_effects.h
implicitReadWrite.h
interpreter.h
global_copyprop.h
local_copyprop.h
Expand Down
Loading
Loading