[PATCH] Use ".arch_extension" ARM directive to specify the additional CPU features

Sumanth Gundapaneni sgundapa at codeaurora.org
Sun Feb 8 09:15:11 PST 2015


================
Comment at: include/llvm/MC/MCStreamer.h:142
@@ -141,2 +141,3 @@
   virtual void emitArch(unsigned Arch);
+  virtual void emitArchExtension(StringRef StringValue);
   virtual void emitObjectArch(unsigned Arch);
----------------
t.p.northover wrote:
> sgundapa wrote:
> > sgundapa wrote:
> > > t.p.northover wrote:
> > > > rengolin wrote:
> > > > > sgundapa wrote:
> > > > > > t.p.northover wrote:
> > > > > > > I think this either needs a generic implementation, or (probably better for now) moving to ARMELFStreamer.cpp entirely. OK, so it's not called, but 
> > > > > > p
> > > > > ARMELFStreamer? But this is assembly only...
> > > > Yep, but it's essentially en ELF-dialect assembly feature so the AsmStreamer seems to live in the same file.
> > > A quick google on ".arch_extension" suggests that this directive is specific to ARM as.
> > > It is fair to move it to the ARMELFStreamer.cpp file at this moment
> > > 
> > Ignore my previous comment as I thought I added the member to MCTargetStreamer.
> > emitArchExtension is part of ARMTargetStreamer. My code is based on below comments in MCStreamer.h
> > Any thing wrong here ?
> > 
> > /// If target foo wants to use this, it should implement 3 classes:
> > /// * FooTargetStreamer : public MCTargetStreamer
> > /// * FooTargetAsmStreamer : public FooTargetStreamer
> > /// * FooTargetELFStreamer : public FooTargetStreamer
> > ///
> > /// FooTargetStreamer should have a pure virtual method for each directive. For
> > /// example, for a ".bar symbol_name" directive, it should have
> > /// virtual emitBar(const MCSymbol &Symbol) = 0;
> > ///
> > /// The FooTargetAsmStreamer and FooTargetELFStreamer classes implement the
> > /// method. The assembly streamer just prints ".bar symbol_name". The object
> > /// streamer does whatever is needed to implement .bar in the object file.
> Oh bother, looks like I was confused by what classes lived where. I assumed that because it was in MCStreamer.h, the definition belonged to "class MCStreamer". Sorry about that, you can ignore my comment.
No problem. Your comments made me go through MCStreamer more deeper.

================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:644
@@ -643,3 +643,1 @@
 
-  // FIXME: remove krait check when GNU tools support krait cpu
-  if (CPUString != "generic" && CPUString != "krait")
----------------
rengolin wrote:
> t.p.northover wrote:
> > Any news on this, by the way? It's rather a horrific hack to have to carry, and it only seems to be getting worse.
> You know, I've been thinking... With the number of third-party ARM CPUs out there, maybe it would be a better model to emit .cpu as a core ARM name, and use .arch_extension to all little differences, for all other cores.
> 
> Of course, "if (Krait)" is not what I'm suggesting, but maybe a spec of each CPU somewhere, possible extracted from TableGen files, could go a long way of doing this right and transparent...
>  to emit .cpu as a core ARM name, and use .arch_extension to all little differences, for all other cores
This is exactly what I am trying to implement here.

> possible extracted from TableGen files, could go a long way of doing this right and transparent.
I second your opinion except that I have no clue on how to implement it. This is my first patch to LLVM :). I need a lot more time to go through most of the components of LLVM

http://reviews.llvm.org/D7316

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list