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.

Sign up for GitHub

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

Remove dep. on runtime workaround in binary.cpp #2104

Merged
merged 2 commits into from
Feb 6, 2025
Merged

Remove dep. on runtime workaround in binary.cpp #2104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4dcb3b
Remove dep. on runtime workaround in `binary.cpp`
ctodTT Feb 4, 2025
26a66d2
Remove all instances of `defaultStrideComputation`
ctodTT Feb 6, 2025
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
15 changes: 2 additions & 13 deletions runtime/include/tt/runtime/detail/workarounds.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ struct Env {
#endif
get(bool maxpool2dPreshard = true, bool swapBinaryOperands = true,
bool readUpdateIndexFromDeviceForKVCache = true,
bool defaultStrideComputation = true,
bool toLayoutAPIAssumeSingleChip = true,
bool usePaddingPairSignatureWithQueueId = true)
#if defined(TT_RUNTIME_WORKAROUNDS) && TT_RUNTIME_WORKAROUNDS == 1
;
#else
{
return Env(true, true, true, true, true, true);
return Env(true, true, true, true, true);
}
#endif
// TODO(bug #855): Ideally we should have an op that preshards for maxpool2d
Expand All @@ -42,13 +41,6 @@ struct Env {
// to be able to pluck this update index from a runtime tensor.
bool readUpdateIndexFromDeviceForKVCache;

// TODO(bug #2045): Our current stride calculation is incorrect for tilized
// tensors. The current solution is to remove stride entirely from the
// flatbuffer and calculate the stride in runtime assuming using the default
// method ignoring details like grid, layout etc. Once we have a more
// sophisticated way for handling this, we can remove this workaround.
bool defaultStrideComputation;

// TODO(bug #1778): We currently don't have device grid information (mesh
// shape, offset) in the flatbuffer TensorDesc nor in the mlir LayoutAttr. We
// need to add this information to the tensorDesc so that the runtime toLayout
Expand All @@ -70,13 +62,12 @@ struct Env {
private:
constexpr Env(bool maxpool2dPreshard, bool swapBinaryOperands,
bool readUpdateIndexFromDeviceForKVCache,
bool defaultStrideComputation, bool toLayoutAPIAssumeSingleChip,
bool toLayoutAPIAssumeSingleChip,
bool usePaddingPairSignatureWithQueueId)
: maxpool2dPreshard(maxpool2dPreshard),
swapBinaryOperands(swapBinaryOperands),
readUpdateIndexFromDeviceForKVCache(
readUpdateIndexFromDeviceForKVCache),
defaultStrideComputation(defaultStrideComputation),
toLayoutAPIAssumeSingleChip(toLayoutAPIAssumeSingleChip),
usePaddingPairSignatureWithQueueId(usePaddingPairSignatureWithQueueId) {
}
Expand All @@ -91,8 +82,6 @@ inline std::ostream &operator<<(std::ostream &os, const Env &env) {
os << "\t"
<< "readUpdateIndexFromDeviceForKVCache: "
<< env.readUpdateIndexFromDeviceForKVCache << "\n";
os << "\t"
<< "defaultStrideComputation: " << env.defaultStrideComputation << "\n";
os << "\t"
<< "toLayoutAPIAssumeSingleChip: " << env.toLayoutAPIAssumeSingleChip
<< "\n";
Expand Down
10 changes: 5 additions & 5 deletions runtime/lib/binary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "flatbuffers/idl.h"

#include "tt/runtime/detail/logger.h"
#include "tt/runtime/detail/workarounds.h"
#include "tt/runtime/types.h"
#include "tt/runtime/utils.h"
#include "ttmlir/Target/Common/system_desc_bfbs_generated.h"
Expand Down Expand Up @@ -39,10 +38,11 @@ static std::string asJson(void const *fbb, uint8_t const *binarySchema,

static std::vector<uint32_t>
calculateStride(std::vector<uint32_t> const &shape) {
LOG_ASSERT(
workaround::Env::get().defaultStrideComputation,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're not using this anymore, maybe we should remove the workaround entirely from workaround env, ttrt etc., and move the TODO comment here

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! let me know if I missed anything, but otherwise will probably merge once CI passes

"Stride currently removed from flatbuffer thus defaultStrideComputation"
"workaround must be enabled");
// TODO(bug #2045): Our current stride calculation is incorrect for tilized
// tensors. The current solution is to remove stride entirely from the
// flatbuffer and calculate the stride in runtime assuming using the default
// method ignoring details like grid, layout etc. Once we have a more
// sophisticated way for handling this, we can remove this workaround.
LOG_ASSERT(!shape.empty());
std::vector<uint32_t> stride(shape.size(), 1);
for (size_t i = shape.size() - 1; i > 0; i--) {
Expand Down
3 changes: 1 addition & 2 deletions runtime/lib/common/workarounds.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ namespace tt::runtime::workaround {
#if defined(TT_RUNTIME_WORKAROUNDS) && TT_RUNTIME_WORKAROUNDS == 1
const Env &Env::get(bool maxpool2dPreshard, bool swapBinaryOperands,
bool readUpdateIndexFromDeviceForKVCache,
bool defaultStrideComputation,
bool toLayoutAPIAssumeSingleChip,
bool usePaddingPairSignatureWithQueueId) {
static const Env config(maxpool2dPreshard, swapBinaryOperands,
readUpdateIndexFromDeviceForKVCache,
defaultStrideComputation, toLayoutAPIAssumeSingleChip,
toLayoutAPIAssumeSingleChip,
usePaddingPairSignatureWithQueueId);
return config;
}
Expand Down
8 changes: 0 additions & 8 deletions runtime/tools/python/ttrt/common/run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,6 @@ def initialize_api():
choices=[True, False],
help="disable read update index for kv cache workaround",
)
Run.register_arg(
name="--disable-default-stride-computation",
type=bool,
default=False,
choices=[True, False],
help="disable runtime default stride computation workaround",
)
Run.register_arg(
name="--disable-to-layout-api-assume-single-chip",
type=bool,
Expand Down Expand Up @@ -431,7 +424,6 @@ def convert_input_layouts(device, inputs, fbb, program_index):
not self["--disable-maxpool2d-preshard"],
not self["--disable-swap-binary-operands"],
not self["--disable-read-update-index-for-kv-cache"],
not self["--disable-default-stride-computation"],
not self["--disable-to-layout-api-assume-single-chip"],
not self["--disable-pad-op-padding-pairs-signature"],
)
Expand Down
Loading