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

[SPARC][IAS] Add support for call dest, imm form #119078

Merged
merged 3 commits into from
Dec 17, 2024
Merged

Conversation

koachan
Copy link
Contributor

@koachan koachan commented Dec 7, 2024

This follows GCC behavior of allowing a trailing immediate, that is ignored by the assembler.

This follows the GCC behavior of allowing a trailing immediate, that is
ignored by the assembler.
@llvmbot llvmbot added backend:Sparc mc Machine (object) code labels Dec 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 7, 2024

@llvm/pr-subscribers-backend-sparc

@llvm/pr-subscribers-mc

Author: Koakuma (koachan)

Changes

This follows GCC behavior of allowing a trailing immediate, that is ignored by the assembler.


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

4 Files Affected:

  • (modified) llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp (+22-4)
  • (modified) llvm/lib/Target/Sparc/DelaySlotFiller.cpp (+10-1)
  • (modified) llvm/lib/Target/Sparc/SparcInstrInfo.td (+26)
  • (modified) llvm/test/MC/Sparc/sparc-ctrl-instructions.s (+21)
diff --git a/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp b/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
index 1f427638e099c3..bc239480baa891 100644
--- a/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
+++ b/llvm/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp
@@ -220,6 +220,7 @@ class SparcOperand : public MCParsedAsmOperand {
     k_MemoryImm,
     k_ASITag,
     k_PrefetchTag,
+    k_TailRelocSym, // Special kind of immediate for TLS relocation purposes.
   } Kind;
 
   SMLoc StartLoc, EndLoc;
@@ -265,7 +266,7 @@ class SparcOperand : public MCParsedAsmOperand {
   bool isMembarTag() const { return Kind == k_Immediate; }
   bool isASITag() const { return Kind == k_ASITag; }
   bool isPrefetchTag() const { return Kind == k_PrefetchTag; }
-  bool isTailRelocSym() const { return Kind == k_Immediate; }
+  bool isTailRelocSym() const { return Kind == k_TailRelocSym; }
 
   bool isCallTarget() const {
     if (!isImm())
@@ -354,6 +355,11 @@ class SparcOperand : public MCParsedAsmOperand {
     return Prefetch;
   }
 
+  const MCExpr *getTailRelocSym() const {
+    assert((Kind == k_TailRelocSym) && "Invalid access!");
+    return Imm.Val;
+  }
+
   /// getStartLoc - Get the location of the first token of this operand.
   SMLoc getStartLoc() const override {
     return StartLoc;
@@ -380,6 +386,9 @@ class SparcOperand : public MCParsedAsmOperand {
     case k_PrefetchTag:
       OS << "Prefetch tag: " << getPrefetchTag() << "\n";
       break;
+    case k_TailRelocSym:
+      OS << "TailReloc: " << getTailRelocSym() << "\n";
+      break;
     }
   }
 
@@ -454,7 +463,7 @@ class SparcOperand : public MCParsedAsmOperand {
 
   void addTailRelocSymOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
-    addExpr(Inst, getImm());
+    addExpr(Inst, getTailRelocSym());
   }
 
   static std::unique_ptr<SparcOperand> CreateToken(StringRef Str, SMLoc S) {
@@ -503,6 +512,15 @@ class SparcOperand : public MCParsedAsmOperand {
     return Op;
   }
 
+  static std::unique_ptr<SparcOperand> CreateTailRelocSym(const MCExpr *Val,
+                                                          SMLoc S, SMLoc E) {
+    auto Op = std::make_unique<SparcOperand>(k_TailRelocSym);
+    Op->Imm.Val = Val;
+    Op->StartLoc = S;
+    Op->EndLoc = E;
+    return Op;
+  }
+
   static bool MorphToIntPairReg(SparcOperand &Op) {
     unsigned Reg = Op.getReg();
     assert(Op.Reg.Kind == rk_IntReg);
@@ -1070,7 +1088,7 @@ ParseStatus SparcAsmParser::parseTailRelocSym(OperandVector &Operands) {
   };
 
   if (getLexer().getKind() != AsmToken::Percent)
-    return Error(getLoc(), "expected '%' for operand modifier");
+    return ParseStatus::NoMatch;
 
   const AsmToken Tok = Parser.getTok();
   getParser().Lex(); // Eat '%'
@@ -1099,7 +1117,7 @@ ParseStatus SparcAsmParser::parseTailRelocSym(OperandVector &Operands) {
     return ParseStatus::Failure;
 
   const MCExpr *Val = adjustPICRelocation(VK, SubExpr);
-  Operands.push_back(SparcOperand::CreateImm(Val, S, E));
+  Operands.push_back(SparcOperand::CreateTailRelocSym(Val, S, E));
   return ParseStatus::Success;
 }
 
diff --git a/llvm/lib/Target/Sparc/DelaySlotFiller.cpp b/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
index 403c924769385c..a48e72dc7b69b7 100644
--- a/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
+++ b/llvm/lib/Target/Sparc/DelaySlotFiller.cpp
@@ -309,9 +309,13 @@ void Filler::insertCallDefsUses(MachineBasicBlock::iterator MI,
 
   switch(MI->getOpcode()) {
   default: llvm_unreachable("Unknown opcode.");
-  case SP::CALL: break;
+  case SP::CALL:
+  case SP::CALLi:
+    break;
   case SP::CALLrr:
   case SP::CALLri:
+  case SP::CALLrri:
+  case SP::CALLrii:
     assert(MI->getNumOperands() >= 2);
     const MachineOperand &Reg = MI->getOperand(0);
     assert(Reg.isReg() && "CALL first operand is not a register.");
@@ -372,8 +376,13 @@ bool Filler::needsUnimp(MachineBasicBlock::iterator I, unsigned &StructSize)
   switch (I->getOpcode()) {
   default: llvm_unreachable("Unknown call opcode.");
   case SP::CALL: structSizeOpNum = 1; break;
+  case SP::CALLi:
   case SP::CALLrr:
   case SP::CALLri: structSizeOpNum = 2; break;
+  case SP::CALLrri:
+  case SP::CALLrii:
+    structSizeOpNum = 3;
+    break;
   case SP::TLS_CALL: return false;
   case SP::TAIL_CALLri:
   case SP::TAIL_CALL: return false;
diff --git a/llvm/lib/Target/Sparc/SparcInstrInfo.td b/llvm/lib/Target/Sparc/SparcInstrInfo.td
index bb5b9f2d736f93..0b50eaa144844b 100644
--- a/llvm/lib/Target/Sparc/SparcInstrInfo.td
+++ b/llvm/lib/Target/Sparc/SparcInstrInfo.td
@@ -1036,6 +1036,19 @@ let Uses = [O6],
     let Inst{29-0} = disp;
   }
 
+  // call with trailing imm argument.
+  // The imm argument is discarded.
+  let isAsmParserOnly = 1 in {
+    def CALLi : InstSP<(outs), (ins calltarget:$disp, i32imm:$imm, variable_ops),
+                    "call $disp, $imm",
+                    [],
+                    IIC_jmp_or_call> {
+      bits<30> disp;
+      let op = 1;
+      let Inst{29-0} = disp;
+    }
+  }
+
   // indirect calls: special cases of JMPL.
   let isCodeGenOnly = 1, rd = 15 in {
     def CALLrr : F3_1<2, 0b111000,
@@ -1049,6 +1062,19 @@ let Uses = [O6],
                       [(call ADDRri:$addr)],
                       IIC_jmp_or_call>;
   }
+
+  let isAsmParserOnly = 1, rd = 15 in {
+    def CALLrri : F3_1<2, 0b111000,
+                      (outs), (ins (MEMrr $rs1, $rs2):$addr, i32imm:$imm, variable_ops),
+                      "call $addr, $imm",
+                      [],
+                      IIC_jmp_or_call>;
+    def CALLrii : F3_2<2, 0b111000,
+                      (outs), (ins (MEMri $rs1, $simm13):$addr, i32imm:$imm, variable_ops),
+                      "call $addr, $imm",
+                      [],
+                      IIC_jmp_or_call>;
+  }
 }
 
 // Section B.25 - Jump and Link Instruction
diff --git a/llvm/test/MC/Sparc/sparc-ctrl-instructions.s b/llvm/test/MC/Sparc/sparc-ctrl-instructions.s
index 109e7c9b011487..05a691de4f9669 100644
--- a/llvm/test/MC/Sparc/sparc-ctrl-instructions.s
+++ b/llvm/test/MC/Sparc/sparc-ctrl-instructions.s
@@ -5,23 +5,44 @@
         ! CHECK:              !   fixup A - offset: 0, value: foo, kind: fixup_sparc_call30
         call foo
 
+        ! CHECK: call foo, 0  ! encoding: [0b01AAAAAA,A,A,A]
+        ! CHECK:              !   fixup A - offset: 0, value: foo, kind: fixup_sparc_call30
+        call foo, 0
+
         ! CHECK: call %g1+%i2 ! encoding: [0x9f,0xc0,0x40,0x1a]
         call %g1 + %i2
 
+        ! CHECK: call %g1+%i2, 1 ! encoding: [0x9f,0xc0,0x40,0x1a]
+        call %g1 + %i2, 1
+
         ! CHECK: call %o1+8   ! encoding: [0x9f,0xc2,0x60,0x08]
         call %o1 + 8
 
+        ! CHECK: call %o1+8, 2   ! encoding: [0x9f,0xc2,0x60,0x08]
+        call %o1 + 8, 2
+
         ! CHECK: call %g1     ! encoding: [0x9f,0xc0,0x40,0x00]
         call %g1
 
+        ! CHECK: call %g1, 3     ! encoding: [0x9f,0xc0,0x40,0x00]
+        call %g1, 3
+
         ! CHECK: call %g1+%lo(sym)   ! encoding: [0x9f,0xc0,0b011000AA,A]
         ! CHECK-NEXT:                ! fixup A - offset: 0, value: %lo(sym), kind: fixup_sparc_lo10
         call %g1+%lo(sym)
 
+        ! CHECK: call %g1+%lo(sym), 4   ! encoding: [0x9f,0xc0,0b011000AA,A]
+        ! CHECK-NEXT:                ! fixup A - offset: 0, value: %lo(sym), kind: fixup_sparc_lo10
+        call %g1+%lo(sym), 4
+
         ! CHECK-LABEL: .Ltmp0:
         ! CHECK: call .Ltmp0-4 ! encoding: [0b01AAAAAA,A,A,A]
         call . - 4
 
+        ! CHECK-LABEL: .Ltmp1:
+        ! CHECK: call .Ltmp1-4, 5 ! encoding: [0b01AAAAAA,A,A,A]
+        call . - 4, 5
+
         ! CHECK: jmp %g1+%i2  ! encoding: [0x81,0xc0,0x40,0x1a]
         jmp %g1 + %i2
 

// call with trailing imm argument.
// The imm argument is discarded.
let isAsmParserOnly = 1 in {
def CALLi : InstSP<(outs), (ins calltarget:$disp, i32imm:$imm, variable_ops),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

variable_ops is only meaningful for SelectionDAG, it is not needed here.

def CALLi : InstSP<(outs), (ins calltarget:$disp, i32imm:$imm, variable_ops),
"call $disp, $imm",
[],
IIC_jmp_or_call> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scheduling info isn't needed either, these instructions will never go through CodeGen (won't they?).

llvm/lib/Target/Sparc/DelaySlotFiller.cpp Outdated Show resolved Hide resolved
@@ -5,23 +5,44 @@
! CHECK: ! fixup A - offset: 0, value: foo, kind: fixup_sparc_call30
call foo

! CHECK: call foo, 0 ! encoding: [0b01AAAAAA,A,A,A]
! CHECK: ! fixup A - offset: 0, value: foo, kind: fixup_sparc_call30
call foo, 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the immediate be an expression involving symbols?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, only a single bare integer is allowed in that position.

(An exception exists for the %tgd_call and %tldm_call expressions but that is already handled separately via TLS_CALL)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the current implementation does allow symbols though?
I'm not sure if it is worth issuing an error in this case given the obscurity of the feature, but it may be something to consider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, it does allow symbols :(
Do I need to write a custom parser to prevent that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to write a custom parser to prevent that?

I'll leave that up to you :)

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@koachan koachan merged commit ad64946 into llvm:main Dec 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:Sparc mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants