================
@@ -1468,13 +1500,40 @@ mlir::LogicalResult
CIRToLLVMAtomicFetchOpLowering::matchAndRewrite(
}
mlir::LLVM::AtomicOrdering llvmOrder = getLLVMMemOrder(op.getMemOrder());
- llvm::StringRef llvmSyncScope = getLLVMSyncScope(op.getSyncScope());
+ llvm::StringRef llvmSyncScope = getLLVMSyncScope(op.getSyncScope(), op);
mlir::LLVM::AtomicBinOp llvmBinOp =
getLLVMAtomicBinOp(op.getBinop(), isInt, isSignedInt);
auto rmwVal = mlir::LLVM::AtomicRMWOp::create(
rewriter, op.getLoc(), llvmBinOp, adaptor.getPtr(), adaptor.getVal(),
llvmOrder, llvmSyncScope, /*alignment=*/0, op.getIsVolatile());
+ // CIRGen decides the metadata for a C++/HIP atomic from the atomic options
+ // in effect, so those markers are simply carried across.
+ for (llvm::StringRef marker :
+ {cir::CIRDialect::getAMDGPUNoFineGrainedMemoryAttrName(),
+ cir::CIRDialect::getAMDGPUNoRemoteMemoryAttrName(),
+ cir::CIRDialect::getAMDGPUIgnoreDenormalModeAttrName()})
+ if (mlir::Attribute a = op->getAttr(marker))
+ rmwVal->setAttr(marker, a);
----------------
steffenlarsen wrote:
> Does the LLVM dialect use the same name for the attribute? The fact that
> we're using a CIR dialect name to set an LLVM attribute feels brittle.
I don't believe the LLVM dialect has these attributes at all. These are
represented as metadata on the atomics in LLVM IR. The ROCDL dialect has it,
but I'd argue it too small a thing to bring in ROCDL as a dependency for. I do
agree though that the brittleness is awkward, especially since missing it
wouldn't mean failure, just worse atomics. That said, if the chain breaks
somewhere, I would expect testing to catch it.
> I also wondered if the attribute should come from adaptor instead of op.
AFAIK, it shouldn't make a difference for attributes. I won't mind changing it
if we want to keep things consistent though.
https://github.com/llvm/llvm-project/pull/225364
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
