RISCV: Remove unneeded relocations when mixing relax/norelax code

RISCVAsmBackend::ForceRelocs is introduced to create required
relocations in mixed relax/norelax code. However, it also leads to
redundant relocations.

Consider the example adapted from llvm#77436
```
.option norelax
j label
// For assembly input, RISCVAsmParser::ParseInstruction sets ForceRelocs (https://reviews.llvm.org/D46423).
// For direct object emission, RISCVELFStreamer sets ForceRelocs (llvm#77436)
.option relax
call foo  // linker-relaxable

.option norelax
j label   // redundant relocation due to ForceRelocs
.option relax

label:
```

Follow-up to llvm#140494

Author: MaskRay

llvm/include/llvm/MC/MCAssembler.h

@@ -64,6 +64,7 @@ class MCAssembler {
   std::unique_ptr<MCObjectWriter> Writer;
 
   bool HasLayout = false;
+  bool HasFinalLayout = false;
   bool RelaxAll = false;
 
   SectionListType Sections;
@@ -197,6 +198,7 @@ class MCAssembler {
   void layout();
 
   bool hasLayout() const { return HasLayout; }
+  bool hasFinalLayout() const { return HasFinalLayout; }
   bool getRelaxAll() const { return RelaxAll; }
   void setRelaxAll(bool Value) { RelaxAll = Value; }
 

llvm/lib/MC/MCAssembler.cpp

@@ -897,6 +897,8 @@ void MCAssembler::layout() {
   // example, to set the index fields in the symbol data).
   getWriter().executePostLayoutBinding(*this);
 
+  this->HasFinalLayout = true;
+
   // Evaluate and apply the fixups, generating relocation entries as necessary.
   for (MCSection &Sec : *this) {
     for (MCFragment &Frag : Sec) {

llvm/lib/MC/MCExpr.cpp

@@ -390,6 +390,12 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm,
       unsigned Count;
       if (DF) {
         Displacement += DF->getContents().size();
+      } else if (auto *RF = dyn_cast<MCRelaxableFragment>(FI);
+                 RF && Asm->hasFinalLayout()) {
+        // A relaxable fragment has indeterminate size before finishLayout.
+        // Afterwards (during relocation generation), it can be treated as a
+        // data fragment.
+        Displacement += RF->getContents().size();
       } else if (auto *AF = dyn_cast<MCAlignFragment>(FI);
                  AF && Layout && AF->hasEmitNops() &&
                  !Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign(

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

@@ -2820,21 +2820,6 @@ bool RISCVAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
 bool RISCVAsmParser::parseInstruction(ParseInstructionInfo &Info,
                                       StringRef Name, SMLoc NameLoc,
                                       OperandVector &Operands) {
-  // Ensure that if the instruction occurs when relaxation is enabled,
-  // relocations are forced for the file. Ideally this would be done when there
-  // is enough information to reliably determine if the instruction itself may
-  // cause relaxations. Unfortunately instruction processing stage occurs in the
-  // same pass as relocation emission, so it's too late to set a 'sticky bit'
-  // for the entire file.
-  if (getSTI().hasFeature(RISCV::FeatureRelax)) {
-    auto *Assembler = getTargetStreamer().getStreamer().getAssemblerPtr();
-    if (Assembler != nullptr) {
-      RISCVAsmBackend &MAB =
-          static_cast<RISCVAsmBackend &>(Assembler->getBackend());
-      MAB.setForceRelocs();
-    }
-  }
-
   // Apply mnemonic aliases because the destination mnemonic may have require
   // custom operand parsing. The generic tblgen'erated code does this later, at
   // the start of MatchInstructionImpl(), but that's too late for custom

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

@@ -104,9 +104,9 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
   return Infos[Kind - FirstTargetFixupKind];
 }
 
-// If linker relaxation is enabled, or the relax option had previously been
-// enabled, always emit relocations even if the fixup can be resolved. This is
-// necessary for correctness as offsets may change during relaxation.
+// If linker relaxation is enabled, always emit relocations even if the fixup
+// can be resolved. This is necessary for correctness as offsets may change
+// during relaxation.
 bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
                                             const MCFixup &Fixup,
                                             const MCValue &Target,
@@ -124,7 +124,7 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm,
     break;
   }
 
-  return STI->hasFeature(RISCV::FeatureRelax) || ForceRelocs;
+  return STI->hasFeature(RISCV::FeatureRelax);
 }
 
 bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &,
@@ -570,6 +570,18 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
   }
 }
 
+bool RISCVAsmBackend::isPCRelFixupResolved(const MCAssembler &Asm,
+                                           const MCSymbol *SymA,
+                                           const MCFragment &F) {
+  if (!PCRelTemp)
+    PCRelTemp = Asm.getContext().createTempSymbol();
+  PCRelTemp->setFragment(const_cast<MCFragment *>(&F));
+  MCValue Res;
+  MCExpr::evaluateSymbolicAdd(&Asm, false, MCValue::get(SymA),
+                              MCValue::get(nullptr, PCRelTemp), Res);
+  return !Res.getSubSym();
+}
+
 bool RISCVAsmBackend::evaluateTargetFixup(
     const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF,
     const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) {
@@ -613,7 +625,9 @@ bool RISCVAsmBackend::evaluateTargetFixup(
   Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant();
   Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset();
 
-  return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20;
+  if (AUIPCFixup->getTargetKind() != RISCV::fixup_riscv_pcrel_hi20)
+    return false;
+  return isPCRelFixupResolved(Asm, AUIPCTarget.getAddSym(), *AUIPCDF);
 }
 
 bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
@@ -659,11 +673,14 @@ bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F,
     return false;
   }
 
+  if (IsResolved &&
+      (getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel))
+    IsResolved = isPCRelFixupResolved(Asm, Target.getAddSym(), F);
   IsResolved = MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue,
                                       IsResolved, STI);
   // If linker relaxation is enabled and supported by the current relocation,
   // append a RELAX relocation.
-  if (Fixup.needsRelax()) {
+  if (!IsResolved && Fixup.needsRelax()) {
     auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
     Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr),
                                      FixedValueA);

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h

@@ -25,16 +25,18 @@ class RISCVAsmBackend : public MCAsmBackend {
   const MCSubtargetInfo &STI;
   uint8_t OSABI;
   bool Is64Bit;
-  bool ForceRelocs = false;
   const MCTargetOptions &TargetOptions;
+  // Temporary symbol used to check whether a PC-relative fixup is resolved.
+  MCSymbol *PCRelTemp = nullptr;
+
+  bool isPCRelFixupResolved(const MCAssembler &Asm, const MCSymbol *SymA,
+                            const MCFragment &F);
 
 public:
   RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
                   const MCTargetOptions &Options);
   ~RISCVAsmBackend() override = default;
 
-  void setForceRelocs() { ForceRelocs = true; }
-
   // Return Size with extra Nop Bytes for alignment directive in code section.
   bool shouldInsertExtraNopBytesForCodeAlign(const MCAlignFragment &AF,
                                              unsigned &Size) override;

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

@@ -34,13 +34,6 @@ RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
   setTargetABI(RISCVABI::computeTargetABI(STI.getTargetTriple(), Features,
                                           MAB.getTargetOptions().getABIName()));
   setFlagsFromFeatures(STI);
-  // `j label` in `.option norelax; j label; .option relax; ...; label:` needs a
-  // relocation to ensure the jump target is correct after linking. This is due
-  // to a limitation that shouldForceRelocation has to make the decision upfront
-  // without knowing a possibly future .option relax. When RISCVAsmParser is used,
-  // its ParseInstruction may call setForceRelocs as well.
-  if (STI.hasFeature(RISCV::FeatureRelax))
-    static_cast<RISCVAsmBackend &>(MAB).setForceRelocs();
 }
 
 RISCVELFStreamer &RISCVTargetELFStreamer::getStreamer() {

llvm/test/CodeGen/RISCV/option-relax-relocation.ll

@@ -2,7 +2,7 @@
 ;; after linker relaxation. See https://github.com/ClangBuiltLinux/linux/issues/1965
 
 ; RUN: llc -mtriple=riscv64 -mattr=-relax -filetype=obj < %s \
-; RUN:     | llvm-objdump -d -r - | FileCheck %s
+; RUN:     | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,NORELAX
 ; RUN: llc -mtriple=riscv64 -mattr=+relax -filetype=obj < %s \
 ; RUN:     | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,RELAX
 
@@ -12,14 +12,20 @@
 ; CHECK-NEXT:           R_RISCV_CALL_PLT     f
 ; RELAX-NEXT:           R_RISCV_RELAX        *ABS*
 ; CHECK-NEXT:   jalr    ra
+; CHECK-NEXT:   j       {{.*}}
+; CHECK-NEXT:   j       {{.*}}
+; NORELAX-NEXT: li      a0, 0x0
+; RELAX-NEXT:           R_RISCV_JAL  .L0
 
 define dso_local noundef signext i32 @main() local_unnamed_addr #0 {
 entry:
-  callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"() #2
+  callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"()
           to label %asm.fallthrough [label %label]
 
 asm.fallthrough:                                  ; preds = %entry
   tail call void @f()
+  callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"()
+          to label %asm.fallthrough [label %label]
   br label %label
 
 label:                                            ; preds = %asm.fallthrough, %entry

llvm/test/MC/RISCV/option-relax.s

@@ -21,15 +21,11 @@
 
 # CHECK-INST: call foo
 # CHECK-RELOC: R_RISCV_CALL_PLT foo 0x0
-# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
+# CHECK-RELOC-NOT: R_RISCV
 call foo
 
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
 .dword .L2-.L1
-# CHECK-RELOC-NEXT: R_RISCV_JAL
 jal zero, .L1
-# CHECK-RELOC-NEXT: R_RISCV_BRANCH
 beq s1, s1, .L1
 
 .L2:
@@ -41,8 +37,6 @@ beq s1, s1, .L1
 # CHECK-RELOC-NEXT: R_RISCV_RELAX - 0x0
 call bar
 
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
 .dword .L2-.L1
 # CHECK-RELOC-NEXT: R_RISCV_JAL
 jal zero, .L1
@@ -57,8 +51,6 @@ beq s1, s1, .L1
 # CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0
 call baz
 
-# CHECK-RELOC-NEXT: R_RISCV_ADD64
-# CHECK-RELOC-NEXT: R_RISCV_SUB64
 .dword .L2-.L1
 # CHECK-RELOC-NEXT: R_RISCV_JAL
 jal zero, .L1