Regression in optimizations/predictions of fasmg

Index > Compiler Internals > Regression in optimizations/predictions of fasmg

Author

Thread

VEG

Joined: 06 Feb 2013
Posts: 80

VEG 27 Dec 2017, 19:15

BTW, there is some regression between versions g.hx7tl and g.hxmfv. One of jz had been encoded as a two-byte instruction (74 7D), but after some change in the fasmg it started to assemble as a 6-byte instruction (0F 84 7D 00 00 00), from the same source code. I'll try to reproduce it in a smaller file outside of my project later.

The simplest case

Code:

jz lbl
repeat 0x7D
        nop
end repeat
lbl:

works nice, so the problem is definitely more complicated.

27 Dec 2017, 19:15

Tomasz Grysztar

Joined: 16 Jun 2003
Posts: 8359
Location: Kraków, Poland

Tomasz Grysztar 27 Dec 2017, 21:27

If you'd like to pinpoint what change caused the effect you observe, please use the Fossil repository of fasmg.

27 Dec 2017, 21:27

VEG

Joined: 06 Feb 2013
Posts: 80

VEG 28 Dec 2017, 09:47

OK, I did it. This problem appeared exactly in the version g.hxmfv (revision 405f908cac), because the previous one (g.hxhsr, revision d7f8ec884e) doesn't have this problem.

28 Dec 2017, 09:47

Tomasz Grysztar

Joined: 16 Jun 2003
Posts: 8359
Location: Kraków, Poland

Tomasz Grysztar 28 Dec 2017, 10:54

Version hxmfv is in the "lowmem" branch, this is a separate fork of fasmg that I made to test fasmg self-hosting on a machine with little RAM, and the changes made there may have some side-effects affecting the prediction heuristics. Please compare the versions in trunk only, they are the official releases.

28 Dec 2017, 10:54

VEG

Joined: 06 Feb 2013
Posts: 80

VEG 28 Dec 2017, 11:44

OK, I'll do it. But g.hxmfv had been published on the downloads page, so I thought that it is an official release.

Last edited by VEG on 28 Dec 2017, 12:44; edited 1 time in total

28 Dec 2017, 11:44

VEG

Joined: 06 Feb 2013
Posts: 80

VEG 28 Dec 2017, 12:43

ed742ff89c doesn't have the problem.
ab6f50673b does have the problem.
https://flatassembler.net/fossil/repo/fasmg/info/ab6f50673b5cd584

The only change (according to diff):

Code:

Index: source/symbols.inc
==================================================================
--- source/symbols.inc
+++ source/symbols.inc
@@ -495,13 +495,10 @@
 ;  ebx - SymbolTree_Leaf
 ;  edx - SymbolTree_Foliage
 ;  edi - SymbolTree_Root
        mov     [symbol_class],dl
        test    ebx,ebx
-       jnz     scope_selected
-       mov     ebx,[local_namespace]
-       test    ebx,ebx
        jnz     scope_selected
        mov     ebx,[current_namespace]
     scope_selected:
        mov     esi,eax
        mov     edx,FNV_OFFSET

28 Dec 2017, 12:43

Tomasz Grysztar

Joined: 16 Jun 2003
Posts: 8359
Location: Kraków, Poland

Tomasz Grysztar 28 Dec 2017, 12:53

This was an important bug-fix that unfortunately may also have a similar effect on the quality of predictions for some local symbols. It may not be easy to work around it without bringing back the problem that seriously affected performance.

28 Dec 2017, 12:53

Tomasz Grysztar

Joined: 16 Jun 2003
Posts: 8359
Location: Kraków, Poland

Tomasz Grysztar 28 Dec 2017, 13:14

Please try applying the following patch and see if it has any effect:

Code:

--- source/directives.inc
+++ source/directives.inc
@@ -3335,13 +3335,12 @@
     macro_header_measured:
        mov     [instruction_body],esi
        dec     esi
        mov     [line_end],esi
        mov     eax,ebx
-       push    eax
+       mov     ecx,[local_namespace]
        xor     ebx,ebx
-       xor     ecx,ecx
        mov     dl,SYMCLASS_PARAMETER
        call    get_abstract_symbol
        mov     edx,[ebx+SymbolTree_Leaf.definition]
        test    edx,edx
        jz      create_namespace_counter
@@ -3360,13 +3359,13 @@
        inc     [edx+ValueDefinition.reference_count]
        mov     [edx+ValueDefinition.type],VALTYPE_PLAIN
        mov     ecx,[current_pass]
        mov     [edx+ValueDefinition.pass],ecx
     create_local_namespace:
        inc     [edx+ValueDefinition.value]
        mov     ecx,[edx+ValueDefinition.value]
-       pop     eax
+       mov     eax,ebx
        xor     ebx,ebx
        mov     dl,SYMCLASS_PARAMETER
        call    get_abstract_symbol
        call    get_local_namespace
        mov     [new_local_namespace],ebx

In addition to that, please also try changing this line:

Code:

+       mov     ecx,[local_namespace]

Code:

+       mov     ecx,[parameter_namespace]

and see if the two variants behave differently on your source.

28 Dec 2017, 13:14

Tomasz Grysztar

Joined: 16 Jun 2003
Posts: 8359
Location: Kraków, Poland

Tomasz Grysztar 28 Dec 2017, 20:06

I did some testing and noticed that this patch definitely fixes at least some of the incompatible behavior caused by that fix, so I decided to publish it as an official release anyway (it is "i17rx"). But please still let me know if it fixes your issue.

28 Dec 2017, 20:06

VEG

Joined: 06 Feb 2013
Posts: 80

VEG 29 Dec 2017, 05:55

Tomasz Grysztar, yeah, it fixed my problem also. Thank you.

29 Dec 2017, 05:55

< Last Thread | Next Thread >

Forum Rules:

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You can download files in this forum