[PATCH] D83655: [AsmPrinter] Split up .gcc_except_table

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Oct 31 11:13:10 PDT 2020


MaskRay added a comment.

In D83655#2366269 <https://reviews.llvm.org/D83655#2366269>, @rahmanl wrote:

> In D83655#2366222 <https://reviews.llvm.org/D83655#2366222>, @MaskRay wrote:
>
>> In D83655#2366206 <https://reviews.llvm.org/D83655#2366206>, @rahmanl wrote:
>>
>>> In D83655#2366091 <https://reviews.llvm.org/D83655#2366091>, @jrtc27 wrote:
>>>
>>>> I wonder if it'd be nicer to push this logic into getLSDASection by passing an (optional?) Function given that's where the normal LSDASection gets created? Having the check for ELF here seems to indicate that the logic isn't quite in the right place and belongs in the MCOFI subclass.
>>>
>>> I agree with @jrtc27 here. How about we implement this like `MCObjectFileInfo::getStackSizeSection(const MCSection&)` or `MCObjectFileInfo::getBBAddrMapSection(const MCSection&)`. The intended logic seems quite similar.
>>
>> It would be inconvenient.
>>
>> 1. getStackSizeSection and getBBAddrMapSection do not use different section names. They will need to know the suffix
>> 2. MCObjectFileInfo does not know whether function sections is enabled
>>
>> MCObjectFileInfo is a good place for getStackSizeSection and getBBAddrMapSection because they need just information from MCObjectFileInfo. However, it is different for .gcc_except_table
>
> Sorry for the misunderstanding. I am fine with your logic, but prefer if the code is better organized: I had a stab at it D90523 <https://reviews.llvm.org/D90523>.

Thanks! This does look cleaner, moreoever, EHStreamer.cpp does not have to include 3 additional headers. Tweaked a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83655



More information about the llvm-commits mailing list