<div dir="ltr">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?)</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 14, 2017 at 6:18 PM Reid Kleckner via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rnk<br>
Date: Mon Aug 14 18:17:47 2017<br>
New Revision: 310905<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=310905&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=310905&view=rev</a><br>
Log:<br>
Avoid PointerIntPair of constexpr EvalInfo structs<br>
<br>
They are stack allocated, so their alignment is not to be trusted.<br>
32-bit MSVC only guarantees 4 byte stack alignment, even though alignof<br>
would tell you otherwise. I tried fixing this with __declspec align, but<br>
that apparently upsets GCC. Hopefully this version will satisfy all<br>
compilers.<br>
<br>
See PR32018 for some info about the mingw issues.<br>
<br>
Should supercede <a href="https://reviews.llvm.org/D34873" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34873</a><br>
<br>
Modified:<br>
    cfe/trunk/lib/AST/ExprConstant.cpp<br>
<br>
Modified: cfe/trunk/lib/AST/ExprConstant.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=310905&r1=310904&r2=310905&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=310905&r1=310904&r2=310905&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)<br>
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Aug 14 18:17:47 2017<br>
@@ -537,7 +537,7 @@ namespace {<br>
   /// rules.  For example, the RHS of (0 && foo()) is not evaluated.  We can<br>
   /// evaluate the expression regardless of what the RHS is, but C only allows<br>
   /// certain things in certain situations.<br>
-  struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo {<br>
+  struct EvalInfo {<br>
     ASTContext &Ctx;<br>
<br>
     /// EvalStatus - Contains information about the evaluation.<br>
@@ -977,24 +977,22 @@ namespace {<br>
   /// RAII object used to optionally suppress diagnostics and side-effects from<br>
   /// a speculative evaluation.<br>
   class SpeculativeEvaluationRAII {<br>
-    /// Pair of EvalInfo, and a bit that stores whether or not we were<br>
-    /// speculatively evaluating when we created this RAII.<br>
-    llvm::PointerIntPair<EvalInfo *, 1, bool> InfoAndOldSpecEval;<br>
-    Expr::EvalStatus Old;<br>
+    EvalInfo *Info;<br>
+    Expr::EvalStatus OldStatus;<br>
+    bool OldIsSpeculativelyEvaluating;<br>
<br>
     void moveFromAndCancel(SpeculativeEvaluationRAII &&Other) {<br>
-      InfoAndOldSpecEval = Other.InfoAndOldSpecEval;<br>
-      Old = Other.Old;<br>
-      Other.InfoAndOldSpecEval.setPointer(nullptr);<br>
+      Info = Other.Info;<br>
+      OldStatus = Other.OldStatus;<br>
+      Other.Info = nullptr;<br>
     }<br>
<br>
     void maybeRestoreState() {<br>
-      EvalInfo *Info = InfoAndOldSpecEval.getPointer();<br>
       if (!Info)<br>
         return;<br>
<br>
-      Info->EvalStatus = Old;<br>
-      Info->IsSpeculativelyEvaluating = InfoAndOldSpecEval.getInt();<br>
+      Info->EvalStatus = OldStatus;<br>
+      Info->IsSpeculativelyEvaluating = OldIsSpeculativelyEvaluating;<br>
     }<br>
<br>
   public:<br>
@@ -1002,8 +1000,8 @@ namespace {<br>
<br>
     SpeculativeEvaluationRAII(<br>
         EvalInfo &Info, SmallVectorImpl<PartialDiagnosticAt> *NewDiag = nullptr)<br>
-        : InfoAndOldSpecEval(&Info, Info.IsSpeculativelyEvaluating),<br>
-          Old(Info.EvalStatus) {<br>
+        : Info(&Info), OldStatus(Info.EvalStatus),<br>
+          OldIsSpeculativelyEvaluating(Info.IsSpeculativelyEvaluating) {<br>
       Info.EvalStatus.Diag = NewDiag;<br>
       Info.IsSpeculativelyEvaluating = true;<br>
     }<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>