The International Simutrans Forum

 

Author Topic: Fixes for left shift of negative values  (Read 488 times)

0 Members and 1 Guest are viewing this topic.

Offline ceeac

  • *
  • Posts: 45
Fixes for left shift of negative values
« on: July 20, 2019, 03:25:00 PM »
Continuation of https://forum.simutrans.com/index.php/topic,19115.0.html

These are some fixes for left shifts of negative signed integers in simgraph16.cc. From my tests, the assembly output is the same for gcc/clang/msvc, so program behaviour should not be affected.

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2710
  • Languages: EN
Re: Fixes for left shift of negative values
« Reply #1 on: July 20, 2019, 04:22:39 PM »
I was not aware such shifts were undefined behaviour lol. Guess I became too complacent with languages like Java where such shifts are well defined.

This might effect debug performance as the multiplication cannot be optimized away. However in release mode any sensible optimizer should optimize them away if the platform offered bitshift instructions allow it.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5525
  • Languages: EN, NO
Re: Fixes for left shift of negative values
« Reply #2 on: July 20, 2019, 07:13:41 PM »
Is this because C doesn't require negative integers to be represented with two's complement? I don't think it is a big problem that Simutrans won't work on architectures that don't use that. Do some compilers actually give warnings for this? GCC 8.3.0 doesn't even say anything with pedantic warnings turned on. Then again, it does't warn about the right-hand operand possibly being negative either (only when it is absolutely sure it is). That, I think, is plain impossible on x86.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4587
  • Languages: EN, DE, AT
Re: Fixes for left shift of negative values
« Reply #3 on: July 20, 2019, 08:43:11 PM »
The sanitizer-compiled builds warn about this at runtime.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5525
  • Languages: EN, NO
Re: Fixes for left shift of negative values
« Reply #4 on: July 20, 2019, 10:50:58 PM »
Is there no right-shifting of negative values in Simutrans? I would think those were worse, since the logic for doing that is different even with two's complement. x86 therefore has two different right-shift instructions, but only one for left. Or have right-shifts been changed likewise already?

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2710
  • Languages: EN
Re: Fixes for left shift of negative values
« Reply #5 on: July 21, 2019, 01:24:33 AM »
Is there no right-shifting of negative values in Simutrans?
The most likely place for such shifts would be with all the emulated fixed point maths for industry and the like. Simutrans really should be using its own fixed point library...

Fortunately most of the maths do not involve negative numbers, which is likely why this is not an issue.

When dealing with the graphics, especially pixel decoding or packing, unsigned shifts should be used as the integer is being treated as a field of bits and not a numeric value.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5525
  • Languages: EN, NO
Re: Fixes for left shift of negative values
« Reply #6 on: July 21, 2019, 08:21:09 AM »
The shifts in the patch looked very much like fixed point to me. More so with the original code than with the multiplication thrown in. While the two do functionally the same, how it is written can tell something about why it does what it does. It would have been clearer if Simutrans had a dedicated fixed-point type.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9514
  • Languages: De,EN,JP
Re: Fixes for left shift of negative values
« Reply #7 on: July 21, 2019, 02:42:02 PM »
While technically true, any machine with char != 8 bits and single complement will fail with Simutrans compilation. So this is rather a non-issue.

Moreover, the Bresenham algorithm is one of the oldest line drawing algorithm and is used in zillions of programs, even on ancient machines. If that could ever fail, then I have never heard of it.