[libc] {u}lkbits broken on riscv32 #115799
Merged
Conversation
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
…stems where long is 32 bits while long _Accum is 64 bits
|
@llvm/pr-subscribers-libc Author: William Tran-Viet (smallp-o-p) Changes
This is probably inconvenient on systems that have a word size larger than 64 bits? Full diff: https://github.com/llvm/llvm-project/pull/115799.diff 2 Files Affected:
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 3758f0809960e5..5419462d4f5b3b 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -728,8 +728,8 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
libc.src.stdfix.kbits
libc.src.stdfix.ukbits
# TODO: https://github.com/llvm/llvm-project/issues/115778
- # libc.src.stdfix.lkbits
- # libc.src.stdfix.ulkbits
+ libc.src.stdfix.lkbits
+ libc.src.stdfix.ulkbits
)
endif()
diff --git a/libc/include/llvm-libc-types/stdfix-types.h b/libc/include/llvm-libc-types/stdfix-types.h
index 759e59251a5e87..542d45ea97e96f 100644
--- a/libc/include/llvm-libc-types/stdfix-types.h
+++ b/libc/include/llvm-libc-types/stdfix-types.h
@@ -14,12 +14,12 @@ typedef signed short int int_r_t;
typedef signed int int_lr_t;
typedef signed short int_hk_t;
typedef signed int int_k_t;
-typedef signed long int_lk_t;
+typedef signed long long int_lk_t;
typedef unsigned char uint_uhr_t;
typedef unsigned short int uint_ur_t;
typedef unsigned int uint_ulr_t;
typedef unsigned short int uint_uhk_t;
typedef unsigned int uint_uk_t;
-typedef unsigned long uint_ulk_t;
+typedef unsigned long long uint_ulk_t;
#endif // LLVM_LIBC_TYPES_STDFIX_TYPES_H
|
| @@ -728,8 +728,8 @@ if(LIBC_COMPILER_HAS_FIXED_POINT) | |||
| libc.src.stdfix.kbits | |||
| libc.src.stdfix.ukbits | |||
| # TODO: https://github.com/llvm/llvm-project/issues/115778 | |||
There was a problem hiding this comment.
If this patch fixes this issue, you should remove this comment as well.
michaelrj-google
approved these changes
Nov 12, 2024
There was a problem hiding this comment.
@Caslyn requested this get merged to unblock the downstream builders. We can clean up the comment in a followup.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/10242 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/923 Here is the relevant piece of the build log for the reference |
|
Test failures are just flakes. Change seems to be passing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
int_lk_tto asigned long longand auint_ulk_tto anunsigned long longto guarantee they both fit in 8 bytes, whichlong _Accumandunsigned long _Accumare defaulted to on 32bit architectures.This is probably inconvenient on systems that have a word size larger than 64 bits?
#115778