m-labs / migen

A Python toolbox for building complex digital hardware
https://m-labs.hk/migen
Other
1.21k stars 209 forks source link

Python 3.6 support #62

Closed sbourdeauducq closed 6 years ago

sbourdeauducq commented 7 years ago

https://bugs.python.org/issue27213 breaks fhdl.tracer.

povauboin commented 7 years ago

Hi, I was able to fix it this way: https://github.com/povauboin/litex/commit/699299b5367af8a0675d6f2b808e2be80c2eedb4

sbourdeauducq commented 7 years ago

There are also issues with CALL_FUNCTION_EX (e.g. with CSR naming in MiSoC).

mithro commented 7 years ago

Conda is now defaulting to Python 3.5, I had to use a pin using the following to prevent it getting auto-upgraded;

echo "python ==3.5.4" > build/conda/conda-meta/pinned # Make sure it stays at version 3.5
mithro commented 7 years ago

@sbourdeauducq What is the current status of migen on Python 3.6? Is @povauboin 's fix suitable?

sbourdeauducq commented 7 years ago

Needs cleanup and review.

FelixVi commented 6 years ago

I used the patch under Cygwin(2.882, 64-bit) with Python 3.6.3-1 and the patch seems to work fine. Got a working bios on the papilio_pro target.

sbourdeauducq commented 6 years ago

Can you have a look into what the problem is exactly, also check that it doesn't break 3.5, and clean up the code?

hdante commented 6 years ago

Hello, I'm not sure if it's related, but auto-generated verilog names are not working with python 3.6.3. I'm using github HEAD branch.

cr1901 commented 6 years ago

check that it doesn't break 3.5

@FelixVi confirmed the patch breaks 3.5 today. Perhaps have a version guard until 3.6 is the minimum required version?

sbourdeauducq commented 6 years ago

I suspect this can be written in a way that is compatible with both versions. Please understand what the patch does, clean it up, and look into that.

cr1901 commented 6 years ago

I suspect this can be written in a way that is compatible with both versions. Please understand what the patch does, clean it up, and look into that.

In Python 3.6, all opcodes are now two bytes according to the dis module documentation. So any index increments that were changed in the patch are correct and need to be version-gated unless I'm missing something.

That only leaves line 10 of the patch, which will also need to be version-gated. But before I talk about that, I'd like to know:

As the tracer code is implemented for Python 3.5, why do you only allow name extraction if the call opcode was CALL_FUNCTION or CALL_FUNCTION_VAR (i.e. disallow functions with **kwargs, but allow functions using *args)?

sbourdeauducq commented 6 years ago

That's an oversight. It should support CALL_FUNCTION, CALL_FUNCTION_VAR, CALL_FUNCTION_KW and CALL_FUNCTION_VAR_KW.