Skip to content

Add -Wunused to default Triton builds#10267

Draft
neildhar wants to merge 1 commit into
mainfrom
neildhar/pr10267
Draft

Add -Wunused to default Triton builds#10267
neildhar wants to merge 1 commit into
mainfrom
neildhar/pr10267

Conversation

@neildhar
Copy link
Copy Markdown
Collaborator

@neildhar neildhar commented May 8, 2026

Add -Wunused in the default build mode. This catches dead variables,
fields, and functions.

Setting it only in this mode avoids issues when switching between modes
(e.g. release builds where vars only used in asserts become unused).


@neildhar neildhar changed the base branch from main to neildhar/pr10261 May 8, 2026 15:02
@neildhar neildhar force-pushed the neildhar/pr10267 branch from f3f2d85 to a0a35b0 Compare May 8, 2026 15:02
Comment thread third_party/amd/lib/TritonAMDGPUToLLVM/TensorPtrOpsToLLVM.cpp Outdated
@neildhar neildhar force-pushed the neildhar/pr10267 branch from a0a35b0 to 2bbfacc Compare May 8, 2026 15:40
@neildhar neildhar force-pushed the neildhar/pr10261 branch from 8551de7 to 98973c7 Compare May 8, 2026 15:41
Base automatically changed from neildhar/pr10261 to main May 8, 2026 16:29
@peterbell10
Copy link
Copy Markdown
Contributor

IMO adding warnings is kind of pointless because nobody looks at the logs. We should enable it as an error in CI if we want to keep it clean.

@neildhar
Copy link
Copy Markdown
Collaborator Author

neildhar commented May 8, 2026

adding warnings is kind of pointless because nobody looks at the logs

We also enable -Werror by default, so this will be an error.

Copy link
Copy Markdown
Contributor

@peterbell10 peterbell10 left a comment

Choose a reason for hiding this comment

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

Oh, somehow I didn't realise. I guess we must not have many warnings enabled then.

const TypeConverter *typeConverter,
ConversionPatternRewriter &rewriter,
Location loc) {
static inline Value getOffsetedBase(Value v, gpu::MemDescType memDescTy,
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.

static inline is a bit of an oxymoron. Should just be inline.

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.

static here is not completely redundant. It still gives the function internal linkage. In practice, this likely only really makes a difference when Triton plugins are enabled, since it will affect whether the function gets exported by libtriton.so.

Comment thread CMakeLists.txt
set(CMAKE_C_FLAGS_TRITONRELBUILDWITHASSERTS "-O2 -g")
set(CMAKE_CXX_FLAGS_TRITONRELBUILDWITHASSERTS "-O2 -g")
set(CMAKE_C_FLAGS_TRITONRELBUILDWITHASSERTS "-O2 -g -Wunused")
set(CMAKE_CXX_FLAGS_TRITONRELBUILDWITHASSERTS "-O2 -g -Wunused")
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.

Why only for this build type?

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.

This is mostly so we don't break release builds with assertions disabled, since many variables may become unused if they are only used inside asserts.

Add `-Wunused` in the default build mode. This catches dead variables,
fields, and functions.

Setting it only in this mode avoids issues when switching between modes
(e.g. release builds where vars only used in asserts become unused).
@neildhar neildhar force-pushed the neildhar/pr10267 branch from 2bbfacc to 6ae9483 Compare May 11, 2026 15:23
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