[llvm] r294088 - MachineCopyPropagation: Respect implicit operands of COPY

[Quentin Colombet via llvm-commits]

Hi,

SGTM (I’ve already reviewed the phab)

Cheers,
-Quentin
> On Feb 7, 2017, at 9:14 AM, Hans Wennborg <hans at chromium.org> wrote:
> 
> Quentin: Matthias has asked for this to be merged to 4.0 (see
> PR31881). What do you think?
> 
> Thanks,
> Hans
> 
> On Fri, Feb 3, 2017 at 6:27 PM, Matthias Braun via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: matze
>> Date: Fri Feb  3 20:27:20 2017
>> New Revision: 294088
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=294088&view=rev
>> Log:
>> MachineCopyPropagation: Respect implicit operands of COPY
>> 
>> The code missed to check implicit operands of COPY instructions for
>> defs/uses.
>> 
>> Differential Revision: https://reviews.llvm.org/D29522
>> 
>> Added:
>>    llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir
>> Modified:
>>    llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp?rev=294088&r1=294087&r2=294088&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp Fri Feb  3 20:27:20 2017
>> @@ -61,6 +61,7 @@ namespace {
>> 
>>   private:
>>     void ClobberRegister(unsigned Reg);
>> +    void ReadRegister(unsigned Reg);
>>     void CopyPropagateBlock(MachineBasicBlock &MBB);
>>     bool eraseIfRedundant(MachineInstr &Copy, unsigned Src, unsigned Def);
>> 
>> @@ -120,6 +121,18 @@ void MachineCopyPropagation::ClobberRegi
>>   }
>> }
>> 
>> +void MachineCopyPropagation::ReadRegister(unsigned Reg) {
>> +  // If 'Reg' is defined by a copy, the copy is no longer a candidate
>> +  // for elimination.
>> +  for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
>> +    Reg2MIMap::iterator CI = CopyMap.find(*AI);
>> +    if (CI != CopyMap.end()) {
>> +      DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());
>> +      MaybeDeadCopies.remove(CI->second);
>> +    }
>> +  }
>> +}
>> +
>> /// Return true if \p PreviousCopy did copy register \p Src to register \p Def.
>> /// This fact may have been obscured by sub register usage or may not be true at
>> /// all even though Src and Def are subregisters of the registers used in
>> @@ -212,12 +225,14 @@ void MachineCopyPropagation::CopyPropaga
>> 
>>       // If Src is defined by a previous copy, the previous copy cannot be
>>       // eliminated.
>> -      for (MCRegAliasIterator AI(Src, TRI, true); AI.isValid(); ++AI) {
>> -        Reg2MIMap::iterator CI = CopyMap.find(*AI);
>> -        if (CI != CopyMap.end()) {
>> -          DEBUG(dbgs() << "MCP: Copy is no longer dead: "; CI->second->dump());
>> -          MaybeDeadCopies.remove(CI->second);
>> -        }
>> +      ReadRegister(Src);
>> +      for (const MachineOperand &MO : MI->implicit_operands()) {
>> +        if (!MO.isReg() || !MO.readsReg())
>> +          continue;
>> +        unsigned Reg = MO.getReg();
>> +        if (!Reg)
>> +          continue;
>> +        ReadRegister(Reg);
>>       }
>> 
>>       DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI->dump());
>> @@ -234,6 +249,14 @@ void MachineCopyPropagation::CopyPropaga
>>       // ...
>>       // %xmm2<def> = copy %xmm9
>>       ClobberRegister(Def);
>> +      for (const MachineOperand &MO : MI->implicit_operands()) {
>> +        if (!MO.isReg() || !MO.isDef())
>> +          continue;
>> +        unsigned Reg = MO.getReg();
>> +        if (!Reg)
>> +          continue;
>> +        ClobberRegister(Reg);
>> +      }
>> 
>>       // Remember Def is defined by the copy.
>>       for (MCSubRegIterator SR(Def, TRI, /*IncludeSelf=*/true); SR.isValid();
>> @@ -269,17 +292,8 @@ void MachineCopyPropagation::CopyPropaga
>>       if (MO.isDef()) {
>>         Defs.push_back(Reg);
>>         continue;
>> -      }
>> -
>> -      // If 'Reg' is defined by a copy, the copy is no longer a candidate
>> -      // for elimination.
>> -      for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
>> -        Reg2MIMap::iterator CI = CopyMap.find(*AI);
>> -        if (CI != CopyMap.end()) {
>> -          DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());
>> -          MaybeDeadCopies.remove(CI->second);
>> -        }
>> -      }
>> +      } else if (MO.readsReg())
>> +        ReadRegister(Reg);
>>     }
>> 
>>     // The instruction has a register mask operand which means that it clobbers
>> 
>> Added: llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir?rev=294088&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir (added)
>> +++ llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir Fri Feb  3 20:27:20 2017
>> @@ -0,0 +1,22 @@
>> +# RUN: llc -o - %s -mtriple=armv7s-- -run-pass=machine-cp | FileCheck %s
>> +---
>> +# Test that machine copy prop recognizes the implicit-def operands on a COPY
>> +# as clobbering the register.
>> +# CHECK-LABEL: name: func
>> +# CHECK: %d2 = VMOVv2i32 2, 14, _
>> +# CHECK: %s5 = COPY %s0, implicit %q1, implicit-def %q1
>> +# CHECK: VST1q32 %r0, 0, %q1, 14, _
>> +# The following two COPYs must not be removed
>> +# CHECK: %s4 = COPY %s20, implicit-def %q1
>> +# CHECK: %s5 = COPY %s0, implicit killed %d0, implicit %q1, implicit-def %q1
>> +# CHECK: VST1q32 %r2, 0, %q1, 14, _
>> +name: func
>> +body: |
>> +  bb.0:
>> +    %d2 = VMOVv2i32 2, 14, _
>> +    %s5 = COPY %s0, implicit %q1, implicit-def %q1
>> +    VST1q32 %r0, 0, %q1, 14, _
>> +    %s4 = COPY %s20, implicit-def %q1
>> +    %s5 = COPY %s0, implicit killed %d0, implicit %q1, implicit-def %q1
>> +    VST1q32 %r2, 0, %q1, 14, _
>> +...
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits