Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LTCG Versioning Method #79

Open
RadWolfie opened this issue Jun 4, 2019 · 7 comments
Open

Fix LTCG Versioning Method #79

RadWolfie opened this issue Jun 4, 2019 · 7 comments
Labels
enhancement New feature or request needs-developer-discussion Need developer discussion to come with agreement. OOVPA Any OOVPA change relative task
Milestone

Comments

@RadWolfie
Copy link
Member

RadWolfie commented Jun 4, 2019

Due to D3D8LTCG's OOVPA signatures using two separate versioning which is understandable. However, the signatures with version "1024" appear are not always in 3911's database. Which I had noticed and so are the other versions too.

I suggest a better versioning method to keep the LTCG's database sane and easier to find.

// * About the number of OOVPA
// * 1024 and 1xxx - Cmpatible with known functions probably.
// * 2028 and 2xxx - Remade by Link-time Code Generation, will not work with known functions.

My 2 proposals are:

  • Keep the versioning method, except change 1XXX and 2XXX to use first 3 numbers. Such as 1391 (3911), 1392 (3925), 1403 (4034), and so on.

OR

  • Add "LT(CG)" at the end of the function's symbol for 2XXX only. I have an idea to able separate D3D8 symbols and D3D8LTCG by add D3D8LTCG flag to existing D3D8 database.
    • Doing this way will allow full version (3911) support for both 1XXX (without "LT(CG)") and 2XXX (with "LT(CG)"), I think. I'll need to try it out later.

TASKS:

  • Create a branch from this repository to contain __LTCG prefix and use actual versioning from existing version files.
  • Create a draft pull request to Cxbx-Reloaded, or use XbSymbolDatabaseTool from here, for any regression may be found.
    • This will be the point we will need to rely on testers with LTCG titles response back with before and after cache/log file.
@RadWolfie RadWolfie added enhancement New feature or request OOVPA Any OOVPA change relative needs-developer-discussion Need developer discussion to come with agreement. task labels Jun 4, 2019
@JayFoxRox
Copy link
Contributor

JayFoxRox commented Jun 7, 2019

I've left some comments via Discord. But they didn't find their way into this issue:

Instead of hacks like this 1xxx or 2xxx, just fix it properly, please.
Even kernel version 6000 would only be ~0x1770; so bits 0x2000, 0x4000, 0x8000 remain untouched.

It could become OPTIMIZED | 3911 (instead of 2xxx ~ 2911) or RECOGNIZED | 3911 (instead of 1xxx ~ 1911). So it would add flags / state to the unused bits:

  • 0x0000 [00]: non-LTCG
  • 0x4000 [01]: OPTIMIZED
  • 0x8000 [10]: RECOGNIZED
  • 0xC000 [11]: remains unused

You could also always add 0x2000 as another flag (or fall-back to it, if 0x8000 is already used - I didn't check); 0x2000 either as boolean, or as extension for state number, giving you 8 options when combined with 0x4000 and 0x8000.

It's basically the same as 1xxx / 2xxx (encoding multiple things in one field), except my suggestion is more readable and less error prone (avoids accidental match of last 3 digits).

Obviously, you could also turn these flags into macros: OPTIMIZED(3911) if that's considered more readable.

If the codebase already uses bitmasks in the upper bits and has no room for more flags, the entire encoding could be changed to something like VERSION_3911 (which would refer to an index, instead of a 4 digit number for the first version).

@RadWolfie
Copy link
Member Author

RadWolfie commented Jun 7, 2019

Adding the flags will not help. The symbol cache doesn't have any "flag" support and shouldn't provide support for it anyway. Also, generated symbol strings are output differently for LTCG.

My second proposal is closely identical to your suggestion, @JayFoxRox. Except, it will break Cxbx-Reloaded. Meaning, Cxbx-Reloaded will need an update to match the symbol renames.

@JayFoxRox
Copy link
Contributor

We've discussed this on Discord again.

My proposal wouldn't work, because the Version number is used as a string as part of the macro. So math can't be done:

XbSymbolDatabase/OOVPA.h

Lines 160 to 161 in c89ee86

#define OOVPA_TABLE_ENTRY_FULL(Oovpa, DebugName, Version) \
{ & Oovpa ## _ ## Version.Header, DebugName, Version }

Additionally, the version field might only be 13 bit (although the comment is apparently outdated):

unsigned short Version;// : 13; // 2^13 = 8192, enough to store lowest and higest possible Library Version number in


Since then, @RadWolfie has also clarified that the original post contains 2 different proposals. The second proposal doesn't cause these issues. My understanding is that it moves these markers into the symbol name (which is also a bit dirty, and might have some drawbacks / but also some benefits). It would essentially add the functions as a separate function handler.

@RadWolfie
Copy link
Member Author

RadWolfie commented Jun 7, 2019

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 1024, 1036, 1048, 1060),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_12, 2024),

Convert above to below as like proposal 2

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 3911, 4034, 4627, 5849),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_LTCG_12, 3911),

or something @JayFoxRox suggest to use the LTCG at the end of the symbol.

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 3911, 4034, 4627, 5849),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_12_LTCG, 3911),

Plus he also advise to use double underlines, __ for any numbers/"LTCG" after the mangle symbol name.

@RadWolfie
Copy link
Member Author

Just want to share my discovery when worked on D3D's render state variables pull request #177. I notice there are functions that are exactly the same except shifted by one to five offsets, sometimes more than five. Which is majority affecting LTCG titles. Due to current and future versioning' limitation, I think we should re-haul OOVPA design to something else*. Doing so will cut down redundant signatures that are only shifted one to five offsets into one simple signature. Then future versioning design could work better.

* Something else could be like tuple design which could transform into custom instruction internally. How would it work is a mystery for me. However, this type of work will require backward compatibility removal, aka OOVPA(_NO)_XREF, first.

@jarupxx
Copy link
Contributor

jarupxx commented Jan 28, 2023

Similar to RadWolfie's first proposals, but adding a minor number. It also avoids conflicts between D3D and D3D8LTCG 1xxx. Such as

D3DDevice_DrawIndexedVerticesUP, 3911
D3DDevice_DrawIndexedVerticesUP, 1024
D3DDevice_SelectVertexShader_0, 2072
D3DDevice_SelectVertexShader_0, 2084
D3DDevice_DrawIndexedVerticesUP, 3911
D3DDevice_DrawIndexedVerticesUP, 3911.1
D3DDevice_SelectVertexShader_0, 5849.1
D3DDevice_SelectVertexShader_0, 5849.2

@RadWolfie
Copy link
Member Author

@jarupxx, I am not seeing how minor number will work with current implementation nor with group of symbols in the database list.

@RadWolfie RadWolfie added this to the 4.0 milestone Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-developer-discussion Need developer discussion to come with agreement. OOVPA Any OOVPA change relative task
Projects
None yet
Development

No branches or pull requests

3 participants