Jump to content
Sign in to follow this  
Picxie

Bug With If Statement Comparing To Two Added Constants

Recommended Posts

Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant.

EG

#define APPLES	0x10
#define PEARS	0x3
#define APPLES_PEARS	APPLES+PEARS
#define ALL_FRUIT	0x13

#define NO_FRUIT	0x80


char result;
void main() 
{
// TWO #defined constants
if(result >= (APPLES + PEARS))
{
	result = NO_FRUIT;
} 

// A #defined of two #defined constants
if(result >= APPLES_PEARS)
{
	result = NO_FRUIT;
}

// a single #defined constant
if(result >= ALL_FRUIT)
{
	result = NO_FRUIT;
}

// NB also fails with <=
if(result <= (APPLES + PEARS))
{
	result = NO_FRUIT;
}
if(result <= ALL_FRUIT)
{
	result = NO_FRUIT;
}
// ==
if(result == (APPLES + PEARS))
{
	result = NO_FRUIT;
}
if(result == ALL_FRUIT)
{
	result = NO_FRUIT;
}

// !=
if(result != (APPLES + PEARS))
{
	result = NO_FRUIT;
}
if(result != ALL_FRUIT)
{
	result = NO_FRUIT;
}

// <
if(result < (APPLES + PEARS))
{
	result = NO_FRUIT;
}
if(result < ALL_FRUIT)
{
	result = NO_FRUIT;
}

// >
if(result > (APPLES + PEARS))
{
	result = NO_FRUIT;
}

if(result > ALL_FRUIT)
{
	result = NO_FRUIT;
}

}

 

produces the following

void main() 

{
// TWO #defined constants
if(result >= (APPLES + PEARS))
0004  0E13		  MOVLW 0x13
0006  84D8		  BSF STATUS,Z
0008  6001		  CPFSLT gbl_result
000A  B4D8		  BTFSC STATUS,Z
000C  D001		  BRA	label1
000E  D002		  BRA	label2
0010			label1
0014			label2

{
	result = NO_FRUIT;
0010  0E80		  MOVLW 0x80
0012  6E01		  MOVWF gbl_result

} 

// A #defined of two #defined constants
if(result >= APPLES_PEARS)
0014  0E13		  MOVLW 0x13
0016  84D8		  BSF STATUS,Z
0018  6001		  CPFSLT gbl_result
001A  B4D8		  BTFSC STATUS,Z
001C  D001		  BRA	label3
001E  D002		  BRA	label4
0020			label3
0024			label4

{
	result = NO_FRUIT;
0020  0E80		  MOVLW 0x80
0022  6E01		  MOVWF gbl_result

}

// a single #defined constant
if(result >= ALL_FRUIT)
0024  0E13		  MOVLW 0x13
0026  6001		  CPFSLT gbl_result
0028  D001		  BRA	label5
002A  D002		  BRA	label6
002C			label5
0030			label6

{
	result = NO_FRUIT;
002C  0E80		  MOVLW 0x80
002E  6E01		  MOVWF gbl_result

}

// NB also fails with <=
if(result <= (APPLES + PEARS))
0030  0E13		  MOVLW 0x13
0032  6401		  CPFSGT gbl_result
0034  D002		  BRA	label7
0036  84D8		  BSF STATUS,Z
0038  E002		  BZ	label8
003A			label7
003E			label8

{
	result = NO_FRUIT;
003A  0E80		  MOVLW 0x80
003C  6E01		  MOVWF gbl_result

}
if(result <= ALL_FRUIT)
003E  5001		  MOVF gbl_result, W
0040  0813		  SUBLW 0x13
0042  E302		  BNC	label9
0048			label9

{
	result = NO_FRUIT;
0044  0E80		  MOVLW 0x80
0046  6E01		  MOVWF gbl_result

}
// ==
if(result == (APPLES + PEARS))
0048  0E13		  MOVLW 0x13
004A  6201		  CPFSEQ gbl_result
004C  D005		  BRA	label10
004E  0A00		  XORLW 0x00
0050  6201		  CPFSEQ gbl_result
0052  D002		  BRA	label10
0058			label10

{
	result = NO_FRUIT;
0054  0E80		  MOVLW 0x80
0056  6E01		  MOVWF gbl_result

}
if(result == ALL_FRUIT)
0058  0E13		  MOVLW 0x13
005A  6201		  CPFSEQ gbl_result
005C  D002		  BRA	label11
0062			label11

{
	result = NO_FRUIT;
005E  0E80		  MOVLW 0x80
0060  6E01		  MOVWF gbl_result

}

// !=
if(result != (APPLES + PEARS))
0062  0E13		  MOVLW 0x13
0064  6201		  CPFSEQ gbl_result
0066  0A00		  XORLW 0x00
0068  1801		  XORWF gbl_result, W
006A  0A00		  XORLW 0x00
006C  E002		  BZ	label12
0072			label12

{
	result = NO_FRUIT;
006E  0E80		  MOVLW 0x80
0070  6E01		  MOVWF gbl_result

}
if(result != ALL_FRUIT)
0072  0E13		  MOVLW 0x13
0074  6201		  CPFSEQ gbl_result
0076  D001		  BRA	label13
0078  D002		  BRA	label14
007A			label13
007E			label14

{
	result = NO_FRUIT;
007A  0E80		  MOVLW 0x80
007C  6E01		  MOVWF gbl_result

}

// <
if(result < (APPLES + PEARS))
007E  0E00		  MOVLW 0x00
0080  0A13		  XORLW 0x13
0082  6001		  CPFSLT gbl_result
0084  0A13		  XORLW 0x13
0086  E002		  BZ	label15
008C			label15

{
	result = NO_FRUIT;
0088  0E80		  MOVLW 0x80
008A  6E01		  MOVWF gbl_result

}
if(result < ALL_FRUIT)
008C  0E13		  MOVLW 0x13
008E  6001		  CPFSLT gbl_result
0090  D002		  BRA	label16
0096			label16

{
	result = NO_FRUIT;
0092  0E80		  MOVLW 0x80
0094  6E01		  MOVWF gbl_result

}

// >
if(result > (APPLES + PEARS))
0096  5001		  MOVF gbl_result, W
0098  0E00		  MOVLW 0x00
009A  0A00		  XORLW 0x00
009C  0E13		  MOVLW 0x13
009E  E104		  BNZ	label17
00A0  6401		  CPFSGT gbl_result
00A2  D002		  BRA	label17
00A8			label17

{
	result = NO_FRUIT;
00A4  0E80		  MOVLW 0x80
00A6  6E01		  MOVWF gbl_result

}

if(result > ALL_FRUIT)
00A8  0E13		  MOVLW 0x13
00AA  6401		  CPFSGT gbl_result

{
	result = NO_FRUIT;
00AE  0E80		  MOVLW 0x80
00B0  6E01		  MOVWF gbl_result

}

}
00AC  0012		  RETURN
00B2  0012		  RETURN

Share this post


Link to post
Share on other sites
Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant.

EG

....

[code/]

 

Is that the actual program?

You don't initialize result. Furthermore it is signed and you assign the value -128 to result at some point.

Share this post


Link to post
Share on other sites
Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant.

EG

....

[code/]

 

Is that the actual program?

You don't initialize result. Furthermore it is signed and you assign the value -128 to result at some point.

 

No, its a selection of samples of different if statements to illustrate the output of the compilor.

Share this post


Link to post
Share on other sites
Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant.

EG

 

#define APPLES 0x10

#define PEARS 0x3

#define APPLES_PEARS APPLES+PEARS

#define ALL_FRUIT 0x13

 

#define NO_FRUIT 0x80

 

 

char result;

void main()

{

// TWO #defined constants

if(result >= (APPLES + PEARS))

{

result = NO_FRUIT;

}

 

// A #defined of two #defined constants

if(result >= APPLES_PEARS)

{

result = NO_FRUIT;

}

 

// a single #defined constant

if(result >= ALL_FRUIT)

{

result = NO_FRUIT;

}

 

// NB also fails with <=

if(result <= (APPLES + PEARS))

{

result = NO_FRUIT;

}

if(result <= ALL_FRUIT)

{

result = NO_FRUIT;

}

// ==

if(result == (APPLES + PEARS))

{

result = NO_FRUIT;

}

if(result == ALL_FRUIT)

{

result = NO_FRUIT;

}

 

// !=

if(result != (APPLES + PEARS))

{

result = NO_FRUIT;

}

if(result != ALL_FRUIT)

{

result = NO_FRUIT;

}

 

// <

if(result < (APPLES + PEARS))

{

result = NO_FRUIT;

}

if(result < ALL_FRUIT)

{

result = NO_FRUIT;

}

 

// >

if(result > (APPLES + PEARS))

{

result = NO_FRUIT;

}

 

if(result > ALL_FRUIT)

{

result = NO_FRUIT;

}

 

}

[code/]

 

produces the following

[code]

void main()

 

{

// TWO #defined constants

if(result >= (APPLES + PEARS))

0004 0E13 MOVLW 0x13

0006 84D8 BSF STATUS,Z

0008 6001 CPFSLT gbl_result

000A B4D8 BTFSC STATUS,Z

000C D001 BRA label1

000E D002 BRA label2

0010 label1

0014 label2

 

{

result = NO_FRUIT;

0010 0E80 MOVLW 0x80

0012 6E01 MOVWF gbl_result

 

}

 

// A #defined of two #defined constants

if(result >= APPLES_PEARS)

0014 0E13 MOVLW 0x13

0016 84D8 BSF STATUS,Z

0018 6001 CPFSLT gbl_result

001A B4D8 BTFSC STATUS,Z

001C D001 BRA label3

001E D002 BRA label4

0020 label3

0024 label4

 

{

result = NO_FRUIT;

0020 0E80 MOVLW 0x80

0022 6E01 MOVWF gbl_result

 

}

 

// a single #defined constant

if(result >= ALL_FRUIT)

0024 0E13 MOVLW 0x13

0026 6001 CPFSLT gbl_result

0028 D001 BRA label5

002A D002 BRA label6

002C label5

0030 label6

 

{

result = NO_FRUIT;

002C 0E80 MOVLW 0x80

002E 6E01 MOVWF gbl_result

 

}

 

// NB also fails with <=

if(result <= (APPLES + PEARS))

0030 0E13 MOVLW 0x13

0032 6401 CPFSGT gbl_result

0034 D002 BRA label7

0036 84D8 BSF STATUS,Z

0038 E002 BZ label8

003A label7

003E label8

 

{

result = NO_FRUIT;

003A 0E80 MOVLW 0x80

003C 6E01 MOVWF gbl_result

 

}

if(result <= ALL_FRUIT)

003E 5001 MOVF gbl_result, W

0040 0813 SUBLW 0x13

0042 E302 BNC label9

0048 label9

 

{

result = NO_FRUIT;

0044 0E80 MOVLW 0x80

0046 6E01 MOVWF gbl_result

 

}

// ==

if(result == (APPLES + PEARS))

0048 0E13 MOVLW 0x13

004A 6201 CPFSEQ gbl_result

004C D005 BRA label10

004E 0A00 XORLW 0x00

0050 6201 CPFSEQ gbl_result

0052 D002 BRA label10

0058 label10

 

{

result = NO_FRUIT;

0054 0E80 MOVLW 0x80

0056 6E01 MOVWF gbl_result

 

}

if(result == ALL_FRUIT)

0058 0E13 MOVLW 0x13

005A 6201 CPFSEQ gbl_result

005C D002 BRA label11

0062 label11

 

{

result = NO_FRUIT;

005E 0E80 MOVLW 0x80

0060 6E01 MOVWF gbl_result

 

}

 

// !=

if(result != (APPLES + PEARS))

0062 0E13 MOVLW 0x13

0064 6201 CPFSEQ gbl_result

0066 0A00 XORLW 0x00

0068 1801 XORWF gbl_result, W

006A 0A00 XORLW 0x00

006C E002 BZ label12

0072 label12

 

{

result = NO_FRUIT;

006E 0E80 MOVLW 0x80

0070 6E01 MOVWF gbl_result

 

}

if(result != ALL_FRUIT)

0072 0E13 MOVLW 0x13

0074 6201 CPFSEQ gbl_result

0076 D001 BRA label13

0078 D002 BRA label14

007A label13

007E label14

 

{

result = NO_FRUIT;

007A 0E80 MOVLW 0x80

007C 6E01 MOVWF gbl_result

 

}

 

// <

if(result < (APPLES + PEARS))

007E 0E00 MOVLW 0x00

0080 0A13 XORLW 0x13

0082 6001 CPFSLT gbl_result

0084 0A13 XORLW 0x13

0086 E002 BZ label15

008C label15

 

{

result = NO_FRUIT;

0088 0E80 MOVLW 0x80

008A 6E01 MOVWF gbl_result

 

}

if(result < ALL_FRUIT)

008C 0E13 MOVLW 0x13

008E 6001 CPFSLT gbl_result

0090 D002 BRA label16

0096 label16

 

{

result = NO_FRUIT;

0092 0E80 MOVLW 0x80

0094 6E01 MOVWF gbl_result

 

}

 

// >

if(result > (APPLES + PEARS))

0096 5001 MOVF gbl_result, W

0098 0E00 MOVLW 0x00

009A 0A00 XORLW 0x00

009C 0E13 MOVLW 0x13

009E E104 BNZ label17

00A0 6401 CPFSGT gbl_result

00A2 D002 BRA label17

00A8 label17

 

{

result = NO_FRUIT;

00A4 0E80 MOVLW 0x80

00A6 6E01 MOVWF gbl_result

 

}

 

if(result > ALL_FRUIT)

00A8 0E13 MOVLW 0x13

00AA 6401 CPFSGT gbl_result

 

{

result = NO_FRUIT;

00AE 0E80 MOVLW 0x80

00B0 6E01 MOVWF gbl_result

 

}

 

}

00AC 0012 RETURN

00B2 0012 RETURN

 

[code/]

 

I think we've seen something like this before... with a while statement and the difference between 6 and 5+1.

 

The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS)).

 

Orin.

Share this post


Link to post
Share on other sites
I think we've seen something like this before... with a while statement and the difference between 6 and 5+1.

 

The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS)).

 

Orin.

 

mmhhhh, I shall give that a try out as a test tomorrow.

Share this post


Link to post
Share on other sites
...The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS))...

 

You are quite right. We just started looking at this problem and it happens exactly as you described. We'll check if it can get fixed.

 

...An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant...

 

Even though sum was promoted by compiler the resulting code should work. Can you please point where you think wrong code was generated.

 

Regards,

Pavel

Share this post


Link to post
Share on other sites
...The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS))...

 

You are quite right. We just started looking at this problem and it happens exactly as you described. We'll check if it can get fixed.

 

...An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant...

 

Even though sum was promoted by compiler the resulting code should work. Can you please point where you think wrong code was generated.

 

Regards,

Pavel

 

Hi Pavel

 

This is the bad code , result will always end up set to NO_FRUIT

 

// TWO #defined constants

if(result >= (APPLES + PEARS))

0004 0E13 MOVLW 0x13

0006 84D8 BSF STATUS,Z

0008 6001 CPFSLT gbl_result

000A B4D8 BTFSC STATUS,Z

000C D001 BRA label1

000E D002 BRA label2

0010 label1

0014 label2

 

{

result = NO_FRUIT;

0010 0E80 MOVLW 0x80

0012 6E01 MOVWF gbl_result

 

}

 

this is the good code

 

// a single #defined constant

if(result >= ALL_FRUIT)

0024 0E13 MOVLW 0x13

0026 6001 CPFSLT gbl_result

0028 D001 BRA label5

002A D002 BRA label6

002C label5

0030 label6

 

{

result = NO_FRUIT;

002C 0E80 MOVLW 0x80

002E 6E01 MOVWF gbl_result

 

}

 

 

thanks

Share this post


Link to post
Share on other sites
I think we've seen something like this before... with a while statement and the difference between 6 and 5+1.

 

The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS)).

 

Orin.

 

mmhhhh, I shall give that a try out as a test tomorrow.

 

I've tried casting as Orin suggested

 

It does fix the bug

 

	// TWO #defined constants
if(result >= (APPLES + PEARS))
0004  0E13		  MOVLW 0x13
0006  84D8		  BSF STATUS,Z
0008  6001		  CPFSLT gbl_result
000A  B4D8		  BTFSC STATUS,Z
000C  D001		  BRA	label1
000E  D002		  BRA	label2
0010			label1
0014			label2

{
	result = NO_FRUIT;
0010  0E80		  MOVLW 0x80
0012  6E01		  MOVWF gbl_result

} 

// TWO #defined constants cast to char
if(result >= (char)(APPLES + PEARS))
0014  0E13		  MOVLW 0x13
0016  6001		  CPFSLT gbl_result
0018  D001		  BRA	label3
001A  D002		  BRA	label4
001C			label3
0020			label4

{
	result = NO_FRUIT;
001C  0E80		  MOVLW 0x80
001E  6E01		  MOVWF gbl_result

}

Share this post


Link to post
Share on other sites
...The expressions with the '+' seem to get promoted to an unsigned int (at least the test I did produced the same code if I cast the sum of the constants to unsigned int as with no cast) - so the compiler is trying to compare against 0x0013. Best to cast the constant to the type you want the compiler to use in the comparison... eg. if(result >= (char)(APPLES + PEARS))...

 

You are quite right. We just started looking at this problem and it happens exactly as you described. We'll check if it can get fixed.

 

...An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant...

 

Even though sum was promoted by compiler the resulting code should work. Can you please point where you think wrong code was generated.

 

Regards,

Pavel

 

Hi Pavel

 

This is the bad code , result will always end up set to NO_FRUIT

 

// TWO #defined constants

if(result >= (APPLES + PEARS))

0004 0E13 MOVLW 0x13

0006 84D8 BSF STATUS,Z

0008 6001 CPFSLT gbl_result

000A B4D8 BTFSC STATUS,Z

000C D001 BRA label1

000E D002 BRA label2

0010 label1

0014 label2

 

{

result = NO_FRUIT;

0010 0E80 MOVLW 0x80

0012 6E01 MOVWF gbl_result

 

}

 

 

It looks like the compiler is assuming that CPFSLT sets Z. CPFSLT doesn't affect any status bits.

 

Orin.

Share this post


Link to post
Share on other sites
Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant...

 

Problem fixed. Fix will be available in the next release.

 

Regards,

Pavel

Share this post


Link to post
Share on other sites
Sourceboost 6.84, and 6.85

Fault occured after upgrading from Sourceboost 6.5

Tried with PIC18F4685 and PIC16F877

Examples shown using PIC18F4685

Always reproducable.

 

An if statement comparing to two added constant generates different (wrong) code to to when comparing against a single constant...

 

Problem fixed. Fix will be available in the next release.

 

Regards,

Pavel

 

Thanks Pavel!

When is the next release expected? Or can I get a prerelease version with the fix?

Share this post


Link to post
Share on other sites
...When is the next release expected? Or can I get a prerelease version with the fix?...

 

I sent you a PM.

 

Regards,

Pavel

Share this post


Link to post
Share on other sites

Your content will need to be approved by a moderator

Guest
You are commenting as a guest. If you have an account, please sign in.
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
Sign in to follow this  

×