Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL] Subword assignment support via rewriting to read-modify-write #3658

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
9a16fad
Eliminate bit-index during expand-whens
zyedidia Jul 28, 2022
729a577
Fix builder insertion point ordering
zyedidia Jul 28, 2022
8bd876a
Move transformation to separate function
zyedidia Jul 28, 2022
98bc0c4
Initialization checking
zyedidia Jul 29, 2022
bf0853e
Comments and clarify implementation
zyedidia Jul 29, 2022
c61b5dd
Memoize initialization reachability check
zyedidia Jul 29, 2022
1a19152
Fix null operand in comb cycles
zyedidia Aug 1, 2022
56114a2
Single int subscript support
zyedidia Aug 1, 2022
68fa8a0
Range index parsing
zyedidia Aug 1, 2022
2e564f8
Move canonicalizations to firrtl folds
zyedidia Aug 2, 2022
8dfee61
Add some subword assignment tests
zyedidia Aug 2, 2022
9aa4a56
Fix initialization checking and add test
zyedidia Aug 3, 2022
0940fba
Fix for SInt
zyedidia Aug 3, 2022
8632e5f
Remove redundant test
zyedidia Aug 3, 2022
6ffc360
Fix typo
zyedidia Aug 3, 2022
da54006
Improve wording around uninitialized wires
zyedidia Aug 3, 2022
dbae003
Auto format
zyedidia Aug 3, 2022
2ddfb84
Make flow same as parent op
zyedidia Aug 3, 2022
f4e231b
Add parser and flow tests
zyedidia Aug 3, 2022
258cfa0
Restructure parser to avoid needing lookahead
zyedidia Aug 4, 2022
4408b2f
Improve comments
zyedidia Aug 4, 2022
83f2a56
Update more comments
zyedidia Aug 4, 2022
a95efb9
Skip subword initialization checking entirely if unused
zyedidia Aug 4, 2022
e505573
Use replaceOpWithNewOpAndCopyName
zyedidia Aug 5, 2022
a6dfb07
Remove braces for single line if statements
zyedidia Aug 5, 2022
f430e94
Add e2e test
zyedidia Aug 5, 2022
824ee37
Improve canonicalizations
zyedidia Aug 12, 2022
0030d29
Test comment
zyedidia Aug 12, 2022
23dd0f0
Comment wrapping
zyedidia Aug 12, 2022
b6e781b
Format
zyedidia Aug 12, 2022
6cdd494
Use DenseSet for cache
zyedidia Aug 12, 2022
f45729f
Remove debug
zyedidia Aug 12, 2022
b6c0f8d
Remove recursion in initialization checking
zyedidia Aug 12, 2022
6ef1bd5
Remove dead code from test
zyedidia Aug 12, 2022
a34fd1b
Add canonicalization tests
zyedidia Aug 12, 2022
b4f7851
Add LHS bits canonicalization
zyedidia Aug 12, 2022
3bebbf6
Remove recursion in canonicalizeBits
zyedidia Aug 15, 2022
134e1ad
Format
zyedidia Aug 15, 2022
da7d08e
Fix bug in initialization checking with subwordUninit set
zyedidia Aug 22, 2022
0ae0295
Auto format
zyedidia Aug 22, 2022
3929bd2
Tidy
zyedidia Aug 22, 2022
61994ba
Handle bits in getDeclarationKind
zyedidia Aug 23, 2022
fcee9dc
Remove canonicalizations from FIRRTLFolds
zyedidia Aug 24, 2022
6e7d49c
Undo canonicalization tests now that they have been removed
zyedidia Aug 24, 2022
d7d90ee
Canonicalize through asSInt
zyedidia Aug 29, 2022
b1a1af2
Comment
zyedidia Aug 29, 2022
2bbbbab
Helpful error message for CHIRRTL mem bit-index
zyedidia Aug 31, 2022
a60e178
Add better sint test
zyedidia Aug 31, 2022
a429b0e
Add chirrtl mem error test
zyedidia Sep 1, 2022
4e3230c
Revert "Remove canonicalizations from FIRRTLFolds"
zyedidia Sep 1, 2022
01ba8fc
Revert "Undo canonicalization tests now that they have been removed"
zyedidia Sep 1, 2022
3acda81
Pass by reference
zyedidia Sep 2, 2022
0cd42fe
Additional canonicalization and fold for optimizing invalid values
zyedidia Sep 2, 2022
b25575e
Add test for invalid value optimization
zyedidia Sep 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion lib/Dialect/FIRRTL/FIRRTLFolds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1096,14 +1096,51 @@ OpFoldResult BitsPrimOp::fold(ArrayRef<Attribute> operands) {
LogicalResult BitsPrimOp::canonicalize(BitsPrimOp op,
PatternRewriter &rewriter) {
auto *inputOp = op.getInput().getDefiningOp();
if (!inputOp)
return failure();
// bits(bits(x, ...), ...) -> bits(x, ...).
if (auto innerBits = dyn_cast_or_null<BitsPrimOp>(inputOp)) {
if (auto innerBits = dyn_cast<BitsPrimOp>(inputOp)) {
auto newLo = op.getLo() + innerBits.getLo();
auto newHi = newLo + op.getHi() - op.getLo();
replaceOpWithNewOpAndCopyName<BitsPrimOp>(
rewriter, op, innerBits.getInput(), newHi, newLo);
return success();
}
// bits(mux(sel, a, b), ...) -> mux(sel, bits(a, ...), bits(b, ...)).
if (auto mux = dyn_cast<MuxPrimOp>(inputOp)) {
auto bitsHigh = rewriter.create<BitsPrimOp>(op->getLoc(), mux.getHigh(),
op.getHi(), op.getLo());
auto bitsLow = rewriter.create<BitsPrimOp>(op->getLoc(), mux.getLow(),
op.getHi(), op.getLo());
replaceOpWithNewOpAndCopyName<MuxPrimOp>(rewriter, op, mux.getSel(),
bitsHigh, bitsLow);
return success();
}
// bits(cat(a, b), ...) -> bits(a, ...), or bits(b, ...), or cat(bits(a, ...),
zyedidia marked this conversation as resolved.
Show resolved Hide resolved
// bits(b, ...)).
if (auto cat = dyn_cast<CatPrimOp>(inputOp)) {
auto rhsT = cat.getRhs().getType().cast<IntType>();
if (!rhsT.hasWidth())
return failure();
uint32_t lhsLo = rhsT.getWidth().value();
uint32_t rhsHi = lhsLo - 1;
if (op.getLo() >= lhsLo) {
// Only indexing the lhs.
replaceOpWithNewOpAndCopyName<BitsPrimOp>(
rewriter, op, cat.getLhs(), op.getHi() - lhsLo, op.getLo() - lhsLo);
} else if (op.getHi() <= rhsHi) {
// Only indexing the rhs.
replaceOpWithNewOpAndCopyName<BitsPrimOp>(rewriter, op, cat.getRhs(),
op.getHi(), op.getLo());
} else {
auto bitsLhs = rewriter.create<BitsPrimOp>(op->getLoc(), cat.getLhs(),
op.getHi() - lhsLo, 0);
auto bitsRhs = rewriter.create<BitsPrimOp>(op->getLoc(), cat.getRhs(),
rhsHi, op.getLo());
replaceOpWithNewOpAndCopyName<CatPrimOp>(rewriter, op, bitsLhs, bitsRhs);
}
return success();
}
return failure();
}

Expand Down
4 changes: 2 additions & 2 deletions lib/Dialect/FIRRTL/FIRRTLOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ Flow firrtl::foldFlow(Value val, Flow accumulatedFlow) {
return foldFlow(op.getInput(),
op.isFieldFlipped() ? swap() : accumulatedFlow);
})
.Case<SubindexOp, SubaccessOp>(
.Case<SubindexOp, SubaccessOp, BitsPrimOp>(
[&](auto op) { return foldFlow(op.getInput(), accumulatedFlow); })
// Registers, Wires, and behavioral memory ports are always Duplex.
.Case<RegOp, RegResetOp, WireOp, MemoryPortOp>(
Expand All @@ -153,7 +153,7 @@ DeclKind firrtl::getDeclarationKind(Value val) {

return TypeSwitch<Operation *, DeclKind>(op)
.Case<InstanceOp>([](auto) { return DeclKind::Instance; })
.Case<SubfieldOp, SubindexOp, SubaccessOp>(
.Case<SubfieldOp, SubindexOp, SubaccessOp, BitsPrimOp>(
[](auto op) { return getDeclarationKind(op.getInput()); })
.Default([](auto) { return DeclKind::Other; });
}
Expand Down
85 changes: 58 additions & 27 deletions lib/Dialect/FIRRTL/Import/FIRParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,7 @@ ParseResult FIRStmtParser::parseExpImpl(Value &result, const Twine &message,
///
/// exp ::= exp '.' fieldId
/// ::= exp '[' intLit ']'
/// ::= exp '[' intLit ':' intLit ']'
/// XX ::= exp '.' DoubleLit // TODO Workaround for #470
/// ::= exp '[' exp ']'
ParseResult FIRStmtParser::parseOptionalExpPostscript(Value &result) {
Expand All @@ -1414,7 +1415,9 @@ ParseResult FIRStmtParser::parseOptionalExpPostscript(Value &result) {
continue;
}

// Subindex: exp ::= exp '[' intLit ']' | exp '[' exp ']'
// Subindex: exp ::= exp '[' intLit ']'
// ::= exp '[' intLit ':' intLit ']'
// ::= exp '[' exp ']'
if (consumeIf(FIRToken::l_square)) {
if (getToken().isAny(FIRToken::integer, FIRToken::string)) {
if (parsePostFixIntSubscript(result))
Expand Down Expand Up @@ -1481,51 +1484,79 @@ ParseResult FIRStmtParser::parsePostFixFieldId(Value &result) {
return success();
}

/// exp ::= exp '[' intLit ']'
/// exp ::= exp '[' intLit ']' | exp '[' intLit ':' intLit ']'
///
/// The "exp '['" part of the production has already been parsed.
///
ParseResult FIRStmtParser::parsePostFixIntSubscript(Value &result) {
auto loc = getToken().getLoc();
int32_t indexNo;
if (parseIntLit(indexNo, "expected index") ||
parseToken(FIRToken::r_square, "expected ']'"))
if (parseIntLit(indexNo, "expected index"))
return failure();

if (indexNo < 0)
return emitError(loc, "invalid index specifier"), failure();

// Make sure the index expression is valid and compute the result type for the
// expression.
// TODO: This should ideally be folded into a `tryCreate` method on the
// builder (https://llvm.discourse.group/t/3504).
NamedAttribute attrs = {getConstants().indexIdentifier,
builder.getI32IntegerAttr(indexNo)};
auto resultType = SubindexOp::inferReturnType({result}, attrs, {});
if (!resultType) {
// Emit the error at the right location. translateLocation is expensive.
(void)SubindexOp::inferReturnType({result}, attrs, translateLocation(loc));
return failure();
}

// Check if we already have created this Subindex op.
auto &value = moduleContext.getCachedSubaccess(result, indexNo);
if (value) {
result = value;
if (result.getType().isa<IntType>()) {
NamedAttrList bitAttrs;
bitAttrs.append(getConstants().hiIdentifier,
builder.getI32IntegerAttr(indexNo));
if (consumeIf(FIRToken::colon)) {
if (parseIntLit(indexNo, "expected low index"))
return failure();
}
if (parseToken(FIRToken::r_square, "expected ']'"))
return failure();
bitAttrs.append(getConstants().loIdentifier,
builder.getI32IntegerAttr(indexNo));

auto resultType = BitsPrimOp::inferReturnType({result}, bitAttrs, {});
if (!resultType) {
(void)BitsPrimOp::inferReturnType({result}, bitAttrs,
translateLocation(loc));
return failure();
}
locationProcessor.setLoc(loc);
OpBuilder::InsertionGuard guard(builder);
builder.setInsertionPointAfterValue(result);
auto op = builder.create<BitsPrimOp>(resultType, result, bitAttrs);
result = op.getResult();
return success();
}
} else {
if (parseToken(FIRToken::r_square, "expected ']'"))
return failure();
NamedAttribute attrs = {getConstants().indexIdentifier,
builder.getI32IntegerAttr(indexNo)};
auto resultType = SubindexOp::inferReturnType({result}, attrs, {});
if (!resultType) {
(void)SubindexOp::inferReturnType({result}, attrs,
translateLocation(loc));
return failure();
}
// Check if we already have created this Subindex op.
auto &value = moduleContext.getCachedSubaccess(result, indexNo);
if (value) {
result = value;
return success();
}

// Create the result operation, inserting at the location of the declaration.
// This will cache the subindex operation for further uses.
locationProcessor.setLoc(loc);
OpBuilder::InsertionGuard guard(builder);
builder.setInsertionPointAfterValue(result);
auto op = builder.create<SubindexOp>(resultType, result, attrs);
// Create the result operation, inserting at the location of the
// declaration. This will cache the subindex operation for further uses.
locationProcessor.setLoc(loc);
OpBuilder::InsertionGuard guard(builder);
builder.setInsertionPointAfterValue(result);

// Insert the newly creatd operation into the cache.
value = op.getResult();
auto op = builder.create<SubindexOp>(resultType, result, attrs);

// Insert the newly created operation into the cache.
value = op.getResult();
result = value;
}

result = value;
return success();
}

Expand Down
Loading