[libc-commits] [libc] [libc][docs] codify Policy on Assembler Sources (PR #88185)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Fri Apr 19 09:54:12 PDT 2024
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/88185
>From 7b562fab1daab1d509a85bd0c5739a9a346be347 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 13:00:02 -0700
Subject: [PATCH 1/7] [libc][docs] codify Policy on Assembler Sources
It would be helpful in future code reviews to document a policy with regards to
where and when Assembler sources are appropriate. That way when reviewers point
out infractions, they can point to this written policy, which may help
contributors understand that it's not the solely personal preferences of
reviewers but rather a previously agreed upon rule by maintainers.
Link: https://github.com/llvm/llvm-project/pull/87837
Link: https://github.com/llvm/llvm-project/pull/88157
Link: https://discourse.llvm.org/t/hand-written-in-assembly-in-libc-setjmp-longjmp/73249/12
---
libc/docs/dev/code_style.rst | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index 22a18b7a4cc1dd..a87e971f4dd8e9 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -186,3 +186,32 @@ We expect contributions to be free of warnings from the `minimum supported
compiler versions`__ (and newer).
.. __: https://libc.llvm.org/compiler_support.html#minimum-supported-versions
+
+Policy on Assembler sources
+===========================
+
+Coding in high level languages such as C++ provides benefits relative to low
+level languages like Assembler, such as:
+
+* Improved safety
+* Instrumentation
+
+ * Code coverage
+ * Profile collection
+* Sanitization
+* Debug info
+
+While its not impossible to have Assembler code that correctly provides all of
+the above, we do not wish to maintain such Assembler sources in llvm-libc.
+
+That said, there a few functions provided by llvm-libc that are more difficult
+to implement or maintain in C++ than Assembler. We do use inline or out-of-line
+Assembler in an intentionally minimal set of places; typically places where the
+stack or individual register state must be manipulated very carefully for
+correctness.
+
+Contributions adding Assembler for performance are not welcome. Contributors
+should strive to stick with C++ for as long as it remains reasonable to do so.
+llvm-libc maintainers reserve the right to reject Assembler contributions that
+could they feel could be better maintained if rewritten in C++, and to revisit
+this policy in the future.
>From 34d8d6587dbf83cdbae480b076d817bab5d67dc1 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 13:13:41 -0700
Subject: [PATCH 2/7] fix grammer mistake
---
libc/docs/dev/code_style.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index a87e971f4dd8e9..743fc0bd2a29be 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -213,5 +213,5 @@ correctness.
Contributions adding Assembler for performance are not welcome. Contributors
should strive to stick with C++ for as long as it remains reasonable to do so.
llvm-libc maintainers reserve the right to reject Assembler contributions that
-could they feel could be better maintained if rewritten in C++, and to revisit
-this policy in the future.
+they feel could be better maintained if rewritten in C++, and to revisit this
+policy in the future.
>From 07c2a0df20b718713c3066335a9f6ca7c799d61b Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 13:24:09 -0700
Subject: [PATCH 3/7] s/Assembler/Assembly/
---
libc/docs/dev/code_style.rst | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index 743fc0bd2a29be..afab7d08ac1156 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -187,11 +187,11 @@ compiler versions`__ (and newer).
.. __: https://libc.llvm.org/compiler_support.html#minimum-supported-versions
-Policy on Assembler sources
+Policy on Assembly sources
===========================
Coding in high level languages such as C++ provides benefits relative to low
-level languages like Assembler, such as:
+level languages like Assembly, such as:
* Improved safety
* Instrumentation
@@ -201,17 +201,17 @@ level languages like Assembler, such as:
* Sanitization
* Debug info
-While its not impossible to have Assembler code that correctly provides all of
-the above, we do not wish to maintain such Assembler sources in llvm-libc.
+While its not impossible to have Assembly code that correctly provides all of
+the above, we do not wish to maintain such Assembly sources in llvm-libc.
That said, there a few functions provided by llvm-libc that are more difficult
-to implement or maintain in C++ than Assembler. We do use inline or out-of-line
-Assembler in an intentionally minimal set of places; typically places where the
+to implement or maintain in C++ than Assembly. We do use inline or out-of-line
+Assembly in an intentionally minimal set of places; typically places where the
stack or individual register state must be manipulated very carefully for
correctness.
-Contributions adding Assembler for performance are not welcome. Contributors
+Contributions adding Assembly for performance are not welcome. Contributors
should strive to stick with C++ for as long as it remains reasonable to do so.
-llvm-libc maintainers reserve the right to reject Assembler contributions that
+llvm-libc maintainers reserve the right to reject Assembly contributions that
they feel could be better maintained if rewritten in C++, and to revisit this
policy in the future.
>From 257fbfaeb0d37a691ecd9ef216d0cc5add68ad93 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 13:24:30 -0700
Subject: [PATCH 4/7] s/its/it's/
---
libc/docs/dev/code_style.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index afab7d08ac1156..02368ced4e8077 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -201,7 +201,7 @@ level languages like Assembly, such as:
* Sanitization
* Debug info
-While its not impossible to have Assembly code that correctly provides all of
+While it's not impossible to have Assembly code that correctly provides all of
the above, we do not wish to maintain such Assembly sources in llvm-libc.
That said, there a few functions provided by llvm-libc that are more difficult
>From 0749f082f33b30fb3fdd5f8888177c4ade17dd6b Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 16 Apr 2024 14:40:54 -0700
Subject: [PATCH 5/7] reword
---
libc/docs/dev/code_style.rst | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index 02368ced4e8077..cf8770ef884788 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -194,24 +194,35 @@ Coding in high level languages such as C++ provides benefits relative to low
level languages like Assembly, such as:
* Improved safety
+* Compile time diagnostics
* Instrumentation
* Code coverage
* Profile collection
* Sanitization
-* Debug info
+* Automatic generation of debug info
While it's not impossible to have Assembly code that correctly provides all of
the above, we do not wish to maintain such Assembly sources in llvm-libc.
That said, there a few functions provided by llvm-libc that are more difficult
-to implement or maintain in C++ than Assembly. We do use inline or out-of-line
-Assembly in an intentionally minimal set of places; typically places where the
-stack or individual register state must be manipulated very carefully for
-correctness.
-
-Contributions adding Assembly for performance are not welcome. Contributors
-should strive to stick with C++ for as long as it remains reasonable to do so.
-llvm-libc maintainers reserve the right to reject Assembly contributions that
-they feel could be better maintained if rewritten in C++, and to revisit this
-policy in the future.
+to implement or maintain in C++ than Assembly.
+
+We do use inline or out-of-line Assembly in an intentionally minimal set of
+places; typically places where the stack or individual register state must be
+manipulated very carefully for correctness, or instances where a specific
+instruction sequence does not have a corresponding compiler builtin function
+today.
+
+Contributions adding functions implemented purely in Assembly for performance
+are not welcome.
+
+Contributors should strive to stick with C++ for as long as it remains
+reasonable to do so. Ideally, bugs should be filed against compiler vendors,
+and links to those bug reports should appear in commit messages or comments
+that seek to add Assembly to llvm-libc.
+
+Patches containing any amount of Assembly ideally should be approved by 2
+maintainers. llvm-libc maintainers reserve the right to reject Assembly
+contributions that they feel could be better maintained if rewritten in C++,
+and to revisit this policy in the future.
>From c06aa9d3d5ee40fbeb4a39f59a1c5e29a6837bd3 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 16 Apr 2024 14:42:51 -0700
Subject: [PATCH 6/7] mention all compilers
---
libc/docs/dev/code_style.rst | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index cf8770ef884788..f36bee5c03b3ed 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -205,8 +205,9 @@ level languages like Assembly, such as:
While it's not impossible to have Assembly code that correctly provides all of
the above, we do not wish to maintain such Assembly sources in llvm-libc.
-That said, there a few functions provided by llvm-libc that are more difficult
-to implement or maintain in C++ than Assembly.
+That said, there a few functions provided by llvm-libc that are impossible
+to reliably implement in C++ for all compilers supported for building
+llvm-libc.
We do use inline or out-of-line Assembly in an intentionally minimal set of
places; typically places where the stack or individual register state must be
>From a9c0ce3a1e3ff4e186bfff5ea5bcce6a3d068414 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Fri, 19 Apr 2024 09:53:46 -0700
Subject: [PATCH 7/7] fix grammatical error
---
libc/docs/dev/code_style.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index f36bee5c03b3ed..4a3e450d5dfbc5 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -205,7 +205,7 @@ level languages like Assembly, such as:
While it's not impossible to have Assembly code that correctly provides all of
the above, we do not wish to maintain such Assembly sources in llvm-libc.
-That said, there a few functions provided by llvm-libc that are impossible
+That said, there are a few functions provided by llvm-libc that are impossible
to reliably implement in C++ for all compilers supported for building
llvm-libc.
More information about the libc-commits
mailing list