Skip to content

Commit

Permalink
[flang] fix unsafe memory access using mlir::ValueRange (#78435)
Browse files Browse the repository at this point in the history
When running the `flang/test/HLFIR/simplify-hlfir-intrinsics.fir` test
case on AIX we encounter issues building op as they are not found in the
mlir context:
```
LLVM ERROR: Building op `arith.subi` but it isn't known in this MLIRContext: the dialect may not be loaded or this operation hasn't been added by the dialect. See also https://mlir.llvm.org/getting_started/Faq/#registered-loaded-dependent-whats-up-with-dialects-management
LLVM ERROR: Building op `hlfir.yield_element` but it isn't known in this MLIRContext: the dialect may not be loaded or this operation hasn't been added by the dialect. See also https://mlir.llvm.org/getting_started/Faq/#registered-loaded-dependent-whats-up-with-dialects-management
LLVM ERROR: Building op `hlfir.yield_element` but it isn't known in this MLIRContext: the dialect may not be loaded or this operation hasn't been added by the dialect. See also https://mlir.llvm.org/getting_started/Faq/#registered-loaded-dependent-whats-up-with-dialects-management
```
The issue is caused by the "Merge disjoint stack slots" pass and the
error is not present if the source is built with `-mllvm
--no-stack-coloring`

Thanks to investigation by @stefanp-ibm we found that "the
initializer_list {inputIndices[1], inputIndices[0]} has a lifetime that
only exists for the range of the constructor for ValueRange. Once we get
to stack coloring we merge the stack slot for that element with another
stack slot and then it gets overwritten which corrupts
transposedIndices"

The changes below prevents the corruption of transposedIndices and
passes the test case.

Co-authored-by: Mark Danial <mark.danial@ibm.com>
  • Loading branch information
madanial0 and madanial0 committed Jan 18, 2024
1 parent 296a684 commit fe4d502
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ class TransposeAsElementalConversion
auto genKernel = [&array](mlir::Location loc, fir::FirOpBuilder &builder,
mlir::ValueRange inputIndices) -> hlfir::Entity {
assert(inputIndices.size() == 2 && "checked in TransposeOp::validate");
mlir::ValueRange transposedIndices{{inputIndices[1], inputIndices[0]}};
const std::initializer_list<mlir::Value> initList = {inputIndices[1],
inputIndices[0]};
mlir::ValueRange transposedIndices(initList);
hlfir::Entity element =
hlfir::getElementAt(loc, builder, array, transposedIndices);
hlfir::Entity val = hlfir::loadTrivialScalar(loc, builder, element);
Expand Down
3 changes: 0 additions & 3 deletions flang/test/HLFIR/simplify-hlfir-intrinsics.fir
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// XFail the following test case on AIX due to potential miscompilation
// TODO: Crash fir-opt on AIX
// XFAIL: system-aix
// RUN: fir-opt --simplify-hlfir-intrinsics %s | FileCheck %s

// box with known extents
Expand Down

0 comments on commit fe4d502

Please sign in to comment.