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

[Renato Golin]

================
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:
> 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 
ARMELFStreamer? But this is assembly only...

================
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")
----------------
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...

================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:682
@@ +681,3 @@
+  // Enable hwdiv through ".arch_extension idiv"
+  if (Subtarget->isKrait() && (Subtarget->hasDivide()
+      || Subtarget->hasDivideInARMMode()))
----------------
sgundapa wrote:
> rengolin wrote:
> > echristo wrote:
> > > rengolin wrote:
> > > > shouldn't you check here based on the instruction set you're using? Something like:
> > > > 
> > > >     if (Subtarget->isKrait() && (
> > > >         Subtarget->hasDivide() ||
> > > >         (!Subtarget->isThumb() && Subtarget->hasDivideInARMMode())
> > > >       )
> > > > 
> > > > To make sure we don't turn it on when it's thumb and it only has div on ARM.
> > > Using subtargets here is not going to be allowed as soon as I can separate the code out so I'd prefer you come up with another way of figuring out the ISA here.
> > > 
> > > Honestly a loop over all functions in the module and collecting cpu information is probably how we're going to need to go and should be simple. I'm not sure how ARM attribute merging works between functions so if someone could detail that it would be appreciated.
> > ARM has no function specific attribute in EABI. The only thing we do is in the assembler just for the sake of accepting otherwise invalid instructions (bump up support) for the rare case where you're pretty sure no one will call that function without support for that instruction. This has mainly two users: ifunc and unwinders.
> > 
> > The point here is to work around a GAS deficiency in representing "krait" as an A9+DIV in a *global* context. So, your function specific attribute will either not exist, or be replicated to all functions, whatever is easier for you.
> > 
> > Regarding getting the ISA, I was wrong, and we should use the build attributes anyway.
> Guys, I am bit confused here what needs to be done.
> 
> Renato,  "hasDivide()" checks for thumb2 and "hasDivideInARMMode()" checks for arm mode, the two modes where hwdiv is allowed.
> Or, I am not understanding what you meant ?
> 
> ARMAsmPrinter.cpp has loads of references to Subtarget. Wouldn't it be a good idea to clean them in a single go ?
> Renato, "hasDivide()" checks for thumb2 and "hasDivideInARMMode()" checks for arm mode, the two modes where hwdiv is allowed.

That's what I meant, so it should be ok.

> ARMAsmPrinter.cpp has loads of references to Subtarget. Wouldn't it be a good idea to clean them in a single go ?

Yes, I don't think this is work for this patch.

http://reviews.llvm.org/D7316

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