<html dir="ltr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style id="owaParaStyle" type="text/css">
<!--
p
        {margin-top:0;
        margin-bottom:0}
-->
P {margin-top:0;margin-bottom:0;}</style>
</head>
<body ocsi="0" fpstyle="1" style="word-wrap:break-word">
<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;">Hi all,<br>
please disregard my last attachment. When I applied the change to the actual implementation I have received errors as the MipsGenAsmMatcher.inc is using generated the name MipsOperand instead of MipsAsmOperand.In fact none, of the targets I checked doesn't
 use 'xxxAsmOperand' but rather 'xxxOperand', probably due to generated name. I will check this again and send new patch.<br>
<br>
Vladimir<br>
<div style="font-family: Times New Roman; color: #000000; font-size: 16px">
<hr tabindex="-1">
<div style="direction: ltr;" id="divRpF131928"><font color="#000000" face="Tahoma" size="2"><b>From:</b> Medic, Vladimir<br>
<b>Sent:</b> Monday, July 16, 2012 3:06 PM<br>
<b>To:</b> Jim Grosbach<br>
<b>Cc:</b> llvm-commits@cs.uiuc.edu; Jovanovic, Petar<br>
<b>Subject:</b> RE: [llvm-commits] Mips Assembler stubs for llvm<br>
</font><br>
</div>
<div></div>
<div>
<div style="direction:ltr; font-family:Tahoma; color:#000000; font-size:10pt">Hi Jim,<br>
thank you for your feedback. I have attached a new patch that includes your remarks.
<br>
As for the separate patch for MatchRegisterName() I don't really understand how that can be accomplished using __attribute__((unused)) since the call should be completely avoid for only one particular target, Mips in this case. Shouldn't that be enclosed in
 some #ifdef statement?<br>
<br>
Vladimir<br>
<div style="font-family:Times New Roman; color:#000000; font-size:16px">
<hr tabindex="-1">
<div id="divRpF194253" style="direction:ltr"><font color="#000000" face="Tahoma" size="2"><b>From:</b> Jim Grosbach [grosbach@apple.com]<br>
<b>Sent:</b> Friday, July 13, 2012 7:03 PM<br>
<b>To:</b> Medic, Vladimir<br>
<b>Cc:</b> llvm-commits@cs.uiuc.edu; Jovanovic, Petar<br>
<b>Subject:</b> Re: [llvm-commits] Mips Assembler stubs for llvm<br>
</font><br>
</div>
<div></div>
<div>Hi Vladimir,
<div><br>
</div>
<div>This is great. I'm looking forward to seeing the mips assembler take shape.</div>
<div><br>
</div>
<div>Any explicit checks for target names in the TableGen tool itself are a bad idea[*]. We should probably just add __attribute__((unused)) to that function so you can ignore it if it's not suitable to the target's needs. That should be done as a separate
 patch.</div>
<div><br>
</div>
<div>Other than that, the only things I see that should be adjusted are mild style things. </div>
<div>* Function naming is starting w/ lower case, camel-case afterwards (for those not already part of the MC API). For example, "ParseMemOperand" should be "parseMemOperand". </div>
<div>* For the time being, I'd also add "llvm_unreachable("unimplemented!")" in the empty implementations of things like addExpr(), et. al.. </div>
<div>* The class "MipsOperand" should be "MipsAsmOperand" for both clarity and consistency with other targets.</div>
<div>* "MipsAmParserVariant" has a type and should be "MipsAsmParserVariant"</div>
<div><br>
</div>
<div>Regards,</div>
<div>  Jim</div>
<div><br>
</div>
<div><br>
</div>
<div>* Yes, ARM and x86 both do that. Those are horrible hacks that should be removed.</div>
<div><br>
</div>
<div><br>
</div>
<div>
<div>
<div>On Jul 13, 2012, at 8:36 AM, "Medic, Vladimir" <<a href="mailto:vmedic@mips.com" target="_blank">vmedic@mips.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div style="font-family:Helvetica; font-size:medium; font-style:normal; font-variant:normal; font-weight:normal; letter-spacing:normal; line-height:normal; orphans:2; text-indent:0px; text-transform:none; white-space:normal; widows:2; word-spacing:0px">
<div style="direction:ltr; font-family:Tahoma; font-size:10pt">Hi all,<br>
please find attached the patch that adds stubbed methods for mips assembly matcher. It does not create a working assembler but it enables the tableGen to generate MipsGenAsmMatcher.inc. The actual implementation of assembler will be provided in a separate patch
 for easier review.<br>
The critical part of this patch is a change in AsmMatcherEmitter.cpp. As per following discussion<a href="http://comments.gmane.org/gmane.comp.compilers.llvm.devel/47243" target="_blank">http://comments.gmane.org/gmane.comp.compilers.llvm.devel/47243</a>  we
 have disabled the emission of MatchRegisterName() for Mips platform. As we found no example on how a feature should be disabled for specific platform we based our solution on target name. Suggestions on this matter are most welcome.<br>
<br>
We are looking forward to your comments and remarks<br>
<br>
Kind regards<br>
<br>
Vladimir<br>
</div>
<span><MipsAssemblerStubs.patch></span>_______________________________________________<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>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>