-
Notifications
You must be signed in to change notification settings - Fork 2
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
Scheduling revamp #53
Conversation
…ling now recognizes constant input operations.
…tencyConfig-objects were added; tests were updated according to new features.
if (bool(noQuestion) == bool(noAttr)) { | ||
return parser.emitError(latencyLoc) << "'" << result.name.getStringRef() << | ||
"' op requires either '?' or valid latency specified"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
parser.parseColon() || | ||
parser.parseCustomTypeWithFallback(firstRawTypes[0]) || | ||
parser.parseRParen() || | ||
parser.parseColon() || | ||
parser.parseCustomTypeWithFallback(resRawTypes[0]) || | ||
parser.parseOptionalAttrDict(result.attributes)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
if (bool(noQuestion) == bool(noAttr)) { | ||
return parser.emitError(latencyLoc) << "'" << result.name.getStringRef() << | ||
"' op requires either '?' or valid latency specified"; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indent.
Also looks like copy-paste of 197-205 lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed indent. Corresponding function instead of copy-paste will be added later in this MR.
if (parser.parseOperand(secondRawOperands[0]) || | ||
parser.parseColon() || | ||
parser.parseCustomTypeWithFallback(secondRawTypes[0]) || | ||
parser.parseRParen() || | ||
parser.parseColon() || | ||
parser.parseCustomTypeWithFallback(resRawTypes[0]) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indent.
Also looks similar to complex expression from 219-224 lines. Can we not to copy-paste them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed indent. Corresponding function instead of copy-paste will be added later in this MR.
// | ||
// Part of the Utopia HLS Project, under the Apache License v2.0 | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// Copyright 2024 ISP RAS (http://www.ispras.ru) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2024 -> 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Graph::Graph(ModuleOp module) | ||
: Graph(mlir::utils::findFirstOccurence<KernelOp>(module)) {} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra empty line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
return (isFloat) ? Ops::NEQ_FLOAT : Ops::NEQ_INT; | ||
} | ||
|
||
assert(false && "Shouldn't reach this"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-informative; can we print useful information about op
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed to ignore some duplicating code at Tablegen-related components.
This Merge Request introduces the following changes:
dfcir.latency
operation added;%0 = dfcir.add[2] (...)
, which means 2 pipeline stages for addition, and%0 = dfcir.add[?] (...)
if the operation wasn't scheduled yet);--out-scheduled-dfcir <PATH>
option was introduced.Remaining tasks:
dfcir.const
anddfcir.scalarInput
recognition in scheduling;dfcir.offset
deletion after scheduling;docs/latency_config.md
.