Skip to content

[MIPS] Opcode of mina.fmt and max.fmt in MIPS R6 are wrong #85608

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
Cyanoxygen opened this issue Mar 18, 2024 · 2 comments
Closed

[MIPS] Opcode of mina.fmt and max.fmt in MIPS R6 are wrong #85608

Cyanoxygen opened this issue Mar 18, 2024 · 2 comments

Comments

@Cyanoxygen
Copy link
Contributor

Cyanoxygen commented Mar 18, 2024

While proposing #64207 (comment), I discovered that the opcode of two FPU compare instructions are documented wrong, and the mistake is present in LLVM code.

The opcode of mina.fmt and max.fmt should be swapped, according to binutils:

{"max.d",               "D,S,T",        0x4620001e, 0xffe0003f, WR_1|RD_2|RD_3|FP_D,    0,              I37,            0,      0 },
{"mina.s",              "D,S,T",        0x4600001d, 0xffe0003f, WR_1|RD_2|RD_3|FP_S,    0,              I37,            0,      0 },

And the program misbehaves with code compiled by LLVM.
Consider the following code which contains min.fmt and max.fmt:

owo:
    min.s $f0, $f1, $f0
    min.d $f0, $f1, $f0
    max.s $f0, $f1, $f0
    max.d $f0, $f1, $f0

Expected behavior

gcc -o minmax-valid.o -c owo.s
objdump of the code above should print the following encoding:

0000000000000000 <owo>:
   0:   4600081c        min.s   $f0,$f1,$f0
   4:   4620081c        min.d   $f0,$f1,$f0
   8:   4600081e        max.s   $f0,$f1,$f0
   c:   4620081e        max.d   $f0,$f1,$f0

Actual behavior

clang -o minmax-invalid.o -c owo.s
objdump of the binary compiled from the code above with LLVM showed mina.fmt instead of max.fmt:

0000000000000000 <owo>:
   0:   4600081c        min.s   $f0,$f1,$f0
   4:   4620081c        min.d   $f0,$f1,$f0
   8:   4600081d        mina.s  $f0,$f1,$f0
   c:   4620081d        mina.d  $f0,$f1,$f0

Note

Optionally, we can match @llvm.minnum.f{32,64} and @llvm.maxnum.f{32,64} to these instructions, as it behaves similar to UCOMISD on x86, I suppose.

The fix is on its way.

@Cyanoxygen Cyanoxygen changed the title [MIPS] Opcode of mina.fmt and max.fmt in MIPS64 R6 are wrong [MIPS] Opcode of mina.fmt and max.fmt in MIPS R6 are wrong Mar 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 18, 2024

@llvm/issue-subscribers-backend-mips

Author: Cinhi Young (Cyanoxygen)

While proposing https://github.com//issues/64207#issuecomment-1989915063, I discovered that the opcode of two FPU compare instructions are documented wrong, and the mistake is present in LLVM code.

The opcode of mina.fmt and max.fmt should be swapped, according to binutils:

{"max.d",               "D,S,T",        0x4620001e, 0xffe0003f, WR_1|RD_2|RD_3|FP_D,    0,              I37,            0,      0 },
{"mina.s",              "D,S,T",        0x4600001d, 0xffe0003f, WR_1|RD_2|RD_3|FP_S,    0,              I37,            0,      0 },

And the program misbehaves with code compiled by LLVM.
Consider the following code which contains min.fmt and max.fmt:

owo:
    min.s $f0, $f1, $f0
    min.d $f0, $f1, $f0
    max.s $f0, $f1, $f0
    max.d $f0, $f1, $f0

Expected behavior

gcc -o minmax-valid.o -c owo.s
objdump of the code above should print the following encoding:

0000000000000000 &lt;owo&gt;:
   0:   4600081c        min.s   $f0,$f1,$f0
   4:   4620081c        min.d   $f0,$f1,$f0
   8:   4600081e        max.s   $f0,$f1,$f0
   c:   4620081e        max.d   $f0,$f1,$f0

Actual behavior

clang -o minmax-invalid.o -c owo.s
objdump of the binary compiled from the code above with LLVM showed mina.fmt instead of max.fmt:

0000000000000000 &lt;owo&gt;:
   0:   4600081c        min.s   $f0,$f1,$f0
   4:   4620081c        min.d   $f0,$f1,$f0
   8:   4600081d        mina.s  $f0,$f1,$f0
   c:   4620081d        mina.d  $f0,$f1,$f0

Note

Optionally, we can match @<!-- -->llvm.minnum.f{32,64} and @<!-- -->llvm.maxnum.f{32,64} to these instructions, as it behaves similar to UCOMISD on x86, I suppose.

The fix is on its way.

@wzssyqa
Copy link
Contributor

wzssyqa commented Apr 3, 2024

The PR has been merged. Let's close this bug report.

@wzssyqa wzssyqa closed this as completed Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants