Skip to content

[NFC] Delete dead variables and functions#10261

Merged
neildhar merged 1 commit into
mainfrom
neildhar/pr10261
May 8, 2026
Merged

[NFC] Delete dead variables and functions#10261
neildhar merged 1 commit into
mainfrom
neildhar/pr10261

Conversation

@neildhar
Copy link
Copy Markdown
Collaborator

@neildhar neildhar commented May 7, 2026

Delete some dead variables and functions identified by running with
-Wunused.


@neildhar neildhar changed the title Add -Wunused to default Triton builds [WIP] Add -Wunused to default Triton builds May 8, 2026
Copy link
Copy Markdown
Collaborator

@jeffniu-openai jeffniu-openai left a comment

Choose a reason for hiding this comment

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

wow that's a lot of them

@neildhar neildhar force-pushed the neildhar/pr10261 branch 2 times, most recently from 7dce3ae to 7d9f4a1 Compare May 8, 2026 15:02
@neildhar neildhar changed the title [WIP] Add -Wunused to default Triton builds [NFC] Delete dead variables and functions May 8, 2026
@neildhar neildhar force-pushed the neildhar/pr10261 branch from 7d9f4a1 to 8551de7 Compare May 8, 2026 15:02
@neildhar neildhar marked this pull request as ready for review May 8, 2026 15:05
@neildhar neildhar enabled auto-merge (squash) May 8, 2026 15:13
Delete some dead variables and functions identified by running with
`-Wunused`.
@neildhar neildhar force-pushed the neildhar/pr10261 branch from 8551de7 to 98973c7 Compare May 8, 2026 15:40
@neildhar neildhar merged commit f451bad into main May 8, 2026
9 checks passed
@neildhar neildhar deleted the neildhar/pr10261 branch May 8, 2026 16:29
result.reserve(vs.size() * nRepeat);
for (auto v : vs)
for (auto _ : llvm::seq(nRepeat))
for (int i = 0; i < nRepeat; ++i)
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.

:(

shape = to_vector(indexOp.getType().getShape());
offsets = {indexOp.getIndex()};
for (auto i : llvm::seq(std::max<int>(0, shape.size() - 1)))
for (int i = 0, e = std::max<int>(0, shape.size() - 1); i < e; ++i)
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.

Definitely worse imo. You could add [[maybe_unused]] to the i declaration if you want the error to pass cleanly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I apologise, I didn't realize people would have a particular preference. I've generally leaned away from using llvm::seq because it's a less common pattern and can be slightly less efficient (e.g. https://godbolt.org/z/6sEhaP7c6).

That said, neither of those is a particularly compelling reason, so if you'd like I can put up a PR to change these back.

Region *parentRegion = parentBlock->getParent();
Region &newParentRegion =
newInitArgOp->getRegion(parentRegion->getRegionNumber());
newInitArg = parentRegion->getArgument(argIndex);
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.

Should this be newParentRegion.getArgument? I have no idea, but by the naming it looks like what they were going for.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure either, cc @htyu

using ConvertOpToLLVMPattern<triton::DotScaledOp>::ConvertOpToLLVMPattern;

ScaledDotOpConversion(LLVMTypeConverter &converter, int computeCapability,
ScaledDotOpConversion(LLVMTypeConverter &converter, int,
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.

Remove the constructor argument as well?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

My bad, I missed this, will clean up in the next PR.

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.

5 participants