[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-07 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 101688.
mharoush marked an inline comment as done.
mharoush added a comment.

Simplified the AsmRewrite condition in x86AsmParser. Some fixes to simplify the 
Intel State Machine immediate value rewrite treatment divergence.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278

Files:
  include/llvm/MC/MCParser/MCAsmParser.h
  lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -713,12 +713,13 @@
   std::unique_ptr ParseIntelOffsetOfOperator();
   bool ParseIntelDotOperator(const MCExpr *Disp, const MCExpr *&NewDisp);
   unsigned IdentifyIntelOperator(StringRef Name);
-  unsigned ParseIntelOperator(unsigned OpKind);
+  unsigned ParseIntelOperator(unsigned OpKind, bool AddImmPrefix);
   std::unique_ptr
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
   bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
-  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
+  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier);
   std::unique_ptr
   ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
bool isSymbol, unsigned Size);
@@ -1187,7 +1188,7 @@
 InlineAsmIdentifierInfo &Info, bool AllowBetterSizeMatch) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
   // some other label reference.
-  if (isa(Disp) && Info.OpDecl && !Info.IsVarDecl) {
+  if (isa(Disp) && Info.OpDecl && !Info.isVarDecl()) {
 // Insert an explicit size if the user didn't have one.
 if (!Size) {
   Size = getPointerWidth();
@@ -1306,8 +1307,9 @@
 return false;
   return true;
 }
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier) {
+  ReplaceEnumIdentifier = false;
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
 
@@ -1358,7 +1360,7 @@
 if (OpKind == IOK_OFFSET) 
   return Error(IdentLoc, "Dealing OFFSET operator as part of"
 "a compound immediate expression is yet to be supported");
-int64_t Val = ParseIntelOperator(OpKind);
+int64_t Val = ParseIntelOperator(OpKind,SM.getAddImmPrefix());
 if (!Val)
   return true;
 StringRef ErrMsg;
@@ -1368,11 +1370,39 @@
 PrevTK == AsmToken::RBrac) {
   return false;
   } else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
 if (ParseIntelIdentifier(Val, Identifier, Info,
  /*Unevaluated=*/false, End))
   return true;
-SM.onIdentifierExpr(Val, Identifier);
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;
+  // SM should treat the value as it would an explicit integer in the 
+  // expression.
+  if(SM.onInteger(CE->getValue(), ErrMsg)) 
+return Error(IdentLoc, ErrMsg);
+  // In case we are called on a bracketed expression,
+  if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed 
+// expression in order to match the behavior for the equivalent 
+// integer tokens.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc,Len,
+CE->getValue());
+break;
+  }
+  // Set force rewrite flag only when not bracketed expression.
+  ReplaceEnumIdentifier = true;
+} else {
+  // Notify the SM a variable identifier was found.
+  InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+  	  SMInfo = Info;
+  SM.onIdentifierExpr(Val, Identifier);
+}
   }
   break;
 }
@@ -1452,7 +1482,8 @@
   // may have already parsed an immediate displacement before the bracketed
   // expression.
   IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
-  if (ParseIntelExpression(SM, End))
+  bool ReplaceEnumIdentifier;
+  if (ParseIntelExpression(SM, End, ReplaceEnum

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-07 Thread Matan via Phabricator via cfe-commits 
mharoush marked an inline comment as done.
mharoush added inline comments.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722
+  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier);
   std::unique_ptr

rnk wrote:
> mharoush wrote:
> > rnk wrote:
> > > Please try to eliminate the need for this extra boolean out parameter.
> > This flag is used in case we try to compile something like mov edx, A+6 ( 
> > when A is defined as ConstantEnum ) the original condition (Line:1905) will 
> > skip the rewrite since we have an identifier as the first Token, however if 
> > we try to compile 6+A it will be rewritten as expected.
> > 
> > I tried to solve this in different ways, I got either the exact opposite 
> > (A+6 was replaced and 6+A wasn't) or a collision of rewrites when trying to 
> > preform other forms of replacements and leaving this section intact. 
> > 
> > I can perhaps change the way we handle general expressions to the same way 
> > we handle them under parseIntelBracExpression, meaning use the first 
> > iteration of X86AsmParser to reformat the string (write imm prefixes and 
> > replace identifiers when needed) then refactor the string to its canonical 
> > form on the second pass. 
> > 
> > In short this was the simplest solution that worked without regression, 
> > maybe you have a better idea?
> > 
> > If the issue is the method signature pollution I can move this flag into 
> > the SM class as a member to indicate a rewrite is needed, however since 
> > this flag and method are limited to this context alone, I am not sure if 
> > the overhead is wanted in such case . 
> I suspect there is a way to simplify the code so that this corner case no 
> longer exists. I don't really have time to dig through the test cases to come 
> up with it, but please make an effort.
I believe this should resolve the complex if statement, However I still need 
the boolean value to enforce the rewrite for enum identifiers.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

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

2017-06-07 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 101689.

Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -644,8 +644,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -657,16 +657,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -773,7 +783,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,46 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+enum { A = 17 };
+	int r;
+}y;
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  __asm {mov eax, x::y.r}
+ // CHECK: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-07 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 101693.
mharoush added a comment.

added struct case to the test


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -644,8 +644,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -657,16 +657,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -773,7 +783,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,45 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+	enum { A = 17 };
+	int r;
+} y;
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-06-12 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 102172.
mharoush marked 6 inline comments as done.
mharoush added a comment.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp

Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -644,8 +644,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -657,16 +657,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -773,7 +783,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,55 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+	enum { A = 17 };
+	int r;
+} y;
+}
+// CHECK-LABEL: x86_enum_only
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  // Other constant type folding is currently unwanted.
+  __asm mov eax, [a]
+  }
+
+// CHECK-LABEL: x86_enum_namespaces
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+// CHECK-LABEL: x86_enum_arithmethic
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+// CHECK-LABEL: x86_enum_mem
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-12 Thread Matan via Phabricator via cfe-commits 
mharoush marked 3 inline comments as done.
mharoush added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-19 Thread Matan via Phabricator via cfe-commits 
mharoush added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

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

2017-06-28 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 104390.
mharoush added a comment.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/ms-inline-asm.c
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
  test/CodeGenCXX/ms-inline-asm-return.cpp

Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -644,8 +644,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -657,16 +657,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -773,7 +783,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: test/CodeGen/ms-inline-asm.c
===
--- test/CodeGen/ms-inline-asm.c
+++ test/CodeGen/ms-inline-asm.c
@@ -42,7 +42,7 @@
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
@@ -61,7 +61,7 @@
 mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -94,7 +94,7 @@
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -265,7 +265,7 @@
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -312,13 +312,13 @@
 void t25() {
 // CHECK: t25
   __asm mov eax, 0h
-// CHECK: mov eax, $$0h
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHECK: mov eax, $$0a2h
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010b
-// CHECK: mov eax, $$10100010b
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010BU
 // CHECK: mov eax, $$162
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,60 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+	enum { A = 17 };
+	int r;
+} y;
+}
+// CHECK-LABEL: x86_enum_only
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  // Other constant type folding is currently unwanted.
+  __asm mov eax, [a]
+  }
+
+// CHECK-LABEL: x86_enum_namespaces
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK-SAME: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+// CHECK-LABEL: x86_enum_arithmethic
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$21

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-28 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 104391.
mharoush marked an inline comment as done.
mharoush added a comment.

simplified the rewrite condition of complex expressions and eliminated the need 
to use the ReplaceEnumIdentifier flag. This requires making small adjustments 
to some of the older test cases seen in [https://reviews.llvm.org/D33277]. Made 
some minor alterations and clarifications to satisfy reviewer comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278

Files:
  include/llvm/MC/MCParser/MCAsmParser.h
  lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -713,7 +713,7 @@
   std::unique_ptr ParseIntelOffsetOfOperator();
   bool ParseIntelDotOperator(const MCExpr *Disp, const MCExpr *&NewDisp);
   unsigned IdentifyIntelOperator(StringRef Name);
-  unsigned ParseIntelOperator(unsigned OpKind);
+  unsigned ParseIntelOperator(unsigned OpKind, bool AddImmPrefix);
   std::unique_ptr
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
@@ -1187,7 +1187,7 @@
 InlineAsmIdentifierInfo &Info, bool AllowBetterSizeMatch) {
   // If we found a decl other than a VarDecl, then assume it is a FuncDecl or
   // some other label reference.
-  if (isa(Disp) && Info.OpDecl && !Info.IsVarDecl) {
+  if (isa(Disp) && Info.OpDecl && !Info.isVarDecl()) {
 // Insert an explicit size if the user didn't have one.
 if (!Size) {
   Size = getPointerWidth();
@@ -1358,7 +1358,7 @@
 if (OpKind == IOK_OFFSET) 
   return Error(IdentLoc, "Dealing OFFSET operator as part of"
 "a compound immediate expression is yet to be supported");
-int64_t Val = ParseIntelOperator(OpKind);
+int64_t Val = ParseIntelOperator(OpKind,SM.getAddImmPrefix());
 if (!Val)
   return true;
 StringRef ErrMsg;
@@ -1368,13 +1368,40 @@
 PrevTK == AsmToken::RBrac) {
   return false;
   } else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
 if (ParseIntelIdentifier(Val, Identifier, Info,
- /*Unevaluated=*/false, End))
+  /*Unevaluated=*/false, End))
   return true;
-SM.onIdentifierExpr(Val, Identifier);
-  }
-  break;
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;
+  // Pass the enum identifier integer value to the SM calculator.
+  if (SM.onInteger(CE->getValue(), ErrMsg))
+return Error(IdentLoc, ErrMsg);
+  // Match the behavior of integer tokens when getAddImmPrefix flag is
+  // set.
+  if (SM.getAddImmPrefix()) {
+assert(isParsingInlineAsm() &&
+   "Expected to be parsing inline assembly.");
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed
+// expression.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc, Len,
+CE->getValue());
+break;
+  }
+}
+else {
+  // Notify the SM a variable identifier was found.
+  InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+  SMInfo = Info;
+  SM.onIdentifierExpr(Val, Identifier);
+}
+  }  break;
 }
 case AsmToken::Integer: {
   StringRef ErrMsg;
@@ -1452,6 +1479,7 @@
   // may have already parsed an immediate displacement before the bracketed
   // expression.
   IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
+  
   if (ParseIntelExpression(SM, End))
 return nullptr;
 
@@ -1560,6 +1588,14 @@
   assert((End.getPointer() == EndPtr || !Result) &&
  "frontend claimed part of a token?");
 
+  // Check if the search yielded a constant integer (enum identifier).
+  if (Result && Info.isConstEnum()) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(Info.ConstIntValue.getSExtValue(),
+ getParser().getContext());
+return false;
+  }
   // If the identifier lookup was unsuccessful, assume that we are dealing with
   // a label.
   if (!Result) {
@@ -1758,7 +1794,7 @@
 /// variable.  A variable's size is t

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-28 Thread Matan via Phabricator via cfe-commits 
mharoush marked 4 inline comments as done.
mharoush added inline comments.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382
+if (const MCConstantExpr *CE = 
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;

rnk wrote:
> rnk wrote:
> > Please use clang-format here and elsewhere
> not addressed?
Persistent text wrapping of my editor, fixed.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1378
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.

coby wrote:
> assumption ~~> assertion
I can assert the Identifier Info is indeed an enum. However, this is a general 
handle for any kind of value that may by passed back from the identifier 
parsing. There is no point in this assertion here since the value of type 
MCConstantExper is always safe. We simply make the decision at the parse 
identifier method.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1380
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {

coby wrote:
> I think this whole section better suites within its own function. something 
> like 'ParseInlineAsmEnumValue'
not much added value here and only a few lines can be moved (~5). Its not 
really worth it.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1383
+  StringRef ErrMsg;
+  // SM should treat the value as it would an explicit integer in the 
+  // expression.

coby wrote:
> rephrase
Your comment is too general, I made some clarifications.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1388
+  // In case we are called on a bracketed expression,
+  if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum

coby wrote:
> 'isParsingInlineAsm()' is unnecessary here (you can only reach this piece of 
> code when parsing inline asm)
This is just for readability and consistency with SM.onInteger(). Replaced with 
assertion to ease your mind



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1826
   }
-
-  // Rewrite the type operator and the C or C++ type or variable in terms of an
-  // immediate.  E.g. TYPE foo -> $$4
-  unsigned Len = End.getPointer() - TypeLoc.getPointer();
-  InstInfo->AsmRewrites->emplace_back(AOK_Imm, TypeLoc, Len, CVal);
-
+  // Only when in bracketed mode, preform explicit rewrite
+  if (AddImmPrefix) {

coby wrote:
> Not keen to the use of SM.getAddImmPrefix() as a mean of distinguish whether 
> we are parsing a bracketed expression. I know it is currently turned on when 
> parsing it, but it isn't asserted/guaranteed.
> Regardless - I'm pretty sure we can manage without this rewrite, or at the 
> very least - should, now that TYPE/LENGTH/SIZE are part of the State Machine.
I meant this flag is only set when parsing bracketed expressions. However, I 
will rephrase the comment since we only care about the AddImmPrefix flag, which 
reflects the SM matching flag. The objective here is to match the behavior 
required on integer constants when the SM flag is set.



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1907
 unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
-if (StartTok.getString().size() == Len)
-  // Just add a prefix if this wasn't a complex immediate expression.
-  InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
-else
-  // Otherwise, rewrite the complex expression as a single immediate.
+if (StartTok.getString().size() != Len || ReplaceEnumIdentifier)
+  // Rewrite the complex expression as a single immediate.

coby wrote:
> you may just perform an AOK_Imm rewrite regardless the complexity of the 
> immediate expression, and neglect 'ReplaceEnumIdentifier'
Thanks, this seems fine after the condition refinement. Just had to change some 
of the older inline asm tests which expects the IR statement to contain a 
binary/octal/hex base immediate value which this rewrite will replace with the 
decimal value.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-07-05 Thread Matan via Phabricator via cfe-commits 
mharoush added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

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

2017-07-05 Thread Matan via Phabricator via cfe-commits 
mharoush added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D33277



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

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

2017-07-25 Thread Matan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL308965: This patch enables the usage of constant Enum 
identifiers within Microsoft… (authored by mharoush).

Changed prior to commit:
  https://reviews.llvm.org/D33277?vs=104390&id=108044#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  cfe/trunk/lib/Sema/SemaStmtAsm.cpp
  cfe/trunk/test/CodeGen/ms-inline-asm.c
  cfe/trunk/test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
  cfe/trunk/test/CodeGenCXX/ms-inline-asm-return.cpp

Index: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp
@@ -645,8 +645,8 @@
   // Referring to parameters is not allowed in naked functions.
   if (CheckNakedParmReference(Result.get(), *this))
 return ExprError();
-
-  QualType T = Result.get()->getType();
+  Expr *Res = Result.get();
+  QualType T = Res->getType();
 
   if (T->isDependentType()) {
 return Result;
@@ -658,16 +658,26 @@
   }
 
   // Otherwise, it needs to be a complete type.
-  if (RequireCompleteExprType(Result.get(), diag::err_asm_incomplete_type)) {
+  if (RequireCompleteExprType(Res, diag::err_asm_incomplete_type)) {
 return ExprError();
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
 
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
-Info.IsVarDecl = true;
+  if (!Res->isRValue()) {
+Info.setKindVariable();
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant, currently we do not allow
+  // other constant integers to be folded.
+  if (isa(T) &&
+Res->EvaluateAsRValue(EvlResult, getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.setKindConstEnum();
+  }
   return Result;
 }
 
@@ -774,7 +784,7 @@
   fillInlineAsmTypeInfo(Context, Result.get()->getType(), Info);
 
   // Fields are "variables" as far as inline assembly is concerned.
-  Info.IsVarDecl = true;
+  Info.setKindVariable();
 
   return Result;
 }
Index: cfe/trunk/test/CodeGen/ms-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ms-inline-asm.c
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c
@@ -42,7 +42,7 @@
 void t6(void) {
   __asm int 0x2c
 // CHECK: t6
-// CHECK: call void asm sideeffect inteldialect "int $$0x2c", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 }
 
 void t7() {
@@ -61,7 +61,7 @@
 mov eax, ebx
   }
 // CHECK: t7
-// CHECK: call void asm sideeffect inteldialect "int $$0x2cU", "~{dirflag},~{fpsr},~{flags}"()
+// CHECK: call void asm sideeffect inteldialect "int $$44", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "", "~{dirflag},~{fpsr},~{flags}"()
 // CHECK: call void asm sideeffect inteldialect "mov eax, ebx", "~{eax},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -94,7 +94,7 @@
 // CHECK: t9
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$0x07
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -265,7 +265,7 @@
 // CHECK: t21
 // CHECK: call void asm sideeffect inteldialect
 // CHECK-SAME: push ebx
-// CHECK-SAME: mov ebx, $$07H
+// CHECK-SAME: mov ebx, $$7
 // CHECK-SAME: pop ebx
 // CHECK-SAME: "~{ebx},~{esp},~{dirflag},~{fpsr},~{flags}"()
 }
@@ -312,13 +312,13 @@
 void t25() {
 // CHECK: t25
   __asm mov eax, 0h
-// CHECK: mov eax, $$0h
+// CHECK: mov eax, $$4294967295
   __asm mov eax, 0fhU
 // CHECK: mov eax, $$15
   __asm mov eax, 0a2h
-// CHECK: mov eax, $$0a2h
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010b
-// CHECK: mov eax, $$10100010b
+// CHECK: mov eax, $$162
   __asm mov eax, 10100010BU
 // CHECK: mov eax, $$162
 // CHECK: "~{eax},~{dirflag},~{fpsr},~{flags}"()
Index: cfe/trunk/test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- cfe/trunk/test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ cfe/trunk/test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,60 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCHECK %s
+namespace x {
+enum { A = 12 };
+struct y_t {
+	enum { A = 17 };
+	int r;
+} y;
+}
+// CHECK-LABEL: x86_enum_only
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  // Other constant type folding is currently unwanted.
+  __asm mov eax, [a]
+  }
+
+// CHECK-LABEL: x86_enum_namespaces
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: call void asm
+  // CHECK-SAME: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK-SAME: mov eax, $$17
+  __asm mov eax, x::y_t::A
+  // CHECK-NEXT: call void asm
+  // CHECK-SAME: mov eax, $$1
+  __asm {mov eax, A}
+}
+
+/

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

2017-05-17 Thread Matan via Phabricator via cfe-commits 
mharoush created this revision.
mharoush added a project: clang-c.
Herald added a subscriber: eraman.

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

part 1 out of 2.


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Parse/ParseStmtAsm.cpp
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp


Index: lib/Parse/ParseStmtAsm.cpp
===
--- lib/Parse/ParseStmtAsm.cpp
+++ lib/Parse/ParseStmtAsm.cpp
@@ -94,6 +94,23 @@
 return Info.OpDecl;
   }
 
+  // Try to evaluate the inline asm identifier lookup result as an
+  // enumerated type. This method expects LookupResult to be the
+  // result of a call to LookupInlineAsmIdentifier.
+  bool EvaluateLookupAsEnum(void *LookupResult, int64_t &Result) {
+if (!LookupResult) return false;
+Expr *Res = static_cast (LookupResult);
+if (Res && isa(Res->getType())) {
+  Expr::EvalResult EvlResult;
+  if (Res->EvaluateAsRValue(EvlResult,
+  TheParser.getActions().getASTContext())) {
+Result = EvlResult.Val.getInt().getExtValue();
+return true;
+  }
+}
+return false;
+  }
+
   StringRef LookupInlineAsmLabel(StringRef Identifier, llvm::SourceMgr &LSM,
  llvm::SMLoc Location,
  bool Create) override {
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  __asm mov eax, [a]
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y::A
+  // CHECK: mov eax, $$1
+  __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+  
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, A[arr + A] }
+}
\ No newline at end of file


Index: lib/Parse/ParseStmtAsm.cpp
===
--- lib/Parse/ParseStmtAsm.cpp
+++ lib/Parse/ParseStmtAsm.cpp
@@ -94,6 +94,23 @@
 return Info.OpDecl;
   }
 
+  // Try to evaluate the inline asm identifier lookup result as an
+  // enumerated type. This method expects LookupResult to be the
+  // result of a call to LookupInlineAsmIdentifier.
+  bool EvaluateLookupAsEnum(void *LookupResult, int64_t &Result) {
+if (!LookupResult) return false;
+Expr *Res = static_cast (LookupResult);
+if (Res && isa(Res->getType())) {
+  Expr::EvalResult EvlResult;
+  if (Res->EvaluateAsRValue(EvlResult,
+  TheParser.getActions().getASTContext())) {
+Result = EvlResult.Val.getInt().getExtValue();
+return true;
+  }
+}
+return false;
+  }
+
   StringRef LookupInlineAsmLabel(StringRef Identifier, llvm::SourceMgr &LSM,
  llvm::SMLoc Location,
  bool Create) override {
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,50 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_only(){
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  __asm mov eax, [a]
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y::A
+  // CHECK: mov eax, $$1
+  __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+  
+  // CH

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-05-17 Thread Matan via Phabricator via cfe-commits 
mharoush created this revision.

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

part 2 out of 2.
[https://reviews.llvm.org/D33277]


Repository:
  rL LLVM

https://reviews.llvm.org/D33278

Files:
  include/llvm/MC/MCParser/MCAsmParser.h
  lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -718,7 +718,8 @@
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
   bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
-  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
+  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier);
   std::unique_ptr
   ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
bool isSymbol, unsigned Size);
@@ -1306,8 +1307,9 @@
 return false;
   return true;
 }
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier) {
+  ReplaceEnumIdentifier = false;
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
 
@@ -1368,11 +1370,40 @@
 PrevTK == AsmToken::RBrac) {
   return false;
   } else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
+Info.clear();
 if (ParseIntelIdentifier(Val, Identifier, Info,
  /*Unevaluated=*/false, End))
   return true;
-SM.onIdentifierExpr(Val, Identifier);
+// Check if the parsed identifier was a constant Integer. Here we 
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE = 
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;
+  // SM should treat the value as it would an explicit integer in the 
+  // expression.
+  if(SM.onInteger(CE->getValue(), ErrMsg)) 
+return Error(IdentLoc, ErrMsg);
+  // In case we are called on a bracketed expression,
+  if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed 
+// expression in order to match the behavior for the equivalent 
+// integer tokens.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc,Len, 
+CE->getValue());
+break;
+  }
+  // Set force rewrite flag only when not bracketed expression.
+  ReplaceEnumIdentifier = true;
+} else {
+  // Notify the SM a variable identifier was found.
+  InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+  	  SMInfo = Info;
+  SM.onIdentifierExpr(Val, Identifier);
+}
   }
   break;
 }
@@ -1452,7 +1483,8 @@
   // may have already parsed an immediate displacement before the bracketed
   // expression.
   IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
-  if (ParseIntelExpression(SM, End))
+  bool ReplaceEnumIdentifier;
+  if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
 return nullptr;
 
   const MCExpr *Disp = nullptr;
@@ -1559,7 +1591,15 @@
   // failed parsing.
   assert((End.getPointer() == EndPtr || !Result) &&
  "frontend claimed part of a token?");
-
+  
+  // Try to evaluate the result as a constant integer (enum identifier).
+  int64_t ConstVal = 0;
+  if (SemaCallback->EvaluateLookupAsEnum(Result, ConstVal)) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(ConstVal, getParser().getContext());
+return false;
+  }
   // If the identifier lookup was unsuccessful, assume that we are dealing with
   // a label.
   if (!Result) {
@@ -1852,18 +1892,21 @@
   AsmToken StartTok = Tok;
   IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
/*AddImmPrefix=*/false);
-  if (ParseIntelExpression(SM, End))
+  // The parsed expression may contain enum identifier tokens which we must 
+  // replace, ReplaceEnumIdentifier flag lets us know when to force rewrite.
+  bool ReplaceEnumIdentifier;
+  if (ParseIntelExpression(SM, End, ReplaceEnumIde

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-05-22 Thread Matan via Phabricator via cfe-commits 
mharoush marked an inline comment as done.
mharoush added inline comments.



Comment at: include/llvm/MC/MCParser/MCAsmParser.h:64
 unsigned &Offset) = 0;
+  virtual bool EvaluateLookupAsEnum(void *LookupResult,int64_t &Result) = 0;
 };

rnk wrote:
> It would be cleaner if InlineAsmIdentifierInfo simply contained an APInt 
> indicating that the identifier in question was an enum, or other integral 
> constant expression. Less callbacks to implement.
Done



Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722
+  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier);
   std::unique_ptr

rnk wrote:
> Please try to eliminate the need for this extra boolean out parameter.
This flag is used in case we try to compile something like mov edx, A+6 ( when 
A is defined as ConstantEnum ) the original condition (Line:1905) will skip the 
rewrite since we have an identifier as the first Token, however if we try to 
compile 6+A it will be rewritten as expected.

I tried to solve this in different ways, I got either the exact opposite (A+6 
was replaced and 6+A wasn't) or a collision of rewrites when trying to preform 
other forms of replacements and leaving this section intact. 

I can perhaps change the way we handle general expressions to the same way we 
handle them under parseIntelBracExpression, meaning use the first iteration of 
X86AsmParser to reformat the string (write imm prefixes and replace identifiers 
when needed) then refactor the string to its canonical form on the second pass. 

In short this was the simplest solution that worked without regression, maybe 
you have a better idea?

If the issue is the method signature pollution I can move this flag into the SM 
class as a member to indicate a rewrite is needed, however since this flag and 
method are limited to this context alone, I am not sure if the overhead is 
wanted in such case . 


Repository:
  rL LLVM

https://reviews.llvm.org/D33278



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

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-05-22 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 99778.
mharoush added a comment.

Using identifier info to pass enum information.


Repository:
  rL LLVM

https://reviews.llvm.org/D33278

Files:
  include/llvm/MC/MCParser/MCAsmParser.h
  lib/Target/X86/AsmParser/X86AsmParser.cpp

Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -718,7 +718,8 @@
   ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
   std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
   bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
-  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
+  bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier);
   std::unique_ptr
   ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
bool isSymbol, unsigned Size);
@@ -1306,8 +1307,9 @@
 return false;
   return true;
 }
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, 
+bool &ReplaceEnumIdentifier) {
+  ReplaceEnumIdentifier = false;
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
 
@@ -1368,11 +1370,40 @@
 PrevTK == AsmToken::RBrac) {
   return false;
   } else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
+Info.clear();
 if (ParseIntelIdentifier(Val, Identifier, Info,
  /*Unevaluated=*/false, End))
   return true;
-SM.onIdentifierExpr(Val, Identifier);
+// Check if the parsed identifier was a constant Integer. Here we 
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE = 
+dyn_cast_or_null(Val)) {
+  StringRef ErrMsg;
+  // SM should treat the value as it would an explicit integer in the 
+  // expression.
+  if(SM.onInteger(CE->getValue(), ErrMsg)) 
+return Error(IdentLoc, ErrMsg);
+  // In case we are called on a bracketed expression,
+  if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed 
+// expression in order to match the behavior for the equivalent 
+// integer tokens.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc,Len, 
+CE->getValue());
+break;
+  }
+  // Set force rewrite flag only when not bracketed expression.
+  ReplaceEnumIdentifier = true;
+} else {
+  // Notify the SM a variable identifier was found.
+  InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+  	  SMInfo = Info;
+  SM.onIdentifierExpr(Val, Identifier);
+}
   }
   break;
 }
@@ -1452,7 +1483,8 @@
   // may have already parsed an immediate displacement before the bracketed
   // expression.
   IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
-  if (ParseIntelExpression(SM, End))
+  bool ReplaceEnumIdentifier;
+  if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
 return nullptr;
 
   const MCExpr *Disp = nullptr;
@@ -1559,7 +1591,15 @@
   // failed parsing.
   assert((End.getPointer() == EndPtr || !Result) &&
  "frontend claimed part of a token?");
-
+  
+  // Check if the search yielded a constant integer (enum identifier).
+  if (Result && Info.IsConstEnum) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(Info.ConstIntValue.getSExtValue(), 
+ getParser().getContext());
+return false;
+  }
   // If the identifier lookup was unsuccessful, assume that we are dealing with
   // a label.
   if (!Result) {
@@ -1852,18 +1892,21 @@
   AsmToken StartTok = Tok;
   IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
/*AddImmPrefix=*/false);
-  if (ParseIntelExpression(SM, End))
+  // The parsed expression may contain enum identifier tokens which we must 
+  // replace, ReplaceEnumIdentifier flag lets us know when to force rewrite.
+  bool ReplaceEnumIdentifier;
+  if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
 return nullptr;
-
   bool isSymbol = SM.getSym() &&

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

2017-05-22 Thread Matan via Phabricator via cfe-commits 
mharoush updated this revision to Diff 99780.
mharoush added a comment.

removed functions and dropped an irrelevant test case.


Repository:
  rL LLVM

https://reviews.llvm.org/D33277

Files:
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/x86-ms-inline-asm-enum_feature.cpp


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
-
+  
+  Expr *Res = Result.get();
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
+  if (!Res->isRValue()) {
 Info.IsVarDecl = true;
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant
+  if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult, 
+   getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+  }
   return Result;
 }
 
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y::A
+  // CHECK: mov eax, $$1
+  __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+  
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
   }
 
   fillInlineAsmTypeInfo(Context, T, Info);
-
+  
+  Expr *Res = Result.get();
   // We can work with the expression as long as it's not an r-value.
-  if (!Result.get()->isRValue())
+  if (!Res->isRValue()) {
 Info.IsVarDecl = true;
+return Result;
+  }
 
+  Expr::EvalResult EvlResult;
+  // Try to evaluate the identifier as enum constant
+  if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult, 
+   getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+  }
   return Result;
 }
 
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+  enum { A = 1 };
+  // CHECK: mov eax, $$12
+  __asm mov eax, x::A
+  // CHECK: mov eax, $$17
+  __asm mov eax, x::y::A
+  // CHECK: mov eax, $$1
+  __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+  enum { A = 1, B };
+  // CHECK: mov eax, $$21
+  __asm mov eax, (A + 9) * 2 + A
+  // CHECK: mov eax, $$4
+  __asm mov eax, A << 2
+  // CHECK: mov eax, $$2
+  __asm mov eax, B & 3
+  // CHECK: mov eax, $$5
+  __asm mov eax, 3 + (B & 3)
+  // CHECK: mov eax, $$8
+  __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+  int arr[4];
+  enum { A = 4, B };
+  
+  // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+  __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+  // CHECK: mov eax, dword ptr $$4$0
+  __asm { mov eax, dword ptr [arr + A] }
+  // CHECK: mov eax, dword ptr $$8$0
+  __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-22 Thread Matan via Phabricator via cfe-commits 
mharoush marked an inline comment as done.
mharoush added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:100
+  // result of a call to LookupInlineAsmIdentifier.
+  bool EvaluateLookupAsEnum(void *LookupResult, int64_t &Result) {
+if (!LookupResult) return false;

rnk wrote:
> Please format this, clang-format will do the job
Revised



Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12
+  const int a = 0;
+  // CHECK-NOT: mov eax, [$$0]
+  __asm mov eax, [a]

rnk wrote:
> Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing 
> ms-inline-asm.c tests do so that this test is easier to debug when it fails.
This test case was just meant verify that other Integer constants are not 
folded since we get a different behavior for statements such as mov eax, [a]. 
#---
In this example X86AsmParser regards the address of the variable 'a' and not 
its value i.e. we end up with the value of 'a' in eax (loaded from the stack) 
and not with the value pointed by the const int value of 'a' as its address.
---#

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


Repository:
  rL LLVM

https://reviews.llvm.org/D33277



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