[PATCH] D132818: [LoongArch] Fixup value adjustment in applyFixup

Ray Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 30 20:23:05 PDT 2022


wangleiat added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchAsmBackend.cpp:189-190
   MCFixupKindInfo Info = getFixupKindInfo(Kind);
-  // TODO: Apply any target-specific value adjustments.
+  if (!Value)
+    return; // Doesn't change encoding.
+  // Apply any target-specific value adjustments.
----------------
xen0n wrote:
> This is placed here due to review comment on D132323, does the situation change enough to make this change appropriate?
Personal preference, just put the processing of `Value` together (Increases the readability of the code). Maybe it was put in the front at the time because of performance? If you think this is not good, I can modify it back.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCCodeEmitter.cpp:266
         break;
+      case LoongArch::B:
+        FixupKind = LoongArch::fixup_loongarch_b26;
----------------
xen0n wrote:
> Do we want to also handle `BL` here? Given it's the only other insn of the format.
> Do we want to also handle `BL` here? Given it's the only other insn of the format.

It is not needed. This is because `BL` are handled specifically in both the `LowerCall` phase and `ASMParser`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132818



More information about the llvm-commits mailing list