diff options
| -rw-r--r-- | clang/lib/CodeGen/CGCall.cpp | 51 | ||||
| -rw-r--r-- | clang/lib/CodeGen/CGExpr.cpp | 32 | ||||
| -rw-r--r-- | clang/lib/CodeGen/CGExprCXX.cpp | 2 | ||||
| -rw-r--r-- | clang/lib/CodeGen/CodeGenFunction.h | 16 | ||||
| -rw-r--r-- | clang/test/CodeGenCXX/cxx1z-eval-order.cpp | 55 | ||||
| -rw-r--r-- | clang/www/cxx_status.html | 10 |
6 files changed, 90 insertions, 76 deletions
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 525697c3527..233f6c17d48 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3173,7 +3173,7 @@ void CodeGenFunction::EmitCallArgs( CallArgList &Args, ArrayRef<QualType> ArgTypes, llvm::iterator_range<CallExpr::const_arg_iterator> ArgRange, const FunctionDecl *CalleeDecl, unsigned ParamsToSkip, - bool ForceRightToLeftEvaluation) { + EvaluationOrder Order) { assert((int)ArgTypes.size() == (ArgRange.end() - ArgRange.begin())); auto MaybeEmitImplicitObjectSize = [&](unsigned I, const Expr *Arg) { @@ -3191,11 +3191,18 @@ void CodeGenFunction::EmitCallArgs( }; // We *have* to evaluate arguments from right to left in the MS C++ ABI, - // because arguments are destroyed left to right in the callee. - if (CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee() || - ForceRightToLeftEvaluation) { - // Insert a stack save if we're going to need any inalloca args. - bool HasInAllocaArgs = false; + // because arguments are destroyed left to right in the callee. As a special + // case, there are certain language constructs that require left-to-right + // evaluation, and in those cases we consider the evaluation order requirement + // to trump the "destruction order is reverse construction order" guarantee. + bool LeftToRight = + CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee() + ? Order == EvaluationOrder::ForceLeftToRight + : Order != EvaluationOrder::ForceRightToLeft; + + // Insert a stack save if we're going to need any inalloca args. + bool HasInAllocaArgs = false; + if (CGM.getTarget().getCXXABI().isMicrosoft()) { for (ArrayRef<QualType>::iterator I = ArgTypes.begin(), E = ArgTypes.end(); I != E && !HasInAllocaArgs; ++I) HasInAllocaArgs = isInAllocaArgument(CGM.getCXXABI(), *I); @@ -3203,30 +3210,24 @@ void CodeGenFunction::EmitCallArgs( assert(getTarget().getTriple().getArch() == llvm::Triple::x86); Args.allocateArgumentMemory(*this); } + } - // Evaluate each argument. - size_t CallArgsStart = Args.size(); - for (int I = ArgTypes.size() - 1; I >= 0; --I) { - CallExpr::const_arg_iterator Arg = ArgRange.begin() + I; - MaybeEmitImplicitObjectSize(I, *Arg); - EmitCallArg(Args, *Arg, ArgTypes[I]); - EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], (*Arg)->getExprLoc(), - CalleeDecl, ParamsToSkip + I); - } + // Evaluate each argument in the appropriate order. + size_t CallArgsStart = Args.size(); + for (unsigned I = 0, E = ArgTypes.size(); I != E; ++I) { + unsigned Idx = LeftToRight ? I : E - I - 1; + CallExpr::const_arg_iterator Arg = ArgRange.begin() + Idx; + if (!LeftToRight) MaybeEmitImplicitObjectSize(Idx, *Arg); + EmitCallArg(Args, *Arg, ArgTypes[Idx]); + EmitNonNullArgCheck(Args.back().RV, ArgTypes[Idx], (*Arg)->getExprLoc(), + CalleeDecl, ParamsToSkip + Idx); + if (LeftToRight) MaybeEmitImplicitObjectSize(Idx, *Arg); + } + if (!LeftToRight) { // Un-reverse the arguments we just evaluated so they match up with the LLVM // IR function. std::reverse(Args.begin() + CallArgsStart, Args.end()); - return; - } - - for (unsigned I = 0, E = ArgTypes.size(); I != E; ++I) { - CallExpr::const_arg_iterator Arg = ArgRange.begin() + I; - assert(Arg != ArgRange.end()); - EmitCallArg(Args, *Arg, ArgTypes[I]); - EmitNonNullArgCheck(Args.back().RV, ArgTypes[I], (*Arg)->getExprLoc(), - CalleeDecl, ParamsToSkip + I); - MaybeEmitImplicitObjectSize(I, *Arg); } } diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 63aa3467850..7e12f5e7358 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4123,15 +4123,33 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, llvm::Value *Callee, CGM.getContext().VoidPtrTy); // C++17 requires that we evaluate arguments to a call using assignment syntax - // right-to-left. It also requires that we evaluate arguments to operators - // <<, >>, and ->* left-to-right, but that is not possible under the MS ABI, - // so there is no corresponding "force left-to-right" case. - bool ForceRightToLeft = false; - if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(E)) - ForceRightToLeft = OCE->isAssignmentOp(); + // right-to-left, and that we evaluate arguments to certain other operators + // left-to-right. Note that we allow this to override the order dictated by + // the calling convention on the MS ABI, which means that parameter + // destruction order is not necessarily reverse construction order. + // FIXME: Revisit this based on C++ committee response to unimplementability. + EvaluationOrder Order = EvaluationOrder::Default; + if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(E)) { + if (OCE->isAssignmentOp()) + Order = EvaluationOrder::ForceRightToLeft; + else { + switch (OCE->getOperator()) { + case OO_LessLess: + case OO_GreaterGreater: + case OO_AmpAmp: + case OO_PipePipe: + case OO_Comma: + case OO_ArrowStar: + Order = EvaluationOrder::ForceLeftToRight; + break; + default: + break; + } + } + } EmitCallArgs(Args, dyn_cast<FunctionProtoType>(FnType), E->arguments(), - E->getDirectCallee(), /*ParamsToSkip*/ 0, ForceRightToLeft); + E->getDirectCallee(), /*ParamsToSkip*/ 0, Order); const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall( Args, FnType, /*isChainCall=*/Chain); diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 34ab6adad2a..4f54a0962f8 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -184,7 +184,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( RtlArgs = &RtlArgStorage; EmitCallArgs(*RtlArgs, MD->getType()->castAs<FunctionProtoType>(), drop_begin(CE->arguments(), 1), CE->getDirectCallee(), - /*ParamsToSkip*/0, /*ForceRightToLeftEvaluation*/true); + /*ParamsToSkip*/0, EvaluationOrder::ForceRightToLeft); } } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 378a2562f8a..db579854886 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -3318,13 +3318,22 @@ public: static bool isObjCMethodWithTypeParams(const T *) { return false; } #endif + enum class EvaluationOrder { + ///! No language constraints on evaluation order. + Default, + ///! Language semantics require left-to-right evaluation. + ForceLeftToRight, + ///! Language semantics require right-to-left evaluation. + ForceRightToLeft + }; + /// EmitCallArgs - Emit call arguments for a function. template <typename T> void EmitCallArgs(CallArgList &Args, const T *CallArgTypeInfo, llvm::iterator_range<CallExpr::const_arg_iterator> ArgRange, const FunctionDecl *CalleeDecl = nullptr, unsigned ParamsToSkip = 0, - bool ForceRightToLeftEvaluation = false) { + EvaluationOrder Order = EvaluationOrder::Default) { SmallVector<QualType, 16> ArgTypes; CallExpr::const_arg_iterator Arg = ArgRange.begin(); @@ -3364,15 +3373,14 @@ public: for (auto *A : llvm::make_range(Arg, ArgRange.end())) ArgTypes.push_back(getVarArgType(A)); - EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip, - ForceRightToLeftEvaluation); + EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip, Order); } void EmitCallArgs(CallArgList &Args, ArrayRef<QualType> ArgTypes, llvm::iterator_range<CallExpr::const_arg_iterator> ArgRange, const FunctionDecl *CalleeDecl = nullptr, unsigned ParamsToSkip = 0, - bool ForceRightToLeftEvaluation = false); + EvaluationOrder Order = EvaluationOrder::Default); /// EmitPointerWithAlignment - Given an expression with a pointer /// type, emit the value and compute our best estimate of the diff --git a/clang/test/CodeGenCXX/cxx1z-eval-order.cpp b/clang/test/CodeGenCXX/cxx1z-eval-order.cpp index 4b7c7f10614..1106719a474 100644 --- a/clang/test/CodeGenCXX/cxx1z-eval-order.cpp +++ b/clang/test/CodeGenCXX/cxx1z-eval-order.cpp @@ -151,16 +151,15 @@ int dotstar_lhs_before_rhs() { // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c()->*make_a(); - // FIXME: The corresponding case for Windows ABIs is unimplementable if the - // operand has a non-trivially-destructible type, because the order of - // construction of function arguments is defined by the ABI there (it's the - // reverse of the order in which the parameters are destroyed in the callee). - // But we could follow the C++17 rule in the case where the operand type is - // trivially-destructible. - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // FIXME: For MS ABI, the order of destruction of parameters here will not be + // reverse construction order (parameters are destroyed left-to-right in the + // callee). That sadly seems unavoidable; the rules are not implementable as + // specified. If we changed parameter destruction order for these functions + // to right-to-left, we could make the destruction order match for all cases + // other than indirect calls, but we can't completely avoid the problem. + // + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c()->*make_b(); // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( @@ -228,17 +227,13 @@ void shift_lhs_before_rhs() { // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c() >> make_a(); - // FIXME: This is unimplementable for Windows ABIs, see above. - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // FIXME: This is not correct for Windows ABIs, see above. + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() << make_b(); - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() >> make_b(); // CHECK: } } @@ -249,11 +244,9 @@ void comma_lhs_before_rhs() { // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c() , make_a(); - // FIXME: This is unimplementable for Windows ABIs, see above. - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // FIXME: This is not correct for Windows ABIs, see above. + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() , make_b(); } @@ -267,16 +260,12 @@ void andor_lhs_before_rhs() { // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c() || make_a(); - // FIXME: This is unimplementable for Windows ABIs, see above. - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // FIXME: This is not correct for Windows ABIs, see above. + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() && make_b(); - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK-WINDOWS: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() || make_b(); } diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index 1fe28d5dcbc..170fc6090ef 100644 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -740,14 +740,12 @@ In Clang 3.7, a warning is emitted for all cases that would change meaning. <span id="p0136">(9): This is the resolution to a Defect Report, so is applied to all language versions supporting inheriting constructors. </span><br> -<span id="p0145">(10): Under the MS ABI, this feature is not fully implementable, -because the calling convention requires that function parameters are destroyed -from left to right in the callee. In order to guarantee that destruction order -is reverse construction order, the operands of overloaded +<span id="p0145">(10): Under the MS ABI, function parameters are destroyed from +left to right in the callee. As a result, function parameters in calls to <tt>operator<<</tt>, <tt>operator>></tt>, <tt>operator->*</tt>, <tt>operator&&</tt>, <tt>operator||</tt>, and <tt>operator,</tt> -functions are evaluated right-to-left under the MS ABI when called using operator -syntax, not left-to-right as P0145R3 requires. +functions using expression syntax are no longer guaranteed to be destroyed in +reverse construction order in that ABI. </span> </p> </details> |
