User Details
- User Since
- Sep 21 2020, 2:09 PM (141 w, 1 d)
Today
LGTM
Yesterday
This should probably be more similar to the putc test, which writes to a file so that the result can be checked.
Mon, Jun 5
LGTM
I like the general idea of having a more thorough test for pthreads as a whole, but it should probably be in a separate file instead of replacing the pthread_join test. I think it would fit better in the pthread_test file.
Every platform we support gets broken occasionally by patches that forget to take it into account. Without buildbots telling us when the platform is broken, then we can't fix it. An example for this would be the arm32 failing to update for several months and our strtol tests breaking on it, leading to this patch: https://reviews.llvm.org/D151935.
For this reason we require anyone adding a new platform to demonstrate their plan for supporting that platform through CI. You can look at the "CI Builders" section on this page for more details: https://libc.llvm.org/porting.html
Ideally we would not like to replace the current optional implementation. Instead we would prefer to modify the current optional so that it supports the features that we need, and only replace it if modification isn't practical. Can you explain why you think that modifying the existing implementation of optional cannot work for your use case?
Fri, Jun 2
The patch is fine, but I am confused as to how the str_to_float tests were passing with the high_precision_decimal tests failing, given that str_to_float uses high_precision_decimal.
Thu, Jun 1
Tue, May 30
Thu, May 25
LGTM
LGTM
address comments, still need to mess with the is_lowest_block and zero_blocks_after_point functions. They aren't strictly necessary, but they do provide significant speed improvements.
updates and cleanup
Wed, May 24
LGTM
Tue, May 23
LGTM
Mon, May 22
LGTM
Fri, May 19
changed how MPFR is included to support downstream configs in the same way as MPFRUtils
Thu, May 18
cleanup for all three modes after some testing.
LGTM
LGTM
I'll test this patch on aarch64 to check the float128 support
Abandoned in favor of https://reviews.llvm.org/D150905
LGTM
Wed, May 17
LGTM
Tue, May 16
LGTM
Mon, May 15
Fri, May 12
LGTM, thank you for adding this!
Thu, May 11
Currently ryu_long_double_constants is empty, this is because it's 12 megabytes and arcanist complains when I try to upload it. I can add it for testing purposes, or you can generate it yourself with ryu_tablegen.py. It writes to the console so pipe it to a file. You'll also need to set POW10_ADDITIONAL_BITS_TABLE to 120, IDX_SIZE to 128, and MID_INT_SIZE to 320 in ryu_long_double_constants.h.
Tue, May 9
I agree that optional supporting UInt is good, my question is whether replacing the existing implementation with a completely new one is the best strategy.
Before I review this patch, I want to confirm that this is the right strategy. If the main goal is to support Uint<> in optional is there a way to do that without a complete replacement of optional?
May 8 2023
I think you meant to put me as a reviewer instead of the description.
May 4 2023
May 3 2023
add comment to clarify the variable named "a"
fix issues that came up in testing before landing
May 2 2023
LGTM
address comments and fix how types are set up.
May 1 2023
Apr 28 2023
move time_t change into separate patch.
address comments
Apr 27 2023
switch to system atof for exception detection.
Apr 26 2023
LGTM with a nit.
Apr 25 2023
remove unnecessary char* initializer for cstring and rename the header to c_string.h to clarify that this isn't related to cstring (the libc++ header).
That's fine, I'll land this for you.
Apr 24 2023
move to LIBC_INLINE
LGTM, thank you for catching this.
address comments
address comments
LGTM
we should probably figure out what the appropriate waitpid errno values are before landing this. It's possible that the syscall returns them as negative numbers, but I haven't tested it yet.
Overall LGTM, though are we sure that having the riscv 64 bit and 32 bit targets is the correct path? On other platforms so far we've separated the 32 bit and 64 bit targets.
Apr 20 2023
LGTM