[llvm-commits] Mips Assembler stubs for llvm

[Medic, Vladimir]

Hi all,
I made some more investigation into Jim's proposal regarding using  __attribute__((unused)) to avoid call to emitMatchRegisterName () for mips target, and it seems to me that the call should be enclosed in something like
#if (!defined) __mips__
...
#endif
and the function itself narked with attribute unused to suppress the warnings.
Anyway, as Jim said it should be done in a separate patch is it OK to commit the current patch as is?

Thanks

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

Hi all,
please find attached the updated version of the patch for Mips assembler stub version. It clears a trailing white space error from the previous patch.
Looking forward to your comments on it.

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

Hi all,
I have checked the other targets and the AsmMatcherEmitter.cpp as well:
/// emitValidateOperandClass - Emit the function to validate an operand class.
static void emitValidateOperandClass(AsmMatcherInfo &Info,
                                     raw_ostream &OS) {
  OS << "static unsigned validateOperandClass(MCParsedAsmOperand *GOp, "
     << "MatchClassKind Kind) {\n";
  OS << "  " << Info.Target.getName() << "Operand &Operand = *("
     << Info.Target.getName() << "Operand*)GOp;\n";

It is obvious that it expects _Target_Operand class name rather then _Target_AsmOperand. The patch with this change is attached to this email.

Regards

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

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  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/20120727/e624098d/attachment.html>