News:

SimuTranslator
Make Simutrans speak your language.

r7596: warning: comparison is always true due to limited range of data type

Started by captain crunch, October 13, 2015, 12:51:02 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

captain crunch

===> HOSTCXX network/network_file_transfer.cc
network/network_file_transfer.cc: In function 'const char* network_connect(const char*, karte_t*)':
network/network_file_transfer.cc:208:20: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    for(uint8 i=0; i<300; i++) {
                    ^


Ters

I wonder if there even are any advantages to gain from using sized types for local variables.

jamespetts

I think that the only time to do so would be when one wants to compare a value with or assign a value to or from another of the same type which ultimately comes from/goes to the heap. Certainly, for loop iterators, one should be using the basic system types (int, uint32, sint32 or possibly the 64-bit versions if one is targeting 64-bit platforms).
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ters

Quote from: jamespetts on October 13, 2015, 04:23:26 PM
uint32, sint32 or possibly the 64-bit versions if one is targeting 64-bit platforms

That is exactly what I wonder if is over-engineering it. int8 is just as much a basic system type as int32, with uint16 more debatable, though that's platform dependent. Of course, on one hand, one needs to ensure that the data type is big enough. Using an int usually won't do once you reach numbers in the billions, though it might (some platforms have 64-bit ints on x64, others stick to 32-bits for old time's sake). But will the compiler figure out that it doesn't need the full size of the given data type if it can foresee all possible values?


DrSuperGood

QuoteI wonder if there even are any advantages to gain from using sized types for local variables.
The only possible time it would make a different is during a function call or if so many local variables are used it has to be pushed onto the stack since otherwise it will be retained in a register. In theory the 1 byte could take fewer cycles to push out and pull in than 4 or 8 bytes.

In reality it likely makes no difference at all since you may find the CPU takes the same amount of time to push/pull 1 bytes as it does for 4 (or 8 in x64) bytes. This is due to the lowest level cache working at speeds comparable to the CPU instruction rate and being integrated as part of the CPU. Even if one or more level of cache gets depleted, the saving of 3-7 bytes is so insignificant in the big picture of things the smallest optimization somewhere else would be much bigger.

Generally unless a loop integer is being used specifically as an argument to a function that expects a certain type, possibly for range restriction purposes, one should use uint32. uint64 should not be used because it could possibly cause a massive slowdown in a 32bit build where manipulation of 64bit integers require multiple instructions. Dynamically choosing based on build is probably a bad idea since it is another potential source of inconsistent behaviour between builds and gives you no advantage.

Ters

Quote from: DrSuperGood on October 14, 2015, 05:56:21 AM
The only possible time it would make a different is during a function call or if so many local variables are used it has to be pushed onto the stack since otherwise it will be retained in a register. In theory the 1 byte could take fewer cycles to push out and pull in than 4 or 8 bytes.

What I really was wondering was whether a compiler would realize it only needed to push one byte even for a local variable declared as int, if it could see that the value never exceeded 255.

Quote from: DrSuperGood on October 14, 2015, 05:56:21 AM
Generally unless a loop integer is being used specifically as an argument to a function that expects a certain type, possibly for range restriction purposes, one should use uint32. uint64 should not be used because it could possibly cause a massive slowdown in a 32bit build where manipulation of 64bit integers require multiple instructions. Dynamically choosing based on build is probably a bad idea since it is another potential source of inconsistent behaviour between builds and gives you no advantage.

32-bit operations might be slower than 64-bit on some 64-bit platforms. Will the compiler just treat the uint32 as a uint64, except when it makes a difference (mostly overflow and some types of shifting)? But with C, I guess it all boils down to that you need to know the architecture you are developing on.

DrSuperGood

Quote32-bit operations might be slower than 64-bit on some 64-bit platforms.
Not too sure how this would make sense. I would imagine a 64-bit chip would use the same functional units for 32-bit integer operations as it does for 64-bit integer operations except with some additional logic to discard and deal with the results of the upper half of the word. There would not even be a difference in execution time when using equivalent values because the functional unit execution time possibly is dependant on number complexity rather than number field length.

QuoteWill the compiler just treat the uint32 as a uint64, except when it makes a difference (mostly overflow and some types of shifting)? But with C, I guess it all boils down to that you need to know the architecture you are developing on.
Logically it cannot treat a unit32 as a unit64 because of various status flags after operations. Additionally there are many hacky tricks the compiler can use to improve performance under certain circumstances such as packing 4 different uint32 or two different uint64 values into a single 128 bit SSE register and then performing the same operations on all of them in a single instruction.

Even thinking about this is kind of a waste of time. Any possible potential optimization through using the "best" type that is not done automatically will likely decrease code quality and give insignificant performance gain (no one would notice). There are thousands of other places in Simutrans where optimizations would have a much bigger effect.

Ters

Quote from: DrSuperGood on October 14, 2015, 03:35:26 PM
Not too sure how this would make sense. I would imagine a 64-bit chip would use the same functional units for 32-bit integer operations as it does for 64-bit integer operations except with some additional logic to discard and deal with the results of the upper half of the word. There would not even be a difference in execution time when using equivalent values because the functional unit execution time possibly is dependant on number complexity rather than number field length.

It is possible that instructions on 32-bit data types are not as compact. This is the case for 16- vs 32-bit operand sizes on 80386 and compatibles. I think at least some processors in that family are slower at decoding instructions with the operand size prefix.

Quote from: DrSuperGood on October 14, 2015, 03:35:26 PM
Even thinking about this is kind of a waste of time. Any possible potential optimization through using the "best" type that is not done automatically will likely decrease code quality and give insignificant performance gain (no one would notice). There are thousands of other places in Simutrans where optimizations would have a much bigger effect.

Well, my point was the possibility of not thinking which type is best, and let the compiler decide if the best choice is int8, int16, int32 or int64, as long as it can foresee all possible values. But I don't think C lets you get that "abstract".

DrSuperGood

QuoteWell, my point was the possibility of not thinking which type is best, and let the compiler decide if the best choice is int8, int16, int32 or int64, as long as it can foresee all possible values. But I don't think C lets you get that "abstract".
Do not under estimate the capabilities of compilers when it comes to loops.


uint32_t counter = 0;
for (uint32_t i = 0 ; i < 300000 ; i+= 1) {
    counter+= 3;
}
DoSomething(counter);

Many modern compilers (eg ARM c++ compilers) will optimize that loop to a single constant equivalent to...

DoSomething(900000);

Eliminating the existence of the loop entirely and making its type have no importance at all. They might even start to inline DoSomething if it is statically linking it.

Ters

That is still a completely different issue than what I was thinking about. I was thinking of whether

int max = 0;
for (int i = 0; i < 100; ++i) {
    if (array[i] > max) max = array[i];
}


would use the cl register (or any of the other 8-bit registers) for i on x86 if that would be advantageous for some reason (x86 has few registers, and there might be many other local variables in the surrounding code that you don't want to spill to the stack). Because if using int means that the compiler will use ecx (or any other 32-bit register) no matter what, explicitly declaring the loop counter as an 8-bit variable (uint8) can actually be a good idea in some situations. (On x86, this will only be usable for 8-bit values. Using 16-bit values don't gain extra registers, and need the operand size prefix.)

DrSuperGood

Quotewould use the cl register (or any of the other 8-bit registers) for i on x86 if that would be advantageous for some reason (x86 has few registers, and there might be many other local variables in the surrounding code that you don't want to spill to the stack). Because if using int means that the compiler will use ecx (or any other 32-bit register) no matter what, explicitly declaring the loop counter as an 8-bit variable (uint8) can actually be a good idea in some situations. (On x86, this will only be usable for 8-bit values. Using 16-bit values don't gain extra registers, and need the operand size prefix.)
The cl register is just the lower 8 bits of the counter register (ECX). All the 8 bit registers are just fragments of the bigger 32 bit extended registers.

Ters

Quote from: DrSuperGood on October 15, 2015, 03:31:53 PM
The cl register is just the lower 8 bits of the counter register (ECX). All the 8 bit registers are just fragments of the bigger 32 bit extended registers.

Yes, but it frees the ch register for another 8-bit value.