Skip to content

Commit 1bd9d47

Browse files
[CIR] Backport volatile bit-fields (#1857)
This PR backports support for volatile bit-fields in AAPCS. The corresponding upstream PRs are: - llvm/llvm-project#151252 - llvm/llvm-project#151875
1 parent ab5e066 commit 1bd9d47

File tree

4 files changed

+397
-4
lines changed

4 files changed

+397
-4
lines changed

clang/lib/CIR/CodeGen/CIRGenBuilder.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,13 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
779779
const CIRGenBitFieldInfo &info,
780780
bool isLvalueVolatile, bool useVolatile) {
781781
auto offset = useVolatile ? info.VolatileOffset : info.Offset;
782+
783+
// If using AAPCS and the field is volatile, load with the size of the
784+
// declared field
785+
storageType =
786+
useVolatile ? cir::IntType::get(storageType.getContext(),
787+
info.VolatileStorageSize, info.IsSigned)
788+
: storageType;
782789
return cir::GetBitfieldOp::create(*this, loc, resultType, addr.getPointer(),
783790
storageType, info.Name, info.Size, offset,
784791
info.IsSigned, isLvalueVolatile,
@@ -790,6 +797,13 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
790797
mlir::Value src, const CIRGenBitFieldInfo &info,
791798
bool isLvalueVolatile, bool useVolatile) {
792799
auto offset = useVolatile ? info.VolatileOffset : info.Offset;
800+
801+
// If using AAPCS and the field is volatile, load with the size of the
802+
// declared field
803+
storageType =
804+
useVolatile ? cir::IntType::get(storageType.getContext(),
805+
info.VolatileStorageSize, info.IsSigned)
806+
: storageType;
793807
return cir::SetBitfieldOp::create(
794808
*this, loc, resultType, dstAddr.getPointer(), storageType, src,
795809
info.Name, info.Size, offset, info.IsSigned, isLvalueVolatile,

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,7 @@ LValue CIRGenFunction::emitLValueForBitField(LValue base,
290290
auto useVolatile = useVolatileForBitField(CGM, base, info, field);
291291
unsigned Idx = layout.getCIRFieldNo(field);
292292

293-
if (useVolatile ||
294-
(IsInPreservedAIRegion ||
293+
if ((IsInPreservedAIRegion ||
295294
(getDebugInfo() && rec->hasAttr<BPFPreserveAccessIndexAttr>()))) {
296295
llvm_unreachable("NYI");
297296
}

clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,103 @@ void CIRRecordLowering::computeVolatileBitfields() {
405405
!cirGenTypes.getModule().getCodeGenOpts().AAPCSBitfieldWidth)
406406
return;
407407

408-
for ([[maybe_unused]] auto &I : bitFields) {
409-
assert(!cir::MissingFeatures::armComputeVolatileBitfields());
408+
for (auto &[field, info] : bitFields) {
409+
mlir::Type resLTy = cirGenTypes.convertTypeForMem(field->getType());
410+
411+
if (astContext.toBits(astRecordLayout.getAlignment()) <
412+
getSizeInBits(resLTy).getQuantity())
413+
continue;
414+
415+
// CIRRecordLowering::setBitFieldInfo() pre-adjusts the bit-field offsets
416+
// for big-endian targets, but it assumes a container of width
417+
// info.storageSize. Since AAPCS uses a different container size (width
418+
// of the type), we first undo that calculation here and redo it once
419+
// the bit-field offset within the new container is calculated.
420+
const unsigned oldOffset =
421+
isBE() ? info.StorageSize - (info.Offset + info.Size) : info.Offset;
422+
// Offset to the bit-field from the beginning of the struct.
423+
const unsigned absoluteOffset =
424+
astContext.toBits(info.StorageOffset) + oldOffset;
425+
426+
// Container size is the width of the bit-field type.
427+
const unsigned storageSize = getSizeInBits(resLTy).getQuantity();
428+
// Nothing to do if the access uses the desired
429+
// container width and is naturally aligned.
430+
if (info.StorageSize == storageSize && (oldOffset % storageSize == 0))
431+
continue;
432+
433+
// Offset within the container.
434+
unsigned offset = absoluteOffset & (storageSize - 1);
435+
// Bail out if an aligned load of the container cannot cover the entire
436+
// bit-field. This can happen for example, if the bit-field is part of a
437+
// packed struct. AAPCS does not define access rules for such cases, we let
438+
// clang to follow its own rules.
439+
if (offset + info.Size > storageSize)
440+
continue;
441+
442+
// Re-adjust offsets for big-endian targets.
443+
if (isBE())
444+
offset = storageSize - (offset + info.Size);
445+
446+
const CharUnits storageOffset =
447+
astContext.toCharUnitsFromBits(absoluteOffset & ~(storageSize - 1));
448+
const CharUnits end = storageOffset +
449+
astContext.toCharUnitsFromBits(storageSize) -
450+
CharUnits::One();
451+
452+
const ASTRecordLayout &layout =
453+
astContext.getASTRecordLayout(field->getParent());
454+
// If we access outside memory outside the record, than bail out.
455+
const CharUnits recordSize = layout.getSize();
456+
if (end >= recordSize)
457+
continue;
458+
459+
// Bail out if performing this load would access non-bit-fields members.
460+
bool conflict = false;
461+
for (const auto *f : recordDecl->fields()) {
462+
// Allow sized bit-fields overlaps.
463+
if (f->isBitField() && !f->isZeroLengthBitField())
464+
continue;
465+
466+
const CharUnits fOffset = astContext.toCharUnitsFromBits(
467+
layout.getFieldOffset(f->getFieldIndex()));
468+
469+
// As C11 defines, a zero sized bit-field defines a barrier, so
470+
// fields after and before it should be race condition free.
471+
// The AAPCS acknowledges it and imposes no restritions when the
472+
// natural container overlaps a zero-length bit-field.
473+
if (f->isZeroLengthBitField()) {
474+
if (end > fOffset && storageOffset < fOffset) {
475+
conflict = true;
476+
break;
477+
}
478+
}
479+
480+
const CharUnits fEnd =
481+
fOffset +
482+
astContext.toCharUnitsFromBits(
483+
getSizeInBits(cirGenTypes.convertTypeForMem(f->getType()))
484+
.getQuantity()) -
485+
CharUnits::One();
486+
// If no overlap, continue.
487+
if (end < fOffset || fEnd < storageOffset)
488+
continue;
489+
490+
// The desired load overlaps a non-bit-field member, bail out.
491+
conflict = true;
492+
break;
493+
}
494+
495+
if (conflict)
496+
continue;
497+
// Write the new bit-field access parameters.
498+
// As the storage offset now is defined as the number of elements from the
499+
// start of the structure, we should divide the Offset by the element size.
500+
info.VolatileStorageOffset =
501+
storageOffset /
502+
astContext.toCharUnitsFromBits(storageSize).getQuantity();
503+
info.VolatileStorageSize = storageSize;
504+
info.VolatileOffset = offset;
410505
}
411506
}
412507

0 commit comments

Comments
 (0)