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 |
