Skip to content

[AIROCMLIR-390] clang-format: break .td DAG operands onto separate lines#2377

Open
bogdan-petkovic wants to merge 7 commits into
developfrom
bpetkovi/clang-format-td
Open

[AIROCMLIR-390] clang-format: break .td DAG operands onto separate lines#2377
bogdan-petkovic wants to merge 7 commits into
developfrom
bpetkovi/clang-format-td

Conversation

@bogdan-petkovic
Copy link
Copy Markdown
Contributor

@bogdan-petkovic bogdan-petkovic commented May 18, 2026

Motivation

Resolves #1936.
Recent versions of git-clang-format include .td (TableGen) files in their default extension list. The repo's .clang-format only sets BasedOnStyle: LLVM, which uses TableGen defaults that pack multiple DAG operands onto a single line.
The result is hard to read; e.g. before this PR, commonConvArgs in mlir/include/mlir/Dialect/Rock/IR/RockOps.td looked like:

dag commonConvArgs = (ins TensorOrMemRefOf:$filter, TensorOrMemRefOf:$input, TensorOrMemRefOf:$output, OptionalAttr<Rock_GemmFeaturesAttr>:$features, OptionalAttr:$derivedBlockSize, OptionalAttr:$gridSize, IndexArrayLength<4>:$padding, IndexArrayLength<2>:$strides, ...

The goal of this PR is to make git clang-format origin/develop produce one operand per line for (ins ...) / (outs ...) DAGs.

Technical Details

Configuration changes:

  • .clang-format and mlir/.clang-format: add two clang-format options introduced in clang-format 19:
    • TableGenBreakInsideDAGArg: BreakElements — break after each DAG element, aligned to the first element.
    • TableGenBreakingDAGArgOperators: [ins, outs] — restrict the breaking behavior to DAGs starting with ins or outs, so unrelated DAG expressions are not reflowed.
  • .githooks/pre-commit: extend the staged-file regex to include .td, so staged TableGen changes get formatted at commit time alongside C/C++.
  • CONTRIBUTING.md: note that clang-format >= 19 is now required for this repo (CI is already on LLVM 20 via mlir/utils/jenkins/Dockerfile). Mention that .td files are covered by git clang-format origin/develop.

NFC follow-up commit:

  • Bulk reformat of all 13 .td files under mlir/ that contain (ins ...) / (outs ...) DAGs, using clang-format-20 -i. Pure formatting, no semantic changes. The commit SHA is added to .git-blame-ignore-revs so git blame skips it, in line with the existing entries in that file.

Test Plan

Verified locally with clang-format-20 / git-clang-format-20:

  1. Both updated .clang-format files parse with clang-format 20.
  2. Idempotency: re-running clang-format-20 -i on every reformatted .td file produces no further changes.
  3. CI-equivalent gate: git-clang-format-20 --binary clang-format-20 --diff origin/develop returns "clang-format did not modify any files".
  4. commonConvArgs in RockOps.td now matches the layout requested in the issue exactly (one operand per line, aligned after (ins ).

Test Result

  • PR CI
  • Nightly CI
  • Weekly CI

Submission Checklist

Add TableGenBreakInsideDAGArg: BreakElements and
TableGenBreakingDAGArgOperators: [ins, outs] to the root and mlir/
.clang-format configs. Extend .githooks/pre-commit to run clang-format
on staged .td files, and note in CONTRIBUTING.md that clang-format >= 19
is required (20 matches CI).
Fixes #1936

Signed-off-by: bogdan-petkovic <bpetkovi@amd.com>
Apply clang-format-20 to all .td files under mlir/. Pure formatting
change driven by the new TableGenBreakInsideDAGArg/TableGenBreakingDAGArgOperators
settings; no semantic changes.

Signed-off-by: bogdan-petkovic <bpetkovi@amd.com>
Signed-off-by: bogdan-petkovic <bpetkovi@amd.com>
@bogdan-petkovic bogdan-petkovic self-assigned this May 18, 2026
@bogdan-petkovic bogdan-petkovic requested a review from causten as a code owner May 18, 2026 15:06
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2377      +/-   ##
===========================================
+ Coverage    79.50%   81.53%   +2.03%     
===========================================
  Files          100      125      +25     
  Lines        31016    42344   +11328     
  Branches      4819     6985    +2166     
===========================================
+ Hits         24659    34524    +9865     
- Misses        4245     5194     +949     
- Partials      2112     2626     +514     
Flag Coverage Δ
mfma 81.34% <ø> (+1.84%) ⬆️
navi4x 81.48% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 111 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Enables clang-format's TableGen DAG-breaking options so (ins …)/(outs …) operands lay out one-per-line, wires .td files into the pre-commit hook, documents the new clang-format >= 19 requirement, and bulk-reformats 13 .td files under mlir/ (with the reformat SHA added to .git-blame-ignore-revs).

Changes:

  • Add TableGenBreakInsideDAGArg: BreakElements and TableGenBreakingDAGArgOperators: [ins, outs] to both .clang-format files; extend pre-commit hook to .td; update CONTRIBUTING.md.
  • Append the bulk-reformat commit to .git-blame-ignore-revs.
  • Apply clang-format-20 -i to 13 .td files (NFC).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.clang-format Adds TableGen DAG-break options
mlir/.clang-format Same TableGen DAG-break options
.githooks/pre-commit Includes .td in staged file regex
CONTRIBUTING.md Documents clang-format >= 19 requirement and .td coverage
.git-blame-ignore-revs Adds NFC reformat commit
mlir/include/mlir/Conversion/RocMLIRPasses.td Reformatted dependentDialects/options
mlir/include/mlir/Dialect/MIGraphX/Passes.td Reformatted; file header banner split across lines
mlir/include/mlir/Dialect/MIGraphX/IR/MIGraphX.td Reformatted op defs; header banner split
mlir/include/mlir/Dialect/MIGraphX/IR/MIGraphXBase.td License comment reflow
mlir/include/mlir/Dialect/MIGraphX/IR/MIGraphXTypes.td Reformatted; header banner split
mlir/include/mlir/Dialect/Rock/Passes.td Reformatted pass defs
mlir/include/mlir/Dialect/Rock/IR/RockOps.td Bulk DAG-arg reflow
mlir/include/mlir/Dialect/Rock/IR/RockAttrDefs.td Reformatted; header banner split
mlir/include/mlir/Dialect/Rock/IR/RockAcceptingViewOpInterface.td Header banner split
mlir/include/mlir/Dialect/Rock/IR/RockAccelTuningParamAttrInterface.td Class decl reflow
mlir/include/mlir/Dialect/Rock/IR/RockGemmWrapperInterface.td License comment reflow
mlir/include/mlir/Dialect/Rock/IR/RockTuningParamAttrInterface.td Reflow + blank-line cleanups
mlir/include/mlir/Dialect/Rock/IR/RockWriterOpInterface.td License comment reflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mlir/include/mlir/Dialect/Rock/IR/RockAttrDefs.td Outdated
Comment thread mlir/include/mlir/Dialect/MIGraphX/Passes.td Outdated
Comment thread mlir/include/mlir/Dialect/MIGraphX/IR/MIGraphXTypes.td Outdated
Comment thread mlir/include/mlir/Dialect/MIGraphX/IR/MIGraphX.td Outdated
Comment thread mlir/include/mlir/Dialect/Rock/IR/RockAcceptingViewOpInterface.td Outdated
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