Skip to content

Adding Pre-formatting to PRs#117

Merged
ammallya merged 14 commits into
developfrom
ammallya/pre-formatting
May 30, 2025
Merged

Adding Pre-formatting to PRs#117
ammallya merged 14 commits into
developfrom
ammallya/pre-formatting

Conversation

@ammallya
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator

@jayhawk-commits jayhawk-commits left a comment

Choose a reason for hiding this comment

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

Could you please link the test runs for this work?

Comment thread .github/workflows/pre-formatting.yml Outdated
Comment thread .github/workflows/pre-formatting.yml Outdated
Comment thread .github/workflows/pre-formatting.yml Outdated
Comment thread .github/workflows/pre-formatting.yml Outdated
@ammallya
Copy link
Copy Markdown
Collaborator Author

#129 - this is the demo test PR, As you see it lints removes the white space and creates the commit

Comment thread .github/workflows/pre-formatting.yml Outdated
@jayhawk-commits
Copy link
Copy Markdown
Collaborator

Documentation on what formatter options we pick on the contribution guide would be helpful, but that can be put in a separate pull request.

@ammallya ammallya merged commit c925588 into develop May 30, 2025
11 checks passed
@ammallya ammallya deleted the ammallya/pre-formatting branch May 30, 2025 21:45
ammallya pushed a commit that referenced this pull request Sep 24, 2025
* Silence uninteresting call warnings in test_graph.cpp
* Properly address uninteresting call warnings in other files
ammallya pushed a commit that referenced this pull request Sep 24, 2025
* Silence uninteresting call warnings in test_graph.cpp
* Properly address uninteresting call warnings in other files

[ROCm/hipDNN commit: e1a48d5]
eidenyoshida pushed a commit that referenced this pull request May 15, 2026
…hift, AssertKRingShiftTailWrapOnly, BAddrInterleave (#7513)

## Summary

Downstream **hipSPARSELt** / **TensileCreateLibrary** builds broke after
**[#7443](#7443 (*Manual
revert KRingShift*) because **TensileLite** stopped understanding
several **Assert\*** / **KRingShift** fields that were **still present**
in checked-in **Tensile logic YAML**. This PR **removes those stale
keywords** from the affected **hipBLASLt** and **hipSPARSELt** logic
files so parsing matches the reverted **Python** behavior.

## Root cause (what broke)

- **[#7443](#7443 removed
handlers in `projects/hipblaslt/tensilelite/Tensile/Contractions.py` for
keys such as **`AssertFree1DivByMT1LowbitGT1`**,
**`AssertKRingShiftTailWrapOnly`**, and related **KRingShift** /
**BAddrInterleave** plumbing.
- Shipped **YAML** under
**`projects/hipsparselt/.../Tensile/Logic/...`** (e.g. **gfx950** /
**gfx942**) and **`projects/hipblaslt/.../Tensile/Logic/...`** (e.g.
**gfx1250** GridBased) still contained those fields.
- **TensileCreateLibrary** then hit **`RuntimeError: Unknown assertion
key: AssertFree1DivByMT1LowbitGT1`** (and would be vulnerable to the
next unknown key if only partially cleaned).

So the regression was **YAML ↔ parser skew** after the revert, not the
drop Docker image or venv.

## What this PR does

- **hipSPARSELt:** strips **`AssertFree1DivByMT1LowbitGT1`**,
**`KRingShift`**, **`AssertKRingShiftTailWrapOnly`**,
**`BAddrInterleave`** from the **Equality** logic YAMLs under
**`aquavanjaram/gfx942`** and **`gfx950`** (the paths exercised by
**gfx942;gfx950** builds).
- **hipBLASLt:** same cleanup on the **8** **`gfx1250` GridBased** logic
YAMLs that still carried those keys.

No change to kernel math here—this aligns **on-disk logic** with the
**post-#7443** **TensileLite** parser.

## How we know it’s fixed

- **Math CI:** **[hipblaslt-drop-build
#117](https://math-ci.amd.com/job/image-builder/job/hipblaslt-drop-build/117/)**
completed **SUCCESS** with a **`ROCM_LIBS_REF`** that includes this fix,
exercising **hipBLASLt** + **hipSPARSELt** with **gfx942;gfx950** (full
drop-style lane).
- **This PR’s presubmit:** touches `projects/hipsparselt/**` (and
**hipBLASLt** logic), so **TheRock** runs the **hipSPARSELt** /
**sparselt** workstream for this change—not only **blas**—which directly
exercises **TensileCreateLibrary** on the updated YAML.
- **PR tag:** includes **`[project:hipsparselt]`** so **hipSPARSELt** is
**built and tested** on this PR alongside **hipBLASLt**, not only after
merge.

## CI coverage context (why #7443 slipped)

- **[#7519](#7519 —
documents that **hipBLASLt-only** PRs previously did **not** reliably
pull **hipSPARSELt** into the **TheRock** matrix, so **#7443** could
merge green while still breaking downstream **hipSPARSELt** / drop
builds.
- **[#7514](#7514 —
follow-up to wire **`projects/hipblaslt/**`** so **`sparselt`** runs
when **hipBLASLt** changes, closing that gap long-term (orthogonal to
this YAML fix, but related).

## Checklist

- [ ] Confirm no remaining **`AssertFree1DivByMT1LowbitGT1`** / stray
**KRingShift** keys in the touched trees (`git grep` on the branch).
- [ ] After merge, spot-check **TheRock** + optional
**hipblaslt-drop-build** on **`develop`**.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
aledudek pushed a commit that referenced this pull request May 20, 2026
…hift, AssertKRingShiftTailWrapOnly, BAddrInterleave (#7513)

## Summary

Downstream **hipSPARSELt** / **TensileCreateLibrary** builds broke after
**[#7443](#7443 (*Manual
revert KRingShift*) because **TensileLite** stopped understanding
several **Assert\*** / **KRingShift** fields that were **still present**
in checked-in **Tensile logic YAML**. This PR **removes those stale
keywords** from the affected **hipBLASLt** and **hipSPARSELt** logic
files so parsing matches the reverted **Python** behavior.

## Root cause (what broke)

- **[#7443](#7443 removed
handlers in `projects/hipblaslt/tensilelite/Tensile/Contractions.py` for
keys such as **`AssertFree1DivByMT1LowbitGT1`**,
**`AssertKRingShiftTailWrapOnly`**, and related **KRingShift** /
**BAddrInterleave** plumbing.
- Shipped **YAML** under
**`projects/hipsparselt/.../Tensile/Logic/...`** (e.g. **gfx950** /
**gfx942**) and **`projects/hipblaslt/.../Tensile/Logic/...`** (e.g.
**gfx1250** GridBased) still contained those fields.
- **TensileCreateLibrary** then hit **`RuntimeError: Unknown assertion
key: AssertFree1DivByMT1LowbitGT1`** (and would be vulnerable to the
next unknown key if only partially cleaned).

So the regression was **YAML ↔ parser skew** after the revert, not the
drop Docker image or venv.

## What this PR does

- **hipSPARSELt:** strips **`AssertFree1DivByMT1LowbitGT1`**,
**`KRingShift`**, **`AssertKRingShiftTailWrapOnly`**,
**`BAddrInterleave`** from the **Equality** logic YAMLs under
**`aquavanjaram/gfx942`** and **`gfx950`** (the paths exercised by
**gfx942;gfx950** builds).
- **hipBLASLt:** same cleanup on the **8** **`gfx1250` GridBased** logic
YAMLs that still carried those keys.

No change to kernel math here—this aligns **on-disk logic** with the
**post-#7443** **TensileLite** parser.

## How we know it’s fixed

- **Math CI:** **[hipblaslt-drop-build
#117](https://math-ci.amd.com/job/image-builder/job/hipblaslt-drop-build/117/)**
completed **SUCCESS** with a **`ROCM_LIBS_REF`** that includes this fix,
exercising **hipBLASLt** + **hipSPARSELt** with **gfx942;gfx950** (full
drop-style lane).
- **This PR’s presubmit:** touches `projects/hipsparselt/**` (and
**hipBLASLt** logic), so **TheRock** runs the **hipSPARSELt** /
**sparselt** workstream for this change—not only **blas**—which directly
exercises **TensileCreateLibrary** on the updated YAML.
- **PR tag:** includes **`[project:hipsparselt]`** so **hipSPARSELt** is
**built and tested** on this PR alongside **hipBLASLt**, not only after
merge.

## CI coverage context (why #7443 slipped)

- **[#7519](#7519 —
documents that **hipBLASLt-only** PRs previously did **not** reliably
pull **hipSPARSELt** into the **TheRock** matrix, so **#7443** could
merge green while still breaking downstream **hipSPARSELt** / drop
builds.
- **[#7514](#7514 —
follow-up to wire **`projects/hipblaslt/**`** so **`sparselt`** runs
when **hipBLASLt** changes, closing that gap long-term (orthogonal to
this YAML fix, but related).

## Checklist

- [ ] Confirm no remaining **`AssertFree1DivByMT1LowbitGT1`** / stray
**KRingShift** keys in the touched trees (`git grep` on the branch).
- [ ] After merge, spot-check **TheRock** + optional
**hipblaslt-drop-build** on **`develop`**.

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants