[llvm] r323710 - [RAFast] Don't dereference MBB::end

[Quentin Colombet via llvm-commits]

Hi Hans,

Could we pull that commit in the release please?

With GISel, this is a pattern that can come up more often.

Cheers,
-Quentin

> On Jan 29, 2018, at 3:42 PM, Quentin Colombet via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: qcolombet
> Date: Mon Jan 29 15:42:37 2018
> New Revision: 323710
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=323710&view=rev
> Log:
> [RAFast] Don't dereference MBB::end
> 
> When RAFast sees liveins in on a basic block, it uses that information
> to initialize the availability of the registers. The called
> method uses an instruction as one of its argument and in the liveins
> case, RAFast was dereferencing MBB::begin which can be MBB::end for
> empty basic block.
> 
> Change the API of definePhysReg to use MachineBasicBlock::iterator
> instead of MachineInstr so that we don't dereference an
> invalid iterator while making the call.
> 
> rdar://problem/36952401
> 
> Added:
>    llvm/trunk/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir
> Modified:
>    llvm/trunk/lib/CodeGen/RegAllocFast.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/RegAllocFast.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocFast.cpp?rev=323710&r1=323709&r2=323710&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/RegAllocFast.cpp (original)
> +++ llvm/trunk/lib/CodeGen/RegAllocFast.cpp Mon Jan 29 15:42:37 2018
> @@ -193,9 +193,10 @@ namespace {
>     void spillVirtReg(MachineBasicBlock::iterator MI, unsigned VirtReg);
> 
>     void usePhysReg(MachineOperand &MO);
> -    void definePhysReg(MachineInstr &MI, MCPhysReg PhysReg, RegState NewState);
> +    void definePhysReg(MachineBasicBlock::iterator MI, MCPhysReg PhysReg,
> +                       RegState NewState);
>     unsigned calcSpillCost(MCPhysReg PhysReg) const;
> -    void assignVirtToPhysReg(LiveReg&, MCPhysReg PhysReg);
> +    void assignVirtToPhysReg(LiveReg &, MCPhysReg PhysReg);
> 
>     LiveRegMap::iterator findLiveVirtReg(unsigned VirtReg) {
>       return LiveVirtRegs.find(TargetRegisterInfo::virtReg2Index(VirtReg));
> @@ -434,8 +435,8 @@ void RegAllocFast::usePhysReg(MachineOpe
> /// Mark PhysReg as reserved or free after spilling any virtregs. This is very
> /// similar to defineVirtReg except the physreg is reserved instead of
> /// allocated.
> -void RegAllocFast::definePhysReg(MachineInstr &MI, MCPhysReg PhysReg,
> -                                 RegState NewState) {
> +void RegAllocFast::definePhysReg(MachineBasicBlock::iterator MI,
> +                                 MCPhysReg PhysReg, RegState NewState) {
>   markRegUsedInInstr(PhysReg);
>   switch (unsigned VirtReg = PhysRegState[PhysReg]) {
>   case regDisabled:
> @@ -857,7 +858,7 @@ void RegAllocFast::allocateBasicBlock(Ma
>   // Add live-in registers as live.
>   for (const MachineBasicBlock::RegisterMaskPair LI : MBB.liveins())
>     if (MRI->isAllocatable(LI.PhysReg))
> -      definePhysReg(*MII, LI.PhysReg, regReserved);
> +      definePhysReg(MII, LI.PhysReg, regReserved);
> 
>   VirtDead.clear();
>   Coalesced.clear();
> 
> Added: llvm/trunk/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir?rev=323710&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir (added)
> +++ llvm/trunk/test/CodeGen/AArch64/fast-regalloc-empty-bb-with-liveins.mir Mon Jan 29 15:42:37 2018
> @@ -0,0 +1,26 @@
> +# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
> +# RUN: llc -mtriple aarch64-apple-ios -run-pass regallocfast -o - %s | FileCheck %s
> +# This test used to crash the fast register alloc.
> +# Basically, when a basic block has liveins, the fast regalloc
> +# was deferencing the begin iterator of this block. However,
> +# when this block is empty and it will just crashed!
> +---
> +name:            crashing
> +tracksRegLiveness: true
> +body:             |
> +  ; CHECK-LABEL: name: crashing
> +  ; CHECK: bb.0:
> +  ; CHECK:   successors: %bb.1(0x80000000)
> +  ; CHECK:   liveins: %x0, %x1
> +  ; CHECK: bb.1:
> +  ; CHECK:   renamable %w0 = MOVi32imm -1
> +  ; CHECK:   RET_ReallyLR implicit killed %w0
> +  bb.1:
> +    liveins: %x0, %x1
> +
> +  bb.2:
> +    %0:gpr32 = MOVi32imm -1
> +    %w0 = COPY %0
> +    RET_ReallyLR implicit %w0
> +
> +...
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits