[SYCL][NFC] Refactor addSYCLDefaultTriple #20938
Conversation
This patch merges two loops traversing SYCL triples into one. Also optimizes the usage of addSYCLDefaultTriple inside CreateOffloadingDeviceToolChains. The default triple is guranted to be added at the beginning of the list. Instead of building the default triple again in CreateOffloadingDeviceToolChains, we re-use the value added by addSYCLDefaultTriple to the list of the triples.
clang/lib/Driver/Driver.cpp
Outdated
| // Check current set of triples to see if the default has already been set. | ||
| for (const auto &SYCLTriple : SYCLTriples) { | ||
| // TODO: Do we need this check? Don't we already detect the default triple | ||
| // in the previous condition? |
There was a problem hiding this comment.
Since we allow for spir64 or spirv64, this checks for the latter. Perhaps we can just use this check and not have the one above?
There was a problem hiding this comment.
Good point. I forgot that we have two spellings to produce SPIR-V binaries. I'll try to avoid unnecessary checks.
clang/lib/Driver/Driver.cpp
Outdated
| if (SYCLTriple.isNVPTX() || SYCLTriple.isAMDGCN()) | ||
| return false; | ||
| } | ||
| // TODO: Why do we insert at the beginning? This is expensive operation. |
There was a problem hiding this comment.
insert at the beginning isn't required. Just need to be sure that the use knows where the triple is added (there is one more use in Driver.cpp). It may also impact toolchain ordering which will require test updates.
There was a problem hiding this comment.
It would be great if we can add at the end of the container. If it doesn't require a ton of work to update the test, I'll do that.
There was a problem hiding this comment.
All tests pass unmodified. 😲
bader
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
NOTE: 75c6eb0 is probably a functional change, but I hope this fixes a bug not caught by testing. |
bader
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
bader
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
2 participants
This patch merges two loops traversing SYCL triples into one.
Also optimizes the usage of addSYCLDefaultTriple inside
CreateOffloadingDeviceToolChains. The default triple is guranted to be
added at the beginning of the list. Instead of building the default
triple again in CreateOffloadingDeviceToolChains, we re-use the value
added by addSYCLDefaultTriple to the list of the triples.