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

[Matan via Phabricator via llvm-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<X86Operand>
----------------
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