This is an archive of the discontinued LLVM Phabricator instance.

Differential D71282

Fix bug 44190 - wrong code with #pragma pack(1)
ClosedPublic

Authored by yaxunl on Dec 10 2019, 9:05 AM.

Details

Reviewers
rjmccall
efriedma
• jnspaulsson
Commits
rG21b43885b81a: Fix bug 44190 - wrong code with #pragma pack(1)
Summary

https://github.com/llvm/llvm-project/commit/5b330e8d6122c336d81dfd11c864e6c6240a381e caused
a regression on s390:

https://bugs.llvm.org/show_bug.cgi?id=44190

we need to copy if if either the argument is non-byval or the argument is underaligned.

Diff Detail

Event Timeline

yaxunl created this revision.Dec 10 2019, 9:05 AM
rjmccall added inline comments.Dec 10 2019, 9:47 AM
clang/lib/CodeGen/CGCall.cpp
4009

Thanks for the fix. Please remove all the unnecessary parens, but otherwise LGTM.

yaxunl updated this revision to Diff 233133.Dec 10 2019, 10:01 AM

remove parenthesis

yaxunl marked 2 inline comments as done.Dec 10 2019, 10:01 AM
yaxunl added inline comments.
clang/lib/CodeGen/CGCall.cpp
4009

fixed

rjmccall accepted this revision.Dec 10 2019, 10:10 AM
This revision is now accepted and ready to land.Dec 10 2019, 10:10 AM
Closed by commit rG21b43885b81a: Fix bug 44190 - wrong code with #pragma pack(1) (authored by yaxunl). · Explain WhyDec 10 2019, 11:05 AM
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptDec 10 2019, 11:05 AM

Revision Contents

PathSize
clang/
lib/
CodeGen/
5 lines
test/
CodeGen/
23 lines

Diff 233152

clang/lib/CodeGen/CGCall.cpp

Show First 20 Lines • Show All 3,999 Lines • ▼ Show 20 Lines case ABIArgInfo::Indirect: {
if (Addr.getAlignment() < Align && if (Addr.getAlignment() < Align &&
llvm::getOrEnforceKnownAlignment(V, Align.getQuantity(), *TD) < llvm::getOrEnforceKnownAlignment(V, Align.getQuantity(), *TD) <
Align.getQuantity()) { Align.getQuantity()) {
NeedCopy = true; NeedCopy = true;
} else if (I->hasLValue()) { } else if (I->hasLValue()) {
auto LV = I->getKnownLValue(); auto LV = I->getKnownLValue();
auto AS = LV.getAddressSpace(); auto AS = LV.getAddressSpace();
if ((!ArgInfo.getIndirectByVal() && if (!ArgInfo.getIndirectByVal() ||
(LV.getAlignment() >= (LV.getAlignment() < getContext().getTypeAlignInChars(I->Ty))) {
rjmccallUnsubmitted
Done
ReplyInline Actions

Thanks for the fix. Please remove all the unnecessary parens, but otherwise LGTM.

rjmccall: Thanks for the fix. Please remove all the unnecessary parens, but otherwise LGTM.
yaxunlAuthorUnsubmitted
Done
ReplyInline Actions

fixed

yaxunl: fixed
getContext().getTypeAlignInChars(I->Ty)))) {
NeedCopy = true; NeedCopy = true;
} }
if (!getLangOpts().OpenCL) { if (!getLangOpts().OpenCL) {
if ((ArgInfo.getIndirectByVal() && if ((ArgInfo.getIndirectByVal() &&
(AS != LangAS::Default && (AS != LangAS::Default &&
AS != CGM.getASTAllocaAddressSpace()))) { AS != CGM.getASTAllocaAddressSpace()))) {
NeedCopy = true; NeedCopy = true;
} }
▲ Show 20 Lines • Show All 639 Lines • Show Last 20 Lines

clang/test/CodeGen/s390x-packed-struct-func-arg.c

  • This file was added.
// RUN: %clang_cc1 -triple s390x-linux-gnu -target-cpu z13 -emit-llvm -o - %s \
// RUN: | FileCheck %s
struct S0 {
long f1;
int f2 : 4;
} d;
#pragma pack(1)
struct S1 {
struct S0 S0_member;
};
void f(struct S0 arg) {
arg.f2 = 1;
}
void g() {
struct S1 g;
// CHECK: alloca %struct.S0, align 8
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 {{.*}}, i8* align 1 {{.*}}, i64 16
f(g.S0_member);
}