[dpdk-stable] [dpdk-dev] [PATCH 1/2] mk: fix unsupported flag error on armhf architercture

[Phil Yang (Arm Technology China)]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Tuesday, November 12, 2019 2:03 PM
> To: Phil Yang (Arm Technology China) <Phil.Yang at arm.com>
> Cc: Bruce Richardson <bruce.richardson at intel.com>; dev at dpdk.org;
> david.marchand at redhat.com; jerinj at marvell.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli at arm.com>; Gavin Hu (Arm Technology China)
> <Gavin.Hu at arm.com>; Joyce Kong (Arm Technology China)
> <Joyce.Kong at arm.com>; nd <nd at arm.com>; stable at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] mk: fix unsupported flag error on armhf
> architercture
> 
> 12/11/2019 06:25, Phil Yang (Arm Technology China):
> > From: Bruce Richardson <bruce.richardson at intel.com>
> > > On Mon, Nov 11, 2019 at 06:34:20PM +0800, Phil Yang wrote:
> > > > The older version (e.g. version 7.4.0 ) of GNU C compiler for the armhf
> > > > architecture doesn't support the flag '-Wno-address-of-packed-
> member',
> > > > so remove this flag for aarch32.
> > > >
> > > > Fixes: a385972c3675 ("mk: disable warning for packed member pointer")
> > > > Cc: stable at dpdk.org
> > > >
> > > > Signed-off-by: Phil Yang <phil.yang at arm.com>
> > > > Reviewed-by: Gavin Hu <gavin.hu at arm.com>
> > > > Tested-by: Joyce Kong <joyce.kong at arm.com>
> > > >
> > > > ---
> > > >  mk/toolchain/gcc/rte.vars.mk | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/mk/toolchain/gcc/rte.vars.mk
> b/mk/toolchain/gcc/rte.vars.mk
> > > > index 9fc7041..ec0cbbf 100644
> > > > --- a/mk/toolchain/gcc/rte.vars.mk
> > > > +++ b/mk/toolchain/gcc/rte.vars.mk
> > > > @@ -100,7 +100,9 @@ WERROR_FLAGS += -Wno-format-truncation
> > > >  endif
> > > >
> > > >  # disable packed member unalign warnings
> > > > +ifneq ($(CONFIG_RTE_ARCH_ARM), y)
> > > >  WERROR_FLAGS += -Wno-address-of-packed-member
> > > > +endif
> > > >
> > > You don't need to do this, as gcc will not complain about this unknown
> flag
> > > unless you have other issues in your code.[1] I think it's better to keep
> the
> > > code clean in this case, otherwise we'll have the code littered with
> > > conditionals for various flags.
> > >
> > > /Bruce
> > >
> > > [1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
> > > "When an unrecognized warning option is requested (e.g.,
> > > -Wunknown-warning), GCC emits a diagnostic stating that the option is
> not
> > > recognized. However, if the -Wno- form is used, the behavior is slightly
> > > different: no diagnostic is produced for -Wno-unknown-warning unless
> other
> > > diagnostics are being produced. This allows the use of new -Wno- options
> > > with old compilers, but if something goes wrong, the compiler warns that
> an
> > > unrecognized option is present."
> >
> > Thanks, Bruce.
> >
> > There are thousands of warnings with alignment when compiling for
> ARMv7 (ARMv7 supports unaligned memory access).
> > Without this new flag, the build system works fine for ARMv7.  So I add this
> conditional for ARMv7 only.
> > I agree with you we should clean up the code, but it needs a lot of effort to
> achieve that.
> > This patch is going to make the ARMv7 target build successfully during this
> period.
> 
> I would go the opposite direction.
> I think we should remove all warnings disablement like "-Wno-something",
> and clean the code.

Thanks, Thomas.

I think it is a compiler issue rather than a code defect in this case, as It cannot recognize the '-Waddress-of-packed-member' flag. 
"arm-linux-gnueabihf-gcc: error: unrecognized command line option '-Waddress-of-packed-member'"

> 
> If you want to allow building ARMv7, you can just allow warnings as not
> errors.
> So we will still see something is wrong but it won't prevent from building.

Yes. We did this already, all the alignment issues are emitted as warnings on ARMv7. 
Please see these two commits:
79fa5c2a9992 ("mk: ignore alignment errors for ARMv7")
6433c19105e8 ("mk: generalize strict alignment warning handling")

> 
>