BST mis-handles constant numeric comparisons with negative CON symbols

Ding-Batty · 2024-03-01 19:46

I cam across this during a recent development effort, and I don't recall seeing anything about this in the Forums, nor could I find anything with searches (forum and Google).

It seems that constant expression evaluations done by the BST Spin compiler that involve numeric comparisons with negative-valued constant symbols do not work correctly. My best guess is that the value of the CONstant is handled as an unsigned number internally in the compiler, and it does not force the value to be signed for the comparison operators (>, <, =>, =<).

Here are two test programs to show the problem. The output of each is shown as comments in the code.

The first one uses FIT directives to show the problem strictly at compile-time, no Propeller needed:

{{
  Test an oddity in the BST compiler, with constant expressions (i.e.
  evaluated at compile time) interpreting negative CONstants as non-
  negative, using compile-time "FIT" assertions.
}}

CON
  MINUS_2      = -2
  MINUS_2_LT_0 = MINUS_2 < 0

' ========================================================================
' Now, use the "FIT" directive to tell if an expression is true or false,
' and to throw an error if false:
' - Create an expression that is true for the desired condition
' - Wrap it in the absolute-value operator, to get a 0..1 value
' - Put that in a FIT directive after an "ORG 1":
'   If the condition is false, "ORG 1" "FIT 0" should fail, but "FIT 1" will
'   succeed.

' BST 0.19.3 compilation throws the following errors:
'   negative_fit_assertions - Error at (31,41) PC exceeds cog memory by 1 longs
'   negative_fit_assertions - Error at (32,41) PC exceeds cog memory by 1 longs
'
' Propeller Tool 1.3.2 has no failure
'
' FlexProp 6.8.0 P1 bytecode output, and non-bytecode both have no failure

DAT
                ORG 1

                FIT     ||(MINUS_2 < 0 )
                FIT     ||(MINUS_2_LT_0)

' No need to run anything, the test is a compile time.
PUB Main
  ' Do nothing

The second one reports the value of the constant expressions at runtime by
the Propeller.

{{
  Test an oddity in the BST compiler, with constant expressions (i.e.
  evaluated at compile time) interpreting negative CONstants as non-
  negative.
}}
OBJ
  Ser    : "fullDuplexSerial"  ' uses a cog

CON
  _CLKMODE = XTAL1 + PLL16X 
  _XINFREQ = 5_000_000

  BAUDRATE = 115_200
  RXPIN    = 31    ' The usual pins.
  TXPIN    = 30
  CR = $0D

  MINUS_2      = -2
  MINUS_2_LT_0 = MINUS_2 < 0

' ========================================================================
PUB Main
  waitcnt(clkfreq * 4 + cnt)   ' Wait 4 seconds at startup.
  Ser.Start(RXPIN, TXPIN, %0000, BAUDRATE)

  ' It seems that BST (ver. 0.19.3 on Windows) has a problem with the
  ' numeric comparison operators at compile time, when used with constant
  ' values. It seems to interpret the constant as non-negative.
  ' This can also be triggered in BST when not expected, if the compiler
  ' optimization "Fold Constants" which adds the "constant(...)" wrapper
  ' around expressions made entirely of constants.

  ' These three tests should all report a value of "true" (-1).
  '
  ' BST 0.19.3 output:
  '   Test:          MINUS_2 < 0 : -1
  '   Test: constant(MINUS_2 < 0): 0
  '   Test:          MINUS_2_LT_0: 0
  '
  ' Propeller Tool 1.3.2 output:
  '   Test:          MINUS_2 < 0 : -1
  '   Test: constant(MINUS_2 < 0): -1
  '   Test:          MINUS_2_LT_0: -1
  '
  ' FlexProp 6.8.0 P1 bytecode output, and non-bytecode:
  '   Test:          MINUS_2 < 0 : -1
  '   Test: constant(MINUS_2 < 0): -1
  '   Test:          MINUS_2_LT_0: -1


  Report(string("         MINUS_2 < 0 "),          MINUS_2 < 0 )
  Report(string("constant(MINUS_2 < 0)"), constant(MINUS_2 < 0))
  Report(string("         MINUS_2_LT_0"),          MINUS_2_LT_0)

' ------------------------------------------------------------------------

PUB Report(s, v)
  Ser.str(string("Test: "))
  Ser.str(s)
  Ser.str(string(": "))
  Ser.dec(v)
  Ser.tx(CR)

There are work-arounds for this, but you have to know you need to be careful.

I haven't tested the latest Propeller Tool version, because I want to see if I can install and use it and hte old version at the same time (potential install directory and registry collisions).

Wuerfel_21 · 2024-03-01 19:59

There shouldn't be any difference between Propeller Tool 1.3.2 and 2.x.

Is there anything you need that flexspin bytecode mode doesn't do? The code generation in there should be on par with or better than BST in every case. If not, that's a bug

Ding-Batty · 2024-03-01 20:03

FlexSpin bug: FIT expressions do not like the Boolean "OR" operator. (I ended up using "+").

Generally I want to make sure that my code is easily used by the greatest number of people (I'm preparing the code for release here on the Forums and in the ObEx), so I don't want to limit it to a specific compiler.

Eample: I like the BST listing better than the FlexSpin listing for bytecode, because of how it handles DAT blocks.

Electrodude · 2024-03-01 20:20

@Ding-Batty said:
FlexSpin bug: FIT expressions do not like the Boolean "OR" operator. (I ended up using "+").

That seems like correct behavior. Since or is logical OR, it only ever returns 0 or -1, neither of which make much sense as the argument for fit. You probably want bitwise OR | (or, as you found, just use +).

Wuerfel_21 · 2024-03-01 20:21

@Ding-Batty said:
FlexSpin bug: FIT expressions do not like the Boolean "OR" operator. (I ended up using "+").

That sounds delightfully obscure. Can you give us an example for that? (and ideally file it straight to github )

Generally I want to make sure that my code is easily used by the greatest number of people (I'm preparing the code for release here on the Forums and in the ObEx), so I don't want to limit it to a specific compiler.

BST is pretty much dead though, so if the bugs make your program sad, it's not so bad to just put a note that it doesn't work. Good on you for checking it so thoroughly.

Eample: I like the BST listing better than the FlexSpin listing for bytecode, because of how it handles DAT blocks.

Oh right, I think I half-assed it and just treated the DAT section as a big hex blob (the DAT assembler is shared between backends and therefore didn't really need debugging). Would have to figure out where the ASM backend pulls the DAT listing comments from.

Ding-Batty · 2024-03-01 20:25

It may be by design, but it does not match the behavior of other Spin compilers.

Example: I use FIT statements as compile-time asserts (the only way I know of to do so for all Spin compilers). Here is a sample:

DAT
                org     1

  ' The COLORS_BASE_PIN must be either 0 or 9, because the code uses a
  ' MOVS or MOVD for the six color bits, for speed.
  ' Error means the COLORS_BASE_PIN was given an unspported value.
  ' For some reason FlexSpin doesn't like the "OR" operator:
  '   "error: syntax error, unexpected instruction"
  'fit ||((COLORS_BASE_PIN == 0) OR (COLORS_BASE_PIN == 9))
  fit ||((COLORS_BASE_PIN == 0) + (COLORS_BASE_PIN == 9))

PropTool and BST both have no problem with the "OR" operator.

Wuerfel_21 · 2024-03-01 20:46

Oh, that's just a lexer bug (OR assembly instruction taking precedence over the OR operator). I think I ran into that before, but in Spin2 mode (so I just changed it to ||).

I'll submit that issue then.

EDIT: Wait, it doesn't happen with other directives (such as LONG), so there should be an easy way to just fix it right now

Ding-Batty · 2024-03-01 20:57

@Wuerfel_21 said:

@Ding-Batty said:
FlexSpin bug: FIT expressions do not like the Boolean "OR" operator. (I ended up using "+").

That sounds delightfully obscure. Can you give us an example for that? (and ideally file it straight to github )

Issue submitted: https://github.com/totalspectrum/spin2cpp/issues/430

Wuerfel_21 · 2024-03-01 21:00

Fixed: https://github.com/totalspectrum/spin2cpp/pull/431

Ding-Batty · 2024-03-01 21:00

@Wuerfel_21 -- that was quick! https://github.com/totalspectrum/spin2cpp/pull/431

Wuerfel_21 · 2024-03-01 21:31

The listing thing seems more involved. I actually can't tell how the ASM backend manages to do it...

Electrodude · 2024-03-15 14:24

@Ding-Batty said:
Example: I use FIT statements as compile-time asserts (the only way I know of to do so for all Spin compilers). Here is a sample:

DAT
                org     1

  ' The COLORS_BASE_PIN must be either 0 or 9, because the code uses a
  ' MOVS or MOVD for the six color bits, for speed.
  ' Error means the COLORS_BASE_PIN was given an unspported value.
  ' For some reason FlexSpin doesn't like the "OR" operator:
  '   "error: syntax error, unexpected instruction"
  'fit ||((COLORS_BASE_PIN == 0) OR (COLORS_BASE_PIN == 9))
  fit ||((COLORS_BASE_PIN == 0) + (COLORS_BASE_PIN == 9))

Oh, that's a clever trick. Seems like the #define STATIC_ASSERT(cond, msg) ((void)sizeof(struct{int:-!(cond);})) trick you used to have to do in C before it got its builtin _Static_assert in C11.

Ding-Batty · 2024-03-15 17:05

@Electrodude said:
Oh, that's a clever trick. Seems like the #define STATIC_ASSERT(cond, msg) ((void)sizeof(struct{int:-!(cond);})) trick you used to have to do in C before it got its builtin _Static_assert in C11.

It is clever, but I didn't invent it. I seem to recall seeing something like it in someone else's code years ago, but I haven't been able to find it either in my archives, or in the forums. I've used it a fair amount.

I hadn't seen that C preprocessor trick before, so I just went Internet hunting. static_assert() was added to C compilers as of about 2011, so hunting for the macro version was harder than I expected. I eventually found this document, that discusses how it works, and how to do better error handling: https://www10.cs.fau.de/publications/reports/TechRep_2009-10.pdf

BST mis-handles constant numeric comparisons with negative CON symbols

Comments