[PATCH] D14385: Correct atomic libcall support for __atomic_*_fetch builtins.

[Saleem Abdulrasool via cfe-commits]

compnerd added a comment.

Unfortunate that they have this crazy behavior.


================
Comment at: lib/CodeGen/CGAtomic.cpp:901
@@ -897,1 +900,3 @@
+      PostOp = llvm::Instruction::Add;
+    // Fall through.
     case AtomicExpr::AO__c11_atomic_fetch_add:
----------------
I think we should use `[[clang::fallthrough]]` instead.  It annotates as well as the comment, aids the static analyzer, and should be ignored by compilers that don't support it.

================
Comment at: lib/CodeGen/CGAtomic.cpp:985
@@ -996,1 +984,3 @@
 
+    assert(UseOptimizedLibcall || !PostOp);
+
----------------
Can you add an explanatory message in the assert?  (Yes, thats had to come up with since its obvious if you are familiar with this path).

================
Comment at: lib/CodeGen/CGAtomic.cpp:998
@@ -1007,1 +997,3 @@
       llvm::Value *ResVal = Res.getScalarVal();
+      llvm::Value *LoadVal1 = Args[1].RV.getScalarVal();
+      if (PostOp)
----------------
Am I mistaken, but, we don't need this unless we have an operation to perform?  Cant we sink this into the PostOp condition?


http://reviews.llvm.org/D14385