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

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

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.
> 
> 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__ 


- Naveen