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.

Sign up for GitHub

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

Fix bug in artifact creation of the LX6 instructions #453

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Fix bug in artifact creation of the LX6 instructions #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions build_tools/ci/cpu_comparison/run_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,9 @@ function run_test() {
# Example of running a test directly from an .mlir file with a function.
run_test --test_file ${THIS_DIR}/test_files/matmul_int32.mlir

# An example of an arbitrary graph with three matmuls which form three dispatches.
run_test --test_file ${THIS_DIR}/test_files/three_matmuls.mlir

# Example of generating a matmul test from a template, and then running it.
test_name=${OUTPUT_DIR}/test_from_template.mlir
matmul_template_dir=${THIS_DIR}/matmul_template
Expand Down
31 changes: 31 additions & 0 deletions build_tools/ci/cpu_comparison/test_files/three_matmuls.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// This test shows arbitrary matmuls that would have producer consumer relationships
// across different dispatches running on CI.

// These 4 lines are required by the script which generates input data:
//
// input 32x32xf32
// input 32x32xf32
// input 32x4xf32
// input 4x32xf32

!A_TYPE = tensor<32x32xf32>
!B_TYPE = tensor<32x4xf32>
!C_TYPE = tensor<4x32xf32>
!D_TYPE = tensor<4x4xf32>
func.func @two_mm(%lhs : !A_TYPE,
%rhs : !A_TYPE, %rhs_2 : !B_TYPE, %lhs_2 : !C_TYPE) -> !D_TYPE {
%empty = tensor.empty() : !A_TYPE
%empty_2 = tensor.empty() : !B_TYPE
%empty_3 = tensor.empty() : !D_TYPE
%cst = arith.constant 0.0 : f32
%fill = linalg.fill ins(%cst : f32) outs(%empty : !A_TYPE) -> !A_TYPE
%fill_2 = linalg.fill ins(%cst : f32) outs(%empty_2 : !B_TYPE) -> !B_TYPE
%fill_3 = linalg.fill ins(%cst : f32) outs(%empty_3 : !D_TYPE) -> !D_TYPE
%2 = linalg.matmul ins(%lhs, %rhs : !A_TYPE, !A_TYPE)
outs(%fill : !A_TYPE) -> !A_TYPE
%3 = linalg.matmul ins(%2, %rhs_2 : !A_TYPE, !B_TYPE)
outs(%fill_2 : !B_TYPE) -> !B_TYPE
%4 = linalg.matmul ins(%lhs_2, %3 : !C_TYPE, !B_TYPE)
outs(%fill_3 : !D_TYPE) -> !D_TYPE
return %4 : !D_TYPE
}
3 changes: 2 additions & 1 deletion compiler/plugins/target/AMD-AIE/iree-amd-aie/Target/AIETarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ LogicalResult AIETargetBackend::serializeExecutable(

SmallString<128> aie2xclbin(options.mlirAieInstallDir);
llvm::sys::path::append(aie2xclbin, "bin", "aie2xclbin");
std::vector<uint32_t> npuInstrs;
std::unique_ptr<llvm::MemoryBuffer> xclbinIn;

FlatbufferBuilder builder;
Expand Down Expand Up @@ -359,6 +358,8 @@ LogicalResult AIETargetBackend::serializeExecutable(

std::ifstream instrFile(static_cast<std::string>(npuInstPath));
std::string line;
// Vector to store LX6 instructions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I've heard that AIE2p has LX7 instructions (not a request for change!)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's true that forthcoming gens will have LX7 (and higher?) but the instructions themselves don't actually have anything to do with the LX arch (which is Tensilica's naming scheme for their archs and not connected to our firmware that runs these instructions). But you're right the comment will be out of date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets have a clean up PR for this, I noticed you did LX[0-9] in some places, maybe we should say LX6/LX7 instead?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the safest term would be command processor but who cares (honestly when I first started it was useful knowing that it was LX6 because there's a datasheet type thing out there for that so it clarified what people were really talking about).

std::vector<uint32_t> npuInstrs;
while (std::getline(instrFile, line)) {
std::istringstream iss(line);
uint32_t a;
Expand Down
Loading