merged access to packed structures fields generates unaligned operations

[iguryanov]

Hi,

new optimization introduced around 2018.09 release merges short memory accesses. This is an issue for existing arc700 code at least. For example - u-boot. That codebase does not rely much on volatiles and pointers to complex structures are often passed as uchar*.

Perhaps such optimization should not be applied to packed structures?

(as a side note, even Linux might suffer is it would fire unaligned exception handler for disassembling such unaligned accesses more often - all performance gains might be lost in a worst case).

Here are some pieces from u-boot use-case:

struct ip_udp_hdr {
	u8		ip_hl_v;	/* header length and version	*/
	u8		ip_tos;		/* type of service		*/
	u16		ip_len;		/* total length			*/
	u16		ip_id;		/* identification		*/
	u16		ip_off;		/* fragment offset field	*/
	u8		ip_ttl;		/* time to live			*/
	u8		ip_p;		/* protocol			*/
	u16		ip_sum;		/* checksum			*/
	struct in_addr	ip_src;		/* Source IP address		*/
	struct in_addr	ip_dst;		/* Destination IP address	*/
	u16		udp_src;	/* UDP source port		*/
	u16		udp_dst;	/* UDP destination port		*/
	u16		udp_len;	/* Length of UDP packet		*/
	u16		udp_xsum;	/* Checksum			*/
} __attribute__((packed));

void net_set_ip_header(uchar *pkt, struct in_addr dest, struct in_addr source,
		       u16 pkt_len, u8 proto)
{
	struct ip_udp_hdr *ip = (struct ip_udp_hdr *)pkt;

	/*
	 *	Construct an IP header.
	 */
	/* IP_HDR_SIZE / 4 (not including UDP) */
	ip->ip_hl_v  = 0x45;
	ip->ip_tos   = 0;
	ip->ip_len   = htons(pkt_len);
	ip->ip_p     = proto;
	ip->ip_id    = htons(net_ip_id++);
	ip->ip_off   = htons(IP_FLAGS_DFRAG);	/* Don't fragment */
	ip->ip_ttl   = 255; // <======
	ip->ip_sum   = 0; 
	/* already in network byte order */
	net_copy_ip((void *)&ip->ip_src, &source);
	/* already in network byte order */
	net_copy_ip((void *)&ip->ip_dst, &dest);

	ip->ip_sum   = compute_ip_checksum(ip, IP_HDR_SIZE);
}

/usr/local/arcgcc821/lib/gcc/arc-snps-linux-uclibc/8.2.1/include -pipe -D__ARC__ -mA7 \ 
-mno-sdata -Os -g -mvolatile-cache -fomit-frame-pointer  \
-I/home/avinashp/bbic6/perforce/u-boot/main/u-boot-2009.06/board/ruby \ 
-Wall -Wstrict-prototypes -fno-stack-protector -c -o net.o net.c

Old logic preserved if nop is inserted manually :

a8000b70:   208a 0fff               mov   r0,-1
a8000b74:   ad08                    stb_s r0,[r13,0x8]
a8000b76:   264a 7000               nop
a8000b7a:   d811                    mov_s r0,0x11
a8000b7c:   ad09                    stb_s r0,[r13,0x9]

And this is how things are merged now (sorry, don't have bigger context):

a8000b50:   1d08 1f80 0000 11ff     st    0x11ff,[r13,8]

Regards,
Igor.

[iguryanov]

issue was actually reported against old u-boot 2009.06. It was already addressed in u-boot couple years ago - https://patchwork.ozlabs.org/patch/792238/

I see one of the reasons for this issue now is that structure type is not packed and variable is not volatile. Most reliable would be to build whole 2009.06 with "-fno-store-merging"

void
NetSetIP(volatile uchar * xip, IPaddr_t dest, int dport, int sport, int len)
{
	IP_t *ip = (IP_t *)xip;

...	ip->ip_p     = 17;		/* UDP */
	ip->ip_sum   = 0;
...
}