Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excessive initialization in descriptor and completion_record #14

Open
vinser52 opened this issue Mar 29, 2022 · 4 comments
Open

Excessive initialization in descriptor and completion_record #14

vinser52 opened this issue Mar 29, 2022 · 4 comments

Comments

@vinser52
Copy link

The descriptor struct contains excessive bytes array initialization in default ctor. I believe the following code can be safely removed: https://github.com/intel/DML/blob/develop/include/dml/detail/common/types.hpp#L21-L24. The bytes array is zero-initialized.

The same is also applicable for completion_record.

I am not sure about performance impact - I hope compiler will optimize it out when compiled with -O2.

@EgorKupaev
Copy link
Contributor

Hi @vinser52, I'm not sure that the bytes array is initialized with zeroes in initialization list of a constructor. Especially when () are used. Could you point me to some reference or the standard's rule?

I'm 99% sure that the excessive initialization will be optimized away for -O2, but if there is no requirement from C++ standard to zero-initialize class' array members as if {} were used, then there might be a bug. I tried to be explicit.

@vinser52
Copy link
Author

Hi @EgorKupaev, you can refer to the following links:
https://en.cppreference.com/w/cpp/language/value_initialization
https://en.cppreference.com/w/cpp/language/zero_initialization

Value initialization is performed when a non-static data member or a base class is initialized using a [member initializer](https://en.cppreference.com/w/cpp/language/initializer_list) with an empty pair of parentheses or braces.
The effects of value initialization are:

1) if T is a class type with no [default constructor](https://en.cppreference.com/w/cpp/language/default_constructor) or with a user-provided or deleted [default constructor](https://en.cppreference.com/w/cpp/language/default_constructor), the object is [default-initialized](https://en.cppreference.com/w/cpp/language/default_initialization);
2) if T is a class type with a default constructor that is neither user-provided nor deleted (that is, it may be a class with an implicitly-defined or defaulted default constructor), the object is [zero-initialized](https://en.cppreference.com/w/cpp/language/zero_initialization) and the semantic constraints for default-initialization are checked, and if T has a non-trivial default constructor, the object is [default-initialized](https://en.cppreference.com/w/cpp/language/default_initialization);
3) if T is an array type, each element of the array is value-initialized;
4) otherwise, the object is [zero-initialized](https://en.cppreference.com/w/cpp/language/zero_initialization).

Also, I have created a simple test program:

#include <iostream>
#include <memory>
#include <cstring>
#include <algorithm>


using byte_t = std::uint8_t;

struct A {
    A() : bytes() {
#if 0
        for (auto& byte : bytes)
        {
            byte = 0u;
        }
#endif
    }

    byte_t bytes[64];
};

using allocator_type = std::allocator<A>;
using allocator_traits = std::allocator_traits<allocator_type>;

int main() {
  allocator_type alloc;
  A* pA = allocator_traits::allocate(alloc, 1);

  std::memset(pA, 5, sizeof(A));
  
  std::cout << std::all_of(pA->bytes, pA->bytes+64, [](byte_t byte) { return byte == 5; }) << std::endl;

  allocator_traits::construct(alloc, pA);

  std::cout << std::all_of(pA->bytes, pA->bytes+64, [](byte_t byte) { return byte == 0; }) << std::endl;

  return 0;
}

@mzhukova
Copy link
Contributor

mzhukova commented Apr 3, 2023

Hi @vinser52, as I can see this issue was opened a long time ago, so just checking in to see if it could be closed?

@vinser52
Copy link
Author

Hi @mzhukova, I think the issue I raised was not resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants