[PATCH] D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for alignment when linker relaxation enabled

[James Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: lib/MC/MCAssembler.cpp:840
+      } else if (dyn_cast<MCAlignFragment>(&Frag)) {
+        MCAlignFragment &AF = cast<MCAlignFragment>(Frag);
+        // Insert fixup type for the alignment if the target define
----------------
This should be moved into the if (and the `dyn_cast` should never have existed on its own in the first place; if the result is discarded it should be an `isa`).


================
Comment at: lib/MC/MCAssembler.cpp:844
+        if (auto *ELFSec = dyn_cast<const MCSectionELF>(AF.getParent())) {
+          if (ELFSec->getSectionName() == StringRef(".text"))
+            getBackend().insertFixupForAlignDirectiveInTextSection(*this,
----------------
Hard-coding the section name seems wrong. What if somebody uses a section attribute? GNU as will do the right thing for any executable section regardless of its name, and we should too; from an initial glance it seems that `MCSection::UseCodeAlign()` will tell you exactly the right thing, and as a bonus is no longer ELF-specific.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D47755