Dear Albert ARIBAUD, > Hi Marek, > > On Sat, 1 Sep 2012 17:12:29 +0200, Marek Vasut <ma...@denx.de> wrote: > > > > > One place where lumping and misalignement prevention did clash > > > > > was raised in the previous discussion: a 7+1 bytes > > > > > function-local char array was allocated on a non-aligned > > > > > address (which is possible and normal because it is a char) and > > > > > was initialized with some content. The compiler lumped the > > > > > initialization as two misaligned 32-byte native accesses, > > > > > despite misaligned native accesses being forbidden by compiler > > > > > command line options. This was a compiler bug. > > > > > > > > But that'd mean that instead of fixing a compiler, we'd be doing a > > > > workaround in our code? > > > > > > Not exactly. > > > > > > First, in this instance, a fix to the compiler has been at least > > > requested, if not already applied (I would need to check this). The > > > fix causes the compiler to still generate misaligned 32-bit > > > accesses *if* misaligned native accesses are allowed, and to use > > > only allowed accesses otherwise. > > > > But then again, this is compiler bug we exposed, no need to hide it. > > I'm firm on this one. > > I guess I was not clear: this issue with an 8-char local array was > *not* in U-Boot. So we exposed nothing there, and none of the > discussion led to any conclusion that we should hide anything under the > carpet. Actually, if/when we meet a compiler issue, my suggestion is > always to explicitly and lavishly comment the 'fix', whatever it is, > with warnings such as /* CAUTION! BRAINDEAD COMPILER! There is an issue > with compiler X versions Y and up where [...] */. And keep an eye on the > compiler.
Agreed > > > Second, I do not ask U-Boot contributors to mark code as explicitly > > > unaligned when the misaligned access is caused by a compiler or > > > code error; I ask them to mark code as unaligned when the misaligned > > > access is *unavoidable* because the HW or some standard imposes it. > > > > I see, I'm starting to see your point. Maybe because I've missed the > > previous discussion. > > I think so. > > > > Here, the specification from which the USB struc is derived imposes > > > a misaligned field. This, rather than any compiler bug, makes the > > > misaligned access *unavoidable*. And because it is, I ask that it be > > > marked so, by the explicit use of unaligned accessors. > > > > Still, this is unaligned only on ARM, not on maybe some other arches, > > right? > > The notion of 'misaligned' (rather than unaligned) is pretty much > architecture-independent: the fact that a 32-bit int is not on a > 4-byte boundary makes it misaligned. Now, depending on arches, this > misalignment may be irrelevant because the arch can do native misaligned > accesses with little or no penalty, or may be a worry because the arch > can (be made to) do native misaligned accesses but at a performance > cost, or it may be a blocker because the arch cannot do native > misaligned accesses. > > So maybe on some other arches misalignment is 'not a problem', or > maybe it is 'a serious issue'. Anyway, for ARM is ranges from one ed > to the other, and for any arch, on a given system the seriousness of the > issue may be set by the designers of the system. > > > > Yes. However, letting the compiler generate workarounds may end up > > > letting it generate workarounds for misaligned accesses caused by > > > errors or bugs also. Marking the code explicitly helps telling > > > which is which too. > > > > Does this work across architectures too? Like, on arm it's > > misaligned, on intel it isnt. > > Each architecture has its own capabilites regarding native misaligned > accesses... This is why I consider that as a general rule U-Boot should > always align its data properly, because (hopefully) all architectures > can do aligned native accesses; OTOH, if we accept misaligned code on > the grounds that 'it works on such and suh arches' or that 'any normal > arch should be able to handle misaligned accesses some way' or 'no one > in their right mind would physically forbit misaligned accesses', then > we're just giving Murphy a chance to kick us at some point. > > Consider this an application of Postel's principle: we liberally > accept architectures that maybe allow misaligned accesses and maybe > handle them well; and we conservatively do not do such accesses unless > we have no other choice. > > > > Here it is barely an ad hoc solution, as the alternative would be > > > fixing the hardware or worse, spec (can someone tell us where this > > > misaligned struct field originates from exactly, hw or USB spec?) > > > > http://www.intel.com/technology/usb/download/ehci-r10.pdf I think > > you're looking around 3.6 . > > I see section 3.6 (Queue Head) but I don't readily see the link > between the header and the patch we're discussiong (but I'm not an > USB expert either). Can you connect e few more dots for me? Thanks in > advance. Nevermind, I took a second look and noticed how the structure looks. Anyway, I see USB is pioneering this new rule, nice :-) I now basically see the issue at hand, sorry for being a blind ass. Remind me next time to look first and talk afterwards. 361 struct usb_hub_descriptor { 362 unsigned char bLength; 363 unsigned char bDescriptorType; 364 unsigned char bNbrPorts; 365 unsigned short wHubCharacteristics; Let's apply this patch once (if?) WD pulls my USB tree. > > > > Best regards, > > > > Marek Vasut > > Amicalement, _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
