Fix bit-select sensitivity lists leaking LLHD ops

5iri · 5iri · commit ff252008f273 · 2026-01-21T05:22:42.000+05:30
diff --git a/lib/Conversion/MooreToCore/MooreToCore.cpp b/lib/Conversion/MooreToCore/MooreToCore.cpp
@@ -532,6 +532,21 @@ struct WaitEventOpConversion : public OpConversionPattern<WaitEventOp> {
     // Determine the values used during event detection that are defined outside
     // the `wait_event`'s body region. We want to wait for a change on these
     // signals before we check if any interesting event happened.
+    //
+    // For bit-select sensitivity lists like `@(posedge data[0])`, we need to
+    // observe the extracted i1 trigger value, not the full multi-bit signal.
+    // The extract op's input is a `moore.read` which is defined inside the
+    // body, so we need to trace back to the signal reference that the read
+    // operates on.
+    SmallDenseMap<Value, Value> signalRefToTrigger;
+    for (Value trigger : valuesBefore) {
+      if (auto extractOp = trigger.getDefiningOp<ExtractOp>()) {
+        // The extract input is the read result, trace back to the signal ref
+        if (auto readOp = extractOp.getInput().getDefiningOp<ReadOp>())
+          signalRefToTrigger[readOp.getInput()] = trigger;
+      }
+    }
+
     SmallVector<Value> observeValues;
     auto setInsertionPointAfterDef = [&](Value value) {
       if (auto *op = value.getDefiningOp())
@@ -543,6 +558,33 @@ struct WaitEventOpConversion : public OpConversionPattern<WaitEventOp> {
     getValuesToObserve(&clonedOp.getBody(), setInsertionPointAfterDef,
                        typeConverter, rewriter, observeValues);
 
+    // Replace multi-bit observed values with their corresponding i1 trigger
+    // values if they came from signals that have bit-select triggers.
+    for (Value &observed : observeValues) {
+      if (observed.getType().isSignlessInteger(1))
+        continue;
+      // Check if this observed value is a probe of a signal that has a
+      // bit-select trigger. The observed value is `llhd.prb %sig` where %sig
+      // is the remapped signal ref.
+      auto probeOp = observed.getDefiningOp<llhd::ProbeOp>();
+      if (!probeOp)
+        continue;
+      // Find the original Moore signal ref that maps to this signal
+      for (auto [signalRef, trigger] : signalRefToTrigger) {
+        auto remappedRef = rewriter.getRemappedValue(signalRef);
+        if (remappedRef == probeOp.getSignal()) {
+          // Replace the multi-bit probe with the converted i1 trigger.
+          OpBuilder::InsertionGuard g(rewriter);
+          if (auto *defOp = trigger.getDefiningOp())
+            rewriter.setInsertionPointAfter(defOp);
+          auto i1Type = rewriter.getI1Type();
+          observed = typeConverter->materializeTargetConversion(
+              rewriter, loc, i1Type, trigger);
+          break;
+        }
+      }
+    }
+
     // Create the `llhd.wait` op that suspends the current process and waits for
     // a change in the interesting values listed in `observeValues`. When a
     // change is detected, execution resumes in the "check" block.
diff --git a/lib/Dialect/LLHD/Transforms/Deseq.cpp b/lib/Dialect/LLHD/Transforms/Deseq.cpp
@@ -288,6 +288,79 @@ bool Deseq::analyzeProcess() {
   for (auto [index, trigger] : llvm::enumerate(triggers))
     booleanLattice.insert({trigger, getPresentTrigger(index)});
 
+  // For triggers that are extracts, find other extracts from the same signal
+  // at the same bit position and seed them as equivalent. This handles cases
+  // like `@(posedge data[0])` where the trigger extracts from a block argument
+  // that carries the old signal value, but the "present" value in the edge
+  // detection logic extracts from a hoisted probe of the same signal.
+  for (unsigned triggerIndex = 0; triggerIndex < triggers.size();
+       ++triggerIndex) {
+    Value trigger = triggers[triggerIndex];
+    auto extractOp = trigger.getDefiningOp<comb::ExtractOp>();
+    if (!extractOp)
+      continue;
+
+    // Find the signal being probed by tracing through block arguments.
+    auto traceToProbe = [&](Value value) -> Value {
+      SmallPtrSet<Value, 8> seen;
+      SmallVector<Value> worklist;
+      worklist.push_back(value);
+      while (!worklist.empty()) {
+        auto v = worklist.pop_back_val();
+        if (!seen.insert(v).second)
+          continue;
+        if (auto probeOp = v.getDefiningOp<ProbeOp>())
+          return probeOp.getSignal();
+        if (auto arg = dyn_cast<BlockArgument>(v)) {
+          for (auto *pred : arg.getOwner()->getPredecessors()) {
+            auto *term = pred->getTerminator();
+            if (auto br = dyn_cast<cf::BranchOp>(term)) {
+              worklist.push_back(br.getDestOperands()[arg.getArgNumber()]);
+            } else if (auto condBr = dyn_cast<cf::CondBranchOp>(term)) {
+              if (condBr.getTrueDest() == arg.getOwner())
+                worklist.push_back(
+                    condBr.getTrueDestOperands()[arg.getArgNumber()]);
+              if (condBr.getFalseDest() == arg.getOwner())
+                worklist.push_back(
+                    condBr.getFalseDestOperands()[arg.getArgNumber()]);
+            } else if (auto waitOp = dyn_cast<WaitOp>(term)) {
+              if (waitOp.getDest() == arg.getOwner())
+                worklist.push_back(
+                    waitOp.getDestOperands()[arg.getArgNumber()]);
+            }
+          }
+        }
+      }
+      return {};
+    };
+
+    Value triggerSignal = traceToProbe(extractOp.getInput());
+    if (!triggerSignal)
+      continue;
+    unsigned triggerLowBit = extractOp.getLowBit();
+
+    // Find other extracts at the same bit position from probes of the same
+    // signal and seed them as equivalent to this trigger.
+    process.getBody().walk([&](comb::ExtractOp otherExtract) {
+      if (otherExtract == extractOp)
+        return;
+      if (otherExtract.getLowBit() != triggerLowBit)
+        return;
+      if (otherExtract.getType() != extractOp.getType())
+        return;
+      Value otherSignal = traceToProbe(otherExtract.getInput());
+      if (otherSignal != triggerSignal)
+        return;
+      // This extract is equivalent to the trigger - seed it as present trigger.
+      if (!booleanLattice.contains(otherExtract.getResult())) {
+        LLVM_DEBUG(llvm::dbgs() << "- Equivalent extract: " << otherExtract
+                                << " for trigger " << trigger << "\n");
+        booleanLattice.insert(
+            {otherExtract.getResult(), getPresentTrigger(triggerIndex)});
+      }
+    });
+  }
+
   // Ensure the wait op destination operands, i.e. the values passed from the
   // past into the present, are the observed values.
   for (auto [operand, blockArg] :
@@ -1015,11 +1088,57 @@ void Deseq::implementRegister(DriveInfo &drive) {
   OpBuilder builder(drive.op);
   auto loc = drive.op.getLoc();
 
+  // If the clock trigger is an extract op inside the process, we need to
+  // create an equivalent extract outside the process. This happens when the
+  // sensitivity list is a bit-select like `@(posedge data[0])`.
+  Value clockValue = drive.clock.clock;
+  if (auto extractOp = clockValue.getDefiningOp<comb::ExtractOp>()) {
+    if (!clockValue.getParentRegion()->isProperAncestor(&process.getBody())) {
+      // The extract is inside the process. Find the underlying probe by
+      // tracing through block arguments.
+      Value input = extractOp.getInput();
+      while (auto arg = dyn_cast<BlockArgument>(input)) {
+        // Find a branch that feeds this block argument
+        Value tracedValue;
+        for (auto *pred : arg.getOwner()->getPredecessors()) {
+          auto *term = pred->getTerminator();
+          if (auto br = dyn_cast<cf::BranchOp>(term)) {
+            tracedValue = br.getDestOperands()[arg.getArgNumber()];
+            break;
+          } else if (auto condBr = dyn_cast<cf::CondBranchOp>(term)) {
+            if (condBr.getTrueDest() == arg.getOwner()) {
+              tracedValue = condBr.getTrueDestOperands()[arg.getArgNumber()];
+              break;
+            }
+            if (condBr.getFalseDest() == arg.getOwner()) {
+              tracedValue = condBr.getFalseDestOperands()[arg.getArgNumber()];
+              break;
+            }
+          } else if (auto waitOp = dyn_cast<WaitOp>(term)) {
+            if (waitOp.getDest() == arg.getOwner()) {
+              tracedValue = waitOp.getDestOperands()[arg.getArgNumber()];
+              break;
+            }
+          }
+        }
+        if (!tracedValue)
+          break;
+        input = tracedValue;
+      }
+      // If we traced to a value outside the process, create an extract there.
+      if (input.getParentRegion()->isProperAncestor(&process.getBody())) {
+        builder.setInsertionPoint(process);
+        clockValue = comb::ExtractOp::create(builder, loc, extractOp.getType(),
+                                             input, extractOp.getLowBit());
+      }
+    }
+  }
+
   // Materialize the clock as a `!seq.clock` value. Insert an inverter for
   // negedge clocks.
-  auto &clockCast = materializedClockCasts[drive.clock.clock];
+  auto &clockCast = materializedClockCasts[clockValue];
   if (!clockCast)
-    clockCast = seq::ToClockOp::create(builder, loc, drive.clock.clock);
+    clockCast = seq::ToClockOp::create(builder, loc, clockValue);
   auto clock = clockCast;
   if (!drive.clock.risingEdge) {
     auto &clockInv = materializedClockInverters[clock];
diff --git a/lib/Dialect/LLHD/Transforms/Mem2Reg.cpp b/lib/Dialect/LLHD/Transforms/Mem2Reg.cpp
@@ -653,8 +653,8 @@ struct Promoter {
   Value resolveSlot(Value projectionOrSlot);
 
   void captureAcrossWait();
-  void captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
-                         Liveness &liveness, DominanceInfo &dominance);
+  void captureValueAcrossWait(Value value, ArrayRef<WaitOp> waitOps,
+                              Liveness &liveness, DominanceInfo &dominance);
 
   void constructLattice();
   void propagateBackward();
@@ -847,7 +847,9 @@ Value Promoter::resolveSlot(Value projectionOrSlot) {
 /// Explicitly capture any probes that are live across an `llhd.wait` as block
 /// arguments and destination operand of that wait. This ensures that replacing
 /// the probe with a reaching definition later on will capture the value of the
-/// reaching definition before the wait.
+/// reaching definition before the wait. Also capture any extract ops of probes
+/// that are live across wait, to allow the extract result (e.g., i1) to be
+/// captured instead of the full probe result (e.g., i8).
 void Promoter::captureAcrossWait() {
   if (region.hasOneBlock())
     return;
@@ -861,53 +863,82 @@ void Promoter::captureAcrossWait() {
   Liveness liveness(region.getParentOp());
 
   SmallVector<WaitOp> crossingWaitOps;
+
+  // Capture probes that are live across wait ops.
   for (auto &block : region) {
     for (auto probeOp : block.getOps<ProbeOp>()) {
       for (auto waitOp : waitOps)
         if (liveness.getLiveness(waitOp->getBlock())->isLiveOut(probeOp))
           crossingWaitOps.push_back(waitOp);
       if (!crossingWaitOps.empty()) {
-        captureAcrossWait(probeOp, crossingWaitOps, liveness, dominance);
+        captureValueAcrossWait(probeOp, crossingWaitOps, liveness, dominance);
+        crossingWaitOps.clear();
+      }
+    }
+  }
+
+  // Capture extract ops of probes that are live across wait ops. This handles
+  // cases like `@(posedge data[0])` where the extract result (i1) should be
+  // captured instead of the probe result (i8), enabling the probe to be
+  // hoisted.
+  for (auto &block : region) {
+    for (auto extractOp : block.getOps<comb::ExtractOp>()) {
+      // Only handle extracts of probe results.
+      if (!extractOp.getInput().getDefiningOp<ProbeOp>())
+        continue;
+      for (auto waitOp : waitOps)
+        if (liveness.getLiveness(waitOp->getBlock())->isLiveOut(extractOp))
+          crossingWaitOps.push_back(waitOp);
+      if (!crossingWaitOps.empty()) {
+        captureValueAcrossWait(extractOp, crossingWaitOps, liveness, dominance);
         crossingWaitOps.clear();
       }
     }
   }
 }
 
-/// Add a probe as block argument to a list of wait ops and update uses of the
-/// probe to use the added block arguments as appropriate. This may insert
-/// additional block arguments in case the probe and added block arguments both
+/// Add a value as block argument to a list of wait ops and update uses of the
+/// value to use the added block arguments as appropriate. This may insert
+/// additional block arguments in case the value and added block arguments both
 /// reach the same block.
-void Promoter::captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
-                                 Liveness &liveness, DominanceInfo &dominance) {
+void Promoter::captureValueAcrossWait(Value value, ArrayRef<WaitOp> waitOps,
+                                      Liveness &liveness,
+                                      DominanceInfo &dominance) {
   LLVM_DEBUG({
-    llvm::dbgs() << "Capture " << probeOp << "\n";
+    llvm::dbgs() << "Capture " << value << "\n";
     for (auto waitOp : waitOps)
       llvm::dbgs() << "- Across " << waitOp << "\n";
   });
 
-  // Calculate the merge points for this probe once it gets promoted to block
+  // Get the block where the value is defined.
+  Block *defBlock = nullptr;
+  if (auto *defOp = value.getDefiningOp())
+    defBlock = defOp->getBlock();
+  else
+    defBlock = cast<BlockArgument>(value).getOwner();
+
+  // Calculate the merge points for this value once it gets promoted to block
   // arguments across the wait ops.
   auto &domTree = dominance.getDomTree(&region);
   llvm::IDFCalculatorBase<Block, false> idfCalculator(domTree);
 
-  // Calculate the set of blocks which will define this probe as a distinct
+  // Calculate the set of blocks which will define this value as a distinct
   // value.
   SmallPtrSet<Block *, 4> definingBlocks;
-  definingBlocks.insert(probeOp->getBlock());
+  definingBlocks.insert(defBlock);
   for (auto waitOp : waitOps)
     definingBlocks.insert(waitOp.getDest());
   idfCalculator.setDefiningBlocks(definingBlocks);
 
-  // Calculate where the probe is live.
+  // Calculate where the value is live.
   SmallPtrSet<Block *, 16> liveInBlocks;
   for (auto &block : region)
-    if (liveness.getLiveness(&block)->isLiveIn(probeOp))
+    if (liveness.getLiveness(&block)->isLiveIn(value))
       liveInBlocks.insert(&block);
   idfCalculator.setLiveInBlocks(liveInBlocks);
 
   // Calculate the merge points where we will have to insert block arguments for
-  // this probe.
+  // this value.
   SmallVector<Block *> mergePointsVec;
   idfCalculator.calculate(mergePointsVec);
   SmallPtrSet<Block *, 16> mergePoints(mergePointsVec.begin(),
@@ -916,29 +947,28 @@ void Promoter::captureAcrossWait(ProbeOp probeOp, ArrayRef<WaitOp> waitOps,
     mergePoints.insert(waitOp.getDest());
   LLVM_DEBUG(llvm::dbgs() << "- " << mergePoints.size() << " merge points\n");
 
-  // Perform a depth-first search starting at the block containing the probe,
+  // Perform a depth-first search starting at the block containing the value,
   // which dominates all its uses. When we encounter a block that is a merge
   // point, insert a block argument.
   struct WorklistItem {
     DominanceInfoNode *domNode;
     Value reachingDef;
   };
   SmallVector<WorklistItem> worklist;
-  worklist.push_back({domTree.getNode(probeOp->getBlock()), probeOp});
+  worklist.push_back({domTree.getNode(defBlock), value});
 
   while (!worklist.empty()) {
     auto item = worklist.pop_back_val();
     auto *block = item.domNode->getBlock();
 
-    // If this block is a merge point, insert a block argument for the probe.
+    // If this block is a merge point, insert a block argument for the value.
     if (mergePoints.contains(block))
-      item.reachingDef =
-          block->addArgument(probeOp.getType(), probeOp.getLoc());
+      item.reachingDef = block->addArgument(value.getType(), value.getLoc());
 
-    // Replace any uses of the probe in this block with the current reaching
+    // Replace any uses of the value in this block with the current reaching
     // definition.
     for (auto &op : *block)
-      op.replaceUsesOfWith(probeOp, item.reachingDef);
+      op.replaceUsesOfWith(value, item.reachingDef);
 
     // If the terminator of this block branches to a merge point, add the
     // current reaching definition as a destination operand.
