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
Add two new build targets to enable the possibility of using clang-cl as an assembler for Windows on Arm #19523
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall a good effort, thank you. I do have a few smaller nits, though
Configurations/50-win-onecore.conf
Outdated
@@ -82,7 +82,35 @@ my %targets = ( | |||
ex_libs => "onecore.lib", | |||
multilib => "-arm64", | |||
}, | |||
|
|||
"VC-WIN64-CLANGASM-ARM" => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather have your definitions in a separate file, say Configurations/50-win-clang-cl.conf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way to look at the whole __ASSEMBLER__
thing
Configurations/windows-makefile.tmpl
Outdated
return <<"EOF"; | ||
if(($config{CC} !~ /clang-cl/) and ($config{AS} =~ /clang-cl/)) | ||
{ | ||
return <<"EOF"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessary, as the added /D__ASSEMBLER__
can be used with cl.exe too, without harm.
Configurations/windows-makefile.tmpl
Outdated
} else { | ||
return <<"EOF"; | ||
$target: "$gen0" $deps | ||
cmd /C "set "ASM=\$(AS)" & $generator \$@.S" | ||
\$(CPP) $incs $cppflags $defs \$@.S > \$@.i | ||
move /Y \$@.i \$@ | ||
del /Q \$@.S | ||
EOF | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can also be dropped
Configurations/windows-makefile.tmpl
Outdated
$target: "$gen0" $deps | ||
cmd /C "set "ASM=\$(AS)" & $generator \$@.S" | ||
\$(CPP) $incs $cppflags $defs \$@.S > \$@.i | ||
\$(CPP) /D__ASSEMBLER__ /E $incs $cppflags $defs \$@.S > \$@.i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop /E
, as it's already present (well, /EP
is...). Also, don't convert the initial TAB to spaces.
6ef59ec
to
3c8d25d
Compare
@levitte Hi! Think I covered all the changes you mentioned, hopefully. Thanks a lot for the review. If you could be so kind to take a look and see what you think while I figure out how to sort the CLA issue. |
@levitte Do you have further comments for this PR please? Thanks a lot for all the comments. |
This needs to be rebased on top of fresh master branch. |
an assembler for Windows on Arm builds and also clang-cl as the compiler as well. Make appropriate changes to armcap source and peralsm scripts.
3c8d25d
to
449bbab
Compare
@t8m Just rebased and retested. Thanks. |
@paulidale still OK? |
This pull request is ready to merge |
Merged to master and 3.1. Thank you. |
an assembler for Windows on Arm builds and also clang-cl as the compiler as well. Make appropriate changes to armcap source and peralsm scripts. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #19523)
an assembler for Windows on Arm builds and also clang-cl as the compiler as well. Make appropriate changes to armcap source and peralsm scripts. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #19523) (cherry picked from commit b863e1e)
an assembler for Windows on Arm builds and also clang-cl as the compiler as well. Make appropriate changes to armcap source and peralsm scripts. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#19523)
Currently it is not possible to use the assembler files for Windows on Arm as there is no NASM for aarch64. This patch makes the appropriate changes to use
clang-cl
as an assembler along with the possibility of also using it as a compiler. Forcibly adding__ASSEMBLER__
was required as cl does not add it by itself.