Page MenuHomePhabricator

[MC] Handle layout change due to relaxation of another section
AbandonedPublic

Authored by reames on Mar 13 2020, 4:03 PM.

Details

Summary

This is an alternate fix for the test case from https://reviews.llvm.org/D76114.

The (rather fundamental) issue revealed by this test case is that our relaxation implementation was not correctly adjusting the size and offsets of fragments in one section based on changes in size of another if the layout order of the two happened to be such that the former was visited before the later. To trigger this, you need one section which refers to labels in another, but the fragments involved aren't relaxable. (If they were, that would trigger invalidation.) Any of fill, align, or the like can trip this.

An alternate approach would be to make all fragments with expression computable sizes relaxable. This would require either a) storing the size in the fragment, or b) peaking ahead to compute the previously computed size from the offset of the next fragment.

I'm curious, which do folks think is less ugly?

p.s. This is mostly posted for discussion purposes. Jain, you're welcome to take any of this code and incorporate into a refresh on the original review if desired.

Diff Detail

Event Timeline

reames created this revision.Mar 13 2020, 4:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2020, 4:03 PM

Thanks! I actually had almost identical implementation at first :), but then I realized I needed to avoid "stats::FragmentLayouts" to be increased in MCAsmLayout::layoutFragment when adjusting the offset of valid fragments and changed my implementation to https://reviews.llvm.org/D76114?id=250118. Also, one thing I am not sure is if marking fragments invalid will always trigger relaxation if needed in the following iteration of the while loop?

Never mind my last comment. I think I misunderstood the meaning of stats::FragmentLayouts. I have updated https://reviews.llvm.org/D76114 accordingly. Thanks.

skan added a comment.Mar 14 2020, 2:00 AM

I prefer the current solution. Making all fragments with expression computable sizes relaxable is much more ugly.

reames abandoned this revision.Mar 16 2020, 3:27 PM