Skip to content

cuda : use std::string instead of VLA for archName - #30463

Merged
angt merged 2 commits into
ggml-org:masterfrom
angt:cuda-use-std-string-instead-of-vla-for-archname
Oct 25, 2026
Merged

cuda : use std::string instead of VLA for archName#30463
angt merged 2 commits into
ggml-org:masterfrom
angt:cuda-use-std-string-instead-of-vla-for-archname

Conversation

@angt

@angt angt commented Oct 13, 2026

Copy link
Copy Markdown
Member

Overview

Modernize the code (old C style) and fix the following warnings:

ggml/src/ggml-cuda/ggml-cuda.cu:180:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
  180 |     char archName[archLen + 1];
      |                   ^~~~~~~~~~~
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: note: read of non-const variable 'archLen' is not allowed in a constant expression
ggml/src/ggml-cuda/ggml-cuda.cu:179:9: note: declared here
  179 |     int archLen = strlen(devName);
      |         ^

Additional information

Requirements

@angt
cuda : use std::string instead of VLA for archName
d5aefcf 
Modernize the code (old C style) and fix the following warnings:

```
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
  180 |     char archName[archLen + 1];
      |                   ^~~~~~~~~~~
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: note: read of non-const variable 'archLen' is not allowed in a constant expression
ggml/src/ggml-cuda/ggml-cuda.cu:179:9: note: declared here
  179 |     int archLen = strlen(devName);
      |         ^
```

Signed-off-by: Adrien Gallouët <angt@huggingface.co>
@angt
angt requested a review from a team as a code owner October 13, 2026 15:48
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning CUDA Related to the CUDA backend labels Oct 13, 2026
@JohannesGaessler

JohannesGaessler commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Trivially fixing warnings is fine but please don't submit PRs introducing functional changes simply to "modernize the code".

@angt

angt commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

Trivially fixing warnings is fine but please don't submit PRs introducing functional changes simply to "modernize the code".

I can assure you that fixing others' warnings is not funny.

Anyway I took the opportunity to improve the code: fixing a buggy behavior for names of length ≤ 3 (UB) and avoiding dangerous functions like strcpy/strtok, plus simplifying the -generic case.

That said, maybe you spotted a regression I missed?

@JohannesGaessler

JohannesGaessler commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

If there are functional advantages I am willing to review related PRs. Otherwise my preferred solution would be to suppress warnings.

@IMbackK

IMbackK commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

i think using std::string is better here.

@JohannesGaessler

JohannesGaessler commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Yes, it is better. To me it's a question of opportunity cost.

@IMbackK IMbackK self-assigned this Oct 13, 2026
am17an
am17an approved these changes Oct 16, 2026
View reviewed changes

@am17an am17an left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks fine to me.

JohannesGaessler
JohannesGaessler reviewed Oct 17, 2026
View reviewed changes

@JohannesGaessler JohannesGaessler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please keep the comments explaining what is being stripped.

@angt
Add a comment about archName
e52763a 
Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>
@angt
angt merged commit d854d98 into ggml-org:master Oct 25, 2026
16 of 17 checks passed
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
@angt
cuda : use std::string instead of VLA for archName (ggml-org#30463)
6747193 
* cuda : use std::string instead of VLA for archName

Modernize the code (old C style) and fix the following warnings:

```
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
  180 |     char archName[archLen + 1];
      |                   ^~~~~~~~~~~
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: note: read of non-const variable 'archLen' is not allowed in a constant expression
ggml/src/ggml-cuda/ggml-cuda.cu:179:9: note: declared here
  179 |     int archLen = strlen(devName);
      |         ^
```

Signed-off-by: Adrien Gallouët <angt@huggingface.co>

* Add a comment about archName

Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>

---------

Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 30, 2026
@angt @edwardyoon
cuda : use std::string instead of VLA for archName (ggml-org#30463)
06dbf92 
* cuda : use std::string instead of VLA for archName

Modernize the code (old C style) and fix the following warnings:

```
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
  180 |     char archName[archLen + 1];
      |                   ^~~~~~~~~~~
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: note: read of non-const variable 'archLen' is not allowed in a constant expression
ggml/src/ggml-cuda/ggml-cuda.cu:179:9: note: declared here
  179 |     int archLen = strlen(devName);
      |         ^
```

Signed-off-by: Adrien Gallouët <angt@huggingface.co>

* Add a comment about archName

Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>

---------

Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>
huaxel pushed a commit to huaxel/strix-llama.cpp that referenced this pull request Nov 1, 2026
@angt @huaxel
cuda : use std::string instead of VLA for archName (ggml-org#30463)
a49ff25 
* cuda : use std::string instead of VLA for archName

Modernize the code (old C style) and fix the following warnings:

```
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
  180 |     char archName[archLen + 1];
      |                   ^~~~~~~~~~~
ggml/src/ggml-cuda/ggml-cuda.cu:180:19: note: read of non-const variable 'archLen' is not allowed in a constant expression
ggml/src/ggml-cuda/ggml-cuda.cu:179:9: note: declared here
  179 |     int archLen = strlen(devName);
      |         ^
```

Signed-off-by: Adrien Gallouët <angt@huggingface.co>

* Add a comment about archName

Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>

---------

Signed-off-by: Adrien Gallouët <angt@huggingface.co>
Signed-off-by: Adrien Gallouët <adrien@gallouet.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@am17an am17an am17an approved these changes
@JohannesGaessler JohannesGaessler JohannesGaessler approved these changes

Assignees

@IMbackK IMbackK

Labels

CUDA Related to the CUDA backend ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@angt @JohannesGaessler @IMbackK @am17an