<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"><base href="x-msg://18215/"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">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">vmedic@mips.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div ocsi="0" fpstyle="1" 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-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 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">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></body></html>