[PATCH liburing 0/2] Changes for src/Makefile

[PATCH liburing 0/2] Changes for src/Makefile

[Alviro Iskandar Setiawan]

Hello sir,

This patchset changes src/Makefile. 2 patches here:

1. Remove -fomit-frame-pointer flag, because it's already covered
   by the -O2 optimization flag.

2. Add file dependencies for the object files. When the header
   files are modified, the compiled object are not going to be
   recompiled because the header files are not marked as dependency
   for the objects. Add those header files as dependency so it is
   safe not to do "make clean" after changing those files.

please review,
thx

Signed-off-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
---
Alviro Iskandar Setiawan (2):
  src/Makefile: Remove `-fomit-frame-pointer` from default build
  src/Makefile: Add header files as dependency

 src/Makefile | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)


base-commit: 6231f56da7881bde6fb011e1b54d672f8fe5a224
-- 
2.32.0

[PATCH liburing 1/2] src/Makefile: Remove `-fomit-frame-pointer` from default build

[Alviro Iskandar Setiawan]

-fomit-frame-pointer is already turned on by -O1 optimization flag.

The liburing default compilation uses -O2 optimization, -O2 turns on
all optimization flags specified by -O1. Therefore, we don't need to
specify -fomit-frame-pointer here. Remove it.

Link: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
Signed-off-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
---
 src/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile b/src/Makefile
index 3e1192f..f19d45e 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -8,7 +8,7 @@ libdevdir ?= $(prefix)/lib
 CPPFLAGS ?=
 override CPPFLAGS += -D_GNU_SOURCE \
 	-Iinclude/ -include ../config-host.h
-CFLAGS ?= -g -fomit-frame-pointer -O2 -Wall -Wextra -fno-stack-protector
+CFLAGS ?= -g -O2 -Wall -Wextra -fno-stack-protector
 override CFLAGS += -Wno-unused-parameter -Wno-sign-compare -DLIBURING_INTERNAL
 SO_CFLAGS=-fPIC $(CFLAGS)
 L_CFLAGS=$(CFLAGS)
-- 
2.32.0

[PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Alviro Iskandar Setiawan]

When the header files are modified, the compiled object are not going
to be recompiled because the header files are not marked as dependency
for the objects. Add those header files as dependency so it is safe
not to do "make clean" after changing those files.

Signed-off-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
---
 src/Makefile | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index f19d45e..b9428b7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -46,9 +46,16 @@ endif
 liburing_objs := $(patsubst %.c,%.ol,$(liburing_srcs))
 liburing_sobjs := $(patsubst %.c,%.os,$(liburing_srcs))
 
-$(liburing_srcs): syscall.h lib.h
-
-$(liburing_objs) $(liburing_sobjs): include/liburing/io_uring.h
+$(liburing_objs) $(liburing_sobjs): \
+	syscall.h \
+	lib.h \
+	arch/syscall-defs.h \
+	arch/x86/syscall.h \
+	arch/x86/lib.h \
+	arch/aarch64/syscall.h \
+	arch/generic/syscall.h \
+	arch/generic/lib.h \
+	include/liburing/io_uring.h
 
 %.os: %.c
 	$(QUIET_CC)$(CC) $(CPPFLAGS) $(SO_CFLAGS) -c -o $@ $<
@@ -78,8 +85,6 @@ ifeq ($(ENABLE_SHARED),1)
 	ln -sf $(relativelibdir)$(libname) $(libdevdir)/liburing.so
 endif
 
-$(liburing_objs): include/liburing.h
-
 clean:
 	@rm -f $(all_targets) $(liburing_objs) $(liburing_sobjs) $(soname).new
 	@rm -f *.so* *.a *.o
-- 
2.32.0

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Ammar Faizi]

On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> When the header files are modified, the compiled object are not going
> to be recompiled because the header files are not marked as dependency
> for the objects. Add those header files as dependency so it is safe
> not to do "make clean" after changing those files.
> 
> Signed-off-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
> ---
>   src/Makefile | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/src/Makefile b/src/Makefile
> index f19d45e..b9428b7 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -46,9 +46,16 @@ endif
>   liburing_objs := $(patsubst %.c,%.ol,$(liburing_srcs))
>   liburing_sobjs := $(patsubst %.c,%.os,$(liburing_srcs))
>   
> -$(liburing_srcs): syscall.h lib.h
> -
> -$(liburing_objs) $(liburing_sobjs): include/liburing/io_uring.h
> +$(liburing_objs) $(liburing_sobjs): \
> +	syscall.h \
> +	lib.h \
> +	arch/syscall-defs.h \
> +	arch/x86/syscall.h \
> +	arch/x86/lib.h \
> +	arch/aarch64/syscall.h \
> +	arch/generic/syscall.h \
> +	arch/generic/lib.h \
> +	include/liburing/io_uring.h

This is ugly, it blindly adds all of them to the dependency while
they're actually not dependencies for all the C files here. For
example, when compiling for x86, we don't touch aarch64 files.

It is not a problem for liburing at the moment, because we don't
have many files in the src directory now. But I think we better
provide a long term solution on this.

For the headers files, I think we should rely on the compilers to
generate the dependency list with something like:

    "-MT ... -MMD -MP -MF"

Then include the generated dependency list to the Makefile.

What do you think?

-- 
Ammar Faizi

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Alviro Iskandar Setiawan]

On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
> This is ugly, it blindly adds all of them to the dependency while
> they're actually not dependencies for all the C files here. For
> example, when compiling for x86, we don't touch aarch64 files.
>
> It is not a problem for liburing at the moment, because we don't
> have many files in the src directory now. But I think we better
> provide a long term solution on this.
>
> For the headers files, I think we should rely on the compilers to
> generate the dependency list with something like:
>
>     "-MT ... -MMD -MP -MF"
>
> Then include the generated dependency list to the Makefile.
>
> What do you think?

Yes, I think it's better to do that. I'll fix this in v2.
thx

-- Viro

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Alviro Iskandar Setiawan]

On Wed, Mar 9, 2022 at 6:06 AM Alviro Iskandar Setiawan wrote:
> On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
> > This is ugly, it blindly adds all of them to the dependency while
> > they're actually not dependencies for all the C files here. For
> > example, when compiling for x86, we don't touch aarch64 files.
> >
> > It is not a problem for liburing at the moment, because we don't
> > have many files in the src directory now. But I think we better
> > provide a long term solution on this.
> >
> > For the headers files, I think we should rely on the compilers to
> > generate the dependency list with something like:
> >
> >     "-MT ... -MMD -MP -MF"
> >
> > Then include the generated dependency list to the Makefile.
> >
> > What do you think?
>
> Yes, I think it's better to do that. I'll fix this in v2.
> thx

Sir, I am a bit confused with the include dependency files to the Makefile.

I use like this:

   -MT <object_filename> -MMD -MP -MF <dependency_file>

the dependency file is generated, but how to include them dynamically?
I think it shouldn't be included one by one.

So after this

   [...] -MT "setup.os" -MMD -MP -MF ".deps/setup.os.d" [...]
   [...] -MT "queue.os" -MMD -MP -MF ".deps/queue.os.d" [...]
   [...] -MT "register.os" -MMD -MP -MF ".deps/register.os.d" [...]
   [...] -MT "syscall.os" -MMD -MP -MF ".deps/syscall.os.d" [...]

files .deps/{setup,queue,registers,syscall}.os.d are generated, but I
have to include them to Makefile right? How to include them all at
once?

-- Viro

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Ammar Faizi]

On 3/9/22 7:23 AM, Alviro Iskandar Setiawan wrote:
> On Wed, Mar 9, 2022 at 6:06 AM Alviro Iskandar Setiawan wrote:
>> On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
>>> This is ugly, it blindly adds all of them to the dependency while
>>> they're actually not dependencies for all the C files here. For
>>> example, when compiling for x86, we don't touch aarch64 files.
>>>
>>> It is not a problem for liburing at the moment, because we don't
>>> have many files in the src directory now. But I think we better
>>> provide a long term solution on this.
>>>
>>> For the headers files, I think we should rely on the compilers to
>>> generate the dependency list with something like:
>>>
>>>      "-MT ... -MMD -MP -MF"
>>>
>>> Then include the generated dependency list to the Makefile.
>>>
>>> What do you think?
>>
>> Yes, I think it's better to do that. I'll fix this in v2.
>> thx
> 
> Sir, I am a bit confused with the include dependency files to the Makefile.
> 
> I use like this:
> 
>     -MT <object_filename> -MMD -MP -MF <dependency_file>
> 
> the dependency file is generated, but how to include them dynamically?
> I think it shouldn't be included one by one.
> 
> So after this
> 
>     [...] -MT "setup.os" -MMD -MP -MF ".deps/setup.os.d" [...]
>     [...] -MT "queue.os" -MMD -MP -MF ".deps/queue.os.d" [...]
>     [...] -MT "register.os" -MMD -MP -MF ".deps/register.os.d" [...]
>     [...] -MT "syscall.os" -MMD -MP -MF ".deps/syscall.os.d" [...]
> 
> files .deps/{setup,queue,registers,syscall}.os.d are generated, but I
> have to include them to Makefile right? How to include them all at
> once?

Untested, but I think you can do something like:

   -include $(liburing_objs:%=.deps/%.d)

where liburing_objs is the variable that contains:
{setup,queue,registers,syscall}.os.

-- 
Ammar Faizi

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Ammar Faizi]

On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> When the header files are modified, the compiled object are not going
> to be recompiled because the header files are not marked as dependency
> for the objects. Add those header files as dependency so it is safe
> not to do "make clean" after changing those files.

Another missing part is the test files, they should also be recompiled
when changes to files in src/ are made. With this change, they are not.

The same also for examples/.

-- 
Ammar Faizi

Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency

[Alviro Iskandar Setiawan]

On Wed, Mar 9, 2022 at 5:59 AM Ammar Faizi wrote:
> On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> > When the header files are modified, the compiled object are not going
> > to be recompiled because the header files are not marked as dependency
> > for the objects. Add those header files as dependency so it is safe
> > not to do "make clean" after changing those files.
>
> Another missing part is the test files, they should also be recompiled
> when changes to files in src/ are made. With this change, they are not.
>
> The same also for examples/.

oc oc, that will be separate patches. Will be done in v2.

-- Viro