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

[Xtensa] Implement base CallConvention. #83280

Merged
merged 6 commits into from
Apr 23, 2024
Merged

Conversation

andreisfr
Copy link
Contributor

Implement base Calling Convention functionality. Implement stack load/store register operations.

llvm/lib/Target/Xtensa/XtensaUtils.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaUtils.h Outdated Show resolved Hide resolved
namespace llvm {

// Check address offset for load/store instructions
// The offset should be multiple of scale
Copy link
Contributor

Choose a reason for hiding this comment

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

It is documented in the header file, no need to duplicate the comment here.

llvm/lib/Target/Xtensa/XtensaUtils.h Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaCallingConv.td Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaOperators.td Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp Outdated Show resolved Hide resolved
@s-barannikov
Copy link
Contributor

I wouldn't mind if this PR included LowerCall implementation, it won't be much different from LowerFormalArguments/LowerReturn. The tests could then just call a function with the same signature so that we can see how arguments are recieved and passed.
The PR should also include XtensaFrameLowering changes, at least a part of them, so that the tests are representative.
There should be tests for passing arguments on the stack.

@@ -238,6 +239,11 @@ def L32R : RI16_Inst<0x01, (outs AR:$t), (ins L32Rtarget:$label),
let imm16 = label;
}

//extending loads
Copy link
Contributor

Choose a reason for hiding this comment

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

(not related to this PR)
I guess I'm late but still. It might make sense to split instruction definitions and patterns. E.g. extract pattern for i32 load from L32I definition and put it here together with other patterns for loads.
This will make instruction definitions more readable and patterns grouped so that you don't have to look through the entire file when searching for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s-barannikov , I prepared second commit with call lowering. Could you please take a look? We assume in the call lowering implementation that in most cases we should generate indirect call instruction and thus we need to load address constant from ConstantPool. So, I added base implementation of the ConstantPool to second commit. If it is not very complex to review both commits, I can merge them or drop second commit in other case.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is very difficult indeed. I left some comments and will revisit once they are addressed.

@gerekon
Copy link

gerekon commented Mar 1, 2024

Track in espressif#4

Implement base Calling Convention functionality. Implement stack
load/store register operations.
Implement lowering call operations. Implement stack pseudo
instructions ADJCALLSTACKDOWN, ADJCALLSTACKUP. Implement basic
support of the ConstantPool, integer and symbols constants placement
in literal section.
@llvmbot llvmbot added the mc Machine (object) code label Mar 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/pr-subscribers-mc

Author: Andrei Safronov (andreisfr)

Changes

Implement base Calling Convention functionality. Implement stack load/store register operations.


Patch is 102.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83280.diff

31 Files Affected:

  • (modified) llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp (+134-1)
  • (modified) llvm/lib/Target/Xtensa/CMakeLists.txt (+3)
  • (modified) llvm/lib/Target/Xtensa/MCTargetDesc/CMakeLists.txt (+1)
  • (modified) llvm/lib/Target/Xtensa/MCTargetDesc/XtensaMCTargetDesc.cpp (+24-2)
  • (added) llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.cpp (+87)
  • (added) llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.h (+46)
  • (modified) llvm/lib/Target/Xtensa/Xtensa.td (+6)
  • (modified) llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp (+188)
  • (modified) llvm/lib/Target/Xtensa/XtensaAsmPrinter.h (+13)
  • (added) llvm/lib/Target/Xtensa/XtensaCallingConv.td (+27)
  • (added) llvm/lib/Target/Xtensa/XtensaConstantPoolValue.cpp (+208)
  • (added) llvm/lib/Target/Xtensa/XtensaConstantPoolValue.h (+265)
  • (modified) llvm/lib/Target/Xtensa/XtensaFrameLowering.cpp (+20)
  • (modified) llvm/lib/Target/Xtensa/XtensaFrameLowering.h (+4)
  • (modified) llvm/lib/Target/Xtensa/XtensaISelDAGToDAG.cpp (+51-1)
  • (modified) llvm/lib/Target/Xtensa/XtensaISelLowering.cpp (+543)
  • (modified) llvm/lib/Target/Xtensa/XtensaISelLowering.h (+44)
  • (modified) llvm/lib/Target/Xtensa/XtensaInstrInfo.cpp (+136-1)
  • (modified) llvm/lib/Target/Xtensa/XtensaInstrInfo.h (+29)
  • (modified) llvm/lib/Target/Xtensa/XtensaInstrInfo.td (+54-1)
  • (modified) llvm/lib/Target/Xtensa/XtensaOperands.td (+1-1)
  • (added) llvm/lib/Target/Xtensa/XtensaOperators.td (+36)
  • (modified) llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp (+69-5)
  • (added) llvm/lib/Target/Xtensa/XtensaUtils.cpp (+59)
  • (added) llvm/lib/Target/Xtensa/XtensaUtils.h (+27)
  • (added) llvm/test/CodeGen/Xtensa/call.ll (+49)
  • (added) llvm/test/CodeGen/Xtensa/calling-conv.ll (+78)
  • (added) llvm/test/CodeGen/Xtensa/constantpool.ll (+28)
  • (added) llvm/test/CodeGen/Xtensa/stack-access.ll (+35)
  • (modified) llvm/test/MC/Xtensa/Core/invalid.s (-4)
  • (added) llvm/test/MC/Xtensa/directive-literal.s (+42)
diff --git a/llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp b/llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp
index 3f808298527f8f..9fd05ee96a9290 100644
--- a/llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp
+++ b/llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp
@@ -8,7 +8,9 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "MCTargetDesc/XtensaMCExpr.h"
 #include "MCTargetDesc/XtensaMCTargetDesc.h"
+#include "MCTargetDesc/XtensaTargetStreamer.h"
 #include "TargetInfo/XtensaTargetInfo.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -22,6 +24,7 @@
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/Casting.h"
 
@@ -35,6 +38,12 @@ class XtensaAsmParser : public MCTargetAsmParser {
 
   SMLoc getLoc() const { return getParser().getTok().getLoc(); }
 
+  XtensaTargetStreamer &getTargetStreamer() {
+    MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
+    return static_cast<XtensaTargetStreamer &>(TS);
+  }
+
+  bool ParseDirective(AsmToken DirectiveID) override;
   bool parseRegister(MCRegister &Reg, SMLoc &StartLoc, SMLoc &EndLoc) override;
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
@@ -45,6 +54,9 @@ class XtensaAsmParser : public MCTargetAsmParser {
   unsigned validateTargetOperandClass(MCParsedAsmOperand &Op,
                                       unsigned Kind) override;
 
+  bool processInstruction(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out,
+                          const MCSubtargetInfo *STI);
+
 // Auto-generated instruction matching functions
 #define GET_ASSEMBLER_HEADER
 #include "XtensaGenAsmMatcher.inc"
@@ -62,6 +74,7 @@ class XtensaAsmParser : public MCTargetAsmParser {
     return ParseStatus::NoMatch;
   }
   ParseStatus parsePCRelTarget(OperandVector &Operands);
+  bool parseLiteralDirective(SMLoc L);
 
 public:
   enum XtensaMatchResultTy {
@@ -148,7 +161,13 @@ struct XtensaOperand : public MCParsedAsmOperand {
 
   bool isImm12() const { return isImm(-2048, 2047); }
 
-  bool isImm12m() const { return isImm(-2048, 2047); }
+  // Convert MOVI to literal load, when immediate is not in range (-2048, 2047)
+  bool isImm12m() const {
+    if (Kind == Immediate)
+      return true;
+
+    return false;
+  }
 
   bool isOffset4m32() const {
     return isImm(0, 60) &&
@@ -348,6 +367,67 @@ static SMLoc RefineErrorLoc(const SMLoc Loc, const OperandVector &Operands,
   return Loc;
 }
 
+bool XtensaAsmParser::processInstruction(MCInst &Inst, SMLoc IDLoc,
+                                         MCStreamer &Out,
+                                         const MCSubtargetInfo *STI) {
+  Inst.setLoc(IDLoc);
+  const unsigned Opcode = Inst.getOpcode();
+  switch (Opcode) {
+  case Xtensa::L32R: {
+    const MCSymbolRefExpr *OpExpr =
+        (const MCSymbolRefExpr *)Inst.getOperand(1).getExpr();
+    XtensaMCExpr::VariantKind Kind = XtensaMCExpr::VK_Xtensa_None;
+    const MCExpr *NewOpExpr = XtensaMCExpr::create(OpExpr, Kind, getContext());
+    Inst.getOperand(1).setExpr(NewOpExpr);
+  } break;
+  case Xtensa::MOVI: {
+    XtensaTargetStreamer &TS = this->getTargetStreamer();
+
+    // Expand MOVI operand
+    if (!Inst.getOperand(1).isExpr()) {
+      uint64_t ImmOp64 = Inst.getOperand(1).getImm();
+      int32_t Imm = ImmOp64;
+      if ((Imm < -2048) || (Imm > 2047)) {
+        XtensaTargetStreamer &TS = this->getTargetStreamer();
+        MCInst TmpInst;
+        TmpInst.setLoc(IDLoc);
+        TmpInst.setOpcode(Xtensa::L32R);
+        const MCExpr *Value = MCConstantExpr::create(ImmOp64, getContext());
+        MCSymbol *Sym = getContext().createTempSymbol();
+        const MCExpr *Expr = MCSymbolRefExpr::create(
+            Sym, MCSymbolRefExpr::VK_None, getContext());
+        const MCExpr *OpExpr = XtensaMCExpr::create(
+            Expr, XtensaMCExpr::VK_Xtensa_None, getContext());
+        TmpInst.addOperand(Inst.getOperand(0));
+        MCOperand Op1 = MCOperand::createExpr(OpExpr);
+        TmpInst.addOperand(Op1);
+        TS.emitLiteral(Sym, Value, IDLoc);
+        Inst = TmpInst;
+      }
+    } else {
+      MCInst TmpInst;
+      TmpInst.setLoc(IDLoc);
+      TmpInst.setOpcode(Xtensa::L32R);
+      const MCExpr *Value = Inst.getOperand(1).getExpr();
+      MCSymbol *Sym = getContext().createTempSymbol();
+      const MCExpr *Expr =
+          MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
+      const MCExpr *OpExpr = XtensaMCExpr::create(
+          Expr, XtensaMCExpr::VK_Xtensa_None, getContext());
+      TmpInst.addOperand(Inst.getOperand(0));
+      MCOperand Op1 = MCOperand::createExpr(OpExpr);
+      TmpInst.addOperand(Op1);
+      Inst = TmpInst;
+      TS.emitLiteral(Sym, Value, IDLoc);
+    }
+  } break;
+  default:
+    break;
+  }
+
+  return true;
+}
+
 bool XtensaAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
                                               OperandVector &Operands,
                                               MCStreamer &Out,
@@ -361,6 +441,7 @@ bool XtensaAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   default:
     break;
   case Match_Success:
+    processInstruction(Inst, IDLoc, Out, STI);
     Inst.setLoc(IDLoc);
     Out.emitInstruction(Inst, getSTI());
     return false;
@@ -686,6 +767,58 @@ bool XtensaAsmParser::ParseInstruction(ParseInstructionInfo &Info,
   return false;
 }
 
+bool XtensaAsmParser::parseLiteralDirective(SMLoc L) {
+  MCAsmParser &Parser = getParser();
+  MCSymbol *Sym;
+  const MCExpr *Value;
+  SMLoc LiteralLoc = getLexer().getLoc();
+  XtensaTargetStreamer &TS = this->getTargetStreamer();
+
+  if (Parser.parseExpression(Value))
+    return true;
+
+  const MCSymbolRefExpr *SE = dyn_cast<MCSymbolRefExpr>(Value);
+  if (!SE)
+    return Error(LiteralLoc, "literal label must be a symbol");
+  else {
+    Sym = getContext().getOrCreateSymbol(SE->getSymbol().getName());
+  }
+
+  if (Parser.parseToken(AsmToken::Comma, "expected comma"))
+    return true;
+
+  SMLoc OpcodeLoc = getLexer().getLoc();
+  if (parseOptionalToken(AsmToken::EndOfStatement))
+    return Error(OpcodeLoc, "expected value");
+
+  if (Parser.parseExpression(Value))
+    return true;
+
+  TS.emitLiteral(Sym, Value, LiteralLoc);
+
+  return false;
+}
+
+bool XtensaAsmParser::ParseDirective(AsmToken DirectiveID) {
+  StringRef IDVal = DirectiveID.getString();
+  SMLoc Loc = getLexer().getLoc();
+
+  if (IDVal == ".literal_position") {
+    XtensaTargetStreamer &TS = this->getTargetStreamer();
+    TS.emitLiteralPosition();
+    Lex();
+    return false;
+  }
+
+  if (IDVal == ".literal") {
+    parseLiteralDirective(Loc);
+    Lex();
+    return false;
+  }
+
+  return true;
+}
+
 // Force static initialization.
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeXtensaAsmParser() {
   RegisterMCAsmParser<XtensaAsmParser> X(getTheXtensaTarget());
diff --git a/llvm/lib/Target/Xtensa/CMakeLists.txt b/llvm/lib/Target/Xtensa/CMakeLists.txt
index 2064511e75b82e..726efadc87c0b2 100644
--- a/llvm/lib/Target/Xtensa/CMakeLists.txt
+++ b/llvm/lib/Target/Xtensa/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_TARGET_DEFINITIONS Xtensa.td)
 
 tablegen(LLVM XtensaGenAsmMatcher.inc -gen-asm-matcher)
 tablegen(LLVM XtensaGenAsmWriter.inc -gen-asm-writer)
+tablegen(LLVM XtensaGenCallingConv.inc -gen-callingconv)
 tablegen(LLVM XtensaGenDAGISel.inc -gen-dag-isel)
 tablegen(LLVM XtensaGenDisassemblerTables.inc -gen-disassembler)
 tablegen(LLVM XtensaGenInstrInfo.inc -gen-instr-info)
@@ -15,6 +16,7 @@ add_public_tablegen_target(XtensaCommonTableGen)
 
 add_llvm_target(XtensaCodeGen
   XtensaAsmPrinter.cpp
+  XtensaConstantPoolValue.cpp
   XtensaFrameLowering.cpp
   XtensaInstrInfo.cpp
   XtensaISelDAGToDAG.cpp
@@ -22,6 +24,7 @@ add_llvm_target(XtensaCodeGen
   XtensaRegisterInfo.cpp
   XtensaSubtarget.cpp
   XtensaTargetMachine.cpp
+  XtensaUtils.cpp
 
   LINK_COMPONENTS
   AsmPrinter
diff --git a/llvm/lib/Target/Xtensa/MCTargetDesc/CMakeLists.txt b/llvm/lib/Target/Xtensa/MCTargetDesc/CMakeLists.txt
index 6841b44f9d569c..dc12863394c7ad 100644
--- a/llvm/lib/Target/Xtensa/MCTargetDesc/CMakeLists.txt
+++ b/llvm/lib/Target/Xtensa/MCTargetDesc/CMakeLists.txt
@@ -6,6 +6,7 @@ add_llvm_component_library(LLVMXtensaDesc
   XtensaMCCodeEmitter.cpp
   XtensaMCExpr.cpp
   XtensaMCTargetDesc.cpp
+  XtensaTargetStreamer.cpp
 
   LINK_COMPONENTS
   MC
diff --git a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaMCTargetDesc.cpp b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaMCTargetDesc.cpp
index 48674d15bdfbe2..8914ebf658cc43 100644
--- a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaMCTargetDesc.cpp
+++ b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaMCTargetDesc.cpp
@@ -10,6 +10,7 @@
 #include "XtensaMCTargetDesc.h"
 #include "XtensaInstPrinter.h"
 #include "XtensaMCAsmInfo.h"
+#include "XtensaTargetStreamer.h"
 #include "TargetInfo/XtensaTargetInfo.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -63,16 +64,29 @@ createXtensaMCSubtargetInfo(const Triple &TT, StringRef CPU, StringRef FS) {
   return createXtensaMCSubtargetInfoImpl(TT, CPU, CPU, FS);
 }
 
+static MCTargetStreamer *
+createXtensaAsmTargetStreamer(MCStreamer &S, formatted_raw_ostream &OS,
+                              MCInstPrinter *InstPrint, bool isVerboseAsm) {
+  return new XtensaTargetAsmStreamer(S, OS);
+}
+
+static MCTargetStreamer *
+createXtensaObjectTargetStreamer(MCStreamer &S, const MCSubtargetInfo &STI) {
+  return new XtensaTargetELFStreamer(S);
+}
+
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeXtensaTargetMC() {
   // Register the MCAsmInfo.
-  TargetRegistry::RegisterMCAsmInfo(getTheXtensaTarget(), createXtensaMCAsmInfo);
+  TargetRegistry::RegisterMCAsmInfo(getTheXtensaTarget(),
+                                    createXtensaMCAsmInfo);
 
   // Register the MCCodeEmitter.
   TargetRegistry::RegisterMCCodeEmitter(getTheXtensaTarget(),
                                         createXtensaMCCodeEmitter);
 
   // Register the MCInstrInfo.
-  TargetRegistry::RegisterMCInstrInfo(getTheXtensaTarget(), createXtensaMCInstrInfo);
+  TargetRegistry::RegisterMCInstrInfo(getTheXtensaTarget(),
+                                      createXtensaMCInstrInfo);
 
   // Register the MCInstPrinter.
   TargetRegistry::RegisterMCInstPrinter(getTheXtensaTarget(),
@@ -89,4 +103,12 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeXtensaTargetMC() {
   // Register the MCAsmBackend.
   TargetRegistry::RegisterMCAsmBackend(getTheXtensaTarget(),
                                        createXtensaMCAsmBackend);
+
+  // Register the asm target streamer.
+  TargetRegistry::RegisterAsmTargetStreamer(getTheXtensaTarget(),
+                                            createXtensaAsmTargetStreamer);
+
+  // Register the ELF target streamer.
+  TargetRegistry::RegisterObjectTargetStreamer(
+      getTheXtensaTarget(), createXtensaObjectTargetStreamer);
 }
diff --git a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.cpp b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.cpp
new file mode 100644
index 00000000000000..4163e64de48f09
--- /dev/null
+++ b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.cpp
@@ -0,0 +1,87 @@
+//===-- XtensaTargetStreamer.cpp - Xtensa Target Streamer Methods ---------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides Xtensa specific target streamer methods.
+//
+//===----------------------------------------------------------------------===//
+
+#include "XtensaTargetStreamer.h"
+#include "XtensaInstPrinter.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/MC/MCAssembler.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCObjectFileInfo.h"
+#include "llvm/MC/MCSectionELF.h"
+#include "llvm/Support/FormattedStream.h"
+
+using namespace llvm;
+
+XtensaTargetStreamer::XtensaTargetStreamer(MCStreamer &S)
+    : MCTargetStreamer(S) {}
+
+XtensaTargetAsmStreamer::XtensaTargetAsmStreamer(MCStreamer &S,
+                                                 formatted_raw_ostream &OS)
+    : XtensaTargetStreamer(S), OS(OS) {}
+
+void XtensaTargetAsmStreamer::emitLiteral(MCSymbol *LblSym, const MCExpr *Value,
+                                          SMLoc L) {
+  const MCAsmInfo *MAI = Streamer.getContext().getAsmInfo();
+
+  OS << "\t.literal\t";
+  LblSym->print(OS, MAI);
+  OS << ", ";
+  Value->print(OS, MAI);
+  OS << '\n';
+}
+
+void XtensaTargetAsmStreamer::emitLiteralPosition() {
+  OS << "\t.literal_position\n";
+}
+
+XtensaTargetELFStreamer::XtensaTargetELFStreamer(MCStreamer &S)
+    : XtensaTargetStreamer(S) {}
+
+static std::string getLiteralSectionName(std::string CSectionName) {
+  std::size_t Pos = CSectionName.find(".text");
+  std::string SectionName;
+  if (Pos != std::string::npos) {
+    if (Pos > 0)
+      SectionName = CSectionName.substr(0, Pos + 5);
+    else
+      SectionName = "";
+    SectionName += ".literal";
+    SectionName += CSectionName.substr(Pos + 5);
+  } else {
+    SectionName = CSectionName;
+    SectionName += ".literal";
+  }
+  return SectionName;
+}
+
+void XtensaTargetELFStreamer::emitLiteral(MCSymbol *LblSym, const MCExpr *Value,
+                                          SMLoc L) {
+  MCContext &Context = getStreamer().getContext();
+  MCStreamer &OutStreamer = getStreamer();
+  MCSectionELF *CS = (MCSectionELF *)OutStreamer.getCurrentSectionOnly();
+  std::string SectionName = getLiteralSectionName(CS->getName().str());
+
+  MCSection *ConstSection = Context.getELFSection(
+      SectionName, ELF::SHT_PROGBITS, ELF::SHF_EXECINSTR | ELF::SHF_ALLOC);
+
+  OutStreamer.pushSection();
+  OutStreamer.switchSection(ConstSection);
+  OutStreamer.emitLabel(LblSym, L);
+  OutStreamer.emitValue(Value, 4, L);
+  OutStreamer.popSection();
+}
+
+MCELFStreamer &XtensaTargetELFStreamer::getStreamer() {
+  return static_cast<MCELFStreamer &>(Streamer);
+}
diff --git a/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.h b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.h
new file mode 100644
index 00000000000000..a7d8b6dd9c792b
--- /dev/null
+++ b/llvm/lib/Target/Xtensa/MCTargetDesc/XtensaTargetStreamer.h
@@ -0,0 +1,46 @@
+//===-- XtensaTargetStreamer.h - Xtensa Target Streamer --------*- C++ -*--===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_XTENSA_XTENSATARGETSTREAMER_H
+#define LLVM_LIB_TARGET_XTENSA_XTENSATARGETSTREAMER_H
+
+#include "llvm/MC/MCELFStreamer.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/Support/SMLoc.h"
+
+namespace llvm {
+class formatted_raw_ostream;
+
+class XtensaTargetStreamer : public MCTargetStreamer {
+public:
+  XtensaTargetStreamer(MCStreamer &S);
+  virtual void emitLiteral(MCSymbol *LblSym, const MCExpr *Value, SMLoc L) = 0;
+  virtual void emitLiteralPosition() = 0;
+};
+
+class XtensaTargetAsmStreamer : public XtensaTargetStreamer {
+  formatted_raw_ostream &OS;
+
+public:
+  XtensaTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
+  void emitLiteral(MCSymbol *LblSym, const MCExpr *Value, SMLoc L) override;
+  void emitLiteralPosition() override;
+};
+
+class XtensaTargetELFStreamer : public XtensaTargetStreamer {
+public:
+  XtensaTargetELFStreamer(MCStreamer &S);
+  MCELFStreamer &getStreamer();
+  void emitLiteral(MCSymbol *LblSym, const MCExpr *Value, SMLoc L) override;
+  void emitLiteralPosition() override {}
+};
+} // end namespace llvm
+
+#endif
diff --git a/llvm/lib/Target/Xtensa/Xtensa.td b/llvm/lib/Target/Xtensa/Xtensa.td
index b953540be94de0..460a15e808b3a4 100644
--- a/llvm/lib/Target/Xtensa/Xtensa.td
+++ b/llvm/lib/Target/Xtensa/Xtensa.td
@@ -35,6 +35,12 @@ def : Proc<"generic", []>;
 
 include "XtensaRegisterInfo.td"
 
+//===----------------------------------------------------------------------===//
+// Calling Convention Description
+//===----------------------------------------------------------------------===//
+
+include "XtensaCallingConv.td"
+
 //===----------------------------------------------------------------------===//
 // Instruction Descriptions
 //===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp b/llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp
index 87dbf2eb5166cd..3c8977a742eaef 100644
--- a/llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp
+++ b/llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp
@@ -12,8 +12,12 @@
 //===----------------------------------------------------------------------===//
 
 #include "XtensaAsmPrinter.h"
+#include "MCTargetDesc/XtensaMCExpr.h"
 #include "TargetInfo/XtensaTargetInfo.h"
+#include "XtensaConstantPoolValue.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/MC/MCExpr.h"
@@ -25,12 +29,193 @@
 
 using namespace llvm;
 
+static MCSymbolRefExpr::VariantKind
+getModifierVariantKind(XtensaCP::XtensaCPModifier Modifier) {
+  switch (Modifier) {
+  case XtensaCP::no_modifier:
+    return MCSymbolRefExpr::VK_None;
+  case XtensaCP::TPOFF:
+    return MCSymbolRefExpr::VK_TPOFF;
+  }
+  report_fatal_error("Invalid XtensaCPModifier!");
+}
+
 void XtensaAsmPrinter::emitInstruction(const MachineInstr *MI) {
   MCInst LoweredMI;
   lowerToMCInst(MI, LoweredMI);
   EmitToStreamer(*OutStreamer, LoweredMI);
 }
 
+void XtensaAsmPrinter::emitMachineConstantPoolValue(
+    MachineConstantPoolValue *MCPV) {
+  XtensaConstantPoolValue *ACPV = static_cast<XtensaConstantPoolValue *>(MCPV);
+  MCSymbol *MCSym;
+
+  assert(ACPV->isExtSymbol() && "unrecognized constant pool value");
+
+  XtensaConstantPoolSymbol *XtensaSym = cast<XtensaConstantPoolSymbol>(ACPV);
+  const char *Sym = XtensaSym->getSymbol();
+  std::string SymName(Sym);
+  if (XtensaSym->isPrivateLinkage())
+    SymName = ".L" + SymName;
+  MCSym = GetExternalSymbolSymbol(StringRef(SymName));
+
+  MCSymbol *LblSym = GetCPISymbol(ACPV->getLabelId());
+  // TODO find a better way to check whether we emit data to .s file
+  if (OutStreamer->hasRawTextSupport()) {
+    std::string SymName("\t.literal ");
+    SymName += LblSym->getName();
+    SymName += ", ";
+    SymName += MCSym->getName();
+
+    StringRef Modifier = ACPV->getModifierText();
+    SymName += Modifier;
+
+    OutStreamer->emitRawText(StringRef(SymName));
+  } else {
+    MCSymbolRefExpr::VariantKind VK =
+        getModifierVariantKind(ACPV->getModifier());
+
+    if (ACPV->getModifier() != XtensaCP::no_modifier) {
+      std::string SymName(MCSym->getName());
+      MCSym = GetExternalSymbolSymbol(StringRef(SymName));
+    }
+
+    const MCExpr *Expr = MCSymbolRefExpr::create(MCSym, VK, OutContext);
+    uint64_t Size = getDataLayout().getTypeAllocSize(ACPV->getType());
+    OutStreamer->emitCodeAlignment(
+        Align(4), OutStreamer->getContext().getSubtargetInfo());
+    OutStreamer->emitLabel(LblSym);
+    OutStreamer->emitValue(Expr, Size);
+  }
+}
+
+void XtensaAsmPrinter::emitMachineConstantPoolEntry(
+    const MachineConstantPoolEntry &CPE, int i) {
+  if (CPE.isMachineConstantPoolEntry()) {
+    XtensaConstantPoolValue *ACPV =
+        static_cast<XtensaConstantPoolValue *>(CPE.Val.MachineCPVal);
+    ACPV->setLabelId(i);
+    emitMachineConstantPoolValue(CPE.Val.MachineCPVal);
+  } else {
+    MCSymbol *LblSym = GetCPISymbol(i);
+    // TODO find a better way to check whether we emit data to .s file
+    if (OutStreamer->hasRawTextSupport()) {
+      std::string str("\t.literal ");
+      str += LblSym->getName();
+      str += ", ";
...
[truncated]

Copy link

github-actions bot commented Mar 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@s-barannikov
Copy link
Contributor

@andreisfr Please don't amend / force push future commits. This makes it difficult to track comments.

llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/AsmParser/XtensaAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/Xtensa/XtensaAsmPrinter.cpp Outdated Show resolved Hide resolved
Fix type promotion in callconv implementation. Minor fixes in
constantpool, targetstreamer and asmparser. Fix minor format issues.
Use SmallString instead of std::string.
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

lgtm with a few nits

XtensaMCExpr::VariantKind Kind = XtensaMCExpr::VK_Xtensa_None;
const MCExpr *NewOpExpr = XtensaMCExpr::create(OpExpr, Kind, getContext());
Inst.getOperand(1).setExpr(NewOpExpr);
} break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting is off, break before brace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -4,10 +4,6 @@ LBL0:

# Out of range immediates

# imm12m
movi a1, 3000
Copy link
Contributor

Choose a reason for hiding this comment

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

Lost the range verification test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this test, because constant pool is implemented now. The Xtensa asm allows mnemonics like "movi a1, imm" even if "imm" is out of range, we can just replace it with loading contant from constant pool

Comment on lines 777 to 785
if (!SE)
return Error(LiteralLoc, "literal label must be a symbol");

if (Parser.parseToken(AsmToken::Comma, "expected comma"))
return true;

SMLoc OpcodeLoc = getLexer().getLoc();
if (parseOptionalToken(AsmToken::EndOfStatement))
return Error(OpcodeLoc, "expected value");
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe these error conditions are tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added test for this checks

Comment on lines 111 to 114
void print(raw_ostream *O) const {
if (O)
print(*O);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it

if (O)
print(*O);
}
void dump() const;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be wrapped in LLVM_ENABLE_DUMP guard like other dump functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

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.

I understand nothing in constant pools, so I've left only nitpicks.

return static_cast<XtensaTargetStreamer &>(TS);
}

bool ParseDirective(AsmToken DirectiveID) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is deprecated, use parseDirective (starting with lowercase letter).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


TS.emitLiteral(Sym, Value, LiteralLoc);

return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

The method should consume the terminating newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

XtensaTargetStreamer &TS = this->getTargetStreamer();
TS.emitLiteralPosition();
Lex();
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Parse the newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

SMLoc L) {
MCContext &Context = getStreamer().getContext();
MCStreamer &OutStreamer = getStreamer();
MCSectionELF *CS = (MCSectionELF *)OutStreamer.getCurrentSectionOnly();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MCSectionELF *CS = (MCSectionELF *)OutStreamer.getCurrentSectionOnly();
auto *CS = static_cast<MCSectionELF *>OutStreamer.getCurrentSectionOnly();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

StringRef Modifier = ACPV->getModifierText();
LiteralStr << Modifier;

OutStreamer->emitRawText(StringRef(LiteralStr.str()));
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to use XtensaTargetStreamer::emitLiteral and avoid if/else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored XtensaTargetStreamer to implement some if/else code from AsmPrinter in XtensaAsmTargetStreamer and XtensaElfTargetStreamer, PTAL.


XtensaConstantPoolConstant::XtensaConstantPoolConstant(
const Constant *C, unsigned ID, XtensaCP::XtensaCPKind Kind)
: XtensaConstantPoolValue((Type *)C->getType(), ID, Kind), CVal(C) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

This this cast necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it

using namespace llvm;

XtensaConstantPoolValue::XtensaConstantPoolValue(
Type *Ty, unsigned id, XtensaCP::XtensaCPKind kind,
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize variable names. In other methods, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if (Constants[i].isMachineConstantPoolEntry() &&
(Constants[i].getAlign() >= Alignment)) {
XtensaConstantPoolValue *CPV =
(XtensaConstantPoolValue *)Constants[i].Val.MachineCPVal;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use auto * and static_cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


public:
static XtensaConstantPoolMBB *
Create(LLVMContext &C, const MachineBasicBlock *mbb, unsigned ID);
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize parameter names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

/// XtensaConstantPoolJumpTable - Xtensa-specific constantpool values for Jump
/// Table symbols.
class XtensaConstantPoolJumpTable : public XtensaConstantPoolValue {
unsigned IDX; // Jump Table Index.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unsigned IDX; // Jump Table Index.
unsigned Idx; // Jump Table Index.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

bool needs64BitAlign = (ValVT == MVT::i32 && OrigAlign == Align(8));
bool needs128BitAlign = (ValVT == MVT::i32 && OrigAlign == Align(16));

if (ValVT == MVT::i32 || ValVT == MVT::f32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (ValVT == MVT::i32 || ValVT == MVT::f32) {
if (ValVT == MVT::i32) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

// Get a count of how many bytes are to be pushed on the stack.
unsigned NumBytes = CCInfo.getStackSize();

unsigned StackAlignment = TFL->getStackAlignment();
Copy link
Contributor

Choose a reason for hiding this comment

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

The method is deprecated, consider using getStackAlign.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

// Check if use node maybe lowered to the ADDMI instruction
SDNode &OpNode = *Op.getNode();
if ((OpNode.hasOneUse() && OpNode.use_begin()->getOpcode() == ISD::ADD) &&
(value >= -32768) && (value <= 32512) && ((value & 0xff) == 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using isShiftedInt from MathExtras.h here and in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -23,5 +24,139 @@

using namespace llvm;

static inline const MachineInstrBuilder &
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static inline const MachineInstrBuilder &
static const MachineInstrBuilder &

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


if (isInt<8>(Amount)) // addi sp, sp, amount
BuildMI(MBB, I, DL, get(Xtensa::ADDI), Reg).addReg(SP).addImm(Amount);
else { // Expand immediate that doesn't fit in 12-bit.
Copy link
Contributor

Choose a reason for hiding this comment

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

8 bit?
Missing braces around if.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Move some functionality from AsmPrinter to the TargetStreamer. Add test for
"literal" asm directive. Code formatting.
@andreisfr
Copy link
Contributor Author

@s-barannikov , may I ask you to review new changes?

@sstefan1 sstefan1 merged commit 36209d3 into llvm:main Apr 23, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants