[PATCH] D132482: RISCV: permit unaligned nop-slide padding emission

[Luís Marques via Phabricator via llvm-commits]

luismarques added a comment.

The code changes seem fine now. Any additional thoughts about that, @jrtc27?

The `llvm/test/MC/RISCV/align.s` test update needs to be fixed, see my inline comment. After that I think it LGTM.



================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:369
+  // The canonical nop on RVC is c.nop.
+  if (Count % 4 == 2) {
+    OS.write(STI->getFeatureBits()[RISCV::FeatureStdExtC] ? "\x01\0" : "\0\0",
----------------
Nit: given the earlier adjustment, isn't this the same as `if (Count % 4)`? Maybe your way is clearer anyway...


================
Comment at: llvm/test/MC/RISCV/align.s:44
 # C-EXT-RELAX-RELOC-NOT: R_RISCV_ALIGN - 0x2
-# C-EXT-RELAX-INST-NOT:  c.nop
+# C-EXT-RELAX-INST:  c.nop
 	bne     zero, a0, .LBB0_2
----------------
This is the wrong test update. This was supposed to test the condition mentioned in the earlier comment about only having one `c.nop`. I think what you want to do is re-order lines 51 and 52.


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

https://reviews.llvm.org/D132482