topic Re: CW code generation in CodeWarrior for QorIQ

CW code generation

badaboom — Thu, 29 Oct 2020 09:01:28 GMT

Hi,

i've got a problem with code generation by CW 7.1 build 14.

Some settings:

__fourbyteints__ 0 (defined 0, by clearing ' 4-byte integers' checkbox on the setting panel )
MCF52235

Very simple clearing of a bit:

Enable int. by clearing corresp. bit mask:

MCF_INTC0_IMRL &= ~(MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL );

Used definitions:

#define MCF_INTC0_IMRL (*(vuint32*)(&__IPSBAR[0xC0C]))

#define MCF_INTC_IMRL_INT_MASK15 (0x8000)

#define MCF_INTC_IMRL_MASKALL (0x1)

MCF_INTC0_IMRL &= ~ (MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL );

Generated code:

; 398: MCF_INTC0_IMRL &= ~(MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL ); 
;
0x000000D4  0x41F900000000           lea      ___IPSBAR+3084,a0
0x000000DA  0x203C00007FFE           move.l   #32766,d0             ; '....'
0x000000E0  0xC190                   and.l    d0,(a0)

This also clears b31...16; and THAT was not intended. It seems that 'it' generated the mask 16 bit wide.

When you clear f.e. b14 then there's no problem.

MCF_INTC0_IMRL &= ~ (MCF_INTC_IMRL_INT_MASK14 | MCF_INTC_IMRL_MASKALL );

;397: MCF_INTC0_IMRL &= ~ (MCF_INTC_IMRL_INT_MASK14 | MCF_INTC_IMRL_MASKALL ); 
;
0x000000C6  0x41F900000000           lea      ___IPSBAR+3084,a0
0x000000CC  0x203CFFFFBFFE           move.l   #-16386,d0            ; '....'
0x000000D2  0xC190                   and.l    d0,(a0)

Also when you cast to a uint32 the code is correct.

MCF_INTC0_IMRL &= ~ (uint32)(MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL );

;399:MCF_INTC0_IMRL & = ~(uint32 (MCF_INTC_IMRL_INT_MASK15 |  MCF_INTC_IMRL_MASKALL ); 
;
0x000000E2  0x41F900000000           lea      ___IPSBAR+3084,a0
0x000000E8  0x203CFFFF7FFE           move.l   #-32770,d0            ; '....'
0x000000EE  0xC190                   and.l    d0,(a0)

Also when you try to clear b31, the compiler uses 64 bit sizes??

;400: MCF_INTC0_IMRL &= ~ (MCF_INTC_IMRL_INT_MASK31 | MCF_INTC_IMRL_MASKALL ); 
;
0x000000F0  0x4DF900000000           lea      ___IPSBAR+3084,a6
0x000000F6  0x2F560004               move.l   (a6),4(a7)
0x000000FA  0x41EF0014               lea      20(a7),a0
0x000000FE  0x2E88                   move.l   a0,(a7)
0x00000100  0x4EB900000000           jsr      ___rt_ultoi64
0x00000106  0x72FF                   moveq    #-1,d1
0x00000108  0x203C7FFFFFFE           move.l   #2147483646,d0        ; '....'
0x0000010E  0x2F400010               move.l   d0,16(a7)
0x00000112  0x2F41000C               move.l   d1,12(a7)
0x00000116  0x2010                   move.l   (a0),d0
0x00000118  0x2F6800040008           move.l   4(a0),8(a7)
0x0000011E  0x2F400004               move.l   d0,4(a7)
0x00000122  0x41EF001C               lea      28(a7),a0
0x00000126  0x2E88                   move.l   a0,(a7)
0x00000128  0x4EB900000000           jsr      ___rt_and64
0x0000012E  0x2CA80004               move.l   4(a0),(a6)
;
;  401:  MCF_INTC0_IMRL &= ~ (uint32)(MCF_INTC_IMRL_INT_MASK31 | MCF_INTC_IMRL_MASKALL ); 
;
0x00000132  0x41F900000000           lea      ___IPSBAR+3084,a0
0x00000138  0x203C7FFFFFFE           move.l   #2147483646,d0        ; '....'
0x0000013E  0xC190                   and.l    d0,(a0)
;

But when you: #define __fourbyteints__ 1

then the generated code is ok.

My question is, how can avoid this code generation error without the need of

explicitly casting when there seems no need to do so.

thanks,

René

Re: CW code generation

CompilerGuru — Fri, 27 Feb 2009 00:05:38 GMT

Sounds like an you should submit a service request for this (if you not already did)

https://www.freescale.com/webapp/servicerequest.create_SR.framework?regFlag=fromOpenSR

The problem is how could any user of the compiler know how to safely workaround the issue?

Pretty much only a compiler engineer working on that compiler can give you a safe answer. Also it's

important to get such issues fixed.

Daniel

Re: CW code generation

jbezem — Fri, 27 Feb 2009 15:40:53 GMT

I didn't check any settings of the compiler and your variant, but if you clear '__fourbyteints__', you'd have 16-bit integers, I assume. That means that your 0x8000 bit definition is taken to be a negative number (signed!).

Bitwise negation of signed numbers is never a good idea, but since we're operating on 16-bit integers, the negated mask becomes 0x7FFE (positive again!), and the generated code seems OK to me, even if it is not what you intended.

With 4-byte integers the situation is different: 0x8000 is effectively 0x00008000, so your inverted bit-mask becomes 0xFFFF7FFE, what you intended.

To remedy this without too many casts, you could use 0x8000U, which in case of 2-byte ints would be converted to long (still signed, but 32-bit!), and everything would work as expected; I didn't verify this, though.

In case of 4-byte ints this mask gets converted to 'unsigned int', so it will work also.

Or you add at least one cast to convert one of the bit-masks to the type of your target, like ((vuint32)0x8000U), to make everything clear.

IMHO this is not a compiler problem.

HTH,

Johan

Re: CW code generation

CompilerGuru — Fri, 27 Feb 2009 15:55:19 GMT

Silly me, I did not realize that the compiler actually was correct.

With 16 bit ints, the constant 0x8000 has type unsigned int, hence the expression

(0x8000 | 1) has type unsigned int to and the value 0x8001u.

Therefore the complement (still type unsigned int) has the value 0x7FFEu, and &= 0x7FFEu does indeed clear the more significant 16 bits too.

Not sure why the compiler uses 64 bit for clearing the 31st bit. How are the constants defined?

That could also be a compiler issue, although not wrong, just inefficient.

If the operation should always work on at least 32 bits I would make the constants of type unsigned long by using

a uL suffix

> #define MCF_INTC_IMRL_INT_MASK15 (0x8000uL)

Daniel

Re: CW code generation

badaboom — Thu, 29 Oct 2020 09:01:30 GMT

Thanks for the replies..

I think it's better to cast at the operation then

to check the 'Coldfire module header files' for all '0x8000's

MCF_INTC0_IMRL &= ~ (uint32)(MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL );
MCF_INTC0_IMRL &= ~ (uint32)(MCF_INTC_IMRL_INT_MASK31 | MCF_INTC_IMRL_MASKALL );
becomes:

;398: MCF_INTC0_IMRL &= ~(uint32)(MCF_INTC_IMRL_INT_MASK15|
                                  MCF_INTC_IMRL_MASKALL);
;
0x000000DC  0x41F900000000           lea      ___IPSBAR+3084,a0
0x000000E2  0x203CFFFF7FFE           move.l   #-32770,d0            ; '....'
0x000000E8  0xC190                   and.l    d0,(a0)
;
;399: MCF_INTC0_IMRL &= ~(uint32)(MCF_INTC_IMRL_INT_MASK31 |
                                  MCF_INTC_IMRL_MASKALL);
;
0x000000EA  0x41F900000000           lea      ___IPSBAR+3084,a0
0x000000F0  0x203C7FFFFFFE           move.l   #2147483646,d0        ; '....'
0x000000F6  0xC190                   and.l    d0,(a0)

Just redefining the constant as: #define MCF_INTC_IMRL_INT_MASK15 (0x8000u) doesn't force the compiler to use unsigned. After negation the result is positive and will fit in THE 16bit wide int, and therefore clearing the high word (as positive result).

Only making it 32 bit wide solves this.

#define MCF_INTC_IMRL_INT_MASK15 (0x8000uL) or

#define MCF_INTC_IMRL_INT_MASK15 (uint32)(0x8000)

I think I agree it's not a compiler issue but let's say I wasn't forwarned because the operation output is an vuint32 and there's wasn't a compiler warning "Implicit arithmetic conversion from 'signed xxx' to 'unsigned xxx'.

René

Re: CW code generation

jbezem — Fri, 27 Feb 2009 21:44:19 GMT

badaboom wrote:
...
I think it's better to cast at the operation ...

Agreed.

...
Just redefining the constant as: #define MCF_INTC_IMRL_INT_MASK15 (0x8000u) doesn't force the compiler to use unsigned.

Yes, it does, but contrary to what I suggested, it doesn't convert to 'signed long', but leaves it to be an 'unsigned int'; my mistake. So it remains 16-bit wide until after the negation, and then gets zero-extended to 32-bits before and-ing with the resulting value. Not what you intended.

...
Only making it 32 bit wide solves this.
#define MCF_INTC_IMRL_INT_MASK15 (0x8000uL) or
#define MCF_INTC_IMRL_INT_MASK15 (uint32)(0x8000)

Correct. I'd use the cast as you initially indicated, since it will provide correct results independent from the size of an 'int'. Using 'uL' may prove to extend to 64-bits when using 4-byte ints, which works, but is an exaggeration and slow. If you don't care about portability, though, you may go ahead.

I think I agree it's not a compiler issue but let's say I wasn't forwarned because the operation output is an vuint32 and there's wasn't a compiler warning "Implicit arithmetic conversion from 'signed xxx' to 'unsigned xxx'.

...

That's why I use PC Lint from Gimpel.

BR,

Johan

Re: CW code generation

MrBean — Tue, 03 Mar 2009 20:32:00 GMT

CompilerGuru wrote:

With 16 bit ints, the constant 0x8000 has type unsigned int, hence the expression
(0x8000 | 1) has type unsigned int to and the value 0x8001u.
Therefore the complement (still type unsigned int) has the value 0x7FFEu, and &= 0x7FFEu does indeed clear the more significant 16 bits too.

If that were true, then 0x4000|1 would result in the complement 0xCFFEu.

Extending 0xCFFEu to 32 bits would result in 0x0000CFFEu.

It does not.

The problem starts with the fact that then 16bit int constant is apparently signed.

When this is expanded to 32bits because &= other argument is 32bit, the compiler does not take in account that this other argument and the destination is NOT signed (it is vuint32).

This is what happens:

0x4000|1 = 0x4001, ~0x4001 = 0xCFFE, ->32bit = 0xFFFFCFFE, &=

0x8000|1 = 0x8001, ~0x8001 = 0x7FFE, ->32bit = 0x00007FFE, &=

The conversion to 32 bits is not done at the same point as the conversion to unsigned.

The compiler seems to think it is signed.

This does not make sense to me as the result is stored in a vuint32.

Message Edited by MrBean on 2009-03-03 13:34 01:34 PM

Re: CW code generation

jbezem — Tue, 03 Mar 2009 20:49:12 GMT

MrBean wrote:
...
This is what happens:
0x4000|1 = 0x4001, ~0x4001 = 0xCFFE, ->32bit = 0xFFFFCFFE, &=
0x8000|1 = 0x8001, ~0x8001 = 0x7FFE, ->32bit = 0x00007FFE, &=
Sorry to disagree:
0x4000|1 = 0x4001, ~0x4001 = 0xBFFE, ->32bit = 0xFFFFBFFE, &=
0x8000|1 = 0x8001, ~0x8001 = 0x7FFE, ->32bit = 0x00007FFE, &=

(Note the 'B' instead of 'C'). Other than that, for 16-bit ints, I agree.
And AFAIK, unadorned constants are (usually?/always?/must be?) considered 'int' and signed, even according to the Standard.

FWIW,

Johan

Re: CW code generation

MrBean — Tue, 03 Mar 2009 20:52:38 GMT

sorry for the calculation typo, BFFE is right.

But still, i think it should either warn or not do it in this fashion.

Re: CW code generation

MrBean — Thu, 29 Oct 2020 09:01:32 GMT

To show how inconsistent the behaviour is to me, try explaining this one :

;  401:  MCF_INTC0_IMRL = ~( ~MCF_INTC0_IMRL |  ( MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL ) ;
0x0000017E  0x203900000000           move.l   ___IPSBAR+3084,d00x00000184  0x4680                   not.l    d00x00000186  0x008000008001           ori.l    #0x8001,d0            ; '....'0x0000018C  0x4680                   not.l    d00x0000018E  0x23C000000000           move.l   d0,___IPSBAR+3084

Message Edited by MrBean on 2009-03-03 15:05 03:05 PM

Re: CW code generation

jbezem — Tue, 03 Mar 2009 22:40:25 GMT

OK, assuming ints are 16-bit:

- in the bit-or expression, the negation of MCF_INTC0_IMRL has the highest precedence; the macro has type uint32, and is loaded into d0.

- then the negation is performed using d0.

- since the negated expression still has the type uint32, the integer 0x8000 is implicitly converted to uint32 (left-to-right associativity of operator bit-or); however, I read-up on conversion of constants: Since the constant is given as 0x8000 (and not 32768, the same value but a different notation!) it is considered of type 'unsigned int', not signed. See H&S 5th edition, 2.7.1: "An interesting point to note ... is that integers in the range of 2^15 through 2^16-1 will have positive values when written as decimal constants but negative values when written as octal or hexadecimal constants (and cast to type 'int')");

- and or-ed into the result (later optimized and taken together with the next constant)

- then 0x0001 is taken (type: signed int), and also converted to uint32 (sign-extension is trivial here), and ored into the result.

And finally the result is negated again, and stored into the variable/register.

QED IMHO.

FWIW,

Johan

Re: CW code generation

jbezem — Tue, 03 Mar 2009 22:44:16 GMT

BTW, isn't there a parenthesis missing in the commented source-line? I see two opening and only one closing paren. Cut&Paste error?

The reasoning remains almost the same even if the evaluation order changes a little; the anomaly lies in the difference between 0x8000 and 32768.

BR,

Johan

Re: CW code generation

CompilerGuru — Wed, 04 Mar 2009 00:00:10 GMT

> If that were true, then 0x4000|1 would result in the complement 0xCFFEu.

> Extending 0xCFFEu to 32 bits would result in 0x0000CFFEu.

> It does not.

Well my text was using 0x8000 not 0x4000. Different number, different rule :smileyhappy:.

The constant 0x8000 has type unsigned int because it does not fit into a (16 bit signed) int type.

The constant 0x4000 fits, and therefore has type (signed) int. Therefore the extension to long in

(long)(~(0x4000+1))

is a sign extension, the long gets negative.

The following expressions though will result in a positive long <= 0xFFFF, the extension to long was done from a unsigned type (assuming 16 bit ints):

(long)(~(0x4000u+1))

(long)((unsigned int)~(0x4000+1))

(long)(~(0x8000 - 0x4000 + 1))

So if you intend to use 32 bit arithmetic in ANSI-C I would recommend to use 32 bit constants, say with a 16 bit int type to write it in one of those forms:

0x4000L

0x4000uL

(long)0x4000

(unsigned long)0x4000

Daniel

Re: CW code generation

CompilerGuru — Wed, 04 Mar 2009 00:02:29 GMT

>The reasoning remains almost the same even if the evaluation order changes a little;

> the anomaly lies in the difference between 0x8000 and 32768

Is that not the same?

Re: CW code generation

MrBean — Thu, 29 Oct 2020 09:01:33 GMT

Stop seeing all my typo's

jbezem wrote:
The reasoning remains almost the same even if the evaluation order changes a little; the anomaly lies in the difference between 0x8000 and 32768.

The 0x8000 and 32768 difference is a result, not a cause, it is compiler ouput.

The definitions are not changed.

The output indeed shows that evaluation order is of influence.

The logic of it is still unclear to me.

For completeness:

;  394: MCF_INTC0_IMRL = ~( ~MCF_INTC0_IMRL |   ( MCF_INTC_IMRL_INT_MASK14 | MCF_INTC_IMRL_MASKALL )) ;
;
0x00000150  0x203900000000           move.l   ___IPSBAR+3084,d0
0x00000156  0x4680                   not.l    d0
0x00000158  0x008000004001           ori.l    #0x4001,d0            ; '..@.'
0x0000015E  0x4680                   not.l    d0
0x00000160  0x23C000000000           move.l   d0,___IPSBAR+3084
;
;  395: MCF_INTC0_IMRL = ~( ~MCF_INTC0_IMRL |   ( MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL )) ;
;  396: 
;
0x00000166  0x203900000000           move.l   ___IPSBAR+3084,d0
0x0000016C  0x4680                   not.l    d0
0x0000016E  0x008000008001           ori.l    #0x8001,d0            ; '....'
0x00000174  0x4680                   not.l    d0
0x00000176  0x23C000000000           move.l   d0,___IPSBAR+3084
;
;  397: MCF_INTC0_IMRL =  (  MCF_INTC0_IMRL &  ~( MCF_INTC_IMRL_INT_MASK14 | MCF_INTC_IMRL_MASKALL )) ;
;
0x0000017C  0x203CFFFFBFFE           move.l   #-16386,d0            ; '....'
0x00000182  0xC1B900000000           and.l    d0,___IPSBAR+3084
;
;  398: MCF_INTC0_IMRL =  (  MCF_INTC0_IMRL &  ~( MCF_INTC_IMRL_INT_MASK15 | MCF_INTC_IMRL_MASKALL )) ;
;0x00000188  0x203C00007FFE           move.l   #32766,d0             ; '....'0x0000018E  0xC1B900000000           and.l    d0,___IPSBAR+3084;

Message Edited by MrBean on 2009-03-03 17:12 05:12 PM

Re: CW code generation

jbezem — Wed, 04 Mar 2009 00:19:05 GMT

I didn't check the 'for completeness' yet, but the difference between 0x8000 and 32768 is in the language definition, and therefore in the compiler. The quote I gave is from " C A Reference Manual" by Harbison and Steele, the best reference on C I have ever seen.

To make it even more clear: The compiler _has_ to interpret 0x8000 and 32768 differently.

I'll have a look at the rest later.

BR,

Johan

Re: CW code generation

MrBean — Wed, 04 Mar 2009 00:21:30 GMT

Hmm.. maybe it has to do with a 2's complement being not exactly the same as a bitwise not ...

Re: CW code generation

jbezem — Wed, 04 Mar 2009 00:39:18 GMT

From what I've seen, and IMHO, the compiler is correct in all cases, and I'll explain why:

- the first is trivial: MASK14 means a signed int 0x4000 is used, or-ed with a signed int 0x0001 to produce a positive signed int, and then sign-extended and or-ed with the negated uint32: 0x0004001 is correct.

- the second is a repetition of your previous post; my argument changes a little because of the parentheses: 0x8000 is unsigned int, combined with 0x0001 as signed int, gives an unsigned int 0x8001; zero-extended to be combined with a negated uint32: 0x00008001 is correct.

- the third combines two signed int values into a signed int 0x4001, negates all bits to get a signed int 0xBFFE (negative!), and gets sign-extended to uint32: 0xFFFFBFFE is correct.

- the fourth case is different again: 0x8000 is unsigned int, combined with signed int 0x0001 to give unsigned int 0x8001, all bis negated to unsigned int 0x7FFE, and zero-extended to be combined with uint32: 0x00007FFE is correct.

Any objections?

FWIW,

Johan

Re: CW code generation

jbezem — Wed, 04 Mar 2009 00:49:24 GMT

From a mathematical point of view, they are the same, but on a 16-bit integer machine, unadorned constants between 2^15 and 2^16-1 are treated differently if specified as hex/octal (0xFFFF is unsigned int) or if specified in decimal (65535 is signed long).

Did I write 'long'? Yes I did, and correctly so... I may have to revise my four answers then once again. Sorry, MrBean! The day was long, I'll leave that for tomorrow.

Update: No, of course not. All constants in the four cases of MrBean were in hex, so I do not have to revise my answers. OK, shut down the machine and relax a bit.

Bye,

Johan

Message Edited by jbezem on 2009-03-03 05:59 PM

Re: CW code generation

MrBean — Wed, 04 Mar 2009 01:41:52 GMT

CompilerGuru wrote:
The constant 0x8000 has type unsigned int because it does not fit into a (16 bit signed) int type.
The constant 0x4000 fits, and therefore has type (signed) int. Therefore the extension to long in
(long)(~(0x4000+1))
is a sign extension
....

*That* makes sense !

That also means all header files provided by Freescale are wrong/dangerous, once the '4-byte integers' checkbox on the setting panel is switched off.

So: Leave the checkbox on, or change all header files so that constants have a type that fits the register they are meant for.

The first is easiest, the latter would be more correct.

Thanx for all the input everyone !

Message Edited by MrBean on 2009-03-03 18:48 06:48 PM