Skip to content

Conversation

@ermilindwalekar
Copy link

@ermilindwalekar ermilindwalekar commented Dec 2, 2025

Summary

Pickup patches that didn't make it to upstream release/21.x

Noteworthy changes

  1. 260eca3 [MLIR] Cherry-pick upstream fix for tablegen builder codegen (Op::create() forwarding regression):

LLVM 21.x introduced new tablegen codegen for OpTy::create().
The initial implementation generated create() functions that dropped forwarding and always took builder parameters by value, even when the ODS signature specified T&& or move-only types.
This caused invalid code to be emitted for operations whose ODS builders legitimately use move semantics, e.g.

OpBuilder<(ins "::mlir::Value":$in,
              "Whatever&&":$descriptor)>

The generated create() attempted to copy these arguments instead of forwarding them, resulting in incorrect symantics for Whatever&&
Upstream MLIR fixed this by updating the create() generator to accept rvalue references when appropritate and also forward argument correctly using std::forward

Without this patch, we would need to rewrite our ODS to avoid Whatever&&, forcing us to use const Whatever&

  1. b12665b is a direct cherry-pick from MLIR upstream. It replaces our own patch (which is the exact same) from 20.x: 78577c6

  2. d503ca4 is redone according to new-er MLIR handling of quantization scales: it was by accident completely broken in our LLVM 20.x (see the commit diff in d73f0c6) and is now restored. To simplify the LLVM update process, the check is still rather "dumb" and only discards scales outside of expressed type range i.e. -largest_expressed_type < scale < largest_expressed_type. As a consequence, values such as zero or '-2.22213e-18' (for fp16 types) are still allowed (in fact, our LIT tests use such values right now) even though -2.22213e-18 cannot be represented in fp16 as it is less than smallest float16.

JIRA ticket

Related PR in NPU Compiler and/or OpenVINO repository with sub-module update

@ermilindwalekar ermilindwalekar requested a review from a team December 2, 2025 18:16
@ermilindwalekar ermilindwalekar changed the base branch from npu/release/21.x to release/21.x December 2, 2025 18:18
@ermilindwalekar ermilindwalekar force-pushed the EISW-194514 branch 2 times, most recently from fb54d05 to d186c40 Compare December 3, 2025 10:42
@andrey-golubev
Copy link

I see that:

  • 6a27c93 is not cherry-picked at all

As Harald suggested, please mention 260eca3 addition in the PR description (and why it's necessary).

hrotuna
hrotuna previously approved these changes Dec 4, 2025
@hrotuna
Copy link

hrotuna commented Dec 4, 2025

6a27c93 is not cherry-picked at all

Looks like LLVM 21 already contains this change.

@Maxim-Doronin Maxim-Doronin changed the base branch from release/21.x to npu/release/21.x December 4, 2025 17:21
@Maxim-Doronin Maxim-Doronin dismissed hrotuna’s stale review December 4, 2025 17:21

The base branch was changed.

andrey-golubev
andrey-golubev previously approved these changes Dec 8, 2025
@andrey-golubev
Copy link

I'd wait for @hrotuna / @nikita-kud approval, but to me this looks fine. Thanks!

hrotuna
hrotuna previously approved these changes Dec 8, 2025
@Maxim-Doronin
Copy link

Hi! Please make sure you have your git setup properly #10 (comment)

hrotuna
hrotuna previously approved these changes Dec 16, 2025
Copy link

@andrey-golubev andrey-golubev left a comment

Choose a reason for hiding this comment

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

Please don't introduce merge commits:

Image

they needlessly clutter the git history. fyi @Maxim-Doronin

Vladislav Vinogradov and others added 28 commits January 12, 2026 15:10
add support for SymbolContainer trait in MLIR infrastructure
…lvm#37)

* added integral type check
* changed to f8e_m_ format, removed isF8 helper method, formatting
* added negative tests, removed duplicate test
* Added endline in parse-uniform-invalid.mlir
* changed to f8E5M2/f8E4M3FN format, enabled default type parsing
* updated parse-any-invalid error message checks
* Fix <anonymous> declared here

* Fix Ubuntu24 (llvm#33)
* Expanding Quant dialect with Quantile Quantized type

* Adding quantile mlir tests

* Adding check on quantiles array size and updated mlir tests
* relax negative scale conditions
* adapt tests now handling negative scales
* Adding missing check in QuantileQuantizedPerAxisType::verify
* Adding minor test for illegal quantile array size for per axis type
This reverses partially changed in commit 2c1ae80
…and-line parser reset

When LLVM TableGen flags are globally appended, mlir-src-sharder fails with an unknown argument error because its command-line parser is reset during execution and does not retain the inherited flags.
After the original API change to DefaultTimingManager::setOutput() (see
362aa43), users are forced to provide
their own implementation of OutputStrategy. However, default MLIR
implementations are usually sufficient. Expose Text and Json strategies
via factory-like method to avoid the problem in downstream projects.
…vm#159766)

Support custom types (3/N): allow custom tensor and buffer types in
function signatures and at call-sites. This is one of the major building
blocks to move in the direction of module-level one-shot-bufferization
support.

To achieve this, `BufferizationOptions::FunctionArgTypeConverterFn`
callback is converted to work with tensor-like and buffer-like types,
instead of the builtin counterparts. The default behavior for builtins
remains unchanged, while custom types by default go through
`TensorLikeType::getBufferType()` which is a general conversion
interface.
…vm#170)

Duplicate of PR merged on npu/release/19.x llvm#151
Cherry-pick from upstream llvm: llvm#147721; adding std::move to getChecked method.
…#167705)

Generally, to_tensor and to_buffer already perform sufficient
verification. However, there are some unnecessarily strict constraints:
* builtin tensor requires its buffer counterpart to always be memref
* to_buffer on ranked tensor requires to always return memref

These checks are assertions (i.e. preconditions), however, they actually
prevent an apparently useful bufferization where builtin tensors could
become custom buffers. Lift these assertions, maintaining the
verification procedure unchanged, to allow builtin -> custom
bufferizations at operation boundary level.
…ted code (llvm#168536) (llvm#181)

ODS generate code can be included and used outside of the `mlir`
namespace and so references to symbols in the mlir namespace
must be fully qualified.
The recent changes in the MLIR TableGen interface for generated
OpTy::build functions involves a new OpTy::create function that is
generated passing arguments without forwarding. This is problematic with
arguments that are move only such as `std::unique_ptr`. My particular
use case involves `std::unique_ptr<mlir::Region>` which is desirable as
the `mlir::OperationState` object accepts calls to
`addRegion(std::unique_ptr<mlir::Region>`.

In Discord, the use of `extraClassDeclarations` was suggested which I
may go with regardless since I still have to define the builder function
anyways, but perhaps you would consider this trivial change as it
supports a broader class of argument types for this approach.

Consider the declaration in TableGen:
```
  let builders = [
    OpBuilder<(ins "::mlir::Value":$cdr,
                   "::mlir::ValueRange":$packs,
                   "std::unique_ptr<::mlir::Region>":$body)>
  ];
  ```

  Which currently generates:
  ```cpp
 ExpandPacksOp ExpandPacksOp::create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::Value cdr, ::mlir::ValueRange packs, std::unique_ptr<::mlir::Region> body) {
  ::mlir::OperationState __state__(location, getOperationName());
  build(builder, __state__, std::forward<decltype(cdr)>(cdr), std::forward<decltype(packs)>(packs), std::forward<decltype(body)>(body));
  auto __res__ = ::llvm::dyn_cast<ExpandPacksOp>(builder.create(__state__));
  assert(__res__ && "builder didn't return the right type");
  return __res__;
}

```
With this change it will generate:
```cpp
ExpandPacksOp ExpandPacksOp::create(::mlir::OpBuilder &builder, ::mlir::Location location, ::mlir::Value cdr, ::mlir::ValueRange packs, std::unique_ptr<::mlir::Region>&&body) {
  ::mlir::OperationState __state__(location, getOperationName());
  build(builder, __state__, static_cast<decltype(cdr)>(cdr), std::forward<decltype(packs)>(packs), std::forward<decltype(body)>(body));
  auto __res__ = ::llvm::dyn_cast<ExpandPacksOp>(builder.create(__state__));
  assert(__res__ && "builder didn't return the right type");
  return __res__;
}
```

Another option could be to make this function a template but then it
would not be hidden in the generated translation unit. I don't know if
that was the original intent. Thank you for your consideration.
…eConversionsAndLegality (llvm#160344)

In a downstream project, there is a need for a type conversion pattern
for scf.index_switch operation. A test is added into
`mlir/test/Dialect/SparseTensor/scf_1_N_conversion.mlir` (not sure this
functionality is really required for sparse tensors, but the test
showcase that the new conversion pattern is functional)
…lvm#87948)

This commit moves the code responsible for adding newlines and tracking
indent, so that it can be used not only for operation printers, but also
for attribute and type printers.

It could be useful for nested attributes, where proper formatting with
newlines and indents would benefit the readability of the IR. Currently,
everything is printed on one line, which makes it difficult to read if
the attribute is more verbose and there are multiple levels of nesting.

Co-authored-by: Andruszkiewicz, Jacenty <[email protected]>
@andrey-golubev
Copy link

Note: had to sign all the commits, otherwise, merging is blocked :| No code changes other than that.

@Maxim-Doronin Maxim-Doronin merged commit 8d12776 into intel-staging:npu/release/21.x Jan 13, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.