[PATCH] D66969: Output XCOFF object text section

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 28 13:37:20 PDT 2019


hubert.reinterpretcast added a comment.

In D66969#1683070 <https://reviews.llvm.org/D66969#1683070>, @DiggerLin wrote:

> we can not generate a raw data of the '.text" XCOFF section.
>  there are upcoming patch which deal with the  DS csect , I think we will add the test in that patch @jasonliu.


I believe that the `.text` section testing should be separate. The fact that the patch you mention would enable the ability to generate a non-zero size XCOFF `.text` section does not mean that it should cover testing beyond its own scope.

>From the current patch, we have identified a need for follow-up patches to test `.text` sections that aren't empty, to test the placement of symbol names, and to address the naming of a field that this patch uses for a different purpose. If the second sounds out-of-place, then that is because the current commit message does not actually describe the scope of the patch well. The patch did a refactoring of interfaces, and that portion of the patch is why the second follow-up patch is tied to this one.

Please update the commit message to describe the scope of the current patch (especially that necessary dependencies for there to be non-empty `.text` sections have not landed) and to indicate a plan for follow-on patches.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66969/new/

https://reviews.llvm.org/D66969





More information about the llvm-commits mailing list