Fixed barrier_next setting #1707
Conversation
|
@ptitSeb Excuse me, is there any problem with this pr? Please let me know, thanks! |
| @@ -210,6 +210,7 @@ uintptr_t dynarec64_D9(dynarec_rv64_t* dyn, uintptr_t addr, uintptr_t ip, int ni | |||
|
|
|||
| case 0xE8: | |||
| INST_NAME("FLD1"); | |||
| LAST_BARRIER_NEXT(BARRIER_FLOAT); | |||
There was a problem hiding this comment.
I don't undrstand why a FLD1 or FLDZ would need a float barrier_next ????
There was a problem hiding this comment.
No, it's not that FLD1 needs a float barrier_next, it's that when the current instruction is FLD1, its previous instruction's barrier_next is float, and FLD1's barrier is float, I don't know if I understand correctly, the previous instruction's barrier_next should be the same as the current instruction's barrier
There was a problem hiding this comment.
To know what the barrier_next of the previous instruction should be set to, one must be aware of what the current instruction is.
There was a problem hiding this comment.
This is wrong. Barrier next is not use a lot, and should only be used in rare case when float state needs to be synchronized. Basicaly around the CALL opcode. And it's not used on RV64 backend unless I missed something. So I'm not sure what you are trying to fix
(side note, I'll probably removed BARRIER_NEXT, it's not usefull anymore)
There was a problem hiding this comment.
I'm sorry, that may have been my misunderstanding. The problem I'm trying to solve is that the status word is incorrect when a floating point instruction such as FLD1 or FLDZ is executed, which can be proven by testing ‘FNSTSW AX’.
There was a problem hiding this comment.
#include <stdio.h>
#include <stdint.h>
void fnstsw_ax() {
uint16_t sw;
asm volatile (
"fnstsw %0\n"
: "=a"(sw)
);
printf("Initial Status Word: 0x%04x\n", sw);
asm volatile (
"fld1\n"
"fnstsw %0\n":"=a"(sw)
);
printf("Status Word After FLD1: 0x%04x\n", sw);
asm volatile (
"fldz\n"
"fnstsw %0\n":"=a"(sw)
);
printf("Status Word After FLDZ: 0x%04x\n", sw);
asm volatile (
"faddp\n"
"fnstsw %0\n":"=a"(sw)
);
printf("Status Word After FADDP: 0x%04x\n", sw);
asm volatile (
"fnstsw %0\n"
:"=a"(sw)
);
printf("Final Status Word: 0x%04x\n", sw);
}
int main() {
fnstsw_ax();
return 0;
}
Here is my test code, the printf results are different than on X86
There was a problem hiding this comment.
FLD1/FLDZ should only change the "TOP" part of the status word (so bit 11-13). The other bits are not really emulated. What kind of difference do you have (I'm in holyday and don't have x86 or Rv64 hardware currently, only Arm64).
There was a problem hiding this comment.
But the top value is wrong now.
Here are the results on x86：
Initial Status Word:0x0000
Status Word After FLD1:0x3800
Status Word After FLDZ:0x3000
Status Word After FADDP:0x3800
Final Status Word: 0x3800
Here are the results on the RV64：
Initial Status Word:0x0000
Status Word After FLD1:0x0000
Status Word After FLDZ:0x3800
Status Word After FADDP:0x3000
Final Status Word: 0x3800
After executing FLD1, the top value should be 7, but it is still 0.
There was a problem hiding this comment.
I'll check this next week. Can't test for now. But the solution is not a "BARRIER_FLOAT" anyway, it's something else.
There was a problem hiding this comment.
Ok, thanks, I'll keep an eye out for code updates!
|
Sorry, I didn't noticed the PR before. I'm fine with the move of the BARRIER_NEXT macro from beofre INST_EPILOG to after INST_EPILOG, but I don't understand the LAST_BARRIER_NEXT... |
|
When testing the “FNSTSW AX” instruction, I found that the results were inconsistent with the x86 instruction. The reason for this is that barrier_next and barrier are not set in native_pass0, while in native_pass “ #if STEP == 0
if(ninst && dyn->insts[ninst-1].x64.barrier_next) {
BARRIER(dyn->insts[ninst-1].x64.barrier_next);
}
#endif “ The if judgment of this code never passes, I think this is problematic, every floating point and FPU status word operation instruction's barrier should be set with its previous instruction's barrier_next, so I made some modifications, but only for the instructions I use for testing, if the modification is reasonable, I will continue to improve it, if the modification is not reasonable, I will continue to improve it, if the modification is not reasonable, I will continue to improve it. , I will continue to improve it, if the modification is not reasonable, you can reject this pull request, but please be aware of this issue, the current “FNSTSW AX” instruction will cause an error.