Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSL same as ASH #14

Closed
tenko opened this issue Dec 29, 2021 · 11 comments
Closed

LSL same as ASH #14

tenko opened this issue Dec 29, 2021 · 11 comments

Comments

@tenko
Copy link

tenko commented Dec 29, 2021

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
Copy link
Owner

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
Copy link
Author

tenko commented Dec 30, 2021

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
Copy link
Owner

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
Copy link
Author

tenko commented Dec 31, 2021

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

@rochus-keller
Copy link
Owner

rochus-keller commented Jan 2, 2022

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
Copy link
Owner

rochus-keller commented Jan 2, 2022

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
Copy link
Owner

rochus-keller commented Jan 2, 2022

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
Copy link
Author

tenko commented Jan 3, 2022

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
Copy link
Owner

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
Copy link
Owner

Can we close this?

@tenko
Copy link
Author

tenko commented Jan 17, 2022

We can close this.

@tenko tenko closed this as completed Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants