Skip to content

Conversation

czadowanie
Copy link

Hello :)

Fixes #20955, I ran into it myself. Now I'm not certain about my x86_64 encoding skills (I just tried to follow how R/Ms are placed on https://www.felixcloutier.com/x86, extrapolated the rest). The sample given in the issue does compile and run though (so does zig itself built with this branch (at least on x86_64-linux-gnu)).

I also took the liberty to add some logging whenever the encoder can't find an instruction based on a mnemonic. It helped me spot the issue, I hope it might be useful going forward.

prefix: Instruction.Prefix,
mnemonic: Mnemonic,
ops: []const Instruction.Operand,
show_canditates: bool,
Copy link

Choose a reason for hiding this comment

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

candidates

@andrewrk
Copy link
Member

Hi @czadowanie, thanks for the patch.

@jacobly0 has a rather large branch of this backend that is the expected path forward. Would you mind checking if the issue reproduces against that branch, and if so, consider sending the patch against that branch instead of master?

I don't know if Jacob is looking for collaboration on the branch, or wants to try to finish it solo, but the first step is to ask and find out.

@jacobly0
Copy link
Member

jacobly0 commented Jan 16, 2025

It would be easier to tell if there were tests I could check, but this does not look even slightly close to the correct fix to the issue, so I'm not very inclined to manually verify. I would suggest starting with a small repro source file that has an extern function not called main, build with zig build-obj -fno-llvm -fno-lld repro.zig and verify the output with objdump -d repro.o to make sure it was encoded correctly.

@czadowanie
Copy link
Author

@jacobly0 Yeah, turns out the sample in the issue doesn't in fact work with my patch. It should produce -100, but produces 100.

const S = struct { x: f32, y: ?f32 };

noinline fn bug(self: S) f32 {
    return if (self.y) |y| self.x - y else 0.0;
}

export fn exported(x: f32, y: f32, has_y: bool) f32 {
    return bug(S{ .x = x, .y = if (has_y) y else null });
}

The rewrite still rejects it for the same reason but after copy-pasting takesRegisterOrMemoryAndCanFit produces this:

  ...
  c1:   c5 fa 5c c0             vsubss xmm0,xmm0,xmm0 ; c0 being eax in disguise
  ...

Sorry.

@czadowanie czadowanie closed this Jan 17, 2025
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.

x86 backend compile bug: error(x86_64_encoder): no encoding found for: none vsubss xmm0 xmm0 r32 none

3 participants