Skip to content

Commit 2cfee4e

Browse files
fabianschuikiclaude
andcommitted
[ImportVerilog] Fix signal captures in timing controls
When a task reads a module-scope signal in an event control expression like `@(posedge clk)`, the signal must be captured as an extra argument to the task function. Previously, `convertTimingControl` set the `rvalueReadCallback` to null to prevent event control reads from polluting implicit `@*` sensitivity lists. This also suppressed the function capture callback, causing the signal to be used across region boundaries without being plumbed as an argument. Replace the callback nulling with an `isInsideTimingControl` flag. The implicit event callback checks this flag before recording reads, while function capture callbacks (which don't check it) continue to work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 838a8bb commit 2cfee4e

File tree

3 files changed

+50
-8
lines changed

3 files changed

+50
-8
lines changed

lib/Conversion/ImportVerilog/ImportVerilogInternals.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,12 @@ struct Context {
420420
/// used to collect all variables assigned in a task scope.
421421
std::function<void(mlir::Operation *)> variableAssignCallback;
422422

423+
/// Whether we are currently converting expressions inside a timing control,
424+
/// such as `@(posedge clk)`. This is used by the implicit event control
425+
/// callback to avoid adding reads from explicit event controls to the
426+
/// implicit sensitivity list.
427+
bool isInsideTimingControl = false;
428+
423429
/// The time scale currently in effect.
424430
slang::TimeScale timeScale;
425431

lib/Conversion/ImportVerilog/TimingControls.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "ImportVerilogInternals.h"
1010
#include "slang/ast/TimingControl.h"
1111
#include "llvm/ADT/ScopeExit.h"
12+
#include "llvm/Support/SaveAndRestore.h"
1213

1314
using namespace circt;
1415
using namespace ImportVerilog;
@@ -218,15 +219,11 @@ Context::convertTimingControl(const slang::ast::TimingControl &ctrl,
218219
const slang::ast::Statement &stmt) {
219220
// Convert the timing control. Implicit event control will create a new empty
220221
// `WaitEventOp` and assign it to `implicitWaitOp`. This op will be populated
221-
// further down.
222+
// further down. Mark that we are inside a timing control so the implicit
223+
// event callback can skip reads that are part of the event expression itself.
222224
moore::WaitEventOp implicitWaitOp;
223225
{
224-
auto previousCallback = rvalueReadCallback;
225-
llvm::scope_exit done([&] { rvalueReadCallback = previousCallback; });
226-
// Reads happening as part of the event control should not be added to a
227-
// surrounding implicit event control's list of implicitly observed
228-
// variables.
229-
rvalueReadCallback = nullptr;
226+
llvm::SaveAndRestore flagGuard(isInsideTimingControl, true);
230227
if (failed(handleRoot(*this, ctrl, &implicitWaitOp)))
231228
return failure();
232229
}
@@ -241,7 +238,8 @@ Context::convertTimingControl(const slang::ast::TimingControl &ctrl,
241238
llvm::scope_exit done([&] { rvalueReadCallback = previousCallback; });
242239
if (implicitWaitOp) {
243240
rvalueReadCallback = [&](moore::ReadOp readOp) {
244-
readValues.insert(readOp.getInput());
241+
if (!isInsideTimingControl)
242+
readValues.insert(readOp.getInput());
245243
if (previousCallback)
246244
previousCallback(readOp);
247245
};

test/Conversion/ImportVerilog/basic.sv

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3736,6 +3736,44 @@ module testRecursiveCaptureFunction();
37363736
endfunction
37373737
endmodule
37383738

3739+
// Task that reads a module-scope signal in an event control expression. The
3740+
// signal must be captured as an extra argument to the task function.
3741+
// CHECK-LABEL: moore.module @CaptureInEventControl
3742+
module CaptureInEventControl;
3743+
// CHECK: [[CLK:%.+]] = moore.variable : <l1>
3744+
logic clk;
3745+
// CHECK: [[DATA:%.+]] = moore.variable : <i32>
3746+
int data;
3747+
3748+
initial begin
3749+
// CHECK: call @waitForClk([[CLK]])
3750+
waitForClk();
3751+
// CHECK: call @readOnClk([[CLK]], [[DATA]])
3752+
readOnClk();
3753+
end
3754+
3755+
// CHECK: func.func private @waitForClk(%arg0: !moore.ref<l1>)
3756+
task automatic waitForClk;
3757+
// CHECK: moore.wait_event {
3758+
// CHECK: [[TMP:%.+]] = moore.read %arg0
3759+
// CHECK: moore.detect_event posedge [[TMP]]
3760+
// CHECK: }
3761+
@(posedge clk);
3762+
endtask
3763+
3764+
// CHECK: func.func private @readOnClk(%arg0: !moore.ref<l1>, %arg1: !moore.ref<i32>)
3765+
task automatic readOnClk;
3766+
int result;
3767+
// CHECK: moore.wait_event {
3768+
// CHECK: [[TMP:%.+]] = moore.read %arg0
3769+
// CHECK: moore.detect_event posedge [[TMP]]
3770+
// CHECK: }
3771+
@(posedge clk);
3772+
// CHECK: [[TMP:%.+]] = moore.read %arg1
3773+
result = data;
3774+
endtask
3775+
endmodule
3776+
37393777
// CHECK-LABEL: moore.module @RealLiteral() {
37403778
module RealLiteral();
37413779
// CHECK-NEXT: [[REALCONSTANT:%.+]] = moore.constant_real 5.000000e-01 : f64

0 commit comments

Comments
 (0)