Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] Use pre-commit (copy IREE) for linting #1020

Merged
merged 16 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,3 @@
# Target emission
/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AMDAIETarget* @makslevental
/compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/XCLBinGen* @makslevental @nirvedhmeshram @newling

12 changes: 0 additions & 12 deletions .github/workflows/black.yml

This file was deleted.

4 changes: 2 additions & 2 deletions .github/workflows/ci-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
timezoneLinux: "Asia/Singapore"
timezoneMacos: "Asia/Singapore"
timezoneWindows: "Singapore Standard Time"

- name: "Checking out repository"
env:
BRANCH_NAME: ${{ github.ref }}
Expand Down Expand Up @@ -107,7 +107,7 @@ jobs:
- name: Create artifacts
if: ${{ !cancelled() }}
run: |
pushd third_party/iree/third_party/llvm-project
pushd third_party/iree/third_party/llvm-project
$llvm_sha_short = "$(git rev-parse --short HEAD)"
popd
tar cf llvm-dist-windows-$llvm_sha_short.tar llvm-install
Expand Down
29 changes: 29 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright 2019 The IREE Authors
#
# Licensed under the Apache License v2.0 with LLVM Exceptions.
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
#
# History in iree-amd-aie: copied from IREE January 2025.
# To apply fixes locally, something like the following should work
# > pip-install pre-commit
# > pre-commit run --all-files
# Fixes of tabs (replace with space) must be done manually.

name: Lint

on: [pull_request]

permissions:
contents: read

jobs:
pre-commit:
runs-on: ubuntu-24.04
steps:
- name: Checking out repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Setting up python
uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0
- name: Running pre-commit
uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# This file was copied from IREE #
##################################

# iree-amd-aie specific (CI or otherwise).
# iree-amd-aie specific (CI or otherwise).
*-build/
*-install/
kernel-doc
Expand Down
54 changes: 54 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Pre-commit (https://pre-commit.com) configuration for assorted lint checks.
#
# See https://pre-commit.com/hooks.html for more hooks.
#
# History in iree-amd-aie. January 2025: copied from IREE and trimmed down to
# the subset of hooks that are relevant.

exclude: "third_party/"

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v3.2.0
hooks:
- id: check-merge-conflict

- id: check-yaml
# * Extensions can't be included in the mkdocs schema, so skip checking
# https://github.com/squidfunk/mkdocs-material/issues/6378
# * clang-format files use `---` to split for multiple languages,
# resulting in errors like `expected a single document in the stream`
exclude: "mkdocs.yml|.clang-format"

- id: end-of-file-fixer
exclude_types: ["image", "jupyter"]

- id: trailing-whitespace
exclude_types: ["image", "jupyter"]

- repo: https://github.com/psf/black
rev: 23.3.0
hooks:
- id: black
name: Run Black to format Python files

- repo: https://github.com/pre-commit/mirrors-clang-format
# Loosely track the most recent versions in
# * Runner images: https://github.com/actions/runner-images/
# * Editor extensions: https://github.com/microsoft/vscode-cpptools
rev: v18.1.3
hooks:
- id: clang-format
name: Run clang-format on C/C++/etc. files
exclude_types: ["jupyter"]

- repo: https://github.com/Lucas-C/pre-commit-hooks
rev: v1.5.5
hooks:
- id: forbid-tabs
exclude: ".gitmodules|Makefile"

- repo: https://github.com/jlebar/pre-commit-hooks.git
rev: f2d115a052860b09b2888b4f104be614bf3b4779
hooks:
- id: do-not-submit
1 change: 0 additions & 1 deletion LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,3 @@ conflicts with the conditions of the GPLv2, you may retroactively and
prospectively choose to deem waived or otherwise exclude such Section(s) of
the License, but only in their entirety and only with respect to the Combined
Software.

13 changes: 6 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ git \
[email protected]:nod-ai/iree-amd-aie.git # https://github.com/nod-ai/iree-amd-aie.git
```

The above avoids cloning entire repo histories for submodules, and skips a few, currently, unused,
The above avoids cloning entire repo histories for submodules, and skips a few, currently, unused,
submodules that are nested in IREE.

### Dependencies
Expand Down Expand Up @@ -78,7 +78,7 @@ cmake --build <WHERE_YOU_WOULD_LIKE_TO_BUILD>

### Instructions

The bare minimum configure command for IREE with the amd-aie plugin
The bare minimum configure command for IREE with the amd-aie plugin

```
cmake \
Expand Down Expand Up @@ -106,7 +106,7 @@ you can opt-out of everything (except the `llvm-cpu` backend) with
-DIREE_INPUT_TOSA=OFF \
-DIREE_HAL_DRIVER_DEFAULTS=OFF \
-DIREE_TARGET_BACKEND_DEFAULTS=OFF \
-DIREE_TARGET_BACKEND_LLVM_CPU=ON
-DIREE_TARGET_BACKEND_LLVM_CPU=ON
```

With the above you can also skip cloning the `stablehlo` and `torch-mlir` submodules/repos but in this case you will need to add
Expand All @@ -121,14 +121,14 @@ If you're "bringing your own LLVM", i.e., you have a prebuilt/compiled distribut
-DIREE_BUILD_BUNDLED_LLVM=OFF
```

In this case you will need `lit` somewhere in your environment and you will need to add to CMake `-DLLVM_EXTERNAL_LIT=<SOMEWHERE>`
In this case you will need `lit` somewhere in your environment and you will need to add to CMake `-DLLVM_EXTERNAL_LIT=<SOMEWHERE>`
(e.g., `pip install lit; SOMEWHERE=$(which lit)`).

See [Bringing your own LLVM](#bringing-your-own-llvm) below for more information on using prebuilt/compiled distributions of LLVM.

## Testing

Lit tests (i.e., compiler tests) specific to AIE can be run with something like
Lit tests (i.e., compiler tests) specific to AIE can be run with something like

```
cd <WHERE_YOU_WOULD_LIKE_TO_BUILD>
Expand All @@ -137,7 +137,7 @@ ctest -R amd-aie --output-on-failure -j 10

(the `-j 10` runs `10` tests in parallel)

Other tests, which run on device, are in the `build_tools` subdirectory.
Other tests, which run on device, are in the `build_tools` subdirectory.
See [build_tools/ci/run_all_runtime_tests.sh](build_tools/ci/run_all_runtime_tests.sh) for an example script that shows how to run all the runtime tests.

## Pro-tips
Expand Down Expand Up @@ -181,4 +181,3 @@ Note, this is roughly equivalent to [passing](https://github.com/nod-ai/iree-amd
## Architectural overview (out of date)

![image](https://github.com/nod-ai/iree-amd-aie/assets/74956/3fa73139-5fdf-4658-86c3-0705352c4ea0)

Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ def get_output_type(self, N, OH, OW, OC, output_element_type):


class ConvolutionMlirGenerator:

def __init__(
self,
conv_type,
Expand Down
1 change: 0 additions & 1 deletion build_tools/ci/cpu_comparison/input_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ def bf16_to_f32(bfloat16_array):


def generate_bfloat16_data(nb_values, lower_bound, upper_bound, rng):

float_data = rng.integers(lower_bound, upper_bound, nb_values).astype(np.float32)

# Convert float32 data to bfloat16
Expand Down
7 changes: 0 additions & 7 deletions build_tools/ci/cpu_comparison/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ def add_aie_compilation_flags(self, flags):
self.aie_compilation_flags = list(set(self.aie_compilation_flags))

def run(self, config):

# If the target device is not in the set of devices to run on, then
# return False. ie. don't raise an error because is legitimate,
# we just won't run the test.
Expand Down Expand Up @@ -237,7 +236,6 @@ def __init__(
self.function_name = function_name

def vs_cpu(self, config):

filename = self.get_filename(config)

if self.use_ukernel and not config.vitis_dir:
Expand All @@ -256,7 +254,6 @@ def vs_cpu(self, config):
return True

def benchmark(self, config):

filename = self.get_filename(config)

if self.use_ukernel and not config.vitis_dir:
Expand All @@ -277,7 +274,6 @@ def benchmark(self, config):
return True

def generate(self, config, template_name):

generate_matmul_test(
self.get_filename(config),
template_name,
Expand Down Expand Up @@ -890,7 +886,6 @@ def shell_out(cmd: list, workdir=None, verbose: int = 0, raise_on_error=True, en


def print_program_memory_size(test_dir):

# Get all the .elf files in `test_dir`.
# These elfs contain many sections, one of which is the program memory. Some digging into the elf format
# see https://github.com/newling/aie-rt/commit/d0f08bc4a37092a919d6a0d51a44d9f0ae274bb9
Expand Down Expand Up @@ -1567,7 +1562,6 @@ def aie_vs_llvm_cpu(


class Tests:

def add_aie_compilation_flags(self, flags):
for test in self.tests:
test.add_aie_compilation_flags(flags)
Expand Down Expand Up @@ -2182,7 +2176,6 @@ def all_tests(
not_match = []

for test in tests.tests:

skip = test.name in skip_test_set or any(
(label in skip_test_set for label in test.labels)
)
Expand Down
1 change: 0 additions & 1 deletion build_tools/ci/reset_npu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,3 @@ set -e
sudo modprobe -r amdxdna
sudo modprobe drm_shmem_helper
sudo modprobe amdxdna dyndbg==pflm timeout_in_sec=10

4 changes: 2 additions & 2 deletions build_tools/ci/run_matmul_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ function run_matmul_test() {
# should still be checked to compile (set num_repeat_runs=0 in this case).
local num_repeat_runs="1"

# Run the test 'num_corruption_repeat_runs' times without an NPU reset in
# Run the test 'num_corruption_repeat_runs' times without an NPU reset in
# between. This can be used to check for corruption, i.e. the AIE might be
# left in a bad state in between runs. Additionally, this increases the speed
# of the repeated test
Expand Down Expand Up @@ -570,7 +570,7 @@ run_matmul_test \
# ObjectFifo Matmul tests
###################################################################

# Run repeatedly to check for non-deterministic hangs and numerical
# Run repeatedly to check for non-deterministic hangs and numerical
# issues.
repeat_shapes=(
'32x32x32'
Expand Down
2 changes: 1 addition & 1 deletion build_tools/clang_llvm_tools_not_to_build.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,4 @@ CLANG_TOOL_CLANG_SCAN_DEPS
CLANG_TOOL_CLANG_SHLIB
CLANG_TOOL_DIAGTOOL
CLANG_TOOL_LIBCLANG
CLANG_TOOL_NVPTX_ARCH
CLANG_TOOL_NVPTX_ARCH
1 change: 0 additions & 1 deletion cmake/iree_aie_utils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,3 @@ function(replace_string_in_file _file _match_string _replace_string)
file(WRITE "${_file}" "${_file_contents}")
file(LOCK "${_lock_file}" RELEASE)
endfunction()

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace mlir::iree_compiler::XDNAOPLIB {
#define GEN_PASS_DEF_XDNAOPLIBHELLOWORLD
#include "xdna-oplib/Transforms/Passes.h.inc"

}
} // namespace mlir::iree_compiler::XDNAOPLIB


#endif // IREE_XDNA_OPLIB_TRANSFORMS_PASSDETAIL_H_
#endif // IREE_XDNA_OPLIB_TRANSFORMS_PASSDETAIL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include "mlir/Pass/PassManager.h"
#include "xdna-oplib/Transforms/Passes.h"

#include "mlir/Pass/PassManager.h"

namespace mlir::iree_compiler::XDNAOPLIB {

void addXDNAOPLIBPreprocessingExtensions(OpPassManager &pm) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ namespace mlir::iree_compiler::XDNAOPLIB {
void addXDNAOPLIBPreprocessingExtensions(OpPassManager &pm);

// Hello world pass to show the XDNA OpLib is functional
std::unique_ptr<OperationPass<>>
createXDNAOPLIBHelloWorldPass();
std::unique_ptr<OperationPass<>> createXDNAOPLIBHelloWorldPass();

// Registration for all XDNA OpLib passes.
void registerXDNAOPLIBPasses();

}
} // namespace mlir::iree_compiler::XDNAOPLIB

#endif // IREE_XDNA_OPLIB_TRANSFORMS_PASSES_H_
#endif // IREE_XDNA_OPLIB_TRANSFORMS_PASSES_H_
7 changes: 3 additions & 4 deletions compiler/plugins/target/AMD-AIE/aie/AIEDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,9 @@ LogicalResult DMABDOp::verify() {
<< "Invalid step size; must be a positive integer.";
}
if (dim.getStride() > bufferType.getNumElements()) {
return emitOpError()
<< "Step size " << std::to_string(dim.getStride()) << " "
<< "exceeds memref size "
<< std::to_string(bufferType.getNumElements());
return emitOpError() << "Step size " << std::to_string(dim.getStride())
<< " " << "exceeds memref size "
<< std::to_string(bufferType.getNumElements());
}
if (dim.getSize() >= ((1UL << 10) - 1)) {
return emitOpError() << "Size may not exceed " << ((1UL << 10) - 1);
Expand Down
6 changes: 3 additions & 3 deletions compiler/plugins/target/AMD-AIE/aie/AIEDialect.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ void printObjectFifoProducerTile(mlir::OpAsmPrinter &printer,
llvm::SmallVectorImpl<mlir::OpAsmParser::UnresolvedOperand> &tiles,
BDDimLayoutArrayArrayAttr &dimensions);

[[maybe_unused]] void printObjectFifoConsumerTiles(mlir::OpAsmPrinter &printer,
mlir::Operation *op, mlir::OperandRange tiles,
BDDimLayoutArrayArrayAttr dimsPerTileAttr);
[[maybe_unused]] void printObjectFifoConsumerTiles(
mlir::OpAsmPrinter &printer, mlir::Operation *op, mlir::OperandRange tiles,
BDDimLayoutArrayArrayAttr dimsPerTileAttr);

TileOp getTileOp(mlir::Operation &op);
int32_t getBufferElementTypeWidthInBytes(DMABDOp &op);
Expand Down
2 changes: 2 additions & 0 deletions compiler/plugins/target/AMD-AIE/aie/AIEEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/Dialect.h"

// clang-format off: must include AIEEnums.h.inc after the above includes
#include "aie/AIEEnums.h.inc"
// clang-format on

#endif
2 changes: 1 addition & 1 deletion compiler/plugins/target/AMD-AIE/aie/AIEOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def AIE_FlowOp: AIE_Op<"flow"> {
}];
let builders = [
OpBuilder<(
ins "::mlir::Value":$source,
ins "::mlir::Value":$source,
"::mlir::iree_compiler::AMDAIE::StrmSwPortType":$source_bundle,
"uint8_t":$source_channel,
"::mlir::Value":$dest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,4 @@ void registerAMDAIENormalizeAddressSpaces() {
});
}

} // namespace mlir::iree_compiler::AMDAIE
} // namespace mlir::iree_compiler::AMDAIE
Loading
Loading