[PATCH] D130247: [LoongArch] Refactor insertDivByZeroTrap

[Ray Wang via Phabricator via llvm-commits]

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