crosire / d3d8to9

A D3D8 pseudo-driver which converts API calls and bytecode shaders to equivalent D3D9 ones.
BSD 2-Clause "Simplified" License
892 stars 80 forks source link

Support large addresses? #3

Closed CookiePLMonster closed 7 years ago

CookiePLMonster commented 7 years ago

At the moment code uses the highest bit in vertex shader handle/pointer to identify whether it's a shader - this makes the code incompatible with large addresses. Do you think bottom 1 or 2 bits could be used for this instead (with some handle shifting if the entity is not a pointer). It should be pretty safe to assume these allocations are 4 byte aligned.

crosire commented 7 years ago

The bottom 16 bits are reserved for the FVF codes that can be passed into the SetVertexShader method. That's why I use the highest bit.

CookiePLMonster commented 7 years ago

I see... what about using the top bit AND shr'ing the pointer by 1 (thus assuming at-least-2-byte alignment on pointer which can be assumed safely) then? Maybe large addresses could be supported this way.

crosire commented 7 years ago

"shr'ing"? Shifting?

CookiePLMonster commented 7 years ago

SHifting to Right, yes. Since the addresses are at least 4 byte aligned bottom bit can safely be discarded, so the top bit can be reused for tagging.

crosire commented 7 years ago

Yes, sounds reasonable. To be 100% safe vertex_shader_info could be declared like this: struct alignas(4) vertex_shader_info { ... }; or struct __declspec(align(4)) vertex_shader_info { ... }; for compatibility with older compilers.

CookiePLMonster commented 7 years ago

malloc (which new should use internally) is guaranteed to be aligned to 8 bytes on x86 platforms (and 16 bytes on x64). Moreover, malloc/new do not respect __declspec(align) and probably alignas too, so you'd need to use _aligned_malloc for that.

I'd leave it as is and assume 8b alignment is real. After all, only one byte is needed so for it to break you would need to get a pointer which isn't even an even number!

EDIT: If in doubt, maybe put assert((mem & 1) == 0)?

crosire commented 7 years ago

Well, alignas technically does work with new unless the type is over-aligned, which since we are on x86 won't happen as allocations align to 8 bytes (alignof(std::max_align_t)), as you mentioned. But yeah, could also do static_assert(alignof(std::max_align_t) >= 4, "Well, shit") or nothing at all. Nothing at all is probably enough here.

CookiePLMonster commented 7 years ago

You are right about alignas, true. And since we only need a single bit the assert could even be >= 2 - which makes it even less likely it's ever an issue.

I'd say opt for nothing and maybe just include this assumption in a comment above the code doing the bit shifting witchcraft.

CookiePLMonster commented 7 years ago

See #8