[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 0012b1ea84e671a9e0c9f7f2d1564315ed9cbcca
19f3b677d92ed88b825b455d738055da05f91e0d -- clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGExprAgg.cpp clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/arm-mve-intrinsics/vld24.c
clang/test/CodeGen/arm-vfp16-arguments2.cpp
clang/test/CodeGenCXX/address-space-cast-coerce.cpp
clang/test/CodeGenCXX/cxx2a-consteval.cpp clang/test/CodeGenCXX/trivial_abi.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index db6feb4bf1..262aa30547 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1355,7 +1355,8 @@ void CodeGenFunction::CreateCoercedStore(llvm::Value
*Src, Address Dst,
// FIXME: This isn't really that useful with opaque types, but it impacts a
// lot of regression tests.
if (SrcTy != Dst.getElementType()) {
-if (llvm::StructType *DstSTy =
dyn_cast(Dst.getElementType())) {
+if (llvm::StructType *DstSTy =
+dyn_cast(Dst.getElementType())) {
assert(!SrcSize.isScalable());
Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
SrcSize.getFixedValue(), *this);
@@ -1368,7 +1369,8 @@ void CodeGenFunction::CreateCoercedStore(llvm::Value
*Src, Address Dst,
// If the value is supposed to be a pointer, convert it before storing
it.
Src = CoerceIntOrPtrToIntOrPtr(Src, Dst.getElementType(), *this);
Builder.CreateStore(Src, Dst, DstIsVolatile);
-} else if (llvm::StructType *STy =
dyn_cast(Src->getType())) {
+} else if (llvm::StructType *STy =
+ dyn_cast(Src->getType())) {
// Prefer scalar stores to first-class aggregate stores.
Dst = Dst.withElementType(SrcTy);
for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
``
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -177,7 +179,12 @@ kernel void KernelTwoMember(struct StructTwoMember u) {
// AMDGCN-LABEL: define{{.*}} amdgpu_kernel void @KernelLargeTwoMember
// AMDGCN-SAME: (%struct.LargeStructTwoMember %[[u_coerce:.*]])
// AMDGCN: %[[u:.*]] = alloca %struct.LargeStructTwoMember, align 8,
addrspace(5)
-// AMDGCN: store %struct.LargeStructTwoMember %[[u_coerce]], ptr addrspace(5)
%[[u]]
+// AMDGCN: %[[U_PTR0:.*]] = getelementptr inbounds
%struct.LargeStructTwoMember, ptr addrspace(5) %[[u]], i32 0, i32 0
efriedma-quic wrote:
Unifying the codepaths makes FCA promotion happen more often.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -46,9 +46,9 @@ int mane() {
char1 f1{1};
char1 f2{1};
-// CHECK: [[TMP:%.+]] = alloca i16
efriedma-quic wrote:
The revised version of casting integers is a bit more aggressive; it's hard to
make it precisely match the old code while still preserving the correct
semantics.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -44,20 +44,20 @@ struct S1 f1(struct S1 s1) { return s1; }
// CHECK-SOFT: define{{.*}} void @_Z2f22S2(ptr dead_on_unwind noalias
nocapture writable writeonly sret(%struct.S2) align 8 %agg.result, [4 x i32]
%s2.coerce)
// CHECK-HARD: define{{.*}} arm_aapcs_vfpcc [2 x <2 x i32>] @_Z2f22S2([2 x <2
x i32>] returned %s2.coerce)
-// CHECK-FULL: define{{.*}} arm_aapcs_vfpcc %struct.S2 @_Z2f22S2(%struct.S2
returned %s2.coerce)
+// CHECK-FULL: define{{.*}} arm_aapcs_vfpcc %struct.S2 @_Z2f22S2(%struct.S2
%s2.coerce)
efriedma-quic wrote:
This is also the instcombine issue.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -48,10 +48,13 @@ uint8x16x4_t test_vld4q_u8(const uint8_t *addr) // CHECK-LABEL: @test_vst2q_u32( // CHECK-NEXT: entry: -// CHECK-NEXT:[[VALUE_COERCE_FCA_0_0_EXTRACT:%.*]] = extractvalue [[STRUCT_UINT32X4X2_T:%.*]] [[VALUE_COERCE:%.*]], 0, 0 -// CHECK-NEXT:[[VALUE_COERCE_FCA_0_1_EXTRACT:%.*]] = extractvalue [[STRUCT_UINT32X4X2_T]] [[VALUE_COERCE]], 0, 1 -// CHECK-NEXT:call void @llvm.arm.mve.vst2q.p0.v4i32(ptr [[ADDR:%.*]], <4 x i32> [[VALUE_COERCE_FCA_0_0_EXTRACT]], <4 x i32> [[VALUE_COERCE_FCA_0_1_EXTRACT]], i32 0) -// CHECK-NEXT:call void @llvm.arm.mve.vst2q.p0.v4i32(ptr [[ADDR]], <4 x i32> [[VALUE_COERCE_FCA_0_0_EXTRACT]], <4 x i32> [[VALUE_COERCE_FCA_0_1_EXTRACT]], i32 1) +// CHECK-NEXT:[[TMP0:%.*]] = extractvalue [[STRUCT_UINT32X4X2_T:%.*]] [[VALUE_COERCE:%.*]], 0 efriedma-quic wrote: Apparently I've stumbled over some limitation of instcombine. https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/efriedma-quic updated
https://github.com/llvm/llvm-project/pull/93115
>From 816ceb271c28ff6c4dc05485000deec087674e37 Mon Sep 17 00:00:00 2001
From: Eli Friedman
Date: Thu, 23 May 2024 18:38:04 -0700
Subject: [PATCH] Fix codegen of consteval functions returning an empty class,
and related issues
If a class is empty, don't store it to memory: the store might overwrite
useful data. Similarly, if a class has tail padding that might overlap
other fields, don't store the tail padding to memory.
The problem here turned out a bit more general than I initially thought:
basically all uses of EmitAggregateStore were broken. Call lowering had
a method that did mostly the right thing, though: CreateCoercedStore.
Adapt CreateCoercedStore so it always does the conservatively right
thing, and use it for both calls and ConstantExpr.
Also, along the way, fix the "overlap" bit in AggValueSlot: the bit was
set incorrectly for empty classes in some cases.
Fixes #93040.
---
clang/lib/CodeGen/CGCall.cpp | 140 --
clang/lib/CodeGen/CGExprAgg.cpp | 23 +--
clang/lib/CodeGen/CodeGenFunction.h | 3 +-
clang/test/CodeGen/arm-mve-intrinsics/vld24.c | 43 --
clang/test/CodeGen/arm-vfp16-arguments2.cpp | 10 +-
.../amdgpu-kernel-arg-pointer-type.cu | 11 +-
clang/test/CodeGenCUDA/builtins-amdgcn.cu | 121 +++
.../CodeGenCXX/address-space-cast-coerce.cpp | 6 +-
clang/test/CodeGenCXX/cxx2a-consteval.cpp | 24 ++-
clang/test/CodeGenCXX/trivial_abi.cpp | 20 +++
clang/test/CodeGenHIP/dpp-const-fold.hip | 8 +-
.../CodeGenOpenCL/addr-space-struct-arg.cl| 11 +-
.../amdgpu-abi-struct-arg-byref.cl| 42 --
13 files changed, 255 insertions(+), 207 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 97449a5e51e73..262aa3054754c 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1336,75 +1336,56 @@ static llvm::Value *CreateCoercedLoad(Address Src,
llvm::Type *Ty,
return CGF.Builder.CreateLoad(Tmp);
}
-// Function to store a first-class aggregate into memory. We prefer to
-// store the elements rather than the aggregate to be more friendly to
-// fast-isel.
-// FIXME: Do we need to recurse here?
-void CodeGenFunction::EmitAggregateStore(llvm::Value *Val, Address Dest,
- bool DestIsVolatile) {
- // Prefer scalar stores to first-class aggregate stores.
- if (llvm::StructType *STy = dyn_cast(Val->getType())) {
-for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
- Address EltPtr = Builder.CreateStructGEP(Dest, i);
- llvm::Value *Elt = Builder.CreateExtractValue(Val, i);
- Builder.CreateStore(Elt, EltPtr, DestIsVolatile);
-}
- } else {
-Builder.CreateStore(Val, Dest, DestIsVolatile);
- }
-}
-
/// CreateCoercedStore - Create a store to \arg DstPtr from \arg Src,
/// where the source and destination may have different types. The
/// destination is known to be aligned to \arg DstAlign bytes.
///
/// This safely handles the case when the src type is larger than the
/// destination type; the upper bits of the src will be lost.
-static void CreateCoercedStore(llvm::Value *Src,
- Address Dst,
- bool DstIsVolatile,
- CodeGenFunction &CGF) {
- llvm::Type *SrcTy = Src->getType();
- llvm::Type *DstTy = Dst.getElementType();
- if (SrcTy == DstTy) {
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
- }
-
- llvm::TypeSize SrcSize = CGF.CGM.getDataLayout().getTypeAllocSize(SrcTy);
-
- if (llvm::StructType *DstSTy = dyn_cast(DstTy)) {
-Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
- SrcSize.getFixedValue(), CGF);
-DstTy = Dst.getElementType();
- }
-
- llvm::PointerType *SrcPtrTy = llvm::dyn_cast(SrcTy);
- llvm::PointerType *DstPtrTy = llvm::dyn_cast(DstTy);
- if (SrcPtrTy && DstPtrTy &&
- SrcPtrTy->getAddressSpace() != DstPtrTy->getAddressSpace()) {
-Src = CGF.Builder.CreateAddrSpaceCast(Src, DstTy);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+void CodeGenFunction::CreateCoercedStore(llvm::Value *Src, Address Dst,
+ llvm::TypeSize DstSize,
+ bool DstIsVolatile) {
+ if (!DstSize)
return;
- }
- // If the source and destination are integer or pointer types, just do an
- // extension or truncation to the desired type.
- if ((isa(SrcTy) || isa(SrcTy)) &&
- (isa(DstTy) || isa(DstTy))) {
-Src = CoerceIntOrPtrToIntOrPtr(Src, DstTy, CGF);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
+ llvm::Type *SrcTy = Src->getType();
+ llvm::TypeSize SrcSize = CGM.getDataLayout().getTypeAllocSize(SrcTy);
+
+ // GEP into structs to try to
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
efriedma-quic wrote: (I'd like a re-review of the latest version: I made significant revisions to address the tail-padding issues.) https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/zygoloid approved this pull request. https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/zygoloid edited https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -1336,75 +1336,56 @@ static llvm::Value *CreateCoercedLoad(Address Src,
llvm::Type *Ty,
return CGF.Builder.CreateLoad(Tmp);
}
-// Function to store a first-class aggregate into memory. We prefer to
-// store the elements rather than the aggregate to be more friendly to
-// fast-isel.
-// FIXME: Do we need to recurse here?
-void CodeGenFunction::EmitAggregateStore(llvm::Value *Val, Address Dest,
- bool DestIsVolatile) {
- // Prefer scalar stores to first-class aggregate stores.
- if (llvm::StructType *STy = dyn_cast(Val->getType())) {
-for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
- Address EltPtr = Builder.CreateStructGEP(Dest, i);
- llvm::Value *Elt = Builder.CreateExtractValue(Val, i);
- Builder.CreateStore(Elt, EltPtr, DestIsVolatile);
-}
- } else {
-Builder.CreateStore(Val, Dest, DestIsVolatile);
- }
-}
-
/// CreateCoercedStore - Create a store to \arg DstPtr from \arg Src,
/// where the source and destination may have different types. The
/// destination is known to be aligned to \arg DstAlign bytes.
///
/// This safely handles the case when the src type is larger than the
/// destination type; the upper bits of the src will be lost.
-static void CreateCoercedStore(llvm::Value *Src,
- Address Dst,
- bool DstIsVolatile,
- CodeGenFunction &CGF) {
- llvm::Type *SrcTy = Src->getType();
- llvm::Type *DstTy = Dst.getElementType();
- if (SrcTy == DstTy) {
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
- }
-
- llvm::TypeSize SrcSize = CGF.CGM.getDataLayout().getTypeAllocSize(SrcTy);
-
- if (llvm::StructType *DstSTy = dyn_cast(DstTy)) {
-Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
- SrcSize.getFixedValue(), CGF);
-DstTy = Dst.getElementType();
- }
-
- llvm::PointerType *SrcPtrTy = llvm::dyn_cast(SrcTy);
- llvm::PointerType *DstPtrTy = llvm::dyn_cast(DstTy);
- if (SrcPtrTy && DstPtrTy &&
- SrcPtrTy->getAddressSpace() != DstPtrTy->getAddressSpace()) {
-Src = CGF.Builder.CreateAddrSpaceCast(Src, DstTy);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+void CodeGenFunction::CreateCoercedStore(llvm::Value *Src, Address Dst,
+ llvm::TypeSize DstSize,
+ bool DstIsVolatile) {
+ if (!DstSize)
return;
- }
- // If the source and destination are integer or pointer types, just do an
- // extension or truncation to the desired type.
- if ((isa(SrcTy) || isa(SrcTy)) &&
- (isa(DstTy) || isa(DstTy))) {
-Src = CoerceIntOrPtrToIntOrPtr(Src, DstTy, CGF);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
+ llvm::Type *SrcTy = Src->getType();
+ llvm::TypeSize SrcSize = CGM.getDataLayout().getTypeAllocSize(SrcTy);
+
+ // GEP into structs to try to make types match.
+ // FIXME: This isn't really that useful with opaque types, but it impacts a
+ // lot of regression tests.
+ if (SrcTy != Dst.getElementType()) {
+if (llvm::StructType *DstSTy =
+dyn_cast(Dst.getElementType())) {
+ assert(!SrcSize.isScalable());
+ Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
+ SrcSize.getFixedValue(), *this);
+}
}
- llvm::TypeSize DstSize = CGF.CGM.getDataLayout().getTypeAllocSize(DstTy);
-
- // If store is legal, just bitcast the src pointer.
- if (isa(SrcTy) ||
- isa(DstTy) ||
- SrcSize.getFixedValue() <= DstSize.getFixedValue()) {
-Dst = Dst.withElementType(SrcTy);
-CGF.EmitAggregateStore(Src, Dst, DstIsVolatile);
+ if (SrcSize.isScalable() || SrcSize <= DstSize) {
+if (SrcTy->isIntegerTy() && Dst.getElementType()->isPointerTy() &&
+SrcSize == CGM.getDataLayout().getTypeAllocSize(Dst.getElementType()))
{
+ // If the value is supposed to be a pointer, convert it before storing
it.
+ Src = CoerceIntOrPtrToIntOrPtr(Src, Dst.getElementType(), *this);
zygoloid wrote:
It looks like we used to create either an `inttoptr` or `ptrtoint` here and now
we only handle the `inttoptr` case. Is that OK?
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -4772,7 +4772,8 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Build all the stores needed to initialize an aggregate at Dest with the
/// value Val.
zygoloid wrote:
This comment looks out of date.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -1336,75 +1336,56 @@ static llvm::Value *CreateCoercedLoad(Address Src,
llvm::Type *Ty,
return CGF.Builder.CreateLoad(Tmp);
}
-// Function to store a first-class aggregate into memory. We prefer to
-// store the elements rather than the aggregate to be more friendly to
-// fast-isel.
-// FIXME: Do we need to recurse here?
-void CodeGenFunction::EmitAggregateStore(llvm::Value *Val, Address Dest,
- bool DestIsVolatile) {
- // Prefer scalar stores to first-class aggregate stores.
- if (llvm::StructType *STy = dyn_cast(Val->getType())) {
-for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
- Address EltPtr = Builder.CreateStructGEP(Dest, i);
- llvm::Value *Elt = Builder.CreateExtractValue(Val, i);
- Builder.CreateStore(Elt, EltPtr, DestIsVolatile);
-}
- } else {
-Builder.CreateStore(Val, Dest, DestIsVolatile);
- }
-}
-
/// CreateCoercedStore - Create a store to \arg DstPtr from \arg Src,
/// where the source and destination may have different types. The
/// destination is known to be aligned to \arg DstAlign bytes.
///
/// This safely handles the case when the src type is larger than the
/// destination type; the upper bits of the src will be lost.
-static void CreateCoercedStore(llvm::Value *Src,
- Address Dst,
- bool DstIsVolatile,
- CodeGenFunction &CGF) {
- llvm::Type *SrcTy = Src->getType();
- llvm::Type *DstTy = Dst.getElementType();
- if (SrcTy == DstTy) {
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
- }
-
- llvm::TypeSize SrcSize = CGF.CGM.getDataLayout().getTypeAllocSize(SrcTy);
-
- if (llvm::StructType *DstSTy = dyn_cast(DstTy)) {
-Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
- SrcSize.getFixedValue(), CGF);
-DstTy = Dst.getElementType();
- }
-
- llvm::PointerType *SrcPtrTy = llvm::dyn_cast(SrcTy);
- llvm::PointerType *DstPtrTy = llvm::dyn_cast(DstTy);
- if (SrcPtrTy && DstPtrTy &&
- SrcPtrTy->getAddressSpace() != DstPtrTy->getAddressSpace()) {
-Src = CGF.Builder.CreateAddrSpaceCast(Src, DstTy);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+void CodeGenFunction::CreateCoercedStore(llvm::Value *Src, Address Dst,
+ llvm::TypeSize DstSize,
+ bool DstIsVolatile) {
+ if (!DstSize)
return;
- }
- // If the source and destination are integer or pointer types, just do an
- // extension or truncation to the desired type.
- if ((isa(SrcTy) || isa(SrcTy)) &&
- (isa(DstTy) || isa(DstTy))) {
-Src = CoerceIntOrPtrToIntOrPtr(Src, DstTy, CGF);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
+ llvm::Type *SrcTy = Src->getType();
+ llvm::TypeSize SrcSize = CGM.getDataLayout().getTypeAllocSize(SrcTy);
+
+ // GEP into structs to try to make types match.
+ // FIXME: This isn't really that useful with opaque types, but it impacts a
+ // lot of regression tests.
+ if (SrcTy != Dst.getElementType()) {
+if (llvm::StructType *DstSTy =
+dyn_cast(Dst.getElementType())) {
+ assert(!SrcSize.isScalable());
+ Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
+ SrcSize.getFixedValue(), *this);
+}
}
- llvm::TypeSize DstSize = CGF.CGM.getDataLayout().getTypeAllocSize(DstTy);
-
- // If store is legal, just bitcast the src pointer.
- if (isa(SrcTy) ||
- isa(DstTy) ||
- SrcSize.getFixedValue() <= DstSize.getFixedValue()) {
-Dst = Dst.withElementType(SrcTy);
-CGF.EmitAggregateStore(Src, Dst, DstIsVolatile);
+ if (SrcSize.isScalable() || SrcSize <= DstSize) {
+if (SrcTy->isIntegerTy() && Dst.getElementType()->isPointerTy() &&
+SrcSize == CGM.getDataLayout().getTypeAllocSize(Dst.getElementType()))
{
+ // If the value is supposed to be a pointer, convert it before storing
it.
+ Src = CoerceIntOrPtrToIntOrPtr(Src, Dst.getElementType(), *this);
efriedma-quic wrote:
inttoptr comes from constructs like a struct containing a pointer, which the
calling convention code sometimes chooses to just coerce to an integer instead
of trying to compute a prettier type. The other direction would be very
unusual: it would mean we somehow coerced some non-pointer value to a pointer.
And this isn't really a correctness thing, anyway; type-punning in memory is
basically the same as doing it in a register. This just makes the intended
types more clear for optimizations.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -1336,75 +1336,56 @@ static llvm::Value *CreateCoercedLoad(Address Src,
llvm::Type *Ty,
return CGF.Builder.CreateLoad(Tmp);
}
-// Function to store a first-class aggregate into memory. We prefer to
-// store the elements rather than the aggregate to be more friendly to
-// fast-isel.
-// FIXME: Do we need to recurse here?
-void CodeGenFunction::EmitAggregateStore(llvm::Value *Val, Address Dest,
- bool DestIsVolatile) {
- // Prefer scalar stores to first-class aggregate stores.
- if (llvm::StructType *STy = dyn_cast(Val->getType())) {
-for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
- Address EltPtr = Builder.CreateStructGEP(Dest, i);
- llvm::Value *Elt = Builder.CreateExtractValue(Val, i);
- Builder.CreateStore(Elt, EltPtr, DestIsVolatile);
-}
- } else {
-Builder.CreateStore(Val, Dest, DestIsVolatile);
- }
-}
-
/// CreateCoercedStore - Create a store to \arg DstPtr from \arg Src,
/// where the source and destination may have different types. The
/// destination is known to be aligned to \arg DstAlign bytes.
///
/// This safely handles the case when the src type is larger than the
/// destination type; the upper bits of the src will be lost.
-static void CreateCoercedStore(llvm::Value *Src,
- Address Dst,
- bool DstIsVolatile,
- CodeGenFunction &CGF) {
- llvm::Type *SrcTy = Src->getType();
- llvm::Type *DstTy = Dst.getElementType();
- if (SrcTy == DstTy) {
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
- }
-
- llvm::TypeSize SrcSize = CGF.CGM.getDataLayout().getTypeAllocSize(SrcTy);
-
- if (llvm::StructType *DstSTy = dyn_cast(DstTy)) {
-Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
- SrcSize.getFixedValue(), CGF);
-DstTy = Dst.getElementType();
- }
-
- llvm::PointerType *SrcPtrTy = llvm::dyn_cast(SrcTy);
- llvm::PointerType *DstPtrTy = llvm::dyn_cast(DstTy);
- if (SrcPtrTy && DstPtrTy &&
- SrcPtrTy->getAddressSpace() != DstPtrTy->getAddressSpace()) {
-Src = CGF.Builder.CreateAddrSpaceCast(Src, DstTy);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+void CodeGenFunction::CreateCoercedStore(llvm::Value *Src, Address Dst,
+ llvm::TypeSize DstSize,
+ bool DstIsVolatile) {
+ if (!DstSize)
return;
- }
- // If the source and destination are integer or pointer types, just do an
- // extension or truncation to the desired type.
- if ((isa(SrcTy) || isa(SrcTy)) &&
- (isa(DstTy) || isa(DstTy))) {
-Src = CoerceIntOrPtrToIntOrPtr(Src, DstTy, CGF);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
+ llvm::Type *SrcTy = Src->getType();
+ llvm::TypeSize SrcSize = CGM.getDataLayout().getTypeAllocSize(SrcTy);
+
+ // GEP into structs to try to make types match.
+ // FIXME: This isn't really that useful with opaque types, but it impacts a
+ // lot of regression tests.
+ if (SrcTy != Dst.getElementType()) {
+if (llvm::StructType *DstSTy =
+dyn_cast(Dst.getElementType())) {
+ assert(!SrcSize.isScalable());
+ Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
+ SrcSize.getFixedValue(), *this);
+}
}
- llvm::TypeSize DstSize = CGF.CGM.getDataLayout().getTypeAllocSize(DstTy);
-
- // If store is legal, just bitcast the src pointer.
- if (isa(SrcTy) ||
- isa(DstTy) ||
- SrcSize.getFixedValue() <= DstSize.getFixedValue()) {
-Dst = Dst.withElementType(SrcTy);
-CGF.EmitAggregateStore(Src, Dst, DstIsVolatile);
+ if (SrcSize.isScalable() || SrcSize <= DstSize) {
+if (SrcTy->isIntegerTy() && Dst.getElementType()->isPointerTy() &&
+SrcSize == CGM.getDataLayout().getTypeAllocSize(Dst.getElementType()))
{
+ // If the value is supposed to be a pointer, convert it before storing
it.
+ Src = CoerceIntOrPtrToIntOrPtr(Src, Dst.getElementType(), *this);
efriedma-quic wrote:
The original code implicitly handled int->int, int->ptr, and ptr->int. But
ptr->int doesn't come up in practice
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
@@ -1336,75 +1336,56 @@ static llvm::Value *CreateCoercedLoad(Address Src,
llvm::Type *Ty,
return CGF.Builder.CreateLoad(Tmp);
}
-// Function to store a first-class aggregate into memory. We prefer to
-// store the elements rather than the aggregate to be more friendly to
-// fast-isel.
-// FIXME: Do we need to recurse here?
-void CodeGenFunction::EmitAggregateStore(llvm::Value *Val, Address Dest,
- bool DestIsVolatile) {
- // Prefer scalar stores to first-class aggregate stores.
- if (llvm::StructType *STy = dyn_cast(Val->getType())) {
-for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
- Address EltPtr = Builder.CreateStructGEP(Dest, i);
- llvm::Value *Elt = Builder.CreateExtractValue(Val, i);
- Builder.CreateStore(Elt, EltPtr, DestIsVolatile);
-}
- } else {
-Builder.CreateStore(Val, Dest, DestIsVolatile);
- }
-}
-
/// CreateCoercedStore - Create a store to \arg DstPtr from \arg Src,
/// where the source and destination may have different types. The
/// destination is known to be aligned to \arg DstAlign bytes.
///
/// This safely handles the case when the src type is larger than the
/// destination type; the upper bits of the src will be lost.
-static void CreateCoercedStore(llvm::Value *Src,
- Address Dst,
- bool DstIsVolatile,
- CodeGenFunction &CGF) {
- llvm::Type *SrcTy = Src->getType();
- llvm::Type *DstTy = Dst.getElementType();
- if (SrcTy == DstTy) {
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
- }
-
- llvm::TypeSize SrcSize = CGF.CGM.getDataLayout().getTypeAllocSize(SrcTy);
-
- if (llvm::StructType *DstSTy = dyn_cast(DstTy)) {
-Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
- SrcSize.getFixedValue(), CGF);
-DstTy = Dst.getElementType();
- }
-
- llvm::PointerType *SrcPtrTy = llvm::dyn_cast(SrcTy);
- llvm::PointerType *DstPtrTy = llvm::dyn_cast(DstTy);
- if (SrcPtrTy && DstPtrTy &&
- SrcPtrTy->getAddressSpace() != DstPtrTy->getAddressSpace()) {
-Src = CGF.Builder.CreateAddrSpaceCast(Src, DstTy);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
+void CodeGenFunction::CreateCoercedStore(llvm::Value *Src, Address Dst,
+ llvm::TypeSize DstSize,
+ bool DstIsVolatile) {
+ if (!DstSize)
return;
- }
- // If the source and destination are integer or pointer types, just do an
- // extension or truncation to the desired type.
- if ((isa(SrcTy) || isa(SrcTy)) &&
- (isa(DstTy) || isa(DstTy))) {
-Src = CoerceIntOrPtrToIntOrPtr(Src, DstTy, CGF);
-CGF.Builder.CreateStore(Src, Dst, DstIsVolatile);
-return;
+ llvm::Type *SrcTy = Src->getType();
+ llvm::TypeSize SrcSize = CGM.getDataLayout().getTypeAllocSize(SrcTy);
+
+ // GEP into structs to try to make types match.
+ // FIXME: This isn't really that useful with opaque types, but it impacts a
+ // lot of regression tests.
+ if (SrcTy != Dst.getElementType()) {
+if (llvm::StructType *DstSTy =
+dyn_cast(Dst.getElementType())) {
+ assert(!SrcSize.isScalable());
+ Dst = EnterStructPointerForCoercedAccess(Dst, DstSTy,
+ SrcSize.getFixedValue(), *this);
+}
}
- llvm::TypeSize DstSize = CGF.CGM.getDataLayout().getTypeAllocSize(DstTy);
-
- // If store is legal, just bitcast the src pointer.
- if (isa(SrcTy) ||
- isa(DstTy) ||
- SrcSize.getFixedValue() <= DstSize.getFixedValue()) {
-Dst = Dst.withElementType(SrcTy);
-CGF.EmitAggregateStore(Src, Dst, DstIsVolatile);
+ if (SrcSize.isScalable() || SrcSize <= DstSize) {
+if (SrcTy->isIntegerTy() && Dst.getElementType()->isPointerTy() &&
+SrcSize == CGM.getDataLayout().getTypeAllocSize(Dst.getElementType()))
{
+ // If the value is supposed to be a pointer, convert it before storing
it.
+ Src = CoerceIntOrPtrToIntOrPtr(Src, Dst.getElementType(), *this);
zygoloid wrote:
Yeah, it definitely seems like it'd be a pretty big surprise to need this. OK.
https://github.com/llvm/llvm-project/pull/93115
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
https://github.com/efriedma-quic closed https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
RSilicon wrote: Is this worth back porting as it is a bugfix over code gen? https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
AaronBallman wrote: > Is this worth back porting as it is a bugfix over code gen? IMO, it's worth considering, but if we want to go down this route, I think we need to do so relatively quickly -- we have about two weeks until rc3, and given the size of this change, I'm not certain we should try landing it any later than rc3 just due to risk. https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix codegen of consteval functions returning an empty class, and related issues (PR #93115)
efriedma-quic wrote: /cherry-pick 1762e01cca0186f1862db561cfd9019164b8c654 https://github.com/llvm/llvm-project/pull/93115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
