[PATCH] D64832: [Xtensa 6/10] Add Xtensa basic assembler parser.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/CMakeLists.txt:1
+include_directories( ${CMAKE_CURRENT_BINARY_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/.. )
+
----------------
Should not be necessary.


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/CMakeLists.txt:17
+
+add_dependencies(LLVMXtensaAsmParser XtensaCommonTableGen)
----------------
Isn't this dependency implicit already?


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp:306
+
+  if ((!MatchRegisterName(Name)) && (!MatchRegisterAltName(Name))) {
+    getParser().Lex(); // Eat identifier token.
----------------
(nit) extra parentheses.


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp:350
+  SMLoc S = getLoc();
+  SMLoc E = SMLoc::getFromPointer(S.getPointer() - 1);
+  getLexer().Lex();
----------------
Is it EndLoc < StartLoc? Why don't just use .getLoc(), .getEndLoc() on register name token?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64832/new/

https://reviews.llvm.org/D64832