Skip to content

Commit 9eaf40c

Browse files
committed
Fix bit-select sensitivity lists leaking LLHD ops
1 parent f404969 commit 9eaf40c

File tree

3 files changed

+215
-24
lines changed

3 files changed

+215
-24
lines changed

lib/Conversion/MooreToCore/MooreToCore.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,21 @@ struct WaitEventOpConversion : public OpConversionPattern<WaitEventOp> {
532532
// Determine the values used during event detection that are defined outside
533533
// the `wait_event`'s body region. We want to wait for a change on these
534534
// signals before we check if any interesting event happened.
535+
//
536+
// For bit-select sensitivity lists like `@(posedge data[0])`, we need to
537+
// observe the extracted i1 trigger value, not the full multi-bit signal.
538+
// The extract op's input is a `moore.read` which is defined inside the
539+
// body, so we need to trace back to the signal reference that the read
540+
// operates on.
541+
SmallDenseMap<Value, Value> signalRefToTrigger;
542+
for (Value trigger : valuesBefore) {
543+
if (auto extractOp = trigger.getDefiningOp<ExtractOp>()) {
544+
// The extract input is the read result, trace back to the signal ref
545+
if (auto readOp = extractOp.getInput().getDefiningOp<ReadOp>())
546+
signalRefToTrigger[readOp.getInput()] = trigger;
547+
}
548+
}
549+
535550
SmallVector<Value> observeValues;
536551
auto setInsertionPointAfterDef = [&](Value value) {
537552
if (auto *op = value.getDefiningOp())
@@ -543,6 +558,33 @@ struct WaitEventOpConversion : public OpConversionPattern<WaitEventOp> {
543558
getValuesToObserve(&clonedOp.getBody(), setInsertionPointAfterDef,
544559
typeConverter, rewriter, observeValues);
545560

561+
// Replace multi-bit observed values with their corresponding i1 trigger
562+
// values if they came from signals that have bit-select triggers.
563+
for (Value &observed : observeValues) {
564+
if (observed.getType().isSignlessInteger(1))
565+
continue;
566+
// Check if this observed value is a probe of a signal that has a
567+
// bit-select trigger. The observed value is `llhd.prb %sig` where %sig
568+
// is the remapped signal ref.
569+
auto probeOp = observed.getDefiningOp<llhd::ProbeOp>();
570+
if (!probeOp)
571+
continue;
572+
// Find the original Moore signal ref that maps to this signal
573+
for (auto [signalRef, trigger] : signalRefToTrigger) {
574+
auto remappedRef = rewriter.getRemappedValue(signalRef);
575+
if (remappedRef == probeOp.getSignal()) {
576+
// Replace the multi-bit probe with the converted i1 trigger.
577+
OpBuilder::InsertionGuard g(rewriter);
578+
if (auto *defOp = trigger.getDefiningOp())
579+
rewriter.setInsertionPointAfter(defOp);
580+
auto i1Type = rewriter.getI1Type();
581+
observed = typeConverter->materializeTargetConversion(rewriter, loc,
582+
i1Type, trigger);
583+
break;
584+
}
585+
}
586+
}
587+
546588
// Create the `llhd.wait` op that suspends the current process and waits for
547589
// a change in the interesting values listed in `observeValues`. When a
548590
// change is detected, execution resumes in the "check" block.

lib/Dialect/LLHD/Transforms/Deseq.cpp

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,79 @@ bool Deseq::analyzeProcess() {
288288
for (auto [index, trigger] : llvm::enumerate(triggers))
289289
booleanLattice.insert({trigger, getPresentTrigger(index)});
290290

291+
// For triggers that are extracts, find other extracts from the same signal
292+
// at the same bit position and seed them as equivalent. This handles cases
293+
// like `@(posedge data[0])` where the trigger extracts from a block argument
294+
// that carries the old signal value, but the "present" value in the edge
295+
// detection logic extracts from a hoisted probe of the same signal.
296+
for (unsigned triggerIndex = 0; triggerIndex < triggers.size();
297+
++triggerIndex) {
298+
Value trigger = triggers[triggerIndex];
299+
auto extractOp = trigger.getDefiningOp<comb::ExtractOp>();
300+
if (!extractOp)
301+
continue;
302+
303+
// Find the signal being probed by tracing through block arguments.
304+
auto traceToProbe = [&](Value value) -> Value {
305+
SmallPtrSet<Value, 8> seen;
306+
SmallVector<Value> worklist;
307+
worklist.push_back(value);
308+
while (!worklist.empty()) {
309+
auto v = worklist.pop_back_val();
310+
if (!seen.insert(v).second)
311+
continue;
312+
if (auto probeOp = v.getDefiningOp<ProbeOp>())
313+
return probeOp.getSignal();
314+
if (auto arg = dyn_cast<BlockArgument>(v)) {
315+
for (auto *pred : arg.getOwner()->getPredecessors()) {
316+
auto *term = pred->getTerminator();
317+
if (auto br = dyn_cast<cf::BranchOp>(term)) {
318+
worklist.push_back(br.getDestOperands()[arg.getArgNumber()]);
319+
} else if (auto condBr = dyn_cast<cf::CondBranchOp>(term)) {
320+
if (condBr.getTrueDest() == arg.getOwner())
321+
worklist.push_back(
322+
condBr.getTrueDestOperands()[arg.getArgNumber()]);
323+
if (condBr.getFalseDest() == arg.getOwner())
324+
worklist.push_back(
325+
condBr.getFalseDestOperands()[arg.getArgNumber()]);
326+
} else if (auto waitOp = dyn_cast<WaitOp>(term)) {
327+
if (waitOp.getDest() == arg.getOwner())
328+
worklist.push_back(waitOp.getDestOperands()[arg.getArgNumber()]);
329+
}
330+
}
331+
}
332+
}
333+
return {};
334+
};
335+
336+
Value triggerSignal = traceToProbe(extractOp.getInput());
337+
if (!triggerSignal)
338+
continue;
339+
unsigned triggerLowBit = extractOp.getLowBit();
340+
341+
// Find other extracts at the same bit position from probes of the same
342+
// signal and seed them as equivalent to this trigger.
343+
process.getBody().walk([&](comb::ExtractOp otherExtract) {
344+
if (otherExtract == extractOp)
345+
return;
346+
if (otherExtract.getLowBit() != triggerLowBit)
347+
return;
348+
if (otherExtract.getType() != extractOp.getType())
349+
return;
350+
Value otherSignal = traceToProbe(otherExtract.getInput());
351+
if (otherSignal != triggerSignal)
352+
return;
353+
// This extract is equivalent to the trigger - seed it as present trigger.
354+
if (!booleanLattice.contains(otherExtract.getResult())) {
355+
LLVM_DEBUG(llvm::dbgs()
356+
<< "- Equivalent extract: " << otherExtract << " for trigger "
357+
<< trigger << "\n");
358+
booleanLattice.insert(
359+
{otherExtract.getResult(), getPresentTrigger(triggerIndex)});
360+
}
361+
});
362+
}
363+
291364
// Ensure the wait op destination operands, i.e. the values passed from the
292365
// past into the present, are the observed values.
293366
for (auto [operand, blockArg] :
@@ -1015,11 +1088,57 @@ void Deseq::implementRegister(DriveInfo &drive) {
10151088
OpBuilder builder(drive.op);
10161089
auto loc = drive.op.getLoc();
10171090

1091+
// If the clock trigger is an extract op inside the process, we need to
1092+
// create an equivalent extract outside the process. This happens when the
1093+
// sensitivity list is a bit-select like `@(posedge data[0])`.
1094+
Value clockValue = drive.clock.clock;
1095+
if (auto extractOp = clockValue.getDefiningOp<comb::ExtractOp>()) {
1096+
if (!clockValue.getParentRegion()->isProperAncestor(&process.getBody())) {
1097+
// The extract is inside the process. Find the underlying probe by
1098+
// tracing through block arguments.
1099+
Value input = extractOp.getInput();
1100+
while (auto arg = dyn_cast<BlockArgument>(input)) {
1101+
// Find a branch that feeds this block argument
1102+
Value tracedValue;
1103+
for (auto *pred : arg.getOwner()->getPredecessors()) {
1104+
auto *term = pred->getTerminator();
1105+
if (auto br = dyn_cast<cf::BranchOp>(term)) {
1106+
tracedValue = br.getDestOperands()[arg.getArgNumber()];
1107+
break;
1108+
} else if (auto condBr = dyn_cast<cf::CondBranchOp>(term)) {
1109+
if (condBr.getTrueDest() == arg.getOwner()) {
1110+
tracedValue = condBr.getTrueDestOperands()[arg.getArgNumber()];
1111+
break;
1112+
}
1113+
if (condBr.getFalseDest() == arg.getOwner()) {
1114+
tracedValue = condBr.getFalseDestOperands()[arg.getArgNumber()];
1115+
break;
1116+
}
1117+
} else if (auto waitOp = dyn_cast<WaitOp>(term)) {
1118+
if (waitOp.getDest() == arg.getOwner()) {
1119+
tracedValue = waitOp.getDestOperands()[arg.getArgNumber()];
1120+
break;
1121+
}
1122+
}
1123+
}
1124+
if (!tracedValue)
1125+
break;
1126+
input = tracedValue;
1127+
}
1128+
// If we traced to a value outside the process, create an extract there.
1129+
if (input.getParentRegion()->isProperAncestor(&process.getBody())) {
1130+
builder.setInsertionPoint(process);
1131+
clockValue = comb::ExtractOp::create(builder, loc, extractOp.getType(),
1132+
input, extractOp.getLowBit());
1133+
}
1134+
}
1135+
}
1136+
10181137
// Materialize the clock as a `!seq.clock` value. Insert an inverter for
10191138
// negedge clocks.
1020-
auto &clockCast = materializedClockCasts[drive.clock.clock];
1139+
auto &clockCast = materializedClockCasts[clockValue];
10211140
if (!clockCast)
1022-
clockCast = seq::ToClockOp::create(builder, loc, drive.clock.clock);
1141+
clockCast = seq::ToClockOp::create(builder, loc, clockValue);
10231142
auto clock = clockCast;
10241143
if (!drive.clock.risingEdge) {
10251144
auto &clockInv = materializedClockInverters[clock];

lib/Dialect/LLHD/Transforms/Mem2Reg.cpp

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -653,8 +653,8 @@ struct Promoter {
653653
Value resolveSlot(Value projectionOrSlot);
654654

655655
void captureAcrossWait();
656-
void captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
657-
Liveness &liveness, DominanceInfo &dominance);
656+
void captureValueAcrossWait(Value value, ArrayRef<WaitOp> waitOps,
657+
Liveness &liveness, DominanceInfo &dominance);
658658

659659
void constructLattice();
660660
void propagateBackward();
@@ -847,7 +847,9 @@ Value Promoter::resolveSlot(Value projectionOrSlot) {
847847
/// Explicitly capture any probes that are live across an `llhd.wait` as block
848848
/// arguments and destination operand of that wait. This ensures that replacing
849849
/// the probe with a reaching definition later on will capture the value of the
850-
/// reaching definition before the wait.
850+
/// reaching definition before the wait. Also capture any extract ops of probes
851+
/// that are live across wait, to allow the extract result (e.g., i1) to be
852+
/// captured instead of the full probe result (e.g., i8).
851853
void Promoter::captureAcrossWait() {
852854
if (region.hasOneBlock())
853855
return;
@@ -861,53 +863,81 @@ void Promoter::captureAcrossWait() {
861863
Liveness liveness(region.getParentOp());
862864

863865
SmallVector<WaitOp> crossingWaitOps;
866+
867+
// Capture probes that are live across wait ops.
864868
for (auto &block : region) {
865869
for (auto probeOp : block.getOps<ProbeOp>()) {
866870
for (auto waitOp : waitOps)
867871
if (liveness.getLiveness(waitOp->getBlock())->isLiveOut(probeOp))
868872
crossingWaitOps.push_back(waitOp);
869873
if (!crossingWaitOps.empty()) {
870-
captureAcrossWait(probeOp, crossingWaitOps, liveness, dominance);
874+
captureValueAcrossWait(probeOp, crossingWaitOps, liveness, dominance);
875+
crossingWaitOps.clear();
876+
}
877+
}
878+
}
879+
880+
// Capture extract ops of probes that are live across wait ops. This handles
881+
// cases like `@(posedge data[0])` where the extract result (i1) should be
882+
// captured instead of the probe result (i8), enabling the probe to be hoisted.
883+
for (auto &block : region) {
884+
for (auto extractOp : block.getOps<comb::ExtractOp>()) {
885+
// Only handle extracts of probe results.
886+
if (!extractOp.getInput().getDefiningOp<ProbeOp>())
887+
continue;
888+
for (auto waitOp : waitOps)
889+
if (liveness.getLiveness(waitOp->getBlock())->isLiveOut(extractOp))
890+
crossingWaitOps.push_back(waitOp);
891+
if (!crossingWaitOps.empty()) {
892+
captureValueAcrossWait(extractOp, crossingWaitOps, liveness, dominance);
871893
crossingWaitOps.clear();
872894
}
873895
}
874896
}
875897
}
876898

877-
/// Add a probe as block argument to a list of wait ops and update uses of the
878-
/// probe to use the added block arguments as appropriate. This may insert
879-
/// additional block arguments in case the probe and added block arguments both
899+
/// Add a value as block argument to a list of wait ops and update uses of the
900+
/// value to use the added block arguments as appropriate. This may insert
901+
/// additional block arguments in case the value and added block arguments both
880902
/// reach the same block.
881-
void Promoter::captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
882-
Liveness &liveness, DominanceInfo &dominance) {
903+
void Promoter::captureValueAcrossWait(Value value, ArrayRef<WaitOp> waitOps,
904+
Liveness &liveness,
905+
DominanceInfo &dominance) {
883906
LLVM_DEBUG({
884-
llvm::dbgs() << "Capture " << probeOp << "\n";
907+
llvm::dbgs() << "Capture " << value << "\n";
885908
for (auto waitOp : waitOps)
886909
llvm::dbgs() << "- Across " << waitOp << "\n";
887910
});
888911

889-
// Calculate the merge points for this probe once it gets promoted to block
912+
// Get the block where the value is defined.
913+
Block *defBlock = nullptr;
914+
if (auto *defOp = value.getDefiningOp())
915+
defBlock = defOp->getBlock();
916+
else
917+
defBlock = cast<BlockArgument>(value).getOwner();
918+
919+
// Calculate the merge points for this value once it gets promoted to block
890920
// arguments across the wait ops.
891921
auto &domTree = dominance.getDomTree(&region);
892922
llvm::IDFCalculatorBase<Block, false> idfCalculator(domTree);
893923

894-
// Calculate the set of blocks which will define this probe as a distinct
924+
// Calculate the set of blocks which will define this value as a distinct
895925
// value.
896926
SmallPtrSet<Block *, 4> definingBlocks;
897-
definingBlocks.insert(probeOp->getBlock());
927+
definingBlocks.insert(defBlock);
898928
for (auto waitOp : waitOps)
899929
definingBlocks.insert(waitOp.getDest());
900930
idfCalculator.setDefiningBlocks(definingBlocks);
901931

902-
// Calculate where the probe is live.
932+
// Calculate where the value is live.
903933
SmallPtrSet<Block *, 16> liveInBlocks;
904934
for (auto &block : region)
905-
if (liveness.getLiveness(&block)->isLiveIn(probeOp))
935+
if (liveness.getLiveness(&block)->isLiveIn(value))
906936
liveInBlocks.insert(&block);
907937
idfCalculator.setLiveInBlocks(liveInBlocks);
908938

909939
// Calculate the merge points where we will have to insert block arguments for
910-
// this probe.
940+
// this value.
911941
SmallVector<Block *> mergePointsVec;
912942
idfCalculator.calculate(mergePointsVec);
913943
SmallPtrSet<Block *, 16> mergePoints(mergePointsVec.begin(),
@@ -916,29 +946,29 @@ void Promoter::captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
916946
mergePoints.insert(waitOp.getDest());
917947
LLVM_DEBUG(llvm::dbgs() << "- " << mergePoints.size() << " merge points\n");
918948

919-
// Perform a depth-first search starting at the block containing the probe,
949+
// Perform a depth-first search starting at the block containing the value,
920950
// which dominates all its uses. When we encounter a block that is a merge
921951
// point, insert a block argument.
922952
struct WorklistItem {
923953
DominanceInfoNode *domNode;
924954
Value reachingDef;
925955
};
926956
SmallVector<WorklistItem> worklist;
927-
worklist.push_back({domTree.getNode(probeOp->getBlock()), probeOp});
957+
worklist.push_back({domTree.getNode(defBlock), value});
928958

929959
while (!worklist.empty()) {
930960
auto item = worklist.pop_back_val();
931961
auto *block = item.domNode->getBlock();
932962

933-
// If this block is a merge point, insert a block argument for the probe.
963+
// If this block is a merge point, insert a block argument for the value.
934964
if (mergePoints.contains(block))
935965
item.reachingDef =
936-
block->addArgument(probeOp.getType(), probeOp.getLoc());
966+
block->addArgument(value.getType(), value.getLoc());
937967

938-
// Replace any uses of the probe in this block with the current reaching
968+
// Replace any uses of the value in this block with the current reaching
939969
// definition.
940970
for (auto &op : *block)
941-
op.replaceUsesOfWith(probeOp, item.reachingDef);
971+
op.replaceUsesOfWith(value, item.reachingDef);
942972

943973
// If the terminator of this block branches to a merge point, add the
944974
// current reaching definition as a destination operand.

0 commit comments

Comments
 (0)