View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0038339 | FPC | Compiler | public | 2021-01-10 00:28 | 2021-01-10 11:24 |
Reporter | J. Gareth Moreton | Assigned To | Florian | ||
Priority | normal | Severity | minor | Reproducibility | N/A |
Status | resolved | Resolution | fixed | ||
Platform | i386 and x86_64 | OS | Microsoft Windows | ||
Product Version | 3.3.1 | ||||
Fixed in Version | 3.3.1 | ||||
Summary | 0038339: [Patch] MovzxCmp2CmpMovzx oversight | ||||
Description | An apparent optimisation bug as listed in 0038334 is actually due to a different optimisation bug. The MovzxCmp2CmpMovzx optimisation didn't check to see if the cmp instruction actually used the same register as the movzx instruction. This patch corrects that. | ||||
Steps To Reproduce | Apply patch and confirm correct compilation and no regressions. | ||||
Additional Information | In aoptx86.s before: # [5806] if (reg in paramanager.get_volatile_registers_int(current_procinfo.procdef.proccalloption)) and movl U_$PROCINFO_$$_CURRENT_PROCINFO,%eax movl 24(%eax),%eax # Register dl allocated movb 85(%eax),%dl # Register eax released # Register eax allocated movl U_$PARAMGR_$$_PARAMANAGER,%eax # Register ecx allocated call CPUPARA$_$TCPUPARAMANAGER_$__$$_GET_VOLATILE_REGISTERS_INT$TPROCCALLOPTION$$TCPUREGISTERSET # Register ecx released movzwl %di,%edx # Peephole Optimization: cmpl $15,%edx -> cmpw $15,%ax (smaller and minimises pipeline stall - MovzxCmp2CmpMovzx) cmpw $15,%ax movzwl %ax,%eax # Register eflags allocated jbe .Lj3446 # Register eflags released clc jmp .Lj3447 After: # [5806] if (reg in paramanager.get_volatile_registers_int(current_procinfo.procdef.proccalloption)) and movl U_$PROCINFO_$$_CURRENT_PROCINFO,%eax movl 24(%eax),%eax # Register dl allocated movb 85(%eax),%dl # Register eax released # Register eax allocated movl U_$PARAMGR_$$_PARAMANAGER,%eax # Register ecx allocated call CPUPARA$_$TCPUPARAMANAGER_$__$$_GET_VOLATILE_REGISTERS_INT$TPROCCALLOPTION$$TCPUREGISTERSET # Register ecx released movzwl %di,%edx movzwl %ax,%eax # Register eflags allocated cmpl $15,%edx jbe .Lj3446 # Register eflags released clc jmp .Lj3447 ---- This shows how useful the more complex comments are, because it shows that the instruction "cmpw $15,%ax" was originally "cmpl $15,%edx", which should instantly ring alarm bells because the register changed. It was a complete coincidence that this fault only seemed to affect the JccAdd2SetccAdd optimisation (JccAdd2SetccAdd also affected a function in cutils, but this seemed to go undetected). | ||||
Tags | bugfix, compiler, i386, optimization, patch, x86_64 | ||||
Fixed in Revision | 48124 | ||||
FPCOldBugId | |||||
FPCTarget | - | ||||
Attached Files |
|
|
MovzxCmp2CmpMovzx-fix.patch (913 bytes)
Index: compiler/x86/aoptx86.pas =================================================================== --- compiler/x86/aoptx86.pas (revision 48117) +++ compiler/x86/aoptx86.pas (working copy) @@ -7522,7 +7522,8 @@ (taicpu(hp1).opcode = A_TEST) and MatchOperand(taicpu(hp1).oper[0]^, taicpu(hp1).oper[1]^) ) ) and - (reg2opsize(taicpu(hp1).oper[1]^.reg) <= reg2opsize(taicpu(p).oper[1]^.reg)) then + (reg2opsize(taicpu(hp1).oper[1]^.reg) <= reg2opsize(taicpu(p).oper[1]^.reg)) and + SuperRegistersEqual(taicpu(p).oper[1]^.reg, taicpu(hp1).oper[1]^.reg) then begin PreMessage := debug_op2str(taicpu(hp1).opcode) + debug_opsize2str(taicpu(hp1).opsize) + ' ' + debug_operstr(taicpu(hp1).oper[0]^) + ',' + debug_regname(taicpu(hp1).oper[1]^.reg) + ' -> ' + debug_op2str(taicpu(hp1).opcode); |
|
Thanks, applied. |
Date Modified | Username | Field | Change |
---|---|---|---|
2021-01-10 00:28 | J. Gareth Moreton | New Issue | |
2021-01-10 00:28 | J. Gareth Moreton | File Added: MovzxCmp2CmpMovzx-fix.patch | |
2021-01-10 00:28 | J. Gareth Moreton | Relationship added | related to 0038334 |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: patch | |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: compiler | |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: optimization | |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: i386 | |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: x86_64 | |
2021-01-10 04:02 | J. Gareth Moreton | Tag Attached: bugfix | |
2021-01-10 11:24 | Florian | Assigned To | => Florian |
2021-01-10 11:24 | Florian | Status | new => resolved |
2021-01-10 11:24 | Florian | Resolution | open => fixed |
2021-01-10 11:24 | Florian | Fixed in Version | => 3.3.1 |
2021-01-10 11:24 | Florian | Fixed in Revision | => 48124 |
2021-01-10 11:24 | Florian | FPCTarget | => - |
2021-01-10 11:24 | Florian | Note Added: 0128235 |