Skip to content

Recompute layout until symbol values converge#944

Open
parth-07 wants to merge 1 commit intoqualcomm:mainfrom
parth-07:ReiterateLayoutStep
Open

Recompute layout until symbol values converge#944
parth-07 wants to merge 1 commit intoqualcomm:mainfrom
parth-07:ReiterateLayoutStep

Conversation

@parth-07
Copy link
Contributor

This commit modifies the layout step to recompute layout until the symbol values converge. The reiteration has an upper-limit of 4. On reaching this limit, a note is emitted and layout reiteration is stopped.

Please note that this reiteration limit is for layout iterations before relaxations take place. After a layout relaxation pass, the reiteration count is reset to 0.

Resolves #943

This commit modifies the layout step to recompute layout until the
symbol values converge. The reiteration has an upper-limit of 4. On
reaching this limit, a note is emitted and layout reiteration is
stopped.

Please note that this reiteration limit is for layout iterations
before relaxations take place. After a layout relaxation pass,
the reiteration count is reset to 0.

Resolves qualcomm#943

Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
const llvm::StringMap<ResolveInfo *> &Globals =
m_Module.getNamePool().getGlobals();
for (const auto &Entry : Globals) {
const ResolveInfo *RI = Entry.second;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not be comparing the symbol values of global symbols, as you can clearly see that local symbols are missing.

Can we check for fragment address instead ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it. I believe that just comparing the linker script symbols is enough. If all the linker script symbols and the output section address converge, then all the section-relative symbols will also converge. Please let me know your thoughts on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting observation. Would this capture bumping alignment using dot inside output sections ?

m_Module.getNamePool().getGlobals();
for (const auto &Entry : Globals) {
const ResolveInfo *RI = Entry.second;
ASSERT(RI, "Must not be null!");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this assert needed ?

// Capture current layout snapshot keyed by OutputSectionEntry*.
LayoutSnapshot captureLayoutSnapshot() const;
struct DivergenceResult {
const OutputSectionEntry *section = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

section -> output section

continue;
const LDSymbol *Sym = RI->outSymbol();
if (Sym->scriptDefined())
Result.push_back(Sym);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful to show the expression in the diagnostic instead of symbol, that will allow users do diagnose the problem better.

LayoutSnapshot &S,
const std::vector<const LDSymbol *> &ScriptDefinedSymbols) const {
for (const LDSymbol *Sym : ScriptDefinedSymbols)
S.symbolValues.insert({Sym, Sym->value()});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work for provide as well ? because the symbols are not defined

<< S->name() << maxIterations;
} else if (diverged.symbol) {
config().raise(Diag::note_symbol_value_not_converging)
<< diverged.symbol->name() << maxIterations;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print the symbol, section information and the input file/ input linker script

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect symbol value due to forward reference in linker script

2 participants