[Scummvm-devel] Wii & Gamecube (resp. PowerPC) vs. unaligned memory access

[Johannes Schickel]

> Am 08.05.2011 um 18:28 schrieb Johannes Schickel:
> >> Regarding 3: According to some googling, the "may_alias" attribute has
> >> been around since GCC 3.3 (and "packed" since 2.x). Any reason we
> >> restrict this codepath in endian.h to GCC 4.x ? And out of curiosity,
> >> why the trick with the packed struct (a comment in endian.h explaining
> >> that would be helpful for future generations, too ;).
> > 
> > As explained in the comments above the code you talk about starting with
> > gcc 4.0, the code generators should output CPU specific instructions for
> > unaligned data with the code used in that path.
> 
> I see. So I take then that older versions of GCC, say. 3.3 and 3.4,
> explicitly do *not* do that? (The code comment in endian.h did not clearly
> convey that message to me, hence my question).
> 

Hopefully that was clarified by the old -devel thread about this code you 
linked in another mail of yours. I am not exactly sure, but how I remember it 
is that only 4.0 and higher output the correct bytecode with the code here.

> > Regarding 2: It seems that this means we should tweak endian.h, switching
> > the "#if !defined(SCUMM_NEED_ALIGNMENT)" block starting in line 155 with
> > the "#elif defined(__GNUC__) && (__GNUC__ >= 4)" block starting in line
> > 175. Correct? If people agree, I'll do that in my branch.
> 
> This way, at least for GCC 4.x, we should get both correct and optimal
> results everywhere, no? And we could in addition use the code GCC 4.x code
> for GCC 3.3+ when SCUMM_NEED_ALIGNMENT is *not* set, to avoid hypothetical
> aliasing issues there, couldn't we?
> 

Yes I think that's correct. We just need to be sure we don't use it on pre GCC 
4.0 in the SCUMM_NEED_ALIGNMENT case :-).

I am not sure whether we still support gcc 2.95, but I read it didn't have any 
strict aliasing handling, so it should be unproblematic here.

> > In fact our READ_FOO/WRITE_FOO inline functions might be unproblemtatic
> > in some cases:
> > 
> > - They are used on character "arrays" (thus uint8 * etc.), since a
> > character type is allowed to alias every other type
> > - They are used on the respective uint16/int16 type in case of
> > READ_UINT16 or uint32/int32 type in case of READ_UINT32
> 
> Sure, in most cases it'll be safe. I am more worried about the handful,
> hard to locate rare cases where it is not, and where only some GCC
> versions on some archs produce code that will fail in some cases. :)
> 

Maybe. Maybe even the WinCE bug we saw, which I "fixed" by disabling some 
special option included in -O3 was caused by aliasing? I might try that 
later...

On the other hand it's funny to know that even the aliasing issue example in 
the gcc docs (http://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Type-
Attributes.html#Type-Attributes) does not procude incorrect code for me with 
gcc 4.6 on amd64 nor with Intel's C compiler... At least I do not get the 
abort which *might*/*should* be triggered.

> > In any case it might be safer to just not enforce ANSI aliasing rules. So
> > I have no problem passing -fno-strict-aliasing for g++.
> 
> Well, maybe using "clever" READ_UINT*" implementations means we don't need
> to do that... on the other hand, I have seen quite some places in our
> engines were we cast a pointer to a struct pointer...
> 

Yeah, all that could be possible aliasing issues. Mostly they just cause 
issues in case you write to the location where you read from in exactly the 
same function (or an inlined function in that function!), which is exactly the 
same problem Yotam had in SWORD2 IIRC.

Anyway on a related matter does the MSVC analysis build warn about aliasing 
issues? If so it might be handy to track down possible issues.

Julien any ideas here? :-)

> PS: We (esp. Eugene, he's much better at this than me!) actually regularly
> moderate emails on -devel, probably far more often than you are aware of
> :).

Good to know.

// Johannes