Skip to content

[waveasm] Add NonEmittingOp trait and InsertOp to WaveASM dialect#1209

Open
Hardcode84 wants to merge 1 commit intoiree-org:mainfrom
Hardcode84:waveasm-insert
Open

[waveasm] Add NonEmittingOp trait and InsertOp to WaveASM dialect#1209
Hardcode84 wants to merge 1 commit intoiree-org:mainfrom
Hardcode84:waveasm-insert

Conversation

@Hardcode84
Copy link
Copy Markdown
Contributor

Add NonEmittingOp trait to replace the hardcoded op list in HazardMitigation, and add InsertOp as the dual of ExtractOp for in-place sub-register replacement in wide registers.

NonEmittingOp trait is applied to PackOp, ExtractOp, InsertOp, PrecoloredSRegOp, PrecoloredVRegOp, ConstantOp, DCEProtectOp, and YieldOp. HazardMitigation now checks the trait instead of maintaining a manual isa<> list.

InsertOp replaces a single element in a wide register without extracting all words and rebuilding via PackOp. Liveness and regalloc handle chained inserts and shared-value copies. Used in handleFatRawBufferCast suppress-swizzle path for PackOp-built SRDs.

auto sourceIt = info.ranges.find(source);
while (sourceIt == info.ranges.end()) {
auto defOp = source.getDefiningOp<InsertOp>();
if (!defOp)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe throw a warning or assertion here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, this is expected behavior, chains are always end on non-insert ops (pack).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no but if you dont find the defining op, then shouldnt you raise a warning or assert?

Add NonEmittingOp trait to replace the hardcoded op list in
HazardMitigation, and add InsertOp as the dual of ExtractOp for
in-place sub-register replacement in wide registers.

NonEmittingOp trait is applied to PackOp, ExtractOp, InsertOp,
PrecoloredSRegOp, PrecoloredVRegOp, ConstantOp, DCEProtectOp, and
YieldOp. HazardMitigation now checks the trait instead of maintaining
a manual isa<> list.

InsertOp replaces a single element in a wide register without
extracting all words and rebuilding via PackOp. Liveness and regalloc
handle chained inserts and shared-value copies. Used in
handleFatRawBufferCast suppress-swizzle path for PackOp-built SRDs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
<< "inserted value register class must match source register class: "
"source "
<< vectorType << ", value " << valueType;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This verify function accepts inserted values wider than 1 register word, but LinearScanPass emits s_mov/v_mov with hardcoded size 1. A multi-word insert would only move the first word. Maybe enforce valueSize == 1?

// so new non-emitting ops are covered automatically.
bool isNonEmittingOp(Operation *op) {
return isa<ExtractOp, PackOp, PrecoloredSRegOp, PrecoloredVRegOp, ConstantOp,
DCEProtectOp, YieldOp>(op);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is precoloredAreg op made non emitting when it was not in this list? or was that a bug?

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