hliao created this revision.
hliao added a reviewer: erichkeane.
Herald added subscribers: cfe-commits, jfb.
Herald added a project: clang.
hliao added a comment.

The current BuildAtomicExpr expects the arguments to be in the API order 
instead of the AST order. If RebuildAtomicExpr uses the same BuildAtomicExpr, 
it needs to ensure the order of arguments are in API order; otherwise, 
arguments (especially the one with memory order) will be misplaced.


- Rearrange the atomic expr order to the API order when rebuilding atomic expr 
during template instantiation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67924

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/atomic-expr.cpp

Index: clang/test/AST/atomic-expr.cpp
===================================================================
--- clang/test/AST/atomic-expr.cpp
+++ clang/test/AST/atomic-expr.cpp
@@ -3,7 +3,7 @@
 template<int N = 0>
 void pr43370() {
   int arr[2];
-  __atomic_store_n(arr, 0, 0);
+  __atomic_store_n(arr, 0, 5);
 }
 void useage(){
   pr43370();
@@ -13,7 +13,13 @@
 // CHECK: AtomicExpr
 // CHECK-NEXT: ImplicitCastExpr
 // CHECK-SAME: <ArrayToPointerDecay>
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} <{{[^:]+}}:20> 'int [2]' lvalue Var 0x{{[0-9a-f]+}} 'arr' 'int [2]'
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-f]+}} <{{[^:]+}}:28> 'int' 5
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-f]+}} <{{[^:]+}}:25> 'int' 0
 // CHECK:FunctionDecl 0x{{[0-9a-f]+}} <line:4:1, line:7:1> line:4:6 used pr43370
 // CHECK: AtomicExpr
 // CHECK-NEXT: ImplicitCastExpr
 // CHECK-SAME: <ArrayToPointerDecay>
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}} <{{[^:]+}}:20> 'int [2]' lvalue Var 0x{{[0-9a-f]+}} 'arr' 'int [2]'
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-f]+}} <{{[^:]+}}:28> 'int' 5
+// CHECK-NEXT: IntegerLiteral 0x{{[0-9a-f]+}} <{{[^:]+}}:25> 'int' 0
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3315,7 +3315,8 @@
     // Use this for all of the locations, since we don't know the difference
     // between the call and the expr at this point.
     SourceRange Range{BuiltinLoc, RParenLoc};
-    return getSema().BuildAtomicExpr(Range, Range, RParenLoc, SubExprs, Op);
+    return getSema().BuildAtomicExpr(Range, Range, RParenLoc, SubExprs, Op,
+                                     true);
   }
 
 private:
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4473,7 +4473,8 @@
 
 ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
                                  SourceLocation RParenLoc, MultiExprArg Args,
-                                 AtomicExpr::AtomicOp Op) {
+                                 AtomicExpr::AtomicOp Op,
+                                 bool NeedRearrangeArgs) {
   // All the non-OpenCL operations take one of the following forms.
   // The OpenCL operations take the __c11 forms with one extra argument for
   // synchronization scope.
@@ -4754,19 +4755,56 @@
     IsPassedByAddress = true;
   }
 
+  SmallVector<Expr *, 5> ReArgs;
+  if (NeedRearrangeArgs) {
+    ReArgs.push_back(Args[0]);
+    switch (Form) {
+    case Init:
+    case Load:
+      ReArgs.push_back(Args[1]); // Val1/Order
+      break;
+    case LoadCopy:
+    case Copy:
+    case Arithmetic:
+    case Xchg:
+      ReArgs.push_back(Args[2]); // Val1
+      ReArgs.push_back(Args[1]); // Order
+      break;
+    case GNUXchg:
+      ReArgs.push_back(Args[2]); // Val1
+      ReArgs.push_back(Args[3]); // Val2
+      ReArgs.push_back(Args[1]); // Order
+      break;
+    case C11CmpXchg:
+      ReArgs.push_back(Args[2]); // Val1
+      ReArgs.push_back(Args[4]); // Val2
+      ReArgs.push_back(Args[1]); // Order
+      ReArgs.push_back(Args[3]); // OrderFail
+      break;
+    case GNUCmpXchg:
+      ReArgs.push_back(Args[2]); // Val1
+      ReArgs.push_back(Args[4]); // Val2
+      ReArgs.push_back(Args[5]); // Weak
+      ReArgs.push_back(Args[1]); // Order
+      ReArgs.push_back(Args[3]); // OrderFail
+      break;
+    }
+  } else
+    ReArgs.append(Args.begin(), Args.end());
+
   // The first argument's non-CV pointer type is used to deduce the type of
   // subsequent arguments, except for:
   //  - weak flag (always converted to bool)
   //  - memory order (always converted to int)
   //  - scope  (always converted to int)
-  for (unsigned i = 0; i != Args.size(); ++i) {
+  for (unsigned i = 0; i != ReArgs.size(); ++i) {
     QualType Ty;
     if (i < NumVals[Form] + 1) {
       switch (i) {
       case 0:
         // The first argument is always a pointer. It has a fixed type.
         // It is always dereferenced, a nullptr is undefined.
-        CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
+        CheckNonNullArgument(*this, ReArgs[i], ExprRange.getBegin());
         // Nothing else to do: we already know all we want about this pointer.
         continue;
       case 1:
@@ -4780,12 +4818,12 @@
         else if (Form == Copy || Form == Xchg) {
           if (IsPassedByAddress)
             // The value pointer is always dereferenced, a nullptr is undefined.
-            CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
+            CheckNonNullArgument(*this, ReArgs[i], ExprRange.getBegin());
           Ty = ByValType;
         } else if (Form == Arithmetic)
           Ty = Context.getPointerDiffType();
         else {
-          Expr *ValArg = Args[i];
+          Expr *ValArg = ReArgs[i];
           // The value pointer is always dereferenced, a nullptr is undefined.
           CheckNonNullArgument(*this, ValArg, ExprRange.getBegin());
           LangAS AS = LangAS::Default;
@@ -4802,7 +4840,7 @@
         // The third argument to compare_exchange / GNU exchange is the desired
         // value, either by-value (for the C11 and *_n variant) or as a pointer.
         if (IsPassedByAddress)
-          CheckNonNullArgument(*this, Args[i], ExprRange.getBegin());
+          CheckNonNullArgument(*this, ReArgs[i], ExprRange.getBegin());
         Ty = ByValType;
         break;
       case 3:
@@ -4817,11 +4855,11 @@
 
     InitializedEntity Entity =
         InitializedEntity::InitializeParameter(Context, Ty, false);
-    ExprResult Arg = Args[i];
+    ExprResult Arg = ReArgs[i];
     Arg = PerformCopyInitialization(Entity, SourceLocation(), Arg);
     if (Arg.isInvalid())
       return true;
-    Args[i] = Arg.get();
+    ReArgs[i] = Arg.get();
   }
 
   // Permute the arguments into a 'consistent' order.
@@ -4830,36 +4868,36 @@
   switch (Form) {
   case Init:
     // Note, AtomicExpr::getVal1() has a special case for this atomic.
-    SubExprs.push_back(Args[1]); // Val1
+    SubExprs.push_back(ReArgs[1]); // Val1
     break;
   case Load:
-    SubExprs.push_back(Args[1]); // Order
+    SubExprs.push_back(ReArgs[1]); // Order
     break;
   case LoadCopy:
   case Copy:
   case Arithmetic:
   case Xchg:
-    SubExprs.push_back(Args[2]); // Order
-    SubExprs.push_back(Args[1]); // Val1
+    SubExprs.push_back(ReArgs[2]); // Order
+    SubExprs.push_back(ReArgs[1]); // Val1
     break;
   case GNUXchg:
     // Note, AtomicExpr::getVal2() has a special case for this atomic.
-    SubExprs.push_back(Args[3]); // Order
-    SubExprs.push_back(Args[1]); // Val1
-    SubExprs.push_back(Args[2]); // Val2
+    SubExprs.push_back(ReArgs[3]); // Order
+    SubExprs.push_back(ReArgs[1]); // Val1
+    SubExprs.push_back(ReArgs[2]); // Val2
     break;
   case C11CmpXchg:
-    SubExprs.push_back(Args[3]); // Order
-    SubExprs.push_back(Args[1]); // Val1
-    SubExprs.push_back(Args[4]); // OrderFail
-    SubExprs.push_back(Args[2]); // Val2
+    SubExprs.push_back(ReArgs[3]); // Order
+    SubExprs.push_back(ReArgs[1]); // Val1
+    SubExprs.push_back(ReArgs[4]); // OrderFail
+    SubExprs.push_back(ReArgs[2]); // Val2
     break;
   case GNUCmpXchg:
-    SubExprs.push_back(Args[4]); // Order
-    SubExprs.push_back(Args[1]); // Val1
-    SubExprs.push_back(Args[5]); // OrderFail
-    SubExprs.push_back(Args[2]); // Val2
-    SubExprs.push_back(Args[3]); // Weak
+    SubExprs.push_back(ReArgs[4]); // Order
+    SubExprs.push_back(ReArgs[1]); // Val1
+    SubExprs.push_back(ReArgs[5]); // OrderFail
+    SubExprs.push_back(ReArgs[2]); // Val2
+    SubExprs.push_back(ReArgs[3]); // Weak
     break;
   }
 
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4639,7 +4639,8 @@
                            bool IsExecConfig = false);
   ExprResult BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
                              SourceLocation RParenLoc, MultiExprArg Args,
-                             AtomicExpr::AtomicOp Op);
+                             AtomicExpr::AtomicOp Op,
+                             bool NeedRearrangeArgs = false);
   ExprResult
   BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, SourceLocation LParenLoc,
                         ArrayRef<Expr *> Arg, SourceLocation RParenLoc,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to