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

iree-compile failures with n-D vector.bitcast ops #17780

Open
Max191 opened this issue Jul 1, 2024 · 4 comments · May be fixed by #17810
Open

iree-compile failures with n-D vector.bitcast ops #17780

Max191 opened this issue Jul 1, 2024 · 4 comments · May be fixed by #17810
Labels
bug 🐞 Something isn't working codegen Shared code generation infrastructure and dialects

Comments

@Max191
Copy link
Contributor

Max191 commented Jul 1, 2024

llvm/llvm-project@137a745 enables support for n-D arith.trunci narrow type emulation. This causes iree-compile failures in the benchmark suite, so it is reverted in IREE by #17770.

The compiler failure can be reproduced with these steps:

  1. reapply llvm/llvm-project@137a745 in third_party/llvm-project
  2. Compile the following IR with iree-compile --output-format=vm-bytecode --mlir-print-op-on-diagnostic=false --iree-hal-target-backends=llvm-cpu --iree-input-type=none --iree-llvmcpu-target-triple=aarch64-none-linux-android29 --iree-opt-data-tiling=true --iree-llvmcpu-enable-ukernels=all --iree-llvmcpu-target-cpu-features=+dotprod --iree-vm-emit-polyglot-zip=true --iree-llvmcpu-debug-symbols=false linalg.mlir -o /tmp/linalg.vmfb:
module @jit_forward {
  util.func public @main(%arg0: !hal.buffer_view) -> !hal.buffer_view attributes {iree.abi.stub, iree.reflection = {iree.abi.declaration = "sync func @main(%input0: tensor<1x256xi8>) -> (%output0: tensor<1x2048xi32>)"}} {
    %cst = arith.constant dense_resource<__elided__> : tensor<256x2048xi4>
    %c0_i32 = arith.constant 0 : i32
    %0 = hal.tensor.import %arg0 "input0" : !hal.buffer_view -> tensor<1x256xi8>
    %collapsed = tensor.collapse_shape %0 [[0, 1]] : tensor<1x256xi8> into tensor<256xi8>
    %1 = tensor.empty() : tensor<2048xi32>
    %2 = linalg.fill ins(%c0_i32 : i32) outs(%1 : tensor<2048xi32>) -> tensor<2048xi32>
    %3 = linalg.vecmat ins(%collapsed, %cst : tensor<256xi8>, tensor<256x2048xi4>) outs(%2 : tensor<2048xi32>) -> tensor<2048xi32>
    %expanded = tensor.expand_shape %3 [[0, 1]] output_shape [1, 2048] : tensor<2048xi32> into tensor<1x2048xi32>
    %4 = hal.tensor.export %expanded "output0" : tensor<1x2048xi32> -> !hal.buffer_view
    util.return %4 : !hal.buffer_view
  }
}

This should cause the error:

error: LLVM Translation failed for operation: builtin.unrealized_conversion_cast
    %3 = linalg.vecmat ins(%collapsed, %cst : tensor<256xi8>, tensor<256x2048xi4>) outs(%2 : tensor<2048xi32>) -> tensor<2048xi32>
@hanhanW
Copy link
Contributor

hanhanW commented Jul 1, 2024

cc @banach-space @mub-at-arm this is breaking IREE, so we reverted it on our llvm-project fork. We'll need some help to land it to IREE. Can we revert the upstream commit until it's fixed? I can help provide a repro without IREE specifics.

@banach-space
Copy link
Collaborator

Sorry, I missed this yesterday (for whatever reason I assumed #17780 was identical).

For this one it looks like IREE is missing some patterns. Indeed, this worked for me (we need to add populateVectorBitCastLoweringPatterns):

diff --git a/compiler/src/iree/compiler/Codegen/LLVMCPU/ConvertToLLVM.cpp b/compiler/src/iree/compiler/Codegen/LLVMCPU/ConvertToLLVM.cpp
index 22b4ce09a1..60d689d70c 100644
--- a/compiler/src/iree/compiler/Codegen/LLVMCPU/ConvertToLLVM.cpp
+++ b/compiler/src/iree/compiler/Codegen/LLVMCPU/ConvertToLLVM.cpp
@@ -1049,6 +1049,7 @@ void ConvertToLLVMPass::runOnOperation() {
   arith::populateArithToLLVMConversionPatterns(typeConverter, patterns);
   arith::populateExpandBFloat16Patterns(patterns);
   populateVectorToSCFConversionPatterns(patterns);
+  vector::populateVectorBitCastLoweringPatterns(patterns);
   populateVectorToLLVMMatrixConversionPatterns(typeConverter, patterns);
   populateVectorToLLVMConversionPatterns(
       typeConverter, patterns, targetReassociateFpReductions.getValue());
diff --git a/third_party/llvm-project b/third_party/llvm-project
index 8ded6ce55d..c31a6ef9fc 160000
--- a/third_party/llvm-project
+++ b/third_party/llvm-project
@@ -1 +1 @@
-Subproject commit 8ded6ce55deafcad4b78ec814f1ecc80f9889392
+Subproject commit c31a6ef9fca11ff5c662019f8a4d3feac860b24c

I am travelling this week and most of our team is OOO. This is me trying to say - apologies if there's a delay from our side 😅

I prepared a PR for ^^^:

banach-space added a commit to banach-space/iree that referenced this issue Jul 2, 2024
Fixes iree-org#17780

Signed-off-by: Andrzej Warzynski <andrzej.warzynski@arm.com>
@hanhanW
Copy link
Contributor

hanhanW commented Jul 2, 2024

Thanks thanks, and sorry for bothering you when you're traveling. I think we can land the change with dropping the local revert LLVM commit. It could need additional permission, so I'll coordinate with the integrator this week. Also, I need to figure out if we can directly add it to EmulateNarrowType pass. It can be done later. Thanks again for looking at this!

@banach-space
Copy link
Collaborator

sorry for bothering you when you're traveling.

Not at all - this is a work trip, so no excuses :-)

@ScottTodd ScottTodd added bug 🐞 Something isn't working codegen Shared code generation infrastructure and dialects labels Jul 3, 2024
@hanhanW hanhanW linked a pull request Jul 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working codegen Shared code generation infrastructure and dialects
Projects
None yet
4 participants