Treat __sync_synchronize and asm("":::"memory") as stronger fences.

lib/CodeGen/CGStmt.cpp

 //===--- CGStmt.cpp - Emit LLVM Code from Statements ----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This contains code to emit Stmt nodes as LLVM code.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeGenFunction.h"
 #include "CGDebugInfo.h"
 #include "CodeGenModule.h"
 #include "TargetInfo.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/NaClAsm.h" // @LOCALMOD

[eliben, 2013/08/07 18:06:25]

This is a problem for upstreaming, no?

From the other CL I see all this new module contains is:

bool isAsmMemory(const Instruction *I);

What makes this NaCl specific? Can't you just put this utility function in a
more generic place? If llvm/IR/InlineAsm.h is not suitable, maybe
llvm/IR/InlineAsmUtils.h or something? This would allow upstreaming and avoiding
a localmod in both Clang and LLVM

[JF, 2013/08/07 19:48:12]

Done.

 using namespace clang;
 using namespace CodeGen;
 
 //===----------------------------------------------------------------------===//
 //                              Statement Emission
 //===----------------------------------------------------------------------===//
 
 void CodeGenFunction::EmitStopPoint(const Stmt *S) {
   if (CGDebugInfo *DI = getDebugInfo()) {
     SourceLocation Loc;
@@ skipping 1659 matching lines @@
   bool HasSideEffect = S.isVolatile() || S.getNumOutputs() == 0;
   llvm::InlineAsm::AsmDialect AsmDialect = isa<MSAsmStmt>(&S) ?
     llvm::InlineAsm::AD_Intel : llvm::InlineAsm::AD_ATT;
   llvm::InlineAsm *IA =
     llvm::InlineAsm::get(FTy, AsmString, Constraints, HasSideEffect,
                          /* IsAlignStack */ false, AsmDialect);
   llvm::CallInst *Result = Builder.CreateCall(IA, Args);
   Result->addAttribute(llvm::AttributeSet::FunctionIndex,
                        llvm::Attribute::NoUnwind);
 
+  // @LOCALMOD-START
+  if (getTargetHooks().asmMemoryIsFence() && llvm::NaCl::isAsmMemory(Result)) {

[eliben, 2013/08/07 18:06:25]

It's weird that you're going the generic way with target hooks and yet directly
query a NaCl:: function

[JF, 2013/08/07 19:48:12]

Done.

+    // Targets can ask that ``asm("":::"memory")`` be treated like
+    // ``__sync_synchronize()``.
+    Builder.CreateFence(llvm::SequentiallyConsistent);
+    Builder.CreateCall(
+        llvm::InlineAsm::get(FTy, AsmString, Constraints, HasSideEffect))->
+        addAttribute(llvm::AttributeSet::FunctionIndex,
+                     llvm::Attribute::NoUnwind);
+  }
+  // @LOCALMOD-END
+
   // Slap the source location of the inline asm into a !srcloc metadata on the
   // call.  FIXME: Handle metadata for MS-style inline asms.
   if (const GCCAsmStmt *gccAsmStmt = dyn_cast<GCCAsmStmt>(&S))
     Result->setMetadata("srcloc", getAsmSrcLocInfo(gccAsmStmt->getAsmString(),
                                                    *this));
 
   // Extract all of the register value results from the asm.
   std::vector<llvm::Value*> RegResults;
   if (ResultRegTypes.size() == 1) {
     RegResults.push_back(Result);
@@ skipping 33 matching lines @@
       }
     }
 
     EmitStoreThroughLValue(RValue::get(Tmp), ResultRegDests[i]);
   }
 }
 
 void CodeGenFunction::EmitCapturedStmt(const CapturedStmt &S) {
   llvm_unreachable("not implemented yet");
 }