From 420042e84fadb9b0373f4d38438fcd0b5a6242c9 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 ee4e4257c9fa8..4b5260275f421 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -219,3 +219,32 @@ defines. Code under ``libc/src/`` should ``#include`` a proxy header from ``hdr/``, which contains a guard on ``LLVM_LIBC_FULL_BUILD`` to either include our header from ``libc/include/`` (fullbuild) or the corresponding underlying system header (overlay). + +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 c99828714f510f6fdc626c6cc1d83f6127a3e46f Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 4b5260275f421..3bb56c4622d23 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -246,5 +246,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 e57f0a6892361d913530fbf99279321050a8ab93 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Tue, 9 Apr 2024 13:24:09 -0700 Subject: [PATCH 3/7] s/Assembler/Assembly/ --- libc/docs/dev/code_style.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst index 3bb56c4622d23..8caf0719bb30a 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -220,11 +220,11 @@ defines. Code under ``libc/src/`` should ``#include`` a proxy header from our header from ``libc/include/`` (fullbuild) or the corresponding underlying system header (overlay). -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 @@ -234,17 +234,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 955d7d68d0bb9bf8e09b073a6be5c7376861c4e2 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 8caf0719bb30a..7e3e2949e24f5 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -234,7 +234,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 c6c5db09403a152d0dd38bb88af504500a711639 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 7e3e2949e24f5..9789db51cbe45 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -227,24 +227,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 f72bb2e6b539507317f845c03a5be49071924176 Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 9789db51cbe45..be32b54f21cca 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -238,8 +238,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 280255d8c3e1cfbdcca18b78ddced88b009de06f Mon Sep 17 00:00:00 2001 From: Nick Desaulniers 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 be32b54f21cca..170ef6598a9d8 100644 --- a/libc/docs/dev/code_style.rst +++ b/libc/docs/dev/code_style.rst @@ -238,7 +238,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.