Skip to content

x86asm: add support for FLDZ and FLDLN2 #3

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

Closed
wants to merge 3 commits into from

Conversation

mewmew
Copy link
Contributor

@mewmew mewmew commented Nov 13, 2018

@gopherbot
Copy link
Contributor

This PR (HEAD: 5603d0e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/arch/+/149438 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Cherry Zhang:

Patch Set 1:

(1 comment)

Thanks for the CL. Could we add a test?


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from foo bar:

Patch Set 1:

Patch Set 1:

(1 comment)

Thanks for the CL. Could we add a test?

Hi Cherry!

Sure, I could add a test.

However, based on @quasilyte comment in golang/go#18665 (comment) it seems this may be the wrong approach for adding support for FLDZ and FLDLN2.

Inlining @quasilyte's comment below:

I'm not sure editing auto-generated file is a good idea though
and it doesn't solve the problem with other missing instructions in the x86.csv.
This discussion can be continued in a CL itself.

Kindly,
Robin


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 97cdbbe) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/arch/+/149438 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: fa98ffe) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/arch/+/149438 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from foo bar:

Patch Set 3:

Patch Set 1:

(1 comment)

Thanks for the CL. Could we add a test?

I've now rebased this CL on top of master and added test cases for fldz and fldln2, both of which are passing.

Is there anything else needed before merging this?

Cheers,
Robin


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Zhang:

Patch Set 3: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 3:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=174c7efd


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 3: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/149438.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Nov 26, 2019
Fixes golang/go#18665.

Change-Id: Ia628773266d3e5efba252b910341d00b7fdb7081
GitHub-Last-Rev: fa98ffe
GitHub-Pull-Request: #3
Reviewed-on: https://go-review.googlesource.com/c/arch/+/149438
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/149438 has been merged.

@gopherbot gopherbot closed this Nov 26, 2019
srinivas-pokala added a commit to srinivas-pokala/arch that referenced this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/arch/x86/x86asm: missing instructions: FLDZ, FLDLN2
3 participants