Feels like this alignment attribute may be a bit of a death trap, then? Should other uses of the attribute be scrutinized, or could we come up with a portable way to use it safely? (using a different attribute on different compilers - sounded like maybe one form worked with MSVC and a different form worked with GCC? Maybe the attribute could be improved to use those as appropriate?)
On Mon, Aug 14, 2017 at 6:18 PM Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rnk > Date: Mon Aug 14 18:17:47 2017 > New Revision: 310905 > > URL: http://llvm.org/viewvc/llvm-project?rev=310905&view=rev > Log: > Avoid PointerIntPair of constexpr EvalInfo structs > > They are stack allocated, so their alignment is not to be trusted. > 32-bit MSVC only guarantees 4 byte stack alignment, even though alignof > would tell you otherwise. I tried fixing this with __declspec align, but > that apparently upsets GCC. Hopefully this version will satisfy all > compilers. > > See PR32018 for some info about the mingw issues. > > Should supercede https://reviews.llvm.org/D34873 > > Modified: > cfe/trunk/lib/AST/ExprConstant.cpp > > Modified: cfe/trunk/lib/AST/ExprConstant.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=310905&r1=310904&r2=310905&view=diff > > ============================================================================== > --- cfe/trunk/lib/AST/ExprConstant.cpp (original) > +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Aug 14 18:17:47 2017 > @@ -537,7 +537,7 @@ namespace { > /// rules. For example, the RHS of (0 && foo()) is not evaluated. We > can > /// evaluate the expression regardless of what the RHS is, but C only > allows > /// certain things in certain situations. > - struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo { > + struct EvalInfo { > ASTContext &Ctx; > > /// EvalStatus - Contains information about the evaluation. > @@ -977,24 +977,22 @@ namespace { > /// RAII object used to optionally suppress diagnostics and > side-effects from > /// a speculative evaluation. > class SpeculativeEvaluationRAII { > - /// Pair of EvalInfo, and a bit that stores whether or not we were > - /// speculatively evaluating when we created this RAII. > - llvm::PointerIntPair<EvalInfo *, 1, bool> InfoAndOldSpecEval; > - Expr::EvalStatus Old; > + EvalInfo *Info; > + Expr::EvalStatus OldStatus; > + bool OldIsSpeculativelyEvaluating; > > void moveFromAndCancel(SpeculativeEvaluationRAII &&Other) { > - InfoAndOldSpecEval = Other.InfoAndOldSpecEval; > - Old = Other.Old; > - Other.InfoAndOldSpecEval.setPointer(nullptr); > + Info = Other.Info; > + OldStatus = Other.OldStatus; > + Other.Info = nullptr; > } > > void maybeRestoreState() { > - EvalInfo *Info = InfoAndOldSpecEval.getPointer(); > if (!Info) > return; > > - Info->EvalStatus = Old; > - Info->IsSpeculativelyEvaluating = InfoAndOldSpecEval.getInt(); > + Info->EvalStatus = OldStatus; > + Info->IsSpeculativelyEvaluating = OldIsSpeculativelyEvaluating; > } > > public: > @@ -1002,8 +1000,8 @@ namespace { > > SpeculativeEvaluationRAII( > EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag = > nullptr) > - : InfoAndOldSpecEval(&Info, Info.IsSpeculativelyEvaluating), > - Old(Info.EvalStatus) { > + : Info(&Info), OldStatus(Info.EvalStatus), > + OldIsSpeculativelyEvaluating(Info.IsSpeculativelyEvaluating) { > Info.EvalStatus.Diag = NewDiag; > Info.IsSpeculativelyEvaluating = true; > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
