[cfe-commits] r85967 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/Expr.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGCXX.cpp lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h lib/Sema/SemaStmt.cpp

Mike Stump mrs at apple.com
Tue Nov 3 15:25:49 PST 2009


Author: mrs
Date: Tue Nov  3 17:25:48 2009
New Revision: 85967

URL: http://llvm.org/viewvc/llvm-project?rev=85967&view=rev
Log:
Refine volatile handling, specifically, we must have the canonical
type to look at the volatile specifier.  I found these all from just
hand auditing the code.

Modified:
    cfe/trunk/include/clang/AST/Expr.h
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/AST/ExprConstant.cpp
    cfe/trunk/lib/CodeGen/CGCXX.cpp
    cfe/trunk/lib/CodeGen/CGDecl.cpp
    cfe/trunk/lib/CodeGen/CGExprScalar.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/lib/Sema/SemaStmt.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Nov  3 17:25:48 2009
@@ -135,7 +135,7 @@
   /// with location to warn on and the source range[s] to report with the
   /// warning.
   bool isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
-                              SourceRange &R2) const;
+                              SourceRange &R2, ASTContext &Ctx) const;
 
   /// isLvalue - C99 6.3.2.1: an lvalue is an expression with an object type or
   /// incomplete type other than void. Nonarray expressions that can be lvalues:

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Nov  3 17:25:48 2009
@@ -698,7 +698,7 @@
 /// with location to warn on and the source range[s] to report with the
 /// warning.
 bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1,
-                                  SourceRange &R2) const {
+                                  SourceRange &R2, ASTContext &Ctx) const {
   // Don't warn if the expr is type dependent. The type could end up
   // instantiating to void.
   if (isTypeDependent())
@@ -711,7 +711,7 @@
     return true;
   case ParenExprClass:
     return cast<ParenExpr>(this)->getSubExpr()->
-      isUnusedResultAWarning(Loc, R1, R2);
+      isUnusedResultAWarning(Loc, R1, R2, Ctx);
   case UnaryOperatorClass: {
     const UnaryOperator *UO = cast<UnaryOperator>(this);
 
@@ -724,17 +724,18 @@
       return false;  // Not a warning.
     case UnaryOperator::Deref:
       // Dereferencing a volatile pointer is a side-effect.
-      if (getType().isVolatileQualified())
+      if (Ctx.getCanonicalType(getType()).isVolatileQualified())
         return false;
       break;
     case UnaryOperator::Real:
     case UnaryOperator::Imag:
       // accessing a piece of a volatile complex is a side-effect.
-      if (UO->getSubExpr()->getType().isVolatileQualified())
+      if (Ctx.getCanonicalType(UO->getSubExpr()->getType())
+          .isVolatileQualified())
         return false;
       break;
     case UnaryOperator::Extension:
-      return UO->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2);
+      return UO->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
     }
     Loc = UO->getOperatorLoc();
     R1 = UO->getSubExpr()->getSourceRange();
@@ -744,8 +745,8 @@
     const BinaryOperator *BO = cast<BinaryOperator>(this);
     // Consider comma to have side effects if the LHS or RHS does.
     if (BO->getOpcode() == BinaryOperator::Comma)
-      return BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2) ||
-             BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2);
+      return (BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx) ||
+              BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
 
     if (BO->isAssignmentOp())
       return false;
@@ -762,15 +763,15 @@
     // warning, warn about them.
     const ConditionalOperator *Exp = cast<ConditionalOperator>(this);
     if (Exp->getLHS() &&
-        Exp->getLHS()->isUnusedResultAWarning(Loc, R1, R2))
+        Exp->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx))
       return true;
-    return Exp->getRHS()->isUnusedResultAWarning(Loc, R1, R2);
+    return Exp->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx);
   }
 
   case MemberExprClass:
     // If the base pointer or element is to a volatile pointer/field, accessing
     // it is a side effect.
-    if (getType().isVolatileQualified())
+    if (Ctx.getCanonicalType(getType()).isVolatileQualified())
       return false;
     Loc = cast<MemberExpr>(this)->getMemberLoc();
     R1 = SourceRange(Loc, Loc);
@@ -780,7 +781,7 @@
   case ArraySubscriptExprClass:
     // If the base pointer or element is to a volatile pointer/field, accessing
     // it is a side effect.
-    if (getType().isVolatileQualified())
+    if (Ctx.getCanonicalType(getType()).isVolatileQualified())
       return false;
     Loc = cast<ArraySubscriptExpr>(this)->getRBracketLoc();
     R1 = cast<ArraySubscriptExpr>(this)->getLHS()->getSourceRange();
@@ -838,7 +839,7 @@
     const CompoundStmt *CS = cast<StmtExpr>(this)->getSubStmt();
     if (!CS->body_empty())
       if (const Expr *E = dyn_cast<Expr>(CS->body_back()))
-        return E->isUnusedResultAWarning(Loc, R1, R2);
+        return E->isUnusedResultAWarning(Loc, R1, R2, Ctx);
 
     Loc = cast<StmtExpr>(this)->getLParenLoc();
     R1 = getSourceRange();
@@ -856,20 +857,20 @@
     // If this is a cast to void, check the operand.  Otherwise, the result of
     // the cast is unused.
     if (getType()->isVoidType())
-      return cast<CastExpr>(this)->getSubExpr()
-               ->isUnusedResultAWarning(Loc, R1, R2);
+      return (cast<CastExpr>(this)->getSubExpr()
+              ->isUnusedResultAWarning(Loc, R1, R2, Ctx));
     Loc = cast<CXXFunctionalCastExpr>(this)->getTypeBeginLoc();
     R1 = cast<CXXFunctionalCastExpr>(this)->getSubExpr()->getSourceRange();
     return true;
 
   case ImplicitCastExprClass:
     // Check the operand, since implicit casts are inserted by Sema
-    return cast<ImplicitCastExpr>(this)
-      ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2);
+    return (cast<ImplicitCastExpr>(this)
+            ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
 
   case CXXDefaultArgExprClass:
-    return cast<CXXDefaultArgExpr>(this)
-      ->getExpr()->isUnusedResultAWarning(Loc, R1, R2);
+    return (cast<CXXDefaultArgExpr>(this)
+            ->getExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
 
   case CXXNewExprClass:
     // FIXME: In theory, there might be new expressions that don't have side
@@ -877,11 +878,11 @@
   case CXXDeleteExprClass:
     return false;
   case CXXBindTemporaryExprClass:
-    return cast<CXXBindTemporaryExpr>(this)
-      ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2);
+    return (cast<CXXBindTemporaryExpr>(this)
+            ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
   case CXXExprWithTemporariesClass:
-    return cast<CXXExprWithTemporaries>(this)
-      ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2);
+    return (cast<CXXExprWithTemporaries>(this)
+            ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx));
   }
 }
 

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Nov  3 17:25:48 2009
@@ -167,7 +167,7 @@
 
   bool VisitParenExpr(ParenExpr *E) { return Visit(E->getSubExpr()); }
   bool VisitDeclRefExpr(DeclRefExpr *E) {
-    if (E->getType().isVolatileQualified())
+    if (Info.Ctx.getCanonicalType(E->getType()).isVolatileQualified())
       return true;
     return false;
   }
@@ -197,7 +197,7 @@
   bool VisitUnaryPreDec(UnaryOperator *E) { return true; }
   bool VisitUnaryPostDec(UnaryOperator *E) { return true; }
   bool VisitUnaryDeref(UnaryOperator *E) {
-    if (E->getType().isVolatileQualified())
+    if (Info.Ctx.getCanonicalType(E->getType()).isVolatileQualified())
       return true;
     return Visit(E->getSubExpr());
   }

Modified: cfe/trunk/lib/CodeGen/CGCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXX.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXX.cpp Tue Nov  3 17:25:48 2009
@@ -71,16 +71,17 @@
 
   const Expr *Init = D.getInit();
   QualType T = D.getType();
+  bool isVolatile = getContext().getCanonicalType(T).isVolatileQualified();
 
   if (T->isReferenceType()) {
     ErrorUnsupported(Init, "global variable that binds to a reference");
   } else if (!hasAggregateLLVMType(T)) {
     llvm::Value *V = EmitScalarExpr(Init);
-    EmitStoreOfScalar(V, DeclPtr, T.isVolatileQualified(), T);
+    EmitStoreOfScalar(V, DeclPtr, isVolatile, T);
   } else if (T->isAnyComplexType()) {
-    EmitComplexExprIntoAddr(Init, DeclPtr, T.isVolatileQualified());
+    EmitComplexExprIntoAddr(Init, DeclPtr, isVolatile);
   } else {
-    EmitAggExpr(Init, DeclPtr, T.isVolatileQualified());
+    EmitAggExpr(Init, DeclPtr, isVolatile);
 
     if (const RecordType *RT = T->getAs<RecordType>()) {
       CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Nov  3 17:25:48 2009
@@ -417,17 +417,18 @@
       Loc = Builder.CreateStructGEP(DeclPtr, getByRefValueLLVMField(&D), 
                                     D.getNameAsString());
 
+    bool isVolatile = (getContext().getCanonicalType(D.getType())
+                       .isVolatileQualified());
     if (Ty->isReferenceType()) {
       RValue RV = EmitReferenceBindingToExpr(Init, Ty, /*IsInitializer=*/true);
       EmitStoreOfScalar(RV.getScalarVal(), Loc, false, Ty);
     } else if (!hasAggregateLLVMType(Init->getType())) {
       llvm::Value *V = EmitScalarExpr(Init);
-      EmitStoreOfScalar(V, Loc, D.getType().isVolatileQualified(),
-                        D.getType());
+      EmitStoreOfScalar(V, Loc, isVolatile, D.getType());
     } else if (Init->getType()->isAnyComplexType()) {
-      EmitComplexExprIntoAddr(Init, Loc, D.getType().isVolatileQualified());
+      EmitComplexExprIntoAddr(Init, Loc, isVolatile);
     } else {
-      EmitAggExpr(Init, Loc, D.getType().isVolatileQualified());
+      EmitAggExpr(Init, Loc, isVolatile);
     }
   }
 
@@ -556,6 +557,7 @@
   assert((isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D)) &&
          "Invalid argument to EmitParmDecl");
   QualType Ty = D.getType();
+  CanQualType CTy = getContext().getCanonicalType(Ty);
 
   llvm::Value *DeclPtr;
   if (!Ty->isConstantSizeType()) {
@@ -572,7 +574,7 @@
       DeclPtr->setName(Name.c_str());
 
       // Store the initial value into the alloca.
-      EmitStoreOfScalar(Arg, DeclPtr, Ty.isVolatileQualified(), Ty);
+      EmitStoreOfScalar(Arg, DeclPtr, CTy.isVolatileQualified(), Ty);
     } else {
       // Otherwise, if this is an aggregate, just use the input pointer.
       DeclPtr = Arg;

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Nov  3 17:25:48 2009
@@ -1648,9 +1648,10 @@
 /// expression is cheap enough and side-effect-free enough to evaluate
 /// unconditionally instead of conditionally.  This is used to convert control
 /// flow into selects in some cases.
-static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E) {
+static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E,
+                                                   CodeGenFunction &CGF) {
   if (const ParenExpr *PE = dyn_cast<ParenExpr>(E))
-    return isCheapEnoughToEvaluateUnconditionally(PE->getSubExpr());
+    return isCheapEnoughToEvaluateUnconditionally(PE->getSubExpr(), CGF);
 
   // TODO: Allow anything we can constant fold to an integer or fp constant.
   if (isa<IntegerLiteral>(E) || isa<CharacterLiteral>(E) ||
@@ -1661,7 +1662,9 @@
   // X and Y are local variables.
   if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
     if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl()))
-      if (VD->hasLocalStorage() && !VD->getType().isVolatileQualified())
+      if (VD->hasLocalStorage() && !(CGF.getContext()
+                                     .getCanonicalType(VD->getType())
+                                     .isVolatileQualified()))
         return true;
 
   return false;
@@ -1690,8 +1693,9 @@
   // If this is a really simple expression (like x ? 4 : 5), emit this as a
   // select instead of as control flow.  We can only do this if it is cheap and
   // safe to evaluate the LHS and RHS unconditionally.
-  if (E->getLHS() && isCheapEnoughToEvaluateUnconditionally(E->getLHS()) &&
-      isCheapEnoughToEvaluateUnconditionally(E->getRHS())) {
+  if (E->getLHS() && isCheapEnoughToEvaluateUnconditionally(E->getLHS(),
+                                                            CGF) &&
+      isCheapEnoughToEvaluateUnconditionally(E->getRHS(), CGF)) {
     llvm::Value *CondV = CGF.EvaluateExprAsBool(E->getCond());
     llvm::Value *LHS = Visit(E->getLHS());
     llvm::Value *RHS = Visit(E->getRHS());

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Nov  3 17:25:48 2009
@@ -517,7 +517,7 @@
   //===--------------------------------------------------------------------===//
 
   Qualifiers MakeQualifiers(QualType T) {
-    Qualifiers Quals = T.getQualifiers();
+    Qualifiers Quals = getContext().getCanonicalType(T).getQualifiers();
     Quals.setObjCGCAttr(getContext().getObjCGCAttrKind(T));
     return Quals;
   }

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=85967&r1=85966&r2=85967&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Nov  3 17:25:48 2009
@@ -70,7 +70,7 @@
 
   SourceLocation Loc;
   SourceRange R1, R2;
-  if (!E->isUnusedResultAWarning(Loc, R1, R2))
+  if (!E->isUnusedResultAWarning(Loc, R1, R2, Context))
     return;
 
   // Okay, we have an unused result.  Depending on what the base expression is,





More information about the cfe-commits mailing list