Author: Timm Baeder Date: 2026-07-07T14:14:29+02:00 New Revision: 17d7bf22294173609c3a711a223713780055ed66
URL: https://github.com/llvm/llvm-project/commit/17d7bf22294173609c3a711a223713780055ed66 DIFF: https://github.com/llvm/llvm-project/commit/17d7bf22294173609c3a711a223713780055ed66.diff LOG: [clang][bytecode] Try to avoid temporaries in (compound) assignments (#207652) 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. Added: Modified: clang/lib/AST/ByteCode/Compiler.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 48f5ee8115429..46762ed8291f9 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()) { @@ -5135,10 +5176,18 @@ bool Compiler<Emitter>::visitAssignment(const Expr *LHS, const Expr *RHS, if (!canClassify(E->getType())) return false; - if (!this->visit(RHS)) - return false; - if (!this->visit(LHS)) - return false; + bool NeedsFlip = !isSideEffectFree(RHS); + if (!NeedsFlip) { + if (!this->visit(LHS)) + return false; + if (!this->visit(RHS)) + return false; + } else { + if (!this->visit(RHS)) + return false; + if (!this->visit(LHS)) + return false; + } if (LHS->getType().isVolatileQualified()) return this->emitInvalidStore(LHS->getType().getTypePtr(), E); @@ -5151,7 +5200,7 @@ bool Compiler<Emitter>::visitAssignment(const Expr *LHS, const Expr *RHS, bool Activates = refersToUnion(LHS); bool BitField = LHS->refersToBitField(); - if (!this->emitFlip(PT_Ptr, RHT, E)) + if (NeedsFlip && !this->emitFlip(PT_Ptr, RHT, E)) return false; if (DiscardResult) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
