[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