Skip to content
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

Merged
merged 7 commits into from
Oct 4, 2023

Conversation

Spydr06
Copy link
Member

@Spydr06 Spydr06 commented Oct 3, 2023

This PR adds support for the $if native {} conditional variable, which is then used in the builtin module to differentiate between the C and native backends.

Also, there is improved error and conditional handling of ast.HashStmts in native. 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

  • Add a new comptime if condition, native, to check if the native backend is enabled, and implement the logic for it in the comptime modules (link, link, link)
  • Skip generating code for hash statements or comptime if branches that are not met by the native backend, and collect the external dependencies of the program, such as the external symbols, the linker paths, and the linker libraries, from the AST nodes (link, link, link, link, link, link, link, link)
  • Append the custom linker paths and libraries to the linker command, and handle the external symbols for the ELF and PE formats (link, link, link, link, link, link, link)
  • Replace the FormatMessage function with the FormatMessageW 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)
  • Replace the hard-coded values of the console mode flags and the code page constant with named constants, and avoid using the C macros, in the Windows modules (link, link, link)
  • Replace the hard-coded values of the floating-point constants with reasonable approximations, and avoid using the C macros, in the float module (link, link, link)
  • Return the whole branch struct, instead of just the statements, from the comptime_conditional function, and use the branch variable instead of the stmts variable, in the native modules (link, link, link, link)
  • Print a message that the native backend does not support backtraces yet, and exit with code 1, in the builtin module, and move the code for checking the no_backtrace conditional to the backtraces module (link, link, link)
  • Add a new condition to the $if tinyc conditional, which is !native, to prevent calling the tcc_backtrace function when the native backend is enabled, in the backtraces_windows module (link)
  • Provide the correct file path for the error location, instead of the main file path, when generating an error message, in the gen module (link)
  • Test the new feature of the native backend, and check if the native condition works as expected, in the comptime test file (link)

@Spydr06
Copy link
Member Author

Spydr06 commented Oct 3, 2023

please merge #19500 first to fix the bootstrapping errors

@spytheman
Copy link
Member

spytheman commented Oct 3, 2023

please merge #19500 first to fix the bootstrapping errors

done, please rebase over master (or merge it), then push again

@spytheman spytheman changed the title native, builtin, ast: add $if native {} codition, handle ast.HashStmt correctly and reduce macro usage in builtin native, builtin, ast: handle ast.HashStmt correctly and reduce macro usage in builtin Oct 3, 2023
return (1 / (4 * f32(C.FLT_EPSILON))) * delta <= hi
$if native {
if hi > f32(1.0) {
return delta <= hi * (4 * 1e-5)
Copy link
Member

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 .

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member

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)
Copy link
Member

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work.

@spytheman spytheman merged commit 6d1558b into vlang:master Oct 4, 2023
46 checks passed
@Spydr06 Spydr06 deleted the native-builtin-stuff branch October 4, 2023 15:04
Spydr06 added a commit to Spydr06/v that referenced this pull request Oct 4, 2023
spytheman pushed a commit that referenced this pull request Oct 4, 2023
Wertzui123 pushed a commit to Wertzui123/v that referenced this pull request Oct 8, 2023
Wertzui123 pushed a commit to Wertzui123/v that referenced this pull request Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants