4klang icon indicating copy to clipboard operation
4klang copied to clipboard

Unnecessary code in Power-function

Open vsariola opened this issue 4 years ago • 1 comments

In 4klang.asm, the lines 251-253 are:

fld1
fadd	st0	
fyl2x

I believe they do absolutely nothing to the stack and can be removed. To see this, I added some comments:

         ; Stack: x
fld1     ; Stack: 1 x
fadd st0 ; Stack: 2 x
fyl2x    ; Stack: x*log2(2)      ... which is *drumfill* just x

I think they are a left-over from a time when the power function could compute any power, not just 2^x. Also, I'm just learning x86 assembly so there could be something I'm missing here; however, I just ran some regression tests after deleting these three lines and all my regression tests seem to pass.

Also, the comments seem to be completely off, I think better comments could be:

              ; x
fld1          ; 1 x
fadd st0      ; 2 x
fyl2x         ; x // until this line could be completely removed?
fld1          ; 1 x
fld st1       ; x 1 x
fprem         ; mod(x,1) 1 x
f2xm1         ; 2^mod(x,1)-1 1 x
faddp st1,st0 ; 2^mod(x,1) x
fscale        ; 2^mod(x,1)*2^trunc(x) x
              ; Equal to:
              ; 2^x x
fstp st1      ; 2^x
ret

vsariola avatar Apr 23 '20 09:04 vsariola

You are right, that's a leftover from the very first version of 4klang where a generic power function was called with the base provided by the caller. Same applies to the comments.

It should be safe to modify it, as you already said. I'll update the repo, once i checked all the recent incoming comments/bugs.

Thanks for the hint!

P.S: The original function looked like this:

; //----------------------------------------------------------------------------------------
; //	Power function
; //----------------------------------------------------------------------------------------
; //	Input :		st0		:	base
; //			st1		:	exponent
; //	Output:		st0		:	result
; //----------------------------------------------------------------------------------------
go4kPower:				; // base			exp
	fyl2x				; // log2_base		-
	fld	st0			; // log2_base		log2_base
	frndint				; // (int)log2_base	log2_base
	fxch				; // log2_base		(int)log2_base
	fsub	st0, st1		; // (frac)log2_base	(int)log2_base
	f2xm1				; // 2 ^ '' -	1		(int)log2_base
	fld1				; // 1				2 ^ '' -	1		(int)log2_base
	faddp	st1, st0		; // 2 ^ ''			(int)log2_base
	fscale			
	fstp	st1
	ret	

gopher-atz avatar Apr 23 '20 10:04 gopher-atz