Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CodeGenOpt #2707

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Update CodeGenOpt #2707

merged 1 commit into from
Sep 19, 2023

Conversation

piotrAMD
Copy link
Contributor

There is a flag-day change around CodeGen enums in llvm/llvm-project#66295.

Update the code to use both old and new schemes depending on the LLVM version used.

Change enum to uint32_t in some places to reduce the number of include guards.

@piotrAMD piotrAMD requested a review from a team as a code owner September 18, 2023 10:23
Copy link
Member

@dstutt dstutt left a comment

Choose a reason for hiding this comment

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

LGTM - but fails formatting check.

@piotrAMD
Copy link
Contributor Author

Fixed formatting check.

Copy link
Member

@dstutt dstutt left a comment

Choose a reason for hiding this comment

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

LGTM

@amdvlk-admin
Copy link

Test summary for commit c42cead

CTS tests (Failed: 0/208496)
  • Built with version 1.3.5.2
  • Rhel 9.0, Gfx10
    • Passed: 36978/69503 (53.2%)
    • Failed: 0/69503 (0.0%)
    • Not Supported: 32525/69503 (46.8%)
    • Warnings: 0/69503 (0.0%)
    Ubuntu 22.04, Navi3x
    • Passed: 41144/69503 (59.2%)
    • Failed: 0/69503 (0.0%)
    • Not Supported: 28359/69503 (40.8%)
    • Warnings: 0/69503 (0.0%)
    Ubuntu 20.04, Navi2x
    • Passed: 41178/69490 (59.3%)
    • Failed: 0/69490 (0.0%)
    • Not Supported: 28312/69490 (40.7%)
    • Warnings: 0/69490 (0.0%)

@amdvlk-admin
Copy link

Test summary for commit 5b69909

CTS tests (Failed: 0/208496)
  • Built with version 1.3.5.2
  • Rhel 9.0, Gfx10
    • Passed: 36978/69503 (53.2%)
    • Failed: 0/69503 (0.0%)
    • Not Supported: 32525/69503 (46.8%)
    • Warnings: 0/69503 (0.0%)
    Ubuntu 22.04, Navi3x
    • Passed: 41144/69503 (59.2%)
    • Failed: 0/69503 (0.0%)
    • Not Supported: 28359/69503 (40.8%)
    • Warnings: 0/69503 (0.0%)
    Ubuntu 20.04, Navi2x
    • Passed: 41178/69490 (59.3%)
    • Failed: 0/69490 (0.0%)
    • Not Supported: 28312/69490 (40.7%)
    • Warnings: 0/69490 (0.0%)

There is a flag-day change around CodeGen enums in llvm/llvm-project#66295.

Update the code to use both old and new schemes depending on the LLVM version used.

Change enum to uint32_t in some places to reduce the number of include guards.
@amdvlk-admin
Copy link

Test summary for commit f9ad9ed

CTS tests (Failed: 0/208497)
  • Built with version 1.3.5.2
  • Rhel 9.0, Gfx10
    • Passed: 36978/69503 (53.2%)
    • Failed: 0/69503 (0.0%)
    • Not Supported: 32525/69503 (46.8%)
    • Warnings: 0/69503 (0.0%)
    Ubuntu 22.04, Navi3x
    • Passed: 41144/69504 (59.2%)
    • Failed: 0/69504 (0.0%)
    • Not Supported: 28360/69504 (40.8%)
    • Warnings: 0/69504 (0.0%)
    Ubuntu 20.04, Navi2x
    • Passed: 41178/69490 (59.3%)
    • Failed: 0/69490 (0.0%)
    • Not Supported: 28312/69490 (40.7%)
    • Warnings: 0/69490 (0.0%)

@piotrAMD piotrAMD merged commit e60c1cc into GPUOpen-Drivers:dev Sep 19, 2023
9 checks passed
@piotrAMD piotrAMD deleted the codegenopt branch September 19, 2023 08:30
Copy link
Member

@nhaehnle nhaehnle left a comment

Choose a reason for hiding this comment

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

FWIW, I think (most of?) this could have been done by adding a strategic using directive that exists only in the old version of the code.

dstutt added a commit that referenced this pull request Sep 28, 2023
There is a flag-day change around CodeGen enums in llvm/llvm-project#66295.

Changes were made in #2707 but some
unit tests were missed.
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