Skip to content

[Formal][PropertyAnnotation] Annotating IOG-based invariants#855

Merged
Basmet0 merged 34 commits into
EPFL-LAP:mainfrom
Basmet0:invariant5
May 14, 2026
Merged

[Formal][PropertyAnnotation] Annotating IOG-based invariants#855
Basmet0 merged 34 commits into
EPFL-LAP:mainfrom
Basmet0:invariant5

Conversation

@Basmet0
Copy link
Copy Markdown
Collaborator

@Basmet0 Basmet0 commented Apr 13, 2026

No description provided.

@Jiahui17 Jiahui17 self-requested a review May 5, 2026 11:04
Copy link
Copy Markdown
Member

@Jiahui17 Jiahui17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cosmetic comments (looking into IOG.cpp and HandshakeAnnotateProperties.cpp now..

Comment thread experimental/include/experimental/Support/FormalProperty.h
Comment thread experimental/include/experimental/Support/FormalProperty.h
Comment thread experimental/include/experimental/Support/IOG.h Outdated
Comment thread experimental/include/experimental/Support/IOG.h
Comment thread data/rtl-config-smv.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it is cleaner to add a new operation for this instead of adding more attributes...

Comment on lines +36 to +37
SmallVector<std::string> inputVariables;
inputVariables.push_back("self");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// NOTE: self is ... (see page XX in the nuXmv user manual)
SmallVector<std::string> inputVariables = {"self"};

Comment thread experimental/lib/Support/CreateSmvFormalTestbench.cpp
Comment thread include/dynamatic/Dialect/Handshake/HandshakeOpInternalStateNamer.h
Copy link
Copy Markdown
Member

@Jiahui17 Jiahui17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that we already discussed the IOG part

Comment thread experimental/include/experimental/Support/FormalProperty.h
Comment thread experimental/include/experimental/Support/FormalProperty.h
Comment thread experimental/lib/Support/IOG.cpp Outdated
@Jiahui17
Copy link
Copy Markdown
Member

Jiahui17 commented May 5, 2026

  1. Specialize template " Value toJSON(const std::optional &Opt);"
  2. Example of specialize a template:
    template <>
    struct TypeSystemTraits<ast::ConditionalExpression> : TypeSystemTraitsDefaults {
  3. New handshake operation formal.dead_sink

@Jiahui17
Copy link
Copy Markdown
Member

Jiahui17 commented May 7, 2026

image Pls re-request review when you are done

@Jiahui17
Copy link
Copy Markdown
Member

Jiahui17 commented May 8, 2026

New OP

Instead of terminating IOGs with sinks, they are now terminated with dead buffers
This way, the extra attribute that propagates through handshake -> hw -> python
is no longer necessary
@Basmet0 Basmet0 requested a review from Jiahui17 May 8, 2026 12:43
@Basmet0
Copy link
Copy Markdown
Collaborator Author

Basmet0 commented May 8, 2026

I created the new dead buffer operation instead of using an attribute of the sink operation. I am now doing the merge so that the CI runs

@Jiahui17
Copy link
Copy Markdown
Member

Could you rebase the branch to main?

// consumed by sinks
bool syncOutput = false;

// Determines if the output is modelled as a dead buffer (i.e. accepts one
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the output or all outputs? Technically we can choose whether to have a single output or multiple outputs

namespace {
// This function finds appropriate fork sent states for the consecutive tokens
// invariant: Given the IOG and a set of paths from a start buffer to an end
// buffer, it determines all forks for which:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it determines the sent states of all forks?

if (!iog.contains(forward)) {
continue;
}
Operation *next = forward.getUses().begin()->getOwner();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can shorten this by one forward.getUsers().back() (not super sure now..)?

Comment thread lib/Dialect/Handshake/HandshakeOpInternalStateNamer.cpp
Comment thread experimental/lib/Support/IOG.cpp Outdated
#include "mlir/IR/BuiltinOps.h"

namespace dynamatic {
IOGPathSet::IOGPathSet(const IOG &iog, Operation *startA, Operation *endA)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember to address the remark that I had earier (should this be a class method of IOG itself?)!

// TODO: Handle LoadOp for MC slot
auto slots = loadOp.getInternalSlotStateNamers();
auto *mcOp = loadOp.getAddressResult().getUses().begin()->getOwner();
assert(mcOp);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!mcOp) {
  op->emitError(...);
  llvm::report_fatal_error();
}

// 1. The start buffer is the copied slot of the fork
// 2. The fork is part of a path from start to end along the IOG
std::vector<EagerForkSentNamer> findCopiedSents(const IOG &iog,
const IOGPathSet &pathSet) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The construction of IOGPathSet is actually essential to understanding how this algorithm works
  2. The struct IOGPathSet hides the info that pathSet.start == the start buffer

@Jiahui17
Copy link
Copy Markdown
Member

Good job, thanks!

@Basmet0 Basmet0 merged commit a18bddf into EPFL-LAP:main May 14, 2026
8 checks passed
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