<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br></div>Comments about the patch:<div><div>+    // 1. We find a def that's dead. Insert after it.</div><div>+    // 2. We find a def that doesn't use the current EFLAGS. Insert before it.</div><div>+    Def = Sub;</div><div>+    MachineBasicBlock::reverse_iterator</div><div>+      InsertI = MachineBasicBlock::reverse_iterator(++Def),</div><div>+                InsertE = Sub->getParent()->rend();</div><div>+    for (; InsertI != InsertE; ++InsertI) {</div><div>+      MachineInstr *Instr = &*InsertI;</div><div>+      if (!Instr->readsRegister(X86::EFLAGS, TRI) &&</div><div>+          Instr->modifiesRegister(X86::EFLAGS, TRI)) {</div><div>I don't see code corresponding to the first case (find a def that's dead. insert after it)</div><div><br></div><div>For the testing case, can we still check the generated code to make sure peephole is doing the right thing?</div><div><br></div><div>Thanks,</div><div>Manman</div><div><br></div><div><div>On May 16, 2013, at 7:03 PM, David Majnemer wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">We can but this would prevent us from removing a test instruction if a cmov was choosing between an adc instruction and 0.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, May 16, 2013 at 6:03 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div>Hi David,<div><br></div><div>Can we return false earlier if the definition MI uses EFLAGS, inside the following loop?</div>
<div><br></div><div>Thanks,</div><div>Manman</div><div><br></div><div><div>  for (; RI != RE; ++RI) {</div><div>    MachineInstr *Instr = &*RI;</div><div>    // Check whether CmpInstr can be made redundant by the current instruction.</div>
<div>    if (!IsCmpZero &&</div><div>        isRedundantFlagInstr(CmpInstr, SrcReg, SrcReg2, CmpValue, Instr)) {</div><div>      Sub = Instr;</div><div>      break;</div><div>    }</div><div><br></div><div>    if (Instr->modifiesRegister(X86::EFLAGS, TRI) ||</div>
<div>        Instr->readsRegister(X86::EFLAGS, TRI)) {</div><div>      // This instruction modifies or uses EFLAGS.</div><div><br></div><div>      // MOV32r0 etc. are implemented with xor which clobbers condition code.</div>
<div>      // They are safe to move up, if the definition to EFLAGS is dead and</div><div>      // earlier instructions do not read or write EFLAGS.</div><div>      if (!Movr0Inst && (Instr->getOpcode() == X86::MOV8r0 ||</div>
<div>           Instr->getOpcode() == X86::MOV16r0 ||</div><div>           Instr->getOpcode() == X86::MOV32r0 ||</div><div>           Instr->getOpcode() == X86::MOV64r0) &&</div><div>          Instr->registerDefIsDead(X86::EFLAGS, TRI)) {</div>
<div>--> check whether MI uses EFLAGS</div><div>        Movr0Inst = Instr;</div><div>        continue;</div><div>      }</div><div><br></div><div><div>      // We can't remove CmpInstr.</div><div>      return false;</div>
<div>    }</div><div>  }</div></div><div><br></div><div><div><div class="h5"><div>On May 16, 2013, at 5:17 PM, David Majnemer wrote:</div><br></div></div><blockquote type="cite"><div><div class="h5"><div dir="ltr">Attached is a patch to address PR16031.<div>
<br></div><div>The peephole tries to reorder MOV32r0 instructions so that they are before the instruction that modifies EFLAGS.</div><div>The problem is that the peephole does not consider the case where the instruction that modifies EFLAGS also depends on them.</div>

<div><br></div><div>This patch walks backwards until it finds an instruction that has a def but no use and inserts right before it.</div><div>If it cannot find such an instruction, it skips the optimization.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, May 16, 2013 at 12:03 PM, Manman Ren <span dir="ltr"><<a href="mailto:mren@apple.com" target="_blank">mren@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div style="word-wrap:break-word"><div><br></div>This is related to PR16031.<div><br></div><div>Please take a look,</div><div>Thanks,</div><div>Manman</div><div><div><div><br></div><div><div><div>On May 15, 2013, at 3:45 PM, Pete Cooper wrote:</div>

<br><blockquote type="cite"><div style="word-wrap:break-word">Hi David<div><br></div><div>This test is breaking a buildbot.  Could you please take a look?</div><div><br></div><div><a href="http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/1779" target="_blank">http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/1779</a></div>

<div><br></div><div>Thanks,</div><div>Pete<br><div><div>On May 15, 2013, at 3:03 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>> wrote:</div><br><blockquote type="cite">

<div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Author: majnemer<br>Date: Wed May 15 17:03:08 2013<br>New Revision: 181937<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=181937&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181937&view=rev</a><br>

Log:<br>X86: Remove redundant test instructions<br><br>Increase the number of instructions LLVM recognizes as setting the ZF<br>flag. This allows us to remove test instructions that redundantly<br>recalculate the flag.<br>

<br>Added:<br>   llvm/trunk/test/CodeGen/X86/peep-test-4.ll<br>Modified:<br>   llvm/trunk/lib/Target/X86/X86InstrInfo.cpp<br><br>Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=181937&r1=181936&r2=181937&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=181937&r1=181936&r2=181937&view=diff</a><br>

==============================================================================<br>--- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Wed May 15 17:03:08 2013<br>@@ -3200,8 +3200,37 @@ inline static bool isDefConvertible(Mach<br>

  case X86::OR8ri:     case X86::OR64rr:   case X86::OR32rr:<br>  case X86::OR16rr:    case X86::OR8rr:    case X86::OR64rm:<br>  case X86::OR32rm:    case X86::OR16rm:   case X86::OR8rm:<br>+  case X86::NEG8r:     case X86::NEG16r:   case X86::NEG32r: case X86::NEG64r:<br>

+  case X86::SAR8r1:    case X86::SAR16r1:  case X86::SAR32r1:case X86::SAR64r1:<br>+  case X86::SHR8r1:    case X86::SHR16r1:  case X86::SHR32r1:case X86::SHR64r1:<br>+  case X86::SHL8r1:    case X86::SHL16r1:  case X86::SHL32r1:case X86::SHL64r1:<br>

+  case X86::ADC32ri:   case X86::ADC32ri8:<br>+  case X86::ADC32rr:   case X86::ADC64ri32:<br>+  case X86::ADC64ri8:  case X86::ADC64rr:<br>+  case X86::SBB32ri:   case X86::SBB32ri8:<br>+  case X86::SBB32rr:   case X86::SBB64ri32:<br>

+  case X86::SBB64ri8:  case X86::SBB64rr:<br>  case X86::ANDN32rr:  case X86::ANDN32rm:<br>  case X86::ANDN64rr:  case X86::ANDN64rm:<br>+  case X86::BEXTR32rr: case X86::BEXTR64rr:<br>+  case X86::BEXTR32rm: case X86::BEXTR64rm:<br>

+  case X86::BLSI32rr:  case X86::BLSI32rm:<br>+  case X86::BLSI64rr:  case X86::BLSI64rm:<br>+  case X86::BLSMSK32rr:case X86::BLSMSK32rm:<br>+  case X86::BLSMSK64rr:case X86::BLSMSK64rm:<br>+  case X86::BLSR32rr:  case X86::BLSR32rm:<br>

+  case X86::BLSR64rr:  case X86::BLSR64rm:<br>+  case X86::BZHI32rr:  case X86::BZHI32rm:<br>+  case X86::BZHI64rr:  case X86::BZHI64rm:<br>+  case X86::LZCNT16rr: case X86::LZCNT16rm:<br>+  case X86::LZCNT32rr: case X86::LZCNT32rm:<br>

+  case X86::LZCNT64rr: case X86::LZCNT64rm:<br>+  case X86::POPCNT16rr:case X86::POPCNT16rm:<br>+  case X86::POPCNT32rr:case X86::POPCNT32rm:<br>+  case X86::POPCNT64rr:case X86::POPCNT64rm:<br>+  case X86::TZCNT16rr: case X86::TZCNT16rm:<br>

+  case X86::TZCNT32rr: case X86::TZCNT32rm:<br>+  case X86::TZCNT64rr: case X86::TZCNT64rm:<br>    return true;<br>  }<br>}<br>@@ -3427,13 +3456,16 @@ optimizeCompareInstr(MachineInstr *CmpIn<br>  }<br><br>  // Make sure Sub instruction defines EFLAGS and mark the def live.<br>

-  unsigned LastOperand = Sub->getNumOperands() - 1;<br>-  assert(Sub->getNumOperands() >= 2 &&<br>-         Sub->getOperand(LastOperand).isReg() &&<br>-         Sub->getOperand(LastOperand).getReg() == X86::EFLAGS &&<br>

-         "EFLAGS should be the last operand of SUB, ADD, OR, XOR, AND");<br>-  Sub->getOperand(LastOperand).setIsDef(true);<br>-  Sub->getOperand(LastOperand).setIsDead(false);<br>+  unsigned i = 0, e = Sub->getNumOperands();<br>

+  for (; i != e; ++i) {<br>+    MachineOperand &MO = Sub->getOperand(i);<br>+    if (MO.isReg() && MO.isDef() && MO.getReg() == X86::EFLAGS) {<br>+      MO.setIsDead(false);<br>+      break;<br>+    }<br>

+  }<br>+  assert(i != e && "Unable to locate a def EFLAGS operand");<br>+<br>  CmpInstr->eraseFromParent();<br><br>  // Modify the condition code of instructions in OpsToUpdate.<br><br>Added: llvm/trunk/test/CodeGen/X86/peep-test-4.ll<br>

URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/peep-test-4.ll?rev=181937&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/peep-test-4.ll?rev=181937&view=auto</a><br>

==============================================================================<br>--- llvm/trunk/test/CodeGen/X86/peep-test-4.ll (added)<br>+++ llvm/trunk/test/CodeGen/X86/peep-test-4.ll Wed May 15 17:03:08 2013<br>@@ -0,0 +1,154 @@<br>

+; RUN: llc < %s -march=x86-64 -mattr=+bmi,+bmi2,+popcnt | FileCheck %s<br>+declare void @foo(i32)<br>+<br>+; CHECK: neg:<br>+; CHECK: negl %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+define void @neg(i32 %x) nounwind {<br>

+  %sub = sub i32 0, %x<br>+  %cmp = icmp eq i32 %sub, 0<br>+  br i1 %cmp, label %return, label %bb<br>+<br>+bb:<br>+  tail call void @foo(i32 %sub)<br>+  br label %return<br>+<br>+return:<br>+  ret void<br>+}<br>+<br>+; CHECK: sar:<br>

+; CHECK: sarl %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+define void @sar(i32 %x) nounwind {<br>+  %ashr = ashr i32 %x, 1<br>+  %cmp = icmp eq i32 %ashr, 0<br>+  br i1 %cmp, label %return, label %bb<br>

+<br>+bb:<br>+  tail call void @foo(i32 %ashr)<br>+  br label %return<br>+<br>+return:<br>+  ret void<br>+}<br>+<br>+; CHECK: shr:<br>+; CHECK: shrl %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+define void @shr(i32 %x) nounwind {<br>

+  %ashr = lshr i32 %x, 1<br>+  %cmp = icmp eq i32 %ashr, 0<br>+  br i1 %cmp, label %return, label %bb<br>+<br>+bb:<br>+  tail call void @foo(i32 %ashr)<br>+  br label %return<br>+<br>+return:<br>+  ret void<br>+}<br>+<br>

+; CHECK: shl:<br>+; CHECK: addl %edi, %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+define void @shl(i32 %x) nounwind {<br>+  %shl = shl i32 %x, 1<br>+  %cmp = icmp eq i32 %shl, 0<br>+  br i1 %cmp, label %return, label %bb<br>

+<br>+bb:<br>+  tail call void @foo(i32 %shl)<br>+  br label %return<br>+<br>+return:<br>+  ret void<br>+}<br>+<br>+; CHECK: adc:<br>+; CHECK: movabsq $-9223372036854775808, %rax<br>+; CHECK-NEXT: addq  %rdi, %rax<br>+; CHECK-NEXT: adcq  $0, %rsi<br>

+; CHECK-NEXT: sete  %al<br>+; CHECK-NEXT: ret<br>+define zeroext i1 @adc(i128 %x) nounwind {<br>+  %add = add i128 %x, 9223372036854775808<br>+  %cmp = icmp ult i128 %add, 18446744073709551616<br>+  ret i1 %cmp<br>+}<br>

+<br>+; CHECK: sbb:<br>+; CHECK: cmpq  %rdx, %rdi<br>+; CHECK-NEXT: sbbq  %rcx, %rsi<br>+; CHECK-NEXT: setns %al<br>+; CHECK-NEXT: ret<br>+define zeroext i1 @sbb(i128 %x, i128 %y) nounwind {<br>+  %sub = sub i128 %x, %y<br>

+  %cmp = icmp sge i128 %sub, 0<br>+  ret i1 %cmp<br>+}<br>+<br>+; CHECK: andn:<br>+; CHECK: andnl   %esi, %edi, %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+define void @andn(i32 %x, i32 %y) nounwind {<br>

+  %not = xor i32 %x, -1<br>+  %andn = and i32 %y, %not<br>+  %cmp = icmp eq i32 %andn, 0<br>+  br i1 %cmp, label %return, label %bb<br>+<br>+bb:<br>+  tail call void @foo(i32 %andn)<br>+  br label %return<br>+<br>+return:<br>

+  ret void<br>+}<br>+<br>+; CHECK: bextr:<br>+; CHECK: bextrl   %esi, %edi, %edi<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+declare i32 @llvm.x86.bmi.bextr.32(i32, i32) nounwind readnone<br>+define void @bextr(i32 %x, i32 %y) nounwind {<br>

+  %bextr = tail call i32 @llvm.x86.bmi.bextr.32(i32 %x, i32 %y)<br>+  %cmp = icmp eq i32 %bextr, 0<br>+  br i1 %cmp, label %return, label %bb<br>+<br>+bb:<br>+  tail call void @foo(i32 %bextr)<br>+  br label %return<br>
+<br>
+return:<br>+  ret void<br>+}<br>+<br>+; CHECK: popcnt:<br>+; CHECK: popcntl<br>+; CHECK-NEXT: je<br>+; CHECK: jmp foo<br>+; CHECK: ret<br>+declare i32 @llvm.ctpop.i32(i32) nounwind readnone<br>+define void @popcnt(i32 %x) nounwind {<br>

+  %popcnt = tail call i32 @llvm.ctpop.i32(i32 %x)<br>+  %cmp = icmp eq i32 %popcnt, 0<br>+  br i1 %cmp, label %return, label %bb<br>+;<br>+bb:<br>+  tail call void @foo(i32 %popcnt)<br>+  br label %return<br>+;<br>+return:<br>

+  ret void<br>+}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div>

</blockquote></div><br></div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>

</blockquote></div><br></div></div></div></div></blockquote></div><br></div>
</div></div><span><pr16031.diff></span></blockquote></div><br></div></div></blockquote></div><br></div>
</blockquote></div><br></div></body></html>