Remove n_face_orientations template argument within FEEvaluation #10946
Conversation
| template bool dealii::internal::FEFaceEvaluationFactory< | ||
| deal_II_dimension, | ||
| deal_II_scalar_vectorized::value_type, | ||
| deal_II_scalar_vectorized>:: | ||
| gather_evaluate<1>( | ||
| const unsigned int, | ||
| const deal_II_scalar_vectorized::value_type *, | ||
| const MatrixFreeFunctions::ShapeInfo<deal_II_scalar_vectorized> &, | ||
| const MatrixFreeFunctions::DoFInfo &, | ||
| deal_II_scalar_vectorized *, | ||
| deal_II_scalar_vectorized *, | ||
| deal_II_scalar_vectorized *, | ||
| const bool, | ||
| const bool, | ||
| const unsigned int, | ||
| const unsigned int, | ||
| const std::array<unsigned int, 1>, | ||
| const std::array<unsigned int, 1>, | ||
| const unsigned int, | ||
| const MatrixFreeFunctions::DoFInfo::DoFAccessIndex, | ||
| const std::array<unsigned int, 1>, | ||
| const Table<2, unsigned int> &); | ||
|
|
||
| template bool dealii::internal::FEFaceEvaluationFactory< | ||
| deal_II_dimension, | ||
| deal_II_scalar_vectorized::value_type, | ||
| deal_II_scalar_vectorized>:: | ||
| integrate_scatter<1>( | ||
| const unsigned int, | ||
| deal_II_scalar_vectorized::value_type *, | ||
| const MatrixFreeFunctions::ShapeInfo<deal_II_scalar_vectorized> &, | ||
| const MatrixFreeFunctions::DoFInfo &, | ||
| deal_II_scalar_vectorized *, | ||
| deal_II_scalar_vectorized *, | ||
| deal_II_scalar_vectorized *, | ||
| deal_II_scalar_vectorized *, | ||
| const bool, | ||
| const bool, | ||
| const unsigned int, | ||
| const unsigned int, | ||
| const std::array<unsigned int, 1>, | ||
| const std::array<unsigned int, 1>, | ||
| const unsigned int, | ||
| const MatrixFreeFunctions::DoFInfo::DoFAccessIndex, | ||
| const std::array<unsigned int, 1>, | ||
| const Table<2, unsigned int> &); |
There was a problem hiding this comment.
The key goal of this PR is get rid of these. Once we have introduced a InternalData the code should become significant simpler.
| if (n_face_orientations == VectorizedArrayType::size()) | ||
| return fe_face_evaluation_process_and_io<VectorizedArrayType::size()>( | ||
| p); | ||
| else | ||
| return fe_face_evaluation_process_and_io<1>(p); |
There was a problem hiding this comment.
Here is now the if-statement.
|
@kronbichler Any idea what to do with the warning? |
|
The compiler gets confused by the |
peterrum
force-pushed
the
n_face_orientations
branch
2 times, most recently
from
d5bb865
to
6ae56b2
Compare
| // we allocate small amounts of data on the stack to signal the compiler | ||
| // that this temporary data is only needed for the calculations but the | ||
| // final results can be discarded and need not be written back to | ||
| // memory. For large sizes or when the dofs per face is not a | ||
| // compile-time constant, however, we want to go to the heap in the | ||
| // `scratch_data` variable to not risk a stack overflow. | ||
| constexpr unsigned int stack_array_size_threshold = 100; | ||
|
|
||
| VectorizedArrayType *DEAL_II_RESTRICT temp1; | ||
| VectorizedArrayType | ||
| temp_data[static_dofs_per_face < stack_array_size_threshold ? | ||
| 2 * dofs_per_face : | ||
| 1]; | ||
| if (static_dofs_per_face < stack_array_size_threshold) | ||
| temp1 = &temp_data[0]; | ||
| else | ||
| temp1 = scratch_data; | ||
| VectorizedArrayType *temp1 = scratch_data; |
There was a problem hiding this comment.
@kronbichler Is there a reason why not to do this? You have done similar changes recently at other places.
There was a problem hiding this comment.
In the other function we could not keep it because the allocation depended on the number of components which was turned to a run time argument. Here we still can, so the comment given above is still alive. The question is of course whether compilers got better during the last three years and can avoid some unnecessary write-back (they often write back even more than just the final result). Did you check the number of instructions executed or the run time with this change on and off?
There was a problem hiding this comment.
Did you remove this code to make the compilation warning disappear? In any case, I believe we can restructure the code a bit to maybe avoid the warning (after this has been merged to avoid conflicts).
There was a problem hiding this comment.
Indeed, the last about 10 pushes are related to warnings all over the place...
peterrum
force-pushed
the
n_face_orientations
branch
2 times, most recently
from
a740034
to
fd50917
Compare
Conflicts: include/deal.II/matrix_free/evaluation_kernels.h
peterrum
force-pushed
the
n_face_orientations
branch
from
fd50917
to
3248660
Compare
Follow up to #10927. References #10928. Only the last commit is relevant.
@kronbichler I think this is what we have talked about.