This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by mharoush on May 17 2017, 7:24 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mharoush created this revision.May 17 2017, 7:24 AM
rnk added inline comments.May 17 2017, 10:33 AM
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 updated this revision to Diff 99780.May 22 2017, 9:23 AM

removed functions and dropped an irrelevant test case.

mharoush marked an inline comment as done.May 22 2017, 9:28 AM
mharoush added inline comments.
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 added inline comments.May 22 2017, 9:52 AM
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 updated this revision to Diff 101689.Jun 7 2017, 4:18 AM
mharoush updated this revision to Diff 101693.Jun 7 2017, 4:41 AM

added struct case to the test

mharoush updated this revision to Diff 102172.Jun 12 2017, 4:21 AM
mharoush marked 6 inline comments as done.

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

rnk added inline comments.Jun 26 2017, 8:41 AM
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 this revision to Diff 104390.Jun 28 2017, 5:53 AM

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.

rnk accepted this revision.Jul 20 2017, 9:48 AM

lgtm

This revision is now accepted and ready to land.Jul 20 2017, 9:48 AM
This revision was automatically updated to reflect the committed changes.
kcc added a subscriber: kcc.Jul 25 2017, 10:50 AM

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