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

Max Horn max at quendi.de
Mon May 9 12:45:21 CEST 2011


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



> Since the idea behind that code path was 
> back then to use the (probably) more efficient CPU specific instructions it was 
> only limited to alignment requiring CPU types. The aliasing might have been a 
> problem which was overlooked (or not occuring!) for the non-alignment code 
> path.

So, given that we know are aware that aliasing might be an issue, too, any chance you could comment on this part of my email?

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



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


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


> 
>> Anybody know what the status here is with regards to clang, MSVC, ICC, ...
>> ?
>> 
> 
> icpc (Intel's C++ compiler) supports non-ansi-aliasing rules too and it 
> defaults to that. In fact it supports many of the gcc option arguments, thus 
> when we pass -fstrict-aliasing, it also assumes we conform to ANSI's aliasing 
> rules. Since we do not pass -fstrict-aliasing (or I am too blind to see where) 

You are correct, not blind (well, the Android port passes -fstrict-aliasing explicitly. But it doesn't use icpc.)

> all that shouldn't be an issue for icpc, especially since icpc does not 
> enabled -fstrict-aliasing with optimization levels like g++ does.
> 
> Furthermore icc/icpc does not support "__may_alias__", at least it says it's 
> ignored in the tests I made.


Thanks, so I guess for Intel, we don't have to do anything.

Bye,
Max


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





More information about the Scummvm-devel mailing list