FASMW.ASM

Index > Compiler Internals > FASMW.ASM

Author

Thread

ouadji

Joined: 24 Dec 2008
Posts: 1081
Location: Belgium

ouadji 07 Jun 2010, 23:19

Code:

original (inside FASMW.asm)
...
  .syntax_string:
     mov     al,3
        stosb
       dec     ecx
 jz      .done
       lodsb
       cmp     al,ah
       jne     .syntax_string
      mov     al,3
        stosb
       dec     ecx
 jz      .done
       lodsb                           |(**)
       cmp     al,ah                   |
   je      .syntax_string          |
   xor     edx,edx   ;<------- (*)      |
   jmp     .check_character        |
...
---------------------------------------------------------------
(*) 
useless! this code isn't called if "edx" is different from 0,
and "edx" is not used.

(**)
...
      lodsb
       cmp     al,ah
       je      .syntax_string
      xor     edx,edx
     jmp     .check_character
...
why ? just "jmp .scan_syntax" !!! it's perfect.

Code:

ouadji (it's 100% ok too)
...
   mov     al,3
        stosb
       dec     ecx
 jz      .done
       lodsb
       cmp     al,ah
       jne     .syntax_string
      mov     al,3
        stosb
       dec     ecx
 jz      .done           
    jmp     .scan_syntax
...

it's not very important, and it didn't affect the fasm effectiveness
but "FASM" sometimes lack basic optimizations
why not delete these useless lines of code ?
there are many cases like this in FASM, why ?

_________________
I am not young enough to know everything (Oscar Wilde)-

07 Jun 2010, 23:19

Tyler

Joined: 19 Nov 2009
Posts: 1216
Location: NC, USA

Tyler 07 Jun 2010, 23:30

Maybe it was to make for easy maintaining/expanding. Maybe vestigial structures Tomasz never calls, but forgot to delete.

07 Jun 2010, 23:30

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 07 Jun 2010, 23:35

Quote:

useless! this code isn't called if "edx" is different from 0,
and "edx" is not used.

Odd, I have this:

Code:

  .scan_syntax:
        lodsb
  .check_character:
        cmp     al,20h
        je      .syntax_space
        cmp     al,3Bh
        je      .syntax_comment
        mov     ah,al
        xlatb
        or      al,al
        jz      .syntax_symbol
        or      edx,edx
        jnz     .syntax_neutral ; THIS PART CONSUMES EDX
        cmp     ah,27h
        je      .syntax_string
        cmp     ah,22h
        je      .syntax_string

The way it is now you couldn't enter .syntax_string twice. But maybe there is something more you saw that ensures that EDX is really unneeded?

07 Jun 2010, 23:35

ouadji

Joined: 24 Dec 2008
Posts: 1081
Location: Belgium

ouadji 07 Jun 2010, 23:49

yes, but at the beginning of ". syntax_string", "edx" is always 0.
it's the condition for testing "ah," and to be able to call ". syntax_string"

Code:

...
      or      edx,edx
     jnz     .syntax_neutral
     cmp     ah,27h
      je      .syntax_string ;---> edx=0
       cmp     ah,22h
      je      .syntax_string ;---> edx=0
...
.syntax_string: ... edx is always 0.

Quote:

THIS PART CONSUMES EDX

"edx" is not used .....inside ".syntax_string".

_________________
I am not young enough to know everything (Oscar Wilde)-

07 Jun 2010, 23:49

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 01:26

Damn, sorry, I've looked the jmp points wrong and thought that the EDX reset was needed.

Still, I'll complain a little more Razz

Is your "ouadji (it's 100% ok too)" code OK? It seems to be using "jmp .scan_syntax" to take advantage of the lodsb there, but note that the original code did something different with AL and AH than the main loop code does. Removing "xor edx, edx" only from the original code is really something that could be done with no trouble.

08 Jun 2010, 01:26

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jun 2010, 02:37

The EDX is used as a flag for later.
Follow the code into .check_character

08 Jun 2010, 02:37

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 03:07

bitshifter wrote:

The EDX is used as a flag for later.
Follow the code into .check_character

But he was right, EDX is always zero at .syntax_string entry and is never touched except for that XOR EDX, EDX that won't produce a perceptible change in EDX ever.

08 Jun 2010, 03:07

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jun 2010, 03:16

It can be non-zero...

Code:

.syntax_neutral:
        or      edx,-1
        ...

08 Jun 2010, 03:16

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 03:26

Yes, I fell into the same trap, follow the code more carefully, you'll see that it is impossible for .syntax_string to be reached with EDX != 0.

08 Jun 2010, 03:26

revolution
When all else fails, read the source

Joined: 24 Aug 2004
Posts: 20451
Location: In your JS exploiting you and your system

revolution 08 Jun 2010, 03:28

Hehe, much ado about ...

If there is a bug then say so. Else, what are you all discussing?

Question

Code:

if there is a bug then
    say so.
else
    what are you all discussing?
end if

08 Jun 2010, 03:28

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 03:34

There is no bug at all here, ouadji just proposed the removal of a redundant (but yet not buggy) instruction.

If this is left unpatched nothing wrong will happen, and if patched probably no one will be able to tell the difference without looking into the source Razz

08 Jun 2010, 03:34

bitshifter

Joined: 04 Dec 2007
Posts: 796
Location: Massachusetts, USA

bitshifter 08 Jun 2010, 03:39

Ok, i see, upon entry its always zero anyway.
I was looking to see if it was needed upon exit thus my post...
Maybe i need to get away from this screen and get some air Razz

08 Jun 2010, 03:39

ouadji

Joined: 24 Dec 2008
Posts: 1081
Location: Belgium

ouadji 08 Jun 2010, 11:39

Quote:

Is your "ouadji (it's 100% ok too)" code OK?

yes.

- remove "xor edx,edx" inside,
- and at the end ... "jmp .scan_syntax" directly
This approach saves code (useless),
and this makes the code more consistent with the general algorithm used in "proc fasm_syntax"

Code:

.syntax_string:
  mov     al,3
        stosb
       dec     ecx
 jz      .done
       lodsb
       cmp     al,ah
       jne     .syntax_string
      mov     al,3
        stosb
       dec     ecx
 jz      .done
       jmp     .scan_syntax
.syntax_comment:
        .....

_________________
I am not young enough to know everything (Oscar Wilde)-

08 Jun 2010, 11:39

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 12:44

The code you omit that way is this:

Code:

        lodsb
        cmp     al,ah
        je      .syntax_string

Jumping to .scan_syntax provides the missing lodsb, BUT, if .scan_syntax is reached again (previously was AL=AH required, now it has to pass the filters of the main part), it won't be without destroying AH first (which was an invariant inside .syntax_string previously) and loading a new character in AL (potentially reducing the number of 3s written).

I think the code is not functionally equivalent UNLESS you know of some precondition that you are not telling us. Razz

08 Jun 2010, 12:44

ouadji

Joined: 24 Dec 2008
Posts: 1081
Location: Belgium

ouadji 08 Jun 2010, 15:00

Quote:

it won't be without destroying AH first
(which was an invariant inside .syntax_string previously)

it does not matter, the scan of the current string is finished.

Code:

  .syntax_string:

    mov     al,3
        stosb
       dec     ecx
 jz      .done
       lodsb
       cmp     al,ah
       jne     .syntax_string
      mov     al,3
        stosb
       dec     ecx
 jz      .done
       jmp     .scan_syntax ;----->> (**)

--->> (**)
the engine is again ready to detect another string (the next one)
or any other characters.

Quote:

potentially reducing the number of 3s written

the advantage isn't there !
This makes the code more logical and consistent with the global algorithm.
the ending code of ".syntax_string" is illogical !
(and it makes me sick !) Razz

_________________
I am not young enough to know everything (Oscar Wilde)-

08 Jun 2010, 15:00

LocoDelAssembly
Your code has a bug

Joined: 06 May 2005
Posts: 4624
Location: Argentina

LocoDelAssembly 08 Jun 2010, 16:26

AAAH!! Now I see. Clearly a trick like this couldn't be done with a string parser, but since this is for highlighting, it doesn't matter if FASMW "thinks" that the string {'ouadji''s patch'} are two strings ({'ouadji', 's patch'}), since visually this will make no difference.

Sorry for wasting your time so many times Embarassed

08 Jun 2010, 16:26

ouadji

Joined: 24 Dec 2008
Posts: 1081
Location: Belgium

ouadji 08 Jun 2010, 16:53

Quote:

Sorry for wasting your time so many times

no problem,
I'm used to it with you !
Wink

I am teasing you!

_________________
I am not young enough to know everything (Oscar Wilde)-

08 Jun 2010, 16:53

< 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