[llvm-commits] [llvm] r93923 - in /llvm/trunk: include/llvm/MC/MCStreamer.h lib/MC/MCAsmStreamer.cpp lib/MC/MCStreamer.cpp

[Jim Grosbach]

Hi Chris,

Should these be adding the newline? There's various bits that like to emit comments following the data directive on the same line using the AsmPrinter::EOL methods. Having the newline come out here results in an extra newline and the comment being on the following line from the directive.

For example, from the DwarfException::EmitExceptionTable() bits around DwarfException.cpp:857 or so.
      .byte   3                                           @ Call site format (udata4)
becomes
      .byte   3                                           
                                                                @ Call site format (udata4)

It appears there's some mis-match in usage, though, as other places are assuming these emit the newline. CodeGen/PowerPC/i128-and-beyond.ll fails if it's removed, for example.

Purely a cosmetic thing, so not a huge deal, but it caught my eye, so figured I'd mention it.

Thanks,
  Jim

On Jan 19, 2010, at 2:03 PM, Chris Lattner wrote:

> Author: lattner
> Date: Tue Jan 19 16:03:38 2010
> New Revision: 93923
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=93923&view=rev
> Log:
> add a new EmitIntValue method that MCStreamer impls can optionally define
> and that clients can use.
> 
> Modified:
>    llvm/trunk/include/llvm/MC/MCStreamer.h
>    llvm/trunk/lib/MC/MCAsmStreamer.cpp
>    llvm/trunk/lib/MC/MCStreamer.cpp
> 
> Modified: llvm/trunk/include/llvm/MC/MCStreamer.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=93923&r1=93922&r2=93923&view=diff
> 
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCStreamer.h (original)
> +++ llvm/trunk/include/llvm/MC/MCStreamer.h Tue Jan 19 16:03:38 2010
> @@ -169,6 +169,10 @@
>     virtual void EmitValue(const MCExpr *Value, unsigned Size,
>                            unsigned AddrSpace) = 0;
> 
> +    /// EmitIntValue - Special case of EmitValue that avoids the client having
> +    /// to pass in a MCExpr for constant integers.
> +    virtual void EmitIntValue(uint64_t Value, unsigned Size,unsigned AddrSpace);
> +    
>     /// EmitFill - Emit NumBytes bytes worth of the value specified by
>     /// FillValue.  This implements directives such as '.space'.
>     virtual void EmitFill(uint64_t NumBytes, uint8_t FillValue,
> 
> Modified: llvm/trunk/lib/MC/MCAsmStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=93923&r1=93922&r2=93923&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/MC/MCAsmStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCAsmStreamer.cpp Tue Jan 19 16:03:38 2010
> @@ -61,6 +61,8 @@
>   virtual void EmitBytes(StringRef Data, unsigned AddrSpace);
> 
>   virtual void EmitValue(const MCExpr *Value, unsigned Size,unsigned AddrSpace);
> +  virtual void EmitIntValue(uint64_t Value, unsigned Size, unsigned AddrSpace);
> +
>   virtual void EmitFill(uint64_t NumBytes, uint8_t FillValue,
>                         unsigned AddrSpace);
> 
> @@ -187,19 +189,40 @@
>     OS << Directive << (unsigned)(unsigned char)Data[i] << '\n';
> }
> 
> +/// EmitIntValue - Special case of EmitValue that avoids the client having
> +/// to pass in a MCExpr for constant integers.
> +void MCAsmStreamer::EmitIntValue(uint64_t Value, unsigned Size,
> +                                 unsigned AddrSpace) {
> +  assert(CurSection && "Cannot emit contents before setting section!");
> +  // Need target hooks to know how to print this.
> +  const char *Directive = 0;
> +  switch (Size) {
> +  default: break;
> +  case 1: Directive = MAI.getData8bitsDirective(AddrSpace); break;
> +  case 2: Directive = MAI.getData16bitsDirective(AddrSpace); break;
> +  case 4: Directive = MAI.getData32bitsDirective(AddrSpace); break;
> +  case 8: Directive = MAI.getData64bitsDirective(AddrSpace); break;
> +  }
> +  
> +  assert(Directive && "Invalid size for machine code value!");
> +  OS << Directive << truncateToSize(Value, Size) << '\n';
> +}
> +
> void MCAsmStreamer::EmitValue(const MCExpr *Value, unsigned Size,
>                               unsigned AddrSpace) {
>   assert(CurSection && "Cannot emit contents before setting section!");
>   // Need target hooks to know how to print this.
> +  const char *Directive = 0;
>   switch (Size) {
> -  default: assert(0 && "Invalid size for machine code value!");
> -  case 1: OS << MAI.getData8bitsDirective(AddrSpace); break;
> -  case 2: OS << MAI.getData16bitsDirective(AddrSpace); break;
> -  case 4: OS << MAI.getData32bitsDirective(AddrSpace); break;
> -  case 8: OS << MAI.getData64bitsDirective(AddrSpace); break;
> +  default: break;
> +  case 1: Directive = MAI.getData8bitsDirective(AddrSpace); break;
> +  case 2: Directive = MAI.getData16bitsDirective(AddrSpace); break;
> +  case 4: Directive = MAI.getData32bitsDirective(AddrSpace); break;
> +  case 8: Directive = MAI.getData64bitsDirective(AddrSpace); break;
>   }
> 
> -  OS << *truncateToSize(Value, Size) << '\n';
> +  assert(Directive && "Invalid size for machine code value!");
> +  OS << Directive << *truncateToSize(Value, Size) << '\n';
> }
> 
> /// EmitFill - Emit NumBytes bytes worth of the value specified by
> 
> Modified: llvm/trunk/lib/MC/MCStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCStreamer.cpp?rev=93923&r1=93922&r2=93923&view=diff
> 
> ==============================================================================
> --- llvm/trunk/lib/MC/MCStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCStreamer.cpp Tue Jan 19 16:03:38 2010
> @@ -18,6 +18,13 @@
> MCStreamer::~MCStreamer() {
> }
> 
> +/// EmitIntValue - Special case of EmitValue that avoids the client having to
> +/// pass in a MCExpr for constant integers.
> +void MCStreamer::EmitIntValue(uint64_t Value, unsigned Size,
> +                              unsigned AddrSpace) {
> +  EmitValue(MCConstantExpr::Create(Value, getContext()), Size, AddrSpace);
> +}
> +
> /// EmitFill - Emit NumBytes bytes worth of the value specified by
> /// FillValue.  This implements directives such as '.space'.
> void MCStreamer::EmitFill(uint64_t NumBytes, uint8_t FillValue,
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits