[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] D71436: [X86][AsmParser] re-introduce 'offset' operator

2019-12-30 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61144 tests passed, 0 failed 
and 728 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



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 pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 



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 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 pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61144 tests passed, 0 failed 
and 728 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



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=

[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 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-28 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!




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:
> 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. :)


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 pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



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 pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 



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 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] 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
===
---

[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-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



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-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-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-16 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

Thanks! I made some comments. This code is convoluted. I don't think I can 
reason through all the edge cases, but this seems like an improvement. I'd 
suggest adding the recommended tests and any other corner case inputs you can 
think of, and after another round of review we can land it.




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* %{{.*}})

I guess this has the same semantics as it did before.



Comment at: llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h:72
+  : NeedBracs(false), Imm(0), BaseReg(StringRef()), IndexReg(StringRef()),
+OffsetName(StringRef()), Scale(1) {}
+  // [BaseReg + IndexReg * ScaleExpression + OFFSET name + ImmediateExpression]

These explicit empty StringRef initializations seem unnecessary, I'd suggest 
removing them.



Comment at: llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h:97
   SMLoc Loc;
   unsigned Len;
   int64_t Val;

nit, pack the bool after the unsigned for a smaller struct. Hey, we make a 
vector of these. :)



Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:5935-5940
+auto rewrite_it = it;
+while (++rewrite_it != AsmStrRewrites.end() &&
+   rewrite_it->Loc.getPointer() <= AR.IntelExp.OffsetName.data()) {
+  if (OffsetName.data() == rewrite_it->Loc.getPointer() &&
+  OffsetName.size() == rewrite_it->Len &&
+  rewrite_it->Kind == AOK_Input) {

Maybe this would be simpler with
  auto rewrite_it = find_if(it, AsmStrRewrites.end(), [&](const AsmRewrite 
) {
// ... conditions below
  });



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1481
+InlineAsmIdentifierInfo Info;
+if ((ParseError = ParseIntelOffsetOperator(Val, ID, Info, End)))
+  return true;

While this doesn't trigger warnings, it seems like it would be simpler to do 
the assignment as its own statement and then `if (ParseErrror) return true;`

I see that the local code style uses this pattern already, but I'm not sure 
that's a good reason to promote it and keep using it.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1484
+StringRef ErrMsg;
+if ((ParseError = SM.onOffset(Val, OffsetLoc, ID, Info,
+  isParsingInlineAsm(), ErrMsg)))

ditto



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)) {

Please test this corner case, I imagine it looks like:
  mov eax, offset 3



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");
+  }

Please add a test for this corner case, I'm curious to see if the error 
reporting really works.


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-16 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



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-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-13 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60854 tests passed, 0 failed 
and 726 were skipped.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or apply this patch 
.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 clang-format.patch 
,
 test-results.xml 
,
 diff.json 



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-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-13 Thread pre-merge checks [bot] via Phabricator via cfe-commits 
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60824 tests passed, 4 failed 
and 726 were skipped.

  failed: Clang.CodeGen/ms-inline-asm-64.c
  failed: Clang.CodeGen/ms-inline-asm.c
  failed: Clang.SemaTemplate/instantiation-depth-default.cpp
  failed: LLVM.CodeGen/X86/ms-inline-asm.ll

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
console-log.txt 
,
 CMakeCache.txt 
,
 test-results.xml 
,
 diff.json 



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-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 ) {
+  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 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 ) {
+  if (Sym) {
+ErrMsg = "cannot use more than one symbol in memory operand";
+return true;
+  }
+  Sym = Val;
+  SymName = ID;
+  return false;
+}
 
   public: