Re: [RFC PATCH v2 7/7] powerpc/selftest: reuse ppc-opcode macros to avoid redundancy

[Balamuruhan S <bala24@linux.ibm.com>]

On Thu, 2020-04-30 at 17:27 +0530, Naveen N. Rao wrote:
> Michael Ellerman wrote:
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> > > Michael Ellerman wrote:
> > > > Balamuruhan S <bala24@linux.ibm.com> writes:
> > > > > Avoid redefining macros to encode ppc instructions instead reuse it
> > > > > from
> > > > > ppc-opcode.h, Makefile changes are necessary to compile memcmp_64.S
> > > > > with
> > > > > __ASSEMBLY__ defined from selftests.
> > > > > 
> > > > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > > > > ---
> > > > >  .../selftests/powerpc/stringloops/Makefile    | 34 ++++++++++++++---
> > > > > -
> > > > >  .../powerpc/stringloops/asm/asm-const.h       |  1 +
> > > > >  .../powerpc/stringloops/asm/ppc-opcode.h      | 36 +--------------
> > > > > ----
> > > > >  3 files changed, 29 insertions(+), 42 deletions(-)
> > > > >  create mode 120000
> > > > > tools/testing/selftests/powerpc/stringloops/asm/asm-const.h
> > > > >  mode change 100644 => 120000
> > > > > tools/testing/selftests/powerpc/stringloops/asm/ppc-opcode.h
> > > > > 
> > > > > diff --git a/tools/testing/selftests/powerpc/stringloops/Makefile
> > > > > b/tools/testing/selftests/powerpc/stringloops/Makefile
> > > > > index 7fc0623d85c3..efe76c5a5b94 100644
> > > > > --- a/tools/testing/selftests/powerpc/stringloops/Makefile
> > > > > +++ b/tools/testing/selftests/powerpc/stringloops/Makefile
> > > > > @@ -1,26 +1,44 @@
> > > > >  # SPDX-License-Identifier: GPL-2.0
> > > > >  # The loops are all 64-bit code
> > > > > -CFLAGS += -I$(CURDIR)
> > > > > +GIT_VERSION = $(shell git describe --always --long --dirty || echo
> > > > > "unknown")
> > > > > +CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CURDIR)
> > > > > -I$(CURDIR)/../include
> > > > >  
> > > > >  EXTRA_SOURCES := ../harness.c
> > > > >  
> > > > >  build_32bit = $(shell if ($(CC) $(CFLAGS) -m32 -o /dev/null memcmp.c
> > > > > >/dev/null 2>&1) then echo "1"; fi)
> > > > >  
> > > > > +ifneq ($(build_32bit),1)
> > > > >  TEST_GEN_PROGS := memcmp_64 strlen
> > > > > +TEST_GEN_FILES := memcmp.o memcmp_64.o memcmp_64
> > > > > +MEMCMP := $(OUTPUT)/memcmp.o
> > > > > +MEMCMP_64 := $(OUTPUT)/memcmp_64.o
> > > > > +HARNESS :=  $(OUTPUT)/../harness.o
> > > > > +CFLAGS += -m64 -maltivec
> > > > >  
> > > > > -$(OUTPUT)/memcmp_64: memcmp.c
> > > > > -$(OUTPUT)/memcmp_64: CFLAGS += -m64 -maltivec
> > > > > +OVERRIDE_TARGETS := 1
> > > > > +include ../../lib.mk
> > > > >  
> > > > > -ifeq ($(build_32bit),1)
> > > > > +$(OUTPUT)/memcmp_64: $(MEMCMP_64) $(MEMCMP) $(HARNESS)
> > > > > +	$(CC) $(CFLAGS) memcmp.o memcmp_64.o ../harness.o -o memcmp_64
> > > > > +
> > > > > +$(MEMCMP_64): memcmp_64.S
> > > > > +	$(CC) $(CFLAGS) -D__ASSEMBLY__ -o memcmp_64.o -c memcmp_64.S
> > > > > +
> > > > > +$(MEMCMP): memcmp.c
> > > > > +	$(CC) $(CFLAGS) -o memcmp.o -c memcmp.c
> > > > > +
> > > > > +$(HARNESS): $(EXTRA_SOURCES)
> > > > > +	$(CC) $(CFLAGS) -DGIT_VERSION='"$(GIT_VERSION)"' -o
> > > > > ../harness.o -c $(EXTRA_SOURCES)
> > > > 
> > > > What are you actually trying to do here? Is it just that you need to
> > > > define __ASSEMBLY__ for memcmp_64.S?
> > > 
> > > Adding __ASSEMBLY__ while building memcmp_64.S would be the goal, so as 
> > > to reuse ppc-opcode.h. However, asm/ppc-opcode.h under stringloops test 
> > > is tiny and doesn't seem to justify the change.

Okay, I will drop the last patch that have changes for stringloops Makefile.

make and make clean is not working from inside stringloops directory which is
fixed with this change.


> > 
> > I don't see ppc-opcode.h testing __ASSEMBLY__ though, so I don't think
> > we even need to define it?
> 
> Right -- it's rather 'stringify_in_c' which tests it. 'asm/ppc-opcode.h' 
> under stringloops/ unconditionally defines 'stringify_in_c' this way:
> 	#  define stringify_in_c(...)   __VA_ARGS__ 
> 

It is expecting __ASSEMBLY__ through ppc-opcode.h -> asm-const.h to raw encode
the instruction in assembly file instead to stringify it for c file. we observe
this Assembler messages without defining __ASSEMBLY__,

memcmp_64.S: Assembler messages:
memcmp_64.S:473: Error: unknown pseudo-op: `.long (0x100000c7 | (((0) & 0x1f)
<< 21) | (((0) & 0x1f) << 16) | (((1) & 0x1f) << 11) | (0x1 << 10))'
memcmp_64.S:477: Error: unknown pseudo-op: `.long (0x100000c7 | (((0) & 0x1f)
<< 21) | (((0) & 0x1f) << 16) | (((1) & 0x1f) << 11) | (0x1 << 10))'
memcmp_64.S:586: Error: unknown pseudo-op: `.long (0x10000006 | (((7) & 0x1f)
<< 21) | (((9) & 0x1f) << 16) | (((10) & 0x1f) << 11) | (0x1 << 10))'
memcmp_64.S:607: Error: unknown pseudo-op: `.long (0x10000006 | (((7) & 0x1f)
<< 21) | (((9) & 0x1f) << 16) | (((10) & 0x1f) << 11) | (0x1 << 10))'
memcmp_64.S:616: Error: unknown pseudo-op: `.long (0x10000006 | (((7) & 0x1f)
<< 21) | (((9) & 0x1f) << 16) | (((10) & 0x1f) << 11) | (0x1 << 10))'
make[1]: *** [../../lib.mk:148:
/home/bala/linux/tools/testing/selftests/powerpc/stringloops/memcmp_64] Error 1

-- Bala
> 
> - Naveen
>