[M68K] Made instruction table static #1408
Conversation
|
in all other archs, i use extension .inc for data files like your M68KInstructionTable.h. do you want to follow the same name scheme? |
|
regarding file arch/M68K/M68KInstructionTable.c, it is not compiled in the core, so we better move it elsewhere. one place is contrib/, you may move it there, put it under its own directory, and have Makefile for it at the same time. |
|
Sure I can change the extension and move the file. Having a Makefile would be complex as it would need to support all possible compilers and pretty much no one will use this file. |
|
i suppose that the Makefile only needs to include that line you put at the top of M68KInstructionTblGen.c, no? why do we need to support all compilers? |
|
I was thinking if the Makefile needed to support msvc etc |
|
oh no, this is only for developer/maintainer who needs to take care the code. there is no need to support MSVC, but only Linux/Mac is enough. |
|
Ok cool! Thanks |
|
merged, thanks! |
|
should we merge this into "v4" branch? |
I would say yes. This only changes the internals (not affect on public interface) and fixes a bug, since according to https://www.capstone-engine.org/:
Also, what's the difference between v4, master, and next branches? I'm a little confused by #1319. Thanks! |
Also, should there be a policy where we make bug PRs against the "next" branch first then cherry-pick into "master" (or whichever branch is appropriate)? This would prevent us from "missing" bug fixes in next and all future branches. It looks like that is what we are doing here, but it may make sense to have that written in some "contributing" document. |
|
"v4" is a maintenance branch for 4.0.x version. Actually we dont need to have "v4", but just "master" branch should play a role of stable & maintenance branch. But we discovered that "master" break compatibility of v4.0, i had to create "v4" branch for 4.0.x, which (soon will) revert the related commit to restore compatibility. So far development for the v5 is in the "next" branch. People still do pull req for "master", which is pretty compatible with "next", and i cherry-pick for "next" manually. When v5 is ready for release, we will merge "next" to "master". Perhaps it will help if we make "v4" the default branch, to make it less confused for now? |
|
If a pull req is compatible with "v4", i still cherry-pick from "master" & "next". And vice versa. |
|
already cherry-picked this for "v4" |
This solves the data race in #1397 by having the instruction table created upfront instead of the first time when trying to use the code which would generate a race condition in case of multiple threads trying to use the code at the same time.
The generation code has now been moved into to a separate tool called
M68KInstructionTblGen.cI chose to keep the code still in C as if for some reason we want to generate the table in a dynamic fashion in the future is easier to move over the code to C again. Also I expect very few people will need to use this tool anyway.