[llvm-commits] Mips Assembler stubs for llvm

[Medic, Vladimir]

Hi all,
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.

Vladimir
________________________________
From: Medic, Vladimir
Sent: Monday, July 16, 2012 3:06 PM
To: Jim Grosbach
Cc: llvm-commits at cs.uiuc.edu; Jovanovic, Petar
Subject: RE: [llvm-commits] Mips Assembler stubs for llvm

Hi Jim,
thank you for your feedback. I have attached a new patch that includes your remarks.
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?

Vladimir
________________________________
From: Jim Grosbach [grosbach at apple.com]
Sent: Friday, July 13, 2012 7:03 PM
To: Medic, Vladimir
Cc: llvm-commits at cs.uiuc.edu; Jovanovic, Petar
Subject: Re: [llvm-commits] Mips Assembler stubs for llvm

Hi Vladimir,

This is great. I'm looking forward to seeing the mips assembler take shape.

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.

Other than that, the only things I see that should be adjusted are mild style things.
* 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".
* For the time being, I'd also add "llvm_unreachable("unimplemented!")" in the empty implementations of things like addExpr(), et. al..
* The class "MipsOperand" should be "MipsAsmOperand" for both clarity and consistency with other targets.
* "MipsAmParserVariant" has a type and should be "MipsAsmParserVariant"

Regards,
  Jim


* Yes, ARM and x86 both do that. Those are horrible hacks that should be removed.


On Jul 13, 2012, at 8:36 AM, "Medic, Vladimir" <vmedic at mips.com<mailto:vmedic at mips.com>> wrote:

Hi all,
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.
The critical part of this patch is a change in AsmMatcherEmitter.cpp. As per following discussionhttp://comments.gmane.org/gmane.comp.compilers.llvm.devel/47243  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.

We are looking forward to your comments and remarks

Kind regards

Vladimir
<MipsAssemblerStubs.patch>_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120716/b8d2d79a/attachment.html>