[PATCH] D83366: [MC] Simplify the logic of applying fixup for fragments, NFCI

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 7 23:47:39 PDT 2020


MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCAssembler.cpp:825
+      // shouldInsertFixupForCodeAlign target hook.
+      if (MCAlignFragment *AF = dyn_cast<MCAlignFragment>(&Frag)) {
+        if (Sec.UseCodeAlign() && AF->hasEmitNops()) {
----------------
skan wrote:
> MaskRay wrote:
> > Can you move MCAlignFragment into the switch as well?
> ```      
> ArrayRef<MCFixup> Fixups;
> MutableArrayRef<char> Contents;
> const MCSubtargetInfo *STI = nullptr;
> 
> for (const MCFixup &Fixup : Fixups) {
> ...
> ```
> is not used by `MCAlignFragment`.  I think an early `continue` here can make things more clear.
They have trivial constructors. Moving it can be clearer/more efficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83366





More information about the llvm-commits mailing list