Page MenuHomePhabricator

[llvm-dwp] Refactor handling of section identifiers. NFCI.
ClosedPublic

Authored by ikudrin on Mar 12 2020, 7:57 AM.

Details

Summary

There is a number of places in llvm-dwp.cpp where a section identifier is translated into an index of an internal array of section contributions, and another place where the index is converted to an on-disk value. All these places use direct expressions like <id> - DW_SECT_INFO or <index> + DW_SECT_INFO, exploiting the fact that DW_SECT_INFO is the minimum valid value of that kind.

The patch adds distinct functions for that translation. The goal is to make the code more readable and to prepare it to support index sections of new versions, where the numeric scheme of section indexes is changed.

Diff Detail

Event Timeline

ikudrin created this revision.Mar 12 2020, 7:57 AM

I don't /mind/ this refactor (though the SectionKindToIndex function should use DW_SECT_INFO rather than the magic number 1) - but I'm not sure it's needed/especially useful given my position (which aligns with @labath's) on the original review D75929. So let's shelve this while we continue the design discussion there & see how it shakes out.

ikudrin updated this revision to Diff 253859.Mar 31 2020, 6:57 AM
  • Use a named constant DW_SECT_INFO in the translation function instead of a numeric constant 1.
  • Rename the functions.

It seems like we generally consent about the way proposed in D75929. I am updating this patch because other changes slightly depend on it.

dblaikie accepted this revision.Apr 5 2020, 4:42 PM
dblaikie added inline comments.
llvm/tools/llvm-dwp/llvm-dwp.cpp
262–263

LLVM doesn't generally use top level const on local variables - drop the "const" here.

This revision is now accepted and ready to land.Apr 5 2020, 4:42 PM
This revision was automatically updated to reflect the committed changes.