[PATCH] D41221: [RISCV] writeNopData support generate c.nop

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:37
+      : MCAsmBackend(), OSABI(OSABI), Is64Bit(Is64Bit) {
+    HasStdExtC = (TT.getArchName().substr(7).find('c') != std::string::npos);
+  }
----------------
Is there any way we can avoid spreading parsing logic throughout the backend? Can we get hold of an MCSubtargetInfo and check for `STI.getFeatureBits()[RISCV::FeatureStdExtC]`? The change to Triple.cpp then wouldn't be necessary in this patch (though might be something we want to look at, to make arguments more uniform between clang and the llvm tools).


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:103
   // The canonical nop on RISC-V is addi x0, x0, 0
-  for (uint64_t i = 0; i < Count; i += 4)
+  uint64_t i = Count;
+  for (; i >= 4; i -= 4)
----------------
I think it's kind of confusing define i here, then using it below, but also shadowing it with a new i in the NumOps loop. Would it be clearer to just calculate Nop16Count and Nop32Count, then decrement those values directly?


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:111
+    for (uint64_t i = 0; i != NumNops; ++i)
+    OW->write16(0x01);
+  }
----------------
Indentation.


Repository:
  rL LLVM

https://reviews.llvm.org/D41221