iree icon indicating copy to clipboard operation
iree copied to clipboard

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

Open Max191 opened this issue 1 year ago • 2 comments

https://github.com/llvm/llvm-project/commit/137a7451f458cf7d8e1d88df93dbd8da6888886d 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 https://github.com/iree-org/iree/pull/17770.

The compiler failure can be reproduced with these steps:

  1. reapply https://github.com/llvm/llvm-project/commit/137a7451f458cf7d8e1d88df93dbd8da6888886d 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>

Max191 avatar Jul 01 '24 16:07 Max191

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.

hanhanW avatar Jul 01 '24 17:07 hanhanW

Sorry, I missed this yesterday (for whatever reason I assumed https://github.com/iree-org/iree/issues/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 ^^^:

  • https://github.com/iree-org/iree/pull/17790

banach-space avatar Jul 02 '24 12:07 banach-space

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!

hanhanW avatar Jul 02 '24 17:07 hanhanW

sorry for bothering you when you're traveling.

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

banach-space avatar Jul 02 '24 17:07 banach-space