Skip to content
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

Buffer deallocation pass: non-deterministic output #59118

Closed
bondhugula opened this issue Nov 21, 2022 · 8 comments
Closed

Buffer deallocation pass: non-deterministic output #59118

bondhugula opened this issue Nov 21, 2022 · 8 comments
Assignees
Labels
mlir:bufferization Bufferization infrastructure

Comments

@bondhugula
Copy link
Contributor

bondhugula commented Nov 21, 2022

Each run of the buffer-deallocation produces different IR (a different ordering of bufferization.clone operations). A simple test case is below. A previous commit from Feb 2022 mentions this issue, but this never got fixed.

commit d955ca49379e73485304eb7f500db53e33109b0f
Author: Benjamin Kramer <benny.kra@googlemail.com>
Date:   Thu Feb 17 13:51:27 2022 +0100

    [BufferDeallocation] Don't assume successor operands are unique
    
    This would create a double free when a memref is passed twice to the
    same op. This wasn't a problem at the time the pass was written but is
    common since the introduction of scf.while.
    
    There's a latent non-determinism that's triggered by the test, but this
    change is messy enough as-is so I'll leave that for later.
    
    Differential Revision: https://reviews.llvm.org/D120044
module {
  memref.global "private" constant @__constant_xi32 : memref<i32> = dense<0>
  func.func @buffer_dealloc_issue(%arg0: memref<32x64xf32>, %arg1: memref<32x64x6xf32>, %arg2: memref<i32>, %arg3: memref<32x64xf32>) {
    %c0_i32 = arith.constant 0 : i32
    %0 = memref.get_global @__constant_xi32 : memref<i32>
    %1 = affine.load %arg2[] : memref<i32>
    %2 = arith.maxsi %1, %c0_i32 : i32
    %3:4 = scf.while (%arg4 = %0, %arg5 = %0, %arg6 = %arg0, %arg7 = %arg1) : (memref<i32>, memref<i32>, memref<32x64xf32>, memref<32x64x6xf32>) -> (memref<i32>, memref<i32>, memref<32x64xf32>, memref<32x64x6xf32>) {
      %4 = affine.load %arg5[] : memref<i32>
      %5 = arith.cmpi slt, %4, %2 : i32
      scf.condition(%5) %arg4, %arg5, %arg6, %arg7 : memref<i32>, memref<i32>, memref<32x64xf32>, memref<32x64x6xf32>
    } do {
    ^bb0(%arg4: memref<i32>, %arg5: memref<i32>, %arg6: memref<32x64xf32>, %arg7: memref<32x64x6xf32>):  // no predecessors
      %4 = memref.alloc() : memref<i32>
      %5 = memref.alloc() : memref<32x64xf32>
      %6 = memref.alloc() : memref<32x64x6xf32>
      %7 = memref.alloc() : memref<i32>
      scf.yield %7, %4, %5, %6 : memref<i32>, memref<i32>, memref<32x64xf32>, memref<32x64x6xf32>
    }
    return
  }
}

$ mlir-opt -buffer-deallocation test_case.mlir

@bondhugula bondhugula added mlir:core MLIR Core Infrastructure mlir mlir:bufferization Bufferization infrastructure and removed mlir:core MLIR Core Infrastructure labels Nov 21, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 21, 2022

@llvm/issue-subscribers-mlir-core

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 21, 2022

@llvm/issue-subscribers-mlir

@bondhugula bondhugula changed the title Buffer deallocation pass non-deterministic output Buffer deallocation pass: non-deterministic output Nov 21, 2022
@bondhugula
Copy link
Contributor Author

EugeneZelenko removed the mlir label 18 minutes ago

@EugeneZelenko why is the mlir label removed? This should appear when someone searches for all mlir issues (with label:mlir).

@EugeneZelenko
Copy link
Contributor

@bondhugula: I think it's good idea to not use generic labels when specific one exist. Same logic apply to clang, flang, compiler-rt, etc.

@bondhugula
Copy link
Contributor Author

bondhugula commented Nov 22, 2022

@bondhugula: I think it's good idea to not use generic labels when specific one exist. Same logic apply to clang, flang, compiler-rt, etc.

The specific label here didn't exist - I just created it, and no one actually knows about it (it's quite narrow as well). I'd keep mlir till mlir:bufferization becomes known or broad enough with at least a few issues.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2022

@llvm/issue-subscribers-mlir

@bondhugula
Copy link
Contributor Author

CC: @dfki-mako @d0k

@bondhugula bondhugula self-assigned this Feb 9, 2023
@bondhugula
Copy link
Contributor Author

https://reviews.llvm.org/D143622 fixes this.

@bondhugula bondhugula reopened this Feb 9, 2023
@EugeneZelenko EugeneZelenko removed the mlir label Feb 9, 2023
CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 10, 2023
The buffer-deallocation pass generates a different output on each run
due to an unstable iteration order.

Fixes: llvm/llvm-project#59118

Reviewed By: mehdi_amini

Differential Revision: https://reviews.llvm.org/D143622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:bufferization Bufferization infrastructure
Projects
None yet
Development

No branches or pull requests

3 participants