Skip to contentSkip to conversation

package/micropython: Define MICROPY_NLR_SETJMP for xtensa

PATCH New Chris Packham 4 participants 5 messages Mar 29, 2021 – Apr 4, 2021

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/micropython: Define MICROPY_NLR_SETJMP for xtensa

As suggested on https://github.com/micropython/micropython/issues/6551
define MICROPY_NLR_SETJMP to avoid the xtensa specific implementation of nlr_push.

Fixes:
- http://autobuild.buildroot.net/results/5fc8669b5c768ccfc02bd20d1159bce7fe43683e

---
 package/micropython/micropython.mk | 5 +++++
 1 file changed, 5 insertions(+)
package/micropython/micropython.mk +5 -0
index 80d086f3c8..fa28da9be3 100644
@@ -21,6 +21,11 @@ ifeq ($(BR2_i386)$(BR2_x86_64)$(BR2_arm)$(BR2_armeb),)
 MICROPYTHON_CFLAGS = -DMICROPY_GCREGS_SETJMP=1
 endif
 
+# xtensa has problems with nlr_push, use setjmp based implementation instead
+ifeq ($(BR2_xtensa),y)
+MICROPYTHON_CFLAGS = -DMICROPY_NLR_SETJMP=1
+endif
+
 # When building from a tarball we don't have some of the dependencies that are in
 # the git repository as submodules
 MICROPYTHON_MAKE_OPTS += \
Signature · 1 line
2.31.0
#1 lore raw

PATCH 1/1 package/micropython: Define MICROPY_NLR_SETJMP for xtensa

Quoted 11 lines · On Mon, 29 Mar 2021 20:28:28 +1300 Chris Packham <[email protected]>…

As suggested on https://github.com/micropython/micropython/issues/6551
define MICROPY_NLR_SETJMP to avoid the xtensa specific implementation of nlr_push.

Fixes:
- http://autobuild.buildroot.net/results/5fc8669b5c768ccfc02bd20d1159bce7fe43683e

---
 package/micropython/micropython.mk | 5 +++++
 1 file changed, 5 insertions(+)

The build error really looks like a toolchain issue, but fair enough as a temporary measure what you proposed is fine, so I've applied.

Could you perhaps e-mail the issue to Max Filippov <[email protected]> ? He has been maintaining the Xtensa support in BR, including toolchain fixes, for quite some time.

Thanks a lot!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#2 lore raw

PATCH 1/1 package/micropython: Define MICROPY_NLR_SETJMP for xtensa

Quoted 16 lines · Thomas Petazzoni
Quoted 11 lines · On Mon, 29 Mar 2021 20:28:28 +1300 Chris Packham <[email protected]>…

As suggested on https://github.com/micropython/micropython/issues/6551
define MICROPY_NLR_SETJMP to avoid the xtensa specific implementation of nlr_push.

Fixes:
- http://autobuild.buildroot.net/results/5fc8669b5c768ccfc02bd20d1159bce7fe43683e

---
 package/micropython/micropython.mk | 5 +++++
 1 file changed, 5 insertions(+)
Quoted 6 lines

The build error really looks like a toolchain issue, but fair enough as a temporary measure what you proposed is fine, so I've applied.

Could you perhaps e-mail the issue to Max Filippov <[email protected]> ? He has been maintaining the Xtensa support in BR, including toolchain fixes, for quite some time.

I believe Max is already on the issue I raised upstream[1].

I don't really know anything about xtensa but I suspect there might be a difference between building for bare-metal vs building for an OS. Either that or the xtensa specific code is just broken and no-one else has noticed.

[1] - https://github.com/micropython/micropython/issues/6551

Quoted 8 lines

Thanks a lot!

Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

#3 lore raw

PATCH 1/1 package/micropython: Define MICROPY_NLR_SETJMP for xtensa

Chris, Thomas,

Quoted 12 lines · Chris Packham
Quoted 10 lines · Thomas Petazzoni
Quoted 7 lines · On Mon, 29 Mar 2021 20:28:28 +1300 Chris Packham <[email protected]>…

As suggested on https://github.com/micropython/micropython/issues/6551
define MICROPY_NLR_SETJMP to avoid the xtensa specific implementation of nlr_push.

Fixes:
- http://autobuild.buildroot.net/results/5fc8669b5c768ccfc02bd20d1159bce7fe43683e

Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
---
 package/micropython/micropython.mk | 5 +++++
 1 file changed, 5 insertions(+)

The build error really looks like a toolchain issue, but fair enough as a temporary measure what you proposed is fine, so I've applied.

I'd say that this is an assembly coding issue. The compiler would not generate such code and the linker has no support for connecting long jumps on its own.

Quoted 1 line

I believe Max is already on the issue I raised upstream[1].

Yeah, there are additional issues. I'll try to see whether proposed solution actually works (vs just fixing the build) and what can be done to the failing assembly code.

Signature · 2 lines
Thanks.
-- Max
#4 lore raw

PATCH 1/1 package/micropython: Define MICROPY_NLR_SETJMP for xtensa

In reply to Chris Packham
Quoted 10 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Chris" == Chris Packham <[email protected]> writes:

As suggested on https://github.com/micropython/micropython/issues/6551
define MICROPY_NLR_SETJMP to avoid the xtensa specific implementation of nlr_push.

Fixes:
- http://autobuild.buildroot.net/results/5fc8669b5c768ccfc02bd20d1159bce7fe43683e

Committed to 2020.11.x and 2021.02.x, thanks.

Signature · 1 line
Bye, Peter Korsgaard