Fix def_use/SimplifyDefUse to deal with extern methods properly (#1284)

- Track extern method calls, assuming that any call to any
  non-abstract method might trigger any abstract method implementation
- local_copyprop fixed for the same assumption.
- propagate reads/writes from callee to caller properly.
- @synchronized annotation for abstract methods
- adjust virtual.p4 test to reflect problematic case of non-synchronous function

frontends/common/resolveReferences/resolveReferences.cpp

@@ -404,10 +404,15 @@ void ResolveReferences::postorder(const IR::Type_Method *t) {
 
 bool ResolveReferences::preorder(const IR::Type_Extern *t) {
     refMap->usedName(t->name.name);
-    addToContext(t->typeParameters); return true; }
+    // FIXME -- should the typeParamters be part of the extern's scope?
+    addToContext(t->typeParameters);
+    addToContext(t);
+    return true; }
 
 void ResolveReferences::postorder(const IR::Type_Extern *t) {
-    removeFromContext(t->typeParameters); }
+    removeFromContext(t);
+    removeFromContext(t->typeParameters);
+    }
 
 bool ResolveReferences::preorder(const IR::ParserState *s) {
     refMap->usedName(s->name.name);

frontends/p4/def_use.cpp

@@ -656,26 +656,29 @@ bool ComputeWriteSet::preorder(const IR::MethodCallExpression* expression) {
     }
 
     // Symbolically call some apply methods (actions and tables)
-    const IR::Node* callee = nullptr;
+    std::vector<const IR::IDeclaration *> callee;
     if (mi->is<ActionCall>()) {
         auto action = mi->to<ActionCall>()->action;
-        callee = action;
+        callee.push_back(action);
     } else if (mi->isApply()) {
         auto am = mi->to<ApplyMethod>();
         if (am->isTableApply()) {
             auto table = am->object->to<IR::P4Table>();
-            callee = table;
+            callee.push_back(table);
         }
-    }
-
-    if (callee != nullptr) {
-        LOG3("Analyzing " << dbp(callee));
+    } else if (auto em = mi->to<ExternMethod>()) {
+        // symbolically call all the methods that might be called via this extern method
+        callee = em->mayCall(); }
+    if (!callee.empty()) {
+        LOG3("Analyzing " << DBPrint::Brief << callee << DBPrint::Reset);
         ProgramPoint pt(callingContext, expression);
         ComputeWriteSet cw(this, pt, currentDefinitions);
-        (void)callee->apply(cw);
+        for (auto c : callee)
+            (void)c->getNode()->apply(cw);
         currentDefinitions = cw.currentDefinitions;
         exitDefinitions = exitDefinitions->joinDefinitions(cw.exitDefinitions);
-        LOG3("Definitions after call of " << expression << ": " << currentDefinitions);
+        LOG3("Definitions after call of " << DBPrint::Brief << expression << ": " <<
+             currentDefinitions << DBPrint::Reset);
     }
 
     auto result = LocationSet::empty;
@@ -751,12 +754,15 @@ bool ComputeWriteSet::preorder(const IR::P4Control* control) {
     auto startPoint = ProgramPoint(control);
     enterScope(control->getApplyParameters(), &control->controlLocals, startPoint);
     exitDefinitions = new Definitions();
-    returnedDefinitions = new Definitions();
     for (auto l : control->controlLocals) {
         if (l->is<IR::Declaration_Instance>())
             visit(l);  // process virtual Functions if any
     }
+    returnedDefinitions = new Definitions();
+    BUG_CHECK(controlLocals == nullptr, "Nested controls?");
+    controlLocals = &control->controlLocals;
     visit(control->body);
+    controlLocals = nullptr;;
     auto returned = currentDefinitions->joinDefinitions(returnedDefinitions);
     auto exited = returned->joinDefinitions(exitDefinitions);
     return setDefinitions(exited, control->body);
@@ -932,3 +938,14 @@ bool ComputeWriteSet::preorder(const IR::MethodCallStatement* statement) {
 }
 
 }  // namespace P4
+
+// functions for calling from gdb
+void dump(const P4::StorageLocation *s) { std::cout << *s << std::endl; }
+void dump(const P4::StorageMap *s) { std::cout << *s << std::endl; }
+void dump(const P4::LocationSet *s) { std::cout << *s << std::endl; }
+void dump(const P4::ProgramPoint *p) { std::cout << *p << std::endl; }
+void dump(const P4::ProgramPoint &p) { std::cout << p << std::endl; }
+void dump(const P4::ProgramPoints *p) { std::cout << *p << std::endl; }
+void dump(const P4::ProgramPoints &p) { std::cout << p << std::endl; }
+void dump(const P4::Definitions *d) { std::cout << *d << std::endl; }
+void dump(const P4::AllDefinitions *d) { std::cout << *d << std::endl; }

frontends/p4/def_use.h

@@ -238,6 +238,12 @@ class StorageMap {
         auto result = ::get(storage, decl);
         return result;
     }
+    virtual void dbprint(std::ostream& out) const {
+        for (auto &it : storage)
+            out << it.first << ": " << it.second << std::endl;
+        if (retVal)
+            out << "retVal: " << retVal << std::endl;
+    }
 };
 
 /// Indicates a statement in the program.
@@ -276,6 +282,8 @@ class ProgramPoint : public IHasDbPrint {
     { return stack.empty() ? nullptr : stack.back(); }
     bool isBeforeStart() const
     { return stack.empty(); }
+    std::vector<const IR::Node*>::const_iterator begin() const { return stack.begin(); }
+    std::vector<const IR::Node*>::const_iterator end() const { return stack.end(); }
 };
 }  // namespace P4
 
@@ -408,14 +416,16 @@ class ComputeWriteSet : public Inspector {
     bool                lhs;
     /// For each expression the location set it writes
     std::map<const IR::Expression*, const LocationSet*> writes;
+    const IR::IndexedVector<IR::Declaration>    *controlLocals = nullptr;
 
     /// Creates new visitor, but with same underlying data structures.
     /// Needed to visit some program fragments repeatedly.
     ComputeWriteSet(const ComputeWriteSet* source, ProgramPoint context, Definitions* definitions) :
             allDefinitions(source->allDefinitions), currentDefinitions(definitions),
             returnedDefinitions(nullptr), exitDefinitions(source->exitDefinitions),
-            callingContext(context), storageMap(source->storageMap), lhs(false) {
-        setName("ComputeWriteSet");
+            callingContext(context), storageMap(source->storageMap), lhs(false),
+            controlLocals(source->controlLocals) {
+        visitDagOnce = false;
     }
     void enterScope(const IR::ParameterList* parameters,
                     const IR::IndexedVector<IR::Declaration>* locals,
@@ -438,9 +448,9 @@ class ComputeWriteSet : public Inspector {
  public:
     explicit ComputeWriteSet(AllDefinitions* allDefinitions) :
             allDefinitions(allDefinitions), currentDefinitions(nullptr),
-            returnedDefinitions(nullptr), exitDefinitions(nullptr),
+            returnedDefinitions(nullptr), exitDefinitions(new Definitions()),
             storageMap(allDefinitions->storageMap), lhs(false)
-    { CHECK_NULL(allDefinitions); setName("ComputeWriteSet"); }
+    { CHECK_NULL(allDefinitions); visitDagOnce = false; }
 
     // expressions
     bool preorder(const IR::Literal* expression) override;

frontends/p4/methodInstance.cpp

@@ -194,4 +194,29 @@ Instantiation* Instantiation::resolve(const IR::Declaration_Instance* instance,
     return nullptr;  // unreachable
 }
 
+std::vector<const IR::IDeclaration *> ExternMethod::mayCall() const {
+    std::vector<const IR::IDeclaration *> rv;
+    auto *di = object->to<IR::Declaration_Instance>();
+    if (!di || !di->initializer) {
+        rv.push_back(method);
+    } else if (auto *em_decl = di->initializer->components
+                                .getDeclaration<IR::IDeclaration>(method->name)) {
+        rv.push_back(em_decl);
+    } else {
+        for (auto meth : originalExternType->methods) {
+            auto sync = meth->getAnnotation(IR::Annotation::synchronousAnnotation);
+            if (!sync) continue;
+            for (auto m : sync->expr) {
+                auto mname = m->to<IR::PathExpression>();
+                if (!mname ||  method->name != mname->path->name)
+                    continue;
+                if (auto *am = di->initializer->components
+                                .getDeclaration<IR::IDeclaration>(meth->name)) {
+                    rv.push_back(am);
+                } else if (!meth->getAnnotation(IR::Annotation::optionalAnnotation)) {
+                    error("No implementation for abstract %s in %s called via %s",
+                          meth, di, method); } } } }
+    return rv;
+}
+
 }  // namespace P4

frontends/p4/methodInstance.h

@@ -129,6 +129,11 @@ class ExternMethod final : public MethodInstance {
     const IR::Method*      method;
     const IR::Type_Extern* originalExternType;    // type of object method is applied to
     const IR::Type_Extern* actualExternType;      // with type variables substituted
+
+    /// Set of IR::Method and IR::Function objects that may be called by this method.
+    // If this method is abstract, will consist of (just) the concrete implementation,
+    // otherwise will consist of those methods that are @synchronous with this
+    std::vector<const IR::IDeclaration *> mayCall() const;
 };
 
 /** Represents the call of an extern function */

frontends/p4/parseAnnotations.cpp

@@ -36,7 +36,10 @@ ParseAnnotations::HandlerMap ParseAnnotations::standardHandlers() {
             PARSE(IR::Annotation::lengthAnnotation, Expression),
 
             // @pkginfo has a key-value list argument.
-            PARSE_KV_LIST(IR::Annotation::pkginfoAnnotation)
+            PARSE_KV_LIST(IR::Annotation::pkginfoAnnotation),
+
+            // @synchronous has a list of method names
+            PARSE_EXPRESSION_LIST(IR::Annotation::synchronousAnnotation)
         };
 }