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

[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<X86Operand>
----------------
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