Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release/18.x: [Mips] Fix unable to handle inline assembly ends with compat-branch o… (#77291) #82870

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Feb 24, 2024

Backport 96abee5

Requested by: @brad0

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 24, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 24, 2024

@yingopq What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 24, 2024

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-lld-elf

Author: None (llvmbot)

Changes

Backport 96abee5

Requested by: @brad0


Full diff: https://github.com/llvm/llvm-project/pull/82870.diff

7 Files Affected:

  • (modified) lld/test/ELF/mips-pc-relocs.s (+10-8)
  • (modified) llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (+77-1)
  • (added) llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll (+71)
  • (added) llvm/test/MC/Mips/forbidden-slot.s (+18)
  • (modified) llvm/test/MC/Mips/mips32r6/relocations.s (+11-11)
  • (modified) llvm/test/MC/Mips/mips64r6/relocations.s (+13-13)
  • (modified) llvm/test/MC/Mips/relocation.s (+2-2)
diff --git a/lld/test/ELF/mips-pc-relocs.s b/lld/test/ELF/mips-pc-relocs.s
index 5e7dbed94ca7c4..7d23f9d7469a48 100644
--- a/lld/test/ELF/mips-pc-relocs.s
+++ b/lld/test/ELF/mips-pc-relocs.s
@@ -40,11 +40,13 @@ __start:
 #                                         ^-- (0x20020-0x20000)>>2
 # CHECK-NEXT:    20004:       beqc    $5, $6, 0x20020
 #                                             ^-- (0x20020-4-0x20004)>>2
-# CHECK-NEXT:    20008:       beqzc   $9, 0x20020
-#                                         ^-- (0x20020-4-0x20008)>>2
-# CHECK-NEXT:    2000c:       bc      0x20020
-#                                     ^-- (0x20020-4-0x2000c)>>2
-# CHECK-NEXT:    20010:       aluipc  $2, 0
-#                                         ^-- %hi(0x20020-0x20010)
-# CHECK-NEXT:    20014:       addiu   $2, $2, 12
-#                                             ^-- %lo(0x20020-0x20014)
+# CHECK-NEXT:    20008:       nop
+# CHECK-NEXT:    2000c:       beqzc   $9, 0x20020
+#                                         ^-- (0x20020-4-0x2000c)>>2
+# CHECK-NEXT:    20010:       nop
+# CHECK-NEXT:    20014:       bc      0x20020
+#                                     ^-- (0x20020-4-0x200014)>>2
+# CHECK-NEXT:    20018:       aluipc  $2, 0
+#                                         ^-- %hi(0x20020-0x20018)
+# CHECK-NEXT:    2001c:       addiu   $2, $2, 4
+#                                             ^-- %lo(0x20020-0x2001c)
diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
index 36aab383da68d2..9d6e8dc573a8d1 100644
--- a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -150,6 +150,7 @@ class MipsAsmParser : public MCTargetAsmParser {
   bool IsLittleEndian;
   bool IsPicEnabled;
   bool IsCpRestoreSet;
+  bool CurForbiddenSlotAttr;
   int CpRestoreOffset;
   unsigned GPReg;
   unsigned CpSaveLocation;
@@ -552,6 +553,7 @@ class MipsAsmParser : public MCTargetAsmParser {
 
     CurrentFn = nullptr;
 
+    CurForbiddenSlotAttr = false;
     IsPicEnabled = getContext().getObjectFileInfo()->isPositionIndependent();
 
     IsCpRestoreSet = false;
@@ -723,6 +725,16 @@ class MipsAsmParser : public MCTargetAsmParser {
     return getSTI().hasFeature(Mips::FeatureGINV);
   }
 
+  bool hasForbiddenSlot(const MCInstrDesc &MCID) const {
+    return !inMicroMipsMode() && (MCID.TSFlags & MipsII::HasForbiddenSlot);
+  }
+
+  bool SafeInForbiddenSlot(const MCInstrDesc &MCID) const {
+    return !(MCID.TSFlags & MipsII::IsCTI);
+  }
+
+  void onEndOfFile() override;
+
   /// Warn if RegIndex is the same as the current AT.
   void warnIfRegIndexIsAT(unsigned RegIndex, SMLoc Loc);
 
@@ -2307,7 +2319,41 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
 
   bool FillDelaySlot =
       MCID.hasDelaySlot() && AssemblerOptions.back()->isReorder();
-  if (FillDelaySlot)
+
+  // Get previous instruction`s forbidden slot attribute and
+  // whether set reorder.
+  bool PrevForbiddenSlotAttr = CurForbiddenSlotAttr;
+
+  // Flag represents we set reorder after nop.
+  bool SetReorderAfterNop = false;
+
+  // If previous instruction has forbidden slot and .set reorder
+  // is active and current instruction is CTI.
+  // Then emit a NOP after it.
+  if (PrevForbiddenSlotAttr && !SafeInForbiddenSlot(MCID)) {
+    TOut.emitEmptyDelaySlot(false, IDLoc, STI);
+    // When 'FillDelaySlot' is true, the existing logic will add
+    // noreorder before instruction and reorder after it. So there
+    // need exclude this case avoiding two '.set reorder'.
+    // The format of the first case is:
+    // .set noreorder
+    // bnezc
+    // nop
+    // .set reorder
+    if (AssemblerOptions.back()->isReorder() && !FillDelaySlot) {
+      SetReorderAfterNop = true;
+      TOut.emitDirectiveSetReorder();
+    }
+  }
+
+  // Save current instruction`s forbidden slot and whether set reorder.
+  // This is the judgment condition for whether to add nop.
+  // We would add a couple of '.set noreorder' and '.set reorder' to
+  // wrap the current instruction and the next instruction.
+  CurForbiddenSlotAttr =
+      hasForbiddenSlot(MCID) && AssemblerOptions.back()->isReorder();
+
+  if (FillDelaySlot || CurForbiddenSlotAttr)
     TOut.emitDirectiveSetNoReorder();
 
   MacroExpanderResultTy ExpandResult =
@@ -2322,6 +2368,17 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
     return true;
   }
 
+  // When current instruction was not CTI, recover reorder state.
+  // The format of the second case is:
+  // .set noreoder
+  // bnezc
+  // add
+  // .set reorder
+  if (PrevForbiddenSlotAttr && !SetReorderAfterNop && !FillDelaySlot &&
+      AssemblerOptions.back()->isReorder()) {
+    TOut.emitDirectiveSetReorder();
+  }
+
   // We know we emitted an instruction on the MER_NotAMacro or MER_Success path.
   // If we're in microMIPS mode then we must also set EF_MIPS_MICROMIPS.
   if (inMicroMipsMode()) {
@@ -2331,6 +2388,14 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
 
   // If this instruction has a delay slot and .set reorder is active,
   // emit a NOP after it.
+  // The format of the third case is:
+  // .set noreorder
+  // bnezc
+  // nop
+  // .set noreorder
+  // j
+  // nop
+  // .set reorder
   if (FillDelaySlot) {
     TOut.emitEmptyDelaySlot(hasShortDelaySlot(Inst), IDLoc, STI);
     TOut.emitDirectiveSetReorder();
@@ -2356,6 +2421,17 @@ bool MipsAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
   return false;
 }
 
+void MipsAsmParser::onEndOfFile() {
+  MipsTargetStreamer &TOut = getTargetStreamer();
+  SMLoc IDLoc = SMLoc();
+  // If has pending forbidden slot, fill nop and recover reorder.
+  if (CurForbiddenSlotAttr) {
+    TOut.emitEmptyDelaySlot(false, IDLoc, STI);
+    if (AssemblerOptions.back()->isReorder())
+      TOut.emitDirectiveSetReorder();
+  }
+}
+
 MipsAsmParser::MacroExpanderResultTy
 MipsAsmParser::tryExpandInstruction(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out,
                                     const MCSubtargetInfo *STI) {
diff --git a/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll b/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll
new file mode 100644
index 00000000000000..3e6826f0cd1d13
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/llvm-ir/forbidden-slot-ir.ll
@@ -0,0 +1,71 @@
+target triple = "mipsisa32r6el-unknown-linux-gnu"
+
+; RUN: llc -filetype=asm %s -o - | FileCheck %s --check-prefix=MIPSELR6
+; Function Attrs: noinline nounwind optnone uwtable
+define i1 @foo0() nounwind {
+; MIPSELR6:      bnezc	$1, $BB0_2
+; MIPSELR6-NEXT: nop
+; MIPSELR6:      jr	$ra
+entry:
+  %0 = icmp eq i32 0, 1
+  br i1 %0, label %2, label %3
+  ret i1 %0
+2:                                                
+  ret i1 %0
+3:                                                
+  ret i1 %0
+}
+
+define i32 @foo1() nounwind {
+; MIPSELR6:      addiu	$2, $2, 1
+; MIPSELR6-NEXT: .set	noreorder
+; MIPSELR6-NEXT: beqzc	$2, $tmp0
+; MIPSELR6-NEXT: nop
+; MIPSELR6-NEXT: .set	reorder
+; MIPSELR6:      jrc	$ra
+entry:
+  %0 = tail call i32 asm "1: addiu $0, $0, 1; beqzc $0, 1b", "=r"() nounwind
+  ret i32 %0
+}
+
+define i32 @foo2() nounwind {
+; MIPSELR6:      .set	push
+; MIPSELR6-NEXT: .set	at
+; MIPSELR6-NEXT: .set	macro
+; MIPSELR6-NEXT: .set	reorder
+; MIPSELR6:      .set	noreorder
+; MIPSELR6-NEXT: beqzc	$9, End
+; MIPSELR6-NEXT: nop
+; MIPSELR6-NEXT: .set	reorder
+; MIPSELR6:      addiu	$9, $9, 1
+entry:
+  %0 = tail call i32 asm "beqzc $$t1, End", "=r"() nounwind
+  %1 = tail call i32 asm "addiu $$t1, $$t1, 1", "=r"() nounwind
+  %2 = add nsw i32 %1, %0
+  ret i32 %2
+}
+
+define i32 @foo3() nounwind {
+; MIPSELR6:      addiu	$2, $2, 1
+; MIPSELR6-NEXT: .set	noreorder
+; MIPSELR6-NEXT: beqzc	$2, $tmp1
+; MIPSELR6-NEXT: nop
+; MIPSELR6-NEXT: .set	noreorder
+; MIPSELR6-NEXT: j	End
+; MIPSELR6-NEXT: nop
+; MIPSELR6-NEXT: .set	reorder
+entry:
+  %0 = tail call i32 asm "1: addiu $0, $0, 1; beqzc $0, 1b; j End", "=r"() nounwind
+  ret i32 %0
+}
+
+define i32 @foo4() nounwind {
+; MIPSELR6:      addiu	$2, $2, 1
+; MIPSELR6-NEXT: .set	noreorder
+; MIPSELR6-NEXT: beqzc  $2, $tmp2
+; MIPSELR6-NEXT: addiu	$2, $2, 1
+; MIPSELR6-NEXT: .set	reorder
+entry:
+  %0 = tail call i32 asm "1: addiu $0, $0, 1; beqzc $0, 1b; addiu $0, $0, 1", "=r"() nounwind
+  ret i32 %0
+}
diff --git a/llvm/test/MC/Mips/forbidden-slot.s b/llvm/test/MC/Mips/forbidden-slot.s
new file mode 100644
index 00000000000000..da98e70561695f
--- /dev/null
+++ b/llvm/test/MC/Mips/forbidden-slot.s
@@ -0,0 +1,18 @@
+# RUN: llvm-mc -assemble -mcpu=mips64r6 -arch=mips64el -filetype=obj %s -o tmp.o
+# RUN: llvm-objdump -d tmp.o | FileCheck %s --check-prefix=MIPSELR6
+
+# MIPSELR6:      0000000000000000 <aaa>:
+# MIPSELR6-NEXT: beqzc	$13, 0x0 <aaa>
+# MIPSELR6-NEXT: b	0x0 <aaa>
+# MIPSELR6:      0000000000000008 <bbb>:
+# MIPSELR6-NEXT: beqzc	$13, 0x8 <bbb>
+# MIPSELR6-NEXT: nop    <aaa>
+# MIPSELR6:      b	0x8 <bbb>
+	.set noreorder
+aaa:
+	beqzc $t1, aaa
+	b aaa
+	.set reorder
+bbb:
+	beqzc $t1, bbb
+	b bbb
diff --git a/llvm/test/MC/Mips/mips32r6/relocations.s b/llvm/test/MC/Mips/mips32r6/relocations.s
index dfd75e633bc2f7..8d4464bbbed77a 100644
--- a/llvm/test/MC/Mips/mips32r6/relocations.s
+++ b/llvm/test/MC/Mips/mips32r6/relocations.s
@@ -52,17 +52,17 @@
 # CHECK-ELF: Relocations [
 # CHECK-ELF:     0x0 R_MIPS_PC19_S2 bar
 # CHECK-ELF:     0x4 R_MIPS_PC16 bar
-# CHECK-ELF:     0x8 R_MIPS_PC16 bar
-# CHECK-ELF:     0xC R_MIPS_PC21_S2 bar
-# CHECK-ELF:     0x10 R_MIPS_PC21_S2 bar
-# CHECK-ELF:     0x14 R_MIPS_PC26_S2 bar
-# CHECK-ELF:     0x18 R_MIPS_PC26_S2 bar
-# CHECK-ELF:     0x1C R_MIPS_PCHI16 bar
-# CHECK-ELF:     0x20 R_MIPS_PCLO16 bar
-# CHECK-ELF:     0x24 R_MIPS_PC19_S2 bar
-# CHECK-ELF:     0x28 R_MIPS_PC19_S2 bar
-# CHECK-ELF:     0x2C R_MIPS_LO16 bar
-# CHECK-ELF:     0x30 R_MIPS_LO16 bar
+# CHECK-ELF:     0xC R_MIPS_PC16 bar
+# CHECK-ELF:     0x14 R_MIPS_PC21_S2 bar
+# CHECK-ELF:     0x1C R_MIPS_PC21_S2 bar
+# CHECK-ELF:     0x24 R_MIPS_PC26_S2 bar
+# CHECK-ELF:     0x28 R_MIPS_PC26_S2 bar
+# CHECK-ELF:     0x2C R_MIPS_PCHI16 bar
+# CHECK-ELF:     0x30 R_MIPS_PCLO16 bar
+# CHECK-ELF:     0x34 R_MIPS_PC19_S2 bar
+# CHECK-ELF:     0x38 R_MIPS_PC19_S2 bar
+# CHECK-ELF:     0x3C R_MIPS_LO16 bar
+# CHECK-ELF:     0x40 R_MIPS_LO16 bar
 # CHECK-ELF: ]
 
   addiupc   $2,bar
diff --git a/llvm/test/MC/Mips/mips64r6/relocations.s b/llvm/test/MC/Mips/mips64r6/relocations.s
index 8353ec019a3ca8..8b02be37284aaa 100644
--- a/llvm/test/MC/Mips/mips64r6/relocations.s
+++ b/llvm/test/MC/Mips/mips64r6/relocations.s
@@ -59,19 +59,19 @@
 # CHECK-ELF: Relocations [
 # CHECK-ELF:     0x0 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
 # CHECK-ELF:     0x4 R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0x8 R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0xC R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0x10 R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0x14 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0x18 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
-# CHECK-ELF:     0x1C R_MIPS_PCHI16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x20 R_MIPS_PCLO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x24 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x28 R_MIPS_PC18_S3/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x2C R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x30 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x34 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
-# CHECK-ELF:     0x38 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0xC R_MIPS_PC16/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF:     0x14 R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF:     0x1C R_MIPS_PC21_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF:     0x24 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF:     0x28 R_MIPS_PC26_S2/R_MIPS_NONE/R_MIPS_NONE bar 0xFFFFFFFFFFFFFFFC
+# CHECK-ELF:     0x2C R_MIPS_PCHI16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x30 R_MIPS_PCLO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x34 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x38 R_MIPS_PC18_S3/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x3C R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x40 R_MIPS_PC19_S2/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x44 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
+# CHECK-ELF:     0x48 R_MIPS_LO16/R_MIPS_NONE/R_MIPS_NONE bar 0x0
 # CHECK-ELF: ]
 
   addiupc   $2,bar
diff --git a/llvm/test/MC/Mips/relocation.s b/llvm/test/MC/Mips/relocation.s
index 9c8bb657ea68c1..a92c62744fcaa5 100644
--- a/llvm/test/MC/Mips/relocation.s
+++ b/llvm/test/MC/Mips/relocation.s
@@ -237,7 +237,7 @@ baz:    .long foo                          // RELOC: R_MIPS_32 foo
                                            // ENCLE: addiu $2, $3, %tprel_lo(foo) # encoding: [A,A,0x62,0x24]
                                            // FIXUP: # fixup A - offset: 0, value: %tprel_lo(foo), kind: fixup_Mips_TPREL_LO
 
-// DATA-NEXT:  00C0: D85FFFFF CBFFFFFF EC580000 EC480000
+// DATA-NEXT:  00C0: D85FFFFF 00000000 CBFFFFFF EC580000
                                            // ?????: R_MIPS_GLOB_DAT foo
         .set mips32r6
         beqzc $2, foo                      // RELOC: R_MIPS_PC21_S2 foo
@@ -262,7 +262,7 @@ baz:    .long foo                          // RELOC: R_MIPS_32 foo
                                            // ENCLE: lwpc $2, foo # encoding: [A,A,0b01001AAA,0xec]
                                            // FIXUP: # fixup A - offset: 0, value: foo, kind: fixup_MIPS_PC19_S2
 
-// DATA-NEXT:  00D0: 24620000 24620000 00000000
+// DATA-NEXT:  00D0: EC480000 24620000 24620000 00000000
         addiu $2, $3, %pcrel_hi(foo)       // RELOC: R_MIPS_PCHI16 foo
                                            // ENCBE: addiu $2, $3, %pcrel_hi(foo) # encoding: [0x24,0x62,A,A]
                                            // ENCLE: addiu $2, $3, %pcrel_hi(foo) # encoding: [A,A,0x62,0x24]

@yingopq
Copy link
Contributor

yingopq commented Feb 27, 2024

@yingopq What do you think about merging this PR to the release branch?

Yes, I agree.

llvm#77291)

…n MIPS

Modify:
Add a global variable 'CurForbiddenSlotAttr' to save current
instruction's forbidden slot and whether set reorder. This is the
judgment condition for whether to add nop. We would add a couple of
'.set noreorder' and '.set reorder' to wrap the current instruction and
the next instruction.
Then we can get previous instruction`s forbidden slot attribute and
whether set reorder by 'CurForbiddenSlotAttr'.
If previous instruction has forbidden slot and .set reorder is active
and current instruction is CTI. Then emit a NOP after it.

Fix llvm#61045.

Because https://reviews.llvm.org/D158589 was 'Needs Review' state, not
ending, so we commit pull request again.

(cherry picked from commit 96abee5)
@tstellar tstellar merged commit e2182a6 into llvm:release/18.x Feb 27, 2024
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lld:ELF lld mc Machine (object) code
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants