Skip to content

Commit 95d3ece

Browse files
andykaylorandykaylor
authored
[CIR] Add handling for volatile loads and stores (#156124)
This fills in the missing pieces to handle volatile loads and stores in CIR. This addresses #153280
1 parent 08001cf commit 95d3ece

File tree

9 files changed

+264
-24
lines changed

9 files changed

+264
-24
lines changed

‎clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,15 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
160160
}
161161

162162
cir::LoadOp createLoad(mlir::Location loc, mlir::Value ptr,
163-
uint64_t alignment = 0) {
163+
bool isVolatile = false, uint64_t alignment = 0) {
164164
mlir::IntegerAttr alignmentAttr = getAlignmentAttr(alignment);
165-
assert(!cir::MissingFeatures::opLoadStoreVolatile());
166-
return cir::LoadOp::create(*this, loc, ptr, /*isDeref=*/false,
165+
return cir::LoadOp::create(*this, loc, ptr, /*isDeref=*/false, isVolatile,
167166
alignmentAttr, cir::MemOrderAttr{});
168167
}
169168

170169
mlir::Value createAlignedLoad(mlir::Location loc, mlir::Value ptr,
171170
uint64_t alignment) {
172-
return createLoad(loc, ptr, alignment);
171+
return createLoad(loc, ptr, /*isVolatile=*/false, alignment);
173172
}
174173

175174
mlir::Value createNot(mlir::Value value) {
@@ -250,7 +249,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
250249
bool isVolatile = false,
251250
mlir::IntegerAttr align = {},
252251
cir::MemOrderAttr order = {}) {
253-
return cir::StoreOp::create(*this, loc, val, dst, align, order);
252+
return cir::StoreOp::create(*this, loc, val, dst, isVolatile, align, order);
254253
}
255254

256255
[[nodiscard]] cir::GlobalOp createGlobal(mlir::ModuleOp mlirModule,
@@ -274,7 +273,8 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
274273
mlir::IntegerAttr alignmentAttr = getAlignmentAttr(alignment);
275274
auto addr = createAlloca(loc, getPointerTo(type), type, {}, alignmentAttr);
276275
return cir::LoadOp::create(*this, loc, addr, /*isDeref=*/false,
277-
alignmentAttr, /*mem_order=*/{});
276+
/*isVolatile=*/false, alignmentAttr,
277+
/*mem_order=*/{});
278278
}
279279

280280
cir::PtrStrideOp createPtrStride(mlir::Location loc, mlir::Value base,

‎clang/include/clang/CIR/Dialect/IR/CIROps.td

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,11 @@ def CIR_LoadOp : CIR_Op<"load", [
404404
`cir.load` reads a value (lvalue to rvalue conversion) given an address
405405
backed up by a `cir.ptr` type. A unit attribute `deref` can be used to
406406
mark the resulting value as used by another operation to dereference
407-
a pointer.
407+
a pointer. A unit attribute `volatile` can be used to indicate a volatile
408+
loading. Load can be marked atomic by using `atomic(<mem_order>)`.
409+
410+
`alignment` can be used to specify an alignment that's different from the
411+
default, which is computed from `result`'s type ABI data layout.
408412

409413
Example:
410414

@@ -416,18 +420,26 @@ def CIR_LoadOp : CIR_Op<"load", [
416420
// Load address from memory at address %0. %3 is used by at least one
417421
// operation that dereferences a pointer.
418422
%3 = cir.load deref %0 : !cir.ptr<!cir.ptr<i32>>
423+
424+
// Perform a volatile load from address in %0.
425+
%4 = cir.load volatile %0 : !cir.ptr<i32>, i32
426+
427+
// Others
428+
%x = cir.load align(16) atomic(seq_cst) %0 : !cir.ptr<i32>, i32
419429
```
420430
}];
421431

422432
let arguments = (ins Arg<CIR_PointerType, "the address to load from",
423433
[MemRead]>:$addr,
424434
UnitAttr:$isDeref,
435+
UnitAttr:$is_volatile,
425436
OptionalAttr<I64Attr>:$alignment,
426437
OptionalAttr<CIR_MemOrder>:$mem_order);
427438
let results = (outs CIR_AnyType:$result);
428439

429440
let assemblyFormat = [{
430441
(`deref` $isDeref^)?
442+
(`volatile` $is_volatile^)?
431443
(`align` `(` $alignment^ `)`)?
432444
(`atomic` `(` $mem_order^ `)`)?
433445
$addr `:` qualified(type($addr)) `,` type($result) attr-dict
@@ -452,24 +464,32 @@ def CIR_StoreOp : CIR_Op<"store", [
452464
a volatile store. Store's can be marked atomic by using
453465
`atomic(<mem_order>)`.
454466

455-
`align` can be used to specify an alignment that's different from the
467+
`alignment` can be used to specify an alignment that's different from the
456468
default, which is computed from `result`'s type ABI data layout.
457469

458470
Example:
459471

460472
```mlir
461473
// Store a function argument to local storage, address in %0.
462474
cir.store %arg0, %0 : i32, !cir.ptr<i32>
475+
476+
// Perform a volatile store into memory location at the address in %0.
477+
cir.store volatile %arg0, %0 : i32, !cir.ptr<i32>
478+
479+
// Others
480+
cir.store align(16) atomic(seq_cst) %x, %addr : i32, !cir.ptr<i32>
463481
```
464482
}];
465483

466484
let arguments = (ins CIR_AnyType:$value,
467485
Arg<CIR_PointerType, "the address to store the value",
468486
[MemWrite]>:$addr,
487+
UnitAttr:$is_volatile,
469488
OptionalAttr<I64Attr>:$alignment,
470489
OptionalAttr<CIR_MemOrder>:$mem_order);
471490

472491
let assemblyFormat = [{
492+
(`volatile` $is_volatile^)?
473493
(`align` `(` $alignment^ `)`)?
474494
(`atomic` `(` $mem_order^ `)`)?
475495
$value `,` $addr attr-dict `:` type($value) `,` qualified(type($addr))

‎clang/include/clang/CIR/MissingFeatures.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,8 @@ struct MissingFeatures {
4747
// Load/store attributes
4848
static bool opLoadStoreThreadLocal() { return false; }
4949
static bool opLoadEmitScalarRangeCheck() { return false; }
50-
static bool opLoadBooleanRepresentation() { return false; }
50+
static bool opLoadStoreNontemporal() { return false; }
5151
static bool opLoadStoreTbaa() { return false; }
52-
static bool opLoadStoreVolatile() { return false; }
5352
static bool opLoadStoreAtomic() { return false; }
5453
static bool opLoadStoreObjC() { return false; }
5554

‎clang/lib/CIR/CodeGen/CIRGenBuilder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
375375
bool isVolatile = false) {
376376
mlir::IntegerAttr align = getAlignmentAttr(addr.getAlignment());
377377
return cir::LoadOp::create(*this, loc, addr.getPointer(), /*isDeref=*/false,
378-
/*alignment=*/align,
378+
isVolatile, /*alignment=*/align,
379379
/*mem_order=*/cir::MemOrderAttr{});
380380
}
381381

‎clang/lib/CIR/CodeGen/CIRGenExpr.cpp

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ void CIRGenFunction::emitStoreOfScalar(mlir::Value value, Address addr,
325325
const auto vecTy = cast<cir::VectorType>(elementType);
326326

327327
// TODO(CIR): Use `ABIInfo::getOptimalVectorMemoryType` once it upstreamed
328+
assert(!cir::MissingFeatures::cirgenABIInfo());
328329
if (vecTy.getSize() == 3 && !getLangOpts().PreserveVec3Type)
329330
cgm.errorNYI(addr.getPointer().getLoc(),
330331
"emitStoreOfScalar Vec3 & PreserveVec3Type disabled");
@@ -345,7 +346,7 @@ void CIRGenFunction::emitStoreOfScalar(mlir::Value value, Address addr,
345346
}
346347

347348
assert(currSrcLoc && "must pass in source location");
348-
builder.createStore(*currSrcLoc, value, addr /*, isVolatile*/);
349+
builder.createStore(*currSrcLoc, value, addr, isVolatile);
349350

350351
if (isNontemporal) {
351352
cgm.errorNYI(addr.getPointer().getLoc(), "emitStoreOfScalar nontemporal");
@@ -543,23 +544,52 @@ void CIRGenFunction::emitStoreOfScalar(mlir::Value value, LValue lvalue,
543544
lvalue.getType(), isInit, /*isNontemporal=*/false);
544545
}
545546

546-
mlir::Value CIRGenFunction::emitLoadOfScalar(LValue lvalue,
547-
SourceLocation loc) {
547+
mlir::Value CIRGenFunction::emitLoadOfScalar(Address addr, bool isVolatile,
548+
QualType ty, SourceLocation loc,
549+
LValueBaseInfo baseInfo) {
548550
assert(!cir::MissingFeatures::opLoadStoreThreadLocal());
549-
assert(!cir::MissingFeatures::opLoadEmitScalarRangeCheck());
550-
assert(!cir::MissingFeatures::opLoadBooleanRepresentation());
551-
552-
Address addr = lvalue.getAddress();
553551
mlir::Type eltTy = addr.getElementType();
554552

553+
if (const auto *clangVecTy = ty->getAs<clang::VectorType>()) {
554+
if (clangVecTy->isExtVectorBoolType()) {
555+
cgm.errorNYI(loc, "emitLoadOfScalar: ExtVectorBoolType");
556+
return nullptr;
557+
}
558+
559+
const auto vecTy = cast<cir::VectorType>(eltTy);
560+
561+
// Handle vectors of size 3 like size 4 for better performance.
562+
assert(!cir::MissingFeatures::cirgenABIInfo());
563+
if (vecTy.getSize() == 3 && !getLangOpts().PreserveVec3Type)
564+
cgm.errorNYI(addr.getPointer().getLoc(),
565+
"emitLoadOfScalar Vec3 & PreserveVec3Type disabled");
566+
}
567+
568+
assert(!cir::MissingFeatures::opLoadStoreTbaa());
569+
LValue atomicLValue = LValue::makeAddr(addr, ty, baseInfo);
570+
if (ty->isAtomicType() || isLValueSuitableForInlineAtomic(atomicLValue))
571+
cgm.errorNYI("emitLoadOfScalar: load atomic");
572+
555573
if (mlir::isa<cir::VoidType>(eltTy))
556574
cgm.errorNYI(loc, "emitLoadOfScalar: void type");
557575

558-
mlir::Value loadOp = builder.createLoad(getLoc(loc), addr);
576+
assert(!cir::MissingFeatures::opLoadEmitScalarRangeCheck());
577+
578+
mlir::Value loadOp = builder.createLoad(getLoc(loc), addr, isVolatile);
579+
if (!ty->isBooleanType() && ty->hasBooleanRepresentation())
580+
cgm.errorNYI("emitLoadOfScalar: boolean type with boolean representation");
559581

560582
return loadOp;
561583
}
562584

585+
mlir::Value CIRGenFunction::emitLoadOfScalar(LValue lvalue,
586+
SourceLocation loc) {
587+
assert(!cir::MissingFeatures::opLoadStoreNontemporal());
588+
assert(!cir::MissingFeatures::opLoadStoreTbaa());
589+
return emitLoadOfScalar(lvalue.getAddress(), lvalue.isVolatile(),
590+
lvalue.getType(), loc, lvalue.getBaseInfo());
591+
}
592+
563593
/// Given an expression that represents a value lvalue, this
564594
/// method emits the address of the lvalue, then loads the result as an rvalue,
565595
/// returning the rvalue.

‎clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
101101
void VisitCastExpr(CastExpr *e) {
102102
switch (e->getCastKind()) {
103103
case CK_LValueToRValue:
104-
assert(!cir::MissingFeatures::aggValueSlotVolatile());
104+
// If we're loading from a volatile type, force the destination
105+
// into existence.
106+
if (e->getSubExpr()->getType().isVolatileQualified())
107+
cgf.cgm.errorNYI(e->getSourceRange(),
108+
"AggExprEmitter: volatile lvalue-to-rvalue cast");
105109
[[fallthrough]];
106110
case CK_NoOp:
107111
case CK_UserDefinedConversion:

‎clang/lib/CIR/CodeGen/CIRGenFunction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,8 @@ class CIRGenFunction : public CIRGenTypeCache {
12921292
/// the LLVM value representation. The l-value must be a simple
12931293
/// l-value.
12941294
mlir::Value emitLoadOfScalar(LValue lvalue, SourceLocation loc);
1295+
mlir::Value emitLoadOfScalar(Address addr, bool isVolatile, QualType ty,
1296+
SourceLocation loc, LValueBaseInfo baseInfo);
12951297

12961298
/// Emit code to compute a designator that specifies the location
12971299
/// of the expression.

‎clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,10 +1214,10 @@ mlir::LogicalResult CIRToLLVMLoadOpLowering::matchAndRewrite(
12141214
assert(!cir::MissingFeatures::lowerModeOptLevel());
12151215

12161216
// TODO: nontemporal, syncscope.
1217-
assert(!cir::MissingFeatures::opLoadStoreVolatile());
1217+
assert(!cir::MissingFeatures::opLoadStoreNontemporal());
12181218
mlir::LLVM::LoadOp newLoad = mlir::LLVM::LoadOp::create(
12191219
rewriter, op->getLoc(), llvmTy, adaptor.getAddr(), alignment,
1220-
/*isVolatile=*/false, /*isNonTemporal=*/false,
1220+
op.getIsVolatile(), /*isNonTemporal=*/false,
12211221
/*isInvariant=*/false, /*isInvariantGroup=*/false, ordering);
12221222

12231223
// Convert adapted result to its original type if needed.
@@ -1244,10 +1244,11 @@ mlir::LogicalResult CIRToLLVMStoreOpLowering::matchAndRewrite(
12441244
mlir::Value value = emitToMemory(rewriter, dataLayout,
12451245
op.getValue().getType(), adaptor.getValue());
12461246
// TODO: nontemporal, syncscope.
1247-
assert(!cir::MissingFeatures::opLoadStoreVolatile());
1247+
assert(!cir::MissingFeatures::opLoadStoreNontemporal());
1248+
assert(!cir::MissingFeatures::opLoadStoreTbaa());
12481249
mlir::LLVM::StoreOp storeOp = mlir::LLVM::StoreOp::create(
12491250
rewriter, op->getLoc(), value, adaptor.getAddr(), alignment,
1250-
/*isVolatile=*/false,
1251+
op.getIsVolatile(),
12511252
/*isNonTemporal=*/false, /*isInvariantGroup=*/false, memorder);
12521253
rewriter.replaceOp(op, storeOp);
12531254
assert(!cir::MissingFeatures::opLoadStoreTbaa());

0 commit comments

Comments
 (0)