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

Submitted by Michael Holzheu on July 21, 2017, 11:49 a.m.

Details

Message ID 20170721134943.02c50e8b@TP-holzheu
State New
Series "s390: Move -msoft-float/-fno-optimize-sibling-calls into compel Makefiles"
Headers show

Commit Message

Michael Holzheu July 21, 2017, 11:49 a.m.
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@redhat.com>
Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 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(-)

Patch hide | download patch | download mbox

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
+endif
+
 obj-y			+= src/main.o
 obj-y			+= arch/$(ARCH)/src/lib/handle-elf.o
 obj-y			+= src/lib/handle-elf.o
diff --git a/compel/plugins/Makefile b/compel/plugins/Makefile
index 7127425..948348f 100644
--- a/compel/plugins/Makefile
+++ b/compel/plugins/Makefile
@@ -8,6 +8,20 @@  PLUGIN_ARCH_DIR		:= compel/arch/$(ARCH)/plugins
 # CFLAGS, ASFLAGS, LDFLAGS
 
 #
+# 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
+endif
+
+#
 # UAPI inclusion, referred as <compel/...>
 ccflags-y		+= -I compel/include/uapi
 asflags-y		+= -I compel/include/uapi
diff --git a/criu/pie/Makefile b/criu/pie/Makefile
index 76c3535..1f844cc 100644
--- a/criu/pie/Makefile
+++ b/criu/pie/Makefile
@@ -16,11 +16,17 @@  ifeq ($(SRCARCH),arm)
 	ccflags-y	+= -marm
 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 ($(SRCARCH),s390)
-	ccflags-y	+= -msoft-float
+	ccflags-y	+= -msoft-float -fno-optimize-sibling-calls
 endif
 
 asflags-y	+= -D__ASSEMBLY__
diff --git a/criu/pie/Makefile.library b/criu/pie/Makefile.library
index ceadc1d..a532dd3 100644
--- a/criu/pie/Makefile.library
+++ b/criu/pie/Makefile.library
@@ -41,9 +41,16 @@  ccflags-y		+= $(COMPEL_UAPI_INCLUDES)
 ifeq ($(SRCARCH),arm)
 	ccflags-y	+= -marm
 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 ($(SRCARCH),s390)
-	ccflags-y	+= -msoft-float
+	ccflags-y	+= -msoft-float -fno-optimize-sibling-calls
 endif

Comments

Dmitry Safonov July 21, 2017, 1:23 p.m.
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@redhat.com>
> Signed-off-by: Michael Holzheu <holzheu@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?
Michael Holzheu July 21, 2017, 3:24 p.m.
Am Fri, 21 Jul 2017 16:23:31 +0300
schrieb Dmitry Safonov <dsafonov@virtuozzo.com>:

> 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@redhat.com>
> > Signed-off-by: Michael Holzheu <holzheu@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.

This sounds like a very good idea - I will send a v3 patch.

> 
> 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?

Probably I did not understand the meaning of HOSTCFLAGS and just
did it the same way as in:

ifneq ($(filter arm aarch64,$(ARCH)),)
CFLAGS                  += -DNO_RELOCS
HOSTCFLAGS              += -DNO_RELOCS
endif

But looks like it is not required.

So what is the meaning for HOSTCFLAGS?

Michael
Dmitry Safonov July 21, 2017, 3:37 p.m.
On 07/21/2017 06:24 PM, Michael Holzheu wrote:
> Am Fri, 21 Jul 2017 16:23:31 +0300
> schrieb Dmitry Safonov <dsafonov@virtuozzo.com>:
> 
>> 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@redhat.com>
>>> Signed-off-by: Michael Holzheu <holzheu@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.
> 
> This sounds like a very good idea - I will send a v3 patch.

Thanks :)

> 
>>
>> 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?
> 
> Probably I did not understand the meaning of HOSTCFLAGS and just
> did it the same way as in:
> 
> ifneq ($(filter arm aarch64,$(ARCH)),)
> CFLAGS                  += -DNO_RELOCS
> HOSTCFLAGS              += -DNO_RELOCS
> endif
> 
> But looks like it is not required.
> 
> So what is the meaning for HOSTCFLAGS?

Well, HOSTCFLAGS are meant to be used by cross-compile make.
Some people build criu on x86 for arm embeded devices.
So, this Makefile defines CFLAGS for compel binary tool,
which prepares pie blobs. For cross-compilation HOSTCFLAGS are
used for building compel tool which is used on the host during build.

I'm not sure why there should be -msoft-float or
-fno-optimize-sibling-calls here, as they applyies to compel binary,
meant to be run during building. Maybe there is a reason, but adding
it into HOSTCFLAGS looks pure wrong to me.

NO_RELOCS compile definition is for building compel binary, which
shouldn't handle relocations. It's used ATM for arm/aarch64 pie's,
for which there isn't any code in compel to handle relocs (yet).
Maybe name is a bit misleading.
Michael Holzheu July 21, 2017, 5:14 p.m.
Am Fri, 21 Jul 2017 18:37:25 +0300
schrieb Dmitry Safonov <dsafonov@virtuozzo.com>:

> On 07/21/2017 06:24 PM, Michael Holzheu wrote:
> > Am Fri, 21 Jul 2017 16:23:31 +0300
> > schrieb Dmitry Safonov <dsafonov@virtuozzo.com>:
> > 
> >> 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@redhat.com>
> >>> Signed-off-by: Michael Holzheu <holzheu@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.
> > 
> > This sounds like a very good idea - I will send a v3 patch.
> 
> Thanks :)
> 
> > 
> >>
> >> 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?
> > 
> > Probably I did not understand the meaning of HOSTCFLAGS and just
> > did it the same way as in:
> > 
> > ifneq ($(filter arm aarch64,$(ARCH)),)
> > CFLAGS                  += -DNO_RELOCS
> > HOSTCFLAGS              += -DNO_RELOCS
> > endif
> > 
> > But looks like it is not required.
> > 
> > So what is the meaning for HOSTCFLAGS?
> 
> Well, HOSTCFLAGS are meant to be used by cross-compile make.
> Some people build criu on x86 for arm embeded devices.
> So, this Makefile defines CFLAGS for compel binary tool,
> which prepares pie blobs. For cross-compilation HOSTCFLAGS are
> used for building compel tool which is used on the host during build.

Makes sense.

> 
> I'm not sure why there should be -msoft-float or
> -fno-optimize-sibling-calls here, as they applyies to compel binary,
> meant to be run during building. Maybe there is a reason, but adding
> it into HOSTCFLAGS looks pure wrong to me.

Yes I agree.
 
Michael