On Tue, May 18, 2021 at 6:55 PM Honnappa Nagarahalli
<honnappa.nagaraha...@arm.com> wrote:
>
> <snip>
>
> > >>
> > >> On Thu, May 13, 2021 at 6:41 PM Chengwen Feng
> > >> <fengcheng...@huawei.com> wrote:
> > >>>
> > >>> Currently, the soc_kunpeng930 declares
> > >>> '-march=armv8.2-a+crypto+sve', but some compiler doesn't recognize
> > >>> the march because it doesn't support sve.
> > >>>
> > >>> To solve this bug we use the following scheme:
> > >>> 1. Define 'march_base' tuple which defines support march, it should
> > >>> arrange from lower to higher.
> > >>> e.g. 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'] 2. Define
> > >>> 'march_feature' tuple which defines support feature.
> > >>> e.g. 'march_feature' : ['crypto', 'sve'] 3. Select the most suitable
> > >>> march+feature combination based on 'march_base' and 'march_feature'
> > >>> tuples.
> > >>> 4. Use the selected march+feature combination as the default
> > >>> machine_args.
> > >>>
> > >>> Fixes: 7cf32a22b240 ("config/arm: add Hisilicon kunpeng")
> > >>>
> > >>> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
> > >>> ---
> > >>> config/arm/meson.build | 27 +++++++++++++++++++++++++--
> > >>> 1 file changed, 25 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > >>> 3f34ec9..8551a80 100644
> > >>> --- a/config/arm/meson.build
> > >>> +++ b/config/arm/meson.build
> > >>> @@ -149,7 +149,9 @@ implementer_hisilicon = {
> > >>> ],
> > >>> 'part_number_config': {
> > >>> '0xd01': {
> > >>> - 'machine_args': ['-march=armv8.2-a+crypto',
> > >>> '-mtune=tsv110'],
> > >>> + 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > > If the compiler supports armv8.1-a, you need to choose armv8.1-a.
> > >
> > >>
> > >> Another target has the same issue. Could you fix that all together as
> > >> it is generic problem in the existing infrastructure.
> > > I think this needs to be more generic solution. IMO, the requirement is as
> > follows:
> > >
> > > 1) We need to pick the most closest march_base supported by the
> > > compiler. For ex: If the SoC support v8.2 and the compiler supports
> > > v8.1, we need to pick v8.1
> > > 2) SoCs are allowed to support a base march version + hand picked features
> > from the next 1 base marchs. i.e. armv8.x compliant implementation can
> > include any features from armv8.(x + 1). Please refer to Arm-ARM section A2
> > for more details. So, it is possible that the compiler supports a base march
> > and a bunch of optional features from the next version. We need to test all
> > the features allowed by the architecture and pick the ones that are
> > supported
> > in the compiler.
> > >
> >
> > I try to add 'march_base' : ['-march=armv8-a', '-march=armv8.5-a'] to cn10k,
> > and then find it can't work with ['RTE_ARM_FEATURE_ATOMICS', true] when
> > using gcc7.2:
> > [268/2250] Compiling C object
> > lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> > FAILED: lib/librte_stack.a.p/stack_rte_stack_lf.c.o
> > ...
> > {standard input}: Assembler messages:
> > {standard input}:13: Error: selected processor does not support `caspl
> > x0,x1,x2,x3,[x5]'
> > [347/2250] Compiling C object
> > lib/librte_hash.a.p/hash_rte_cuckoo_hash.c.o
> > ninja: build stopped: subcommand failed.
> > make: *** [cn10k] Error 1
> > It seem can't simplely add '-march=armv8-a' in 'march_base'.
> > And it require a lot of testing to get the right 'march_base' and
> > 'march_feature' parameters.
> > So for v5, I just modify the kunpeng930's config which was well tested.
> >
> > I think the 'march_base' and 'march_feature' could well solve the gcc's
> > minor-
> > version problem.
> > Note: the minor-version means a few version which are closes, not big gap,
> > like gcc5.4 and gcc10.2
> >
> > For that old gcc which could not support the 'march' that defined in
> > 'machine_args' or 'march_base'
> > I think it better use the 'generic' profile else it will compile fail which
> > showed
> > above.
> >
> > So how about add new tuple: fallback_generic? eg:
> > '0xd02': {
> > 'march_base': ['-march=armv8.2-a'],
> > 'march_feature': ['crypto', 'sve'],
> > 'machine_args': [],
> > 'flags': [
> > ['RTE_MACHINE', '"Kunpeng 930"'],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > ['RTE_MAX_LCORE', 1280],
> > ['RTE_MAX_NUMA_NODES', 16]
> > ],
> > 'fallback_generic': true
> > }
> > PS: The premise is that the 'generic' profile is tested.
> Jerin, how big of a problem is this (i.e. having to compile the code with an
> older version of the compiler)? Is it just one old version of the compiler or
> there are several of them? Do they all need to be captured in the meson.build
> file? I am just trying to understand the need for a generic approach.
IMO, We are supporting from gcc 4.7. So a lot of combinations are possible.
>
> >
> >
> > >>
> > >>
> > >>> + 'march_feature' : ['crypto'],
> > >>> + 'machine_args': ['-mtune=tsv110'],
> > >>> 'flags': [
> > >>> ['RTE_MACHINE', '"Kunpeng 920"'],
> > >>> ['RTE_ARM_FEATURE_ATOMICS', true], @@ -158,7 +160,9
> > >>> @@ implementer_hisilicon = {
> > >>> ]
> > >>> },
> > >>> '0xd02': {
> > >>> - 'machine_args': ['-march=armv8.2-a+crypto+sve'],
> > >>> + 'march_base' : ['-march=armv8-a', '-march=armv8.2-a'],
> > >>> + 'march_feature' : ['crypto', 'sve'],
> > >>> + 'machine_args': [],
> > >>> 'flags': [
> > >>> ['RTE_MACHINE', '"Kunpeng 930"'],
> > >>> ['RTE_ARM_FEATURE_ATOMICS', true], @@ -449,8
> > >>> +453,27 @@ else
> > >>> # add/overwrite flags in the proper order
> > >>> dpdk_flags = flags_common + implementer_config['flags'] +
> > >>> part_number_config.get('flags', []) + soc_flags
> > >>>
> > >>> + # select the most suitable march+feature combination
> > >>> + machine_march = []
> > >>> + if part_number_config.has_key('march_base')
> > >>> + foreach arch: part_number_config['march_base']
> > >>> + if cc.has_argument(arch)
> > >>> + machine_march = arch # Set the higher supported
> > >>> + arch as
> > >> possible
> > >>> + endif
> > >>> + endforeach
> > >>> + endif
> > >>> + if part_number_config.has_key('march_feature')
> > >>> + foreach feature: part_number_config['march_feature']
> > >>> + tmp_machine_march = machine_march + '+' + feature
> > >>> + if cc.has_argument(tmp_machine_march)
> > >>> + machine_march = tmp_machine_march # Set the more
> > >>> + supported
> > >> feature as possible
> > >>> + endif
> > >>> + endforeach
> > >>> + endif
> > >>> +
> > >>> # apply supported machine args
> > >>> machine_args = [] # Clear previous machine args
> > >>> + machine_args += machine_march
> > >>> foreach flag: part_number_config['machine_args']
> > >>> if cc.has_argument(flag)
> > >>> machine_args += flag
> > >>> --
> > >>> 2.8.1
> > >>>
>
