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

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 7 15:33:32 PST 2017


I tried to merge this, but it breaks
CodeGen/AMDGPU/attr-amdgpu-num-sgpr.ll (see below).

Also merging r294087 seems to fix this.

Matthias: should these be merged together?

Thanks,
Hans


# End machine code for function max_12_sgprs_14_input_sgprs.

*** Bad machine code: Using an undefined physical register ***
- function:    max_12_sgprs_14_input_sgprs
- basic block: BB#0  (0x4532e98)
- instruction: FLAT_STORE_DWORDX2
- operand 1:   %VGPR0_VGPR1<kill>
LLVM ERROR: Found 1 machine code errors.
/usr/local/google/work/llvm-4.0/llvm.src/test/CodeGen/AMDGPU/attr-amdgpu-num-sgpr.ll:45:14:
error: expected string not found in input
; ALL-LABEL: {{^}}max_12_sgprs_14_input_sgprs:
             ^
<stdin>:13:15: note: scanning from here
max_12_sgprs: ; @max_12_sgprs
              ^

--




On Tue, Feb 7, 2017 at 1:42 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 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
>


More information about the llvm-commits mailing list