← 返回首页
This patch fixes bug #3293 - OpenCL kernel argument problem with some locales by GuillaumeSchmid · Pull Request #3294 · arrayfire/arrayfire · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

This patch fixes bug #3293 - OpenCL kernel argument problem with some locales#3294

Merged
umar456 merged 1 commit into
arrayfire:masterfrom
GuillaumeSchmid:toString_C_locale
Nov 21, 2022
Merged

This patch fixes bug #3293 - OpenCL kernel argument problem with some locales#3294
umar456 merged 1 commit into
arrayfire:masterfrom
GuillaumeSchmid:toString_C_locale

Conversation

Copy link
Copy Markdown
Contributor

Short Description:

There is a problem when the locale is non C when rendering the arguments to OpenCL kernels.

Short description of the change:

The patch is pretty simple, it changes the toString() function to use the stringstream imbued with C locale in src/backend/common/TemplateArg.cpp and changed the to_string calls to this toString function in types.cpp.

Description

The arguments provided to OpenCL uses the C++ standard library function std::to_string(). This function uses the locale to render it's argument to a string.

It is a problem when arrayfire is used in a
software initialised with non "C" locale.

For instance, on a French computer, to_string(1.0) will output the string "1,0000000".
This string is provided to OpenCL kernels, generating a syntax error.

The most portable way to fix this problem is to use a local ostringstream imbued withe "C" locale.

An Other way would be to use C++17 to_chars function, as it only renders it argument with "C" locale, without impact from the application or system locale.
I did not use this latest solution as it would force the use of C++17.

  • [ yes] Rebased on latest master
  • [yes ] Code compiles
  • [yes] Tests pass
  • [no ] Functions added to unified API
  • [no ] Functions documented

syurkevi requested a review from umar456 November 9, 2022 22:14
umar456 force-pushed the toString_C_locale branch 2 times, most recently from 1dc3a33 to 7ab1bf8 Compare November 16, 2022 18:43
umar456 previously approved these changes Nov 16, 2022
umar456 force-pushed the toString_C_locale branch 2 times, most recently from 0135aa2 to 49e8f03 Compare November 16, 2022 23:26
The arguments provided to OpenCL uses the C++ standard library function std::to_string(). This function uses the locale to render it's argument to a string. It is a problem when arrayfire is used in a software initialised with non "C" locale. For instance, on a French computer, to_string(1.0) will output the string "1,0000000". This string is provided to OpenCL kernels, generating a syntax error. The most portable way to fix this problem is to use a local ostringstream imbued withe "C" locale. An Other way would be to use C++17 to_chars function, as it only renders it argument with "C" locale, without impact from the application or system locale. The patch is pretty simple, it changes the toString() function to use the stringstream in src/backend/common/TemplateArg.cpp and changed the to_string calls to this toString function in types.cpp.
Copy link
Copy Markdown
Member

umar456 commented Nov 18, 2022

@GuillaumeSchmid I made some changes to your PR to address a couple of other issues related to the structure of the codebase. Could you give these changes another try to see if addresses the issue you are seeing. I was able to incorporate the to_chars feature suggested in your comment.

Copy link
Copy Markdown
Contributor Author

@umar456 I tested your implementation with to_char on my code, it works.

umar456 merged commit 5a11efe into arrayfire:master Nov 21, 2022
This file contains hidden or 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
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.

2 participants

Footer

© 2026 GitHub, Inc.