[PATCH] D69131: [NFC][XCOFF] Using crtp to refactor the xcoff section header

Digger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 24 11:55:34 PDT 2019


DiggerLin added a comment.

In D69131#1719155 <https://reviews.llvm.org/D69131#1719155>, @jasonliu wrote:

> General comments about the direction of this patch:
>  Previous comments(https://reviews.llvm.org/D68575#inline-617586) suggests to put those two constant value into XCOFF.h. 
>  I guess the reason for that is we want all the other component of in llvm to be able to use that those two constant value. For example, XCOFFObjectWriter.
>  But this patch currently put those two values in XCOFFObjectFile.h. The headers in llvm/Object seems to be strictly used only by all the llvm tooling that has an interest in the object itself, but not used by the llvm backend at all.
>
> I think the direction suggested in D68575 <https://reviews.llvm.org/D68575> makes more sense, as those two constant values are more related to object format, and we would want to use them in more than llvm toolings.


for these two constants  , I do note think we will use them in the xcoffobjectfilewriter.cpp(that is means they only use in xcoff decode tools, if only use in the xcoff decode tool, it is better to put them in the xcoffobjectfile.h).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D69131





More information about the llvm-commits mailing list