Add symbols ASSEMBLER, ASMX86, ASMX64 and deprecate CPUASM, PUREPASCAL - #28
Add symbols ASSEMBLER, ASMX86, ASMX64 and deprecate CPUASM, PUREPASCAL#28Delphier wants to merge 1 commit into
Conversation
|
@davidm-ro Hi, does Oxygene have a predefined symbol called |
|
A few comments out the top of my head:
|
|
|
|
For backward compatibility, and to handle future breaking changes (I'm planning to submit another change similar to #27), I've revised the strategy:
|
I avoided using That said, I am willing to defer to @obones and rework my patches if there's an acceptable alternative. |
|
In the JCL codebase, searching for |
|
Sure enough, JclWin32.pas has an unprotected {$IF DECLARED} that Andreas added back in 2014. Given the number of units that use that directly or indirectly, and how long it's been in there, I think that's a solid argument that avoiding |
|
@boramis, @Delphier, you are mixing up the JCL and this JEDI project. You have to understand that this project holds jedi.inc which is NOT used exclusively by the JCL or the JVCL but by lots of other projects and people around the world. So no, stop mixing up those projects and please DO NOT use $IF in jedi.inc |
|
@obones, it might have been better to discuss this in my jcl WinARM64EC merge request, but we’re not talking about using $IF in jedi.inc. We’re talking about the best way to handle it in the Jcl and what changes would be necessary in jedi.inc to support it. The $IF/$ELSEIF/$ELSE blocks @Delphier mentioned in his first post are how the code would look in the JCL where functions have asm and pure pascal implementations. The specific changes to jedi.inc would be adding the X86ASM/X64ASM defines and deprecation of some existing ones. You can see the changes he’s proposing for jedi.inc specifically in the commit. |
|
Just to clarify a few points:
As I mentioned from the very beginning, this is preparatory groundwork to support WinARM64EC — there is a clear reason behind the changes to |
|
We could shift the discussion in a different direction: is there anything wrong with the |
|
To me |
|
The assembly support for the three compilers supported by Jedi is as follows:
Under what circumstances is it still necessary to use Conversely, if we define If a contributor can provide ARM assembly, we would simply need to define an |
|
Once again, you are assuming that the So I'm requiring that |
|
So I'd like to know, does Jedi.inc plan to support the ARM architecture in the future? |
|
Jedi's PUREPASCAL is borrowed from VCL. However, VCL's PUREPASCAL is independently defined and used within each unit — you could say that the meaning of PUREPASCAL in each VCL unit is completely different. It was a mistake from the very beginning. |
|
Furthermore, regarding the breaking change you mentioned — this PR doesn't break anything; the semantics of PUREPASCAL have not changed. On the contrary, using {IFDEF PUREPASCAL} under a compiler that supports ARM would actually introduce breaking changes. So it must be deprecated. |
|
Under FPC, if someone were to start adding ARM inline ASM functions, what would be the structure for doing so? Say they wanted ARM versions of the routines in JclLogic.pas for performance but that Pascal versions (whether covered by |
|
What about just including “PUREPASCAL” as just an informational comment in an $ELSE block? E.g., As long as it was enforced as a coding standard it would still be easy to find them, even if the actually define is non-functional. |
|
@obones Can we please make some progress on this? If I need to completely rework my JCL pull request I’d like it to be when the code is still somewhat fresh in my mind. |
|
I've seen the comment made by @Delphier on the JCL PR and am wondering what your thoughts are bout his suggestions. |
|
Are you referring to @Delphier's comments on my JCL pull request (linked in my previous comment here)? His suggestion was to split the pull request up and to change jedi.inc with this pull request. I'm not sure what you mean by "don't seem to require a change in Yes, we can make it work without changes to jedi.inc, but that would require all ASM blocks in the Jcl units to look like:
We can certainly do that, but it would be nice to have single symbols for That also doesn't incorporate I also still think there's value in the changes I suggested in #27. Delphi defines CPUARM/CPUARM64 for their ARM targets but Free Pascal only defines CPUAARCH64, and Delphi defines To summarize: jedi.inc:
JCL units: @Delphier also suggested deprecating If we can get the changes above in, I'll work on pulling apart my JCL changes into smaller merge requests. |
|
To clearly illustrate the necessity of adding the symbols |
I can see an advantage when looking at suggested modification to BUT - as @obones is saying: the This means:
|
|
@ronaldhoek No one is asking to use |
|
Ok, fair enough, but should this and #27 both be merged, or only one of the two and if yes, which one? |
|
@obones Does this mean you've accepted this patch?
A few open questions remain:
|
|
Well, yes, it seems legit like that.
|
|
Great, I'll update the patch as soon as possible and do a final code review. I have no other questions. |
|
Both #27 and #28 should be merged, since #28 doesn't do anything with the ARM symbols. @Delphier, if you're planning on submitting another related patch, might be best to just combine all three into a single pull request? For I don't think we need to consider |
|
This is preparatory work for WinARM64EC support.
Changes:
1. Use
ASSEMBLERinstead ofCPUASMASSEMBLERis a predefined Delphi symbol with clearer semantics. By contrast, the "CPU" inCPUASMintroduces ambiguity. SinceCPUASMhas never been used anywhere in JCL, this change has no impact on existing code.2. Add new symbols
ASMX86andASMX64The inline assembly code currently in JCL only targets x86 and x64. With these two new symbols, conditional compilation between assembly and pure Pascal can be written as follows:
3. Deprecate
PUREPASCALWith the conditional compilation pattern above, there is no longer any need to define
PUREPASCAL.Additionally,
PUREPASCALis semantically the opposite ofASSEMBLER, and sinceASSEMBLERis always active in FPC,{$IFDEF PUREPASCAL}will never match in FPC.Currently
JclLogic.pasandJclMath.pasmake use ofPUREPASCAL; if this PR is accepted, I will update them accordingly.