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

Initial RAM optimization for ESP8266 #1233

Merged
merged 1 commit into from Aug 3, 2016

Conversation

Projects
None yet
3 participants
@slaff
Contributor

slaff commented Aug 2, 2016

by putting big constants into ROM, instead of residing in RAM.
Related to #1224.

JerryScript-DCO-1.0-Signed-off-by: Slavey Karadzhov slaff@attachix.com

#ifdef CONFIG_STORE_CONST_IN_ROM
#ifndef ICACHE_RODATA_ATTR
#define ICACHE_RODATA_ATTR __attribute__((section(".irom.text")))
#endif

This comment has been minimized.

@LaszloLango

LaszloLango Aug 2, 2016

Contributor

#endif /* !ICACHE_RODATA_ATTR */

#endif
#ifndef STORE_ATTR
#define STORE_ATTR __attribute__((aligned(4)))
#endif

This comment has been minimized.

@LaszloLango

LaszloLango Aug 2, 2016

Contributor

#endif /* !STORE_ATTR */

#define STORE_ATTR __attribute__((aligned(4)))
#endif
#define STORE_IN_ROM ICACHE_RODATA_ATTR STORE_ATTR
#else

This comment has been minimized.

@LaszloLango

LaszloLango Aug 2, 2016

Contributor

#else /* !CONFIG_STORE_CONST_IN_ROM */

#define STORE_IN_ROM ICACHE_RODATA_ATTR STORE_ATTR
#else
#define STORE_IN_ROM
#endif

This comment has been minimized.

@LaszloLango

LaszloLango Aug 2, 2016

Contributor

#endif /* CONFIG_STORE_CONST_IN_ROM */

@@ -184,7 +184,7 @@ vm_op_set_value (ecma_value_t object, /**< base object */
/**
* Decode table for both opcodes and extended opcodes.
*/
static const uint16_t vm_decode_table[] =
static const uint16_t vm_decode_table[] STORE_IN_ROM =

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor

Just a question: does this align only the table, or each item to 32 bytes?

This comment has been minimized.

@slaff

slaff Aug 2, 2016

Contributor

I would expect that just the table is aligned. If there is a way to check this I can create a small test and tell you for sure.

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor
short table[4] STORE_IN_ROM = { 1,2,3,4 }
in main() ->
  printf("%d\n", (int)sizeof(table)) // eight or sixteen?

This comment has been minimized.

@slaff

slaff Aug 2, 2016

Contributor

printf("%d\n", (int)sizeof(table)) // eight or sixteen?

@zherczeg Eight.

@slaff slaff force-pushed the slaff:feature/memory-optimization branch 2 times, most recently from 350a0dd to 5ab9da0 Aug 2, 2016

@LaszloLango

This comment has been minimized.

Contributor

LaszloLango commented Aug 2, 2016

LGTM

@@ -166,6 +166,15 @@
#endif /* CONFIG_ECMA_COMPACT_PROFILE */
/**
* Normally your compiler stores const(ant)s in ROM. Thus saving RAM.

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor

Normally compilers store...

@@ -40,6 +40,18 @@
# define __attr_pure___ __attribute__((pure))
#endif /* !__attr_pure___ */
#ifdef CONFIG_STORE_CONST_IN_ROM
#ifndef ICACHE_RODATA_ATTR
#define ICACHE_RODATA_ATTR __attribute__((section(".irom.text")))

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor

This attribute seems ESP specific. It would be great if this would be defined by the build system.

This comment has been minimized.

@slaff

slaff Aug 2, 2016

Contributor

Are there any documents describing how to do this?

This comment has been minimized.

@zherczeg

zherczeg Aug 3, 2016

Contributor

I don't know how ESP build system works, but in general you pass -DDEF_NAME=value to the compiler

This comment has been minimized.

@zherczeg

zherczeg Aug 3, 2016

Contributor

Actually the whole mechanism should go to the build system:

CFLAGS+=-DJERRY_CONST_DATA="__attribute__((aligned(4))) __attribute__((section(".irom.text")))"

And jrt should only check whether JERRY_CONST_DATA is defined. If not, it defines the macro as an empty string. The change in the config.h could also be removed in this case.

@@ -40,6 +40,18 @@
# define __attr_pure___ __attribute__((pure))
#endif /* !__attr_pure___ */
#ifdef CONFIG_STORE_CONST_IN_ROM
#ifndef ICACHE_RODATA_ATTR

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor

Defines in this file prefixed by JERRY_

#define STORE_IN_ROM ICACHE_RODATA_ATTR STORE_ATTR
#else /* !CONFIG_STORE_CONST_IN_ROM */
#define STORE_IN_ROM
#endif /* CONFIG_STORE_CONST_IN_ROM */

This comment has been minimized.

@zherczeg

zherczeg Aug 2, 2016

Contributor

I would prefer a JERRY_CONST_DATA define. Some systems might want to use it for other purposes than moving things into ROM.

@slaff slaff force-pushed the slaff:feature/memory-optimization branch 2 times, most recently from 21a0dcd to a3b215d Aug 2, 2016

ifneq ($(MFORCE32),)
# If your compiler supports the -mforce-l32 flag then
# uncomment the lines below to gain additional Kb free RAM
ESP_CFLAGS += -DCONFIG_STORE_CONST_IN_ROM

This comment has been minimized.

@zherczeg

zherczeg Aug 3, 2016

Contributor

The define could be added here :)

@slaff slaff force-pushed the slaff:feature/memory-optimization branch from a3b215d to 1430be7 Aug 3, 2016

@slaff

This comment has been minimized.

Contributor

slaff commented Aug 3, 2016

@zherczeg I added the recommended changes.

ifneq ($(MFORCE32),)
# Your compiler supports the -mforce-l32 flag then
# contants can be placed in ROM to free additional RAM
ESP_CFLAGS += -DJERRY_CONST_DATA="__attribute__((aligned(4))) __attribute__((section(".irom.text")))"

This comment has been minimized.

@LaszloLango

LaszloLango Aug 3, 2016

Contributor

I'm not an expert of GNU make, but the double quote usage is a bit odd to me here. Are you sure this will work as expected? Shouldn't be escaped the quotes around .irom.text?

This comment has been minimized.

@zherczeg

zherczeg Aug 3, 2016

Contributor

You can change them to "

@@ -40,6 +40,10 @@
# define __attr_pure___ __attribute__((pure))
#endif /* !__attr_pure___ */
#ifndef JERRY_CONST_DATA

This comment has been minimized.

@LaszloLango

LaszloLango Aug 3, 2016

Contributor

Some comment would be great, like in the previous version of this PR :)

@zherczeg

This comment has been minimized.

Contributor

zherczeg commented Aug 3, 2016

I like this patch much better. LGTM after Laszlo's comments are fixed.

@slaff slaff force-pushed the slaff:feature/memory-optimization branch from 1430be7 to 82bba0d Aug 3, 2016

Slavey Karadzhov
Initial RAM optimization for ESP8266
by putting big constants into ROM, instead of residing in RAM.
Related to #1224.

JerryScript-DCO-1.0-Signed-off-by: Slavey Karadzhov slaff@attachix.com

@slaff slaff force-pushed the slaff:feature/memory-optimization branch from 82bba0d to ce8abfb Aug 3, 2016

@zherczeg

This comment has been minimized.

Contributor

zherczeg commented Aug 3, 2016

Still LGTM

@LaszloLango

This comment has been minimized.

Contributor

LaszloLango commented Aug 3, 2016

@slaff, thanks for the fixes. LGTM

@LaszloLango LaszloLango merged commit ce8abfb into jerryscript-project:master Aug 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment