[LangRef] allow omitting va_end#203087
Merged
Merged
Conversation
|
@llvm/pr-subscribers-llvm-ir Author: Folkert de Vries (folkertdev) ChangesIn Rust we'd like to be able to omit cc rust-lang/rust#157627 rust-lang/rust#155697 Full diff: https://github.com/llvm/llvm-project/pull/203087.diff 1 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index cf052513c5ef8..fa363f77c4485 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -14631,10 +14631,13 @@ Semantics:
The '``llvm.va_end``' intrinsic works just like the ``va_end`` macro
available in C. In a target-dependent way, it destroys the ``va_list``
-element to which the argument points. Calls to
+element to which the argument points. Calls to ``llvm.va_end`` can be
+omitted when they are a no-op for the given target. ``llvm.va_end``
+is a no-op for all currently supported targets.
+
+When used, calls to ``llvm.va_end`` must be matched exactly with calls to
:ref:`llvm.va_start <int_va_start>` and
-:ref:`llvm.va_copy <int_va_copy>` must be matched exactly with calls to
-``llvm.va_end``.
+:ref:`llvm.va_copy <int_va_copy>`.
.. _int_va_copy:
|
efriedma-quic
left a comment
efriedma-quic
left a comment
Contributor
There was a problem hiding this comment.
https://groups.google.com/g/comp.std.c/c/cld4zwoKaFw indicates that some very old compilers might have needed va_end, but I don't know of any specific ABI that needs it. It's probably fine to loosen the rules like this.
We probably want to mention somewhere that the underlying argument list is destroyed when the function returns.
We probably also want to modify the description of va_copy to make it clear that it's okay to just memcpy on targets where va_list is trivially copyable.
Contributor
Author
|
Yes, we found https://softwarepreservation.computerhistory.org/c_plus_plus/cfront/release_3.0.3/source/incl-master/proto-headers/stdarg.sol which has this bit of code: #define va_start(ap, parmN) {\
va_buf _va;\
_vastart(ap = (va_list)_va, (char *)&parmN + sizeof parmN)
#define va_end(ap) }
#define va_arg(ap, mode) *((mode *)_vaarg(ap, sizeof (mode)))With this expansion it's crucial that I'll have a look at the other updates. |
- the argument list getting destroyed - memcpy being OK when the va_list is trivially copyable
| @@ -14671,6 +14677,10 @@ available in C. In a target-dependent way, it copies the source | |||
| intrinsic is necessary because the ``llvm.va_start`` intrinsic may be | |||
| arbitrarily complex and require, for example, memory allocation. | |||
|
|
|||
| On targets where ``va_list`` is trivially copyable, ``memcpy`` can be | |||
Contributor
There was a problem hiding this comment.
We probably don't want to use the exact phrase "trivially copyable" to describe this, because it overlaps with the term in the C++ standard (and the C++ standard semantics isn't quite what we want).
Maybe just say "On targets where va_copy is equivalent to 'memcpy'"?
efriedma-quic
approved these changes
Jun 15, 2026
| @@ -14671,6 +14677,10 @@ available in C. In a target-dependent way, it copies the source | |||
| intrinsic is necessary because the ``llvm.va_start`` intrinsic may be | |||
| arbitrarily complex and require, for example, memory allocation. | |||
|
|
|||
| On targets where ``llvm.va_copy`` is equivalent to ``memcpy``, ``memcpy`` | |||
| can be used instead to duplicate a ``va_list``. ``llvm.va_copy`` is | |||
| equivalent to ``memcpy`` on all currently supported targets implement | |||
Contributor
There was a problem hiding this comment.
Suggested change
| equivalent to ``memcpy`` on all currently supported targets implement | |
| equivalent to ``memcpy`` on all currently supported targets. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/19593 Here is the relevant piece of the build log for the reference |
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
4 participants
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.
In Rust we'd like to be able to omit
va_end: it is a no-op on all currently supported targets, and the requirement that it is paired exactly withva_startandva_copy(in the same frame) cannot be unified with a language with move semantics.cc rust-lang/rust#157627 rust-lang/rust#155697