[PATCH] D130247: [LoongArch] Refactor insertDivByZeroTrap
Ray Wang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 25 05:34:18 PDT 2022
wangleiat added a comment.
In D130247#3674119 <https://reviews.llvm.org/D130247#3674119>, @MaskRay wrote:
>> Ensure non-terminators don't follow terminators.
>
> Ensure the non-terminator BREAK doesn't follow the terminator BNEZ.
Thanks.
================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:741
static MachineBasicBlock *insertDivByZeroTrap(MachineInstr &MI,
+ MachineBasicBlock *MBB) {
----------------
MaskRay wrote:
> Keep the reference for the non-null semantics.
> Keep the reference for the non-null semantics.
================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:741
static MachineBasicBlock *insertDivByZeroTrap(MachineInstr &MI,
+ MachineBasicBlock *MBB) {
----------------
wangleiat wrote:
> MaskRay wrote:
> > Keep the reference for the non-null semantics.
> > Keep the reference for the non-null semantics.
>
>
> Keep the reference for the non-null semantics.
Thanks, I think it's better to keep the status quo.
Firstly, it is possible to keep the same declaration as the caller, and secondly, if a reference is passed, it will increase the parameters of `BuildMI`.
(Seems to be irrelevant reasons ^_^)
================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:769
+
+ EndMBB->splice(EndMBB->end(), MBB, std::next(MI.getIterator()), MBB->end());
+ EndMBB->transferSuccessorsAndUpdatePHIs(MBB);
----------------
MaskRay wrote:
> Move EndMBB operations after BreakMBB? So that the MBB modify order resembles more on the MBB placement order.
> Move EndMBB operations after BreakMBB? So that the MBB modify order resembles more on the MBB placement order.
Thanks, I'll re-order the basic block settings.
But transferring the remaining MBB and its successor edges to EndMBB(Maybe changing the name would be better?) is something that must be done first.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130247/new/
https://reviews.llvm.org/D130247
More information about the llvm-commits
mailing list