-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 Linear ASM compiler #13989
base: master
Are you sure you want to change the base?
Add Linear ASM compiler #13989
Conversation
dcaaab8
to
5c74747
Compare
Now it works for me 😄 |
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 deserves a release snippet, otherwise this all looks good to me.
What is a release snippet? BTW, I use name Currently, only c6000 support linear assemble |
A release snippet is a small markdown description of the feature that will be put into the release notes when we cut 1.7.0, they go in |
for_machine: 'MachineChoice', info: 'MachineInfo', | ||
linker: T.Optional['DynamicLinker'] = None, |
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.
Avoid quoting type annotations in newly introduced code. It is an older style and unnecessary since we now require a minimum python >=3.7 and started using from __future__ import annotations
.
def get_crt_compile_args(self, crt_val: str, buildtype: str) -> T.List[str]: | ||
return [] | ||
|
||
def sanity_check(self, work_dir: str, environment: 'Environment') -> None: |
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.
Same here.
@@ -1353,6 +1354,26 @@ def detect_masm_compiler(env: 'Environment', for_machine: MachineChoice) -> Comp | |||
_handle_exceptions(popen_exceptions, [comp]) | |||
raise EnvironmentException('Unreachable code (exception to make mypy happy)') | |||
|
|||
def detect_linearasm_compiler(env: 'Environment', for_machine: MachineChoice) -> Compiler: |
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.
Same here.
comp = ['cl6x'] | ||
comp_class: T.Type[Compiler] = TILinearAsmCompiler | ||
arg = '-h' | ||
info = env.machines[for_machine] | ||
cc = detect_c_compiler(env, for_machine) | ||
is_cross = env.is_cross_build(for_machine) | ||
|
||
popen_exceptions: T.Dict[str, Exception] = {} | ||
try: | ||
output = Popen_safe(comp + [arg])[2] |
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.
We are hardcoding the compiler as cl6x
instead of looking it up via the machine info. This seems suboptimal...
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.
On the other hand, it looks like masm handling already does that too. @dcbaker what do you think?
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.
Sigh... We need to fix that.
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.
How to fix that?
Fix #13670
PS: