[PATCH] D73486: [ms] [llvm-ml] Add support for attempted register parsing

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 10 07:21:19 PST 2020


thakis added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:2303
+      for (AsmToken Tok : Tokens) {
+        getLexer().UnLex(Tok);
+      }
----------------
Looks like this unlexes them left-to-right. Don't you have to unlex right-to-left?


================
Comment at: llvm/lib/Target/AVR/AsmParser/AVRAsmParser.cpp:381
+        getLexer().UnLex(std::move(HighTok));
+        getLexer().UnLex(std::move(ColonTok));
+      }
----------------
Same question here. Isn't this in the wrong ordre?


================
Comment at: llvm/lib/Target/Lanai/AsmParser/LanaiAsmParser.cpp:707
+    if (RegNum == 0) {
+      if (PercentTok.hasValue())
+        Lexer.UnLex(PercentTok.getValue());
----------------
Only if RestoreOnFailure (?)


================
Comment at: llvm/lib/Target/Lanai/AsmParser/LanaiAsmParser.cpp:714
   }
+  if (PercentTok.hasValue())
+    Lexer.UnLex(PercentTok.getValue());
----------------
same


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1159
+      for (const AsmToken &T : Tokens) {
+        getLexer().UnLex(T);
+      }
----------------
wrong direction again?


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1192
+        for (const AsmToken &T : Tokens) {
+          getLexer().UnLex(T);
+        }
----------------
itto


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1219
+        for (const AsmToken &T : Tokens) {
+          getLexer().UnLex(T);
+        }
----------------
ditto


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1236
+        for (const AsmToken &T : Tokens) {
+          getLexer().UnLex(T);
+        }
----------------
ditto

(also, this happens often enough that you might want to put it in a helper lambda?)


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1248
+        for (const AsmToken &T : Tokens) {
+          getLexer().UnLex(T);
+        }
----------------
ditto


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1298
+      for (const AsmToken &T : Tokens) {
+        getLexer().UnLex(T);
+      }
----------------
ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73486





More information about the llvm-commits mailing list