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

From: Michael Ellerman <[email-masked]>
Date: Thu, 30 Apr 2020 12:41:01 +1000

"Naveen N. Rao" <[email-masked]> writes:
> Michael Ellerman wrote:
>> Balamuruhan S <[email-masked]> 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 <[email-masked]>
>>> ---
>>>  .../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?

cheers