fma relu combination for convolution-output

[Jokeren]

Thanks for your review!

[Maratyszcza]

Thanks Keren. Looks good, but needs few changes:

1. Please use activation parameter of enum type rather than a bool. In the future, we may want to support other activation types, not just ReLU. I.e.

enum nnp_activation {
    nnp_activation_identity = 0,
    nnp_activation_relu = 1,
};

enum nnp_status nnp_convolution_output(
    enum nnp_convolution_algorithm algorithm,
    size_t batch_size,
    size_t input_channels,
    size_t output_channels,
    struct nnp_size input_size,
    struct nnp_padding input_padding,
    struct nnp_size kernel_size,
    const float input[],
    const float kernel[],
    const float bias[],
    float output[],
    enum nnp_activation activation,
    pthreadpool_t threadpool,
    struct nnp_profile* profile);

1. Please generate separate FFT functions with ReLU, e.g. nnp_ifft8x8_with_bias_with_relu__avx2 instead of argument for inverse FFT functions. Also, not that if you have arg_relu parameter, if arg_relu: statement does not generate assembly instructions to check if arg_relu is non-zero, rather it checks if the Python variable arg_relu evaluates to True.
2. Please use tabs for indentation in C/C++ sources.

[Maratyszcza]

It is preferable to use VMAXPS(ymm_data_lo, ymm_data_lo, ymm_zero) for performance reasons (VBLENDPS may generate multiple microoperations)

[Maratyszcza]

Please use negative signed zero, i.e. Constant.float32x8(-0.0)

[Jokeren]

Could you please tell me the reason for using negative signed zero? Or propose a simple example?

[Maratyszcza]

This is for the backward pass. The backward pass needs to know which values were positive/negative before we applied ReLU. Using negative zero ensures that the sign of the convolution results doesn't change after we apply ReLU. See discussion in #24 for why its important.

[Jokeren]

Sorry that I set tab=8 space previously so that the style is not consistent with yours.

I have changed the format, and it looks just fine.

I will fix other functions later.

Regarding the psimd implementations, I do not have a mac machine. Then, how to test them after adding the relu function?

[Maratyszcza]

You don't need a Mac to test psimd functions, any Linux machine with Clang would work. Just do python configure.py --enable-psimd

[Jokeren]

Hi, @Maratyszcza !

Please view my latest commits to see whether the structures and format meet your requirement.

A problem about testing is proposed in the issues.

[Maratyszcza]

Is the order correct? In include/nnpack.h activation is the second argument

[Maratyszcza]

nnp_convolution_output should return an error code if activation has unknown value. I suggest to check activation inside validate_convolution_arguments. Then in these switch statements you write NNP_UNREACHABLE; to indicate that this case never happens. Compiler will use it for optimization.

[Maratyszcza]

Looks good. Once the tests are working I will merge.

[Jokeren]

Hi, @Maratyszcza , all tests are working!

I am sorry that my former commit logs are not in correct format.

I used [-0.1, 1] uniform distribution for convolution tests.

[bhack]

Any news?

[Maratyszcza]

@bhack This PR is ready to merge, but first NNPACK is moving to a new configuration system.

[Jokeren]

Anything I can help? @Maratyszcza

[Maratyszcza]

@Jokeren There are reports of a bug in nnp_fully_connected_output #43 #44. Could you take a look?

[Maratyszcza]

Manually rebased, merged, and committed as 4dbf75d