From d8cdb61e030507fe412eff2c859de8af743da65b Mon Sep 17 00:00:00 2001 From: driazati <9407960+driazati@users.noreply.github.com> Date: Mon, 11 Apr 2022 10:27:49 -0700 Subject: [PATCH] [ci] Don't diff when running clang-format (#10933) * [ci] Don't diff when running clang-format This takes about 15-20 extra seconds but has the benefit of allowing users to replicate and fix clang format issues locally with ease. * format files * Add --fix flag * Comments Co-authored-by: driazati --- .../template_project/src/host_driven/main.c | 1 - include/tvm/relay/attrs/annotation.h | 1 - src/ir/transform.cc | 1 - src/relay/op/tensor/unary.cc | 1 - src/relay/qnn/op/convolution_transpose.cc | 2 +- src/relay/qnn/op/dense.cc | 4 +- src/runtime/hexagon/hexagon/hexagon_common.cc | 1 - src/target/source/codegen_c.h | 56 ++++++++--------- src/te/schedule/schedule_ops.cc | 2 +- src/tir/transforms/make_unpacked_api.cc | 1 - .../transforms/tensorcore_infer_fragment.cc | 4 +- tests/lint/clang_format.sh | 24 -------- tests/lint/git-black.sh | 2 +- tests/lint/git-clang-format.sh | 60 ++++++++++++------- tests/scripts/ci.py | 10 +++- tests/scripts/task_lint.sh | 2 +- 16 files changed, 82 insertions(+), 90 deletions(-) delete mode 100755 tests/lint/clang_format.sh diff --git a/apps/microtvm/zephyr/template_project/src/host_driven/main.c b/apps/microtvm/zephyr/template_project/src/host_driven/main.c index 44d656028cbc3..61dc66efc3084 100644 --- a/apps/microtvm/zephyr/template_project/src/host_driven/main.c +++ b/apps/microtvm/zephyr/template_project/src/host_driven/main.c @@ -260,7 +260,6 @@ void uart_rx_init(struct ring_buf* rbuf, const struct device* dev) { // The main function of this application. extern void __stdout_hook_install(int (*hook)(int)); void main(void) { - #ifdef CONFIG_LED int ret; led0_pin = device_get_binding(LED0); diff --git a/include/tvm/relay/attrs/annotation.h b/include/tvm/relay/attrs/annotation.h index 79889ce9a7909..1066416838b5b 100644 --- a/include/tvm/relay/attrs/annotation.h +++ b/include/tvm/relay/attrs/annotation.h @@ -54,7 +54,6 @@ struct CompilerAttrs : public tvm::AttrsNode { } }; - } // namespace relay } // namespace tvm #endif // TVM_RELAY_ATTRS_ANNOTATION_H_ diff --git a/src/ir/transform.cc b/src/ir/transform.cc index 53c24bdf0adf0..dfd307d715aea 100644 --- a/src/ir/transform.cc +++ b/src/ir/transform.cc @@ -318,7 +318,6 @@ class ModulePass : public Pass { TVM_DEFINE_OBJECT_REF_METHODS(ModulePass, Pass, ModulePassNode); }; - PassInfo::PassInfo(int opt_level, String name, tvm::Array required) { auto pass_info = make_object(); pass_info->opt_level = opt_level; diff --git a/src/relay/op/tensor/unary.cc b/src/relay/op/tensor/unary.cc index 938efe230a408..c6d149846e567 100644 --- a/src/relay/op/tensor/unary.cc +++ b/src/relay/op/tensor/unary.cc @@ -419,7 +419,6 @@ RELAY_REGISTER_UNARY_OP("bitwise_not") .set_support_level(4) .set_attr("FTVMCompute", RELAY_UNARY_COMPUTE(topi::bitwise_not)); - Array ShapeOfCompute(const Attrs& attrs, const Array& inputs, const Type& out_type) { ICHECK_EQ(inputs.size(), 1); diff --git a/src/relay/qnn/op/convolution_transpose.cc b/src/relay/qnn/op/convolution_transpose.cc index 2b4ec4fd5d564..9710d1fd7ae5b 100644 --- a/src/relay/qnn/op/convolution_transpose.cc +++ b/src/relay/qnn/op/convolution_transpose.cc @@ -107,7 +107,7 @@ bool QnnConv2DTransposeRel(const Array& types, int num_inputs, const Attrs return false; } } - ICHECK(IsScalarType(types[2], DataType::Int(32))); // input_zero_point + ICHECK(IsScalarType(types[2], DataType::Int(32))); // input_zero_point const auto* weight_zp_type = types[3].as(); ICHECK(weight_zp_type->dtype == DataType::Int(32)); // weight_zero_point diff --git a/src/relay/qnn/op/dense.cc b/src/relay/qnn/op/dense.cc index 7b733d4777ec5..adaf509e7daf9 100644 --- a/src/relay/qnn/op/dense.cc +++ b/src/relay/qnn/op/dense.cc @@ -60,8 +60,8 @@ bool QnnDenseRel(const Array& types, int num_inputs, const Attrs& attrs, return false; } } - ICHECK(IsScalarType(types[2], DataType::Int(32))); // input_zero_point - ICHECK(IsScalarType(types[4], DataType::Float(32))); // input_scale + ICHECK(IsScalarType(types[2], DataType::Int(32))); // input_zero_point + ICHECK(IsScalarType(types[4], DataType::Float(32))); // input_scale // weight_zero_point can be a scalar or a vector of the same shape as the weight_scale AssignType(types[5], DataType::Float(32), param->units, reporter); // weight_scale diff --git a/src/runtime/hexagon/hexagon/hexagon_common.cc b/src/runtime/hexagon/hexagon/hexagon_common.cc index 9aee341d64b88..f7bd4ffda7aa4 100644 --- a/src/runtime/hexagon/hexagon/hexagon_common.cc +++ b/src/runtime/hexagon/hexagon/hexagon_common.cc @@ -47,7 +47,6 @@ namespace tvm { namespace runtime { namespace hexagon { - #if defined(__hexagon__) class HexagonTimerNode : public TimerNode { public: diff --git a/src/target/source/codegen_c.h b/src/target/source/codegen_c.h index 4f671950260ef..696ec62c5870e 100644 --- a/src/target/source/codegen_c.h +++ b/src/target/source/codegen_c.h @@ -124,35 +124,35 @@ class CodeGenC : public ExprFunctor, */ virtual void InitFuncState(const PrimFunc& f); // expression - void VisitExpr_(const VarNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const LoadNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const VarNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const LoadNode* op, std::ostream& os) override; // NOLINT(*) void VisitExpr_(const BufferLoadNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const LetNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const CallNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const AddNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const SubNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const MulNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const DivNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const ModNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const MinNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const MaxNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const EQNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const NENode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const LTNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const LENode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const GTNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const GENode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const AndNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const OrNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const CastNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const NotNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const SelectNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const RampNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const ShuffleNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const BroadcastNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const IntImmNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const FloatImmNode* op, std::ostream& os) override; // NOLINT(*) - void VisitExpr_(const StringImmNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const LetNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const CallNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const AddNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const SubNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const MulNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const DivNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const ModNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const MinNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const MaxNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const EQNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const NENode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const LTNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const LENode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const GTNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const GENode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const AndNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const OrNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const CastNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const NotNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const SelectNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const RampNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const ShuffleNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const BroadcastNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const IntImmNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const FloatImmNode* op, std::ostream& os) override; // NOLINT(*) + void VisitExpr_(const StringImmNode* op, std::ostream& os) override; // NOLINT(*) // statment void VisitStmt_(const LetStmtNode* op) override; void VisitStmt_(const StoreNode* op) override; diff --git a/src/te/schedule/schedule_ops.cc b/src/te/schedule/schedule_ops.cc index 75736d0333da2..d9818309c2d6b 100644 --- a/src/te/schedule/schedule_ops.cc +++ b/src/te/schedule/schedule_ops.cc @@ -211,7 +211,7 @@ class SchedulePostProc : public StmtExprMutator { } } } else if (op->attr_key == tir::attr::buffer_bind_scope) { - Array tuple = Downcast >(op->node); + Array tuple = Downcast>(op->node); Tensor tensor = Downcast(tuple[1]); auto it = replace_op_.find(tensor->op.get()); if (it != replace_op_.end()) { diff --git a/src/tir/transforms/make_unpacked_api.cc b/src/tir/transforms/make_unpacked_api.cc index fc43e1449d6a4..c57daeabbe1d2 100644 --- a/src/tir/transforms/make_unpacked_api.cc +++ b/src/tir/transforms/make_unpacked_api.cc @@ -57,7 +57,6 @@ PrimFunc MakeUnpackedAPI(PrimFunc&& func) { const Stmt nop = Evaluate(0); std::vector device_init; - // Collect variables and buffers to map between Array args; Map new_buffer_map; diff --git a/src/tir/transforms/tensorcore_infer_fragment.cc b/src/tir/transforms/tensorcore_infer_fragment.cc index 89b9307198f6a..b050193076466 100644 --- a/src/tir/transforms/tensorcore_infer_fragment.cc +++ b/src/tir/transforms/tensorcore_infer_fragment.cc @@ -107,9 +107,7 @@ class FragmentGetter : public StmtExprVisitor { } // Get memory scope - void VisitStmt_(const AttrStmtNode* op) final { - StmtExprVisitor::VisitStmt_(op); - } + void VisitStmt_(const AttrStmtNode* op) final { StmtExprVisitor::VisitStmt_(op); } // Fragment metadata for all fragments std::unordered_map fragments; diff --git a/tests/lint/clang_format.sh b/tests/lint/clang_format.sh deleted file mode 100755 index f131caff2251c..0000000000000 --- a/tests/lint/clang_format.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env bash -# 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. - -set -e - -# check lastest change, for squash merge into main -./tests/lint/git-clang-format.sh HEAD~1 -# chekc against origin/main for PRs. -./tests/lint/git-clang-format.sh origin/main diff --git a/tests/lint/git-black.sh b/tests/lint/git-black.sh index 48029a43a5b05..647aba9540f89 100755 --- a/tests/lint/git-black.sh +++ b/tests/lint/git-black.sh @@ -17,7 +17,7 @@ # under the License. set -euo pipefail -INPLACE_FORMAT=false +INPLACE_FORMAT=${INPLACE_FORMAT:=false} LINT_ALL_FILES=true REVISION= diff --git a/tests/lint/git-clang-format.sh b/tests/lint/git-clang-format.sh index 8c9e6c5b475ba..829fbaf0a7832 100755 --- a/tests/lint/git-clang-format.sh +++ b/tests/lint/git-clang-format.sh @@ -19,23 +19,35 @@ set -e set -u set -o pipefail -if [[ "$1" == "-i" ]]; then - INPLACE_FORMAT=1 - shift 1 -else - INPLACE_FORMAT=0 -fi -if [[ "$#" -lt 1 ]]; then - echo "Usage: tests/lint/git-clang-format.sh [-i] " - echo "" - echo "Run clang-format on files that changed since " - echo "Examples:" - echo "- Compare last one commit: tests/lint/git-clang-format.sh HEAD~1" - echo "- Compare against upstream/main: tests/lint/git-clang-format.sh upstream/main" - echo "You can also add -i option to do inplace format" - exit 1 -fi +INPLACE_FORMAT=${INPLACE_FORMAT:=false} +LINT_ALL_FILES=true +REVISION=$(git rev-list --max-parents=0 HEAD) + +while (( $# )); do + case "$1" in + -i) + INPLACE_FORMAT=true + shift 1 + ;; + --rev) + LINT_ALL_FILES=false + REVISION=$2 + shift 2 + ;; + *) + echo "Usage: tests/lint/git-clang-format.sh [-i] [--rev ]" + echo "" + echo "Run clang-format on files that changed since or on all files in the repo" + echo "Examples:" + echo "- Compare last one commit: tests/lint/git-clang-format.sh --rev HEAD~1" + echo "- Compare against upstream/main: tests/lint/git-clang-format.sh --rev upstream/main" + echo "The -i will use black to format files in-place instead of checking them." + exit 1 + ;; + esac +done + cleanup() { @@ -58,14 +70,20 @@ fi # Print out specific version ${CLANG_FORMAT} --version -if [[ ${INPLACE_FORMAT} -eq 1 ]]; then - echo "Running inplace git-clang-format against" $1 - git-${CLANG_FORMAT} --extensions h,mm,c,cc --binary=${CLANG_FORMAT} $1 +if [[ "$INPLACE_FORMAT" == "true" ]]; then + echo "Running inplace git-clang-format against $REVISION" + git-${CLANG_FORMAT} --extensions h,mm,c,cc --binary=${CLANG_FORMAT} "$REVISION" exit 0 fi -echo "Running git-clang-format against" $1 -git-${CLANG_FORMAT} --diff --extensions h,mm,c,cc --binary=${CLANG_FORMAT} $1 1> /tmp/$$.clang-format.txt +if [[ "$LINT_ALL_FILES" == "true" ]]; then + echo "Running git-clang-format against all C++ files" + git-${CLANG_FORMAT} --diff --extensions h,mm,c,cc --binary=${CLANG_FORMAT} "$REVISION" 1> /tmp/$$.clang-format.txt +else + echo "Running git-clang-format against $REVISION" + git-${CLANG_FORMAT} --diff --extensions h,mm,c,cc --binary=${CLANG_FORMAT} "$REVISION" 1> /tmp/$$.clang-format.txt +fi + echo "---------clang-format log----------" cat /tmp/$$.clang-format.txt echo "" diff --git a/tests/scripts/ci.py b/tests/scripts/ci.py index 5f2034b190eef..c0ce085ff2153 100755 --- a/tests/scripts/ci.py +++ b/tests/scripts/ci.py @@ -319,18 +319,24 @@ def serve_docs(directory: str = "_docs") -> None: cmd([sys.executable, "-m", "http.server"], cwd=directory_path) -def lint(interactive: bool = False) -> None: +def lint(interactive: bool = False, fix: bool = False) -> None: """ Run CI's Sanity Check step arguments: interactive -- start a shell after running build / test scripts + fix -- where possible (currently black and clang-format) edit files in place with formatting fixes """ + env = {} + if fix: + env["IS_LOCAL"] = "true" + env["INPLACE_FORMAT"] = "true" + docker( name=gen_name(f"ci-lint"), image="ci_lint", scripts=["./tests/scripts/task_lint.sh"], - env={}, + env=env, interactive=interactive, ) diff --git a/tests/scripts/task_lint.sh b/tests/scripts/task_lint.sh index aa648e9f4acc2..11ba773fbf316 100755 --- a/tests/scripts/task_lint.sh +++ b/tests/scripts/task_lint.sh @@ -47,7 +47,7 @@ echo "Linting the C++ code..." tests/lint/cpplint.sh echo "clang-format check..." -tests/lint/clang_format.sh +tests/lint/git-clang-format.sh echo "Rust check..." tests/lint/rust_format.sh