Skip to content

Commit

Permalink
Merge pull request #245 from Xilinx/bump_to_df9be017
Browse files Browse the repository at this point in the history
Merge with fixes of df9be01 (2)
  • Loading branch information
mgehre-amd authored Aug 12, 2024
2 parents 3d278a2 + e57d071 commit df3da6d
Show file tree
Hide file tree
Showing 16 changed files with 301 additions and 63 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ Resolutions to C++ Defect Reports
of two types.
(`CWG1719: Layout compatibility and cv-qualification revisited <https://cplusplus.github.io/CWG/issues/1719.html>`_).

- Alignment of members is now respected when evaluating layout compatibility
of structs.
(`CWG2583: Common initial sequence should consider over-alignment <https://cplusplus.github.io/CWG/issues/2583.html>`_).

- ``[[no_unique_address]]`` is now respected when evaluating layout
compatibility of two types.
(`CWG2759: [[no_unique_address] and common initial sequence <https://cplusplus.github.io/CWG/issues/2759.html>`_).
Expand Down
23 changes: 21 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19184,8 +19184,22 @@ static bool isLayoutCompatible(ASTContext &C, EnumDecl *ED1, EnumDecl *ED2) {
}

/// Check if two fields are layout-compatible.
/// Can be used on union members, which are exempt from alignment requirement
/// of common initial sequence.
static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
FieldDecl *Field2) {
FieldDecl *Field2,
bool AreUnionMembers = false) {
const Type *Field1Parent = Field1->getParent()->getTypeForDecl();
const Type *Field2Parent = Field2->getParent()->getTypeForDecl();
assert(((Field1Parent->isStructureOrClassType() &&
Field2Parent->isStructureOrClassType()) ||
(Field1Parent->isUnionType() && Field2Parent->isUnionType())) &&
"Can't evaluate layout compatibility between a struct field and a "
"union field.");
assert(((!AreUnionMembers && Field1Parent->isStructureOrClassType()) ||
(AreUnionMembers && Field1Parent->isUnionType())) &&
"AreUnionMembers should be 'true' for union fields (only).");

if (!isLayoutCompatible(C, Field1->getType(), Field2->getType()))
return false;

Expand All @@ -19204,6 +19218,11 @@ static bool isLayoutCompatible(ASTContext &C, FieldDecl *Field1,
if (Field1->hasAttr<clang::NoUniqueAddressAttr>() ||
Field2->hasAttr<clang::NoUniqueAddressAttr>())
return false;

if (!AreUnionMembers &&
Field1->getMaxAlignment() != Field2->getMaxAlignment())
return false;

return true;
}

Expand Down Expand Up @@ -19265,7 +19284,7 @@ static bool isLayoutCompatibleUnion(ASTContext &C, RecordDecl *RD1,
E = UnmatchedFields.end();

for ( ; I != E; ++I) {
if (isLayoutCompatible(C, Field1, *I)) {
if (isLayoutCompatible(C, Field1, *I, /*IsUnionMember=*/true)) {
bool Result = UnmatchedFields.erase(*I);
(void) Result;
assert(Result);
Expand Down
26 changes: 26 additions & 0 deletions clang/test/CXX/drs/dr25xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,32 @@ namespace dr2565 { // dr2565: 16 open 2023-06-07
#endif
}

namespace dr2583 { // dr2583: 19
#if __cplusplus >= 201103L
struct A {
int i;
char c;
};

struct B {
int i;
alignas(8) char c;
};

union U {
A a;
B b;
};

union V {
A a;
alignas(64) B b;
};

static_assert(!__is_layout_compatible(A, B), "");
static_assert(__is_layout_compatible(U, V), "");
#endif
} // namespace dr2583

namespace dr2598 { // dr2598: 18
#if __cplusplus >= 201103L
Expand Down
13 changes: 12 additions & 1 deletion clang/test/SemaCXX/type-traits.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,16 @@ union UnionLayout3 {
[[no_unique_address]] CEmptyStruct d;
};

union UnionNoOveralignedMembers {
int a;
double b;
};

union UnionWithOveralignedMembers {
int a;
alignas(16) double b;
};

struct StructWithAnonUnion {
union {
int a;
Expand Down Expand Up @@ -1771,7 +1781,8 @@ void is_layout_compatible(int n)
static_assert(__is_layout_compatible(CStruct, CStructNoUniqueAddress) != bool(__has_cpp_attribute(no_unique_address)), "");
static_assert(__is_layout_compatible(CStructNoUniqueAddress, CStructNoUniqueAddress2) != bool(__has_cpp_attribute(no_unique_address)), "");
static_assert(__is_layout_compatible(CStruct, CStructAlignment), "");
static_assert(__is_layout_compatible(CStruct, CStructAlignedMembers), ""); // FIXME: alignment of members impact common initial sequence
static_assert(!__is_layout_compatible(CStruct, CStructAlignedMembers), "");
static_assert(__is_layout_compatible(UnionNoOveralignedMembers, UnionWithOveralignedMembers), "");
static_assert(__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds), "");
static_assert(__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds2), "");
static_assert(!__is_layout_compatible(CStructWithBitfelds, CStructWithBitfelds3), "");
Expand Down
2 changes: 1 addition & 1 deletion clang/www/cxx_dr_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -15306,7 +15306,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2583.html">2583</a></td>
<td>C++23</td>
<td>Common initial sequence should consider over-alignment</td>
<td class="unknown" align="center">Unknown</td>
<td class="unreleased" align="center">Clang 19</td>
</tr>
<tr class="open" id="2584">
<td><a href="https://cplusplus.github.io/CWG/issues/2584.html">2584</a></td>
Expand Down
9 changes: 5 additions & 4 deletions llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2997,13 +2997,14 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
}
}

if (IsZExtLoad) {
// The zextload from a smaller type to i32 should be handled by the
if (IsZExtLoad || (Opcode == TargetOpcode::G_LOAD &&
ValTy == LLT::scalar(64) && MemSizeInBits == 32)) {
// The any/zextload from a smaller type to i32 should be handled by the
// importer.
if (MRI.getType(LoadStore->getOperand(0).getReg()).getSizeInBits() != 64)
return false;
// If we have a ZEXTLOAD then change the load's type to be a narrower reg
// and zero_extend with SUBREG_TO_REG.
// If we have an extending load then change the load's type to be a
// narrower reg and zero_extend with SUBREG_TO_REG.
Register LdReg = MRI.createVirtualRegister(&AArch64::GPR32RegClass);
Register DstReg = LoadStore->getOperand(0).getReg();
LoadStore->getOperand(0).setReg(LdReg);
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/InstVisitor.h"
#include "llvm/IR/IntrinsicsSPIRV.h"
#include "llvm/IR/TypedPointerType.h"

#include <queue>

Expand Down Expand Up @@ -446,7 +447,8 @@ void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,

for (unsigned OpIdx = 0; OpIdx < CI->arg_size(); OpIdx++) {
Value *ArgOperand = CI->getArgOperand(OpIdx);
if (!isa<PointerType>(ArgOperand->getType()))
if (!isa<PointerType>(ArgOperand->getType()) &&
!isa<TypedPointerType>(ArgOperand->getType()))
continue;

// Constants (nulls/undefs) are handled in insertAssignPtrTypeIntrs()
Expand Down
93 changes: 48 additions & 45 deletions llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "SPIRVSubtarget.h"
#include "SPIRVTargetMachine.h"
#include "SPIRVUtils.h"
#include "llvm/IR/TypedPointerType.h"

using namespace llvm;
SPIRVGlobalRegistry::SPIRVGlobalRegistry(unsigned PointerSize)
Expand Down Expand Up @@ -420,9 +421,10 @@ Register
SPIRVGlobalRegistry::getOrCreateConstNullPtr(MachineIRBuilder &MIRBuilder,
SPIRVType *SpvType) {
const Type *LLVMTy = getTypeForSPIRVType(SpvType);
const PointerType *LLVMPtrTy = cast<PointerType>(LLVMTy);
const TypedPointerType *LLVMPtrTy = cast<TypedPointerType>(LLVMTy);
// Find a constant in DT or build a new one.
Constant *CP = ConstantPointerNull::get(const_cast<PointerType *>(LLVMPtrTy));
Constant *CP = ConstantPointerNull::get(PointerType::get(
LLVMPtrTy->getElementType(), LLVMPtrTy->getAddressSpace()));
Register Res = DT.find(CP, CurMF);
if (!Res.isValid()) {
LLT LLTy = LLT::pointer(LLVMPtrTy->getAddressSpace(), PointerSize);
Expand Down Expand Up @@ -517,6 +519,13 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
LLT RegLLTy = LLT::pointer(MRI->getType(ResVReg).getAddressSpace(), 32);
MRI->setType(Reg, RegLLTy);
assignSPIRVTypeToVReg(BaseType, Reg, MIRBuilder.getMF());
} else {
// Our knowledge about the type may be updated.
// If that's the case, we need to update a type
// associated with the register.
SPIRVType *DefType = getSPIRVTypeForVReg(ResVReg);
if (!DefType || DefType != BaseType)
assignSPIRVTypeToVReg(BaseType, Reg, MIRBuilder.getMF());
}

// If it's a global variable with name, output OpName for it.
Expand Down Expand Up @@ -705,33 +714,37 @@ SPIRVType *SPIRVGlobalRegistry::createSPIRVType(
}
return getOpTypeFunction(RetTy, ParamTypes, MIRBuilder);
}
if (auto PType = dyn_cast<PointerType>(Ty)) {
SPIRVType *SpvElementType;
// At the moment, all opaque pointers correspond to i8 element type.
// TODO: change the implementation once opaque pointers are supported
// in the SPIR-V specification.
SpvElementType = getOrCreateSPIRVIntegerType(8, MIRBuilder);
// Get access to information about available extensions
const SPIRVSubtarget *ST =
static_cast<const SPIRVSubtarget *>(&MIRBuilder.getMF().getSubtarget());
auto SC = addressSpaceToStorageClass(PType->getAddressSpace(), *ST);
// Null pointer means we have a loop in type definitions, make and
// return corresponding OpTypeForwardPointer.
if (SpvElementType == nullptr) {
if (!ForwardPointerTypes.contains(Ty))
ForwardPointerTypes[PType] = getOpTypeForwardPointer(SC, MIRBuilder);
return ForwardPointerTypes[PType];
}
// If we have forward pointer associated with this type, use its register
// operand to create OpTypePointer.
if (ForwardPointerTypes.contains(PType)) {
Register Reg = getSPIRVTypeID(ForwardPointerTypes[PType]);
return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
}

return getOrCreateSPIRVPointerType(SpvElementType, MIRBuilder, SC);
unsigned AddrSpace = 0xFFFF;
if (auto PType = dyn_cast<TypedPointerType>(Ty))
AddrSpace = PType->getAddressSpace();
else if (auto PType = dyn_cast<PointerType>(Ty))
AddrSpace = PType->getAddressSpace();
else
report_fatal_error("Unable to convert LLVM type to SPIRVType", true);
SPIRVType *SpvElementType;
// At the moment, all opaque pointers correspond to i8 element type.
// TODO: change the implementation once opaque pointers are supported
// in the SPIR-V specification.
SpvElementType = getOrCreateSPIRVIntegerType(8, MIRBuilder);
// Get access to information about available extensions
const SPIRVSubtarget *ST =
static_cast<const SPIRVSubtarget *>(&MIRBuilder.getMF().getSubtarget());
auto SC = addressSpaceToStorageClass(AddrSpace, *ST);
// Null pointer means we have a loop in type definitions, make and
// return corresponding OpTypeForwardPointer.
if (SpvElementType == nullptr) {
if (!ForwardPointerTypes.contains(Ty))
ForwardPointerTypes[Ty] = getOpTypeForwardPointer(SC, MIRBuilder);
return ForwardPointerTypes[Ty];
}
// If we have forward pointer associated with this type, use its register
// operand to create OpTypePointer.
if (ForwardPointerTypes.contains(Ty)) {
Register Reg = getSPIRVTypeID(ForwardPointerTypes[Ty]);
return getOpTypePointer(SC, SpvElementType, MIRBuilder, Reg);
}
llvm_unreachable("Unable to convert LLVM type to SPIRVType");

return getOrCreateSPIRVPointerType(SpvElementType, MIRBuilder, SC);
}

SPIRVType *SPIRVGlobalRegistry::restOfCreateSPIRVType(
Expand Down Expand Up @@ -1139,11 +1152,13 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
SPIRV::StorageClass::StorageClass SC) {
const Type *PointerElementType = getTypeForSPIRVType(BaseType);
unsigned AddressSpace = storageClassToAddressSpace(SC);
Type *LLVMTy =
PointerType::get(const_cast<Type *>(PointerElementType), AddressSpace);
Type *LLVMTy = TypedPointerType::get(const_cast<Type *>(PointerElementType),
AddressSpace);
// check if this type is already available
Register Reg = DT.find(PointerElementType, AddressSpace, CurMF);
if (Reg.isValid())
return getSPIRVTypeForVReg(Reg);
// create a new type
auto MIB = BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
MIRBuilder.getDebugLoc(),
MIRBuilder.getTII().get(SPIRV::OpTypePointer))
Expand All @@ -1155,22 +1170,10 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
}

SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
SPIRVType *BaseType, MachineInstr &I, const SPIRVInstrInfo &TII,
SPIRVType *BaseType, MachineInstr &I, const SPIRVInstrInfo &,
SPIRV::StorageClass::StorageClass SC) {
const Type *PointerElementType = getTypeForSPIRVType(BaseType);
unsigned AddressSpace = storageClassToAddressSpace(SC);
Type *LLVMTy =
PointerType::get(const_cast<Type *>(PointerElementType), AddressSpace);
Register Reg = DT.find(PointerElementType, AddressSpace, CurMF);
if (Reg.isValid())
return getSPIRVTypeForVReg(Reg);
MachineBasicBlock &BB = *I.getParent();
auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpTypePointer))
.addDef(createTypeVReg(CurMF->getRegInfo()))
.addImm(static_cast<uint32_t>(SC))
.addUse(getSPIRVTypeID(BaseType));
DT.add(PointerElementType, AddressSpace, CurMF, getSPIRVTypeID(MIB));
return finishCreatingSPIRVType(LLVMTy, MIB);
MachineIRBuilder MIRBuilder(I);
return getOrCreateSPIRVPointerType(BaseType, MIRBuilder, SC);
}

Register SPIRVGlobalRegistry::getOrCreateUndef(MachineInstr &I,
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class SPIRVGlobalRegistry {
DenseMap<const MachineFunction *, DenseMap<Register, SPIRVType *>>
VRegToTypeMap;

// Map LLVM Type* to <MF, Reg>
SPIRVGeneralDuplicatesTracker DT;

DenseMap<SPIRVType *, const Type *> SPIRVToLLVMType;
Expand Down
Loading

0 comments on commit df3da6d

Please sign in to comment.