fix clang-tidy

.github/workflows/buildAndTest.yml

@@ -139,81 +139,6 @@ jobs:
           ninja check-tutorials
           ninja check-reference-designs
 
-#  lint-repo:
-#
-#    name: Check formatting and lint
-#
-#    runs-on: ubuntu-latest
-#
-#    permissions:
-#      contents: write
-#      pull-requests: write
-#
-#    steps:
-#      - name: Get repo
-#        uses: actions/checkout@v3
-#        with:
-#          fetch-depth: 2
-#          submodules: "true"
-#
-#      - uses: actions/setup-python@v4
-#        with:
-#          python-version: '3.9'
-#
-#      - name: Install Python packages
-#        run: |
-#          pip install cmake numpy psutil pybind11 rich pkginfo lit PyYAML
-#
-#      - name: Install Ninja
-#        run: sudo apt-get install -y ninja-build
-#
-#      - name: Get MLIR
-#        id: mlir-wheels
-#        run: |
-#          pip -q download mlir -f https://github.com/Xilinx/mlir-aie/releases/expanded_assets/mlir-distro && unzip -q mlir-*.whl
-#          echo "MLIR_DIR=$PWD/mlir" | tee -a $GITHUB_OUTPUT
-#
-#      - name: Build for compilation db
-#        run: |
-#          mkdir build_assert
-#          pushd build_assert
-#          cmake .. \
-#            -GNinja \
-#            -DCMAKE_BUILD_TYPE=Release \
-#            -DCMAKE_PLATFORM_NO_VERSIONED_SONAME=ON \
-#            -DCMAKE_VISIBILITY_INLINES_HIDDEN=ON \
-#            -DCMAKE_C_VISIBILITY_PRESET=hidden \
-#            -DCMAKE_CXX_VISIBILITY_PRESET=hidden \
-#            -DAIE_COMPILER=NONE \
-#            -DAIE_LINKER=NONE \
-#            -DHOST_COMPILER=NONE \
-#            -DLLVM_ENABLE_ASSERTIONS=ON \
-#            -DLLVM_ENABLE_RTTI=ON \
-#            -DCMAKE_MODULE_PATH=`pwd`/../cmake/modulesXilinx \
-#            -DMLIR_DIR=${{ steps.mlir-wheels.outputs.MLIR_DIR }}/lib/cmake/mlir \
-#            -DLLVM_DIR=${{ steps.mlir-wheels.outputs.MLIR_DIR }}/lib/cmake/llvm \
-#            -DLLVM_EXTERNAL_LIT=$(which lit) \
-#            -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
-#
-#          ninja aie-headers mlir-headers
-#          popd
-#
-#      - uses: cpp-linter/cpp-linter-action@v2
-#        id: linter
-#        env:
-#          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-#        with:
-#          style: file
-#          database: build_assert
-#          files-changed-only: true
-#          # this is a combined list from LLVM and MLIR
-#          tidy-checks: '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-const-correctness,-misc-unused-parameters,-misc-non-private-member-variables-in-classes,-misc-no-recursion,-misc-use-anonymous-namespace,readability-identifier-naming,-misc-const-correctness,bugprone-argument-comment,bugprone-assert-side-effect,bugprone-branch-clone,bugprone-copy-constructor-init,bugprone-dangling-handle,bugprone-dynamic-static-initializers,bugprone-macro-parentheses,bugprone-macro-repeated-side-effects,bugprone-misplaced-widening-cast,bugprone-move-forwarding-reference,bugprone-multiple-statement-macro,bugprone-suspicious-semicolon,bugprone-swapped-arguments,bugprone-terminating-continue,bugprone-unused-raii,bugprone-unused-return-value,misc-redundant-expression,misc-static-assert,misc-unused-using-decls,modernize-use-bool-literals,modernize-loop-convert,modernize-make-unique,modernize-raw-string-literal,modernize-use-equals-default,modernize-use-default-member-init,modernize-use-emplace,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-no-automatic-move,performance-trivially-destructible,performance-unnecessary-copy-initialization,performance-unnecessary-value-param,readability-avoid-const-params-in-decls,readability-const-return-type,readability-container-size-empty,readability-inconsistent-declaration-parameter-name,readability-misleading-indentation,readability-redundant-control-flow,readability-redundant-smartptr-get,readability-simplify-boolean-expr,readability-simplify-subscript-expr,readability-use-anyofallof'
-#          thread-comments: ${{ github.event_name == 'pull_request' && 'update' }}
-#
-#      - name: Pass/Fail
-#        if: steps.linter.outputs.checks-failed > 0
-#        run: exit 1
-
   clang-tidy:
 
     runs-on: ubuntu-22.04
@@ -229,15 +154,10 @@ jobs:
           fetch-depth: 2
           submodules: "true"
 
-      - name: Fetch base branch
-        run: |
-          git remote add upstream "https://github.com/${{ github.event.pull_request.base.repo.full_name }}"
-          git fetch --no-tags --no-recurse-submodules upstream "${{ github.event.pull_request.base.ref }}"
-
       - name: Install clang-tidy
         run: |
           sudo apt-get update
-          sudo apt-get install -y clang-tidy ninja-build
+          sudo apt-get install -y clang-tidy ninja-build clang
 
       - uses: actions/setup-python@v4
         with:
@@ -260,6 +180,8 @@ jobs:
           cmake .. \
             -GNinja \
             -DCMAKE_BUILD_TYPE=Release \
+            -DCMAKE_C_COMPILER=clang \
+            -DCMAKE_CXX_COMPILER=clang++ \
             -DCMAKE_PLATFORM_NO_VERSIONED_SONAME=ON \
             -DCMAKE_VISIBILITY_INLINES_HIDDEN=ON \
             -DCMAKE_C_VISIBILITY_PRESET=hidden \
@@ -281,22 +203,23 @@ jobs:
 
       - name: Analyze
         run: |
-          git diff -U0 "$(git merge-base HEAD "upstream/${{ github.event.pull_request.base.ref }}")" | clang-tidy-diff -p1 -path build -export-fixes fixes.yml
+          git fetch origin main
+          git diff -U0 origin/main | clang-tidy-diff -p1 -path build -export-fixes fixes.yml
           cat fixes.yml
 
-      - name: Run code formatter
+      - name: Post clang-tidy requests
         env:
           GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }}
         run: |
           PULL_REQUEST_ID="$(jq "if (.issue.number != null) then .issue.number else .number end" < "$GITHUB_EVENT_PATH")"
           echo $PULL_REQUEST_ID
           
-          python utils/git/clang-tidy-pr.py \
+          GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}  python utils/git/clang-tidy-pr.py \
           --clang-tidy-fixes fixes.yml \
           --pull-request-id "$PULL_REQUEST_ID" \
           --repository "$GITHUB_REPOSITORY" \
           --repository-root "$PWD" \
-          --request-changes "true" \
+          --request-changes "false" \
           --suggestions-per-comment 10
 
   code-coverage:
@@ -412,10 +335,16 @@ jobs:
           body-path: /home/runner/work/mlir-aie/mlir-aie/build_release/report/index.html
           edit-mode: replace
 
-  code_formatter:
+  Formatting:
 
     runs-on: ubuntu-latest
 
+    continue-on-error: true
+
+    permissions:
+      contents: write
+      pull-requests: write
+
     steps:
       - name: Get the project repository
         uses: actions/checkout@v3
@@ -458,4 +387,17 @@ jobs:
             --issue-number $GITHUB_PR_NUMBER \
             --start-rev $START_REV \
             --end-rev $END_REV \
-            --changed-files "$CHANGED_FILES"
+            --changed-files "$CHANGED_FILES"
+
+      - name: Check files using the black formatter
+        if: success() || failure()
+        uses: rickstaa/action-black@v1
+        id: action_black
+        with:
+          black_args: "."
+
+      - name: Annotate diff changes using reviewdog
+        if: (success() || failure()) && steps.action_black.outputs.is_formatted == 'true'
+        uses: reviewdog/action-suggester@v1
+        with:
+          tool_name: blackfmt

lib/Conversion/AIEVecToLLVM/AIEVecToLLVM.cpp

@@ -29,7 +29,7 @@ namespace xilinx::aievec {
 struct BufferParams {
   uint32_t start;
   uint32_t offsets;
-  uint32_t offsetsHi;
+        uint32_t offsets_hi;
   uint32_t step;
   uint32_t square;
 };
@@ -157,12 +157,12 @@ class FMAOpConversion : public mlir::ConvertOpToLLVMPattern<aievec::FMAOp> {
     BufferParams z = {};
     op.getXstart().getAsInteger(0, x.start);
     op.getXoffsets().getAsInteger(0, x.offsets);
-    op.getXoffsetsHi().getAsInteger(0, x.offsetsHi);
+    op.getXoffsetsHi().getAsInteger(0, x.offsets_hi);
     op.getXstep().getAsInteger(0, x.step);
     op.getXsquare().getAsInteger(0, x.square);
     op.getZstart().getAsInteger(0, z.start);
     op.getZoffsets().getAsInteger(0, z.offsets);
-    op.getZoffsetsHi().getAsInteger(0, z.offsetsHi);
+    op.getZoffsetsHi().getAsInteger(0, z.offsets_hi);
     op.getZstep().getAsInteger(0, z.step);
     op.getZsquare().getAsInteger(0, z.square);
 
@@ -179,10 +179,10 @@ class FMAOpConversion : public mlir::ConvertOpToLLVMPattern<aievec::FMAOp> {
         op->getLoc(), startType, rewriter.getI32IntegerAttr(z.start));
     auto xoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsets_hi}));
     auto zoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)z.offsets, (int32_t)z.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)z.offsets, (int32_t)z.offsets_hi}));
     auto confVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), confType,
         rewriter.getI32VectorAttr({(int32_t)conf[0], (int32_t)conf[1]}));
@@ -234,12 +234,12 @@ class MulOpConversion : public mlir::ConvertOpToLLVMPattern<aievec::MulOp> {
     BufferParams z = {};
     op.getXstart().getAsInteger(0, x.start);
     op.getXoffsets().getAsInteger(0, x.offsets);
-    op.getXoffsetsHi().getAsInteger(0, x.offsetsHi);
+    op.getXoffsetsHi().getAsInteger(0, x.offsets_hi);
     op.getXstep().getAsInteger(0, x.step);
     op.getXsquare().getAsInteger(0, x.square);
     op.getZstart().getAsInteger(0, z.start);
     op.getZoffsets().getAsInteger(0, z.offsets);
-    op.getZoffsetsHi().getAsInteger(0, z.offsetsHi);
+    op.getZoffsetsHi().getAsInteger(0, z.offsets_hi);
     op.getZstep().getAsInteger(0, z.step);
     op.getZsquare().getAsInteger(0, z.square);
 
@@ -256,10 +256,10 @@ class MulOpConversion : public mlir::ConvertOpToLLVMPattern<aievec::MulOp> {
         op->getLoc(), startType, rewriter.getI32IntegerAttr(z.start));
     auto xoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsets_hi}));
     auto zoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)z.offsets, (int32_t)z.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)z.offsets, (int32_t)z.offsets_hi}));
     auto confVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), confType,
         rewriter.getI32VectorAttr({(int32_t)conf[0], (int32_t)conf[1]}));
@@ -390,9 +390,9 @@ class UPDOpConversion : public mlir::ConvertOpToLLVMPattern<aievec::UPDOp> {
       // Determine the load size
       // TODO: no examples of 1024-bit output vectors: doesn't feel right
       // to attempt a 512-bit load to do an update like this
-        int loadSize = vecSizeInBits == 256   ? 128
-                       : vecSizeInBits == 512 ? 256
-                                              : 512;
+            int loadSize = vecSizeInBits == 256   ? 128
+                           : vecSizeInBits == 512 ? 256
+                                                  : 512;
 
       // Create a vectorType for the load proper
       // Load half of the final result vector
@@ -601,11 +601,11 @@ class SelectOpConversion
     op.getSelect().getAsInteger(0, select);
     op.getXstart().getAsInteger(0, x.start);
     op.getXoffsets().getAsInteger(0, x.offsets);
-    op.getXoffsetsHi().getAsInteger(0, x.offsetsHi);
+    op.getXoffsetsHi().getAsInteger(0, x.offsets_hi);
     op.getXsquare().getAsInteger(0, x.square);
     op.getYstart().getAsInteger(0, y.start);
     op.getYoffsets().getAsInteger(0, y.offsets);
-    op.getYoffsetsHi().getAsInteger(0, y.offsetsHi);
+    op.getYoffsetsHi().getAsInteger(0, y.offsets_hi);
     op.getYsquare().getAsInteger(0, y.square);
 
     // Encode the configuration register
@@ -622,10 +622,10 @@ class SelectOpConversion
         op->getLoc(), startType, rewriter.getI32IntegerAttr(y.start));
     auto xoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)x.offsets, (int32_t)x.offsets_hi}));
     auto yoffsetsVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), offsetsType,
-        rewriter.getI32VectorAttr({(int32_t)y.offsets, (int32_t)y.offsetsHi}));
+        rewriter.getI32VectorAttr({(int32_t)y.offsets, (int32_t)y.offsets_hi}));
     auto confVal = rewriter.create<LLVM::ConstantOp>(
         op->getLoc(), confType,
         rewriter.getI32VectorAttr({(int32_t)conf[0], (int32_t)conf[1]}));
@@ -718,7 +718,7 @@ class MatMulOpConversion
   using ConvertOpToLLVMPattern<aievec::MatMulOp>::ConvertOpToLLVMPattern;
 
   struct DecodedMatMulOp {
-    using Kind = enum { I32, I64, BF16 };
+        typedef enum { I32, I64, BF16 } Kind;
 
     Kind kind;
     Value lhs;
@@ -776,22 +776,25 @@ class MatMulOpConversion
     unsigned rhsBitWidth = rhsScaTy.getWidth();
     auto accScaTy = cast<IntegerType>(accVecTy.getElementType());
     unsigned accBitWidth = accScaTy.getWidth();
-    if (accBitWidth == 32) {
-      if (lhsBitWidth == 8) {
-        if (rhsBitWidth == 4) {
-          // <4x16xi8> x <16x8xi4> + <4x8xi32>
-          return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf};
-        }
-        // <4x8xi8> x <8x8xi8> + <4x8xi32>
-        return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 8};
-      }
-      if (rhsBitWidth == 8) {
-        // <4x4xi16> x <4x8xi8> + <4x8xi32>
-        return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 16};
-      }
-      // <4x2xi16> x <2x8xi16> + <4x8xi32>
-      return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 2};
-    }
+          if (accBitWidth == 32) {
+            if (lhsBitWidth == 8) {
+              if (rhsBitWidth == 4) {
+                // <4x16xi8> x <16x8xi4> + <4x8xi32>
+                return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf};
+              } else {
+                // <4x8xi8> x <8x8xi8> + <4x8xi32>
+                return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 8};
+              }
+            } else {
+              if (rhsBitWidth == 8) {
+                // <4x4xi16> x <4x8xi8> + <4x8xi32>
+                return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 16};
+              } else {
+                // <4x2xi16> x <2x8xi16> + <4x8xi32>
+                return {DecodedMatMulOp::Kind::I32, lhs, rhs, acc, signConf | 2};
+              }
+            }
+          }
 
     if (lhsBitWidth == 16) {
       if (rhsBitWidth == 8) {

utils/git/clang-tidy-pr.py

@@ -63,14 +63,14 @@ def get_pull_request_files(
     """Generator of GitHub metadata about files modified by the processed PR"""
 
     # Request a maximum of 100 pages (3000 files)
-    for page in range(1, 10):
+    for page in range(1, 100):
         url = f"{github_api_url}/repos/{repo}/pulls/{pull_request_id:d}/files?page={page:d}"
         print(url, file=sys.stderr)
         result = requests.get(
             f"{github_api_url}/repos/{repo}/pulls/{pull_request_id:d}/files?page={page:d}",
             headers={
                 "Accept": "application/vnd.github.v3+json",
-                # "Authorization": f"token {github_token}",
+                "Authorization": f"token {github_token}",
             },
             timeout=github_api_timeout,
         )
@@ -213,6 +213,8 @@ def generate_single_comment(
             }
 
         diag_message = diag["DiagnosticMessage"]
+        if not diag_message["FilePath"]:
+            continue
 
         # Normalize paths
         diag_message["FilePath"] = posixpath.normpath(
@@ -527,7 +529,7 @@ def main():
     args = parser.parse_args()
 
     # The GitHub API token is sensitive information, pass it through the environment
-    github_token = os.environ.get("INPUT_GITHUB_TOKEN")
+    github_token = os.environ.get("GITHUB_TOKEN")
 
     github_api_url = os.environ.get("GITHUB_API_URL")
     github_api_timeout = 10

utils/git/code-format-helper.py

@@ -38,7 +38,7 @@ def format_run(
 
     def pr_comment_text_for_diff(self, diff: str) -> str:
         return f"""
-:warning: {self.friendly_name}, {self.name} found issues in your code. :warning:
+<b style='color:red;'> {self.friendly_name}, {self.name} found issues in your code. </b>
 
 <details>
 <summary>
@@ -236,7 +236,7 @@ def format_run(
 
     changed_files = []
     if args.changed_files:
-        changed_files = args.changed_files.split(",")
+        changed_files = args.changed_files.split(";")
 
     failed_formatters = []
     for fmt in ALL_FORMATTERS: