<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 5, 2012, at 4:58 PM, Jack Carter wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: jacksprat<br>Date: Thu Jul 5 18:58:21 2012<br>New Revision: 159787<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=159787&view=rev">http://llvm.org/viewvc/llvm-project?rev=159787&view=rev</a><br>Log:<br> Mips specific inline asm operand modifier D.<br><br> Print the second half of a double word operand.<br><br> The include list was cleaned up a bit as well.<br><br> Also the test case was modified to test for both<br> big and little patterns.<br><br></div></blockquote><div><br></div><div>Should probably have made this 3 patches.</div><div><br></div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000"><br></font>+ // This will be shared with other cases in succeeding checkins<br></div></blockquote><div><br></div><div>You probably meant "successive", but this comment isn't really necessary,</div><div>nor is the similar one below. Also, a comment should be a complete prose</div><div>sentence:</div><div><br></div><div><a href="http://llvm.org/docs/CodingStandards.html#commenting">http://llvm.org/docs/CodingStandards.html#commenting</a></div><div><br></div><div>and the rest of the comments should probably be fixed at some point.</div><br><blockquote type="cite"><div>+ case 'D': {<br>+ // Second part of a double word register operand<br>+ if (OpNum == 0)<br>+ return true;<br>+ const MachineOperand &FlagsOP = MI->getOperand(OpNum - 1);<br>+ if (!FlagsOP.isImm())<br>+ return true;<br>+ unsigned Flags = FlagsOP.getImm();<br>+ unsigned NumVals = InlineAsm::getNumOperandRegisters(Flags);<br>+ if (NumVals != 2) {<br>+ if (!Subtarget->isGP32bit() && NumVals == 1 && MO.isReg()) {<br></div></blockquote><div><br></div><div>Given the comment you probably want isGP64bit()? (Which is what you wrote,</div><div>but since you have a predicate I figured the obvious one would be better.</div><div><br></div><div>The NumVals comparisons are a bit odd, might want to add a comment for</div><div>them.</div><br><blockquote type="cite"><div>+ // In 64 bit mode long longs are always just a single reg<br></div></blockquote><div><br></div><div>long long is probably a misnomer since we're already in the backend and</div><div>such things don't exist.</div><br><blockquote type="cite"><div>+ unsigned Reg = MO.getReg();<br>+ O << '$' << MipsInstPrinter::getRegisterName(Reg);<br>+ return false;<br>+ }<br>+ return true;<br>+ }<br>+ unsigned RegOp;<br>+ switch(ExtraCode[0]) {<br>+ // This will have other cases in succeeding checkins<br>+ case 'D':<br>+ RegOp = (!Subtarget->isGP32bit()) ? OpNum : OpNum + 1;<br>+ break;<br>+ }<br>+ if (RegOp >= MI->getNumOperands())<br>+ return true;<br>+ const MachineOperand &MO = MI->getOperand(RegOp);<br>+ if (!MO.isReg())<br>+ return true;<br>+ unsigned Reg = MO.getReg();<br>+ O << '$' << MipsInstPrinter::getRegisterName(Reg);<br>+ return false;<br> }<br>- }<br>+ } // switch<br>+ } // if ExtraCode<br><br></div></blockquote><br></div><div>Not a huge fan of these comments either :)</div><div><br></div><div>-eric</div><br></body></html>