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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:579
     }
-    void onIdentifierExpr(const MCExpr *SymRef, StringRef SymRefName) {
+    bool checkSetScale(int64_t _Scale, StringRef &ErrMsg) {
+      if (checkIndexReg(ErrMsg))
----------------
Call this `NewScale` or something more informative than `_Scale`.


================
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1437
+          int64_t TmpInt;
+          Val->evaluateAsAbsolute(TmpInt);
+          if (SM.onInteger(TmpInt, ErrMsg))
----------------
`evaluateAsAbsolute` may fail. I think you should rewrite this like so:
  if (const auto *CE = dyn_cast<MCConstantExpr>(Val)) {
    if (SM.onInteger(CE->getValue(), ErrMsg)
      return Error(Tok.getLoc(), ErrMsg);
  }
That's all `evaluateAsAbsolute` is doing for you behind the scenes anyway.


https://reviews.llvm.org/D35621