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

Clang frontend generates unnecessary memcpy (that is optimized to load/store) for an empty class #59336

Open
strimo378 opened this issue Dec 5, 2022 · 3 comments

Comments

@strimo378
Copy link
Contributor

strimo378 commented Dec 5, 2022

Hi all,

I found out that Clang codegen emits an unnecessary memcpy for an empty C++ class with an empty struct. I don't know if other (more common) cases are effected.

The following code

struct a {
  struct {};
} b;
a c;
int main() { b = c; }

generates the LLVM IR (see https://godbolt.org/z/4hjx8xxqf)

define dso_local noundef i32 @main() #0 {
  call void @llvm.memcpy.p0.p0.i64(ptr align 1 @b, ptr align 1 @c, i64 1, i1 false), !tbaa.struct !6
  ret i32 0
}

...

!6 = !{}

Interestingly, the memcpy is generated with an empty tbaa.struct metadata, indicating that the memcpy is copying nothing.

During IR optimization the memcpy is optimized to a load/store.

define dso_local noundef i32 @main() local_unnamed_addr #0 {
  %1 = load i8, ptr @c, align 1
  store i8 %1, ptr @b, align 1
  ret i32 0
}

The problem is within the CodeGenFunction::EmitAggregateCopy function in clang/lib/CodeGen/CGExprAgg.cpp. The function only checks if the record is empty using Record->isEmpty(). Either isEmpty is wrong in that case or the calculated data layout of the record is a better choice to skip memcpy emission.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 5, 2022

@llvm/issue-subscribers-clang-codegen

@strimo378
Copy link
Contributor Author

Just another test case with a combination of fields and inheritance:

struct a {};
struct b : a {};
struct c {
  b d;
  void e();
} f;
void g() {
  c h = f;
  h.e();
} 

In that test case, h = f emits a load/store that is unnecessary (and gcc does not emit one).

@strimo378
Copy link
Contributor Author

Pure C is also affected:

struct S {
    char : 8;
};

struct S a,b;

void test() {
    a = b;
}

strimo378 added a commit to emmtrix/llvm-project that referenced this issue Aug 18, 2023
… content

Clang codegen emits an unnecessary memcpy for C++ class without content. This patch fixes the issue.

See llvm#59336

Please let me know if the patch is going into the right direction that it can be accepted. If yes, I'll finish it with test cases and also extend it to C.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants