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

[Eric Christopher]

On Wed Feb 04 2015 at 1:53:13 PM Sumanth Gundapaneni <
sgundapa at codeaurora.org> wrote:

> ================
> Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:682
> @@ +681,3 @@
> +  // Enable hwdiv through ".arch_extension idiv"
> +  if (Subtarget->isKrait() && (Subtarget->hasDivide()
> +      || Subtarget->hasDivideInARMMode()))
> ----------------
> 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 ?
>

Are you offering to do it? ;)

And no, only the ones that involve module level stuff need to be changed.
AsmPrinter is a MachineFunction pass and so can set the subtarget per
function - that way other things can use it when they need (and there are
needs on a per function basis).

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150205/115cba44/attachment.html>