Page MenuHomePhabricator

[MC][ARM] Don't create multiple .ARM.exidx associated to one .text
ClosedPublic

Authored by MaskRay on Feb 24 2020, 6:01 PM.

Details

Summary

Fixed an issue exposed by D74006.

In clang cc1as, MCContext::UseNamesOnTempLabels is true.
When parsing a .fnstart directive, FnStart gets redefined to a temporary symbol of a different name (.Ltmp0, .Ltmp1, ...).
MCContext::getELFSection() called by SwitchToEHSection() will create a different .ARM.exidx each time.

llvm-mc uses Ctx.setUseNamesOnTempLabels(false); and FnStart is unnamed.
MCContext::getELFSection() called by SwitchToEHSection() will reuse the same .ARM.exidx .

Diff Detail

Event Timeline

MaskRay created this revision.Feb 24 2020, 6:01 PM

This can only be tested by clang -c a.s => a.o, which (I think) is generally not recommended...

Should we add an assertion that getELFSection isn't called with an unnamed symbol? Or is there some legitimate use-case I'm missing?

MaskRay updated this revision to Diff 246605.Feb 25 2020, 5:16 PM

Add

assert(!(LinkedToSym && LinkedToSym->getName().empty()));
This revision is now accepted and ready to land.Feb 25 2020, 6:16 PM
This revision was automatically updated to reflect the committed changes.

Hi @MaskRay ,

Quite surprisingly, this patch increases code size by 3-4% on several SPEC CPU2006 benchmarks for arm-linux-gnueabihf at -Oz optimization level:

  1. 473.astar,astar_base.default regressed by 103
  2. 450.soplex,soplex_base.default regressed by 103
  3. 483.xalancbmk,Xalan_base.default regressed by 104

With -Os I see 2-3% code size increase for same benchmarks:

  1. 473.astar,astar_base.default regressed by 102
  2. 450.soplex,soplex_base.default regressed by 102
  3. 483.xalancbmk,Xalan_base.default regressed by 103

Would you please check if the code size increase expected for this patch?

Let me know if you need help reproducing the issue. Thanks!

MaskRay added a comment.EditedMar 16 2020, 9:18 AM

Hi @MaskRay ,

Quite surprisingly, this patch increases code size by 3-4% on several SPEC CPU2006 benchmarks for arm-linux-gnueabihf at -Oz optimization level:

  1. 473.astar,astar_base.default regressed by 103
  2. 450.soplex,soplex_base.default regressed by 103
  3. 483.xalancbmk,Xalan_base.default regressed by 104

    With -Os I see 2-3% code size increase for same benchmarks:
  4. 473.astar,astar_base.default regressed by 102
  5. 450.soplex,soplex_base.default regressed by 102
  6. 483.xalancbmk,Xalan_base.default regressed by 103

    Would you please check if the code size increase expected for this patch?

    Let me know if you need help reproducing the issue. Thanks!

@maxim-kuvyrkov D75095 fixed a misuse and could be applied before D74006. I can't think of how it could increase code size.

Echo the comment from https://reviews.llvm.org/D74006#1920702
Could you try to figure out what's actually different about the object files/executables before/after this patch, using objdump and readelf -S?