[PATCH] D23566: [RISCV 8/10] Add support for all RV32I instructions

[David Chisnall via Phabricator via llvm-commits]

theraven added inline comments.


================
Comment at: lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:146
+    char Prev = '\0';
+    for (char c : Str) {
+      if (c != 'i' && c != 'o' && c != 'r' && c != 'w')
----------------
mgrang wrote:
> Shouldn't this be:
> ```
> for (char &c : Str)
> ```
> Refer: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Do you really think that making a copy of a `char` and reusing it in a register is more expensive than taking a reference to a char in the middle of the string and relying on alias analysis to ensure that we only load via that pointer once?

References to small (register or pair of register) POD values are likely to have more overhead in a range-based `for` loop than copies and should only be used if you need to modify the value in place.


https://reviews.llvm.org/D23566