You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/30 18:21:46 UTC
[GitHub] [tvm] yelite opened a new pull request, #11971: [TVMScript Unified Printer] Base doc and doc printer with LiteralDoc support
yelite opened a new pull request, #11971:
URL: https://github.com/apache/tvm/pull/11971
This PR addes:
- Doc base class
- DocPrinter base class
- PythonDocPrinter
- LiteralDoc and its support in DocPrinter
Tracking issue: #11912
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911423496
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <string>
+#include <memory>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief DocPrinter is responsible for printing Doc tree into text format
+ * \details This is the base class for translating Doc into string.
+ * Each target language needs to have its subclass of DocPrinter
+ * to define the actual logic of printing Doc.
+ *
+ * \sa Doc
+ */
+class DocPrinter {
+ public:
+ /*!
+ * \brief The constructor of DocPrinter
+ *
+ * \param options the option for printer
+ */
+ explicit DocPrinter(const DocPrinterOptions& options);
+ virtual ~DocPrinter() = default;
+
+ /*!
+ * \brief Append a doc into the final content
+ *
+ * \param doc the Doc to be printed
+ *
+ * \sa GetString
+ */
+ void Append(const Doc& doc);
+
+ /*!
+ * \brief Get the printed string of all Doc appended
+ *
+ * The content of each Doc in the returned string will
+ * appear in the same order as they are appended.
+ *
+ * \sa Append
+ */
+ String GetString() const;
+
+ protected:
+ /*!
+ * \brief Get the printed string
+ *
+ * It will dispatch to the PrintTypedDoc method based on
+ * the actual type of Doc.
+ *
+ * \sa PrintTypedDoc
+ */
+ void PrintDoc(const Doc& doc);
+
+ /*!
+ * \brief Virtual method to print a LiteralDoc
+ */
+ virtual void PrintTypedDoc(const LiteralDoc& doc) = 0;
+
+ using OutputStream = std::ostringstream;
Review Comment:
No need for this definition. Also, use `std::ostream` to be a bit more general
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913984952
##########
python/tvm/script/printer/__init__.py:
##########
@@ -0,0 +1,26 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r912419096
##########
python/tvm/script/printer/__init__.py:
##########
@@ -0,0 +1,26 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
BTW, please also add this folder in `./tests/scripts/task_mypy.sh` so that it will have mypy coverage
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914104180
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
+
+} // namespace
+
+class PythonDocPrinter : public DocPrinter {
+ public:
+ PythonDocPrinter(const DocPrinterOptions& options) : DocPrinter(options) {}
+
+ protected:
+ using DocPrinter::PrintDoc;
+
+ void PrintTypedDoc(const LiteralDoc& doc) final;
+};
+
+void PythonDocPrinter::PrintTypedDoc(const LiteralDoc& doc) {
+ const ObjectRef& value = doc->value;
+ if (!value.defined()) {
+ output_ << "None";
+ } else if (const auto* expr_node = value.as<PrimExprNode>()) {
+ PrintLiteralPrimExpr(GetRef<PrimExpr>(expr_node), output_);
+ } else if (const auto* string_obj = value.as<StringObj>()) {
+ PrintLiteralString(GetRef<String>(string_obj), output_);
+ } else {
+ LOG(FATAL) << "Unsupported literal value type " << value->GetTypeKey();
+ }
+}
+
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options) {
+ return std::make_unique<PythonDocPrinter>(options);
+}
+
+TVM_REGISTER_GLOBAL("script.printer.PrintDocAsPython")
+ .set_body_typed([](Doc doc, int indent_spaces = 4) {
+ PythonDocPrinter printer({.indent_spaces = indent_spaces});
Review Comment:
It's removed now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r915464080
##########
tests/python/unittest/test_tvmscript_printer_python_doc_printer.py:
##########
@@ -0,0 +1,56 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pytest
+
+from tvm.script.printer.doc_printer import to_python_script
+from tvm.script.printer.doc import LiteralDoc
+
+
+def format_script(s: str) -> str:
+ """
+ Remove leading and trailing blank lines, and make the minimum idention 0
+ """
+ s = s.strip("\n")
+ non_empty_lines = [line for line in s.splitlines() if line and not line.isspace()]
+ line_indents = [len(line) - len(line.lstrip(" ")) for line in non_empty_lines]
+ spaces_to_remove = min(line_indents)
+ return "\n".join(line[spaces_to_remove:] for line in s.splitlines())
+
+
+@pytest.mark.parametrize(
+ "doc,expected",
+ [
+ (LiteralDoc(None), "None"),
+ (LiteralDoc(True), "True"),
+ (LiteralDoc(False), "False"),
+ (LiteralDoc("test"), '"test"'),
+ (LiteralDoc(""), '""'),
+ (LiteralDoc('""'), r'"\"\""'),
+ (LiteralDoc("\n\t\\test\r"), r'"\n\t\\test\r"'),
+ # TODO: make the roundatrippable problem caused by utf8
+ pytest.param(LiteralDoc("\x88"), r'"\x88"', marks=pytest.mark.xfail),
Review Comment:
Thanks for the detailed explanation!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911472584
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
Review Comment:
A question on this design:
What's the benefit for us to have a common base class, if:
* There is no shared implementation between subclasses
* There is only one interface that users really care about
compared with using a method that satisfies user needs?
For example, replace it with a method:
```C++
AsPythonDoc(Doc doc, std::ostream& os, DocPrinterOptions options);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913929630
##########
python/tvm/script/printer/doc.py:
##########
@@ -0,0 +1,55 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Doc types for TVMScript Unified Printer"""
+
+import tvm._ffi
+from tvm.runtime import Object
+
+from . import _ffi_api
+
+
+class Doc(Object):
+ """Base class of all Docs"""
+
+
+class ExprDoc(Object):
+ """Base class of all expression Docs"""
+
+
+LITERAL_CONSTRUCTORS = [
+ (str, _ffi_api.LiteralDocStr),
+ (float, _ffi_api.LiteralDocFloat),
+ (int, _ffi_api.LiteralDocInt),
+]
+
+
+@tvm._ffi.register_object("script.printer.LiteralDoc")
+class LiteralDoc(ExprDoc):
+ """Doc that represents literal value"""
+
+ def __init__(self, value):
+ if value is None:
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocNone)
+ return
+
+ for (cls, constructor) in LITERAL_CONSTRUCTORS:
+ if isinstance(value, cls):
+ break
+ else:
+ raise TypeError(f"Unsupported type {type(value)} for LiteralDoc")
+
+ self.__init_handle_by_constructor__(constructor, value)
Review Comment:
Sounds good. Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914155167
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
Review Comment:
me either ;-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911472584
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
Review Comment:
A question on this design:
What's the benefit for us to have a common base class, if:
* There is no shared implementation between subclasses
* There is only one interface that users really care about
compared with using a method that satisfies user needs?
For example, replace it with a method:
```C++
void PrintDocPython(Doc doc, DocPrinterOptions options, std::ostream& os);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911473193
##########
tests/python/unittest/test_tvmscript_printer_doc.py:
##########
@@ -0,0 +1,32 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pytest
+
+from tvm.script.printer.doc import LiteralDoc
+
+
+@pytest.mark.parametrize(
+ "value",
+ [None, "test", 0, 1, -2, 0.0, 1.5, -1.3],
+)
+def test_literal_doc_construction(value):
+ doc = LiteralDoc(value)
+ if isinstance(value, float):
+ # FloatImm isn't unpacked to Python's float automatically
+ assert float(doc.value) == pytest.approx(value)
Review Comment:
Didn't know `pytest.approx` before! Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913923853
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
+
+ static constexpr const char* _type_key = "script.printer.Doc";
+ TVM_DECLARE_BASE_OBJECT_INFO(DocNode, Object);
+
+ public:
+ virtual ~DocNode() = default;
+};
+
+/*!
+ * \brief Reference type of DocNode.
+ *
+ * \sa DocNode
+ */
+class Doc : public ObjectRef {
+ protected:
+ Doc() = default;
+
+ public:
+ virtual ~Doc() = default;
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(Doc, ObjectRef, DocNode);
+};
+
+/*!
+ * \brief The base class of expression doc.
+ *
+ * \sa ExprDoc
+ */
+class ExprDocNode : public DocNode {
+ public:
+ void VisitAttrs(AttrVisitor* v) { DocNode::VisitAttrs(v); }
+
+ static constexpr const char* _type_key = "script.printer.ExprDoc";
+ TVM_DECLARE_BASE_OBJECT_INFO(ExprDocNode, DocNode);
+};
+
+/*!
+ * \brief Reference type of ExprDocNode.
+ *
+ * \sa ExprDocNode
+ */
+class ExprDoc : public Doc {
+ protected:
+ ExprDoc() = default;
+
+ public:
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(ExprDoc, Doc, ExprDocNode);
+};
+
+/*!
+ * \brief Doc that represents literal value.
+ *
+ * \sa LiteralDoc
+ */
+class LiteralDocNode : public ExprDocNode {
+ public:
+ /*!
+ * \brief the internal representation of the literal value.
+ *
+ * The actual type is union of IntImm, FloatImm and String, or a
+ * null ObjectRef.
Review Comment:
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914107121
##########
src/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief DocPrinter is responsible for printing Doc tree into text format
+ * \details This is the base class for translating Doc into string.
+ * Each target language needs to have its subclass of DocPrinter
+ * to define the actual logic of printing Doc.
+ *
+ * \sa Doc
+ */
+class DocPrinter {
+ public:
+ /*!
+ * \brief The constructor of DocPrinter
+ *
+ * \param options the option for printer
+ */
+ explicit DocPrinter(const DocPrinterOptions& options);
+ virtual ~DocPrinter() = default;
+
+ /*!
+ * \brief Append a doc into the final content
+ *
+ * \param doc the Doc to be printed
+ *
+ * \sa GetString
+ */
+ void Append(const Doc& doc);
+
+ /*!
+ * \brief Get the printed string of all Doc appended
+ *
+ * The content of each Doc in the returned string will
+ * appear in the same order as they are appended.
+ *
+ * \sa Append
+ */
+ String GetString() const;
+
+ protected:
+ /*!
+ * \brief Get the printed string
+ *
+ * It will dispatch to the PrintTypedDoc method based on
+ * the actual type of Doc.
+ *
+ * \sa PrintTypedDoc
+ */
+ void PrintDoc(const Doc& doc);
+
+ /*!
+ * \brief Virtual method to print a LiteralDoc
+ */
+ virtual void PrintTypedDoc(const LiteralDoc& doc) = 0;
+
+ /*!
+ * \brief Increase the indent level of any content to be
+ * printed after this call
+ */
+ void IncreaseIndent() { indent_ += options_.indent_spaces; }
+
+ /*!
+ * \brief Decrease the indent level of any content to be
+ * printed after this call
+ */
+ void DecreaseIndent() { indent_ -= options_.indent_spaces; }
+
+ /*!
+ * \brief Add a new line into the output stream
+ *
+ * \sa output_
+ */
+ std::ostream& NewLine() {
+ output_ << "\n";
+ output_ << std::string(indent_, ' ');
+ return output_;
+ }
+
+ /*!
+ * \brief The output stream of printer
+ *
+ * All printed content will be stored in this stream and returned
+ * when GetString is called.
+ *
+ * \sa GetString
+ */
+ std::ostringstream output_;
+
+ private:
+ /*! \brief the printer options */
+ DocPrinterOptions options_;
+
+ /*! \brief the current level of indent */
+ int indent_ = 0;
+};
+
+/*!
+ * \brief Get a doc printer to print Doc into Python code
+ *
+ * \param options the option for printer
+ * \return A pointer to the printer
+ */
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options);
Review Comment:
I removed this function per another comment.
BTW, just returning `DocPrinter` won't work because there won't be any virtual table to find (object is truncated). It needs to be a pointer type (raw pointer, smart pointer or TVM ObjectRef)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914104637
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
Review Comment:
Removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914454330
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for converting Doc into text format
+ */
+struct DocPrintingOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief Convert Doc into Python script.
+ *
+ * \param doc the doc to be converted
+ * \param options the option for printer
+ */
+String DocToPythonScript(Doc doc, DocPrintingOptions options);
Review Comment:
likely we don't need this config struct. Indeed we could have one for the entry point if the config really bloats, but for now, there is no such need.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914900763
##########
tests/python/unittest/test_tvmscript_printer_python_doc_printer.py:
##########
@@ -0,0 +1,56 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pytest
+
+from tvm.script.printer.doc_printer import to_python_script
+from tvm.script.printer.doc import LiteralDoc
+
+
+def format_script(s: str) -> str:
+ """
+ Remove leading and trailing blank lines, and make the minimum idention 0
+ """
+ s = s.strip("\n")
+ non_empty_lines = [line for line in s.splitlines() if line and not line.isspace()]
+ line_indents = [len(line) - len(line.lstrip(" ")) for line in non_empty_lines]
+ spaces_to_remove = min(line_indents)
+ return "\n".join(line[spaces_to_remove:] for line in s.splitlines())
+
+
+@pytest.mark.parametrize(
+ "doc,expected",
+ [
+ (LiteralDoc(None), "None"),
+ (LiteralDoc(True), "True"),
+ (LiteralDoc(False), "False"),
+ (LiteralDoc("test"), '"test"'),
+ (LiteralDoc(""), '""'),
+ (LiteralDoc('""'), r'"\"\""'),
+ (LiteralDoc("\n\t\\test\r"), r'"\n\t\\test\r"'),
+ # TODO: make the roundatrippable problem caused by utf8
+ pytest.param(LiteralDoc("\x88"), r'"\x88"', marks=pytest.mark.xfail),
Review Comment:
There were three xfail tests here. I removed the latter two about float number per our discussion yesterday (we want to handle float number precise printing in the IRDocsifier).
This one (on line 45) is about the utf8 encoding. `LiteralDoc("\x88")` will print `'\xc2\x88'` (the utf8 encoded bytes for "\x88"). This is not roundtrippable.
To fix this, we need to either:
1. Add utf8 decoding to StrEscape
2. Output in the form of `b'\xC2\x88'.encode()'`
I don't think unicode support is important to our use case and it doesn't help us reach feature parity with the old printer at all. So I don't want to solve it for now, but just leave an xfail test as a note
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r912011256
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
Review Comment:
Good question. Let me break this into smaller questions and answer them one by one:
> Is there shared implementation between subclasses?
The diagnostic message will be implemented in the DocPrinter. It's language agnostic and will be shared between subclasses.
> Do we have to create a base class for shared code?
No, the code related to diagnostic message could be encapsulated into a class that follows or have similar interface as `std::ostream`.
> Then what's the benefit of having a base class?
Because the Doc node types are not extensible, we know exactly what types of Doc the DocPrinter needs to handle. Coding this information as a base class makes things more discoverable.
> Does it need to be in the public header?
Not really, unless we want developer to implement DocPrinter in an external library. I don't think it's something we target for in the short term. I can remove DocPrinter from public headers.
> For example, replace it with a method:...
Good suggestion. In the TVMScript printer entry point, it only needs a function like `void PrintDocPython(Doc doc, DocPrinterOptions options, std::ostream& os);`. I will create this interface and hide necessary details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913953185
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
Review Comment:
Updated. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913905417
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
Review Comment:
The comment field is on statement doc instead of the base doc, because there will be at least one line for each statement
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913931142
##########
src/script/printer/doc.cc:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include <tvm/runtime/registry.h>
+#include <tvm/script/printer/doc.h>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+TVM_REGISTER_NODE_TYPE(DocNode);
+TVM_REGISTER_NODE_TYPE(ExprDocNode);
+
+LiteralDoc::LiteralDoc(ObjectRef value) {
+ ObjectPtr<LiteralDocNode> n = make_object<LiteralDocNode>();
+ n->value = value;
+ this->data_ = std::move(n);
+}
+TVM_REGISTER_NODE_TYPE(LiteralDocNode);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911462078
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <string>
+#include <memory>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief DocPrinter is responsible for printing Doc tree into text format
+ * \details This is the base class for translating Doc into string.
+ * Each target language needs to have its subclass of DocPrinter
+ * to define the actual logic of printing Doc.
+ *
+ * \sa Doc
+ */
+class DocPrinter {
+ public:
+ /*!
+ * \brief The constructor of DocPrinter
+ *
+ * \param options the option for printer
+ */
+ explicit DocPrinter(const DocPrinterOptions& options);
+ virtual ~DocPrinter() = default;
+
+ /*!
+ * \brief Append a doc into the final content
+ *
+ * \param doc the Doc to be printed
+ *
+ * \sa GetString
+ */
+ void Append(const Doc& doc);
+
+ /*!
+ * \brief Get the printed string of all Doc appended
+ *
+ * The content of each Doc in the returned string will
+ * appear in the same order as they are appended.
+ *
+ * \sa Append
+ */
+ String GetString() const;
+
+ protected:
+ /*!
+ * \brief Get the printed string
+ *
+ * It will dispatch to the PrintTypedDoc method based on
+ * the actual type of Doc.
+ *
+ * \sa PrintTypedDoc
+ */
+ void PrintDoc(const Doc& doc);
+
+ /*!
+ * \brief Virtual method to print a LiteralDoc
+ */
+ virtual void PrintTypedDoc(const LiteralDoc& doc) = 0;
+
+ using OutputStream = std::ostringstream;
Review Comment:
Sounds good. Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r912381949
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
+
+ static constexpr const char* _type_key = "script.printer.Doc";
+ TVM_DECLARE_BASE_OBJECT_INFO(DocNode, Object);
+
+ public:
+ virtual ~DocNode() = default;
+};
+
+/*!
+ * \brief Reference type of DocNode.
+ *
+ * \sa DocNode
+ */
+class Doc : public ObjectRef {
+ protected:
+ Doc() = default;
+
+ public:
+ virtual ~Doc() = default;
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(Doc, ObjectRef, DocNode);
+};
+
+/*!
+ * \brief The base class of expression doc.
+ *
+ * \sa ExprDoc
+ */
+class ExprDocNode : public DocNode {
+ public:
+ void VisitAttrs(AttrVisitor* v) { DocNode::VisitAttrs(v); }
+
+ static constexpr const char* _type_key = "script.printer.ExprDoc";
+ TVM_DECLARE_BASE_OBJECT_INFO(ExprDocNode, DocNode);
+};
+
+/*!
+ * \brief Reference type of ExprDocNode.
+ *
+ * \sa ExprDocNode
+ */
+class ExprDoc : public Doc {
+ protected:
+ ExprDoc() = default;
+
+ public:
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(ExprDoc, Doc, ExprDocNode);
+};
+
+/*!
+ * \brief Doc that represents literal value.
+ *
+ * \sa LiteralDoc
+ */
+class LiteralDocNode : public ExprDocNode {
+ public:
+ /*!
+ * \brief the internal representation of the literal value.
+ *
+ * The actual type is union of IntImm, FloatImm and String, or a
+ * null ObjectRef.
Review Comment:
nitpick: let's list the types line by line
btw, `bool` is missing
```suggestion
* Possible actual types:
* - IntImm (integer or boolean)
* - FloatImm
* - String
* - null
```
##########
src/script/printer/doc.cc:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#include <tvm/runtime/registry.h>
+#include <tvm/script/printer/doc.h>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+TVM_REGISTER_NODE_TYPE(DocNode);
+TVM_REGISTER_NODE_TYPE(ExprDocNode);
+
+LiteralDoc::LiteralDoc(ObjectRef value) {
+ ObjectPtr<LiteralDocNode> n = make_object<LiteralDocNode>();
+ n->value = value;
+ this->data_ = std::move(n);
+}
+TVM_REGISTER_NODE_TYPE(LiteralDocNode);
Review Comment:
nit: let's just move all the registration logic to the same place:
```suggestion
TVM_REGISTER_NODE_TYPE(ExprDocNode);
LiteralDoc::LiteralDoc(ObjectRef value) {
ObjectPtr<LiteralDocNode> n = make_object<LiteralDocNode>();
n->value = value;
this->data_ = std::move(n);
}
TVM_REGISTER_NODE_TYPE(DocNode);
TVM_REGISTER_NODE_TYPE(LiteralDocNode);
```
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
Review Comment:
nit:
```suggestion
#include <tvm/runtime/data_type.h>
```
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
Review Comment:
In TVM we don't use anonymous namespace very frequently because sometimes linkage could lead to some missing frames/symbols in stacktrace
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
+
+ static constexpr const char* _type_key = "script.printer.Doc";
+ TVM_DECLARE_BASE_OBJECT_INFO(DocNode, Object);
+
+ public:
+ virtual ~DocNode() = default;
+};
+
+/*!
+ * \brief Reference type of DocNode.
+ *
+ * \sa DocNode
+ */
+class Doc : public ObjectRef {
+ protected:
+ Doc() = default;
+
+ public:
+ virtual ~Doc() = default;
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(Doc, ObjectRef, DocNode);
+};
+
+/*!
+ * \brief The base class of expression doc.
+ *
+ * \sa ExprDoc
+ */
+class ExprDocNode : public DocNode {
+ public:
+ void VisitAttrs(AttrVisitor* v) { DocNode::VisitAttrs(v); }
+
+ static constexpr const char* _type_key = "script.printer.ExprDoc";
+ TVM_DECLARE_BASE_OBJECT_INFO(ExprDocNode, DocNode);
+};
+
+/*!
+ * \brief Reference type of ExprDocNode.
+ *
+ * \sa ExprDocNode
+ */
+class ExprDoc : public Doc {
+ protected:
+ ExprDoc() = default;
+
+ public:
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(ExprDoc, Doc, ExprDocNode);
+};
+
+/*!
+ * \brief Doc that represents literal value.
+ *
+ * \sa LiteralDoc
+ */
+class LiteralDocNode : public ExprDocNode {
+ public:
+ /*!
+ * \brief the internal representation of the literal value.
+ *
+ * The actual type is union of IntImm, FloatImm and String, or a
+ * null ObjectRef.
+ */
+ ObjectRef value;
+
+ void VisitAttrs(AttrVisitor* v) {
+ ExprDocNode::VisitAttrs(v);
+ v->Visit("value", &value);
+ }
+
+ static constexpr const char* _type_key = "script.printer.LiteralDoc";
+ TVM_DECLARE_FINAL_OBJECT_INFO(LiteralDocNode, ExprDocNode);
+};
+
+/*!
+ * \brief Reference type of LiteralDocNode.
+ *
+ * \sa LiteralDocNode
+ */
+class LiteralDoc : public ExprDoc {
+ protected:
+ explicit LiteralDoc(ObjectRef value);
+
+ public:
+ /*!
+ * \brief Create a LiteralDoc to represent None/null/empty value.
+ */
+ static LiteralDoc None() { return LiteralDoc(ObjectRef(nullptr)); }
+
+ /*!
+ * \brief Create a LiteralDoc to represent integer.
+ * \param v The integer value.
+ */
+ static LiteralDoc Int(int v) { return LiteralDoc(IntImm(DataType::Int(64), v)); }
Review Comment:
Boolean is missing
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
Review Comment:
See [this line](https://github.com/apache/tvm/blob/2c78daed488ea1c9f75c62587098a0c59519d8f0/src/meta_schedule/database/database_utils.cc#L75) for escape printing
##########
src/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief DocPrinter is responsible for printing Doc tree into text format
+ * \details This is the base class for translating Doc into string.
+ * Each target language needs to have its subclass of DocPrinter
+ * to define the actual logic of printing Doc.
+ *
+ * \sa Doc
+ */
+class DocPrinter {
+ public:
+ /*!
+ * \brief The constructor of DocPrinter
+ *
+ * \param options the option for printer
+ */
+ explicit DocPrinter(const DocPrinterOptions& options);
+ virtual ~DocPrinter() = default;
+
+ /*!
+ * \brief Append a doc into the final content
+ *
+ * \param doc the Doc to be printed
+ *
+ * \sa GetString
+ */
+ void Append(const Doc& doc);
+
+ /*!
+ * \brief Get the printed string of all Doc appended
+ *
+ * The content of each Doc in the returned string will
+ * appear in the same order as they are appended.
+ *
+ * \sa Append
+ */
+ String GetString() const;
+
+ protected:
+ /*!
+ * \brief Get the printed string
+ *
+ * It will dispatch to the PrintTypedDoc method based on
+ * the actual type of Doc.
+ *
+ * \sa PrintTypedDoc
+ */
+ void PrintDoc(const Doc& doc);
+
+ /*!
+ * \brief Virtual method to print a LiteralDoc
+ */
+ virtual void PrintTypedDoc(const LiteralDoc& doc) = 0;
+
+ /*!
+ * \brief Increase the indent level of any content to be
+ * printed after this call
+ */
+ void IncreaseIndent() { indent_ += options_.indent_spaces; }
+
+ /*!
+ * \brief Decrease the indent level of any content to be
+ * printed after this call
+ */
+ void DecreaseIndent() { indent_ -= options_.indent_spaces; }
+
+ /*!
+ * \brief Add a new line into the output stream
+ *
+ * \sa output_
+ */
+ std::ostream& NewLine() {
+ output_ << "\n";
+ output_ << std::string(indent_, ' ');
+ return output_;
+ }
+
+ /*!
+ * \brief The output stream of printer
+ *
+ * All printed content will be stored in this stream and returned
+ * when GetString is called.
+ *
+ * \sa GetString
+ */
+ std::ostringstream output_;
+
+ private:
+ /*! \brief the printer options */
+ DocPrinterOptions options_;
+
+ /*! \brief the current level of indent */
+ int indent_ = 0;
+};
+
+/*!
+ * \brief Get a doc printer to print Doc into Python code
+ *
+ * \param options the option for printer
+ * \return A pointer to the printer
+ */
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options);
Review Comment:
No need to return `std::unique_ptr` - RVO is guaranteed.
```suggestion
DocPrinter GetPythonDocPrinter(const DocPrinterOptions& options);
```
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
Review Comment:
quick question: do we have the comment field for the base doc so that it could print things like:
```c++
int x; # this is doc
```
##########
python/tvm/script/printer/doc.py:
##########
@@ -0,0 +1,55 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Doc types for TVMScript Unified Printer"""
+
+import tvm._ffi
+from tvm.runtime import Object
+
+from . import _ffi_api
+
+
+class Doc(Object):
+ """Base class of all Docs"""
+
+
+class ExprDoc(Object):
+ """Base class of all expression Docs"""
+
+
+LITERAL_CONSTRUCTORS = [
+ (str, _ffi_api.LiteralDocStr),
+ (float, _ffi_api.LiteralDocFloat),
+ (int, _ffi_api.LiteralDocInt),
+]
+
+
+@tvm._ffi.register_object("script.printer.LiteralDoc")
+class LiteralDoc(ExprDoc):
+ """Doc that represents literal value"""
+
+ def __init__(self, value):
+ if value is None:
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocNone)
+ return
+
+ for (cls, constructor) in LITERAL_CONSTRUCTORS:
+ if isinstance(value, cls):
+ break
+ else:
+ raise TypeError(f"Unsupported type {type(value)} for LiteralDoc")
+
+ self.__init_handle_by_constructor__(constructor, value)
Review Comment:
In general, we prefer fewer indirections (even if sometimes it doesn't look the smartest):
```suggestion
@tvm._ffi.register_object("script.printer.LiteralDoc")
class LiteralDoc(ExprDoc):
"""Doc that represents literal value"""
def __init__(self, value):
if value is None:
constructor = _ffi_api.LiteralDocNone
elif isinstance(value, str):
constructor = _ffi_api.LiteralDocStr
elif isinstance(value, float):
constructor = _ffi_api.LiteralDocFloat
elif isinstance(value, int):
constructor = _ffi_api.LiteralDocInt
else:
raise TypeError(f"Unsupported type for LiteralDoc: {type(value)}")
self.__init_handle_by_constructor__(constructor, value)
```
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
+
+} // namespace
+
+class PythonDocPrinter : public DocPrinter {
+ public:
+ PythonDocPrinter(const DocPrinterOptions& options) : DocPrinter(options) {}
+
+ protected:
+ using DocPrinter::PrintDoc;
+
+ void PrintTypedDoc(const LiteralDoc& doc) final;
+};
+
+void PythonDocPrinter::PrintTypedDoc(const LiteralDoc& doc) {
+ const ObjectRef& value = doc->value;
+ if (!value.defined()) {
+ output_ << "None";
+ } else if (const auto* expr_node = value.as<PrimExprNode>()) {
+ PrintLiteralPrimExpr(GetRef<PrimExpr>(expr_node), output_);
+ } else if (const auto* string_obj = value.as<StringObj>()) {
+ PrintLiteralString(GetRef<String>(string_obj), output_);
+ } else {
+ LOG(FATAL) << "Unsupported literal value type " << value->GetTypeKey();
+ }
+}
+
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options) {
+ return std::make_unique<PythonDocPrinter>(options);
+}
+
+TVM_REGISTER_GLOBAL("script.printer.PrintDocAsPython")
+ .set_body_typed([](Doc doc, int indent_spaces = 4) {
+ PythonDocPrinter printer({.indent_spaces = indent_spaces});
Review Comment:
Designated initializers are not part of the C++ 11/14 standard IIRC, so let's avoid using it
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
Review Comment:
The logic here is a bit fragmented. AFAICT, all those logic is only used in the following method:
```C++
void PrintTypedDoc(const LiteralDoc& doc) final;
```
While some of the type checking is conducted in `PrintTypedDoc`, the others are done in separate places, which makes the logic less coherent with too many lines of code.
Alternatively I would prefer we merge all of those into `PrintTypeDoc` and the logic can be as succinct as:
```C++
void PythonDocPrinter::PrintTypedDoc(const LiteralDoc& doc) {
const ObjectRef& value = doc->value;
if (!value.defined()) {
output_ << "None";
} else if (const auto* int_imm = value.as<IntImmNode>()) {
if (int_imm->dtype.is_bool()) {
output_ << (int_imm->value ? "True" : "False");
} else {
output_ << int_imm->value;
}
} else if (const auto* float_imm = value.as<FloatImmNode>()) {
output_.precision(17);
output_ << float_imm->value;
} else if (const auto* string_obj = value.as<StringObj>()) {
// TODO: handle escape
// See [this line](https://github.com/apache/tvm/blob/2c78daed488ea1c9f75c62587098a0c59519d8f0/src/meta_schedule/database/database_utils.cc#L75) for escape printing
output_ << "\"" << string_obj->data << "\"";
} else {
LOG(FATAL) << "TypeError: Unsupported literal value type: " << value->GetTypeKey();
}
}
```
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
+
+} // namespace
+
+class PythonDocPrinter : public DocPrinter {
+ public:
+ PythonDocPrinter(const DocPrinterOptions& options) : DocPrinter(options) {}
+
+ protected:
+ using DocPrinter::PrintDoc;
+
+ void PrintTypedDoc(const LiteralDoc& doc) final;
+};
+
+void PythonDocPrinter::PrintTypedDoc(const LiteralDoc& doc) {
+ const ObjectRef& value = doc->value;
+ if (!value.defined()) {
+ output_ << "None";
+ } else if (const auto* expr_node = value.as<PrimExprNode>()) {
+ PrintLiteralPrimExpr(GetRef<PrimExpr>(expr_node), output_);
+ } else if (const auto* string_obj = value.as<StringObj>()) {
+ PrintLiteralString(GetRef<String>(string_obj), output_);
+ } else {
+ LOG(FATAL) << "Unsupported literal value type " << value->GetTypeKey();
+ }
+}
+
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options) {
+ return std::make_unique<PythonDocPrinter>(options);
+}
+
+TVM_REGISTER_GLOBAL("script.printer.PrintDocAsPython")
Review Comment:
Let's think about API consistency between python and c++: if a public API exists in c++, then it should exist also in python, and vice versa. This is a strict standard in non-legacy TVM codebase (e.g. TIR scheduling, MetaSchedule)
In our particular case, I would propose:
- renaming from the `PrintDocAsPython`, use `DocToScriptPython` as the public C++ / python API
- `GetPythonDocPrinter` is Relatively not super useful (only used by the entry function of TVMScript printer) and there is no actual need to expose it in any header file (correct me if I was wrong)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914453925
##########
tests/python/unittest/test_tvmscript_printer_python_doc_printer.py:
##########
@@ -0,0 +1,56 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import pytest
+
+from tvm.script.printer.doc_printer import to_python_script
+from tvm.script.printer.doc import LiteralDoc
+
+
+def format_script(s: str) -> str:
+ """
+ Remove leading and trailing blank lines, and make the minimum idention 0
+ """
+ s = s.strip("\n")
+ non_empty_lines = [line for line in s.splitlines() if line and not line.isspace()]
+ line_indents = [len(line) - len(line.lstrip(" ")) for line in non_empty_lines]
+ spaces_to_remove = min(line_indents)
+ return "\n".join(line[spaces_to_remove:] for line in s.splitlines())
+
+
+@pytest.mark.parametrize(
+ "doc,expected",
+ [
+ (LiteralDoc(None), "None"),
+ (LiteralDoc(True), "True"),
+ (LiteralDoc(False), "False"),
+ (LiteralDoc("test"), '"test"'),
+ (LiteralDoc(""), '""'),
+ (LiteralDoc('""'), r'"\"\""'),
+ (LiteralDoc("\n\t\\test\r"), r'"\n\t\\test\r"'),
+ # TODO: make the roundatrippable problem caused by utf8
+ pytest.param(LiteralDoc("\x88"), r'"\x88"', marks=pytest.mark.xfail),
Review Comment:
quick question: why are these marked as xfail?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913928945
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
Review Comment:
Fixed. The quote is from auto-insert of clangd. I searched around and didn't find configuration to change the auto insertion style ("" vs <>). Let me know if you know there is an option for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r913984623
##########
include/tvm/script/printer/doc.h:
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_H_
+#define TVM_SCRIPT_PRINTER_DOC_H_
+
+#include <tvm/ir/expr.h>
+#include <tvm/node/node.h>
+
+#include "tvm/runtime/data_type.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief The base class of all Doc.
+ *
+ * Doc is an intermediate representation between IR from TVM
+ * and the TVMScript code.
+ * During printing, IR graph is first translated into Doc tree,
+ * then the Doc tree is translated to the target language in
+ * text format.
+ *
+ * \sa Doc
+ */
+class DocNode : public Object {
+ public:
+ void VisitAttrs(AttrVisitor* v) {}
+
+ static constexpr const char* _type_key = "script.printer.Doc";
+ TVM_DECLARE_BASE_OBJECT_INFO(DocNode, Object);
+
+ public:
+ virtual ~DocNode() = default;
+};
+
+/*!
+ * \brief Reference type of DocNode.
+ *
+ * \sa DocNode
+ */
+class Doc : public ObjectRef {
+ protected:
+ Doc() = default;
+
+ public:
+ virtual ~Doc() = default;
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(Doc, ObjectRef, DocNode);
+};
+
+/*!
+ * \brief The base class of expression doc.
+ *
+ * \sa ExprDoc
+ */
+class ExprDocNode : public DocNode {
+ public:
+ void VisitAttrs(AttrVisitor* v) { DocNode::VisitAttrs(v); }
+
+ static constexpr const char* _type_key = "script.printer.ExprDoc";
+ TVM_DECLARE_BASE_OBJECT_INFO(ExprDocNode, DocNode);
+};
+
+/*!
+ * \brief Reference type of ExprDocNode.
+ *
+ * \sa ExprDocNode
+ */
+class ExprDoc : public Doc {
+ protected:
+ ExprDoc() = default;
+
+ public:
+ TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS(ExprDoc, Doc, ExprDocNode);
+};
+
+/*!
+ * \brief Doc that represents literal value.
+ *
+ * \sa LiteralDoc
+ */
+class LiteralDocNode : public ExprDocNode {
+ public:
+ /*!
+ * \brief the internal representation of the literal value.
+ *
+ * The actual type is union of IntImm, FloatImm and String, or a
+ * null ObjectRef.
+ */
+ ObjectRef value;
+
+ void VisitAttrs(AttrVisitor* v) {
+ ExprDocNode::VisitAttrs(v);
+ v->Visit("value", &value);
+ }
+
+ static constexpr const char* _type_key = "script.printer.LiteralDoc";
+ TVM_DECLARE_FINAL_OBJECT_INFO(LiteralDocNode, ExprDocNode);
+};
+
+/*!
+ * \brief Reference type of LiteralDocNode.
+ *
+ * \sa LiteralDocNode
+ */
+class LiteralDoc : public ExprDoc {
+ protected:
+ explicit LiteralDoc(ObjectRef value);
+
+ public:
+ /*!
+ * \brief Create a LiteralDoc to represent None/null/empty value.
+ */
+ static LiteralDoc None() { return LiteralDoc(ObjectRef(nullptr)); }
+
+ /*!
+ * \brief Create a LiteralDoc to represent integer.
+ * \param v The integer value.
+ */
+ static LiteralDoc Int(int v) { return LiteralDoc(IntImm(DataType::Int(64), v)); }
Review Comment:
Updated
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
Review Comment:
Updated. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914454926
##########
python/tvm/script/printer/doc.py:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Doc types for TVMScript Unified Printer"""
+
+import tvm._ffi
+from tvm.runtime import Object
+
+from . import _ffi_api
+
+
+class Doc(Object):
+ """Base class of all Docs"""
+
+
+class ExprDoc(Object):
+ """Base class of all expression Docs"""
+
+
+@tvm._ffi.register_object("script.printer.LiteralDoc")
+class LiteralDoc(ExprDoc):
+ """Doc that represents literal value"""
+
+ def __init__(self, value):
+ if isinstance(value, str):
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocStr, value) # type: ignore
+ elif isinstance(value, float):
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocFloat, value) # type: ignore
+ elif isinstance(value, bool):
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocBoolean, value) # type: ignore
+ elif isinstance(value, int):
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocInt, value) # type: ignore
+ elif value is None:
+ self.__init_handle_by_constructor__(_ffi_api.LiteralDocNone) # type: ignore
Review Comment:
nit: usually we check nullability in the first place
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] yelite commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
yelite commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r914103796
##########
src/script/printer/python_doc_printer.cc:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/runtime/registry.h>
+
+#include "./doc_printer.h"
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+namespace {
+
+/*!
+ * \brief Print a Python literal string
+ *
+ * \param string the string to be printed
+ * \param out the output stream
+ */
+void PrintLiteralString(const String& string, std::ostringstream& out) {
+ // TODO(yelite): Escape and smart quote (choose ' or " automatically)
+ out << "\"" << string << "\"";
+}
+
+/*!
+ * \brief Print a tvm::ir::PrimExpr as Python literal
+ *
+ * This only supports IntImm and FloatImm with size of 64 bits
+ *
+ * \param expr the PrimExpr to be printed
+ * \param out the output stream
+ */
+void PrintLiteralPrimExpr(const PrimExpr& expr, std::ostringstream& out) {
+ const DataType& dtype = expr->dtype;
+
+ if (dtype == DataType::Int(64)) {
+ out << Downcast<IntImm>(expr)->value;
+ } else if (dtype == DataType::Float(64)) {
+ // TODO(yelite): make the float printing roundtrippable
+ std::ostringstream number_value;
+ number_value.precision(17);
+ number_value << Downcast<FloatImm>(expr)->value;
+ out << number_value.str();
+ } else if (dtype == DataType::Bool()) {
+ out << (Downcast<IntImm>(expr)->value ? "True" : "False");
+ } else {
+ LOG(FATAL) << "Cannot print value with dtype " << dtype << " as literal expression";
+ }
+}
+
+} // namespace
+
+class PythonDocPrinter : public DocPrinter {
+ public:
+ PythonDocPrinter(const DocPrinterOptions& options) : DocPrinter(options) {}
+
+ protected:
+ using DocPrinter::PrintDoc;
+
+ void PrintTypedDoc(const LiteralDoc& doc) final;
+};
+
+void PythonDocPrinter::PrintTypedDoc(const LiteralDoc& doc) {
+ const ObjectRef& value = doc->value;
+ if (!value.defined()) {
+ output_ << "None";
+ } else if (const auto* expr_node = value.as<PrimExprNode>()) {
+ PrintLiteralPrimExpr(GetRef<PrimExpr>(expr_node), output_);
+ } else if (const auto* string_obj = value.as<StringObj>()) {
+ PrintLiteralString(GetRef<String>(string_obj), output_);
+ } else {
+ LOG(FATAL) << "Unsupported literal value type " << value->GetTypeKey();
+ }
+}
+
+std::unique_ptr<DocPrinter> GetPythonDocPrinter(const DocPrinterOptions& options) {
+ return std::make_unique<PythonDocPrinter>(options);
+}
+
+TVM_REGISTER_GLOBAL("script.printer.PrintDocAsPython")
Review Comment:
These suggestions sound good to me. I made the following change:
1. Expose a `DocToPythonScript` function in the public C++/Python API (I swapped 'Python' and 'Script' to make this sound more natural IMO. Let me know if you want to use the original name you suggested in the comment)
2. Hide the base DocPrinter to a private header (as is in the previous version of code, just state here for clarity)
4. Remove `GetPythonDocPrinter`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 merged pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged PR #11971:
URL: https://github.com/apache/tvm/pull/11971
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r911423496
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <string>
+#include <memory>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
+ * \brief DocPrinter is responsible for printing Doc tree into text format
+ * \details This is the base class for translating Doc into string.
+ * Each target language needs to have its subclass of DocPrinter
+ * to define the actual logic of printing Doc.
+ *
+ * \sa Doc
+ */
+class DocPrinter {
+ public:
+ /*!
+ * \brief The constructor of DocPrinter
+ *
+ * \param options the option for printer
+ */
+ explicit DocPrinter(const DocPrinterOptions& options);
+ virtual ~DocPrinter() = default;
+
+ /*!
+ * \brief Append a doc into the final content
+ *
+ * \param doc the Doc to be printed
+ *
+ * \sa GetString
+ */
+ void Append(const Doc& doc);
+
+ /*!
+ * \brief Get the printed string of all Doc appended
+ *
+ * The content of each Doc in the returned string will
+ * appear in the same order as they are appended.
+ *
+ * \sa Append
+ */
+ String GetString() const;
+
+ protected:
+ /*!
+ * \brief Get the printed string
+ *
+ * It will dispatch to the PrintTypedDoc method based on
+ * the actual type of Doc.
+ *
+ * \sa PrintTypedDoc
+ */
+ void PrintDoc(const Doc& doc);
+
+ /*!
+ * \brief Virtual method to print a LiteralDoc
+ */
+ virtual void PrintTypedDoc(const LiteralDoc& doc) = 0;
+
+ using OutputStream = std::ostringstream;
Review Comment:
No need to add an extra layer of aliasing. Just use `std::ostringstream`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11971: [TVMScript] Doc Base Class & DocPrinter Scaffolding
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on code in PR #11971:
URL: https://github.com/apache/tvm/pull/11971#discussion_r912257540
##########
include/tvm/script/printer/doc_printer.h:
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+#ifndef TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+#define TVM_SCRIPT_PRINTER_DOC_PRINTER_H_
+
+#include <tvm/script/printer/doc.h>
+
+#include <memory>
+#include <ostream>
+#include <string>
+
+namespace tvm {
+namespace script {
+namespace printer {
+
+/*!
+ * \brief Configurable options for DocPrinter
+ *
+ * \sa DocPrinter
+ */
+struct DocPrinterOptions {
+ int indent_spaces = 4;
+};
+
+/*!
Review Comment:
Thanks for your very detailed response!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org