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

[Shiva Chen via Phabricator via llvm-commits]

shiva0217 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);
+  }
----------------
asb wrote:
> 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).
The latest revision will:
1. Add parseRISCVTriple to get target feature by parsing triple for the backend.
2. Add MCSubtargetInfo in RISCVAsmBackend class and using `STI.getFeatureBits()[RISCV::FeatureStdExtC]` to check target feature.
3. The STI initial by parsing triple, if we remove the change in Triple.cpp, triple with suffix `imac` will not allow, and then STI can't get target feature.



================
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)
----------------
asb wrote:
> 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?
Yes, define Nop32Count and Nop16Count would be more readable.
Thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D41221