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

RTL8195AM - Fix IAR ielftool zero-padding issue #5260

Merged
merged 3 commits into from Oct 23, 2017

Conversation

Projects
None yet
6 participants
@tung7970
Contributor

tung7970 commented Oct 5, 2017

Description

Due to multiple memory regions defined in RTL8195AM's icf file, ielftool does zero-padding between memory regions. This behavior prolongs conversion time and generates huge binary file.

The reason for having multiple memory regions is to do fine-grain control of object files, but this is not necessary for most cases.

This patch set tries to fix zero-padding issue by replacing multiple memory regions with one region with discontinuous segments. The side effect is the placement of object files is up to linker's best-effort.

The conversion time is greatly reduced, measured around 0.061s per conversion.

Related issue: #5241

Status

READY

Migrations

NO

Related PRs

NO

Todos

NO

Deploy notes

NO

Steps to test or reproduce

mbed compile -m REALTEK_RTL8195AM -t IAR

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 5, 2017

How is this fixing the issue? the diff is a bit bigger, and commit does not provide explanation for these changes.

What is the time now (how much in percetage it build now/before) ? What about implications having this fix in, no fine grain control?

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 5, 2017

I have updated description of this PR and will update commit messages.

The first patch is to cleanup ICF related files, no logic change.
The second patch is to adjust memory regions to solve zero padding issues by removing redundant regions, and by merging existing ram regions.

@Archcady Please help review, and if you'd like to take over this PR, please fork from mine and amend with your updates.

@tung7970 tung7970 force-pushed the tung7970:fix-iar branch Oct 5, 2017

@0xc0170

Regarding space, how much are we losing, if any. what other impacts does this have?

define symbol __ICFEDIT_size_heap__ = 0x19000;
/**** End of ICF editor section. ###ICF###*/
define symbol __SRAM_start__ = 0x10007000;

This comment has been minimized.

@0xc0170

0xc0170 Oct 5, 2017

Member

wondering why also name change:

  • it was BD_RAM , now SRAM
  • it was SDRAM, now DRAM

This comment has been minimized.

@tung7970

tung7970 Oct 5, 2017

Contributor

Not losing any space. Removed regions are for ROM and TCM. ROM region is there for linking only, which can be replaced with a symbol file. TCM region can be directly accessed in user program.

Well, I found it very confusing when I was working on RTL8195AM. The BD_RAM is actually a fast SRAM. In GCC's case, it's named SRAM1 and SRAM2. I will check if I can unify naming for all compilers.

This comment has been minimized.

@0xc0170

0xc0170 Oct 5, 2017

Member

👍 I am on the same page, was looking at the linker scripts earlier and was confused. If they get unified , would be +1

@tung7970 tung7970 force-pushed the tung7970:fix-iar branch Oct 6, 2017

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 6, 2017

greentea tests OK.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 6, 2017

@Archcady please review

@studavekar for visibility, we will test this if the time improves

@tung7970 tung7970 force-pushed the tung7970:fix-iar branch Oct 6, 2017

@Archcady

This comment has been minimized.

Contributor

Archcady commented Oct 9, 2017

IAR greentea test finished in 1266.63 sec using command mbed test -t IAR -v -c
mbedgt: completed in 1266.63 sec
To compare the time, I use command mbed test -t ARM -v -c and finish testes in 1503.98 sec.

@tung7970 tung7970 force-pushed the tung7970:fix-iar branch 2 times, most recently Oct 9, 2017

tung7970 added some commits Oct 4, 2017

rtl8195am - cleanup IAR icf file
Cleanup ICF file. No logic change.

Signed-off-by: Tony Wu <tonywu@realtek.com>
rtl8195am - fix IAR zero padding issue
Remove redundant memory regions, and merge multiple RAM regions into one
to solve ielftool zero padding issue.

The side effect is less control over object files placement. It's all up
to linker's best effort.

Signed-off-by: Tony Wu <tonywu@realtek.com>
rtl8195am - remove redundant file
This file is unused and redundant.

Signed-off-by: Tony Wu <tonywu@realtek.com>

@tung7970 tung7970 force-pushed the tung7970:fix-iar branch to a35c76d Oct 11, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 11, 2017

Build : SUCCESS

Build number : 123
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5260/

Triggering tests

/test mbed-os

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 13, 2017

@studavekar Bump for review

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Oct 19, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 19, 2017

/morph build

1 similar comment
@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 19, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 19, 2017

Build : SUCCESS

Build number : 282
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5260/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 20, 2017

Build : SUCCESS

Build number : 291
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5260/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 20, 2017

Build : SUCCESS

Build number : 294
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5260/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci

This comment has been minimized.

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 20, 2017

Build times have are lower with as seen from logs 👍

@theotherjimmy theotherjimmy merged commit d2b7620 into ARMmbed:master Oct 23, 2017

7 checks passed

AWS-CI uVisor Build & Test Success
Details
AWS-CI uvisor Test DIDNT RUN UVISOR TESTS
Details
Cam-CI uvisor Build & Test DIDNT RUN UVISOR TESTS
Details
ci-morph-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
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