[PATCH] [ARM] Implement eabi_attribute, cpu, and fpu directives.

Logan Chien tzuhsiang.chien at gmail.com
Fri Oct 25 09:28:03 PDT 2013



================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:472
@@ +471,3 @@
+  setAttributeItem(Attribute, Value);
+}
+void ARMTargetELFStreamer::emitFPU(StringRef FPU) {
----------------
Renato Golin wrote:
> Logan Chien wrote:
> > Renato Golin wrote:
> > > These two functions look redundant. Is it just in ARM's case?
> > Sorry.  I don't understand what do you mean by redundant.  These are virtual functions used by the AsmPrinter and AsmParser.  May you further describe your question?  Thanks.
> It might just be my lack of contact with this code for a while, but this looks like it's just adding two aliases to the overloaded "setAttributeItem" function.
> 
> My question was related to the need to have this extra level of indirection if they're just calling another function, or if it was better to, either expose the other two, or rename them to "emitAttribute" and "emitTextAttribute".
> 
> This may not be the case with other back-ends, so I'm not sure my question is at all relevant.
To make the code cleaner, I have slightly change this to:

setAttributeItem(Attribute, Value, /* OverwriteExisting= */ true);

So that it will be clear that this is not a simple delegation.  BTW, I wrote setAttributeItem() because I wish to ignore the eabi_attribute request if it has been emitted.  IMO, it is not a good idea to add more argument to emitAttibute(), because the OverwriteExisting option is meaningless to the ARMTargetAsmStreamer.  Hoping this can answer your question.


http://llvm-reviews.chandlerc.com/D1987



More information about the llvm-commits mailing list