[PATCH] D132146: [PowerPC] XCOFF exception section support on integrated and direct assembler paths and exception language and reason code lowering from trap intrinsics

Paul Scoropan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 13 06:41:57 PDT 2022


pscoro added a comment.

In D132146#3785383 <https://reviews.llvm.org/D132146#3785383>, @shchenz wrote:

> You should not add the integrated-as mode change to the existing non integrated-as change.
> Better to post two patches for these two functionalities.



In D132146#3785915 <https://reviews.llvm.org/D132146#3785915>, @RKSimon wrote:

> In D132146#3785383 <https://reviews.llvm.org/D132146#3785383>, @shchenz wrote:
>
>> You should not add the integrated-as mode change to the existing non integrated-as change.
>> Better to post two patches for these two functionalities.
>
> +1 Please split these

How would you recommend splitting these up? Beside the lit tests there is only one file (MCAsmStreamer) that has code specific to the direct assembly path, and only MCXCOFFStreamer and XCOFFObjectWriter are specific to the integrated-as path. The rest is shared code. My assumption was that if there is this much shared code it would be safer to make one review because the second review can't merge until this first one has and for a given feature, the full body of code that is needed for it is present in its review. That being said, do you prefer if I just move MCXCOFFStreamer, XCOFFObjectWriter and the object lit tests to a separate review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132146



More information about the llvm-commits mailing list