Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
On Thu, Feb 15, 2024 at 05:16:01PM +, Alexandru Elisei wrote: > Hi Drew, > > On Thu, Feb 15, 2024 at 05:32:22PM +0100, Andrew Jones wrote: > > On Thu, Feb 15, 2024 at 04:05:56PM +, Alexandru Elisei wrote: > > > Hi Drew, > > > > > > On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote: > > > > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote: > > > > > From: Alexandru Elisei > > > > > > > > > > There are 25 header files today (found with grep -r "#ifndef > > > > > __ASSEMBLY__) > > > > > with functionality relies on the __ASSEMBLY__ prepocessor constant > > > > > being > > > > > correctly defined to work correctly. So far, kvm-unit-tests has > > > > > relied on > > > > > the assembly files to define the constant before including any header > > > > > files which depend on it. > > > > > > > > > > Let's make sure that nobody gets this wrong and define it as a > > > > > compiler > > > > > constant when compiling assembly files. __ASSEMBLY__ is now defined > > > > > for all > > > > > .S files, even those that didn't set it explicitely before. > > > > > > > > > > Reviewed-by: Nikos Nikoleris > > > > > Reviewed-by: Andrew Jones > > > > > Signed-off-by: Alexandru Elisei > > > > > Signed-off-by: Shaoqin Huang > > > > > --- > > > > > Makefile | 5 - > > > > > arm/cstart.S | 1 - > > > > > arm/cstart64.S | 1 - > > > > > powerpc/cstart64.S | 1 - > > > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > index 602910dd..27ed14e6 100644 > > > > > --- a/Makefile > > > > > +++ b/Makefile > > > > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes > > > > > -Wstrict-prototypes > > > > > > > > > > autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d > > > > > > > > > > +AFLAGS = $(CFLAGS) > > > > > +AFLAGS += -D__ASSEMBLY__ > > > > > + > > > > > LDFLAGS += -nostdlib $(no_pie) -z noexecstack > > > > > > > > > > $(libcflat): $(cflatobjs) > > > > > @@ -113,7 +116,7 @@ directories: > > > > > @mkdir -p $(OBJDIRS) > > > > > > > > > > %.o: %.S > > > > > - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< > > > > > + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< > > > > > > > > I think we can drop the two hunks above from this patch and just rely on > > > > the compiler to add __ASSEMBLY__ for us when compiling assembly files. > > > > > > I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I > > > missing something? > > > > > > [1] > > > https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__ > > > > You're right. I'm not opposed to changing all the __ASSEMBLY__ references > > to __ASSEMBLER__. I'll try to do that at some point unless you beat me to > > it. > > Actually, I quite prefer the Linux style of using __ASSEMBLY__ instead of > __ASSEMBLER__, because it makes reusing Linux files easier. That, and the > habit formed by staring at Linux assembly files. Those are good arguments and also saves the churn. OK, let's keep this patch and __ASSEMBLY__ Thanks, drew
Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
Hi Drew, On Thu, Feb 15, 2024 at 05:32:22PM +0100, Andrew Jones wrote: > On Thu, Feb 15, 2024 at 04:05:56PM +, Alexandru Elisei wrote: > > Hi Drew, > > > > On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote: > > > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote: > > > > From: Alexandru Elisei > > > > > > > > There are 25 header files today (found with grep -r "#ifndef > > > > __ASSEMBLY__) > > > > with functionality relies on the __ASSEMBLY__ prepocessor constant being > > > > correctly defined to work correctly. So far, kvm-unit-tests has relied > > > > on > > > > the assembly files to define the constant before including any header > > > > files which depend on it. > > > > > > > > Let's make sure that nobody gets this wrong and define it as a compiler > > > > constant when compiling assembly files. __ASSEMBLY__ is now defined for > > > > all > > > > .S files, even those that didn't set it explicitely before. > > > > > > > > Reviewed-by: Nikos Nikoleris > > > > Reviewed-by: Andrew Jones > > > > Signed-off-by: Alexandru Elisei > > > > Signed-off-by: Shaoqin Huang > > > > --- > > > > Makefile | 5 - > > > > arm/cstart.S | 1 - > > > > arm/cstart64.S | 1 - > > > > powerpc/cstart64.S | 1 - > > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Makefile b/Makefile > > > > index 602910dd..27ed14e6 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes > > > > -Wstrict-prototypes > > > > > > > > autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d > > > > > > > > +AFLAGS = $(CFLAGS) > > > > +AFLAGS += -D__ASSEMBLY__ > > > > + > > > > LDFLAGS += -nostdlib $(no_pie) -z noexecstack > > > > > > > > $(libcflat): $(cflatobjs) > > > > @@ -113,7 +116,7 @@ directories: > > > > @mkdir -p $(OBJDIRS) > > > > > > > > %.o: %.S > > > > - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< > > > > + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< > > > > > > I think we can drop the two hunks above from this patch and just rely on > > > the compiler to add __ASSEMBLY__ for us when compiling assembly files. > > > > I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I > > missing something? > > > > [1] > > https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__ > > You're right. I'm not opposed to changing all the __ASSEMBLY__ references > to __ASSEMBLER__. I'll try to do that at some point unless you beat me to > it. Actually, I quite prefer the Linux style of using __ASSEMBLY__ instead of __ASSEMBLER__, because it makes reusing Linux files easier. That, and the habit formed by staring at Linux assembly files. Thanks, Alex
Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
On Thu, Feb 15, 2024 at 04:05:56PM +, Alexandru Elisei wrote: > Hi Drew, > > On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote: > > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote: > > > From: Alexandru Elisei > > > > > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__) > > > with functionality relies on the __ASSEMBLY__ prepocessor constant being > > > correctly defined to work correctly. So far, kvm-unit-tests has relied on > > > the assembly files to define the constant before including any header > > > files which depend on it. > > > > > > Let's make sure that nobody gets this wrong and define it as a compiler > > > constant when compiling assembly files. __ASSEMBLY__ is now defined for > > > all > > > .S files, even those that didn't set it explicitely before. > > > > > > Reviewed-by: Nikos Nikoleris > > > Reviewed-by: Andrew Jones > > > Signed-off-by: Alexandru Elisei > > > Signed-off-by: Shaoqin Huang > > > --- > > > Makefile | 5 - > > > arm/cstart.S | 1 - > > > arm/cstart64.S | 1 - > > > powerpc/cstart64.S | 1 - > > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index 602910dd..27ed14e6 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes > > > -Wstrict-prototypes > > > > > > autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d > > > > > > +AFLAGS = $(CFLAGS) > > > +AFLAGS += -D__ASSEMBLY__ > > > + > > > LDFLAGS += -nostdlib $(no_pie) -z noexecstack > > > > > > $(libcflat): $(cflatobjs) > > > @@ -113,7 +116,7 @@ directories: > > > @mkdir -p $(OBJDIRS) > > > > > > %.o: %.S > > > - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< > > > + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< > > > > I think we can drop the two hunks above from this patch and just rely on > > the compiler to add __ASSEMBLY__ for us when compiling assembly files. > > I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I > missing something? > > [1] > https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__ You're right. I'm not opposed to changing all the __ASSEMBLY__ references to __ASSEMBLER__. I'll try to do that at some point unless you beat me to it. Thanks, drew
Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
Hi Drew, On Mon, Jan 15, 2024 at 01:44:17PM +0100, Andrew Jones wrote: > On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote: > > From: Alexandru Elisei > > > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__) > > with functionality relies on the __ASSEMBLY__ prepocessor constant being > > correctly defined to work correctly. So far, kvm-unit-tests has relied on > > the assembly files to define the constant before including any header > > files which depend on it. > > > > Let's make sure that nobody gets this wrong and define it as a compiler > > constant when compiling assembly files. __ASSEMBLY__ is now defined for all > > .S files, even those that didn't set it explicitely before. > > > > Reviewed-by: Nikos Nikoleris > > Reviewed-by: Andrew Jones > > Signed-off-by: Alexandru Elisei > > Signed-off-by: Shaoqin Huang > > --- > > Makefile | 5 - > > arm/cstart.S | 1 - > > arm/cstart64.S | 1 - > > powerpc/cstart64.S | 1 - > > 4 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 602910dd..27ed14e6 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes > > -Wstrict-prototypes > > > > autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d > > > > +AFLAGS = $(CFLAGS) > > +AFLAGS += -D__ASSEMBLY__ > > + > > LDFLAGS += -nostdlib $(no_pie) -z noexecstack > > > > $(libcflat): $(cflatobjs) > > @@ -113,7 +116,7 @@ directories: > > @mkdir -p $(OBJDIRS) > > > > %.o: %.S > > - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< > > + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< > > I think we can drop the two hunks above from this patch and just rely on > the compiler to add __ASSEMBLY__ for us when compiling assembly files. I think the precompiler adds __ASSEMBLER__, not __ASSEMBLY__ [1]. Am I missing something? [1] https://gcc.gnu.org/onlinedocs/cpp/macros/predefined-macros.html#c.__ASSEMBLER__ Thanks, Alex > > Thanks, > drew > > > > > -include */.*.d */*/.*.d > > > > diff --git a/arm/cstart.S b/arm/cstart.S > > index 3dd71ed9..b24ecabc 100644 > > --- a/arm/cstart.S > > +++ b/arm/cstart.S > > @@ -5,7 +5,6 @@ > > * > > * This work is licensed under the terms of the GNU LGPL, version 2. > > */ > > -#define __ASSEMBLY__ > > #include > > #include > > #include > > diff --git a/arm/cstart64.S b/arm/cstart64.S > > index bc2be45a..a8ad6dc8 100644 > > --- a/arm/cstart64.S > > +++ b/arm/cstart64.S > > @@ -5,7 +5,6 @@ > > * > > * This work is licensed under the terms of the GNU GPL, version 2. > > */ > > -#define __ASSEMBLY__ > > #include > > #include > > #include > > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > > index 34e39341..fa32ef24 100644 > > --- a/powerpc/cstart64.S > > +++ b/powerpc/cstart64.S > > @@ -5,7 +5,6 @@ > > * > > * This work is licensed under the terms of the GNU LGPL, version 2. > > */ > > -#define __ASSEMBLY__ > > #include > > #include > > #include > > -- > > 2.40.1 > >
Re: [kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
On Thu, Nov 30, 2023 at 04:07:03AM -0500, Shaoqin Huang wrote: > From: Alexandru Elisei > > There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__) > with functionality relies on the __ASSEMBLY__ prepocessor constant being > correctly defined to work correctly. So far, kvm-unit-tests has relied on > the assembly files to define the constant before including any header > files which depend on it. > > Let's make sure that nobody gets this wrong and define it as a compiler > constant when compiling assembly files. __ASSEMBLY__ is now defined for all > .S files, even those that didn't set it explicitely before. > > Reviewed-by: Nikos Nikoleris > Reviewed-by: Andrew Jones > Signed-off-by: Alexandru Elisei > Signed-off-by: Shaoqin Huang > --- > Makefile | 5 - > arm/cstart.S | 1 - > arm/cstart64.S | 1 - > powerpc/cstart64.S | 1 - > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index 602910dd..27ed14e6 100644 > --- a/Makefile > +++ b/Makefile > @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes > -Wstrict-prototypes > > autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d > > +AFLAGS = $(CFLAGS) > +AFLAGS += -D__ASSEMBLY__ > + > LDFLAGS += -nostdlib $(no_pie) -z noexecstack > > $(libcflat): $(cflatobjs) > @@ -113,7 +116,7 @@ directories: > @mkdir -p $(OBJDIRS) > > %.o: %.S > - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< > + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< I think we can drop the two hunks above from this patch and just rely on the compiler to add __ASSEMBLY__ for us when compiling assembly files. Thanks, drew > > -include */.*.d */*/.*.d > > diff --git a/arm/cstart.S b/arm/cstart.S > index 3dd71ed9..b24ecabc 100644 > --- a/arm/cstart.S > +++ b/arm/cstart.S > @@ -5,7 +5,6 @@ > * > * This work is licensed under the terms of the GNU LGPL, version 2. > */ > -#define __ASSEMBLY__ > #include > #include > #include > diff --git a/arm/cstart64.S b/arm/cstart64.S > index bc2be45a..a8ad6dc8 100644 > --- a/arm/cstart64.S > +++ b/arm/cstart64.S > @@ -5,7 +5,6 @@ > * > * This work is licensed under the terms of the GNU GPL, version 2. > */ > -#define __ASSEMBLY__ > #include > #include > #include > diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S > index 34e39341..fa32ef24 100644 > --- a/powerpc/cstart64.S > +++ b/powerpc/cstart64.S > @@ -5,7 +5,6 @@ > * > * This work is licensed under the terms of the GNU LGPL, version 2. > */ > -#define __ASSEMBLY__ > #include > #include > #include > -- > 2.40.1 >
[kvm-unit-tests PATCH v1 01/18] Makefile: Define __ASSEMBLY__ for assembly files
From: Alexandru Elisei There are 25 header files today (found with grep -r "#ifndef __ASSEMBLY__) with functionality relies on the __ASSEMBLY__ prepocessor constant being correctly defined to work correctly. So far, kvm-unit-tests has relied on the assembly files to define the constant before including any header files which depend on it. Let's make sure that nobody gets this wrong and define it as a compiler constant when compiling assembly files. __ASSEMBLY__ is now defined for all .S files, even those that didn't set it explicitely before. Reviewed-by: Nikos Nikoleris Reviewed-by: Andrew Jones Signed-off-by: Alexandru Elisei Signed-off-by: Shaoqin Huang --- Makefile | 5 - arm/cstart.S | 1 - arm/cstart64.S | 1 - powerpc/cstart64.S | 1 - 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 602910dd..27ed14e6 100644 --- a/Makefile +++ b/Makefile @@ -92,6 +92,9 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes autodepend-flags = -MMD -MP -MF $(dir $*).$(notdir $*).d +AFLAGS = $(CFLAGS) +AFLAGS += -D__ASSEMBLY__ + LDFLAGS += -nostdlib $(no_pie) -z noexecstack $(libcflat): $(cflatobjs) @@ -113,7 +116,7 @@ directories: @mkdir -p $(OBJDIRS) %.o: %.S - $(CC) $(CFLAGS) -c -nostdlib -o $@ $< + $(CC) $(AFLAGS) -c -nostdlib -o $@ $< -include */.*.d */*/.*.d diff --git a/arm/cstart.S b/arm/cstart.S index 3dd71ed9..b24ecabc 100644 --- a/arm/cstart.S +++ b/arm/cstart.S @@ -5,7 +5,6 @@ * * This work is licensed under the terms of the GNU LGPL, version 2. */ -#define __ASSEMBLY__ #include #include #include diff --git a/arm/cstart64.S b/arm/cstart64.S index bc2be45a..a8ad6dc8 100644 --- a/arm/cstart64.S +++ b/arm/cstart64.S @@ -5,7 +5,6 @@ * * This work is licensed under the terms of the GNU GPL, version 2. */ -#define __ASSEMBLY__ #include #include #include diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S index 34e39341..fa32ef24 100644 --- a/powerpc/cstart64.S +++ b/powerpc/cstart64.S @@ -5,7 +5,6 @@ * * This work is licensed under the terms of the GNU LGPL, version 2. */ -#define __ASSEMBLY__ #include #include #include -- 2.40.1
