DLTcollab / sse2neon

A translator from Intel SSE intrinsics to Arm/Aarch64 NEON implementation
MIT License
1.3k stars 208 forks source link

Auto-generated integrated test suite #72

Closed jserv closed 4 years ago

jserv commented 4 years ago

Once #69 is fully integrated, we can think of the generation of test suite. The rough flow:

  1. Developers provide a comprehensive list of intrinsics;
  2. Scripts generate the stub/skeleton for test cases;
  3. Developers provide the real test entries such as test_mm_set1_ps and parameter list (e.g. test_mm_set1_ps(mTestFloats[i]));
  4. Automated test system can scan and analyze the status of each test item;
jserv commented 4 years ago

biu-cybercenter-proj-sse2neon contains a partially modular test suite with templates.

afcidk commented 4 years ago

@jserv, I'm now working on this issue. After the discussion with @marktwtn, we decided to keep the tests cases in impl.cpp.

In #100, I've unified enum and getInstructionString using macros. However, it seems that we could not further merge runSingleTest into them (the enum and getInstructionString) since the parameters of the test functions are not the same.

I'm wondering if we could unify the parameters of test cases by only passing this pointer to the test cases. In this way, contributors are be expected to add test functions more easily with less modification to other parts of impl.cpp.

jserv commented 4 years ago

I'm wondering if we could unify the parameters of test cases by only passing this pointer to the test cases. In this way, contributors are be expected to add test functions more easily with less modification to other parts of impl.cpp.

Good idea. Can you show the proposed changes?

afcidk commented 4 years ago

The original runSingleTest function looks like this

result_t SSE2NEONTestImpl::runSingleTest(InstructionTest test, uint32_t i) 
{
  result_t ret = TEST_SUCCESS;
  switch(test) {
    case IT_MM_SETZERO_SI128:
      ret = test_mm_setzero_si128()
      break;
    // ..... many cases here
  }
}

If we make the parameters of the test functions the same, we can use macro to expand that long switch-case statement.

I'll explain the macros (integrating enum, getInstructionString and runSingleTest) below.

#define ENUM(C,c) IT_##C,
#define STR(C,c) #C, 
#define CASE(C,c)                    \
        case IT_##C:                 \
            ret = test##c(*this, i); \ 
            break;
#define TYPE_FOREACH(type)                        \
        type(MM_SETZERO_SI128, mm_setzero_si128), \
        type(MM_SETZERO_PS, mm_setzero_ps),       \
        .......

The enum is defined as

enum InstructionTest { TYPE_FOREACH(ENUM) IT_LAST };

The instructionString is defined as

const char *instructionString[] = {TYPE_FOREACH(STR)};

The runSingleTest is defined as

result_t SSE2NEONTestImpl::runSingleTest(InstructionTest test, uint32_t i)
{
  result_t ret = TEST_SUCCESS;
  switch (test) {
    TYPE_FOREACH(CASE)
    case IT_LAST:
        break;
  }
  return ret
}

All the intrinsics are specified centralized at the TYPE_FOREACH macro. Therefore, contributors only need to modify two places when they need to add test cases:

  1. Add the intrinsic in TYPE_FOREACH macro
  2. Implement the test function in test_xxx(const SSE2NEONTestImpl &impl, uint32_t i) fashion.
jserv commented 4 years ago

How about eliminating capital identifiers such as MM_SETZERO_SI128? Thus, we can use # and/or ## to simplify macros.

afcidk commented 4 years ago

Then the enum names would be it_mm_setzero_si128 instead.