llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> This is only needed if the RHS has side-effects. If we can easily and cheaply prove that it doesn't, avoid the temporary variable. This saves memory and is slightly faster. --- Full diff: https://github.com/llvm/llvm-project/pull/207652.diff 1 Files Affected: - (modified) clang/lib/AST/ByteCode/Compiler.cpp (+85-44) ``````````diff diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 662944a9bd2ca..5cf63c9c2b546 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -37,6 +37,19 @@ static std::optional<bool> getBoolValue(const Expr *E) { return std::nullopt; } +/// Check if \c E has side-effects. This is used to avoid some tempoarary +/// variables and is supposed to be a quick check, not exhausite. That's why +/// we're not using Expr::HasSideEffects(). +static bool isSideEffectFree(const Expr *E) { + if (isa<IntegerLiteral, FloatingLiteral, CharacterLiteral, + CXXBoolLiteralExpr>(E)) + return true; + if (isa<DeclRefExpr>(E)) + return true; + + return false; +} + /// Scope chain managing the variable lifetimes. template <class Emitter> class VariableScope { public: @@ -3222,30 +3235,45 @@ bool Compiler<Emitter>::VisitFloatCompoundAssignOperator( PrimType LHST = classifyPrim(LHSType); - // C++17 onwards require that we evaluate the RHS first. - // Compute RHS and save it in a temporary variable so we can - // load it again later. - if (!visit(RHS)) - return false; + if (isSideEffectFree(RHS)) { + if (!visit(LHS)) + return false; + if (!this->emitLoad(LHST, E)) + return false; + // If necessary, convert LHS to its computation type. + if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType), + LHSComputationType, E)) + return false; + if (!visit(RHS)) + return false; - unsigned TempOffset = this->allocateLocalPrimitive(E, *RT, /*IsConst=*/true); - if (!this->emitSetLocal(*RT, TempOffset, E)) - return false; + } else { + // C++17 onwards require that we evaluate the RHS first. + // Compute RHS and save it in a temporary variable so we can + // load it again later. + if (!visit(RHS)) + return false; - // First, visit LHS. - if (!visit(LHS)) - return false; - if (!this->emitLoad(LHST, E)) - return false; + unsigned TempOffset = + this->allocateLocalPrimitive(E, *RT, /*IsConst=*/true); + if (!this->emitSetLocal(*RT, TempOffset, E)) + return false; - // If necessary, convert LHS to its computation type. - if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType), - LHSComputationType, E)) - return false; + // First, visit LHS. + if (!visit(LHS)) + return false; + if (!this->emitLoad(LHST, E)) + return false; - // Now load RHS. - if (!this->emitGetLocal(*RT, TempOffset, E)) - return false; + // If necessary, convert LHS to its computation type. + if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType), + LHSComputationType, E)) + return false; + + // Now load RHS. + if (!this->emitGetLocal(*RT, TempOffset, E)) + return false; + } switch (E->getOpcode()) { case BO_AddAssign: @@ -3334,7 +3362,6 @@ bool Compiler<Emitter>::VisitCompoundAssignOperator( // Handle floating point operations separately here, since they // require special care. - if (ResultT == PT_Float || RT == PT_Float) return VisitFloatCompoundAssignOperator(E); @@ -3344,33 +3371,47 @@ bool Compiler<Emitter>::VisitCompoundAssignOperator( assert(!E->getType()->isPointerType() && "Handled above"); assert(!E->getType()->isFloatingType() && "Handled above"); - // C++17 onwards require that we evaluate the RHS first. - // Compute RHS and save it in a temporary variable so we can - // load it again later. - // FIXME: Compound assignments are unsequenced in C, so we might - // have to figure out how to reject them. - if (!visit(RHS)) - return false; + if (isSideEffectFree(RHS)) { + if (!visit(LHS)) + return false; + if (!this->emitLoad(*LT, E)) + return false; + if (LT != LHSComputationT && + !this->emitIntegralCast(*LT, *LHSComputationT, + E->getComputationLHSType(), E)) + return false; + if (!visit(RHS)) + return false; + } else { + // C++17 onwards require that we evaluate the RHS first. + // Compute RHS and save it in a temporary variable so we can + // load it again later. + // FIXME: Compound assignments are unsequenced in C, so we might + // have to figure out how to reject them. + if (!visit(RHS)) + return false; - unsigned TempOffset = this->allocateLocalPrimitive(E, *RT, /*IsConst=*/true); + unsigned TempOffset = + this->allocateLocalPrimitive(E, *RT, /*IsConst=*/true); - if (!this->emitSetLocal(*RT, TempOffset, E)) - return false; + if (!this->emitSetLocal(*RT, TempOffset, E)) + return false; - // Get LHS pointer, load its value and cast it to the - // computation type if necessary. - if (!visit(LHS)) - return false; - if (!this->emitLoad(*LT, E)) - return false; - if (LT != LHSComputationT && - !this->emitIntegralCast(*LT, *LHSComputationT, E->getComputationLHSType(), - E)) - return false; + // Get LHS pointer, load its value and cast it to the + // computation type if necessary. + if (!visit(LHS)) + return false; + if (!this->emitLoad(*LT, E)) + return false; + if (LT != LHSComputationT && + !this->emitIntegralCast(*LT, *LHSComputationT, + E->getComputationLHSType(), E)) + return false; - // Get the RHS value on the stack. - if (!this->emitGetLocal(*RT, TempOffset, E)) - return false; + // Get the RHS value on the stack. + if (!this->emitGetLocal(*RT, TempOffset, E)) + return false; + } // Perform operation. switch (E->getOpcode()) { `````````` </details> https://github.com/llvm/llvm-project/pull/207652 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
