rochus-keller / Oberon

Oberon parser, code model & browser, compiler and IDE with debugger, and an implementation of the Oberon+ programming language
GNU General Public License v2.0
450 stars 29 forks source link

LSL same as ASH #14

Closed tenko closed 2 years ago

tenko commented 2 years ago

With the following test program:

module Test
import Out
var
    a : integer
begin 
    a := 1;
    Out.Int(a,5) Out.Ln()
    a := lsl(a,1)
    Out.Int(a,5) Out.Ln()
    a := lsl(a,-1)
    Out.Int(a,5) Out.Ln()
    a := 1;
    a := lsl(a,-1)
    Out.Int(a,5) Out.Ln()
    a := -1;
    Out.Int(a,5) Out.Ln()
    a := lsl(a,1)
    Out.Int(a,5) Out.Ln()
    a := lsl(a,-1)
    Out.Int(a,5) Out.Ln()
end Test

We get the output (Mono):

    1
    2
    0
-2147483648
   -1
   -2
    0

I believe LSL should not preserve the sign bit (leftmost bit) when shifting. Also negative argument are allowed, but set result to zero. In c this is also undefined behavior.

Perhaps ASH with negative argument can replace ASR? Same for LSL, with negative argument gives LSR and rename function to LSH? Otherwise probably negative arguments should be handled to avoid undefined behavior.

rochus-keller commented 2 years ago

LSL is not the same as ASH. ASH was a built-in function in Oberon 90 and Oberon-2. It was replaced in Oberon-07 by LSL and ASR. LSL is a logical left shift and ASR an arithmetic right shift. The system module function ROT of Oberon 90 and Oberon-07 was renamed to ROR for right rotation. Even if the type of n is INTEGER in the Oberon-07 specification, I don't think that negative n make sense (i.e. would contradict the fact that the generic ASH was replaced by explicit left and right shift functions). But that's just an assumption, since the language report is not particularly specific. ROR is currently implemented as a simple unsigned shift operation, not a true rotation.

tenko commented 2 years ago

I am not sure what is correct here.

In mathematics, raise to negative whole number become division and raise to zero is defined as one (which in this case does not make sense)

It seems some Oberon-2 code rely on negative argument : https://github.com/Oleg-N-Cher/OfrontPlus/blob/master/Mod/Lib/MathL.cp

If only positive argument is allowed then the value should probably be clamped to positive to avoid the large negative value in the example.

rochus-keller commented 2 years ago

Don't mix up Oberon-2 and before which only had the ASH feature and also had to accept negative n. Oberon-07 is a different case. By the end of the day we just have to define the features which I will specify and implement in Oberon+. I even consider re-adding ASH for backward compatibility. The Component Pascal platform specific issue report (in contrast to all Oberon language reports) is at least explicit for ROT with "(n > 0: left, n < 0: right)".

tenko commented 2 years ago

Thanks for the clarification. I will wait until this is implemented.

rochus-keller commented 2 years ago

I pushed a commit to Github for IDE 0.9.40 which implements a couple of preliminary specification changes;

for one part all BITxyz operations now accept both INTEGER and LONGINT and the result is LONGINT if one of the operands is LONGINT;

the same applies to the ASR built-in function, but only for positive n; I'm not yet sure what actually happens for negative n, but it obviously works with my Oberon System version and it doesn't work anymore if I just replace >> n by << -n for negative n;

then there are two new BITSHL and BITSHR operations for logical left and right shift for positive n and again INTEGER and LONGINT;

finally I added the optional I and L suffix to integer literals to force either INTEGER (I) or LONGINT (L) type and thus to avoid SHORT() or LONG() operations;

the work in progress version of the spec (not yet commited) is attached: The_Programming_Language_Oberon+.html.gz

rochus-keller commented 2 years ago

Meanwhile I did a lot of practical experiments with the shift functions and finally could derive the true formula for ASR(x,n),

it is ASR(x,n) = x DIV 2^( n MOD w )^ with w the bit width of x.

As you can see it is different from the formula given in the Oberon spec (x DIV 2^n^), and ASR for negative n is not just a left shift (i.e. not the inverse operation of ASR, i.e. not LSL).

The formula for LSL is most likely LSL(x,n) = x * 2^( n MOD w )^ with w the bit width of x (I will do some experiments to confirm this).

Since the Oberon System (and likely other Wirth applications) depends on this feature I can't change it, otherwise these applications won't work anymore.

rochus-keller commented 2 years ago

I added the BITASR(x,n) built-in function and deprecated ASR, ASH, LSL and ROR; please use the BITx functions and only for n >= 0 and < bitwidth; the functions work with both INTEGER and LONGINT.

I also refactored some other built-ins for LONG compatibility.

the work in progress version of the spec (not yet commited) is attached:

The_Programming_Language_Oberon+.html.gz

tenko commented 2 years ago

Happy new year. Looks like a good choice with the BITXXX name convention. Also removal of SHORT() or LONG() makes it much cleaner. I will test this more when I get time to do so.

rochus-keller commented 2 years ago

Thanks, same to you.

SHORT() and LONG() are still available, but since the literals can be typed it is less necessary to use them.

Meanwhile I added all necessary LONGINT and LONGREAL operations to the Oakwood library; commit will go up soon.

rochus-keller commented 2 years ago

Can we close this?

tenko commented 2 years ago

We can close this.