On 14/09/2021 20:48, Rafał Pietrak wrote:
W dniu 13.09.2021 o 13:41, David Brown pisze:
On 13/09/2021 11:51, Rafał Pietrak via Gcc wrote:
Hi,
Thenk you for very prompt reply.
(I'm not sure how much this is a "gcc development list" matter, rather
than perhaps a "gcc help list" or perhaps comp.arch.embedded Usenet
group discussion, but I'll continue here until Jonathan or other gcc
developers say stop.)
Thank you. I appreciate it.
And yet, I do think, that it's about the "core" gcc - it's about a
programmer is able to "talk to gcc" without much "missunderstanding".
But I'm not going to push it much more. I accept, that "talking to gcc"
with d->cr1 &= ~ { ... } syntax is not what majority of programmers
would like to be able to do.
The gcc developers are always interested in new ideas. But they are not
keen on new syntaxes or extensions - there has to be a /very/ good
reason for them. The days of gcc being a maverick that makes up its own
improved C are long, long gone - they'd much rather work with the C and
C++ committees towards improving the languages in general, instead of
having more of their own non-standard additions. So an extension like
this should ideally be a proposal for adding to C23, though gcc could
always implement it earlier. And while I appreciate what you are trying
to do here, it is simply not general enough or important enough to
justify such changes. To get a new feature implemented, it has to do
something you could not do before, or do it /far/ more simply, clearly,
safely or efficiently.
W dniu 13.09.2021 o 10:44, Jonathan Wakely pisze:
On Mon, 13 Sept 2021 at 07:55, Rafał Pietrak via Gcc <gcc@gcc.gnu.org> wrote:
[-----------------]
#elif VARIANT_WORKING
struct cr_s a = (struct cr_s) {
.re = 1,
.te = 1,
.rxneie = 1,
.txeie = 1,
.ue = 1 };
int *b = (int *) &a;
d->cr1 &= ~(*b);
This is a strict aliasing violation. You should either use a union or
memcpy to get the value as an int.
Yes, I know. I know, this is a "trick" I use (I had to use to missleed
gcc).
Don't think of it as a "trick" - think of it as a mistake. A completely
unnecessary mistake, that will likely give you unexpected results at times :
union {
struct cr_s s;
uint32_t raw;
} a = {(struct cr_s) {
.re = 1,
.te = 1,
.rxneie = 1,
.txeie = 1,
.ue = 1 }
};
d->cr1 &= ~(a.raw);
Ha! This is very nice.
But pls note, that if contrary to my VARIANT_WORKING this actually is
kosher (and not an error, like you've said about the my "WORKING"), and
it actually looks very similar to the VARIANT_THE_BEST ... may be there
is a way to implement VARIANT_THE_BEST as "syntactic trick" leading
compiler into this semantics you've outlined above?
The issue I noticed with your "WORKING" is the bit order - that is
orthogonal to the code structure, and easy to fix by re-arranging the
fields in the initial bit-field. It is independent from the structure
of the code.
I'm raising this questions, since CR1 as int (or better as uint32_t) is
already declared in my code. Compiler shouldn't have too hard time
weeding out struct cr_s from union embedding it.
The code to convert between a bit-field of size 32-bit and a uint32_t is
nothing at all, and the compiler can handle that fine :-) But you have
to write the source code in a way that the conversion is well defined
behaviour. When you write something that is not defined behaviour, the
compiler can generate code in ways you don't expect because technically
you haven't told it what you actually want.
I hope you also realise that the "VARIANT_TRADITIONAL" and
"VARIANT_WORKING" versions of your code do different things. The ARM
Cortex-M devices (like the STM-32) are little-endian, and the bitfields
are ordered from the LSB. So either your list of #define's is wrong, or
your struct cr_s bitfiled is wrong. (I haven't used that particular
device myself, so I don't know which is wrong.)
I'm sory. this is my mistake. I've taken a shortcut and quickly written
an at hoc '#defines' for the email. In my code I have:
enum usart_cr1_e {
USART_SBK, USART_RWU, USART_RE, USART_TE, USART_IDLEIE,
USART_RXNEIE, USART_TCIE, USART_TXEIE, USART_PEIE, USART_PS,
USART_PCE, USART_WAKE, USART_M, USART_UE,
};
And gcc produces exactly the same code in both variants:
8000c56: 68d3 ldr r3, [r2, #12]
8000c58: f423 5302 bic.w r3, r3, #8320 ; 0x2080
8000c5c: f023 032c bic.w r3, r3, #44 ; 0x2c
8000c60: 60d3 str r3, [r2, #12]
Great.
Also - perhaps beside the point, but good advice anyway - for this kind
of work you should always use the fixed-size <stdint.h> types such as
"uint32_t", not home-made types like "uint". And you always want
unsigned types when doing bit operations such as bitwise complement.
Yes. That's true. No problem here.
Using a union of the bitfield struct with a "raw" combined field is a
common idiom, and gives you exactly what you need here. It is
guaranteed to work in C, unlike your code (which has undefined
behaviour). If it is important to you, you should note that it is not
defined behaviour in C++ (though maybe gcc guarantees it will work - the
documentation of "-fstrict-aliasing" is not clear on the matter).
Somehow I've missed it until now, always falling back to things like
(int *) or even (void *). That's a good lesson for me.
I always view any use of "void *" with suspicion - it is telling the
compiler you don't know your real type. It has its uses, of course, but
I prefer to be more explicit when I can. I would never use a cast to
"int *" for accessing anything other than an int, and even with
"uint32_t *" I'd normally have a "volatile" in there, and probably have
an access function or wrapper too. (With "volatile", the compiler will
follow your instructions strictly - it lets you do messy casts safely,
but can lead to inefficiencies.)
As Jonathan says, a small inline function or macro (using gcc's
extension of declarations and statements inside an expression) can be
used for a wrapper that will work simply and efficiently while being
safe for both C and C++ :
#define raw32(x) \
({ uint32_t raw; memcpy(&raw, &x, sizeof(uint32_t)); raw;})
struct cr_s a = (struct cr_s) {
.re = 1,
.te = 1,
.rxneie = 1,
.txeie = 1,
.ue = 1 }
};
d->cr1 &= ~raw32(a);
May be I'll test it in some future. For the time being I'm very
satisfied with the assignment to a temporary union variable.
but having said that, I strongly believe (particularly if union variable
is kosher), that gcc recognising VARIANT_THE_BEST as a syntax shortcut
to such union temporary variable could significantly improve code
readibility.
Type-punning via unions is guaranteed to work in C, but undefined
behaviour in C++. (gcc may choose to define it, however.)
For the safest and most portable way of accessing raw memory, memcpy()
always works. gcc (and many other compilers) fully optimise memcpy()
calls that have fixed sizes like this - the code generated here is as
optimal as any other solution. Some compilers, however, will give you a
library call which is obviously not efficient.
gcc has another extension that can also be useful for this kind of thing:
typedef uint32_t __attribute__((may_alias)) uint32_a;
Now you can safely use a cast to "uint32_a *", rather than "uint32_t *",
and access data - gcc knows it may alias the struct :
struct cr_s a = ....
d->cr &= ~ *((uint32_a*) &a);
So thank you for extended explanation, and I'm currently staying off the
list hoping that my perspective will eventually catch some traction :)
This is still copied to the list. If you want to look a bit at the
list, without following everything, I recommend using the gmane.org
Usenet gateway that provides a lot of mailing lists in convenient Usenet
format. There is plenty of interesting discussions on the gcc mailing
lists. I personally haven't done any gcc development - I'd love to do
so, but haven't had the time to dig into it - but I sometimes get to
help others and save the real developers some time.
mvh.,
David
Thank you again,
Rafał
