shl shr optimization bug?

[fxm]

It's a mistake to have removed the syntax with (u)longint, because it works in 32-bit (and obviously in 64-bit).
There has been no change in the code on this.

- KeyPgOpShiftRight → fxm [restored syntax with the (u)longint type because this works on fbc 32-bit]
- KeyPgOpShiftLeft → fxm [restored syntax with the (u)longint type because this works on fbc 32-bit]

[dodicat]

version 1.05:

Syntax
Declare Operator Shl ( ByRef lhs As Integer, ByRef rhs As Integer ) As Integer
Declare Operator Shl ( ByRef lhs As UInteger, ByRef rhs As UInteger ) As UInteger
Declare Operator Shl ( ByRef lhs As LongInt, ByRef rhs As LongInt ) As LongInt
Declare Operator Shl ( ByRef lhs As ULongInt, ByRef rhs As ULongInt ) As ULongInt

current:

Syntax:
declare operator Shl ( byref lhs as integer, byref rhs as integer ) as integer
declare operator Shl ( byref lhs as uinteger, byref rhs as uinteger ) as uinteger

Looks like a real, operational change to me.

I understand the differences and how to use the keywords.
If I used the second form above to stay safe on 32 *or* 64 bit systems, then things have massively changed and my code may now be in trouble.
I DO program with the same source for 32 bit and 64 bit systems.

Do I now need to load multiple FB versions to test if/when there was a change?
Should any normal user be required to do that?
Do I now need to hunt through my 50k lines of code in my libraries to see if I am putting my customers at risk? I haven't needed to touch some of them for 5+ years. I did validation tests when I wrote the code. I am sure there are other FB users with far more code than me.
Does everyone using FB in production systems now need to play this game?


The concerns in my first post above still stand.


david

If you use a and b ulongint then
a shr b
a shl b
will return ulongint in 32 bits and 64 bits in the latest build, I tested an old 1.05 against the 1.08.1 build.
As far as I know integer is the basic non float datatype for both compilers, it is unfortunate that 32 bit and 64 bit have different integer ranges.
It has been a source of difficulties since 64 bit came to being.
I would normally use
#print typeof (a shl b)
to check anyway, especially if the code is in some way mission critical.

[speedfixer]

Thanks, fxm.

I was just concerned that these keywords had some source code change which would cause errors in some old program code.

[SARG]

A workaround could be

Code: Select all

#define my_shl32(a,b) ((a shl b) and &hFFFFFFFF)

dim as ulong c, k=2

c=&b11000000000000000000000000101000
c=my_shl32(c,k) shr k
print bin(c,32)

However I did some changes for SHL when both operands are LONG or ULONG (like for MOD/INTDIV) so 'normal' code is ok.
https://users.freebasic-portal.de/sarg/fbc64_SARG.zip

PS with previous version of compiler the split workaround didn't work when using gas64 (due to optimization of registers)

[srvaldez]

thank you SARG

[SARG]

thank you SARG

You are welcome.
I completed the version and fixed some bugs. https://users.freebasic-portal.de/sarg/fbc64_SARG.zip

Commit is pushed, now it's up to Jeff to decide to implement it.

[Munair]

A compiler might also ignore calculations that cancel each other out:

Code: Select all

c = (c shl 2) shr 2

whereas separate calculations yield separate (correct) results:

Code: Select all

c = 3221225512
c = c shl 2
print c       ' 160
c = c shr 2
print c       ' 40

I wouldn't rely on the first one. But if you know what the compiler does, masking out the upper 32 bits might indeed do the trick, as SARG pointed out.

[coderJeff]

From viewtopic.php?p=299721#p299721
I gather that this is primarily about the speed / performance. Because BYTE and SHORT will still be promoted and some kind of clamping is going to be needed anyway. That is, BYTE and SHORT sized integer will show the same problem in the upper bits using SHL followed by SHR as we saw in long types promoted to integer<64>.

So should expect to see in user code (if they really want SHL operation to a specific size):

Code: Select all

#define shl8(a,b) cubyte((a) shl (b))
#define shl16(a,b) cushort((a) shl (b))
#define shl32(a,b) culong((a) shl (b))

If there is no explicit conversion, then default promotion rules apply. If there are conversions, then the AST is analyzed for optimization.

Commit is pushed, now it's up to Jeff to decide to implement it.

Thanks, been looking at it. I think it is necessary to not only examine the source operands, but also the destination. I know I pointed you towards binary operator as a place to start. The logic probably should move to the optimizer. Ideally the optimization would be transparent - i.e. promotion as usual, and then optimize the promotions out where possible - that should give some consistency and not break older code.