MS inline asm: Allow __asm blocks to set a return value

[rnk]

Summary

If control falls off the end of a function after an __asm block, MSVC
assumes that the inline assembly filled the EAX and possibly EDX
registers with an appropriate return value. This functionality is used
in inline functions returning 64-bit integers in system headers, so we
need some amount of compatibility.

This is implemented in Clang by adding extra output constraints to every
inline asm block, and storing the resulting output registers into the
return value slot. If we see an asm block somewhere in the function
body, we emit a normal epilogue instead of marking the end of the
function with a return type unreachable.

Normal returns in functions not using this functionality will overwrite
the return value slot, and in most cases LLVM should be able to
eliminate the dead stores.

Fixes PR17201.

[majnemer]

Wonderful! Does this correctly handle the 'int main' implicit return case? The asm blob should win if it is present, not the implicit return 0. Perhaps a test should be added?

[rsmith]

I think this should do the right thing for int main; we store the implicit 0 into the return value at the start of the function, so the asm should overwrite it. But a test would be great too :)

[rnk]

• Fix tests. This resulted in something horrible that I'm not happy with, but it works. We should try to figure out something better than rewriting the asm string with complex string matching.

[rnk]

• Add implicit zero return test case.

[majnemer]

lib/CodeGen/TargetInfo.cpp

644

Pos == DigitStart, they point to the first non dollar character.

645

If the character at DigitStart is not a number, DigitStart == DigitEnd.

657

If DigitStart == DigitEnd, wouldn't Pos be unchanged at this point? I guess that would be handled by the next iteration of the loop.

[rnk]

Thanks! It sounds like this rewrite step is still our best idea for handling this. I'm going to land this then.

lib/CodeGen/TargetInfo.cpp

645

That should be OK, getAsInteger will return an error.

657

We should be fine. getAsInteger should fail, so we hit the else clause, stream an empty string, and continue the next iteration looking for dollars. That said, this would already be a malformed inline asm string, which Sema shouldn't produce.

Of course, you never know with stringly typed data structures. :)

[rnk]

Closed by commit rL217187 (authored by @rnk).

lib/CodeGen/CGStmt.cpp

if (Info.isReadWrite()) {
       else
         InOutConstraints += OutputConstraint;
 
       InOutArgTypes.push_back(Arg->getType());
       InOutArgs.push_back(Arg);
     }
   }
 
-  unsigned NumConstraints = S.getNumOutputs() + S.getNumInputs();
+  // If this is a Microsoft-style asm blob, store the return registers (EAX:EDX)
+  // to the return value slot. Only do this when returning in registers.
+  if (isa<MSAsmStmt>(&S)) {
+    const ABIArgInfo &RetAI = CurFnInfo->getReturnInfo();
+    if (RetAI.isDirect() || RetAI.isExtend()) {
+      // Make a fake lvalue for the return value slot.
+      LValue ReturnSlot = MakeAddrLValue(ReturnValue, FnRetTy);
+      CGM.getTargetCodeGenInfo().addReturnRegisterOutputs(
+          *this, ReturnSlot, Constraints, ResultRegTypes, ResultTruncRegTypes,
+          ResultRegDests, AsmString, S.getNumOutputs());
+      SawAsmBlock = true;
+    }
+  }
 
   for (unsigned i = 0, e = S.getNumInputs(); i != e; i++) {
     const Expr *InputExpr = S.getInputExpr(i);
 
     TargetInfo::ConstraintInfo &Info = InputConstraintInfos[i];
 
     if (!Constraints.empty())
       Constraints += ',';
void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   }
   Constraints += InOutConstraints;
 
   // Clobbers
   for (unsigned i = 0, e = S.getNumClobbers(); i != e; i++) {
     StringRef Clobber = S.getClobber(i);
 
     if (Clobber != "memory" && Clobber != "cc")
       Clobber = getTarget().getNormalizedGCCRegisterName(Clobber);
 
-    if (i != 0 || NumConstraints != 0)
+    if (!Constraints.empty())
       Constraints += ',';
 
     Constraints += "~{";
     Constraints += Clobber;
     Constraints += '}';
   }
 
   // Add machine specific clobbers
if (ResultRegTypes.size() == 1) {
     RegResults.push_back(Result);
   } else {
     for (unsigned i = 0, e = ResultRegTypes.size(); i != e; ++i) {
       llvm::Value *Tmp = Builder.CreateExtractValue(Result, i, "asmresult");
       RegResults.push_back(Tmp);
     }
   }
 
+  assert(RegResults.size() == ResultRegTypes.size());
+  assert(RegResults.size() == ResultTruncRegTypes.size());
+  assert(RegResults.size() == ResultRegDests.size());
   for (unsigned i = 0, e = RegResults.size(); i != e; ++i) {
     llvm::Value *Tmp = RegResults[i];
 
     // If the result type of the LLVM IR asm doesn't match the result type of
     // the expression, do the conversion.
     if (ResultRegTypes[i] != ResultTruncRegTypes[i]) {
       llvm::Type *TruncTy = ResultTruncRegTypes[i];
 

lib/CodeGen/CodeGenFunction.h

public:
 
   /// In C++, whether we are code generating a thunk.  This controls whether we
   /// should emit cleanups.
   bool CurFuncIsThunk;
 
   /// In ARC, whether we should autorelease the return value.
   bool AutoreleaseResult;
 
+  /// Whether we processed a Microsoft-style asm block during CodeGen. These can
+  /// potentially set the return value.
+  bool SawAsmBlock;
+
   const CodeGen::CGBlockInfo *BlockInfo;
   llvm::Value *BlockPointer;
 
   llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
   FieldDecl *LambdaThisCaptureField;
 
   /// \brief A mapping from NRVO variables to the flags used to indicate
   /// when the NRVO has been applied to this variable.

lib/CodeGen/CodeGenFunction.cpp

 using namespace CodeGen;
 
 CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext)
     : CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
       Builder(cgm.getModule().getContext(), llvm::ConstantFolder(),
               CGBuilderInserterTy(this)),
       CapturedStmtInfo(nullptr), SanOpts(&CGM.getLangOpts().Sanitize),
       IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
-      BlockInfo(nullptr), BlockPointer(nullptr),
+      SawAsmBlock(false), BlockInfo(nullptr), BlockPointer(nullptr),
       LambdaThisCaptureField(nullptr), NormalCleanupDest(nullptr),
       NextCleanupDestIndex(1), FirstBlockInfo(nullptr), EHResumeBlock(nullptr),
       ExceptionSlot(nullptr), EHSelectorSlot(nullptr),
       DebugInfo(CGM.getModuleDebugInfo()), DisableDebugInfo(false),
       DidCallStackSave(false), IndirectBranch(nullptr), PGO(cgm),
       SwitchInsn(nullptr), SwitchWeights(nullptr), CaseRangeBlock(nullptr),
       UnreachableBlock(nullptr), NumReturnExprs(0), NumSimpleReturnExprs(0),
       CXXABIThisDecl(nullptr), CXXABIThisValue(nullptr), CXXThisValue(nullptr),
else if (isa<CXXConversionDecl>(FD) &&
     llvm_unreachable("no definition for emitted function");
 
   // C++11 [stmt.return]p2:
   //   Flowing off the end of a function [...] results in undefined behavior in
   //   a value-returning function.
   // C11 6.9.1p12:
   //   If the '}' that terminates a function is reached, and the value of the
   //   function call is used by the caller, the behavior is undefined.
-  if (getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() &&
+  if (getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() && !SawAsmBlock &&
       !FD->getReturnType()->isVoidType() && Builder.GetInsertBlock()) {
     if (SanOpts->Return) {
       SanitizerScope SanScope(this);
       EmitCheck(Builder.getFalse(), "missing_return",
                 EmitCheckSourceLocation(FD->getLocation()),
                 None, CRK_Unrecoverable);
     } else if (CGM.getCodeGenOpts().OptimizationLevel == 0)
       Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::trap));

lib/CodeGen/TargetInfo.h

 // These classes wrap the information about a call or function
 // definition used to handle ABI compliancy.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_CODEGEN_TARGETINFO_H
 #define LLVM_CLANG_LIB_CODEGEN_TARGETINFO_H
 
+#include "CGValue.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace llvm {
 class Constant;
 class GlobalValue;
public:
   /// \returns A pointer to a new LLVM type, possibly the same as the original
   /// on success; 0 on failure.
   virtual llvm::Type *adjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
                                           StringRef Constraint,
                                           llvm::Type *Ty) const {
     return Ty;
   }
 
+  /// Adds constraints and types for result registers.
+  virtual void addReturnRegisterOutputs(
+      CodeGen::CodeGenFunction &CGF, CodeGen::LValue ReturnValue,
+      std::string &Constraints, std::vector<llvm::Type *> &ResultRegTypes,
+      std::vector<llvm::Type *> &ResultTruncRegTypes,
+      std::vector<CodeGen::LValue> &ResultRegDests, std::string &AsmString,
+      unsigned NumOutputs) const {}
+
   /// doesReturnSlotInterfereWithArgs - Return true if the target uses an
   /// argument slot for an 'sret' type.
   virtual bool doesReturnSlotInterfereWithArgs() const { return true; }
 
   /// Retrieve the address of a function to call immediately before
   /// calling objc_retainAutoreleasedReturnValue.  The
   /// implementation of objc_autoreleaseReturnValue sniffs the
   /// instruction stream following its return address to decide

lib/CodeGen/TargetInfo.cpp

 // These classes wrap the information about a call or function
 // definition used to handle ABI compliancy.
 //
 //===----------------------------------------------------------------------===//
 
 #include "TargetInfo.h"
 #include "ABIInfo.h"
 #include "CGCXXABI.h"
+#include "CGValue.h"
 #include "CodeGenFunction.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Support/raw_ostream.h"
bool initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
                                llvm::Value *Address) const override;
 
   llvm::Type* adjustInlineAsmType(CodeGen::CodeGenFunction &CGF,
                                   StringRef Constraint,
                                   llvm::Type* Ty) const override {
     return X86AdjustInlineAsmType(CGF, Constraint, Ty);
   }
 
+  void addReturnRegisterOutputs(CodeGenFunction &CGF, LValue ReturnValue,
+                                std::string &Constraints,
+                                std::vector<llvm::Type *> &ResultRegTypes,
+                                std::vector<llvm::Type *> &ResultTruncRegTypes,
+                                std::vector<LValue> &ResultRegDests,
+                                std::string &AsmString,
+                                unsigned NumOutputs) const override;
+
   llvm::Constant *
   getUBSanFunctionSignature(CodeGen::CodeGenModule &CGM) const override {
     unsigned Sig = (0xeb << 0) |  // jmp rel8
                    (0x06 << 8) |  //           .+0x08
                    ('F' << 16) |
                    ('T' << 24);
     return llvm::ConstantInt::get(CGM.Int32Ty, Sig);
   }
 
 };
 
 }
 
+/// Rewrite input constraint references after adding some output constraints.
+/// In the case where there is one output and one input and we add one output,
+/// we need to replace all operand references greater than or equal to 1:
+///     mov $0, $1
+///     mov eax, $1
+/// The result will be:
+///     mov $0, $2
+///     mov eax, $2
+static void rewriteInputConstraintReferences(unsigned FirstIn,
+                                             unsigned NumNewOuts,
+                                             std::string &AsmString) {
+  std::string Buf;
+  llvm::raw_string_ostream OS(Buf);
+  size_t Pos = 0;
+  while (Pos < AsmString.size()) {
+    size_t DollarStart = AsmString.find('$', Pos);
+    if (DollarStart == std::string::npos)
+      DollarStart = AsmString.size();
+    size_t DollarEnd = AsmString.find_first_not_of('$', DollarStart);
+    if (DollarEnd == std::string::npos)
+      DollarEnd = AsmString.size();
+    OS << StringRef(&AsmString[Pos], DollarEnd - Pos);
+    Pos = DollarEnd;
+    size_t NumDollars = DollarEnd - DollarStart;
+    if (NumDollars % 2 != 0 && Pos < AsmString.size()) {
+      // We have an operand reference.
+      size_t DigitStart = Pos;
+      size_t DigitEnd = AsmString.find_first_not_of("0123456789", DigitStart);
+      if (DigitEnd == std::string::npos)
+        DigitEnd = AsmString.size();
+      StringRef OperandStr(&AsmString[DigitStart], DigitEnd - DigitStart);
+      unsigned OperandIndex;
+      if (!OperandStr.getAsInteger(10, OperandIndex)) {
+        if (OperandIndex >= FirstIn)
+          OperandIndex += NumNewOuts;
+        OS << OperandIndex;
+      } else {
+        OS << OperandStr;
+      }
+      Pos = DigitEnd;
+    }
+  }
+  AsmString = std::move(OS.str());
+}
+
+/// Add output constraints for EAX:EDX because they are return registers.
+void X86_32TargetCodeGenInfo::addReturnRegisterOutputs(
+    CodeGenFunction &CGF, LValue ReturnSlot, std::string &Constraints,
+    std::vector<llvm::Type *> &ResultRegTypes,
+    std::vector<llvm::Type *> &ResultTruncRegTypes,
+    std::vector<LValue> &ResultRegDests, std::string &AsmString,
+    unsigned NumOutputs) const {
+  uint64_t RetWidth = CGF.getContext().getTypeSize(ReturnSlot.getType());
+
+  // Use the EAX constraint if the width is 32 or smaller and EAX:EDX if it is
+  // larger.
+  if (!Constraints.empty())
+    Constraints += ',';
+  if (RetWidth <= 32) {
+    Constraints += "={eax}";
+    ResultRegTypes.push_back(CGF.Int32Ty);
+  } else {
+    // Use the 'A' constraint for EAX:EDX.
+    Constraints += "=A";
+    ResultRegTypes.push_back(CGF.Int64Ty);
+  }
+
+  // Truncate EAX or EAX:EDX to an integer of the appropriate size.
+  llvm::Type *CoerceTy = llvm::IntegerType::get(CGF.getLLVMContext(), RetWidth);
+  ResultTruncRegTypes.push_back(CoerceTy);
+
+  // Coerce the integer by bitcasting the return slot pointer.
+  ReturnSlot.setAddress(CGF.Builder.CreateBitCast(ReturnSlot.getAddress(),
+                                                  CoerceTy->getPointerTo()));
+  ResultRegDests.push_back(ReturnSlot);
+
+  rewriteInputConstraintReferences(NumOutputs, 1, AsmString);
+}
+
 /// shouldReturnTypeInRegister - Determine if the given type should be
 /// passed in a register (for the Darwin ABI).
 bool X86_32ABIInfo::shouldReturnTypeInRegister(QualType Ty,
                                                ASTContext &Context) const {
   uint64_t Size = Context.getTypeSize(Ty);
 
   // Type must be register sized.
   if (!isRegisterSize(Size))

test/CodeGen/ms-inline-asm.c

 }
 
 int t8() {
   __asm int 4 ; } comments for single-line asm
   __asm {}
   __asm int 4
   return 10;
 // CHECK: t8
-// CHECK: call void asm sideeffect inteldialect "int $$4\0A\09int $$4", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call i32 asm sideeffect inteldialect "int $$4\0A\09int $$4", "={eax},~{dirflag},~{fpsr},~{flags}"()
 // CHECK: ret i32 10
 }
 
 void t9() {
   __asm {
     push ebx
     { mov ebx, 0x07 }
     __asm { pop ebx }
   }
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect "push ebx\0A\09mov ebx, $$0x07\0A\09pop ebx", "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
 
 unsigned t10(void) {
   unsigned i = 1, j;
   __asm {
     mov eax, i
     mov j, eax
   }
   return j;
 // CHECK: t10
+// CHECK: [[r:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: [[I:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: [[J:%[a-zA-Z0-9]+]] = alloca i32, align 4
 // CHECK: store i32 1, i32* [[I]], align 4
-// CHECK: call void asm sideeffect inteldialect "mov eax, dword ptr $1\0A\09mov dword ptr $0, eax", "=*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
+// CHECK: call i32 asm sideeffect inteldialect "mov eax, dword ptr $2\0A\09mov dword ptr $0, eax", "=*m,={eax},*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}})
 // CHECK: [[RET:%[a-zA-Z0-9]+]] = load i32* [[J]], align 4
 // CHECK: ret i32 [[RET]]
 }
 
 void t11(void) {
   __asm mov eax, 1
 // CHECK: t11
 // CHECK: call void asm sideeffect inteldialect "mov eax, $$1", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 unsigned t12(void) {
   unsigned i = 1, j, l = 1, m;
   __asm {
     mov eax, i
     mov j, eax
     mov eax, l
     mov m, eax
   }
   return j + m;
 // CHECK: t12
-// CHECK: call void asm sideeffect inteldialect "mov eax, dword ptr $2\0A\09mov dword ptr $0, eax\0A\09mov eax, dword ptr $3\0A\09mov dword ptr $1, eax", "=*m,=*m,*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
+// CHECK: call i32 asm sideeffect inteldialect "mov eax, dword ptr $3\0A\09mov dword ptr $0, eax\0A\09mov eax, dword ptr $4\0A\09mov dword ptr $1, eax", "=*m,=*m,={eax},*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}}, i32* %{{.*}})
 }
 
 void t13() {
   char i = 1;
   short j = 2;
   __asm movzx eax, i
   __asm movzx eax, j
 // CHECK: t13

 }
 
 int results[2] = {13, 37};
 int *t30()
 // CHECK: t30
 {
   int *res;
   __asm lea edi, results
-// CHECK: lea edi, dword ptr $1
+// CHECK: lea edi, dword ptr $2
   __asm mov res, edi
 // CHECK: mov dword ptr $0, edi
   return res;
-// CHECK: "=*m,*m,~{edi},~{dirflag},~{fpsr},~{flags}"(i32** %{{.*}}, [2 x i32]* @{{.*}})
+// CHECK: "=*m,={eax},*m,~{edi},~{dirflag},~{fpsr},~{flags}"(i32** %{{.*}}, [2 x i32]* @{{.*}})
 }
 
 void t31() {
 // CHECK: t31
   __asm pushad
 // CHECK: pushad
   __asm popad
 // CHECK: popad

test/CodeGenCXX/ms-inline-asm-return.cpp

+// RUN: %clang_cc1 %s -emit-llvm -o - -fasm-blocks | FileCheck %s
+
+// Check that we take EAX or EAX:EDX and return it from these functions for MSVC
+// compatibility.
+
+extern "C" {
+
+long long f_i64() {
+  __asm {
+    mov eax, 1
+    mov edx, 1
+  }
+}
+// CHECK-LABEL: define i64 @f_i64()
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "=A,~{eax},{{.*}}"
+// CHECK: ret i64 %[[r]]
+
+int f_i32() {
+  __asm {
+    mov eax, 1
+    mov edx, 1
+  }
+}
+// CHECK-LABEL: define i32 @f_i32()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: ret i32 %[[r]]
+
+short f_i16() {
+  __asm {
+    mov eax, 1
+    mov edx, 1
+  }
+}
+// CHECK-LABEL: define signext i16 @f_i16()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r_i16:[^ ]*]] = trunc i32 %[[r]] to i16
+// CHECK: ret i16 %[[r_i16]]
+
+char f_i8() {
+  __asm {
+    mov eax, 1
+    mov edx, 1
+  }
+}
+// CHECK-LABEL: define signext i8 @f_i8()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
+// CHECK: ret i8 %[[r_i8]]
+
+bool f_i1() {
+  __asm {
+    mov eax, 1
+    mov edx, 1
+  }
+}
+// CHECK-LABEL: define zeroext i1 @f_i1()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$1\0A\09mov edx, $$1", "={eax},~{eax},{{.*}}"
+// CHECK: %[[r_i8:[^ ]*]] = trunc i32 %[[r]] to i8
+// CHECK: store i8 %[[r_i8]], i8* %{{.*}}
+// CHECK: %[[r_i1:[^ ]*]] = load i1* %{{.*}}
+// CHECK: ret i1 %[[r_i1]]
+
+struct FourChars {
+  char a, b, c, d;
+};
+FourChars f_s4() {
+  __asm {
+    mov eax, 0x01010101
+  }
+}
+// CHECK-LABEL: define i32 @f_s4()
+// CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$0x01010101", "={eax},~{eax},{{.*}}"
+// CHECK: store i32 %[[r]], i32* %{{.*}}
+// CHECK: %[[r_i32:[^ ]*]] = load i32* %{{.*}}
+// CHECK: ret i32 %[[r_i32]]
+
+struct EightChars {
+  char a, b, c, d, e, f, g, h;
+};
+EightChars f_s8() {
+  __asm {
+    mov eax, 0x01010101
+    mov edx, 0x01010101
+  }
+}
+// CHECK-LABEL: define i64 @f_s8()
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$0x01010101\0A\09mov edx, $$0x01010101", "=A,~{eax},{{.*}}"
+// CHECK: store i64 %[[r]], i64* %{{.*}}
+// CHECK: %[[r_i64:[^ ]*]] = load i64* %{{.*}}
+// CHECK: ret i64 %[[r_i64]]
+
+} // extern "C"