[Clang][x86][Inline Asm] - Enum support for MS syntax

[mharoush]

Summary

This patch enables the usage of constant Enum identifiers within Microsoft style inline assembly statements.

part 1 out of 2.
[D33278]

[rnk]

lib/Parse/ParseStmtAsm.cpp

100 ↗

(On Diff #99295)

Please format this, clang-format will do the job

test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

12 ↗

(On Diff #99295)

Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing ms-inline-asm.c tests do so that this test is easier to debug when it fails.

[mharoush]

removed functions and dropped an irrelevant test case.

[mharoush]

lib/Parse/ParseStmtAsm.cpp

100 ↗

(On Diff #99295)

Revised

test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

12 ↗

(On Diff #99295)

This test case was just meant verify that other Integer constants are not folded since we get a different behavior for statements such as mov eax, [a].
#---
In this example X86AsmParser regards the address of the variable 'a' and not its value i.e. we end up with the value of 'a' in eax (loaded from the stack) and not with the value pointed by the const int value of 'a' as its address.

I can clarify the intention in a comment or completely remove the test case since this isn't really required here.

[rnk]

lib/Sema/SemaStmtAsm.cpp

674 ↗

(On Diff #99780)

Please add a comment explaining that non-enum integer constants should not be folded. I expected that MSVC would fold constexpr integers here, but it does not, so that's worth noting.

test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

12 ↗

(On Diff #99295)

The test case is fine and the intention is clear, I am just suggesting that you use more FileCheck features (CHECK-LABEL and CHECK-SAME) to make the test easier to debug when it fails in the future.

[mharoush]

added struct case to the test

[mharoush]

Restored old test case for constant int folding and added check-label directives for each tester function.

[rnk]

test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

12 ↗

(On Diff #99295)

You didn't implement this suggestion, please take a look at ms-inline-asm.c. It has tests that look like this:

void t13() {
  char i = 1;
  short j = 2;
  __asm movzx eax, i
  __asm movzx eax, j
// CHECK-LABEL: define void @t13()
// CHECK: call void asm sideeffect inteldialect
// CHECK-SAME: movzx eax, byte ptr $0
// CHECK-SAME: movzx eax, word ptr $1
// CHECK-SAME: "*m,*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i8* %{{.*}}i, i16* %{{.*}}j)
}

This is much less error-prone and easier to debug when it fails.

[mharoush]

Updated inline asm tests to look for decimal immediate value instead of looking for the original string e.g. 10 vs 0xA and other variations.
Also updated the test cases to use check-same etc.

[mharoush]

ping

[rnk]

lgtm

[kcc]

Hi.
This patch causes the msan bots to complain. Please fix or revert ASAP.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6702/steps/check-clang%20msan/logs/stdio
Testing: 0 .. 10..
FAIL: Clang :: CodeGen/ms_this.cpp (2142 of 11057)

• TEST 'Clang :: CodeGen/ms_this.cpp' FAILED ****

Script:

/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_msan/./bin/clang -cc1 -internal-isystem /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_msan/lib/clang/6.0.0/include -nostdsysteminc -triple x86_64-pc-win32 -fasm-blocks -emit-llvm /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/CodeGen/ms_this.cpp -o - | /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_msan/./bin/FileCheck /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/CodeGen/ms_this.cpp

Exit Code: 2

Command Output (stderr):

4713==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x3143ba3 in (anonymous namespace)::X86AsmParser::ParseIntelIdentifier(llvm::MCExpr const*&, llvm::StringRef&, llvm::InlineAsmIdentifierInfo&, bool, llvm::SMLoc&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1664:7
#1 0x313f288 in (anonymous namespace)::X86AsmParser::ParseIntelExpression((anonymous namespace)::X86AsmParser::IntelExprStateMachine&, llvm::SMLoc&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1440:13
#2 0x31389e7 in (anonymous namespace)::X86AsmParser::ParseIntelBracExpression(unsigned int, llvm::SMLoc, long, bool, unsigned int) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1555:7
#3 0x31307f8 in ParseIntelOperand /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1961:12
#4 0x31307f8 in (anonymous namespace)::X86AsmParser::ParseOperand() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1225
#5 0x3118cda in (anonymous namespace)::X86AsmParser::ParseInstruction(llvm::ParseInstructionInfo&, llvm::StringRef, llvm::SMLoc, llvm::SmallVectorImpl<std::__1::unique_ptr<llvm::MCParsedAsmOperand, std::__1::default_delete<llvm::MCParsedAsmOperand> > >&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2510:44
#6 0x4bb9a48 in (anonymous namespace)::AsmParser::parseStatement((anonymous namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/MC/MCParser/AsmParser.cpp:2034:42
#7 0x4ba63a3 in (anonymous namespace)::AsmParser::parseMSInlineAsm(void*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&, unsigned int&, unsigned int&, llvm::SmallVectorImpl<std::__1::pair<void*, bool> >&, llvm::SmallVectorImpl<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >&, llvm::SmallVectorImpl<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >&, llvm::MCInstrInfo const*, llvm::MCInstPrinter const*, llvm::MCAsmParserSemaCallback&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/MC/MCParser/AsmParser.cpp:5343:25
#8 0x82f1100 in clang::Parser::ParseMicrosoftAsmStatement(clang::SourceLocation) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmtAsm.cpp:619:15
#9 0x82f6310 in clang::Parser::ParseAsmStatement(bool&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmtAsm.cpp:682:12
#10 0x82d15d9 in clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:277:11
#11 0x82cfff1 in clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, clang::Parser::AllowedConstructsKind, clang::SourceLocation*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:110:20
#12 0x82e677b in clang::Parser::ParseCompoundStatementBody(bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1001:11
#13 0x82e859f in clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1967:21
#14 0x811b151 in clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/Parser.cpp:1214:10
#15 0x8167f18 in clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int, clang::SourceLocation*, clang::Parser::ForRangeInit*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseDecl.cpp:1953:11
#16 0x8118d8c in clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/Parser.cpp:981:10
#17 0x8117646 in clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/Parser.cpp:997:12
#18 0x8114428 in clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/Parser.cpp:847:12
#19 0x8110552 in clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/Parser.cpp:615:12
#20 0x8101168 in clang::ParseAST(clang::Sema&, bool, bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseAST.cpp:147:18
#21 0x65ab84a in clang::FrontendAction::Execute() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:902:8
#22 0x64e936c in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:980:11
#23 0x6786441 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:251:25
#24 0x98efea in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/cc1_main.cpp:221:13
#25 0x988444 in ExecuteCC1Tool /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/driver.cpp:306:12
#26 0x988444 in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/driver.cpp:387

cfe/trunk/lib/Sema/SemaStmtAsm.cpp

ExprResult Sema::LookupInlineAsmIdentifier(CXXScopeSpec &SS,
   if (!Result.isUsable()) return Result;
 
   Result = CheckPlaceholderExpr(Result.get());
   if (!Result.isUsable()) return Result;
 
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
     return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
     return Result;
   }
 
   // Any sort of function type is fine.
   if (T->isFunctionType()) {
     return Result;
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
     return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-    Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+    Info.setKindVariable();
+    return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa<clang::EnumType>(T) &&
+    Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+    Info.ConstIntValue = EvlResult.Val.getInt();
+    Info.setKindConstEnum();
+  }
   return Result;
 }
 
 bool Sema::LookupInlineAsmField(StringRef Base, StringRef Member,
                                 unsigned &Offset, SourceLocation AsmLoc) {
   Offset = 0;
   SmallVector<StringRef, 2> Members;
   Member.split(Members, ".");
ExprResult Result = BuildMemberReferenceExpr(
       SourceLocation(), nullptr, FieldResult, nullptr, nullptr);
   if (Result.isInvalid())
     return Result;
   Info.OpDecl = Result.get();
 
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
 
 StmtResult Sema::ActOnMSAsmStmt(SourceLocation AsmLoc, SourceLocation LBraceLoc,
                                 ArrayRef<Token> AsmToks,
                                 StringRef AsmString,
                                 unsigned NumOutputs, unsigned NumInputs,

cfe/trunk/test/CodeGen/ms-inline-asm.c

 // CHECK: call void asm sideeffect inteldialect "mov ebx, eax\0A\09mov ecx, ebx", "~{ebx},~{ecx},~{dirflag},~{fpsr},~{flags}"()
 // CHECK: ret void
   __asm mov ebx, eax __asm mov ecx, ebx
 }
 
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
   __asm {
     int 0x2cU ; } asm comments are fun! }{
   }
   __asm {
     {
       int 0x2c ; } asm comments are fun! }{
     }
   }
   __asm {}
   __asm {
     ;
     ; label
     mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 int t8() {
   __asm int 4 ; } comments for single-line asm
   __asm {}
   __asm { int 5}
void t9() {
   __asm {
     push ebx
     { mov ebx, 0x07 }
     __asm { pop ebx }
   }
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
 
 unsigned t10(void) {
   unsigned i = 1, j;
   __asm {
     mov eax, i
void t21() {
   __asm {
     __asm push ebx
     __asm mov ebx, 07H
     __asm pop ebx
   }
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
 
 extern void t22_helper(int x);
 void t22() {
   int x = 0;
   __asm {
void t24() {
   __asm call t24_helper
 // CHECK: t24
 // CHECK: call void asm sideeffect inteldialect "call dword ptr $0", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @t24_helper)
 }
 
 void t25() {
 // CHECK: t25
   __asm mov eax, 0ffffffffh
-// CHECK: mov eax, $$0ffffffffh
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHECK: mov eax, $$0a2h
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010b
-// CHECK: mov eax, $$10100010b
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010BU
 // CHECK: mov eax, $$162
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t26() {
 // CHECK: t26
   __asm pushad

cfe/trunk/test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+  enum { A = 17 };
+  int r;
+} y;
+}
+// CHECK-LABEL: x86_enum_only
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  // Other constant type folding is currently unwanted.
+  __asm mov eax, [a]
+  }
+
+// CHECK-LABEL: x86_enum_namespaces
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK-SAME: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+// CHECK-LABEL: x86_enum_arithmethic
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK-SAME: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK-SAME: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK-SAME: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK-SAME: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+// CHECK-LABEL: x86_enum_mem
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}

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

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: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "mov eax, $$16843009", "={eax},~{eax},{{.*}}"
 // CHECK: store i32 %[[r]], i32* %{{.*}}
 // CHECK: %[[r_i32:[^ ]*]] = load i32, i32* %{{.*}}
 // CHECK: ret i32 %[[r_i32]]
 
 struct EightChars {
   char a, b, c, d, e, f, g, h;
 };
 EightChars f_s8() {
   __asm {
     mov eax, 01010101h
     mov edx, 01010101b
   }
 }
 // CHECK-LABEL: define i64 @f_s8()
-// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$01010101h\0A\09mov edx, $$01010101b", "=A,~{eax},{{.*}}"
+// CHECK: %[[r:[^ ]*]] = call i64 asm sideeffect inteldialect "mov eax, $$16843009\0A\09mov edx, $$85", "=A,~{eax},{{.*}}"
 // CHECK: store i64 %[[r]], i64* %{{.*}}
 // CHECK: %[[r_i64:[^ ]*]] = load i64, i64* %{{.*}}
 // CHECK: ret i64 %[[r_i64]]
 
 } // extern "C"
 
 int main() {
   __asm xor eax, eax
 }
 // CHECK-LABEL: define i32 @main()
 // CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "={eax},{{.*}}"
 // CHECK: ret i32 %[[r]]