This is an archive of the discontinued LLVM Phabricator instance.

[NFC] [XCOFF] Update PowerPC readobj test case with expression for Index and ContainingCsectSymbolIndex
ClosedPublic

Authored by edwin on Mar 15 2021, 12:05 AM.

Details

Summary

This patch is to replace the fixed value with expression.
Keep .file section as fixed values as it might be changed. The remaining sections will hardly be modified. So the Index values are sequential. By using expression, we can avoid the fixed value changes in coming patches.

This is a follow-up of patch D97117.

Diff Detail

Event Timeline

edwin created this revision.Mar 15 2021, 12:05 AM
edwin requested review of this revision.Mar 15 2021, 12:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2021, 12:05 AM
edwin retitled this revision from [NFC] Update PowerPC readobj test case with expression to [NFC] Update PowerPC readobj test case with expression for Index and ContainingCsectSymbolIndex.Mar 15 2021, 12:59 AM

What's the motivation for/benefit of doing this?

Please indicate this is for XCOFF in the title and a follow-up of the patch D97117 in the description.

llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
213

This seems not right as for now symbol 0 is always for .file, we don't need to make this one as the base, otherwise there is no meaning at all for this change. Now with this change, when we add some aux entry for .file symbol, we need to update all the following symbol entries. Without the change, with the .text symbol base, we don't need to do any change to existing symbols.

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
151

Same as above, use symbol for .bar as the base

llvm/test/tools/llvm-readobj/XCOFF/symbols.test
37 ↗(On Diff #330565)

We don't need to change this file as the object file is created by XLC and it will not be changed

edwin retitled this revision from [NFC] Update PowerPC readobj test case with expression for Index and ContainingCsectSymbolIndex to [NFC] [XCOFF] Update PowerPC readobj test case with expression for Index and ContainingCsectSymbolIndex.Mar 15 2021, 8:21 PM
edwin edited the summary of this revision. (Show Details)
edwin edited the summary of this revision. (Show Details)
edwin updated this revision to Diff 330894.Mar 16 2021, 1:25 AM
edwin edited the summary of this revision. (Show Details)
edwin added a reviewer: shchenz.

Update patch to address @shchenz's comments.

edwin added inline comments.Mar 16 2021, 1:28 AM
llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll
213

Addressed in new patch. Thanks!

edwin added a comment.Mar 16 2021, 1:37 AM

What's the motivation for/benefit of doing this?

Thank you for your comments. I updated the summary.

To address @hubert.reinterpretcast's comment in D97117, this patch leverage expression to replace the fixed value in Index and ContainingCsectSymbolIndex for Symbols.
Keep .file section as fixed values as it might be changed by coming patches. The remaining sections will hardly be modified. So the Index values are sequential. By using expression, we can avoid the fixed value changes effort.

Please remember to include full context with your patches. No further comments from me though. I'm happy if the xcoff maintainers are.

@edwin, the patch needs to be updated with additional diff context.

edwin updated this revision to Diff 331137.Mar 16 2021, 5:34 PM

Thanks for your reminder, @jhenderson and @hubert.reinterpretcast. Updated.

edwin added inline comments.Mar 16 2021, 5:35 PM
llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
151

Addressed in new patch. Thanks!

shchenz accepted this revision.Mar 16 2021, 6:54 PM

LGTM. Thanks for this improvement.

This revision is now accepted and ready to land.Mar 16 2021, 6:54 PM