[PATCH] D64834: [Xtensa 8/10] Add support of the Xtensa shift/load/store/move and processor control instructions.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp:407
+      return MatchOperand_NoMatch;
+    RegName = StringRef(std::to_string(getLexer().getTok().getIntVal()));
+    break;
----------------
andreisfr wrote:
> barannikov88 wrote:
> > Bug: `std::string` termporary is destroyed at ';' and RegName references freed memory.
> Thank you very much, fixed this issue. 
The issue is still there, RegName references deleted memory. One of the possible way to resolve this issue is to declare RegName as std::string rather than StringRef.

BTW, what does this code do? I couldn't find any registers that contain only digits in their names (all start with a letter), so MatchRegisterName is bound to fail. Nor could I find a test that uses parenthesized numbers.

Are those some special registers not modeled in MC layer (and represented as numbers in assembly language)?
If so, you need to differentiate them from ordinary registers somehow because there is no guarantee that their numbers and / or encodings do not collide with the autogenerated ones for the registers that are being modeled.

As a side node, 'SR' variable should have a more descriptive name (or, at least, its name should be explained in a comment).


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

https://reviews.llvm.org/D64834