Skip to content

Conversation

@justinrosner
Copy link
Contributor

@justinrosner justinrosner commented Oct 31, 2025

Motivation

This PR pulls in the following upstream changes that fix a functional failure on gfx950.

Technical Details

See the linked PRs above for a detailed description of the upstream changes.

While I was investigating this case I also realized that I forgot to turn on, i.e., check the FileCheck output, the conv_bwd_data LIT tests when enabling the CPU lowering path: #2007. This PR addresses that, and fixes up an error in the CPU lowering logic that was masked because of this.

Test Plan

  • Manually run failing parameterSweeps
  • Run Weekly CI (for parameterSweeps)

Test Result

Submission Checklist

@justinrosner justinrosner requested a review from causten as a code owner October 31, 2025 21:37
@justinrosner justinrosner changed the title [UPSTREAM] Track minNumAGPRs in MFI instead of mayUseAGPRs [UPSTREAM] Pull down upstream changes to fix gfx950 functional failure Nov 1, 2025
Copy link
Contributor

@dhernandez0 dhernandez0 Nov 3, 2025

Choose a reason for hiding this comment

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

I don't like that we are fixing an upstream bug and a downstream bug at the same time. While the name of the PR is [UPSTREAM]. I'd split it into two PRs but I'm going to approve it and you can decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to merge this PR such that all of the external commits are preserved as well as the internal commit. So in the rocMLIR git history the internal commit will show up as separate. They are only really joined in the reviewing of this PR.

I'm happy to pull out the internal fix into its own PR if you feel like that is something worth doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's merge it

@justinrosner justinrosner force-pushed the justinr-gfx950-patch branch 3 times, most recently from 3724f22 to 8b18f9c Compare November 4, 2025 13:28
@justinrosner justinrosner merged commit 9c44414 into develop Nov 5, 2025
7 of 16 checks passed
@justinrosner justinrosner deleted the justinr-gfx950-patch branch November 5, 2025 03:34
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.

4 participants