"Smith, Barry F." <bsm...@mcs.anl.gov> writes: >> On Oct 26, 2019, at 9:09 AM, Jed Brown <j...@jedbrown.org> wrote: >> >> "Smith, Barry F." <bsm...@mcs.anl.gov> writes: >> >>> The proposed fix is #if defined(PETSC_USE_AVX512_KERNELS) && && && && && >>> in https://gitlab.com/petsc/petsc/merge_requests/2213/diffs >> >> Looks fine; approved. >> >>> but note that PETSC_USE_AVX512_KERNELS does not even do a configure check >>> to make sure it is valid. The user has to guess that passing that flag will >>> work. Of course a proper configure test is needed and since a proper test >>> is needed it can handle all the issues in one place instead of having one >>> issue in configure and n - 1 in the source code. >> >> What are "all the issues"? 32-bit indices, precision=double, >> scalar=real? So we'll need 8 CPP macros that test each of those >> combinations? > > No, if suddenly there is support for single precision for example, the > developer would modify the configure test to turn on PETSC_USE_AVX512_KERNELS > for that additional case and not touch the source code at all;
1. We still need to distinguish code paths for single and double. They definitely have to touch the source code because double-precision intrinsics don't work on single-precision data. 2. The work is inevitably done incrementally so the developer needs a #if test to work one kernel at a time. It's error-prone to create a new preprocessor macro for testing and change it before submitting the MR. It also forces all single-precision support into a single commit that implements several kernels instead of just one. 3. "git pull && make libs" will fail for every user of master because they need to reconfigure. Testing the branch before merging becomes a many-minutes job instead of 10 seconds to recompile a few files and run a test. Switching back to 'master' after commenting/approval might require getting back the old macros, thus another reconfigure.
