Skip to content

[AscendNPU-IR][A5] Added A5 support for FlipOp#911

Open
Ismayil06 wants to merge 1 commit intotile-ai:npuir-devfrom
Ismayil06:a5-vflip
Open

[AscendNPU-IR][A5] Added A5 support for FlipOp#911
Ismayil06 wants to merge 1 commit intotile-ai:npuir-devfrom
Ismayil06:a5-vflip

Conversation

@Ismayil06
Copy link
Copy Markdown

[AscendNPU-IR][A5] Added A5 support for FlipOp
- Changed hivm::VFlipOp to hfusion::FlipOp
- Added pytest for FlipOp

@github-actions
Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the TileLang project.

Please remember to run bash format.sh in the root directory of the project to ensure your changes are properly linted and formatted. This will help ensure your contribution passes the format check.

We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work!

🚀

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the VflipCodegen implementation in codegen_npuir_dev.cc to utilize mlir::hfusion::FlipOp and ReshapeCastAndInsertSlice, and introduces a new test file to verify the functionality. A redundant double assignment was found in the VflipCodegen function that should be simplified.

Comment thread src/target/codegen_npuir_dev.cc Outdated
builder.create<mlir::hivm::VFlipOp>(builder.getUnknownLoc(), TypeRange{}, src,
dst, npuirop.axis);
Value src = GenExtractSliceFromRegion(npuirop.src, npuirop.src_range);
Value dst = dst = GetVarValue(npuirop.dst);
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.

high

There is a typo in the assignment: Value dst = dst = .... This redundant double assignment is likely a copy-paste error and should be corrected to a single assignment to avoid potential issues with using an uninitialized variable.

  Value dst = GetVarValue(npuirop.dst);

@CeleNewYear CeleNewYear added AscendNPU-IR Want to merge into the npuir branch A5 labels Apr 25, 2026
a = T.alloc_shared((M, N), dtype)
b = T.alloc_shared((M, N), dtype)
T.copy(A, a)
T.npuir_flip(a, b, axis)
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 test case do not trigger GenExtractSliceFromRegion, if don't support, please use GetVarValue

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.

also, Please use flip

@Ismayil06 Ismayil06 force-pushed the a5-vflip branch 2 times, most recently from 69acf78 to e696969 Compare May 4, 2026 09:29
auto loc = builder.getUnknownLoc();
auto srcTy = src.getType().cast<RankedTensorType>();

auto flipOp = builder.create<mlir::hfusion::FlipOp>(loc, srcTy, src, static_cast<uint64_t>(npuirop.axis));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If a negative axis is passed upstream (e.g., axis=-1 in Python), the implicit int64_t → u_int64_t conversion would produce a huge unsigned value. Add negative axis legalization in the NpuirFlip constructor (converting negative to positive), or add an ICHECK(npuirop.axis >= 0) assertion in VflipCodegen .

B_VEC = T.alloc_shared((M, N), dtype)

T.copy(A, A_VEC)
T.npuir_flip(A_VEC, B_VEC, axis)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Replace T.npuir_flip with T.flip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A5 AscendNPU-IR Want to merge into the npuir branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants