Skip to content

Revert "[mlir] IntegerRangeAnalysis: don't loop over splat..." #115388

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

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

IanWood1
Copy link
Contributor

@IanWood1 IanWood1 commented Nov 7, 2024

Hitting assertion in IREE https://github.com/iree-org/iree/actions/runs/11732283897/job/32684201665?pr=19066

iree-compile: /__w/iree/iree/third_party/llvm-project/mlir/include/mlir/IR/BuiltinAttributes.h:423: auto mlir::DenseElementsAttr::getValues() const [T = llvm::APInt]: Assertion `succeeded(range) && "element type cannot be iterated"' failed.
Please report issues to [https://github.com/iree-org/iree/issues](https://github.com/iree-org/iree/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen) and include the crash backtrace.
 #0 0x00007fb2d7967667 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /__w/iree/iree/third_party/llvm-project/llvm/lib/Support/Unix/Signals.inc:723:13
 #1 0x00007fb2d79658b0 llvm::sys::RunSignalHandlers() /__w/iree/iree/third_party/llvm-project/llvm/lib/Support/Signals.cpp:106:18
 #2 0x00007fb2d7967d2a SignalHandler(int) /__w/iree/iree/third_party/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #3 0x00007fb2d199a520 (/lib/x86_64-linux-gnu/libc.so.6+0x42520)
 #4 0x00007fb2d19ee9fc pthread_kill (/lib/x86_64-linux-gnu/libc.so.6+0x969fc)
 #5 0x00007fb2d199a476 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x42476)
 #6 0x00007fb2d19807f3 abort (/lib/x86_64-linux-gnu/libc.so.6+0x287f3)
 #7 0x00007fb2d198071b (/lib/x86_64-linux-gnu/libc.so.6+0x2871b)
 #8 0x00007fb2d1991e96 (/lib/x86_64-linux-gnu/libc.so.6+0x39e96)
 #9 0x00007fb2dc587649 (/__w/iree/iree/build/lib/libIREECompiler.so+0xa7ba649)
#10 0x00007fb2dc55f523 mlir::detail::InferIntRangeInterfaceInterfaceTraits::Model<mlir::arith::ConstantOp>::inferResultRanges(mlir::detail::InferIntRangeInterfaceInterfaceTraits::Concept const*, mlir::Operation*, llvm::ArrayRef<mlir::ConstantIntRanges>, llvm::function_ref<void (mlir::Value, mlir::ConstantIntRanges const&)>) /__w/iree/iree/build/llvm-project/tools/mlir/include/mlir/Interfaces/InferIntRangeInterface.h.inc:123:3
#11 0x00007fb2dc5d3d90 llvm::SmallVectorTemplateCommon<mlir::ConstantIntRanges, void>::begin() /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/ADT/SmallVector.h:267:45
#12 0x00007fb2dc5d3d90 llvm::SmallVector<mlir::ConstantIntRanges, 1u>::~SmallVector() /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1202:31
#13 0x00007fb2dc5d3d90 mlir::intrange::detail::defaultInferResultRanges(mlir::InferIntRangeInterface, llvm::ArrayRef<mlir::IntegerValueRange>, llvm::function_ref<void (mlir::Value, mlir::IntegerValueRange const&)>) /__w/iree/iree/third_party/llvm-project/mlir/lib/Interfaces/InferIntRangeInterface.cpp:166:1
#14 0x00007fb2dc5b9f2c llvm::SmallVectorTemplateCommon<mlir::IntegerValueRange, void>::begin() /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/ADT/SmallVector.h:267:45
#15 0x00007fb2dc5b9f2c llvm::SmallVector<mlir::IntegerValueRange, 1u>::~SmallVector() /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1202:31
#16 0x00007fb2dc5b9f2c mlir::dataflow::IntegerRangeAnalysis::visitOperation(mlir::Operation*, llvm::ArrayRef<mlir::dataflow::IntegerValueRangeLattice const*>, llvm::ArrayRef<mlir::dataflow::IntegerValueRangeLattice*>) /__w/iree/iree/third_party/llvm-project/mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp:107:1
#17 0x00007fb2dc5bd91f mlir::dataflow::AbstractSparseForwardDataFlowAnalysis::visitOperation(mlir::Operation*) /__w/iree/iree/third_party/llvm-project/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp:161:10
#18 0x00007fb2dc5bd0c9 llvm::LogicalResult::failed() const /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/Support/LogicalResult.h:43:43
#19 0x00007fb2dc5bd0c9 llvm::failed(llvm::LogicalResult) /__w/iree/iree/third_party/llvm-project/llvm/include/llvm/Support/LogicalResult.h:71:58
#20 0x00007fb2dc5bd0c9 mlir::dataflow::AbstractSparseForwardDataFlowAnalysis::initializeRecursively(mlir::Operation*) /__w/iree/iree/third_party/llvm-project/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp:70:7
...

Reverts #115229

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Ian Wood (IanWood1)

Changes

Reverts llvm/llvm-project#115229


Full diff: https://github.com/llvm/llvm-project/pull/115388.diff

1 Files Affected:

  • (modified) mlir/lib/Dialect/Arith/IR/InferIntRangeInterfaceImpls.cpp (-5)
diff --git a/mlir/lib/Dialect/Arith/IR/InferIntRangeInterfaceImpls.cpp b/mlir/lib/Dialect/Arith/IR/InferIntRangeInterfaceImpls.cpp
index 59c9759d35393f..8682294c8a6972 100644
--- a/mlir/lib/Dialect/Arith/IR/InferIntRangeInterfaceImpls.cpp
+++ b/mlir/lib/Dialect/Arith/IR/InferIntRangeInterfaceImpls.cpp
@@ -40,11 +40,6 @@ void arith::ConstantOp::inferResultRanges(ArrayRef<ConstantIntRanges> argRanges,
     setResultRange(getResult(), ConstantIntRanges::constant(value));
     return;
   }
-  if (auto splatAttr = llvm::dyn_cast_or_null<SplatElementsAttr>(getValue())) {
-    setResultRange(getResult(), ConstantIntRanges::constant(
-                                    splatAttr.getSplatValue<APInt>()));
-    return;
-  }
   if (auto arrayCstAttr =
           llvm::dyn_cast_or_null<DenseIntElementsAttr>(getValue())) {
     std::optional<ConstantIntRanges> result;

Copy link

github-actions bot commented Nov 7, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link
Contributor

@MaheshRavishankar MaheshRavishankar left a comment

Choose a reason for hiding this comment

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

Please add reason for revert (you could add link to break in IREE as well)

@IanWood1 IanWood1 changed the title Revert "[mlir] IntegerRangeAnalysis: don't loop over splat attr " Revert "[mlir] IntegerRangeAnalysis: don't loop over splat..." Nov 7, 2024
@IanWood1 IanWood1 merged commit cd022b7 into main Nov 7, 2024
6 of 7 checks passed
@IanWood1 IanWood1 deleted the revert-115229-handle_splat branch November 7, 2024 23:48
IanWood1 added a commit that referenced this pull request Nov 8, 2024
Reland #115229 which was
reverted by #115388 because it
was hitting an assertion in IREE. From the original change: If the
`DenseIntElementsAttr` is a splat value, there is no need to loop over
the entire attr. Instead, just update with the splat value.


The problem with the original implementation is that `SplatElementsAttr`
might be an attr of non `APInt` (e.g. float) elements. Instead, check if
`DenseIntElementsAttr` is splat and use the splat value. Added a test to
ensure there's no crash when handling float attrs.
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
…115388)

Hitting assertion in IREE
https://github.com/iree-org/iree/actions/runs/11732283897/job/32684201665?pr=19066

```
iree-compile: /__w/iree/iree/third_party/llvm-project/mlir/include/mlir/IR/BuiltinAttributes.h:423: auto mlir::DenseElementsAttr::getValues() const [T = llvm::APInt]: Assertion `succeeded(range) && "element type cannot be iterated"' failed.
```


Reverts llvm#115229
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
…5399)

Reland llvm#115229 which was
reverted by llvm#115388 because it
was hitting an assertion in IREE. From the original change: If the
`DenseIntElementsAttr` is a splat value, there is no need to loop over
the entire attr. Instead, just update with the splat value.


The problem with the original implementation is that `SplatElementsAttr`
might be an attr of non `APInt` (e.g. float) elements. Instead, check if
`DenseIntElementsAttr` is splat and use the splat value. Added a test to
ensure there's no crash when handling float attrs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants