<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">I attached a slightly modified version of the patch to <a href="http://llvm.org/PR31881" class="">http://llvm.org/PR31881</a> which avoids this problem (you have to consider undef operands as read operations unless you have r294087 in place). However I consider r294087 a bit too risky without having a few weeks of testing in llvm ToT so I would like to keep it out of the 4.0 branch and just go with the modified patch as attached to the PR.</div><div class=""><br class=""></div><div class="">- Matthias</div><div class=""><br class=""></div><div><blockquote type="cite" class=""><div class="">On Feb 7, 2017, at 3:33 PM, Hans Wennborg via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">I tried to merge this, but it breaks<br class="">CodeGen/AMDGPU/attr-amdgpu-num-sgpr.ll (see below).<br class=""><br class="">Also merging r294087 seems to fix this.<br class=""><br class="">Matthias: should these be merged together?<br class=""><br class="">Thanks,<br class="">Hans<br class=""><br class=""><br class=""># End machine code for function max_12_sgprs_14_input_sgprs.<br class=""><br class="">*** Bad machine code: Using an undefined physical register ***<br class="">- function: max_12_sgprs_14_input_sgprs<br class="">- basic block: BB#0 (0x4532e98)<br class="">- instruction: FLAT_STORE_DWORDX2<br class="">- operand 1: %VGPR0_VGPR1<kill><br class="">LLVM ERROR: Found 1 machine code errors.<br class="">/usr/local/google/work/llvm-4.0/llvm.src/test/CodeGen/AMDGPU/attr-amdgpu-num-sgpr.ll:45:14:<br class="">error: expected string not found in input<br class="">; ALL-LABEL: {{^}}max_12_sgprs_14_input_sgprs:<br class=""> ^<br class=""><stdin>:13:15: note: scanning from here<br class="">max_12_sgprs: ; @max_12_sgprs<br class=""> ^<br class=""><br class="">--<br class=""><br class=""><br class=""><br class=""><br class="">On Tue, Feb 7, 2017 at 1:42 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:<br class=""><blockquote type="cite" class="">Hi,<br class=""><br class="">SGTM (I’ve already reviewed the phab)<br class=""><br class="">Cheers,<br class="">-Quentin<br class=""><blockquote type="cite" class="">On Feb 7, 2017, at 9:14 AM, Hans Wennborg <<a href="mailto:hans@chromium.org" class="">hans@chromium.org</a>> wrote:<br class=""><br class="">Quentin: Matthias has asked for this to be merged to 4.0 (see<br class="">PR31881). What do you think?<br class=""><br class="">Thanks,<br class="">Hans<br class=""><br class="">On Fri, Feb 3, 2017 at 6:27 PM, Matthias Braun via llvm-commits<br class=""><<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><blockquote type="cite" class="">Author: matze<br class="">Date: Fri Feb 3 20:27:20 2017<br class="">New Revision: 294088<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294088&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=294088&view=rev</a><br class="">Log:<br class="">MachineCopyPropagation: Respect implicit operands of COPY<br class=""><br class="">The code missed to check implicit operands of COPY instructions for<br class="">defs/uses.<br class=""><br class="">Differential Revision: <a href="https://reviews.llvm.org/D29522" class="">https://reviews.llvm.org/D29522</a><br class=""><br class="">Added:<br class=""> llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir<br class="">Modified:<br class=""> llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp<br class=""><br class="">Modified: llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp?rev=294088&r1=294087&r2=294088&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp?rev=294088&r1=294087&r2=294088&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp (original)<br class="">+++ llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp Fri Feb 3 20:27:20 2017<br class="">@@ -61,6 +61,7 @@ namespace {<br class=""><br class=""> private:<br class=""> void ClobberRegister(unsigned Reg);<br class="">+ void ReadRegister(unsigned Reg);<br class=""> void CopyPropagateBlock(MachineBasicBlock &MBB);<br class=""> bool eraseIfRedundant(MachineInstr &Copy, unsigned Src, unsigned Def);<br class=""><br class="">@@ -120,6 +121,18 @@ void MachineCopyPropagation::ClobberRegi<br class=""> }<br class="">}<br class=""><br class="">+void MachineCopyPropagation::ReadRegister(unsigned Reg) {<br class="">+ // If 'Reg' is defined by a copy, the copy is no longer a candidate<br class="">+ // for elimination.<br class="">+ for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {<br class="">+ Reg2MIMap::iterator CI = CopyMap.find(*AI);<br class="">+ if (CI != CopyMap.end()) {<br class="">+ DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());<br class="">+ MaybeDeadCopies.remove(CI->second);<br class="">+ }<br class="">+ }<br class="">+}<br class="">+<br class="">/// Return true if \p PreviousCopy did copy register \p Src to register \p Def.<br class="">/// This fact may have been obscured by sub register usage or may not be true at<br class="">/// all even though Src and Def are subregisters of the registers used in<br class="">@@ -212,12 +225,14 @@ void MachineCopyPropagation::CopyPropaga<br class=""><br class=""> // If Src is defined by a previous copy, the previous copy cannot be<br class=""> // eliminated.<br class="">- for (MCRegAliasIterator AI(Src, TRI, true); AI.isValid(); ++AI) {<br class="">- Reg2MIMap::iterator CI = CopyMap.find(*AI);<br class="">- if (CI != CopyMap.end()) {<br class="">- DEBUG(dbgs() << "MCP: Copy is no longer dead: "; CI->second->dump());<br class="">- MaybeDeadCopies.remove(CI->second);<br class="">- }<br class="">+ ReadRegister(Src);<br class="">+ for (const MachineOperand &MO : MI->implicit_operands()) {<br class="">+ if (!MO.isReg() || !MO.readsReg())<br class="">+ continue;<br class="">+ unsigned Reg = MO.getReg();<br class="">+ if (!Reg)<br class="">+ continue;<br class="">+ ReadRegister(Reg);<br class=""> }<br class=""><br class=""> DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI->dump());<br class="">@@ -234,6 +249,14 @@ void MachineCopyPropagation::CopyPropaga<br class=""> // ...<br class=""> // %xmm2<def> = copy %xmm9<br class=""> ClobberRegister(Def);<br class="">+ for (const MachineOperand &MO : MI->implicit_operands()) {<br class="">+ if (!MO.isReg() || !MO.isDef())<br class="">+ continue;<br class="">+ unsigned Reg = MO.getReg();<br class="">+ if (!Reg)<br class="">+ continue;<br class="">+ ClobberRegister(Reg);<br class="">+ }<br class=""><br class=""> // Remember Def is defined by the copy.<br class=""> for (MCSubRegIterator SR(Def, TRI, /*IncludeSelf=*/true); SR.isValid();<br class="">@@ -269,17 +292,8 @@ void MachineCopyPropagation::CopyPropaga<br class=""> if (MO.isDef()) {<br class=""> Defs.push_back(Reg);<br class=""> continue;<br class="">- }<br class="">-<br class="">- // If 'Reg' is defined by a copy, the copy is no longer a candidate<br class="">- // for elimination.<br class="">- for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {<br class="">- Reg2MIMap::iterator CI = CopyMap.find(*AI);<br class="">- if (CI != CopyMap.end()) {<br class="">- DEBUG(dbgs() << "MCP: Copy is used - not dead: "; CI->second->dump());<br class="">- MaybeDeadCopies.remove(CI->second);<br class="">- }<br class="">- }<br class="">+ } else if (MO.readsReg())<br class="">+ ReadRegister(Reg);<br class=""> }<br class=""><br class=""> // The instruction has a register mask operand which means that it clobbers<br class=""><br class="">Added: llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir?rev=294088&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir?rev=294088&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir (added)<br class="">+++ llvm/trunk/test/CodeGen/ARM/machine-copyprop.mir Fri Feb 3 20:27:20 2017<br class="">@@ -0,0 +1,22 @@<br class="">+# RUN: llc -o - %s -mtriple=armv7s-- -run-pass=machine-cp | FileCheck %s<br class="">+---<br class="">+# Test that machine copy prop recognizes the implicit-def operands on a COPY<br class="">+# as clobbering the register.<br class="">+# CHECK-LABEL: name: func<br class="">+# CHECK: %d2 = VMOVv2i32 2, 14, _<br class="">+# CHECK: %s5 = COPY %s0, implicit %q1, implicit-def %q1<br class="">+# CHECK: VST1q32 %r0, 0, %q1, 14, _<br class="">+# The following two COPYs must not be removed<br class="">+# CHECK: %s4 = COPY %s20, implicit-def %q1<br class="">+# CHECK: %s5 = COPY %s0, implicit killed %d0, implicit %q1, implicit-def %q1<br class="">+# CHECK: VST1q32 %r2, 0, %q1, 14, _<br class="">+name: func<br class="">+body: |<br class="">+ bb.0:<br class="">+ %d2 = VMOVv2i32 2, 14, _<br class="">+ %s5 = COPY %s0, implicit %q1, implicit-def %q1<br class="">+ VST1q32 %r0, 0, %q1, 14, _<br class="">+ %s4 = COPY %s20, implicit-def %q1<br class="">+ %s5 = COPY %s0, implicit killed %d0, implicit %q1, implicit-def %q1<br class="">+ VST1q32 %r2, 0, %q1, 14, _<br class="">+...<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></blockquote></blockquote><br class=""></blockquote>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></div></blockquote></div><br class=""></body></html>