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

Start adding frontend support for relocations #782

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Conversation

ehaas
Copy link
Collaborator

@ehaas ehaas commented Oct 5, 2024

Add support for initializing pointers with, and doing static asserts with, offsets from global variables.

I'm very open to bikeshedding on names of things / code organization. I moved StringInterner to backend so that Interner could hold StringId values.

Open areas for enhancement:

  • Should also support static variables and not just globals.
  • Model relocation values with name + bigint instead of name + i64. Right now if the offset overflows we correctly warn but the printed diagnostic is incorrect since the value wraps around.

Copy link
Owner

@Vexu Vexu left a comment

Choose a reason for hiding this comment

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

I don't love moving StringInterner to the backend since it isn't needed as much there. I'd prefer it to be something more abstract like:

pub const Ptr = struct {
    decl: DeclHandleTypeOfSomeSort,
    offset: Ref,
};

but I don't want to hold you back on my unfinished code maybe something like using @intFromEnum(decl_name_id) would work for now?

Looks good otherwise!

@@ -927,6 +1003,10 @@ pub fn compare(lhs: Value, op: std.math.CompareOperator, rhs: Value, comp: *cons
return lhs_bigint.order(rhs_bigint).compare(op);
}

pub fn compare(lhs: Value, op: std.math.CompareOperator, rhs: Value, comp: *const Compilation) bool {
return lhs.compareExtra(op, rhs, comp).?;
Copy link
Owner

Choose a reason for hiding this comment

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

This looks dangerous. Is it expected that other places won't reach this or is handling it just out of scope for this PR? Shouldn't be a blocker but a comment reminder wouldn't hurt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think other paths could reach that, but I changed it to use a separate function for comparing pointers. In doing so I discovered a silly oversight with subtraction where I'd switched the operand order :)

@ehaas ehaas force-pushed the reloc branch 2 times, most recently from 6e8bdbb to 0500ef2 Compare October 8, 2024 20:04
@ehaas
Copy link
Collaborator Author

ehaas commented Oct 9, 2024

Did MacOS CI always run on arm64 or did it used to be x86_64? The CI failure also happens on master so I'm trying to figure out if it was always lurking in aro or if something in zig changed that is causing it.

@ehaas
Copy link
Collaborator Author

ehaas commented Oct 9, 2024

This patch fixes it, I think:

diff --git a/src/aro/Parser.zig b/src/aro/Parser.zig
index 21df662..69540ac 100644
--- a/src/aro/Parser.zig
+++ b/src/aro/Parser.zig
@@ -5803,13 +5803,10 @@ pub const Result = struct {
                 if (try a.floatConversion(b, a_spec, b_spec, p, float_types[0])) return;
             }
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[1])) return;
-            if (p.comp.target.cTypeBitSize(.longdouble) == 80) {
+            if (p.comp.target.cTypeBitSize(.longdouble) == 80 or p.comp.target.cTypeBitSize(.longdouble) == 64) {
                 if (try a.floatConversion(b, a_spec, b_spec, p, float_types[0])) return;
             }
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[2])) return;
-            if (p.comp.target.cTypeBitSize(.longdouble) == 64) {
-                if (try a.floatConversion(b, a_spec, b_spec, p, float_types[0])) return;
-            }
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[3])) return;
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[4])) return;
             if (try a.floatConversion(b, a_spec, b_spec, p, float_types[5])) return;

@Vexu Vexu merged commit 69e30f4 into Vexu:master Oct 9, 2024
2 of 3 checks passed
@Vexu
Copy link
Owner

Vexu commented Oct 9, 2024

Did MacOS CI always run on arm64 or did it used to be x86_64? The CI failure also happens on master so I'm trying to figure out if it was always lurking in aro or if something in zig changed that is causing it.

The x86-64 MacOS runner has been deprecated and it was probably switched when I updated to actions/checkout v3 in 59e6078

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