-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[CIR] Add support for __builtin_assume_aligned #152152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Sirui Mu (Lancern) ChangesThis patch upstreams CIRGen and LLVM lowering support for the Full diff: https://github.com/llvm/llvm-project/pull/152152.diff 9 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 72841a1c08441..f24c43eb872b7 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -3143,6 +3143,45 @@ def CIR_AssumeOp : CIR_Op<"assume"> {
}];
}
+def CIR_AssumeAlignedOp : CIR_Op<"assume_aligned", [
+ Pure, AllTypesMatch<["pointer", "result"]>
+]> {
+ let summary = "Tell the optimizer that a pointer is aligned";
+ let description = [{
+ The `cir.assume_aligned` operation takes two or three arguments. The first
+ argument `pointer` gives the pointer value whose alignment is to be assumed,
+ and the second argument `align` is an integer attribute that gives the
+ assumed alignment.
+
+ The `offset` argument is optional. If given, it represents misalignment
+ offset. When it's present, this operation tells the optimizer that the
+ pointer is always misaligned to the alignment by `offset` bytes, a.k.a. the
+ pointer yielded by `(char *)pointer - offset` is aligned to the specified
+ alignment. Note that the `offset` argument is an SSA value rather than an
+ attribute, which means that you could pass a dynamically determined value
+ as the mialignment offset.
+
+ The result of this operation has the same value as the `pointer` argument,
+ but it additionally carries any alignment information indicated by this
+ operation.
+
+ This operation corresponds to the `__builtin_assume_aligned` builtin
+ function.
+ }];
+
+ let arguments = (ins CIR_PointerType:$pointer,
+ I64Attr:$alignment,
+ Optional<CIR_IntType>:$offset);
+ let results = (outs CIR_PointerType:$result);
+
+ let assemblyFormat = [{
+ $pointer
+ `alignment` $alignment
+ (`[` `offset` $offset^ `:` type($offset) `]`)?
+ `:` qualified(type($pointer)) attr-dict
+ }];
+}
+
def CIR_AssumeSepStorageOp : CIR_Op<"assume_separate_storage", [
SameTypeOperands
]> {
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
index 7767bf481c795..ff6d1b8ce3ad9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
@@ -129,6 +129,23 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID,
return RValue::get(nullptr);
}
+ case Builtin::BI__builtin_assume_aligned: {
+ const Expr *ptrExpr = e->getArg(0);
+ mlir::Value ptrValue = emitScalarExpr(ptrExpr);
+ mlir::Value offsetValue =
+ (e->getNumArgs() > 2) ? emitScalarExpr(e->getArg(2)) : nullptr;
+
+ const Expr *alignmentExpr = e->getArg(1);
+ mlir::Attribute alignmentAttr = ConstantEmitter(*this).emitAbstract(
+ alignmentExpr, alignmentExpr->getType());
+ int64_t alignment = mlir::cast<cir::IntAttr>(alignmentAttr).getSInt();
+
+ mlir::Value result = emitAlignmentAssumption(
+ ptrValue, ptrExpr, ptrExpr->getExprLoc(),
+ builder.getI64IntegerAttr(alignment), offsetValue);
+ return RValue::get(result);
+ }
+
case Builtin::BI__builtin_complex: {
mlir::Value real = emitScalarExpr(e->getArg(0));
mlir::Value imag = emitScalarExpr(e->getArg(1));
diff --git a/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
index d6dac50bb1263..0777ed246eb61 100644
--- a/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
+++ b/clang/lib/CIR/CodeGen/CIRGenConstantEmitter.h
@@ -90,6 +90,7 @@ class ConstantEmitter {
/// asserting that it succeeded. This is only safe to do when the
/// expression is known to be a constant expression with either a fairly
/// simple type or a known simple form.
+ mlir::Attribute emitAbstract(const Expr *e, QualType ty);
mlir::Attribute emitAbstract(SourceLocation loc, const APValue &value,
QualType t);
@@ -100,6 +101,7 @@ class ConstantEmitter {
// functions and classes.
mlir::Attribute tryEmitPrivateForVarInit(const VarDecl &d);
+ mlir::TypedAttr tryEmitPrivate(const Expr *e, QualType ty);
mlir::Attribute tryEmitPrivate(const APValue &value, QualType destType);
mlir::Attribute tryEmitPrivateForMemory(const APValue &value, QualType t);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index 5b3bf85cefbb0..d6a19e16eec7a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -650,6 +650,38 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForMemory(const APValue &value,
return (c ? emitForMemory(c, destType) : nullptr);
}
+mlir::TypedAttr ConstantEmitter::tryEmitPrivate(const Expr *e, QualType ty) {
+ assert(!ty->isVoidType() && "can't emit a void constant");
+
+ if (mlir::Attribute attr =
+ ConstExprEmitter(*this).Visit(const_cast<Expr *>(e), ty))
+ return mlir::cast<mlir::TypedAttr>(attr);
+
+ Expr::EvalResult result;
+
+ bool success = false;
+ if (ty->isReferenceType())
+ success = e->EvaluateAsLValue(result, cgm.getASTContext());
+ else
+ success =
+ e->EvaluateAsRValue(result, cgm.getASTContext(), inConstantContext);
+
+ if (success && !result.hasSideEffects()) {
+ mlir::Attribute attr = tryEmitPrivate(result.Val, ty);
+ return mlir::cast<mlir::TypedAttr>(attr);
+ }
+
+ return nullptr;
+}
+
+mlir::Attribute ConstantEmitter::emitAbstract(const Expr *e, QualType ty) {
+ AbstractStateRAII state(*this, true);
+ auto attr = mlir::cast<mlir::Attribute>(tryEmitPrivate(e, ty));
+ if (!attr)
+ cgm.errorNYI(e->getExprLoc(), "emitAbstract failed, emit null constant");
+ return attr;
+}
+
mlir::Attribute ConstantEmitter::emitAbstract(SourceLocation loc,
const APValue &value,
QualType destType) {
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index eb05c93d52891..ee1d1662b504a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -931,6 +931,24 @@ CIRGenFunction::emitArrayLength(const clang::ArrayType *origArrayType,
return builder.getConstInt(*currSrcLoc, SizeTy, countFromCLAs);
}
+mlir::Value CIRGenFunction::emitAlignmentAssumption(
+ mlir::Value ptrValue, QualType ty, SourceLocation loc,
+ SourceLocation assumptionLoc, mlir::IntegerAttr alignment,
+ mlir::Value offsetValue) {
+ assert(!cir::MissingFeatures::sanitizers());
+ return cir::AssumeAlignedOp::create(builder, getLoc(assumptionLoc), ptrValue,
+ alignment, offsetValue);
+}
+
+mlir::Value CIRGenFunction::emitAlignmentAssumption(
+ mlir::Value ptrValue, const Expr *expr, SourceLocation assumptionLoc,
+ mlir::IntegerAttr alignment, mlir::Value offsetValue) {
+ QualType ty = expr->getType();
+ SourceLocation loc = expr->getExprLoc();
+ return emitAlignmentAssumption(ptrValue, ty, loc, assumptionLoc, alignment,
+ offsetValue);
+}
+
// TODO(cir): Most of this function can be shared between CIRGen
// and traditional LLVM codegen
void CIRGenFunction::emitVariablyModifiedType(QualType type) {
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 3d92545a4ae3c..0c48591359f14 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -825,6 +825,18 @@ class CIRGenFunction : public CIRGenTypeCache {
/// ----------------------
/// CIR emit functions
/// ----------------------
+public:
+ mlir::Value emitAlignmentAssumption(mlir::Value ptrValue, QualType ty,
+ SourceLocation loc,
+ SourceLocation assumptionLoc,
+ mlir::IntegerAttr alignment,
+ mlir::Value offsetValue = nullptr);
+
+ mlir::Value emitAlignmentAssumption(mlir::Value ptrValue, const Expr *expr,
+ SourceLocation assumptionLoc,
+ mlir::IntegerAttr alignment,
+ mlir::Value offsetValue = nullptr);
+
private:
void emitAndUpdateRetAlloca(clang::QualType type, mlir::Location loc,
clang::CharUnits alignment);
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 7e1c9fb9164cd..91cd92630b931 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -460,6 +460,28 @@ mlir::LogicalResult CIRToLLVMAssumeOpLowering::matchAndRewrite(
return mlir::success();
}
+mlir::LogicalResult CIRToLLVMAssumeAlignedOpLowering::matchAndRewrite(
+ cir::AssumeAlignedOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const {
+ SmallVector<mlir::Value, 3> opBundleArgs{adaptor.getPointer()};
+
+ auto alignment = rewriter.create<mlir::LLVM::ConstantOp>(
+ op.getLoc(), rewriter.getI64Type(), op.getAlignment());
+ opBundleArgs.push_back(alignment);
+
+ if (mlir::Value offset = adaptor.getOffset())
+ opBundleArgs.push_back(offset);
+
+ auto cond = rewriter.create<mlir::LLVM::ConstantOp>(op.getLoc(),
+ rewriter.getI1Type(), 1);
+ rewriter.create<mlir::LLVM::AssumeOp>(op.getLoc(), cond, "align",
+ opBundleArgs);
+ rewriter.replaceAllUsesWith(op, op.getPointer());
+ rewriter.eraseOp(op);
+
+ return mlir::success();
+}
+
mlir::LogicalResult CIRToLLVMAssumeSepStorageOpLowering::matchAndRewrite(
cir::AssumeSepStorageOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
@@ -2168,6 +2190,7 @@ void ConvertCIRToLLVMPass::runOnOperation() {
patterns.add<
// clang-format off
CIRToLLVMAssumeOpLowering,
+ CIRToLLVMAssumeAlignedOpLowering,
CIRToLLVMAssumeSepStorageOpLowering,
CIRToLLVMBaseClassAddrOpLowering,
CIRToLLVMBinOpLowering,
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
index c5106cb33f452..51b191af24692 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
@@ -44,6 +44,16 @@ class CIRToLLVMAssumeOpLowering
mlir::ConversionPatternRewriter &) const override;
};
+class CIRToLLVMAssumeAlignedOpLowering
+ : public mlir::OpConversionPattern<cir::AssumeAlignedOp> {
+public:
+ using mlir::OpConversionPattern<cir::AssumeAlignedOp>::OpConversionPattern;
+
+ mlir::LogicalResult
+ matchAndRewrite(cir::AssumeAlignedOp op, OpAdaptor,
+ mlir::ConversionPatternRewriter &) const override;
+};
+
class CIRToLLVMAssumeSepStorageOpLowering
: public mlir::OpConversionPattern<cir::AssumeSepStorageOp> {
public:
diff --git a/clang/test/CIR/CodeGen/builtin_call.cpp b/clang/test/CIR/CodeGen/builtin_call.cpp
index c266f1a6d1637..09be7937a4330 100644
--- a/clang/test/CIR/CodeGen/builtin_call.cpp
+++ b/clang/test/CIR/CodeGen/builtin_call.cpp
@@ -111,6 +111,38 @@ void assume(bool arg) {
// OGCG: call void @llvm.assume(i1 %{{.+}})
// OGCG: }
+void *assume_aligned(void *ptr) {
+ return __builtin_assume_aligned(ptr, 16);
+}
+
+// CIR: @_Z14assume_alignedPv
+// CIR: %{{.+}} = cir.assume_aligned %{{.+}} alignment 16 : !cir.ptr<!void>
+// CIR: }
+
+// LLVM: @_Z14assume_alignedPv
+// LLVM: call void @llvm.assume(i1 true) [ "align"(ptr %{{.+}}, i64 16) ]
+// LLVM: }
+
+// OGCG: @_Z14assume_alignedPv
+// OGCG: call void @llvm.assume(i1 true) [ "align"(ptr %{{.+}}, i64 16) ]
+// OGCG: }
+
+void *assume_aligned_misalignment(void *ptr, unsigned misalignment) {
+ return __builtin_assume_aligned(ptr, 16, misalignment);
+}
+
+// CIR: @_Z27assume_aligned_misalignmentPvj
+// CIR: %{{.+}} = cir.assume_aligned %{{.+}} alignment 16[offset %{{.+}} : !u64i] : !cir.ptr<!void>
+// CIR: }
+
+// LLVM: @_Z27assume_aligned_misalignmentPvj
+// LLVM: call void @llvm.assume(i1 true) [ "align"(ptr %{{.+}}, i64 16, i64 %{{.+}}) ]
+// LLVM: }
+
+// OGCG: @_Z27assume_aligned_misalignmentPvj
+// OGCG: call void @llvm.assume(i1 true) [ "align"(ptr %{{.+}}, i64 16, i64 %{{.+}}) ]
+// OGCG: }
+
void assume_separate_storage(void *p1, void *p2) {
__builtin_assume_separate_storage(p1, p2);
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I have just a few suggestions.
(e->getNumArgs() > 2) ? emitScalarExpr(e->getArg(2)) : nullptr; | ||
|
||
const Expr *alignmentExpr = e->getArg(1); | ||
mlir::Attribute alignmentAttr = ConstantEmitter(*this).emitAbstract( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use Expr::EvaluateAsInt()
or Expr::getIntegerConstantExpr()
here to avoid creating the intermediate attribute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use Expr::getIntegerConstantExpr
. Changes to ConstantEmitter
are reverted.
SmallVector<mlir::Value, 3> opBundleArgs{adaptor.getPointer()}; | ||
|
||
auto alignment = rewriter.create<mlir::LLVM::ConstantOp>( | ||
op.getLoc(), rewriter.getI64Type(), op.getAlignment()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would adaptor.getAlignment()
translate the constant correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, adaptor.getAlignment
also works. But are there any differences between op.getAlignment
and adaptor.getAlignment
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping that using the adaptor would save a round trip on the attribute creation (because getAlignment()
returns a literal value which is then converted back to an attribute by the ConstantOp
builder), but after closer inspection it doesn't look like it does. However, I think maybe using getAlignmentAttr()
with either op
or adaptor
would work.
46e1300
to
d517c14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a couple of final suggestions, but this looks basically ready to go.
rewriter.getI1Type(), 1); | ||
mlir::LLVM::AssumeOp::create(rewriter, op.getLoc(), cond, "align", | ||
opBundleArgs); | ||
rewriter.replaceAllUsesWith(op, op.getPointer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rewriter.replaceAllUsesWith(op, op.getPointer()); | |
// The LLVM assume doesn't return a value, so we need to update any users of the op result. | |
rewriter.replaceAllUsesWith(op, adaptor.getPointer()); |
It took me a bit to recognize why this was here, so a comment would be useful. Also, I think using adaptor.getPointer()
rather than op.getPointer()
avoids re-lowering the replaced argument since adaptor.getPointer()
already has it in lowered form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of replaceAllUsesWith
, a more natural way to implement this would be:
rewriter.replaceOp(op, adaptor.getPointer());
I updated the code like this. The comment is nice to have and I included it as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending a nit
operation. | ||
|
||
This operation corresponds to the `__builtin_assume_aligned` builtin | ||
function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a small example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
d517c14
to
72e9fbf
Compare
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/14908 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/21010 Here is the relevant piece of the build log for the reference
|
This patch upstreams CIRGen and LLVM lowering support for the
__builtin_assume_aligned
builtin function.