[PATCH] D35621: X86 Asm can't work properly with symbolic Scale

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

A few minors - does anyone have any more comments?



================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1585
       RewriteIntelBracExpression(*InstInfo->AsmRewrites, SM.getSymName(),
                                  ImmDisp, SM.getImm(), BracLoc, StartInBrac,
                                  End);
----------------
Another use of SM.getImm() - is it safe to move SM.getImm() earlier to replace this as well?


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1595
       Disp = MCBinaryExpr::createAdd(Disp, Imm, getContext());
-    else
-      Disp = Imm;  // An immediate displacement only.
-  }
+    else if (Disp->getKind() == llvm::MCExpr::Constant && ImmVal) {
+      assert(Disp == SM.getSym() && "It's symbolic disp initialized above");
----------------
Flip to match the previous if():
```
else if (ImmVal && Disp->getKind() == llvm::MCExpr::Constant) {
```


================
Comment at: test/MC/X86/intel-syntax-3.s:32
 
-// CHECK-STDERR: scale factor in address must be 1, 2, 4 or 8
   lea RDX, [8 + number * RAX + RCX]
 
----------------
Now that these are passing - move them to another of the intel-syntax test files and add suitable checks


https://reviews.llvm.org/D35621