[HWToLLVM] Do not take illegal shortcut in array_get lowering. (#9172)

Currently, the LLVM lowering of `hw.array_get` will attempt to reuse pointers to array values to avoid rematerializing the array on the stack. This is only legal as long as we can ensure the underlying buffer remains unchanged. It works within the confines of the HWToLLVM lowering, which always allocates a new buffer when modifying an array. However, operations of other dialects (e.g., `arc.state_write`) might still write to the pointed-to buffer between the `load` and the `array_get`, causing it to return an incorrect value.

Author: fzi-hielscher

lib/Conversion/HWToLLVM/HWToLLVM.cpp

@@ -210,23 +210,15 @@ struct ArrayGetOpConversion : public ConvertOpToLLVMPattern<hw::ArrayGetOp> {
   LogicalResult
   matchAndRewrite(hw::ArrayGetOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
-
-    Value arrPtr;
-    if (auto load = adaptor.getInput().getDefiningOp<LLVM::LoadOp>()) {
-      // In this case the array was loaded from an existing address, so we can
-      // just grab that address instead of reallocating the array on the stack.
-      arrPtr = load.getAddr();
-    } else {
-      auto oneC = LLVM::ConstantOp::create(
-          rewriter, op->getLoc(), IntegerType::get(rewriter.getContext(), 32),
-          rewriter.getI32IntegerAttr(1));
-      arrPtr = LLVM::AllocaOp::create(
-          rewriter, op->getLoc(),
-          LLVM::LLVMPointerType::get(rewriter.getContext()),
-          adaptor.getInput().getType(), oneC,
-          /*alignment=*/4);
-      LLVM::StoreOp::create(rewriter, op->getLoc(), adaptor.getInput(), arrPtr);
-    }
+    auto oneC = LLVM::ConstantOp::create(
+        rewriter, op->getLoc(), IntegerType::get(rewriter.getContext(), 32),
+        rewriter.getI32IntegerAttr(1));
+    Value arrPtr = LLVM::AllocaOp::create(
+        rewriter, op->getLoc(),
+        LLVM::LLVMPointerType::get(rewriter.getContext()),
+        adaptor.getInput().getType(), oneC,
+        /*alignment=*/4);
+    LLVM::StoreOp::create(rewriter, op->getLoc(), adaptor.getInput(), arrPtr);
 
     auto arrTy = typeConverter->convertType(op.getInput().getType());
     auto elemTy = typeConverter->convertType(op.getResult().getType());

test/Conversion/HWToLLVM/convert_aggregates.mlir

@@ -151,13 +151,6 @@ func.func @convertConstArray(%arg0 : i1, %arg1 : i32) {
   // CHECK-NEXT: %[[VAL_3:.*]] = llvm.load %[[VAL_2]] : !llvm.ptr -> !llvm.array<2 x i32>
   %0 = hw.aggregate_constant [0 : i32, 1 : i32] : !hw.array<2xi32>
 
-  // COM: Test: when the array argument is already a load from a pointer,
-  // COM: then don't allocate on the stack again but take that pointer directly as a shortcut
-  // CHECK-NEXT: %[[VAL_5:.*]] = llvm.zext %arg0 : i1 to i2
-  // CHECK-NEXT: %[[VAL_6:.*]] = llvm.getelementptr %[[VAL_2]][0, %[[VAL_5]]] : (!llvm.ptr, i2) -> !llvm.ptr, !llvm.array<2 x i32>
-  // CHECK-NEXT: %{{.+}} = llvm.load %[[VAL_6]] : !llvm.ptr -> i32
-  %1 = hw.array_get %0[%arg0] : !hw.array<2xi32>, i1
-
   // COM: Test: nested constant array can also converted to a constant global
   // CHECK: %[[VAL_7:.*]] = llvm.mlir.addressof @[[GLOB2]] : !llvm.ptr
   // CHECK-NEXT: %{{.+}} = llvm.load %[[VAL_7]] : !llvm.ptr -> !llvm.array<2 x array<2 x i32>>
@@ -237,3 +230,22 @@ func.func @nestedStructInject(%arg0: !hw.struct<a: i1, b: !hw.struct<c: i1>>, %a
   %0 = hw.struct_inject %arg0["b"], %arg1 : !hw.struct<a: i1, b: !hw.struct<c: i1>>
   return
 }
+
+// CHECK-LABEL: @issue9171
+func.func @issue9171(%idx: i1) -> i32 {
+  // CHECK: [[ARRPTR:%.+]] = llvm.mlir.addressof
+  // CHECK: [[ARRVAL:%.+]] = llvm.load [[ARRPTR]]
+  %cst = hw.aggregate_constant [0 : i32, 1 : i32] : !hw.array<2xi32>
+
+  // COM: Until we do proper RAW dependency checking array_get cannot assume
+  // COM: the array's backing buffer to be immutable, so it has to rematerialize
+  // COM: [[ARRVAL]] on the stack and must not reuse [[ARRPTR]].
+
+  // CHECK: [[ALLOCA:%.+]] = llvm.alloca
+  // CHECK: llvm.store [[ARRVAL]], [[ALLOCA]]
+  // CHECK: [[VALPTR:%.+]] = llvm.getelementptr [[ALLOCA]]
+  // CHECK: [[RETVAL:%.+]] = llvm.load [[VALPTR]]
+  // CHECK: return [[RETVAL]]
+  %get = hw.array_get %cst[%idx] : !hw.array<2xi32>, i1
+  return %get : i32
+}