On 08/23/2011 02:30 AM, Phil Blundell wrote:
> On Mon, 2011-08-22 at 14:51 -0700, Saul Wold wrote:
>> From: Darren Hart <dvh...@linux.intel.com>
>>
>> Enable machines or distros to select the hard floating point abi for cortexa8
>> machines. I left out the arm7a thumb+neon combinations as they were not
>> present in the original non-hf set.
>>
>> Signed-off-by: Darren Hart <dvh...@linux.intel.com>
>> CC: Jason Kridner <jkrid...@beagleboard.org>
>> CC: Koen Kooi <k...@dominion.thruhere.net>
>> ---
>> meta/conf/machine/include/tune-cortexa8.inc | 16 +++++++++++++---
>> 1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/conf/machine/include/tune-cortexa8.inc
>> b/meta/conf/machine/include/tune-cortexa8.inc
>> index 67c5f0b..edd1717 100644
>> --- a/meta/conf/machine/include/tune-cortexa8.inc
>> +++ b/meta/conf/machine/include/tune-cortexa8.inc
>> @@ -5,12 +5,22 @@ require conf/machine/include/arm/arch-armv7a.inc
>> TUNEVALID[cortexa8] = "Enable Cortex-A8 specific processor optimizations"
>> TUNE_CCARGS += "${@bb.utils.contains("TUNE_FEATURES", "cortexa8",
>> "-mtune=cortex-a8", "", d)}"
>>
>> -AVAILTUNES += "cortexa8 cortexa8t"
>> +# Little Endian base configs
>> +AVAILTUNES += "cortexa8 cortexa8t cortexa8-neon"
>> TUNE_FEATURES_tune-cortexa8 = "${TUNE_FEATURES_tune-armv7a} cortexa8"
>> TUNE_FEATURES_tune-cortexa8t = "${TUNE_FEATURES_tune-armv7at} cortexa8"
>> -TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-cortexa8} neon"
>> -
>> +TUNE_FEATURES_tune-cortexa8-neon = "${TUNE_FEATURES_tune-armv7a-neon}
>> cortexa8"
>> PACKAGE_EXTRA_ARCHS_tune-cortexa8 = "${PACKAGE_EXTRA_ARCHS_tune-armv7a}"
>> PACKAGE_EXTRA_ARCHS_tune-cortexa8t = "${PACKAGE_EXTRA_ARCHS_tune-armv7at}"
>> PACKAGE_EXTRA_ARCHS_tune-cortexa8-neon =
>> "${PACKAGE_EXTRA_ARCHS_tune-armv7a-neon}"
>
> This part of the patch doesn't seem to match any of the description in
> the checkin comment.
True, this one was a change to make the assignments parallel to
each-other. I could submit this as a separate patch if people feel
strongly on the subject.
>
>> +# VFP Tunes
>> +AVAILTUNES += "cortexa8hf cortexa8thf cortexa8hf-neon"
>> +TUNE_FEATURES_tune-cortexa8hf ?= "${TUNE_FEATURES_tune-armv7ahf} cortexa8"
>> +TUNE_FEATURES_tune-cortexa8thf ?= "${TUNE_FEATURES_tune-armv7athf} cortexa8"
>> +TUNE_FEATURES_tune-cortexa8hf-neon ?= "${TUNE_FEATURES_tune-armv7ahf-neon}
>> cortexa8"
>> +PACKAGE_EXTRA_ARCHS_tune-cortexa8hf = "${PACKAGE_EXTRA_ARCHS_tune-armv7ahf}"
>> +PACKAGE_EXTRA_ARCHS_tune-cortexa8thf =
>> "${PACKAGE_EXTRA_ARCHS_tune-armv7athf}"
>> +PACKAGE_EXTRA_ARCHS_tune-cortexa8hf-neon =
>> "${PACKAGE_EXTRA_ARCHS_tune-armv7ahf-neon}"
>
> I don't think "VFP Tunes" is a very good description of these, since it
> might lead folks to think that they need to enable these tunes to get
> vfp instructions (which is untrue: the vfp ISA is controlled by the
> "vfp" tune feature and this is set for both armv6 and armv7a
> automatically). If they are hard-float ABI tunes, which seems to be the
> case, then let's call them that.
I just copied this labeling across from the arm7a tune file.
Consistently wrong is still wrong however :-)
>
> Also, on a broader issue, I continue to feel that adding more and more
> terms to what is already quite a large cross product is not a very good
> way to proceed. There's nothing very cortexa8 specific about the
> hardfloat API and, to do this comprehensively, we'd have to add an -hf
> version of every existing tune definition which has vfp enabled. I
> think it would be better to find a way to express this sort of thing
> which captures the orthogonality of the different options and avoids the
> need to write out every permutation in longhand.
I had a similar thought while writing it up. For the current release
cycle, I won't have time to try to come up with a new way to express
these. This approach is consistent with the existing tune descriptions.
Can we agree that a better method is needed while allowing this in to
get this support into testers hands?
> Also also, on a more tangential note, I remain skeptical that the
> hardfloat ABI is actually a useful thing to support. I'd be interested
> to see any benchmark results which demonstrate that it's worthwhile.
I'm not sold on it either, but this lowers the barrier to people testing
with hardfp. I think having the ability to enable it is a good thing,
even if it is disabled by default.
I will respin this series once we come to a consensus on the concerns
Khem and Phil have raised.
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
