[SYCL] Fix handling of comma separated arch value when calling llvm-offload-binary #20130
Conversation
|
It would be helpful if you can respond to individual comments if they have been addressed or not, OR use the resolve button to resolve the conversation. Thanks! |
| // "-g,-cl-opt-disable"`. Otherwise, only the first arg is processed by | ||
| // ocloc as an arg for -options, and the rest are processed as standalone | ||
| // flags, possibly leading to errors. | ||
| // ocloc -options takes arguments in the form of '-options "-g |
There was a problem hiding this comment.
So, Yixing's change should be merged first, right?
There was a problem hiding this comment.
Yes, I forgot to add a comment here on the PR, but I needed Yixing's change to pass testing.
There was a problem hiding this comment.
ok. I think if this change is ready first, we should go ahead and merge it. It doesn't matter, which change goes first in. Both are required anyway.
There was a problem hiding this comment.
Sounds good. FYI @YixingZhang007, I think this should merge cleanly with your changes but just letting you know.
maarquitos14
left a comment
maarquitos14
left a comment
There was a problem hiding this comment.
LGTM, just a few nits.
| std::string Replace = AfterOptions.str(); | ||
| std::replace(Replace.begin(), Replace.end(), ' ', ','); | ||
| CmdArgs.push_back(Args.MakeArgString(Replace)); | ||
| // Split the options string by spaces and rejoin to normalize whitespace |
There was a problem hiding this comment.
| // Split the options string by spaces and rejoin to normalize whitespace | |
| // Split the options string by spaces and rejoin to normalize whitespace. |
|
@intel/llvm-gatekeepers please consider merging |
jzc
changed the title
|
@intel/llvm-gatekeepers Failures unrelated (all green in fc02714), this PR is ready to merge. |
7 participants
Because
llvm-offload-binaryuses commas to separate the key-values pairs when specifying an image, we cannot specify a value that contains a comma without some special handling. However, if you specify the same key multiple times when callingllvm-offload-binary, the keys will be concatenated together with commas in between. So if we want, for example, the image to have anarchofpvc,bdw, we can callllvm-offload-binarywitharch=pvc,arch=bdwto achieve this.