-
Notifications
You must be signed in to change notification settings - Fork 40
Fix evaluation of expressions containing forward references #272
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
base: main
Are you sure you want to change the base?
Conversation
afeb375 to
cfedfbb
Compare
|
Please add the following
This patch needs to make sure that
|
0ae5bb5 to
1154cc0
Compare
|
Thank you for the suggestions and insights.
I have added the pending-assignments trace.
I will update map-file in a follow-up patch. I am concerned that this patch is already doing a lot of things.
I will add the documentation along with the map-file patch.
I will make this change in a follow-up patch. I do not want to add too much functionality change in a single PR as that can make it difficult to do testing.
I will add more tests for these as part of this pull-request by tomorrow. |
b913a71 to
079b657
Compare
This commit fixes evaluation of expressions containing forward
references. At a high-level, eld evaluates assignments in sequence
and consequently the expressions containing forward references needs to
be reevaluated when the forward reference value is computed for the correct
computation results. For example:
u = v1; // Assignment 1
v1 = v2; // Assignment 2
v2 = 0xa; // Assignment 3
eld evaluates the assignments in order, that is, the evaluation happens
as: [Assignment 1, Assignment 2, Assignment 3]. If we follow this
evaluation order, the symbols `u` and `v1` will have incorrect values
because the value of `v2` is unknown when the assignments of `u` and
`v1` are evaluated.
This commit fixes evaluation of expressions containing forward
references by making the below two changes:
1) Assignments which cannot be completely evaluated during the
sequential evaluation of expressions are marked as pending
assignments. After the layout is done, but before the relaxation,
these pending assignments are recursively reevaluated until there
is no improvement in pending assignments or a MaxIteration limit is
reached.
2) During a layout iteration, assignments may get evaluated multiple times
as layout needs to reset itself based on a few conditions
(new segment needs to be created, and so on...). It is important to reset
the assignments and the symbol values if a layout gets reset. Let's
see why it is important with the help of an example:
SECTIONS {
FOO : {
u = v; // Assignment 1
v = 0xa; // Assignment 2
*(.text.foo)
}
BAR : {
v = 0xb; // Assignment 3
}
v = 0xc; // Assignment 4
}
The sequential assignment evaluation order is: [A1, A2, A3, A4]. When
A1 is evaluated, `v` is not defined, hence we mark A1 as pending
assignment. However, if the layout gets reset after evaluating A2,
then A1 will be evaluated again, but this time, `v` is defined (from
assignment 2 evaluation) and thus A1 can be completely evaluated.
This is wrong because A1 should get the value from the assignment 4
instead of assignment 2!
We fix this issue by resetting the assignments and the symbol values
whenever a layout is reset.
The same issue happens when the layout needs to be recomputed after a
relaxation pass. And the same solution of resetting the assignments and
the symbol values works for this case as well.
This commit also adds a new trace category 'pending-assignments' for
tracing pending assignment evaluation.
Closes qualcomm#169
Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
079b657 to
9edfc82
Compare
This commit fixes evaluation of expressions containing forward
references. At a high-level, eld evaluates assignments in sequence
and consequently the expressions containing forward references needs to
be reevaluated when the forward reference value is computed for the correct
computation results. For example:
eld evaluates the assignments in order, that is, the evaluation happens
as: [Assignment 1, Assignment 2, Assignment 3]. If we follow this
evaluation order, the symbols
uandv1will have incorrect valuesbecause the value of
v2is unknown when the assignments ofuandv1are evaluated.This commit fixes evaluation of expressions containing forward
references by making the below two changes:
Assignments which cannot be completely evaluated during the
sequential evaluation of expressions are marked as pending
assignments. After the layout is done, but before the relaxation,
these pending assignments are recursively reevaluated until there
is no improvement in pending assignments or a MaxIteration limit is
reached.
During a layout iteration, assignments may get evaluated multiple times
as layout needs to reset itself based on a few conditions
(new segment needs to be created, and so on...). It is important to reset
the assignments and the symbol values if a layout gets reset. Let's
see why it is important with the help of an example:
The sequential assignment evaluation order is: [A1, A2, A3, A4]. When
A1 is evaluated,
vis not defined, hence we mark A1 as pendingassignment. However, if the layout gets reset after evaluating A2,
then A1 will be evaluated again, but this time,
vis defined (fromassignment 2 evaluation) and thus A1 can be completely evaluated.
This is wrong because A1 should get the value from the assignment 4
instead of assignment 2!
We fix this issue by resetting the assignments and the symbol values
whenever a layout is reset.
The same issue happens when the layout needs to be recomputed after a
relaxation pass. And the same solution of resetting the assignments and
the symbol values works for this case as well.
This commit also adds a new trace category 'pending-assignments' for
tracing pending assignment evaluation.
Closes #169