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

[Andrei Safronov via Phabricator via llvm-commits]

andreisfr added inline comments.


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


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/CMakeLists.txt:17
+
+add_dependencies(LLVMXtensaAsmParser XtensaCommonTableGen)
----------------
barannikov88 wrote:
> Isn't this dependency implicit already?
I think yes, I removed it.


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


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


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

https://reviews.llvm.org/D64832