[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 22:38:45 PDT 2020


MaskRay added a comment.

In D83366#2138199 <https://reviews.llvm.org/D83366#2138199>, @skan wrote:

> In D83366#2138183 <https://reviews.llvm.org/D83366#2138183>, @MaskRay wrote:
>
> > Note that the number of lines actually increases... so I am not sure this is "simplification"
>
>
> The increased line number comes from `}` and `break`, the logic is simplified and corrected since we do not need to check `isa<MCEncodedFragment>` here.  We extract the code for "MCAlignFragment" out to make it clear that it doesn't have fixup. If you care
>  about the increased line number here, I can remove the unnecessary "{}" here or use `if` statement.


This is: if else if else if -> switch. I think it is just a refactoring and I agree that it is the right thing to do, but I don't find complexity being simplified.



================
Comment at: llvm/lib/MC/MCAssembler.cpp:825
+      // shouldInsertFixupForCodeAlign target hook.
+      if (MCAlignFragment *AF = dyn_cast<MCAlignFragment>(&Frag)) {
+        if (Sec.UseCodeAlign() && AF->hasEmitNops()) {
----------------
Can you move MCAlignFragment into the switch as well?


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