Thu Jun 2 09:01:36 PDT 2011

Hi Rafael,

Thanks for the quick turnaround. That takes care of the crash, but we're not quite there yet. A few questions.

$ llvm-mc -triple armv7-apple-darwin10 t.s
	.section	__TEXT,__text,regular,pure_instructions
t.s:1:14: error: expected absolute expression
.cfi_def_cfa r0, 8

It looks like the % prefix is also expected in the parsing?

For the printer methods, getRegisterName() isn't going to work. That expects the internal enum register number, but the CFI directives from the streamer have the dwarf register number instead.

I confess I'm a little confused at this patch in general. I was under the (mistaken?) impression that the cfi directives referred to registers via dwarf register number. Is that not the case? Is there something about that solution that's insufficient?

Thanks,
  Jim

On Jun 1, 2011, at 7:34 PM, Rafael Espindola wrote:

> Author: rafael
> Date: Wed Jun  1 21:34:55 2011
> New Revision: 132451
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=132451&view=rev
> Log:
> Don't hardcode the %reg format in the streamer.
> 
> Modified:
>    llvm/trunk/include/llvm/MC/MCInstPrinter.h
>    llvm/trunk/lib/MC/MCAsmStreamer.cpp
>    llvm/trunk/lib/MC/MCInstPrinter.cpp
>    llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
>    llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.h
>    llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
>    llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.h
>    llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
>    llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h
>    llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
>    llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.h
> 
> Modified: llvm/trunk/include/llvm/MC/MCInstPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstPrinter.h?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCInstPrinter.h (original)
> +++ llvm/trunk/include/llvm/MC/MCInstPrinter.h Wed Jun  1 21:34:55 2011
> @@ -45,8 +45,8 @@
>   /// "MOV32ri") or empty if we can't resolve it.
>   virtual StringRef getOpcodeName(unsigned Opcode) const;
> 
> -  /// getRegName - Return the assembler register name.
> -  virtual StringRef getRegName(unsigned RegNo) const;
> +  /// printRegName - Print the assembler register name.
> +  virtual void printRegName(raw_ostream &OS, unsigned RegNo) const;
> 
>   unsigned getAvailableFeatures() const { return AvailableFeatures; }
>   void setAvailableFeatures(unsigned Value) { AvailableFeatures = Value; }
> 
> Modified: llvm/trunk/lib/MC/MCAsmStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmStreamer.cpp Wed Jun  1 21:34:55 2011
> @@ -825,7 +825,7 @@
>   if (InstPrinter) {
>     const TargetAsmInfo &asmInfo = getContext().getTargetAsmInfo();
>     unsigned LLVMRegister = asmInfo.getLLVMRegNum(Register, true);
> -    OS << '%' << InstPrinter->getRegName(LLVMRegister);
> +    InstPrinter->printRegName(OS, LLVMRegister);
>   } else {
>     OS << Register;
>   }
> @@ -1169,8 +1169,10 @@
> }
> 
> void MCAsmStreamer::EmitSetFP(unsigned FpReg, unsigned SpReg, int64_t Offset) {
> -  OS << "\t.setfp\t" << InstPrinter->getRegName(FpReg)
> -     << ", "        << InstPrinter->getRegName(SpReg);
> +  OS << "\t.setfp\t";
> +  InstPrinter->printRegName(OS, FpReg);
> +  OS << ", ";
> +  InstPrinter->printRegName(OS, SpReg);
>   if (Offset)
>     OS << ", #" << Offset;
>   EmitEOL();
> @@ -1189,10 +1191,12 @@
>   else
>     OS << "\t.save\t{";
> 
> -  OS << InstPrinter->getRegName(RegList[0]);
> +  InstPrinter->printRegName(OS, RegList[0]);
> 
> -  for (unsigned i = 1, e = RegList.size(); i != e; ++i)
> -    OS << ", " << InstPrinter->getRegName(RegList[i]);
> +  for (unsigned i = 1, e = RegList.size(); i != e; ++i) {
> +    OS << ", ";
> +    InstPrinter->printRegName(OS, RegList[i]);
> +  }
> 
>   OS << "}";
>   EmitEOL();
> 
> Modified: llvm/trunk/lib/MC/MCInstPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCInstPrinter.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCInstPrinter.cpp (original)
> +++ llvm/trunk/lib/MC/MCInstPrinter.cpp Wed Jun  1 21:34:55 2011
> @@ -20,7 +20,6 @@
>   return "";
> }
> 
> -StringRef MCInstPrinter::getRegName(unsigned RegNo) const {
> +void MCInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
>   assert(0 && "Target should implement this");
> -  return "";
> }
> 
> Modified: llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.cpp Wed Jun  1 21:34:55 2011
> @@ -29,8 +29,8 @@
>   return getInstructionName(Opcode);
> }
> 
> -StringRef ARMInstPrinter::getRegName(unsigned RegNo) const {
> -  return getRegisterName(RegNo);
> +void ARMInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
> +  OS << getRegisterName(RegNo);
> }
> 
> void ARMInstPrinter::printInst(const MCInst *MI, raw_ostream &O) {
> 
> Modified: llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.h?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.h (original)
> +++ llvm/trunk/lib/Target/ARM/InstPrinter/ARMInstPrinter.h Wed Jun  1 21:34:55 2011
> @@ -28,7 +28,7 @@
> 
>   virtual void printInst(const MCInst *MI, raw_ostream &O);
>   virtual StringRef getOpcodeName(unsigned Opcode) const;
> -  virtual StringRef getRegName(unsigned RegNo) const;
> +  virtual void printRegName(raw_ostream &OS, unsigned RegNo) const;
> 
>   static const char *getInstructionName(unsigned Opcode);
> 
> 
> Modified: llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp Wed Jun  1 21:34:55 2011
> @@ -26,8 +26,8 @@
>   return getInstructionName(Opcode);
> }
> 
> -StringRef PPCInstPrinter::getRegName(unsigned RegNo) const {
> -  return getRegisterName(RegNo);
> +void PPCInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
> +  OS << getRegisterName(RegNo);
> }
> 
> void PPCInstPrinter::printInst(const MCInst *MI, raw_ostream &O) {
> 
> Modified: llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.h?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.h (original)
> +++ llvm/trunk/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.h Wed Jun  1 21:34:55 2011
> @@ -33,7 +33,7 @@
>     return SyntaxVariant == 1;
>   }
> 
> -  StringRef getRegName(unsigned RegNo) const;
> +  virtual void printRegName(raw_ostream &OS, unsigned RegNo) const;
>   virtual void printInst(const MCInst *MI, raw_ostream &O);
>   virtual StringRef getOpcodeName(unsigned Opcode) const;
> 
> 
> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.cpp Wed Jun  1 21:34:55 2011
> @@ -41,8 +41,9 @@
>             &TM.getSubtarget<X86Subtarget>()));
> }
> 
> -StringRef X86ATTInstPrinter::getRegName(unsigned RegNo) const {
> -  return getRegisterName(RegNo);
> +void X86ATTInstPrinter::printRegName(raw_ostream &OS,
> +                                     unsigned RegNo) const {
> +  OS << '%' << getRegisterName(RegNo);
> }
> 
> void X86ATTInstPrinter::printInst(const MCInst *MI, raw_ostream &OS) {
> 
> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h (original)
> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86ATTInstPrinter.h Wed Jun  1 21:34:55 2011
> @@ -26,7 +26,7 @@
> public:
>   X86ATTInstPrinter(TargetMachine &TM, const MCAsmInfo &MAI);
> 
> -  StringRef getRegName(unsigned RegNo) const;
> +  virtual void printRegName(raw_ostream &OS, unsigned RegNo) const;
>   virtual void printInst(const MCInst *MI, raw_ostream &OS);
>   virtual StringRef getOpcodeName(unsigned Opcode) const;
> 
> 
> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.cpp Wed Jun  1 21:34:55 2011
> @@ -29,8 +29,8 @@
> #define GET_INSTRUCTION_NAME
> #include "X86GenAsmWriter1.inc"
> 
> -StringRef X86IntelInstPrinter::getRegName(unsigned RegNo) const {
> -  return getRegisterName(RegNo);
> +void X86IntelInstPrinter::printRegName(raw_ostream &OS, unsigned RegNo) const {
> +  OS << getRegisterName(RegNo);
> }
> 
> void X86IntelInstPrinter::printInst(const MCInst *MI, raw_ostream &OS) {
> 
> Modified: llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.h?rev=132451&r1=132450&r2=132451&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.h (original)
> +++ llvm/trunk/lib/Target/X86/InstPrinter/X86IntelInstPrinter.h Wed Jun  1 21:34:55 2011
> @@ -27,7 +27,7 @@
>   X86IntelInstPrinter(TargetMachine &TM, const MCAsmInfo &MAI)
>     : MCInstPrinter(MAI) {}
> 
> -  StringRef getRegName(unsigned RegNo) const;
> +  virtual void printRegName(raw_ostream &OS, unsigned RegNo) const;
>   virtual void printInst(const MCInst *MI, raw_ostream &OS);
>   virtual StringRef getOpcodeName(unsigned Opcode) const;
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits