Allocmem array extents #1158
Allocmem array extents #1158
Conversation
Fixed sized allocmem array extents are part of the type, and should not be duplicated as dynamic values.
| auto shapeOp = getOrReadExtentsAndShapeOp(loc, rewriter, load, extents); | ||
| auto arrTy = fir::unwrapRefType(load.memref().getType()); |
There was a problem hiding this comment.
Using fir::unwrapPassByRefType here removes the need of the if BoxType below.
flang/test/Lower/array-temp.f90
Outdated
| ! CHECK: %[[aa:[0-9]+]] = fir.address_of(@_QEaa) : !fir.ref<!fir.array<2650000xf32>>{{$}} | ||
| integer, parameter :: N = 2650000 | ||
| real aa(N) | ||
| ! CHECK: %7 = fir.array_coor %[[aa]](%1) %6 : (!fir.ref<!fir.array<2650000xf32>>, !fir.shape<1>, index) -> !fir.ref<f32> |
There was a problem hiding this comment.
I think you should either run the script to generate the test or manually capture all the ssa value number, otherwise, this test is really likely to break for small unrelated changes adding some other value and changing the numbering.
There was a problem hiding this comment.
oops, I only wanted to focus on the array ops
There was a problem hiding this comment.
There is more than just %7, there is still dozens of ssa values here and there (%1, %6, %32, %33.....) I think you need a more radical sed to replace them by {{.*}} if you do not care about them.
There was a problem hiding this comment.
Ah, yes, thanks for pointing that out
| @@ -797,11 +797,17 @@ class ArrayUpdateConversionBase : public mlir::OpRewritePattern<ArrayOp> { | |||
| LLVM_DEBUG(llvm::outs() << "Yes, conflict was found\n"); | |||
| rewriter.setInsertionPoint(loadOp); | |||
| // Copy in. | |||
| llvm::SmallVector<mlir::Value> extents; | |||
| llvm::SmallVector<mlir::Value> extents, nonconstantExtents; | |||
There was a problem hiding this comment.
nit: please make these two distinct declarations.
| real aa(N) | ||
| ! CHECK: fir.array_coor %[[aa]](%[[shape]]) %{{[0-9]+}} : (!fir.ref<!fir.array<2650000xf32>>, !fir.shape<1>, index) -> !fir.ref<f32> | ||
| aa = -2 | ||
| ! CHECK: %[[temp:[0-9]+]] = fir.allocmem !fir.array<2650000xf32> |
There was a problem hiding this comment.
Nice.
Shouldn't there be a test for a non-constant sized array and a test for a mix of constant and non-constant extents?
Fixed sized allocmem array extents are part of the type, and should not be duplicated as dynamic values.
Fixed sized allocmem array extents are part of the type, and should not
be duplicated as dynamic values.