[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

2019-09-04 Thread Eric Astor via Phabricator via cfe-commits
epastor created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Match cl.exe's mangling for decomposition declarations.

Decomposition declarations are considered to be anonymous structs, and use the 
same convention as for anonymous struct/union declarations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67202

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/MicrosoftMangle.cpp


Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -868,16 +868,11 @@
   }
 
   if (const DecompositionDecl *DD = dyn_cast(ND)) {
-// FIXME: Invented mangling for decomposition declarations:
-//   [X,Y,Z]
-// where X,Y,Z are the names of the bindings.
-llvm::SmallString<128> Name("[");
-for (auto *BD : DD->bindings()) {
-  if (Name.size() > 1)
-Name += ',';
-  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
-}
-Name += ']';
+// Decomposition declarations are considered anonymous, and get
+// numbered with a $S prefix.
+llvm::SmallString<64> Name("$S");
+// Get a unique id for the anonymous struct.
+Name += llvm::utostr(Context.getAnonymousStructId(DD) + 1);
 mangleSourceName(Name);
 break;
   }
Index: clang/include/clang/AST/Mangle.h
===
--- clang/include/clang/AST/Mangle.h
+++ clang/include/clang/AST/Mangle.h
@@ -56,7 +56,7 @@
 
   llvm::DenseMap GlobalBlockIds;
   llvm::DenseMap LocalBlockIds;
-  llvm::DenseMap AnonStructIds;
+  llvm::DenseMap AnonStructIds;
 
 public:
   ManglerKind getKind() const { return Kind; }
@@ -82,9 +82,9 @@
 return Result.first->second;
   }
 
-  uint64_t getAnonymousStructId(const TagDecl *TD) {
-std::pair::iterator, bool>
-Result = AnonStructIds.insert(std::make_pair(TD, 
AnonStructIds.size()));
+  uint64_t getAnonymousStructId(const NamedDecl *D) {
+std::pair::iterator, bool>
+Result = AnonStructIds.insert(std::make_pair(D, AnonStructIds.size()));
 return Result.first->second;
   }
 


Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -868,16 +868,11 @@
   }
 
   if (const DecompositionDecl *DD = dyn_cast(ND)) {
-// FIXME: Invented mangling for decomposition declarations:
-//   [X,Y,Z]
-// where X,Y,Z are the names of the bindings.
-llvm::SmallString<128> Name("[");
-for (auto *BD : DD->bindings()) {
-  if (Name.size() > 1)
-Name += ',';
-  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
-}
-Name += ']';
+// Decomposition declarations are considered anonymous, and get
+// numbered with a $S prefix.
+llvm::SmallString<64> Name("$S");
+// Get a unique id for the anonymous struct.
+Name += llvm::utostr(Context.getAnonymousStructId(DD) + 1);
 mangleSourceName(Name);
 break;
   }
Index: clang/include/clang/AST/Mangle.h
===
--- clang/include/clang/AST/Mangle.h
+++ clang/include/clang/AST/Mangle.h
@@ -56,7 +56,7 @@
 
   llvm::DenseMap GlobalBlockIds;
   llvm::DenseMap LocalBlockIds;
-  llvm::DenseMap AnonStructIds;
+  llvm::DenseMap AnonStructIds;
 
 public:
   ManglerKind getKind() const { return Kind; }
@@ -82,9 +82,9 @@
 return Result.first->second;
   }
 
-  uint64_t getAnonymousStructId(const TagDecl *TD) {
-std::pair::iterator, bool>
-Result = AnonStructIds.insert(std::make_pair(TD, AnonStructIds.size()));
+  uint64_t getAnonymousStructId(const NamedDecl *D) {
+std::pair::iterator, bool>
+Result = AnonStructIds.insert(std::make_pair(D, AnonStructIds.size()));
 return Result.first->second;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

2019-09-05 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 218934.
epastor added a comment.

- Add testing for the new deprecation declarations mangling


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67202/new/

https://reviews.llvm.org/D67202

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-ms-cxx17.cpp


Index: clang/test/CodeGenCXX/mangle-ms-cxx17.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-ms-cxx17.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++1z -fms-extensions -emit-llvm %s -o - 
-triple=i386-pc-win32 -fms-compatibility-version=19.10 | FileCheck 
-allow-deprecated-dag-overlap %s --check-prefix=CHECK --check-prefix=MSVC2017
+// RUN: %clang_cc1 -std=c++1z -fms-extensions -emit-llvm %s -o - 
-triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck 
-allow-deprecated-dag-overlap %s --check-prefix=CHECK --check-prefix=MSVC2015
+
+struct S {
+int x;
+double y;
+};
+S f();
+
+// CHECK-DAG: "?$S1@@3US@@B"
+const auto [x0, y0] = f();
+// CHECK-DAG: "?$S2@@3US@@B"
+const auto [x1, y1] = f();
+
+static union {
+int a;
+double b;
+};
+
+// CHECK-DAG: "?$S4@@3US@@B"
+const auto [x2, y2] = f();
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -868,16 +868,11 @@
   }
 
   if (const DecompositionDecl *DD = dyn_cast(ND)) {
-// FIXME: Invented mangling for decomposition declarations:
-//   [X,Y,Z]
-// where X,Y,Z are the names of the bindings.
-llvm::SmallString<128> Name("[");
-for (auto *BD : DD->bindings()) {
-  if (Name.size() > 1)
-Name += ',';
-  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
-}
-Name += ']';
+// Decomposition declarations are considered anonymous, and get
+// numbered with a $S prefix.
+llvm::SmallString<64> Name("$S");
+// Get a unique id for the anonymous struct.
+Name += llvm::utostr(Context.getAnonymousStructId(DD) + 1);
 mangleSourceName(Name);
 break;
   }
Index: clang/include/clang/AST/Mangle.h
===
--- clang/include/clang/AST/Mangle.h
+++ clang/include/clang/AST/Mangle.h
@@ -56,7 +56,7 @@
 
   llvm::DenseMap GlobalBlockIds;
   llvm::DenseMap LocalBlockIds;
-  llvm::DenseMap AnonStructIds;
+  llvm::DenseMap AnonStructIds;
 
 public:
   ManglerKind getKind() const { return Kind; }
@@ -82,9 +82,9 @@
 return Result.first->second;
   }
 
-  uint64_t getAnonymousStructId(const TagDecl *TD) {
-std::pair::iterator, bool>
-Result = AnonStructIds.insert(std::make_pair(TD, 
AnonStructIds.size()));
+  uint64_t getAnonymousStructId(const NamedDecl *D) {
+std::pair::iterator, bool>
+Result = AnonStructIds.insert(std::make_pair(D, AnonStructIds.size()));
 return Result.first->second;
   }
 


Index: clang/test/CodeGenCXX/mangle-ms-cxx17.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-ms-cxx17.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++1z -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.10 | FileCheck -allow-deprecated-dag-overlap %s --check-prefix=CHECK --check-prefix=MSVC2017
+// RUN: %clang_cc1 -std=c++1z -fms-extensions -emit-llvm %s -o - -triple=i386-pc-win32 -fms-compatibility-version=19.00 | FileCheck -allow-deprecated-dag-overlap %s --check-prefix=CHECK --check-prefix=MSVC2015
+
+struct S {
+int x;
+double y;
+};
+S f();
+
+// CHECK-DAG: "?$S1@@3US@@B"
+const auto [x0, y0] = f();
+// CHECK-DAG: "?$S2@@3US@@B"
+const auto [x1, y1] = f();
+
+static union {
+int a;
+double b;
+};
+
+// CHECK-DAG: "?$S4@@3US@@B"
+const auto [x2, y2] = f();
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -868,16 +868,11 @@
   }
 
   if (const DecompositionDecl *DD = dyn_cast(ND)) {
-// FIXME: Invented mangling for decomposition declarations:
-//   [X,Y,Z]
-// where X,Y,Z are the names of the bindings.
-llvm::SmallString<128> Name("[");
-for (auto *BD : DD->bindings()) {
-  if (Name.size() > 1)
-Name += ',';
-  Name += BD->getDeclName().getAsIdentifierInfo()->getName();
-}
-Name += ']';
+// Decomposition declarations are considered anonymous, and get
+// numbered with a $S prefix.
+llvm::SmallString<64> Name("$S");
+// Get a unique id for the anonymous struct.
+Name += llvm::utostr(Context.getAnonymousStructId(DD) + 1);
 mangleSourceName(Name);
 

[PATCH] D67202: Implement Microsoft-compatible mangling for decomposition declarations.

2019-09-05 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

The change passes ninja check-clang - and I've added a test per your 
suggestions. Thanks!

As for llvm-undname: it works reasonably well, but these are anonymous names. 
It successfully recognizes $S1, etc. as the "name", though!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67202/new/

https://reviews.llvm.org/D67202



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 235617.
epastor added a comment.

Rebase on top of HEAD

- Add compatibility with dc5b614fa9a1 
 (which 
changed handling for call operands in x86 assembly)
- Remove another artifact of local testing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -53,6 +53,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -279,13 +280,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isCallOperand() const override { return CallOperand; }
   void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; }
@@ -617,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread Eric Astor via Phabricator via cfe-commits
epastor added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+getParser().parsePrimaryExpr(Val, End))
+  return Error(Start, "unexpected token!");
+  } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {

rnk wrote:
> epastor wrote:
> > epastor wrote:
> > > rnk wrote:
> > > > Please test this corner case, I imagine it looks like:
> > > >   mov eax, offset 3
> > > Interesting. This corner case didn't trigger in that scenario; we get an 
> > > "expected identifier" error message with good source location, followed 
> > > by another error "use of undeclared label '3'" in debug builds... and in 
> > > release builds, we instead get a crash. On tracing the crash, it's a 
> > > AsmStrRewrite applying to a SMLoc not coming from the same string...
> > > 
> > > As near as I can tell, the issue is that we end up trying to parse "3" as 
> > > a not-yet-declared label, as such expand it to 
> > > `__MSASMLABEL_.${:uid}__3`, and then end up in a bad state because the 
> > > operand rewrite is applying to the expanded symbol... which isn't in the 
> > > same AsmString. If you use an actual undeclared label, you hit the same 
> > > crash in release builds.
> > > 
> > > This is going to take some work; I'll get back to it in a day or two.
> > Fixed; we now get the same errors in this scenario as we do in the current 
> > LLVM trunk, reporting "expected identifier" and "use of undeclared label 
> > '3'".
> > 
> > On the other hand, I'm still working on finding a scenario that DOES 
> > trigger this corner case.
> I see. Well, it sounds like it was a useful test. :)
Very useful indeed!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 235619.
epastor marked 5 inline comments as done.
epastor added a comment.

- Fix a few final issues.
  - Pack the AsmRewrite struct more optimally.
  - Only check if we can fuse rewrites with ones we haven't already reached!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -53,6 +53,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -279,13 +280,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isCallOperand() const override { return CallOperand; }
   void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; }
@@ -617,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= tr

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 235620.
epastor added a comment.

- The three-argument version of find_if is std::find_if, and the qualification 
is required.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -53,6 +53,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -279,13 +280,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isCallOperand() const override { return CallOperand; }
   void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; }
@@ -617,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
=

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread Eric Astor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a7aa252a32a: [X86][AsmParser] re-introduce 
'offset' operator (authored by epastor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -53,6 +53,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -279,13 +280,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isCallOperand() const override { return CallOperand; }
   void setCallOperand(bool IsCallOperand) { CallOperand = IsCallOperand; }
@@ -617,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

[PATCH] D72417: [ms] [X86] Use "P" modifier on all branch-target operands in inline X86 assembly.

2020-01-08 Thread Eric Astor via Phabricator via cfe-commits
epastor created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Extend llvm.org/D71677  to apply to all 
branch-target operands, rather than special-casing call instructions.

Also add a regression test for llvm.org/PR44272, since this finishes fixing it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72417

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  llvm/include/llvm/MC/MCInstrDesc.h
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86InstrControl.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/utils/TableGen/InstrInfoEmitter.cpp
  llvm/utils/TableGen/X86RecognizableInstr.cpp

Index: llvm/utils/TableGen/X86RecognizableInstr.cpp
===
--- llvm/utils/TableGen/X86RecognizableInstr.cpp
+++ llvm/utils/TableGen/X86RecognizableInstr.cpp
@@ -879,9 +879,9 @@
   TYPE("i128mem", TYPE_M)
   TYPE("i256mem", TYPE_M)
   TYPE("i512mem", TYPE_M)
-  TYPE("i64i32imm_pcrel", TYPE_REL)
-  TYPE("i16imm_pcrel",TYPE_REL)
-  TYPE("i32imm_pcrel",TYPE_REL)
+  TYPE("i64i32imm_brtarget",  TYPE_REL)
+  TYPE("i16imm_brtarget", TYPE_REL)
+  TYPE("i32imm_brtarget", TYPE_REL)
   TYPE("ccode",   TYPE_IMM)
   TYPE("AVX512RC",TYPE_IMM)
   TYPE("brtarget32",  TYPE_REL)
@@ -1169,45 +1169,45 @@
   if(OpSize != X86Local::OpSize16) {
 // For instructions without an OpSize prefix, a declared 16-bit register or
 // immediate encoding is special.
-ENCODING("i16imm",ENCODING_IW)
+ENCODING("i16imm",   ENCODING_IW)
   }
-  ENCODING("i16imm",  ENCODING_Iv)
-  ENCODING("i16i8imm",ENCODING_IB)
-  ENCODING("i32imm",  ENCODING_Iv)
-  ENCODING("i32i8imm",ENCODING_IB)
-  ENCODING("i64i32imm",   ENCODING_ID)
-  ENCODING("i64i8imm",ENCODING_IB)
-  ENCODING("i8imm",   ENCODING_IB)
-  ENCODING("u8imm",   ENCODING_IB)
-  ENCODING("i16u8imm",ENCODING_IB)
-  ENCODING("i32u8imm",ENCODING_IB)
-  ENCODING("i64u8imm",ENCODING_IB)
-  ENCODING("i64i32imm_pcrel", ENCODING_ID)
-  ENCODING("i16imm_pcrel",ENCODING_IW)
-  ENCODING("i32imm_pcrel",ENCODING_ID)
-  ENCODING("brtarget32",  ENCODING_ID)
-  ENCODING("brtarget16",  ENCODING_IW)
-  ENCODING("brtarget8",   ENCODING_IB)
-  ENCODING("i64imm",  ENCODING_IO)
-  ENCODING("offset16_8",  ENCODING_Ia)
-  ENCODING("offset16_16", ENCODING_Ia)
-  ENCODING("offset16_32", ENCODING_Ia)
-  ENCODING("offset32_8",  ENCODING_Ia)
-  ENCODING("offset32_16", ENCODING_Ia)
-  ENCODING("offset32_32", ENCODING_Ia)
-  ENCODING("offset32_64", ENCODING_Ia)
-  ENCODING("offset64_8",  ENCODING_Ia)
-  ENCODING("offset64_16", ENCODING_Ia)
-  ENCODING("offset64_32", ENCODING_Ia)
-  ENCODING("offset64_64", ENCODING_Ia)
-  ENCODING("srcidx8", ENCODING_SI)
-  ENCODING("srcidx16",ENCODING_SI)
-  ENCODING("srcidx32",ENCODING_SI)
-  ENCODING("srcidx64",ENCODING_SI)
-  ENCODING("dstidx8", ENCODING_DI)
-  ENCODING("dstidx16",ENCODING_DI)
-  ENCODING("dstidx32",ENCODING_DI)
-  ENCODING("dstidx64",ENCODING_DI)
+  ENCODING("i16imm", ENCODING_Iv)
+  ENCODING("i16i8imm",   ENCODING_IB)
+  ENCODING("i32imm", ENCODING_Iv)
+  ENCODING("i32i8imm",   ENCODING_IB)
+  ENCODING("i64i32imm",  ENCODING_ID)
+  ENCODING("i64i8imm",   ENCODING_IB)
+  ENCODING("i8imm",  ENCODING_IB)
+  ENCODING("u8imm",  ENCODING_IB)
+  ENCODING("i16u8imm",   ENCODING_IB)
+  ENCODING("i32u8imm",   ENCODING_IB)
+  ENCODING("i64u8imm",   ENCODING_IB)
+  ENCODING("i64i32imm_brtarget", ENCODING_ID)
+  ENCODING("i16imm_brtarget",ENCODING_IW)
+  ENCODING("i32imm_brtarget",ENCODING_ID)
+  ENCODING("brtarget32", ENCODING_ID)
+  ENCODING("brtarget16", ENCODING_IW)
+  ENCODING("brtarget8",  ENCODING_IB)
+  ENCODING("i64imm", ENCODING_IO)
+  ENCODING("offset16_8", ENCODING_Ia)
+  ENCODING("offset16_16",ENCODING_Ia)
+  ENCODING("offset16_32",ENCODING_Ia)
+  ENCODING("offset32_8", ENCODING_Ia)
+  ENCODING("offset32_16",ENCODING_Ia)
+  ENCODING("offset32_32",ENCODING_Ia)
+  ENCODING("offset32_64",ENCODING_Ia)
+  ENCODING("offset64_8", ENCODING_Ia)
+  ENCODING("offset64_16",ENCODING_Ia)
+  ENCODING("offset64_32",ENCODING_Ia)
+  ENCODING("offset64_64",ENCODING_Ia)
+  ENCODING("srcidx8",ENCODING_SI)
+  ENCODING("srcidx16",   ENCODING_SI)
+  ENCODING("srcidx32",   ENCODING_

[PATCH] D72417: [ms] [X86] Use "P" modifier on all branch-target operands in inline X86 assembly.

2020-01-09 Thread Eric Astor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c545f6dbcbb: [ms] [X86] Use "P" modifier on all 
branch-target operands in inline X86… (authored by epastor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72417/new/

https://reviews.llvm.org/D72417

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  llvm/include/llvm/MC/MCInstrDesc.h
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86InstrControl.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/utils/TableGen/InstrInfoEmitter.cpp
  llvm/utils/TableGen/X86RecognizableInstr.cpp

Index: llvm/utils/TableGen/X86RecognizableInstr.cpp
===
--- llvm/utils/TableGen/X86RecognizableInstr.cpp
+++ llvm/utils/TableGen/X86RecognizableInstr.cpp
@@ -879,9 +879,9 @@
   TYPE("i128mem", TYPE_M)
   TYPE("i256mem", TYPE_M)
   TYPE("i512mem", TYPE_M)
-  TYPE("i64i32imm_pcrel", TYPE_REL)
-  TYPE("i16imm_pcrel",TYPE_REL)
-  TYPE("i32imm_pcrel",TYPE_REL)
+  TYPE("i64i32imm_brtarget",  TYPE_REL)
+  TYPE("i16imm_brtarget", TYPE_REL)
+  TYPE("i32imm_brtarget", TYPE_REL)
   TYPE("ccode",   TYPE_IMM)
   TYPE("AVX512RC",TYPE_IMM)
   TYPE("brtarget32",  TYPE_REL)
@@ -1169,45 +1169,45 @@
   if(OpSize != X86Local::OpSize16) {
 // For instructions without an OpSize prefix, a declared 16-bit register or
 // immediate encoding is special.
-ENCODING("i16imm",ENCODING_IW)
+ENCODING("i16imm",   ENCODING_IW)
   }
-  ENCODING("i16imm",  ENCODING_Iv)
-  ENCODING("i16i8imm",ENCODING_IB)
-  ENCODING("i32imm",  ENCODING_Iv)
-  ENCODING("i32i8imm",ENCODING_IB)
-  ENCODING("i64i32imm",   ENCODING_ID)
-  ENCODING("i64i8imm",ENCODING_IB)
-  ENCODING("i8imm",   ENCODING_IB)
-  ENCODING("u8imm",   ENCODING_IB)
-  ENCODING("i16u8imm",ENCODING_IB)
-  ENCODING("i32u8imm",ENCODING_IB)
-  ENCODING("i64u8imm",ENCODING_IB)
-  ENCODING("i64i32imm_pcrel", ENCODING_ID)
-  ENCODING("i16imm_pcrel",ENCODING_IW)
-  ENCODING("i32imm_pcrel",ENCODING_ID)
-  ENCODING("brtarget32",  ENCODING_ID)
-  ENCODING("brtarget16",  ENCODING_IW)
-  ENCODING("brtarget8",   ENCODING_IB)
-  ENCODING("i64imm",  ENCODING_IO)
-  ENCODING("offset16_8",  ENCODING_Ia)
-  ENCODING("offset16_16", ENCODING_Ia)
-  ENCODING("offset16_32", ENCODING_Ia)
-  ENCODING("offset32_8",  ENCODING_Ia)
-  ENCODING("offset32_16", ENCODING_Ia)
-  ENCODING("offset32_32", ENCODING_Ia)
-  ENCODING("offset32_64", ENCODING_Ia)
-  ENCODING("offset64_8",  ENCODING_Ia)
-  ENCODING("offset64_16", ENCODING_Ia)
-  ENCODING("offset64_32", ENCODING_Ia)
-  ENCODING("offset64_64", ENCODING_Ia)
-  ENCODING("srcidx8", ENCODING_SI)
-  ENCODING("srcidx16",ENCODING_SI)
-  ENCODING("srcidx32",ENCODING_SI)
-  ENCODING("srcidx64",ENCODING_SI)
-  ENCODING("dstidx8", ENCODING_DI)
-  ENCODING("dstidx16",ENCODING_DI)
-  ENCODING("dstidx32",ENCODING_DI)
-  ENCODING("dstidx64",ENCODING_DI)
+  ENCODING("i16imm", ENCODING_Iv)
+  ENCODING("i16i8imm",   ENCODING_IB)
+  ENCODING("i32imm", ENCODING_Iv)
+  ENCODING("i32i8imm",   ENCODING_IB)
+  ENCODING("i64i32imm",  ENCODING_ID)
+  ENCODING("i64i8imm",   ENCODING_IB)
+  ENCODING("i8imm",  ENCODING_IB)
+  ENCODING("u8imm",  ENCODING_IB)
+  ENCODING("i16u8imm",   ENCODING_IB)
+  ENCODING("i32u8imm",   ENCODING_IB)
+  ENCODING("i64u8imm",   ENCODING_IB)
+  ENCODING("i64i32imm_brtarget", ENCODING_ID)
+  ENCODING("i16imm_brtarget",ENCODING_IW)
+  ENCODING("i32imm_brtarget",ENCODING_ID)
+  ENCODING("brtarget32", ENCODING_ID)
+  ENCODING("brtarget16", ENCODING_IW)
+  ENCODING("brtarget8",  ENCODING_IB)
+  ENCODING("i64imm", ENCODING_IO)
+  ENCODING("offset16_8", ENCODING_Ia)
+  ENCODING("offset16_16",ENCODING_Ia)
+  ENCODING("offset16_32",ENCODING_Ia)
+  ENCODING("offset32_8", ENCODING_Ia)
+  ENCODING("offset32_16",ENCODING_Ia)
+  ENCODING("offset32_32",ENCODING_Ia)
+  ENCODING("offset32_64",ENCODING_Ia)
+  ENCODING("offset64_8", ENCODING_Ia)
+  ENCODING("offset64_16",ENCODING_Ia)
+  ENCODING("offset64_32",ENCODING_Ia)
+  ENCODING("offset64_64",ENCODING_Ia)
+  ENCODING("srcidx8",ENCODING_SI)
+  ENCODING("srcidx16",   ENCODING_SI)
+  ENCODING("srcidx32",   ENCODING_SI)
+  ENCODING("srcidx64",   ENCODING_SI)
+  ENCODING("dstidx8",ENC

[PATCH] D90441: [X86] Add support for vex, vex2, vex3, and evex for MASM

2020-11-04 Thread Eric Astor via Phabricator via cfe-commits
epastor added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2851
+// Parse MASM style pseudo prefixes.
+// FIXME: This prefix should only be used for MASM, not for intel-syntax.
+if (isParsingIntelSyntax()) {

LiuChen3 wrote:
> I tried to limit to MASM. But I found that the  'isParsingMSInlineAsm()' is 
> not accurate.  And then I tried to transmit 'ParsingMSInlineAsm' information 
> correctly in AsmPrinterInlineAsm.cpp (according to the '-fasm-blocks' 
> option). But I was surprised to find that isParsingMSInlineAsm() is actually 
> used as the argument of 'MatchingInlineAsm' in 'MatchAndEmitInstruction()'. 
> This makes me confused. Should that 'MatchingInlineAsm' be 
> 'MatchingMSInlineAsm' ?Is this MatchingInlineAsm only used by llvm-ml.
> It difficult to limit this to MASM at the moment. 
llvm-ml attempts not to touch **anything** involving inline assembly so far. 
The signal that MasmParser.cpp is involved is `Parser.isParsingMasm()`. 
However... while I can't answer the majority of this without more research, I 
suspect you're correct that `MatchingInlineAsm` is misnamed. We need to check 
this, and if so, we should rename it to avoid confusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90441/new/

https://reviews.llvm.org/D90441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

2020-11-17 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

LGTM for llvm-ml.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91410/new/

https://reviews.llvm.org/D91410

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D90441: [X86] Add support for vex, vex2, vex3, and evex for MASM

2020-10-30 Thread Eric Astor via Phabricator via cfe-commits
epastor added inline comments.



Comment at: clang/test/CodeGen/X86/ms-inline-asm-prefix.c:1
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-widows-msvc 
-target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl 
-fasm-blocks -mllvm -x86-asm-syntax=intel -S -o -  | FileCheck %s -check-prefix 
CHECK
+

pengfei wrote:
> pengfei wrote:
> > pengfei wrote:
> > > Maybe need `// REQUIRES: x86-registered-target`
> > You may need add att check too since you modified the att code.
> Should it be avalible only when `-fms-compatibility`
The triple is misspelled; it should be `x86_64-pc-windows-msvc` (the "n" in 
windows is missing)



Comment at: clang/test/CodeGen/X86/ms-inline-asm-prefix.c:1
+// RUN:%clang_cc1 %s -ferror-limit 0 -triple=x86_64-pc-widows-msvc 
-target-feature +avx512f -target-feature +avx2 -target-feature +avx512vl 
-fasm-blocks -mllvm -x86-asm-syntax=intel -S -o -  | FileCheck %s -check-prefix 
CHECK
+

epastor wrote:
> pengfei wrote:
> > pengfei wrote:
> > > pengfei wrote:
> > > > Maybe need `// REQUIRES: x86-registered-target`
> > > You may need add att check too since you modified the att code.
> > Should it be avalible only when `-fms-compatibility`
> The triple is misspelled; it should be `x86_64-pc-windows-msvc` (the "n" in 
> windows is missing)
A broader question: As written, this applies to anything in Intel syntax. Is 
this an Intel syntax feature, or a MASM feature?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90441/new/

https://reviews.llvm.org/D90441

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D75198: [ms] Rename ParsingInlineAsm functions/variables to reflect MS-specificity.

2020-02-26 Thread Eric Astor via Phabricator via cfe-commits
epastor created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
epastor added a child revision: D73227: [ms] [llvm-ml] Use default RIP-relative 
addressing for x64 MASM..

ParsingInlineAsm was a misleading name. These values are only set for MS-style 
inline assembly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75198

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  llvm/include/llvm/MC/MCParser/MCAsmParser.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -611,9 +611,9 @@
 }
 bool onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName,
   const InlineAsmIdentifierInfo &IDInfo,
-  bool ParsingInlineAsm, StringRef &ErrMsg) {
+  bool ParsingMSInlineAsm, StringRef &ErrMsg) {
   // InlineAsm: Treat an enum value as an integer
-  if (ParsingInlineAsm)
+  if (ParsingMSInlineAsm)
 if (IDInfo.isKind(InlineAsmIdentifierInfo::IK_EnumVal))
   return onInteger(IDInfo.Enum.EnumVal, ErrMsg);
   // Treat a symbolic constant like an integer
@@ -634,7 +634,7 @@
 MemExpr = true;
 State = IES_INTEGER;
 IC.pushOperand(IC_IMM);
-if (ParsingInlineAsm)
+if (ParsingMSInlineAsm)
   Info = IDInfo;
 break;
   }
@@ -815,7 +815,7 @@
   }
 }
 bool onOffset(const MCExpr *Val, SMLoc OffsetLoc, StringRef ID,
-  const InlineAsmIdentifierInfo &IDInfo, bool ParsingInlineAsm,
+  const InlineAsmIdentifierInfo &IDInfo, bool ParsingMSInlineAsm,
   StringRef &ErrMsg) {
   PrevState = State;
   switch (State) {
@@ -833,7 +833,7 @@
 // As we cannot yet resolve the actual value (offset), we retain
 // the requested semantics by pushing a '0' to the operands stack
 IC.pushOperand(IC_IMM);
-if (ParsingInlineAsm) {
+if (ParsingMSInlineAsm) {
   Info = IDInfo;
 }
 break;
@@ -899,10 +899,10 @@
 
   bool ParseIntelMemoryOperandSize(unsigned &Size);
   std::unique_ptr
-  CreateMemForInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
-unsigned IndexReg, unsigned Scale, SMLoc Start,
-SMLoc End, unsigned Size, StringRef Identifier,
-const InlineAsmIdentifierInfo &Info);
+  CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
+  unsigned IndexReg, unsigned Scale, SMLoc Start,
+  SMLoc End, unsigned Size, StringRef Identifier,
+  const InlineAsmIdentifierInfo &Info);
 
   bool parseDirectiveEven(SMLoc L);
   bool ParseDirectiveCode(StringRef IDVal, SMLoc L);
@@ -1177,7 +1177,7 @@
 
   // The "flags" and "mxcsr" registers cannot be referenced directly.
   // Treat it as an identifier instead.
-  if (isParsingInlineAsm() && isParsingIntelSyntax() &&
+  if (isParsingMSInlineAsm() && isParsingIntelSyntax() &&
   (RegNo == X86::EFLAGS || RegNo == X86::MXCSR))
 RegNo = 0;
 
@@ -1458,7 +1458,7 @@
   return ParseATTOperand();
 }
 
-std::unique_ptr X86AsmParser::CreateMemForInlineAsm(
+std::unique_ptr X86AsmParser::CreateMemForMSInlineAsm(
 unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
 unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
 const InlineAsmIdentifierInfo &Info) {
@@ -1536,7 +1536,7 @@
   return true;
 StringRef ErrMsg;
 ParseError =
-SM.onOffset(Val, OffsetLoc, ID, Info, isParsingInlineAsm(), ErrMsg);
+SM.onOffset(Val, OffsetLoc, ID, Info, isParsingMSInlineAsm(), ErrMsg);
 if (ParseError)
   return Error(SMLoc::getFromPointer(Name.data()), ErrMsg);
   } else {
@@ -1595,7 +1595,7 @@
   // Symbol reference, when parsing assembly content
   InlineAsmIdentifierInfo Info;
   const MCExpr *Val;
-  if (!isParsingInlineAsm()) {
+  if (!isParsingMSInlineAsm()) {
 if (getParser().parsePrimaryExpr(Val, End)) {
   return Error(Tok.getLoc(), "Unexpected identifier!");
 } else if (SM.onIdentifierExpr(Val, Identifier, Info, false, ErrMsg)) {
@@ -1646,8 +1646,8 @@
 return Error(Loc, "invalid reference to undefined symbol");
   StringRef Identifier = Sym->getName();
   InlineAsmIdentifierInfo Info;
-  if (SM.onIdentifierExpr(Val, Identifier, Info,
-  isParsingInlineAsm(), ErrMsg))
+  if (SM.onIdentifierExpr(Val, Identifier, Info, isParsingMSInlineAsm()

[PATCH] D75198: [ms] Rename ParsingInlineAsm functions/variables to reflect MS-specificity.

2020-02-26 Thread Eric Astor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85b641c27aec: [ms] Rename ParsingInlineAsm 
functions/variables to reflect MS-specificity. (authored by epastor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75198/new/

https://reviews.llvm.org/D75198

Files:
  clang/lib/Parse/ParseStmtAsm.cpp
  llvm/include/llvm/MC/MCParser/MCAsmParser.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -611,9 +611,9 @@
 }
 bool onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName,
   const InlineAsmIdentifierInfo &IDInfo,
-  bool ParsingInlineAsm, StringRef &ErrMsg) {
+  bool ParsingMSInlineAsm, StringRef &ErrMsg) {
   // InlineAsm: Treat an enum value as an integer
-  if (ParsingInlineAsm)
+  if (ParsingMSInlineAsm)
 if (IDInfo.isKind(InlineAsmIdentifierInfo::IK_EnumVal))
   return onInteger(IDInfo.Enum.EnumVal, ErrMsg);
   // Treat a symbolic constant like an integer
@@ -634,7 +634,7 @@
 MemExpr = true;
 State = IES_INTEGER;
 IC.pushOperand(IC_IMM);
-if (ParsingInlineAsm)
+if (ParsingMSInlineAsm)
   Info = IDInfo;
 break;
   }
@@ -815,7 +815,7 @@
   }
 }
 bool onOffset(const MCExpr *Val, SMLoc OffsetLoc, StringRef ID,
-  const InlineAsmIdentifierInfo &IDInfo, bool ParsingInlineAsm,
+  const InlineAsmIdentifierInfo &IDInfo, bool ParsingMSInlineAsm,
   StringRef &ErrMsg) {
   PrevState = State;
   switch (State) {
@@ -833,7 +833,7 @@
 // As we cannot yet resolve the actual value (offset), we retain
 // the requested semantics by pushing a '0' to the operands stack
 IC.pushOperand(IC_IMM);
-if (ParsingInlineAsm) {
+if (ParsingMSInlineAsm) {
   Info = IDInfo;
 }
 break;
@@ -899,10 +899,10 @@
 
   bool ParseIntelMemoryOperandSize(unsigned &Size);
   std::unique_ptr
-  CreateMemForInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
-unsigned IndexReg, unsigned Scale, SMLoc Start,
-SMLoc End, unsigned Size, StringRef Identifier,
-const InlineAsmIdentifierInfo &Info);
+  CreateMemForMSInlineAsm(unsigned SegReg, const MCExpr *Disp, unsigned BaseReg,
+  unsigned IndexReg, unsigned Scale, SMLoc Start,
+  SMLoc End, unsigned Size, StringRef Identifier,
+  const InlineAsmIdentifierInfo &Info);
 
   bool parseDirectiveEven(SMLoc L);
   bool ParseDirectiveCode(StringRef IDVal, SMLoc L);
@@ -1177,7 +1177,7 @@
 
   // The "flags" and "mxcsr" registers cannot be referenced directly.
   // Treat it as an identifier instead.
-  if (isParsingInlineAsm() && isParsingIntelSyntax() &&
+  if (isParsingMSInlineAsm() && isParsingIntelSyntax() &&
   (RegNo == X86::EFLAGS || RegNo == X86::MXCSR))
 RegNo = 0;
 
@@ -1458,7 +1458,7 @@
   return ParseATTOperand();
 }
 
-std::unique_ptr X86AsmParser::CreateMemForInlineAsm(
+std::unique_ptr X86AsmParser::CreateMemForMSInlineAsm(
 unsigned SegReg, const MCExpr *Disp, unsigned BaseReg, unsigned IndexReg,
 unsigned Scale, SMLoc Start, SMLoc End, unsigned Size, StringRef Identifier,
 const InlineAsmIdentifierInfo &Info) {
@@ -1536,7 +1536,7 @@
   return true;
 StringRef ErrMsg;
 ParseError =
-SM.onOffset(Val, OffsetLoc, ID, Info, isParsingInlineAsm(), ErrMsg);
+SM.onOffset(Val, OffsetLoc, ID, Info, isParsingMSInlineAsm(), ErrMsg);
 if (ParseError)
   return Error(SMLoc::getFromPointer(Name.data()), ErrMsg);
   } else {
@@ -1595,7 +1595,7 @@
   // Symbol reference, when parsing assembly content
   InlineAsmIdentifierInfo Info;
   const MCExpr *Val;
-  if (!isParsingInlineAsm()) {
+  if (!isParsingMSInlineAsm()) {
 if (getParser().parsePrimaryExpr(Val, End)) {
   return Error(Tok.getLoc(), "Unexpected identifier!");
 } else if (SM.onIdentifierExpr(Val, Identifier, Info, false, ErrMsg)) {
@@ -1646,8 +1646,8 @@
 return Error(Loc, "invalid reference to undefined symbol");
   StringRef Identifier = Sym->getName();
   InlineAsmIdentifierInfo Info;
-  if (SM.onIdentifierExpr(Val, Identifier, Info,
-  isParsingInlineAsm(), ErrMsg))
+  if (SM.onIdentifierExpr(Val, Identifier, Info, isParsingMSInlineAsm(),
+  ErrMsg))
 retur

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

2019-10-30 Thread Eric Astor via Phabricator via cfe-commits
epastor created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
epastor added reviewers: rnk, thakis.

In Microsoft-compatibility mode, single commas from nested macro expansions
should not be considered as argument separators; we emulate this by marking
them to be ignored. However, in MSVC's preprocessor, subsequent expansions
DO treat these commas as argument separators... so we now ignore each comma
at most once.

Includes a small unit test that validates we match MSVC's behavior as shown in 
https://gcc.godbolt.org/z/y0twaq


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69626

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/microsoft-ext.c


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -820,18 +820,26 @@
 }
   } else if (Tok.is(tok::l_paren)) {
 ++NumParens;
-  } else if (Tok.is(tok::comma) && NumParens == 0 &&
- !(Tok.getFlags() & Token::IgnoredComma)) {
+  } else if (Tok.is(tok::comma)) {
 // In Microsoft-compatibility mode, single commas from nested macro
 // expansions should not be considered as argument separators. We test
-// for this with the IgnoredComma token flag above.
-
-// Comma ends this argument if there are more fixed arguments expected.
-// However, if this is a variadic macro, and this is part of the
-// variadic part, then the comma is just an argument token.
-if (!isVariadic) break;
-if (NumFixedArgsLeft > 1)
-  break;
+// for this with the IgnoredComma token flag.
+if (Tok.getFlags() & Token::IgnoredComma) {
+  // However, in MSVC's preprocessor, subsequent expansions do treat
+  // these commas as argument separators. This leads to a common
+  // workaround used in macros that need to work in both MSVC and
+  // compliant preprocessors. Therefore, the IgnoredComma flag can only
+  // apply once to any given token.
+  Tok.clearFlag(Token::IgnoredComma);
+} else if (NumParens == 0) {
+  // Comma ends this argument if there are more fixed arguments
+  // expected. However, if this is a variadic macro, and this is part 
of
+  // the variadic part, then the comma is just an argument token.
+  if (!isVariadic)
+break;
+  if (NumFixedArgsLeft > 1)
+break;
+}
   } else if (Tok.is(tok::comment) && !KeepMacroComments) {
 // If this is a comment token in the argument list and we're just in
 // -C mode (not -CC mode), discard the comment.


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPM

[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

2019-10-30 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

In D69626#1728113 , @thakis wrote:

> Is this needed to parse system headers?


Interesting question. This change isn't - but I presume the (pre-existing) 
introduction of IgnoredComma was.

> In general, we try to emulate only warts we must emulate, and if feasible we 
> try to emit some -Wmicrosoft warning when the compat path is taken.

Sure - this change actually brings the preprocessor closer to compliance. I 
think the prior use of IgnoredComma had overshot MSVC by a bit. For an 
illustration, see
https://gcc.godbolt.org/z/XakAYf (same code as above, but in MSVC, clang, and 
clang -fms-compatibility)
MSVC shows it's non-compliant on TEST, and -fms-compatibility matches it there. 
However, MSVC matches clang's behavior for WRAPPED_TEST, as adding another 
layer of macro expansion is a standard trick for forcing MSVC's preprocessor to 
act compliant... but clang -fms-compatibility still gives the non-compliant 
result, since that trick doesn't work to remove IgnoredCommas.

If we wanted to detect use of the compatibility feature, we might need to apply 
the preprocessor both ways and see if the results were different; I'm not sure 
all IgnoredCommas will actually make a difference in the parsing. As is, we'd 
need to emit a -Wmicrosoft warning any time we saw an isolated comma as a macro 
parameter... which I think might get confusing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69626/new/

https://reviews.llvm.org/D69626



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

2019-10-31 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

Thanks, Reid; I'm not 100% sure I've checked all the corner cases either, but 
this at least seems like a step forward.

As a reminder: I don't have commit access. Could someone else commit this for 
me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69626/new/

https://reviews.llvm.org/D69626



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69626: Fix Microsoft compatibility handling of commas in nested macro expansions.

2019-11-04 Thread Eric Astor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe6ac471f613: [ms] Fix Microsoft compatibility handling of 
commas in nested macro expansions. (authored by epastor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69626/new/

https://reviews.llvm.org/D69626

Files:
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/microsoft-ext.c


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -820,18 +820,26 @@
 }
   } else if (Tok.is(tok::l_paren)) {
 ++NumParens;
-  } else if (Tok.is(tok::comma) && NumParens == 0 &&
- !(Tok.getFlags() & Token::IgnoredComma)) {
+  } else if (Tok.is(tok::comma)) {
 // In Microsoft-compatibility mode, single commas from nested macro
 // expansions should not be considered as argument separators. We test
-// for this with the IgnoredComma token flag above.
-
-// Comma ends this argument if there are more fixed arguments expected.
-// However, if this is a variadic macro, and this is part of the
-// variadic part, then the comma is just an argument token.
-if (!isVariadic) break;
-if (NumFixedArgsLeft > 1)
-  break;
+// for this with the IgnoredComma token flag.
+if (Tok.getFlags() & Token::IgnoredComma) {
+  // However, in MSVC's preprocessor, subsequent expansions do treat
+  // these commas as argument separators. This leads to a common
+  // workaround used in macros that need to work in both MSVC and
+  // compliant preprocessors. Therefore, the IgnoredComma flag can only
+  // apply once to any given token.
+  Tok.clearFlag(Token::IgnoredComma);
+} else if (NumParens == 0) {
+  // Comma ends this argument if there are more fixed arguments
+  // expected. However, if this is a variadic macro, and this is part 
of
+  // the variadic part, then the comma is just an argument token.
+  if (!isVariadic)
+break;
+  if (NumFixedArgsLeft > 1)
+break;
+}
   } else if (Tok.is(tok::comment) && !KeepMacroComments) {
 // If this is a comment token in the argument list and we're just in
 // -C mode (not -CC mode), discard the comment.


Index: clang/test/Preprocessor/microsoft-ext.c
===
--- clang/test/Preprocessor/microsoft-ext.c
+++ clang/test/Preprocessor/microsoft-ext.c
@@ -23,6 +23,19 @@
 HAS_1_TEMPLATE_PARAMS(int, k),
 AND_2_VALUE_PARAMS(p0, p1));
 
+// Regression test for PR43282; check that we match MSVC's failure to unpack
+// __VA_ARGS__ unless forwarded through another macro.
+#define THIRD_ARGUMENT(A, B, C, ...) C
+#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, 1, 2)
+#define COMBINE(...) __VA_ARGS__
+#define WRAPPED_TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, 1, 2))
+// Check that we match MSVC's failure to unpack __VA_ARGS__, unless forwarded
+// through another macro
+auto packed = TEST(,);
+auto unpacked = WRAPPED_TEST(,);
+// CHECK: auto packed = 2;
+// CHECK: auto unpacked = 1;
+
 // This tests compatibility with behaviour needed for type_traits in VS2012
 // Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
 #define _COMMA ,
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -820,18 +820,26 @@
 }
   } else if (Tok.is(tok::l_paren)) {
 ++NumParens;
-  } else if (Tok.is(tok::comma) && NumParens == 0 &&
- !(Tok.getFlags() & Token::IgnoredComma)) {
+  } else if (Tok.i

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-13 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 233786.
epastor added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Fix tests expecting use of 'r' constraints for offset operands


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator {
@@ -326,6 +325,7 @@
 IES_RSHIFT,
 IES_PLUS,
 IES_MINUS,
+IES_OFFSET,
 IES_NOT,
 IES_MULTIPLY,
 IES_DIVIDE,
@@ -350,16 +350,30 @@
 InlineAsmIdentifierInfo Info;
 short BracCount;
 bool MemExpr;
+bool OffsetOperator;
+SMLoc OffsetOperatorLoc;
+
+bool setSymRef(const MCExpr *Val, StringRef ID, StringRef &ErrMsg) {
+  if (Sym) {
+ErrMsg = "cannot use more than one symbol in memory operand";
+return true;
+  }
+  Sym = Val;
+  SymName = ID;
+  return false;
+}
 
   public:
   

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-13 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 233806.
epastor added a comment.

- Fix Intel named operator parsing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator {
@@ -326,6 +325,7 @@
 IES_RSHIFT,
 IES_PLUS,
 IES_MINUS,
+IES_OFFSET,
 IES_NOT,
 IES_MULTIPLY,
 IES_DIVIDE,
@@ -350,16 +350,30 @@
 InlineAsmIdentifierInfo Info;
 short BracCount;
 bool MemExpr;
+bool OffsetOperator;
+SMLoc OffsetOperatorLoc;
+
+bool setSymRef(const MCExpr *Val, StringRef ID, StringRef &ErrMsg) {
+  if (Sym) {
+ErrMsg = "cannot use more than one symbol in memory operand";
+return true;
+  }
+  Sym = Val;
+  SymName = ID;
+  return false;
+}
 
   public:
 IntelExprStateMachine()
 : State(IES_INIT), PrevState(IES_ERROR), BaseReg(0), IndexReg(0),

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-13 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 233813.
epastor added a comment.

- Respect correct usage of offset.
- Don't assume implicit sizing on offset.
- Check that offset works in compound expressions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,15 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default: llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,8 +279,8 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
+  bool isOffsetOfLocal() const override {
+return isImm() && Imm.LocalRef;
   }
 
   bool needAddressOf() const override {
@@ -613,9 +614,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 @@
 IOK_LENGTH,
 IOK_SIZE,
 IOK_TYPE,
-IOK_OFFSET
   };
 
   class InfixCalculator 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-16 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 234067.
epastor added a comment.

- Apply formatting fixes where reasonable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,13 +279,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
@@ -613,9 +610,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -134,7 +134,6 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-17 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 234371.
epastor marked 6 inline comments as done.
epastor added a comment.

- Address refactoring comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,13 +279,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
@@ -613,9 +610,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ 

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-17 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

Thanks for feedback!




Comment at: clang/test/CodeGen/ms-inline-asm-64.c:18
 // CHECK: call void asm sideeffect inteldialect
-// CHECK-SAME: mov [eax], $0
+// CHECK-SAME: mov qword ptr [eax], $0
 // CHECK-SAME: "r,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}})

rnk wrote:
> I guess this has the same semantics as it did before.
It does; this previously resolved to a "qword ptr" mov automatically, since $0 
unpacked as a (forced) 64-bit register name. Now we need to specify the size of 
the move, since the operand is an immediate rather than a register.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+getParser().parsePrimaryExpr(Val, End))
+  return Error(Start, "unexpected token!");
+  } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {

rnk wrote:
> Please test this corner case, I imagine it looks like:
>   mov eax, offset 3
Interesting. This corner case didn't trigger in that scenario; we get an 
"expected identifier" error message with good source location, followed by 
another error "use of undeclared label '3'" in debug builds... and in release 
builds, we instead get a crash. On tracing the crash, it's a AsmStrRewrite 
applying to a SMLoc not coming from the same string...

As near as I can tell, the issue is that we end up trying to parse "3" as a 
not-yet-declared label, as such expand it to `__MSASMLABEL_.${:uid}__3`, and 
then end up in a bad state because the operand rewrite is applying to the 
expanded symbol... which isn't in the same AsmString. If you use an actual 
undeclared label, you hit the same crash in release builds.

This is going to take some work; I'll get back to it in a day or two.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+  } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+return Error(Start, "offset operator cannot yet handle constants");
+  }

rnk wrote:
> Please add a test for this corner case, I'm curious to see if the error 
> reporting really works.
This error reporting doesn't work, in fact. We instead get "cannot take the 
address of an rvalue of type 'int'", with bad source location. Will investigate 
why we end up there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-18 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 234567.
epastor added a comment.

- Removing accidental artifacts of local testing...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,13 +279,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
@@ -613,9 +610,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+Res->AddressOf= true;
 return Res;
   }
 
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llv

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-18 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 234566.
epastor marked 2 inline comments as done.
epastor added a comment.

Fix issues around enum values and labels

- Only rewrite variables as offset operands; fixes crash due to conflicting 
rewrites for labels
- Recognize inline assembly references to enum values


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436

Files:
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/ms-inline-asm-64.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  clang/test/Parser/ms-inline-asm.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/test/CodeGen/X86/ms-inline-asm.ll
  llvm/test/CodeGen/X86/offset-operator.ll
  llvm/test/MC/X86/pr32530.s

Index: llvm/test/MC/X86/pr32530.s
===
--- /dev/null
+++ llvm/test/MC/X86/pr32530.s
@@ -0,0 +1,13 @@
+// RUN: llvm-mc -triple x86_64-unknown-unknown -x86-asm-syntax=intel %s | FileCheck %s
+
+.text
+// CHECK: movq$msg, %rsi
+// CHECK: movq$msg+314159, %rax
+// CHECK: movq$msg-89793, msg-6535(%rax,%rbx,2)
+  mov rsi,  offset msg
+	mov rax,  offset "msg" + 314159
+	mov qword ptr [rax + 2*rbx + offset msg - 6535],  offset msg - 89793
+.data
+msg:
+  .ascii "Hello, world!\n"
+
Index: llvm/test/CodeGen/X86/offset-operator.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/offset-operator.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -x86-asm-syntax=intel -relocation-model=static < %s | FileCheck %s
+
+; Test we are emitting the 'offset' operator upon an immediate reference of a label:
+; The emitted 'att-equivalent' of this one is "movl $.L.str, %eax"
+
+@.str = private unnamed_addr constant [1 x i8] zeroinitializer, align 1
+
+define i8* @test_offset_operator() {
+; CHECK-LABEL: test_offset_operator:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mov eax, offset .L.str
+; CHECK-NEXT:ret
+entry:
+  ret i8* getelementptr inbounds ([1 x i8], [1 x i8]* @.str, i64 0, i64 0)
+}
Index: llvm/test/CodeGen/X86/ms-inline-asm.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm.ll
@@ -92,7 +92,7 @@
 ; CHECK-LABEL: t30:
 ; CHECK: {{## InlineAsm Start|#APP}}
 ; CHECK: .intel_syntax
-; CHECK: lea edi, dword ptr [{{_?}}results]
+; CHECK: lea edi, dword ptr [offset {{_?}}results]
 ; CHECK: .att_syntax
 ; CHECK: {{## InlineAsm End|#NO_APP}}
 ; CHECK: {{## InlineAsm Start|#APP}}
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -220,8 +220,16 @@
 
   case MachineOperand::MO_ConstantPoolIndex:
   case MachineOperand::MO_GlobalAddress: {
-if (IsATT)
+switch (MI->getInlineAsmDialect()) {
+default:
+  llvm_unreachable("unknown assembly dialect!");
+case InlineAsm::AD_ATT:
   O << '$';
+  break;
+case InlineAsm::AD_Intel:
+  O << "offset ";
+  break;
+}
 PrintSymbolOperand(MO, O);
 break;
   }
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -52,6 +52,7 @@
 
   struct ImmOp {
 const MCExpr *Val;
+bool LocalRef;
   };
 
   struct MemOp {
@@ -278,13 +279,9 @@
 return isImmUnsignedi8Value(CE->getValue());
   }
 
-  bool isOffsetOf() const override {
-return OffsetOfLoc.getPointer();
-  }
+  bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
-  bool needAddressOf() const override {
-return AddressOf;
-  }
+  bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
   bool isMemUnsized() const {
@@ -613,9 +610,16 @@
   }
 
   static std::unique_ptr CreateImm(const MCExpr *Val,
-   SMLoc StartLoc, SMLoc EndLoc) {
+   SMLoc StartLoc, SMLoc EndLoc,
+   StringRef SymName = StringRef(),
+   void *OpDecl = nullptr,
+   bool GlobalRef = true) {
 auto Res = std::make_unique(Immediate, StartLoc, EndLoc);
-Res->Imm.Val = Val;
+Res->Imm.Val  = Val;
+Res->Imm.LocalRef = !GlobalRef;
+Res->SymName  = SymName;
+Res->OpDecl   = OpDecl;
+

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-18 Thread Eric Astor via Phabricator via cfe-commits
epastor added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821
+getParser().parsePrimaryExpr(Val, End))
+  return Error(Start, "unexpected token!");
+  } else if (ParseIntelInlineAsmIdentifier(Val, ID, Info, false, End, true)) {

epastor wrote:
> rnk wrote:
> > Please test this corner case, I imagine it looks like:
> >   mov eax, offset 3
> Interesting. This corner case didn't trigger in that scenario; we get an 
> "expected identifier" error message with good source location, followed by 
> another error "use of undeclared label '3'" in debug builds... and in release 
> builds, we instead get a crash. On tracing the crash, it's a AsmStrRewrite 
> applying to a SMLoc not coming from the same string...
> 
> As near as I can tell, the issue is that we end up trying to parse "3" as a 
> not-yet-declared label, as such expand it to `__MSASMLABEL_.${:uid}__3`, and 
> then end up in a bad state because the operand rewrite is applying to the 
> expanded symbol... which isn't in the same AsmString. If you use an actual 
> undeclared label, you hit the same crash in release builds.
> 
> This is going to take some work; I'll get back to it in a day or two.
Fixed; we now get the same errors in this scenario as we do in the current LLVM 
trunk, reporting "expected identifier" and "use of undeclared label '3'".

On the other hand, I'm still working on finding a scenario that DOES trigger 
this corner case.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1825
+  } else if (Info.isKind(InlineAsmIdentifierInfo::IK_EnumVal)) {
+return Error(Start, "offset operator cannot yet handle constants");
+  }

epastor wrote:
> rnk wrote:
> > Please add a test for this corner case, I'm curious to see if the error 
> > reporting really works.
> This error reporting doesn't work, in fact. We instead get "cannot take the 
> address of an rvalue of type 'int'", with bad source location. Will 
> investigate why we end up there.
Turns out Sema wasn't successfully recognizing inline-assembly references to 
enum values, so we were processing them through as labels, and only recognizing 
the problem late in processing. Fixed now, with accurate error reporting; test 
added.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71436/new/

https://reviews.llvm.org/D71436



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71677: Summary:Use "P" modifier on operands to call instructions in inline X86 assembly.

2019-12-18 Thread Eric Astor via Phabricator via cfe-commits
epastor created this revision.
epastor added a reviewer: rnk.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

This is documented as the appropriate template modifier for call operands.
Fixes PR44272, and adds a regression test.

Also adds support for operand modifiers in Intel-style inline assembly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71677

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/mozilla-ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll

Index: llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define void @func() {
+entry:
+  ret void
+}
+
+define void @main() {
+entry:
+  call void asm sideeffect inteldialect "call ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @func)
+  ret void
+; CHECK-LABEL: main:
+; CHECK: {{## InlineAsm Start|#APP}}
+; CHECK: call{{l|q}} func
+; CHECK: {{## InlineAsm End|#NO_APP}}
+; CHECK: ret{{l|q}}
+}
\ No newline at end of file
Index: llvm/lib/Target/X86/X86AsmPrinter.h
===
--- llvm/lib/Target/X86/X86AsmPrinter.h
+++ llvm/lib/Target/X86/X86AsmPrinter.h
@@ -112,7 +112,7 @@
   void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O,
  const char *Modifier);
   void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo,
-  raw_ostream &O);
+  raw_ostream &O, const char *Modifier);
 
 public:
   X86AsmPrinter(TargetMachine &TM, std::unique_ptr Streamer);
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -337,14 +337,22 @@
   PrintLeaMemReference(MI, OpNo, O, Modifier);
 }
 
+
 void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
-   unsigned OpNo, raw_ostream &O) {
+   unsigned OpNo, raw_ostream &O,
+   const char *Modifier) {
   const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
   unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
   const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
   const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
   const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg);
 
+  // If we really don't want to print out (rip), don't.
+  bool HasBaseReg = BaseReg.getReg() != 0;
+  if (HasBaseReg && Modifier && !strcmp(Modifier, "no-rip") &&
+  BaseReg.getReg() == X86::RIP)
+HasBaseReg = false;
+
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
 PrintOperand(MI, OpNo + X86::AddrSegmentReg, O);
@@ -354,7 +362,7 @@
   O << '[';
 
   bool NeedPlus = false;
-  if (BaseReg.getReg()) {
+  if (HasBaseReg) {
 PrintOperand(MI, OpNo + X86::AddrBaseReg, O);
 NeedPlus = true;
   }
@@ -372,7 +380,7 @@
 PrintOperand(MI, OpNo + X86::AddrDisp, O);
   } else {
 int64_t DispVal = DispSpec.getImm();
-if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
+if (DispVal || (!IndexReg.getReg() && !HasBaseReg)) {
   if (NeedPlus) {
 if (DispVal > 0)
   O << " + ";
@@ -525,11 +533,6 @@
 bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
   const char *ExtraCode,
   raw_ostream &O) {
-  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
-PrintIntelMemReference(MI, OpNo, O);
-return false;
-  }
-
   if (ExtraCode && ExtraCode[0]) {
 if (ExtraCode[1] != 0) return true; // Unknown modifier.
 
@@ -543,14 +546,26 @@
   // These only apply to registers, ignore on mem.
   break;
 case 'H':
-  PrintMemReference(MI, OpNo, O, "H");
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+return true;  // Unsupported modifier in Intel inline assembly.
+  } else {
+PrintMemReference(MI, OpNo, O, "H");
+  }
   return false;
 case 'P': // Don't print @PLT, but do print as memory.
-  

[PATCH] D71677: [ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly.

2019-12-18 Thread Eric Astor via Phabricator via cfe-commits
epastor added a comment.

In D71677#1790458 , @rnk wrote:

> Where is {0:P} actually documented? I don't see it in LangRef, but I do see 
> it in the code.


https://llvm.org/docs/LangRef.html#asm-template-argument-modifiers, under X86; 
specifically documented to be used on the operands of `call` instructions.




Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:2871
+  // differently when referenced in MS-style inline assembly.
+  if (Name.startswith("call") || Name.startswith("lcall")) {
+for (size_t i = 1; i < Operands.size(); ++i) {

rnk wrote:
> I'm trying to think of other edge cases where we'd want the same treatment. 
> In theory, we'd want to do this for every control flow instruction that takes 
> a PC-relative operand, jmp, jcc, jecxz, that might be all. You know, it 
> actually seems reasonable to set up a naked function that contains an asm 
> blob which conditionally branches to another function, so I guess we should 
> support it. In that case, maybe this should be named something like 
> "isBranchTarget" instead of isCallTarget.
That would be a valid option, but currently the "P" modifier is documented for 
LLVM as to-be-used on the operands of `call` instructions. [[ 
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86Operandmodifiers | GNU 
as ]]) adds it more generally to be applied to functions - and occasionally 
constants, if you need a constant to avoid all syntax-specific prefixes.

We could use this for any branch target operand... but we'd need to restrict it 
to apply only to the specific PC-relative operand, which I think means 
augmenting the X86 target tables to annotate which operands of which 
instructions are PC-relative? Also, I'm not sure if that would break 
pre-existing code in enough cases to be worrying.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71677/new/

https://reviews.llvm.org/D71677



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71677: [ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly.

2019-12-21 Thread Eric Astor via Phabricator via cfe-commits
epastor marked 2 inline comments as done.
epastor added inline comments.



Comment at: llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll:15
+; CHECK: {{## InlineAsm Start|#APP}}
+; CHECK: call{{l|q}} func
+; CHECK: {{## InlineAsm End|#NO_APP}}

rnk wrote:
> I'd suggest matching for `{{call(l|q) func$}}` so that you don't accidentally 
> match `callq func(%rip)`. Which makes me wonder, does `@PLT` appear here? The 
> test uses no OS in the triple, which probably means ELF, so we probably use a 
> PLT.
Thanks for that catch - completely missed that it'd match with `(%rip)` on the 
end.

Interestingly... no, no `@PLT` in this test. Not sure what's up with that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71677/new/

https://reviews.llvm.org/D71677



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71677: [ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly.

2019-12-21 Thread Eric Astor via Phabricator via cfe-commits
epastor updated this revision to Diff 235035.
epastor marked an inline comment as done.
epastor added a comment.

Fix test; missing anchor meant the previous version would not catch a 
regression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71677/new/

https://reviews.llvm.org/D71677

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/mozilla-ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll

Index: llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define void @func() {
+entry:
+  ret void
+}
+
+define void @main() {
+entry:
+  call void asm sideeffect inteldialect "call ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @func)
+  ret void
+; CHECK-LABEL: main:
+; CHECK: {{## InlineAsm Start|#APP}}
+; CHECK: {{call(l|q) func$}}
+; CHECK: {{## InlineAsm End|#NO_APP}}
+; CHECK: ret{{l|q}}
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.h
===
--- llvm/lib/Target/X86/X86AsmPrinter.h
+++ llvm/lib/Target/X86/X86AsmPrinter.h
@@ -112,7 +112,7 @@
   void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O,
  const char *Modifier);
   void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo,
-  raw_ostream &O);
+  raw_ostream &O, const char *Modifier);
 
 public:
   X86AsmPrinter(TargetMachine &TM, std::unique_ptr Streamer);
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -337,14 +337,22 @@
   PrintLeaMemReference(MI, OpNo, O, Modifier);
 }
 
+
 void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
-   unsigned OpNo, raw_ostream &O) {
+   unsigned OpNo, raw_ostream &O,
+   const char *Modifier) {
   const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
   unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
   const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
   const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
   const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg);
 
+  // If we really don't want to print out (rip), don't.
+  bool HasBaseReg = BaseReg.getReg() != 0;
+  if (HasBaseReg && Modifier && !strcmp(Modifier, "no-rip") &&
+  BaseReg.getReg() == X86::RIP)
+HasBaseReg = false;
+
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
 PrintOperand(MI, OpNo + X86::AddrSegmentReg, O);
@@ -354,7 +362,7 @@
   O << '[';
 
   bool NeedPlus = false;
-  if (BaseReg.getReg()) {
+  if (HasBaseReg) {
 PrintOperand(MI, OpNo + X86::AddrBaseReg, O);
 NeedPlus = true;
   }
@@ -372,7 +380,7 @@
 PrintOperand(MI, OpNo + X86::AddrDisp, O);
   } else {
 int64_t DispVal = DispSpec.getImm();
-if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
+if (DispVal || (!IndexReg.getReg() && !HasBaseReg)) {
   if (NeedPlus) {
 if (DispVal > 0)
   O << " + ";
@@ -525,11 +533,6 @@
 bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
   const char *ExtraCode,
   raw_ostream &O) {
-  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
-PrintIntelMemReference(MI, OpNo, O);
-return false;
-  }
-
   if (ExtraCode && ExtraCode[0]) {
 if (ExtraCode[1] != 0) return true; // Unknown modifier.
 
@@ -543,14 +546,26 @@
   // These only apply to registers, ignore on mem.
   break;
 case 'H':
-  PrintMemReference(MI, OpNo, O, "H");
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+return true;  // Unsupported modifier in Intel inline assembly.
+  } else {
+PrintMemReference(MI, OpNo, O, "H");
+  }
   return false;
 case 'P': // Don't print @PLT, but do print as memory.
-  PrintMemReference(MI, OpNo, O, "no-rip");
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+Pr

[PATCH] D71677: [ms] [X86] Use "P" modifier on operands to call instructions in inline X86 assembly.

2019-12-22 Thread Eric Astor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc5b614fa9a1: [ms] [X86] Use "P" modifier on 
operands to call instructions in inline X86… (authored by epastor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71677/new/

https://reviews.llvm.org/D71677

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/mozilla-ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.c
  clang/test/CodeGen/ms-inline-asm.cpp
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll

Index: llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/ms-inline-asm-PR44272.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+define void @func() {
+entry:
+  ret void
+}
+
+define void @main() {
+entry:
+  call void asm sideeffect inteldialect "call ${0:P}", "*m,~{dirflag},~{fpsr},~{flags}"(void ()* @func)
+  ret void
+; CHECK-LABEL: main:
+; CHECK: {{## InlineAsm Start|#APP}}
+; CHECK: {{call(l|q) func$}}
+; CHECK: {{## InlineAsm End|#NO_APP}}
+; CHECK: ret{{l|q}}
+}
Index: llvm/lib/Target/X86/X86AsmPrinter.h
===
--- llvm/lib/Target/X86/X86AsmPrinter.h
+++ llvm/lib/Target/X86/X86AsmPrinter.h
@@ -112,7 +112,7 @@
   void PrintMemReference(const MachineInstr *MI, unsigned OpNo, raw_ostream &O,
  const char *Modifier);
   void PrintIntelMemReference(const MachineInstr *MI, unsigned OpNo,
-  raw_ostream &O);
+  raw_ostream &O, const char *Modifier);
 
 public:
   X86AsmPrinter(TargetMachine &TM, std::unique_ptr Streamer);
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -337,14 +337,22 @@
   PrintLeaMemReference(MI, OpNo, O, Modifier);
 }
 
+
 void X86AsmPrinter::PrintIntelMemReference(const MachineInstr *MI,
-   unsigned OpNo, raw_ostream &O) {
+   unsigned OpNo, raw_ostream &O,
+   const char *Modifier) {
   const MachineOperand &BaseReg = MI->getOperand(OpNo + X86::AddrBaseReg);
   unsigned ScaleVal = MI->getOperand(OpNo + X86::AddrScaleAmt).getImm();
   const MachineOperand &IndexReg = MI->getOperand(OpNo + X86::AddrIndexReg);
   const MachineOperand &DispSpec = MI->getOperand(OpNo + X86::AddrDisp);
   const MachineOperand &SegReg = MI->getOperand(OpNo + X86::AddrSegmentReg);
 
+  // If we really don't want to print out (rip), don't.
+  bool HasBaseReg = BaseReg.getReg() != 0;
+  if (HasBaseReg && Modifier && !strcmp(Modifier, "no-rip") &&
+  BaseReg.getReg() == X86::RIP)
+HasBaseReg = false;
+
   // If this has a segment register, print it.
   if (SegReg.getReg()) {
 PrintOperand(MI, OpNo + X86::AddrSegmentReg, O);
@@ -354,7 +362,7 @@
   O << '[';
 
   bool NeedPlus = false;
-  if (BaseReg.getReg()) {
+  if (HasBaseReg) {
 PrintOperand(MI, OpNo + X86::AddrBaseReg, O);
 NeedPlus = true;
   }
@@ -372,7 +380,7 @@
 PrintOperand(MI, OpNo + X86::AddrDisp, O);
   } else {
 int64_t DispVal = DispSpec.getImm();
-if (DispVal || (!IndexReg.getReg() && !BaseReg.getReg())) {
+if (DispVal || (!IndexReg.getReg() && !HasBaseReg)) {
   if (NeedPlus) {
 if (DispVal > 0)
   O << " + ";
@@ -525,11 +533,6 @@
 bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
   const char *ExtraCode,
   raw_ostream &O) {
-  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
-PrintIntelMemReference(MI, OpNo, O);
-return false;
-  }
-
   if (ExtraCode && ExtraCode[0]) {
 if (ExtraCode[1] != 0) return true; // Unknown modifier.
 
@@ -543,14 +546,26 @@
   // These only apply to registers, ignore on mem.
   break;
 case 'H':
-  PrintMemReference(MI, OpNo, O, "H");
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+return true;  // Unsupported modifier in Intel inline assembly.
+  } else {
+PrintMemReference(MI, OpNo, O, "H");
+  }
   return false;
 case 'P': // Don't print @PLT, but do print as memory.
-  PrintMemReference(MI, OpNo, O, "no-rip");
+  if (MI->getInlineAsmDialect() == InlineAsm::AD_Intel) {
+