Skip to content

[KTDF][KTDFArch] Record splat mode on ktdf.read_from_fifo - #45

Open
msdataei wants to merge 5 commits into
torch-spyre:mainfrom
msdataei:msd_spalt_fp32
Open

msdataei wants to merge 5 commits into
torch-spyre:mainfrom
msdataei:msd_spalt_fp32

Conversation

@msdataei

@msdataei msdataei commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

When a FIFO delivers fewer than a full SIMD width of elements, a consumer
needs two pieces of information to form the final vector: that a shuffle is
required, and which shuffle to apply. This change encodes both in the IR via a
new optional splat attribute on ktdf.read_from_fifo and a new
getSubSimdLanes accessor on feature::SIMD.

ktdf.read_from_fifo — new optional splat attribute

The op gains OptionalAttr<KTDF_SplatModeAttr>:$splat. Its one mode,
first_subsimd_lane_to_each_subsimd, names the shuffle that fills every
lane of each sub-SIMD group with the value held by its first lane. Naming the
shuffle rather than asserting a bare obligation means a reader learns both that
a shuffle is owed and how to build it.

Because a trailing OptionalAttr produces no default in the generated builder,
a two-argument builder is added for the common case — (result, fifo_slot) —
so existing callers do not have to spell an absent mode.

feature::SIMD — new getSubSimdLanes accessor

The shuffle mode refers to sub-SIMD group width, but the width is an arch fact,
not part of the attribute. feature::SIMD gains getSubSimdLanes() and
getSubSimdLanes(Type), mirroring the existing getLanes pair, over the
sub_simd_lanes map that device files already declare. The verify and test
paths are extended to cover the new key, and the intrinsics.mlir fixture is
updated to declare sub_simd_lanes on its test device. Keeping the width in
the arch avoids two places having to agree about one fact.

Tests

  • read-from-fifo-op.mlir — round-trip checks for splat on both tensor
    and memref result forms.
  • features-invalid.mlir — verifier rejects a non-map value for
    sub_simd_lanes.
  • Features.cpp — SUBCASE("sub_simd_lanes") covers the test predicate:
    absent provider satisfies empty requirement; absent provider fails non-empty
    requirement; narrow provider fails wider requirement; wider provider satisfies
    narrower requirement.

A load unit cannot always splat a single element: when its smallest access
granularity is wider than the source, the hardware loads the whole granule and
splats that, so one element per sub-SIMD group ends up live rather than one for
the whole vector.  Give the IR the two things a consumer needs to finish that.

ktdf.read_from_fifo gains an optional `splat` attribute, #ktdf.splat<...>, whose
one mode -- first_subsimd_lane_to_all_subsimd_lanes -- names the shuffle that
closes the gap.  It names the shuffle rather than asserting that something is
owed, so a reader learns what to build and not only that it must build.  It is
inherent, not discardable, because it changes what the op means: a read carrying
it hands out a value the shuffle has not been applied to yet.  The attribute
survives onto a memref-typed re-read of the same slot, since reading a buffer
performs no shuffle.

The mode carries no width.  A sub-SIMD group is as wide as the compute unit
says, so feature::SIMD gains getSubSimdLanes, mirroring getLanes, over the
sub_simd_lanes map device files already declare and nothing read.  Stating the
width in a device pattern instead would make two places have to agree about one
arch fact.

A trailing-optional attribute has no default in the generated builder, so
read_from_fifo also gains the two-argument builder every existing caller uses:
a read that owes no shuffle should not have to spell an absent mode.

Signed-off-by: Masoud Ataei <Masoud.Ataei.Jaliseh@ibm.com>
@KFAFSP

KFAFSP commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

If we're going forward with this, I'd like to note that the new attribute does not have matching changes in the corresponding feature test function for feature::SIMD. There should also be a test of that in the unittests.

feature::SIMD::test() silently ignored sub_simd_lanes requirements.
Add the provider->=required check mirroring the lanes block, and add a
matching SUBCASE("sub_simd_lanes") in the Features unit test.

Signed-off-by: Masoud Ataei <Masoud.Ataei.Jaliseh@ibm.com>
@msdataei msdataei changed the title [KTDF][KTDFArch] Say on a read that a splat was left half done [KTDF][KTDFArch] Record a half-done splat on ktdf.read_from_fifo Sep 24, 2026
Signed-off-by: Masoud Ataei <Masoud.Ataei.Jaliseh@ibm.com>
Comment thread include/dataflow-scheduler/Dialect/KTDF/KTDF.td Outdated
Comment thread include/dataflow-scheduler/Dialect/KTDF/KTDF.td Outdated
Comment thread include/dataflow-scheduler/Dialect/KTDF/KTDFAttributes.td Outdated
Comment thread include/dataflow-scheduler/Dialect/KTDF/KTDFAttributes.td Outdated
Comment thread include/dataflow-scheduler/Dialect/KTDF/KTDFEnums.td Outdated
@msdataei msdataei changed the title [KTDF][KTDFArch] Record a half-done splat on ktdf.read_from_fifo [KTDF][KTDFArch] Record splat mode on ktdf.read_from_fifo Sep 24, 2026
Signed-off-by: Masoud Ataei <Masoud.Ataei.Jaliseh@ibm.com>
Signed-off-by: Masoud Ataei <Masoud.Ataei.Jaliseh@ibm.com>
@msdataei

msdataei commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

the changes in this PR is used in the PR torch-spyre/dataflow-scheduler#170

@msdataei

Copy link
Copy Markdown
Contributor Author

Upstream PR: #4697

static constexpr StringLiteral kSplatAttrName = "splat";
static constexpr StringLiteral kZeroPadAttrName = "zero_pad";
static constexpr StringLiteral kLanesAttrName = "lanes";
static constexpr StringLiteral kSubSimdLanesAttrName = "sub_simd_lanes";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Keeping with the style of the existing attribute, this should be called sub_lanes I think.

There was also the open design question on whether SIMD lanes could be multidimensional, i.e., f16 = 64x64 to indicate matrix-type accelerators. I don't think this is quite the way to go, but it would be good to know whether sub SIMD lanes would be any different in that regard.

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.

3 participants