From 90b8e8b15713b91023b97915bbf1c741f61ac8c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20B=C3=B6ck?= Date: Sun, 14 Jun 2026 21:01:32 +0200 Subject: [PATCH 1/3] [CFG] Traverse through `shrsi` in GIID check The GIID implementation uses a white-list of operations to traverse which `shrsi` was previously not part of. This would lead to e.g. data dependencies not correctly being found and used to eliminate memory ordering dependencies. Fixes https://github.com/EPFL-LAP/dynamatic/issues/974 --- lib/Support/CFG.cpp | 13 ++++++------ ...handshake-deactivate-mem-dependencies.mlir | 20 ++++++++++++++++++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/Support/CFG.cpp b/lib/Support/CFG.cpp index 09bbd1c81..d3455b116 100644 --- a/lib/Support/CFG.cpp +++ b/lib/Support/CFG.cpp @@ -694,12 +694,13 @@ static GIIDStatus isGIIDRec( handshake::CmpIOp, handshake::DivSIOp, handshake::DivUIOp, handshake::ExtSIOp, handshake::ExtUIOp, handshake::MulIOp, handshake::OrIOp, handshake::ShLIOp, handshake::ShRUIOp, - handshake::SubIOp, handshake::TruncIOp, handshake::XOrIOp, - handshake::AddFOp, handshake::CmpFOp, handshake::DivFOp, - handshake::MulFOp, handshake::SubFOp>([&](auto) { - // At least one operand must depend on the predecessor - return foldGIIDStatusOr(recurse, defOp->getOperands()); - }) + handshake::ShRSIOp, handshake::SubIOp, handshake::TruncIOp, + handshake::XOrIOp, handshake::AddFOp, handshake::CmpFOp, + handshake::DivFOp, handshake::MulFOp, handshake::SubFOp>( + [&](auto) { + // At least one operand must depend on the predecessor + return foldGIIDStatusOr(recurse, defOp->getOperands()); + }) .Default([&](auto) { // To err on the conservative side, produce the most terminating // kind of failure on encoutering an unsupported operation diff --git a/test/Transforms/handshake-deactivate-mem-dependencies.mlir b/test/Transforms/handshake-deactivate-mem-dependencies.mlir index 6bb191f34..a54ef379d 100644 --- a/test/Transforms/handshake-deactivate-mem-dependencies.mlir +++ b/test/Transforms/handshake-deactivate-mem-dependencies.mlir @@ -1,4 +1,4 @@ -// RUN: dynamatic-opt %s --handshake-deactivate-mem-dependencies | FileCheck %s +// RUN: dynamatic-opt %s --handshake-deactivate-mem-dependencies --split-input-file | FileCheck %s // CHECK-LABEL: handshake.func @test0( handshake.func @test0(%arg0: !handshake.channel, %arg4: memref<8xi32>, %arg5: memref<8xi8>, %arg6: !handshake.control<>, %arg7: !handshake.control<>, %arg8: !handshake.control<>, ...) -> (!handshake.control<>, !handshake.control<>, !handshake.control<>) attributes {argNames = ["var1", "var0", "var2", "var0_start", "var2_start", "start"], resNames = ["var0_end", "var2_end", "end"]} { @@ -20,4 +20,22 @@ handshake.func @test0(%arg0: !handshake.channel, %arg4: memref<8xi32>, %arg end {handshake.bb = 1 : ui32, handshake.name = "end0"} %1#1, %0, %arg8 : <>, <>, <> } +// ----- +// CHECK-LABEL: handshake.func @shrsi_giid( +handshake.func @shrsi_giid(%arg0: !handshake.channel, %arg1: memref<2xi16>, %arg2: !handshake.control<>, %arg3: !handshake.control<>, ...) -> (!handshake.control<>, !handshake.control<>) attributes {argNames = ["var5", "var1", "var1_start", "start"], resNames = ["var1_end", "end"]} { + %0:2 = lsq[%arg1 : memref<2xi16>] (%arg2, %arg3, %addressResult, %addressResult_0, %dataResult_1, %arg3) {groupSizes = [2 : i32], handshake.name = "lsq0"} : (!handshake.control<>, !handshake.control<>, !handshake.channel, !handshake.channel, !handshake.channel, !handshake.control<>) -> (!handshake.channel, !handshake.control<>) + %1 = constant %arg3 {handshake.bb = 0 : ui32, handshake.name = "constant0", value = 0 : i32} : <>, + // CHECK: load + // CHECK-SAME: #handshake, handshake.name = "load0"} : , , , + %2 = extsi %dataResult {handshake.bb = 0 : ui32, handshake.name = "extsi0"} : to + %3 = extui %arg0 {handshake.bb = 0 : ui32, handshake.name = "extui0"} : to + %4 = shrsi %2, %3 {handshake.bb = 0 : ui32, handshake.name = "shrsi0"} : + %5 = trunci %4 {handshake.bb = 0 : ui32, handshake.name = "trunci0"} : to + %addressResult_0, %dataResult_1 = store[%1] %5 {handshake.bb = 0 : ui32, handshake.name = "store1"} : , , , + end {handshake.bb = 0 : ui32, handshake.name = "end0"} %0#1, %arg3 : <>, <> +} From bddc989436c18ea23e6f2c355676efd787b2e15c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20B=C3=B6ck?= Date: Sun, 14 Jun 2026 22:48:58 +0200 Subject: [PATCH 2/3] add other arithmetic ops as well --- lib/Support/CFG.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/Support/CFG.cpp b/lib/Support/CFG.cpp index d3455b116..de6a7db9a 100644 --- a/lib/Support/CFG.cpp +++ b/lib/Support/CFG.cpp @@ -695,8 +695,12 @@ static GIIDStatus isGIIDRec( handshake::ExtSIOp, handshake::ExtUIOp, handshake::MulIOp, handshake::OrIOp, handshake::ShLIOp, handshake::ShRUIOp, handshake::ShRSIOp, handshake::SubIOp, handshake::TruncIOp, - handshake::XOrIOp, handshake::AddFOp, handshake::CmpFOp, - handshake::DivFOp, handshake::MulFOp, handshake::SubFOp>( + handshake::XOrIOp, handshake::NotIOp, handshake::RemSIOp, + handshake::MaxSIOp, handshake::MaxUIOp, handshake::MinSIOp, + handshake::MinUIOp, handshake::MaximumFOp, + handshake::MinimumFOp, handshake::AddFOp, handshake::CmpFOp, + handshake::DivFOp, handshake::MulFOp, handshake::SubFOp, + handshake::NegFOp, handshake::AbsFOp, handshake::ExtFOp>( [&](auto) { // At least one operand must depend on the predecessor return foldGIIDStatusOr(recurse, defOp->getOperands()); From 0002958c07220fefa78fe93d12e6af140ec9644f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20B=C3=B6ck?= Date: Mon, 15 Jun 2026 09:26:43 +0200 Subject: [PATCH 3/3] also add truncf op --- lib/Support/CFG.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Support/CFG.cpp b/lib/Support/CFG.cpp index de6a7db9a..e05541ef4 100644 --- a/lib/Support/CFG.cpp +++ b/lib/Support/CFG.cpp @@ -700,11 +700,11 @@ static GIIDStatus isGIIDRec( handshake::MinUIOp, handshake::MaximumFOp, handshake::MinimumFOp, handshake::AddFOp, handshake::CmpFOp, handshake::DivFOp, handshake::MulFOp, handshake::SubFOp, - handshake::NegFOp, handshake::AbsFOp, handshake::ExtFOp>( - [&](auto) { - // At least one operand must depend on the predecessor - return foldGIIDStatusOr(recurse, defOp->getOperands()); - }) + handshake::NegFOp, handshake::AbsFOp, handshake::ExtFOp, + handshake::TruncFOp>([&](auto) { + // At least one operand must depend on the predecessor + return foldGIIDStatusOr(recurse, defOp->getOperands()); + }) .Default([&](auto) { // To err on the conservative side, produce the most terminating // kind of failure on encoutering an unsupported operation