Introduce ResultError to support Result-based llvm::Errors
#1545
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test summary for commit 19581efDriver commits used in build
CTS tests (Failed: 0/225388)
Rhel 8.2, Gfx10Ubuntu 18.04, Gfx9Ubuntu 20.04, Gfx8Ubuntu 20.04, Gfx103 |
Introduce a new class `Llpc::ResultError` that implements a `Result`-based `llvm::Error` type. This is primarily to support returning either error results or values with `llvm::Expected<T>`. Also implement the necessary utility functions, and move some existing ones to the new header. I also considered using `llvm::StringError` directly, but this did not work well as some `Vkgc::Result` values do not map well to any `std::errc` error codes, and it was difficult to convert back to `Vkgc::Result`, which many existing APIs expect. See the mailing list thread `[RFC] Improving LLPC Result checking` for the proposal details.
|
V2: Silence a clang-tidy warning. |
|
I don’t see these clang-tidy warnings when running it locally, so I guess they will go away once we can update llvm in the containers. |
dstutt
approved these changes
Nov 30, 2021
|
The Jenkins run went even more wrong than normal, so I can't see test passes in it. I think we'd better try again. |
|
retest this please |
|
Hmm, maybe that doesn't work any more. I'll cherry-pick it to our internal branch and do a test run there; that should give some more confidence. |
Test summary for commit e1d583dDriver commits used in build
CTS tests (Failed: 0/225388)
Rhel 8.2, Gfx10Ubuntu 18.04, Gfx9Ubuntu 20.04, Gfx8Ubuntu 20.04, Gfx103 |
|
Rex told me how to internally retrigger the Jenkins build here, and it passed all tests (and it got there before internal testing). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce a new class
Llpc::ResultErrorthat implements aResult-basedllvm::Errortype. This is primarily to support returning either error resultsor values with
llvm::Expected<T>.Also implement the necessary utility functions, and move some existing ones to the
new header.
I also considered using
llvm::StringErrordirectly, but this did not work well assome
Vkgc::Resultvalues do not map well to anystd::errcerror codes, and it wasdifficult to convert back to
Vkgc::Result, which many existing APIs expect.See the mailing list thread
[RFC] Improving LLPC Result checkingfor the proposaldetails.