Some fixes in symbol usage

Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
p4lang · Mar 4, 2025 · f08102f · f08102f
1 parent 78295c0
commit f08102f
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 9 deletions.
diff --git a/include/p4mlir/Dialect/P4HIR/P4HIR_Ops.td b/include/p4mlir/Dialect/P4HIR/P4HIR_Ops.td
@@ -687,6 +687,7 @@ def FuncOp : P4HIR_Op<"func", [
 
   let builders = [
     OpBuilder<(ins "llvm::StringRef":$name, "FuncType":$type,
+               CArg<"bool", "false">:$isExternal,
                CArg<"llvm::ArrayRef<mlir::NamedAttribute>", "{}">:$attrs,
                CArg<"llvm::ArrayRef<mlir::DictionaryAttr>", "{}">:$argAttrs)>
   ];
@@ -699,7 +700,7 @@ def FuncOp : P4HIR_Op<"func", [
                               P4HIR::FuncType type,
                               llvm::ArrayRef<mlir::NamedAttribute> attrs = {},
                               llvm::ArrayRef<mlir::DictionaryAttr> argAttrs = {}) {
-      auto op = builder.create<FuncOp>(loc, name, type, attrs, argAttrs);
+      auto op = builder.create<FuncOp>(loc, name, type, false, attrs, argAttrs);
       op.createEntryBlock();
       op.setAction(true);
       return op;

diff --git a/include/p4mlir/Dialect/P4HIR/P4HIR_ParserOps.td b/include/p4mlir/Dialect/P4HIR/P4HIR_ParserOps.td
@@ -10,6 +10,12 @@ def ParserOp : P4HIR_Op<"parser",
   let regions = (region SizedRegion<1>:$body);
   let hasCustomAssemblyFormat = 1;
 
+  let skipDefaultBuilders = 1;
+
+  let builders = [
+    OpBuilder<(ins "llvm::StringRef":$sym_name, "P4HIR::FuncType":$applyType, "P4HIR::CtorType":$ctorType)>
+  ];
+
   let extraClassDeclaration = [{
     mlir::Region *getCallableRegion() { return &getBody(); }
 

diff --git a/lib/Dialect/P4HIR/P4HIR_Ops.cpp b/lib/Dialect/P4HIR/P4HIR_Ops.cpp
@@ -501,15 +501,16 @@ LogicalResult P4HIR::FuncOp::verify() {
 }
 
 void P4HIR::FuncOp::build(OpBuilder &builder, OperationState &result, llvm::StringRef name,
-                          P4HIR::FuncType type, ArrayRef<NamedAttribute> attrs,
+                          P4HIR::FuncType type, bool isExternal, ArrayRef<NamedAttribute> attrs,
                           ArrayRef<DictionaryAttr> argAttrs) {
     result.addRegion();
 
     result.addAttribute(SymbolTable::getSymbolAttrName(), builder.getStringAttr(name));
     result.addAttribute(getFunctionTypeAttrName(result.name), TypeAttr::get(type));
     result.attributes.append(attrs.begin(), attrs.end());
-    // We default to private visibility
-    result.addAttribute(SymbolTable::getVisibilityAttrName(), builder.getStringAttr("private"));
+    // External functions are private, everything else is public
+    result.addAttribute(SymbolTable::getVisibilityAttrName(),
+                        builder.getStringAttr(isExternal ? "private" : "public"));
 
     function_interface_impl::addArgAndResultAttrs(builder, result, argAttrs,
                                                   /*resultAttrs=*/std::nullopt,
@@ -571,8 +572,8 @@ ParseResult P4HIR::FuncOp::parse(OpAsmParser &parser, OperationState &state) {
     if (parser.parseSymbolName(nameAttr, SymbolTable::getSymbolAttrName(), state.attributes))
         return failure();
 
-    // We default to private visibility
-    state.addAttribute(SymbolTable::getVisibilityAttrName(), builder.getStringAttr("private"));
+    // All functions are public
+    state.addAttribute(SymbolTable::getVisibilityAttrName(), builder.getStringAttr("public"));
 
     llvm::SmallVector<OpAsmParser::Argument, 8> arguments;
     llvm::SmallVector<DictionaryAttr, 1> resultAttrs;
@@ -1006,6 +1007,25 @@ void P4HIR::TupleExtractOp::build(OpBuilder &builder, OperationState &odsState,
 // ParserOp
 //===----------------------------------------------------------------------===//
 
+void P4HIR::ParserOp::build(mlir::OpBuilder &builder, mlir::OperationState &result,
+                            llvm::StringRef sym_name, P4HIR::FuncType applyType,
+                            P4HIR::CtorType ctorType) {
+    result.addRegion();
+
+    result.addAttribute(::SymbolTable::getSymbolAttrName(), builder.getStringAttr(sym_name));
+    result.addAttribute(getApplyTypeAttrName(result.name), TypeAttr::get(applyType));
+    result.addAttribute(getCtorTypeAttrName(result.name), TypeAttr::get(ctorType));
+
+    // Parsers are top-level objects with public visibility
+    result.addAttribute(::SymbolTable::getVisibilityAttrName(), builder.getStringAttr("public"));
+
+    // TBD:
+    // function_interface_impl::addArgAndResultAttrs(builder, result, argAttrs,
+    //                                              /*resultAttrs=*/std::nullopt,
+    //                                              getArgAttrsAttrName(result.name),
+    //                                              getResAttrsAttrName(result.name));
+}
+
 void P4HIR::ParserOp::createEntryBlock() {
     assert(empty() && "can only create entry block for empty parser");
     Block &first = getFunctionBody().emplaceBlock();
@@ -1035,7 +1055,7 @@ void P4HIR::ParserOp::print(mlir::OpAsmPrinter &printer) {
     function_interface_impl::printFunctionAttributes(
         printer, *this,
         // These are all omitted since they are custom printed already.
-        {getApplyTypeAttrName(), getCtorTypeAttrName()});
+        {getApplyTypeAttrName(), getCtorTypeAttrName(), ::SymbolTable::getVisibilityAttrName()});
 
     printer << ' ';
     printer.printRegion(getRegion(), /*printEntryBlockArgs=*/false, /*printBlockTerminators=*/true);
@@ -1052,8 +1072,8 @@ mlir::ParseResult P4HIR::ParserOp::parse(mlir::OpAsmParser &parser, mlir::Operat
     if (parser.parseSymbolName(nameAttr, ::SymbolTable::getSymbolAttrName(), result.attributes))
         return mlir::failure();
 
-    // We default to private visibility
-    result.addAttribute(::SymbolTable::getVisibilityAttrName(), builder.getStringAttr("private"));
+    // Parsers are visible from top-level
+    result.addAttribute(::SymbolTable::getVisibilityAttrName(), builder.getStringAttr("public"));
 
     llvm::SmallVector<OpAsmParser::Argument, 8> arguments;
     llvm::SmallVector<DictionaryAttr, 1> resultAttrs;

diff --git a/tools/p4mlir-translate/translate.cpp b/tools/p4mlir-translate/translate.cpp
@@ -1105,6 +1105,7 @@ bool P4HIRConverter::preorder(const P4::IR::Function *f) {
     assert(funcType.getNumInputs() == argAttrs.size() && "invalid parameter conversion");
 
     auto func = builder.create<P4HIR::FuncOp>(getLoc(builder, f), f->name.string_view(), funcType,
+                                              /* isExternal */ false,
                                               llvm::ArrayRef<mlir::NamedAttribute>(), argAttrs);
     func.createEntryBlock();
 
@@ -1147,6 +1148,7 @@ bool P4HIRConverter::preorder(const P4::IR::Method *m) {
     assert(funcType.getNumInputs() == argAttrs.size() && "invalid parameter conversion");
 
     auto func = builder.create<P4HIR::FuncOp>(getLoc(builder, m), m->name.string_view(), funcType,
+                                              /* isExternal */ true,
                                               llvm::ArrayRef<mlir::NamedAttribute>(), argAttrs);
 
     auto [it, inserted] = p4Symbols.try_emplace(m, mlir::SymbolRefAttr::get(func));