-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CIR] Upstream support for address of and dereference #134317
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,147 @@ using namespace clang; | |
| using namespace clang::CIRGen; | ||
| using namespace cir; | ||
|
|
||
| /// Given an expression of pointer type, try to | ||
| /// derive a more accurate bound on the alignment of the pointer. | ||
| Address CIRGenFunction::emitPointerWithAlignment(const Expr *expr) { | ||
| // We allow this with ObjC object pointers because of fragile ABIs. | ||
| assert(expr->getType()->isPointerType() || | ||
| expr->getType()->isObjCObjectPointerType()); | ||
| expr = expr->IgnoreParens(); | ||
|
|
||
| // Casts: | ||
| if (auto const *ce = dyn_cast<CastExpr>(expr)) { | ||
| if (auto const *ece = dyn_cast<ExplicitCastExpr>(ce)) { | ||
| cgm.errorNYI(expr->getSourceRange(), | ||
| "emitPointerWithAlignment: explicit cast"); | ||
| return Address::invalid(); | ||
| } | ||
|
|
||
| switch (ce->getCastKind()) { | ||
| // Non-converting casts (but not C's implicit conversion from void*). | ||
| case CK_BitCast: | ||
| case CK_NoOp: | ||
| case CK_AddressSpaceConversion: { | ||
| cgm.errorNYI(expr->getSourceRange(), | ||
| "emitPointerWithAlignment: noop cast"); | ||
| return Address::invalid(); | ||
| } break; | ||
|
|
||
| // Array-to-pointer decay. TODO(cir): BaseInfo and TBAAInfo. | ||
| case CK_ArrayToPointerDecay: { | ||
| cgm.errorNYI(expr->getSourceRange(), | ||
| "emitPointerWithAlignment: array-to-pointer decay"); | ||
| return Address::invalid(); | ||
| } | ||
|
|
||
| case CK_UncheckedDerivedToBase: | ||
| case CK_DerivedToBase: { | ||
| cgm.errorNYI(expr->getSourceRange(), | ||
| "emitPointerWithAlignment: derived-to-base cast"); | ||
| return Address::invalid(); | ||
| } | ||
|
|
||
| case CK_AnyPointerToBlockPointerCast: | ||
| case CK_BaseToDerived: | ||
| case CK_BaseToDerivedMemberPointer: | ||
| case CK_BlockPointerToObjCPointerCast: | ||
| case CK_BuiltinFnToFnPtr: | ||
| case CK_CPointerToObjCPointerCast: | ||
| case CK_DerivedToBaseMemberPointer: | ||
| case CK_Dynamic: | ||
| case CK_FunctionToPointerDecay: | ||
| case CK_IntegralToPointer: | ||
| case CK_LValueToRValue: | ||
| case CK_LValueToRValueBitCast: | ||
| case CK_NullToMemberPointer: | ||
| case CK_NullToPointer: | ||
| case CK_ReinterpretMemberPointer: | ||
| // Common pointer conversions, nothing to do here. | ||
| // TODO: Is there any reason to treat base-to-derived conversions | ||
| // specially? | ||
| break; | ||
|
|
||
| case CK_ARCConsumeObject: | ||
| case CK_ARCExtendBlockObject: | ||
| case CK_ARCProduceObject: | ||
| case CK_ARCReclaimReturnedObject: | ||
| case CK_AtomicToNonAtomic: | ||
| case CK_BooleanToSignedIntegral: | ||
| case CK_ConstructorConversion: | ||
| case CK_CopyAndAutoreleaseBlockObject: | ||
| case CK_Dependent: | ||
| case CK_FixedPointCast: | ||
| case CK_FixedPointToBoolean: | ||
| case CK_FixedPointToFloating: | ||
| case CK_FixedPointToIntegral: | ||
| case CK_FloatingCast: | ||
| case CK_FloatingComplexCast: | ||
| case CK_FloatingComplexToBoolean: | ||
| case CK_FloatingComplexToIntegralComplex: | ||
| case CK_FloatingComplexToReal: | ||
| case CK_FloatingRealToComplex: | ||
| case CK_FloatingToBoolean: | ||
| case CK_FloatingToFixedPoint: | ||
| case CK_FloatingToIntegral: | ||
| case CK_HLSLAggregateSplatCast: | ||
| case CK_HLSLArrayRValue: | ||
| case CK_HLSLElementwiseCast: | ||
| case CK_HLSLVectorTruncation: | ||
| case CK_IntToOCLSampler: | ||
| case CK_IntegralCast: | ||
| case CK_IntegralComplexCast: | ||
| case CK_IntegralComplexToBoolean: | ||
| case CK_IntegralComplexToFloatingComplex: | ||
| case CK_IntegralComplexToReal: | ||
| case CK_IntegralRealToComplex: | ||
| case CK_IntegralToBoolean: | ||
| case CK_IntegralToFixedPoint: | ||
| case CK_IntegralToFloating: | ||
| case CK_LValueBitCast: | ||
| case CK_MatrixCast: | ||
| case CK_MemberPointerToBoolean: | ||
| case CK_NonAtomicToAtomic: | ||
| case CK_ObjCObjectLValueCast: | ||
| case CK_PointerToBoolean: | ||
| case CK_PointerToIntegral: | ||
| case CK_ToUnion: | ||
| case CK_ToVoid: | ||
| case CK_UserDefinedConversion: | ||
| case CK_VectorSplat: | ||
| case CK_ZeroToOCLOpaqueType: | ||
| llvm_unreachable("unexpected cast for emitPointerWithAlignment"); | ||
| } | ||
| } | ||
|
|
||
| // Unary & | ||
| if (const UnaryOperator *uo = dyn_cast<UnaryOperator>(expr)) { | ||
| // TODO(cir): maybe we should use cir.unary for pointers here instead. | ||
| if (uo->getOpcode() == UO_AddrOf) { | ||
| cgm.errorNYI(expr->getSourceRange(), "emitPointerWithAlignment: unary &"); | ||
| return Address::invalid(); | ||
| } | ||
| } | ||
|
|
||
| // std::addressof and variants. | ||
| if (auto const *call = dyn_cast<CallExpr>(expr)) { | ||
| switch (call->getBuiltinCallee()) { | ||
| default: | ||
| break; | ||
| case Builtin::BIaddressof: | ||
| case Builtin::BI__addressof: | ||
| case Builtin::BI__builtin_addressof: { | ||
| cgm.errorNYI(expr->getSourceRange(), | ||
| "emitPointerWithAlignment: builtin addressof"); | ||
| return Address::invalid(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Otherwise, use the alignment of the type. | ||
| return makeNaturalAddressForPointer( | ||
| emitScalarExpr(expr), expr->getType()->getPointeeType(), CharUnits()); | ||
| } | ||
|
|
||
| void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst, | ||
| bool isInit) { | ||
| if (!dst.isSimple()) { | ||
|
|
@@ -193,8 +334,23 @@ LValue CIRGenFunction::emitUnaryOpLValue(const UnaryOperator *e) { | |
|
|
||
| switch (op) { | ||
| case UO_Deref: { | ||
| cgm.errorNYI(e->getSourceRange(), "UnaryOp dereference"); | ||
| return LValue(); | ||
| QualType t = e->getSubExpr()->getType()->getPointeeType(); | ||
| assert(!t.isNull() && "CodeGenFunction::EmitUnaryOpLValue: Illegal type"); | ||
|
|
||
| assert(!cir::MissingFeatures::lvalueBaseInfo()); | ||
| assert(!cir::MissingFeatures::opTBAA()); | ||
| Address addr = emitPointerWithAlignment(e->getSubExpr()); | ||
|
|
||
| // Tag 'load' with deref attribute. | ||
| if (auto loadOp = | ||
| dyn_cast<cir::LoadOp>(addr.getPointer().getDefiningOp())) { | ||
| loadOp.setIsDerefAttr(mlir::UnitAttr::get(&getMLIRContext())); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm following correctly, the "deref" attribute is supposed to indicate the pointer is dereferenced... but the way it's implemented, this is a static property of the code structure, not a dynamic property of the execution. There might be some codepath where it doesn't actually get dereferenced... and in fact, the dereference might not be reachable at all. Given that, what can you actually do with "deref" information?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good question. You're correct that the intent is to describe a static property of the source code. Perhaps the dialect documentation needs some clarification on this point. At the time that the CIR is generated in the front end, I think there is likely to be a direct correspondence here. The pointer loaded is being dereferenced. We just emitted it in the call to In any event, I believe this is intended to be used for static analysis. For example, it distinguishes loads of There are https://godbolt.org/z/WP69hqvxj I'm not sure exactly what the static analysis does with this information. Perhaps @bcardosolopes could provide more information. BTW, I see that we miss applying the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We currently use this in the lifetime checker to decide whether the load is worth looking at: Instead of directly tagging the load during CIRGen it's reasonable to argue that such information could instead be provided by the result on an analysis (versus encoded directly into the operation). The design choice at the moment this was created was to not spend extra compile time given the information is conveniently available. My take is to leave it as is until we have other passes that might take advantage of that information, where we could revisit it and work on the guarantees. FWIW, this is the only operation currently in CIR encoding this type of semantic.
Awesome!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @efriedma-quic Are you satisfied with the explanations above?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it really accomplishes what you've set out to do in this context... it interacts weirdly with other operators. It sets That said, this isn't going to have a critical impact on anything; I'm okay with revisiting as needed. Maybe leave a FIXME in the code so anyone reading it later can refer to this discussion. |
||
| } | ||
|
|
||
| LValue lv = LValue::makeAddr(addr, t); | ||
| assert(!cir::MissingFeatures::addressSpace()); | ||
| assert(!cir::MissingFeatures::setNonGC()); | ||
| return lv; | ||
| } | ||
| case UO_Real: | ||
| case UO_Imag: { | ||
|
|
||
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.
Are the rest of the cases NYI or can we assert that Opcode is AddrOf?
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.
Neither. The rest of the opcodes don't require any special handling here. Neither the incubator nor the classic codegen do anything other than the special handling for AddrOf. Any others get handled by
emitScalarExprbelow.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.
Ah? so we still expect the others to get here?
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