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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
