[CRIU] [PATCH v2] s390: Move -msoft-float/-fno-optimize-sibling-calls into compel Makefiles

[Dmitry Safonov]

On 07/21/2017 02:49 PM, Michael Holzheu wrote:
> We currently define "CFLAGS += -msoft-float -fno-optimize-sibling-calls"
> in "Makefile.compel".
> 
> Makefile.compel is included into the toplevel Makefile and so changes
> CFLAGS which are exported to all criu and zdtm Makefiles.
> 
> We must not use -msoft-float outside the compel files. E.g. otherwise for
> zdtm we get the following build error:
> 
> uptime_grow.o: In function `main':
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__muldf3'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__floatdidf'
> /tmp/2/criu/test/zdtm/static/uptime_grow.c:36: undefined reference to `__adddf3
> 
> Fix this and move the CFLAGS definition to the compel Makefile.
> 
> As compel/plugins/Makefile and compel/Makefile are called recursively,
> changed CFLAGS will not be exported back to the top Makefile.
> 
> Reported-by: Adrian Reber <areber at redhat.com>
> Signed-off-by: Michael Holzheu <holzheu at linux.vnet.ibm.com>

Hm, I'm not against the patch, but it sprays this CFLAGS addition across
sources..
What do you think, can we have this in per-arch zone in the top
Makefile?

Something like:
 > --- a/Makefile
 > +++ b/Makefile
 > @@ -70,6 +70,7 @@ ifeq ($(ARCH),s390)
 >          SRCARCH                := s390
 >          VDSO           := y
 >          DEFINES                := -DCONFIG_S390
 > +        CFLAGS_PIE     := -msoft-float -fno-optimize-sibling-calls
 >  endif
 >
 >  LDARCH ?= $(SRCARCH)

(with your that big comment). And add it to relevant
ccflags-y += CFLAGS_PIE

So we could fit it in one place and maybe other arch's have common
PIE CFLAGS, they need to use.

There is also a question inline below.

> ---
>   Makefile.compel           | 10 ----------
>   compel/Makefile           | 14 ++++++++++++++
>   compel/plugins/Makefile   | 14 ++++++++++++++
>   criu/pie/Makefile         |  8 +++++++-
>   criu/pie/Makefile.library |  9 ++++++++-
>   5 files changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/Makefile.compel b/Makefile.compel
> index 5c854e3..1ef7f8c 100644
> --- a/Makefile.compel
> +++ b/Makefile.compel
> @@ -70,13 +70,3 @@ compel/$(LIBCOMPEL_SO): compel/$(LIBCOMPEL_A)
>   compel-install-targets	+= compel/$(LIBCOMPEL_SO)
>   compel-install-targets	+= compel/compel
>   compel-install-targets	+= $(compel-plugins)
> -
> -#
> -# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
> -# (Global Offset Table) relocations with gcc compilers that don't have
> -# commit "S/390: Fix 64 bit sibcall".
> -#
> -ifeq ($(ARCH),s390)
> -CFLAGS += -msoft-float -fno-optimize-sibling-calls
> -HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls
> -endif
> diff --git a/compel/Makefile b/compel/Makefile
> index 43d27f5..81b9614 100644
> --- a/compel/Makefile
> +++ b/compel/Makefile
> @@ -34,6 +34,20 @@ CFLAGS			+= -DNO_RELOCS
>   HOSTCFLAGS		+= -DNO_RELOCS
>   endif
>   
> +#
> +# We assume that compel code does not change floating point registers.
> +# On s390 gcc uses fprs to cache gprs. Therefore disable floating point
> +# with -msoft-float.
> +#
> +# Also ensure with -fno-optimize-sibling-calls that we don't create GOT
> +# (Global Offset Table) relocations with gcc compilers that don't have
> +# commit "S/390: Fix 64 bit sibcall".
> +#
> +ifeq ($(ARCH),s390)
> +CFLAGS += -msoft-float -fno-optimize-sibling-calls
> +HOSTCFLAGS += -msoft-float -fno-optimize-sibling-calls

Hm, why is it needed in HOSTCFLAGS?
And why is it restricted in compel tool?

-- 
              Dmitry