Skip to content

Guard transaction-to-MLIR conversion when no parent aie.device exists#3122

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-code-review-comment
Closed

Guard transaction-to-MLIR conversion when no parent aie.device exists#3122
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-code-review-comment

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

convertTransactionOpsToMLIR assumed the builder insertion point was always nested under aie.device and unconditionally dereferenced the derived DeviceOp. This change makes that failure mode explicit and returns a clean LogicalResult failure with a diagnostic instead of crashing.

  • What changed

    • Preserve the existing search for a parent DeviceOp from the current insertion point.
    • Add an explicit null check after the ancestor lookup.
    • Emit a diagnostic on the enclosing parent op when no aie.device is found.
    • Return failure() before any use of device.getLoc() / device.getBody().
  • Behavioral impact

    • Valid call sites continue to behave unchanged.
    • Invalid insertion points now fail deterministically with a clear error instead of triggering a null dereference.
  • Example

    Operation *parentOp = builder.getBlock()->getParentOp();
    DeviceOp device = llvm::dyn_cast<DeviceOp>(parentOp);
    if (!device)
      device = parentOp->getParentOfType<DeviceOp>();
    if (!device) {
      parentOp->emitError(
          "expected insertion point to be nested under an aie.device op");
      return failure();
    }

Copilot AI changed the title [WIP] Fix code for review comment Guard transaction-to-MLIR conversion when no parent aie.device exists May 29, 2026
Copilot AI requested a review from fifield May 29, 2026 22:30
@fifield
Copy link
Copy Markdown
Collaborator

fifield commented May 29, 2026

wrong branch

@fifield fifield closed this May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants