a feature to the wishlist

[Rafał Pietrak]

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.

> 
>>
>> 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?

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.

> 
> 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]


> 
> 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.

> 
> 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.

So thank you for extended explanation, and I'm currently staying off the
list hoping that my perspective will eventually catch some traction :)


Thank you again,

Rafał