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

#10471: Fixed GCC13 compile time issue #10473

Merged
merged 6 commits into from
Jul 19, 2024
Merged

#10471: Fixed GCC13 compile time issue #10473

merged 6 commits into from
Jul 19, 2024

Conversation

dmakoviichuk-tt
Copy link
Contributor

@dmakoviichuk-tt dmakoviichuk-tt commented Jul 19, 2024

Ticket

Link to Github Issue

Problem description

Provide context for the problem.

What's changed

  1. First compile error is no comments:
   return ttnn::multiply(nr_term, scalar, std::nullopt);
   scalar.deallocate();
  1. Second compile error fixed:
    Describe the approach used to solve the problem.
    Summarize the changes made and its impact.
    return {std::move(program), .override_runtime_arguments_callback = override_runtime_arguments_callback}; doesn't compile.
    This return {.program = std::move(program), .override_runtime_arguments_callback = override_runtime_arguments_callback}; or return {std::move(program), override_runtime_arguments_callback}; will.

  2. added a few casts from int to the uint32_t. I don't like that we need that casts, better to fix it in the function parameter but it might require to fix it in a lot of places which depend on this functions.

Checklist

  • Post commit CI passes
  • Model regression CI testing passes (if applicable)
  • New/Existing tests provide coverage for changes

@dmakoviichuk-tt dmakoviichuk-tt changed the title Fixed Initializer designed list issue #10471: Fixed GCC13 compile time issue Jul 19, 2024
@marty1885
Copy link
Contributor

marty1885 commented Jul 19, 2024

Thanks for the help! Based on your branch, I also need the following diff to make it compile. May you add it to your PR?

diff --git a/tt_eager/tt_dnn/op_library/moreh_norm_backward/moreh_norm_backward.cpp b/tt_eager/tt_dnn/op_library/moreh_norm_backward/moreh_norm_backward.cpp
index d03acdffdc..e99e8b339c 100644
--- a/tt_eager/tt_dnn/op_library/moreh_norm_backward/moreh_norm_backward.cpp
+++ b/tt_eager/tt_dnn/op_library/moreh_norm_backward/moreh_norm_backward.cpp
@@ -211,7 +211,7 @@ operation::ProgramWithCallbacks moreh_norm_backward_(
         "writer_moreh_norm_backward.cpp";
 
     std::vector<uint32_t> reader_compile_time_args =
-    { static_cast<uint32_t>(is_dram(input)), static_cast<uint32_t>(is_dram(output)), static_cast<uint32_t>(is_dram(output_grad)), input_grad_rank };
+    { static_cast<uint32_t>(is_dram(input)), static_cast<uint32_t>(is_dram(output)), static_cast<uint32_t>(is_dram(output_grad)), static_cast<uint32_t>(input_grad_rank) };
     std::vector<uint32_t> writer_compile_time_args =
     { static_cast<uint32_t>(is_dram(input_grad)) };
     const auto reader_kernels_id = CreateReadKernel(program, reader_kernel_file, all_cores, reader_compile_time_args);
diff --git a/ttnn/cpp/ttnn/operations/normalization/groupnorm/groupnorm.hpp b/ttnn/cpp/ttnn/operations/normalization/groupnorm/groupnorm.hpp
index b0abd7d266..81104793cf 100644
--- a/ttnn/cpp/ttnn/operations/normalization/groupnorm/groupnorm.hpp
+++ b/ttnn/cpp/ttnn/operations/normalization/groupnorm/groupnorm.hpp
@@ -73,7 +73,7 @@ struct ExecuteGroupNorm {
         return operation::run(
             GroupNorm{
                 .eps=epsilon,
-                .num_groups=num_groups,
+                .num_groups=static_cast<uint32_t>(num_groups),
                 .output_mem_config=output_mem_config,
                 .program_config=program_config},
                 {input_tensor},

@dmakoviichuk-tt
Copy link
Contributor Author

@marty1885 thanks for the quick reaction. Will do it.

@dmakoviichuk-tt
Copy link
Contributor Author

@marty1885 done. Please confirm it works and we will merge it tomorrow.

@marty1885
Copy link
Contributor

@dmakoviichuk-tt I did a clean build on my side and can confirm this branch compiles on my machine. Thanks for the help!

@dmakoviichuk-tt dmakoviichuk-tt merged commit 875d1e4 into main Jul 19, 2024
5 checks passed
@dmakoviichuk-tt dmakoviichuk-tt deleted the DM/github-10471 branch July 19, 2024 19:00
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

Successfully merging this pull request may close these issues.

5 participants