Recommendations for ARM compilers

Actually, surely on an architecture that has the concept of bus error for unaligned accesses, like yours, packing of struct members that moves elements to "illegal" addresses should plain and simply never have been allowed in the first place.

I would say that using such a feature, you're firmly in "whatever silly result you get is exactly what you asked for" territory.

--
Hans-Bernhard Broeker (broeker@physik.rwth-aachen.de)
Even if all the snow were burnt, ashes would remain.

Mmm, I have to admit I don't know how such a cast can be done.

That's exactly what happened. Let's talk (pseudo)C:

typedef __packed struct { uint8_t a[n]; uint16_t field1; uint16_t field2; uint16_t field3; } the_struct;

void update_field( uint16_t * ); ...

the_struct THESTRUCT;

...

// This line does not generate any error/warning update_field( &THESTRUCT.field1 );

Sorry, I didn't remove them, I just quoted manually. Had to read Help to learn how to do this properly with Google Groups.

The google groups interface to usenet is seriously broken. However you can do fairly well if you follow the instructions in my sig. below.

--
"If you want to post a followup via groups.google.com, don't use
 the broken "Reply" link at the bottom of the article.  Click on 
 "show options" at the top of the article, then click on the 
 "Reply" at the bottom of the article headers." - Keith Thompson

So you are using a non-portable, non-standard and dangerous construct and the compiler doesn't warn of alignment issues when you do something with it that violates the processors alignment restrictions.

Why do you feel the need to use packed in the first place? Every time I've looked at it, it appeared to be far more trouble than it was worth. Most of the adavantages of it appear to be illusory to me.

Robert

well, it's not my code. But it's a real life code, part of the large software project that has been ported from an 8-bit platform to ARM (and then from ADS 1.2 to GCC).

That's what I'm actually interested in: should there be a warning for such a construct?

I don't feel the need for it at all in this particular example. What seems to have happened is: in the process of porting from 8-bit to 32-bit, it became clear that certain structures has to be __packed (for whatever reason) to make the code work correctly, it was also considered worthwhile to apply __packed to all the structures in the project to save memory. Luckily, it was the only bug introduced this way.

[and my apologies to the OP: my remark has led the discussion away from the original goal (as it always happens in a newsgroup)]

A very bad idea if the structure already has aligned fields or the space saving is very small. Most compilers will generate 4 byte accesses for a

32bit word in a packed structure, and performance will be abysmal.

---druck

--
The ARM Club Free Software - http://www.armclub.org.uk/free/
The 32bit Conversions Page - http://www.quantumsoft.co.uk/druck/

There should not be one, there should be at least four:

1) the moment it saw "__packed", the compiler should have output a "Danger Will Robinson! Needless loss of portability encountered!" warning.

2) The moment it saw that the effect of __packed was to place any structure field into a position inaccessible by an ordinary pointer, it should have either refused to do that, or output another warning "Potential loss of usability."

3) If it allowed misaligned elements, it should have refused to let you take the address of such a thing, or printed yet another warning "Unusable pointer value computed."

4) Even it did let you pass all the way through here, it should have refused to let you assign this pointer as if it was a correct pointer to an object, or output an other warning "Use of misaligned pointers is almost guaranteed to fail."

One alternative that would allow to not issue warnings 2 to 4 would be to make all C pointers inherently capable of addressing even misaligned values, either by installing a CPU exception handler for misaligned access that fixes it up, or by introducing an abstraction layer between pointer values and actual addresses that checks every pointer usage for possible misalignment. Both of these tend to be performance desasters, so you don't really want to go down that road.

So the code in question does one questionable, one foolish, and one completely crazy thing, in that order. Sometime between the questionable and the foolish, the compiler made a decision that exposes the last in the code's list of sins as the crazy thing it is, and you say you want to be warned only when it's reached the "crazy" stage. That's a strange view of the world you have there.

I say the compiler should have bluntly refused to cooperate at the foolish step of taking the address of a packed structure element, and given you a stern warning about the effects of its own decision combined with the questionable use of __packed before that. Or it should have simply not packed that densely. But then your ulterior motive for using the __packed hack would have failed to work out, wouldn't it?

Actually, from what you say, the only thing that became clear at that point was that the 8-bit code was not particularly portable to begin with, and that the people who ported it to ARM had no idea how to

*properly* fix that problem once and for good. __packed is hardly ever the right way of doing that, because it's based on the fundamentally wrong concept of equating internal data structures of a C program with externally defined data exchange formats.

That was quite a silly idea, to put it mildly. What on earth would you conserve individual bytes for, given that

1) the smallest memory chips you can easily get your hands on are a factor of 4 larger than the entire supported address range of the previous CPU 2) the new CPU can address more memory than you can afford 3) the new CPU may kill your program on a misaligned access? ?

... the only one you've *found* so far. Absence of evidence is not evidence of absence.

--
Hans-Bernhard Broeker (broeker@physik.rwth-aachen.de)
Even if all the snow were burnt, ashes would remain.

I won't ask you to defend it then ;)

Maybe. In fact my inclination would be that it should, but we are well into implementation defined constructs here.

The biggest problem I see is that the only solutions appear to be - a (yet another) keyword to indicate the pointer passed to the routine may be unaligned. If done globally the cost of accessing via pointers will have just gone through the roof. Can you say code bloat? If done only on routines that get passed pointers to (potentially) unaligned pointers, and you rely on the compiler to warn on those instances you end up with a sort of creeping rot. - Forbid passing pointers to structure members. - Forbid passing unaligned pointers and rely on the compiler warning about all the cases. That would make me a bit nervous but the compiler might be up to it. - Forbid passing unaligned pointers and rely on the user recognizing the forbidden cases. You already know why you don't like that one :)

Frankly I don't see any viable solution on an architecture with alignment restrictions other than to forbid the use of packed. Or if you must use packed forbid taking the address of member elements of packed structures.

Hmm, that strikes me as applying a non-standard keyword to cover up a bug rather than fixing the bug in the first place.

Um, if they want to apply packed to all structure why not just tell the compiler that rather than add the keyword to every structure. You are much less likely to miss an instance that way. It still strikes me as a bad optimization though, it takes more time and space to access unaligned members.

Robert

Indeed, and for this reason it is generally a bad idea to pack all fields in a structure. It is much better to pack just the fields that need to be packed. This way most fields will be aligned to their natural alignment and thus accessing them is efficient.

Packing all fields or assuming all pointers have lower than natural alignment is only feasible on CPUs that have unaligned support in hardware, such as all new ARM cores.

Wilco

No, there shouldn't be - it is an error. The example you gave produces a compile-time error in ADS1.2 as expected. It is illegal to lose the packed qualifier, but it is legal to gain it - it works just like const and volatile.

Wilco

The gcc documentation explicitly states that if the packed attribute is used, that the resultant code is incompatible with code not using the packed attribute. From compiling a number of examples it appears as if all accesses to structures are made using byte accesses when using the packed attribute. On the Philips LPC2xxx series of ARM MCUs, many people uses structures to access the peripheral registers. The code is completely broken when using the packed attribute.

Regards Anton Erasmus

Here it gets interesting. I've just checked (had to ask my colleague with yet unexpired ADS license) - and there's *no* warning or error. ADS1.2 [Build 848]. The commandline is: [-O0 -Otime -asm -Oautoinline -g+ -fs -Wq -Ec]

Could you quote the exact error message, please?

And thanks to all of you who produced such lengthy posts explaining how stupid it is to use __packed ;-) I can't afford replying to every post and don't feel it's really necessary because I completely agree with you on most points.

I can assure you that the bug is about to be fixed properly :), so the question is of purely academic interest.

volatile.

All right, looks like '-Ec' suppresses the error. Haven't tried w/out it yet but the description in the docs gives a good hint on this. No bleeping idea who put it into the makefile and for what reason :)

Went to double-check the options used for gcc now.

volatile.

Well, obviously to get rid of the alignment errors, since they couldn't possibly be a problem ;)

Robert

packed

volatile.

Note that -O0 means that all optimizations are switched off (ie. slow&large code), so -Otime has no effect.

Possibly. It is likely however that the code simply didn't compile without ignoring illegal implicit casts - most code isn't even close to being standards compliant, so all compilers accept quite loose C by default, and have many options to accept even more arcane C.

Wilco

I am very dappy with gcc. Lot of help available online on IRC, newsgroups and ML.

gcc:

(Bare Metal GCC for ARM)

use

-march=armv4 -mtune=arm9t options

just use

-mno-thumb-interwork option to forbide this :)

so do I :)

just use the telnet version that supports gdb, and thats it.

arm-none-eabi-as is the assembler

arm-none-eabi-gcc to compile

arm-none-eabi-ld to link

arm-none-eabi-objcopy to extract bits of code

arm-none-eabi-objdump to examine the files

arm-none-eabi-gdb to upload the data in the board through the network interface.

This can work using Linux or Windows. Since it is CLI, it isvery easy to write scripts an automise procedures.

Since everything is text file in the source tree, it is trivial to use a versionning system like subversion or CVS to allow sharing the development (even with multisite developpers, so that many ppl can contribute to the code, but only my can test it in the board), when the versionning systemallow to tag which version doeswhat, has bugs, include reports and comments, and reload old version of the source tree.

I ve never seen versionning system for other project managers.

Note that my project is not exactly a standard free software project since it is leaded by a company which pays some developpers.

Once Makefiles are written, after changing some line of code, just run 'make' to recompile and upload the newcode in the board.

--
DEMAINE Benoit-Pierre (aka DoubleHP ) http://www.demaine.info/
\_o< If computing were an exact science, IT engineers would not have work >o_/

4-5 years I was using ARM SDT series. And have a few comments on th compiler.

While the IDE was pretty stable and very convenient to use, it seem

to me that the compiler lacked several good optimizations, e.g. loca and global CSE (common subexpression elimination) didn't seem to work This was true for SDT