[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd60c3d08e78d: [clang] Skip stores in init for fields that are empty structs (authored by mstorsjo). Changed prior to commit: https://reviews.llvm.

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157332/new/ https://reviews.llvm.org/D157332 ___

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo marked an inline comment as done. mstorsjo added a comment. Thanks a lot for the guidance! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157332/new/ https://reviews.llvm.org/D157332 ___ cfe-comm

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549575. mstorsjo added a comment. Move the existing comment into the new if statement, add a comment to the new if. Add a comment to the second part of the testcase, which probably is good to keep anyway. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

efriedma added a comment. This approach looks fine. Comment at: clang/lib/CodeGen/CGCall.cpp:5781 // If the value is offset in memory, apply the offset now. + if (!isEmptyRecord(getContext(), RetTy, true)) { The isEmptyRecord call could use a comm

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549517. mstorsjo added a comment. Updated to just check isEmptyRecord in EmitCall. The second part of the test probably is kinda redundant/pointless at this point, and I guess the test comment needs updating too; can do that later when the implementation i

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

efriedma added a comment. You can't, in general, check whether a type is stored in a no_unique_address field. Consider the following; the bad store is in the constructor S2, but the relevant no_unique_address attribute doesn't even show up until the definition of S3. struct S {}; S f();

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo updated this revision to Diff 549099. mstorsjo added a comment. Plumb the info from EmitInitializerForField through AggValueSlot and ReturnValueSlot to EmitCall. The fields/variables could use better names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo added a comment. In D157332#4570290 , @crtrott wrote: > Question: does this bug potentially affect code generation for > AMD/NVIDIA/Intel GPUs? I believe the easiest way to test that is to try compiling `struct S {}; S ret() { return S(); }` i

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

crtrott added a comment. To answer my own question: I was able to reproduce the bug when compiling for NVIDIA GPUs, I was not able to reproduce it for AMD GPUs yet, and I didn't try for Intel GPUs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

crtrott added a comment. Question: does this bug potentially affect code generation for AMD/NVIDIA/Intel GPUs? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157332/new/ https://reviews.llvm.org/D157332

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo added a comment. In D157332#4568341 , @efriedma wrote: > I see what you're getting at here... but I don't think this works quite > right. If the empty class has a non-trivial constructor, we have to pass the > correct "this" address to that co

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

cor3ntin added inline comments. Comment at: clang/test/CodeGenCXX/ctor-empty-nounique.cpp:1 +// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s + This should probably get another run line for powerpc64le Repository: rG LLVM Github

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

efriedma added a comment. I see what you're getting at here... but I don't think this works quite right. If the empty class has a non-trivial constructor, we have to pass the correct "this" address to that constructor. Usually a constructor for an empty class won't do anything with that addre

[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

mstorsjo created this revision. mstorsjo added reviewers: aaron.ballman, efriedma. Herald added subscribers: steven.zhang, pengfei. Herald added a project: All. mstorsjo requested review of this revision. Herald added a project: clang. An empty struct is handled as a struct with a dummy i8, on all