-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
native, builtin, ast: handle ast.HashStmt
correctly and reduce macro usage in builtin
#19498
Conversation
please merge #19500 first to fix the bootstrapping errors |
done, please rebase over master (or merge it), then push again |
$if native {}
codition, handle ast.HashStmt
correctly and reduce macro usage in builtin
ast.HashStmt
correctly and reduce macro usage in builtin
return (1 / (4 * f32(C.FLT_EPSILON))) * delta <= hi | ||
$if native { | ||
if hi > f32(1.0) { | ||
return delta <= hi * (4 * 1e-5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Ubuntu 20.04, __FLT_EPSILON__
is 1.19209290e-7F
with clang and 1.19209289550781250000000000000000000e-7F
with gcc .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But don't we also have other compilers (tcc
, msvc
)? Also, I expect this to differ between different architectures. I used these values as temporary ones suggested by the comments above that function, I'll have to implement some sort of intrinsic that has a better value for each native backend arch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it depends mostly on IEEE 754, i.e. it should be approximately the same for the different compilers, just the source representations of it are a bit different (and whether it comes from a builtin macro, or it is defined in a system header like float.h
).
1e-5
is 0.00001
, i.e. over 80 times bigger than 1.19209290e-7
i.e. 0.000000119
, and thus with 1e-5
, a.eq_epsilon(b)
will be coarser, than it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return (1 / (4 * f64(C.DBL_EPSILON))) * delta <= hi | ||
$if native { | ||
if hi > 1.0 { | ||
return delta <= hi * (4 * 1e-9) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang uses 2.2204460492503131e-16
for __DBL_EPSILON__
and gcc uses
((double)2.22044604925031308084726333618164062e-16L)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #19498 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, __DBL_EPSILON__
is a builtin, but DBL_EPSILON
is defined either as a macro for it, or as a macro expanded to 2.2204460492503131e-16
, defined in float.h
or similar. It is only slightly compiler dependent, IEEE 754 is more relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work.
… `vcommithash()` in favor of `@VHASH` (#19508)
…o usage in `builtin` (vlang#19498)
…emove `vcommithash()` in favor of `@VHASH` (vlang#19508)
This PR adds support for the
$if native {}
conditional variable, which is then used in thebuiltin
module to differentiate between the C and native backends.Also, there is improved error and conditional handling of
ast.HashStmt
s innative
. Additionally, there is now support for#flag -l foo
and#flag -L foo
statements on windows and linux.🤖 Generated by Copilot at 4346ef2
This pull request adds support for the native backend in various modules of the V standard library and the V compiler. It introduces a new comptime if condition,
native
, to allow conditional compilation based on the backend choice. It also improves the compatibility and consistency of the Windows-specific code by using Unicode functions and constants. It modifies the code generation and the ELF and PE format handling to support external dependencies and custom linker paths. It refactors the comptime conditional logic and the hash statement processing to avoid unnecessary code and errors. It adds TODO comments for the native backtrace implementation, which is not supported yet.🤖 Generated by Copilot at 4346ef2
native
, to check if the native backend is enabled, and implement the logic for it in the comptime modules (link, link, link)FormatMessage
function with theFormatMessageW
function, which is the Unicode version of the same function, in the Windows modules, and update the reference URL for it (link, link, link, link, link)float
module (link, link, link)comptime_conditional
function, and use thebranch
variable instead of thestmts
variable, in the native modules (link, link, link, link)builtin
module, and move the code for checking theno_backtrace
conditional to thebacktraces
module (link, link, link)$if tinyc
conditional, which is!native
, to prevent calling thetcc_backtrace
function when the native backend is enabled, in thebacktraces_windows
module (link)gen
module (link)native
condition works as expected, in thecomptime
test file (link)