[LLVMdev] [PATCH] MC: handle .cfi_startproc simple

[Rafael Espíndola]

Really sorry I missed this. Just found it looking for something else
in my inbox.

I think we should support this but
* We should still err on other identifiers ".cfi_startport bar" is invalid.
* If I read the gas documentation correctly, the effect of "simple" is
to skip the initial cfi instructions. We should test if that is the
case and implement it too. Accepting and ignoring "simple" is not very
user friendly.


On 11 July 2013 05:25, Ramkumar Ramachandra <artagnon at gmail.com> wrote:
> The statement
>
>   .cfi_startproc simple
>
> is perfectly valid, and GNU as supports it.  Unfortunately, LLVM barfs
> with:
>
>   error: invalid instruction mnemonic 'simple'
>
> Fix this by modifying ParseDirectiveCFIStartProc() to parse the
> identifier "simple".
>
> Cc: Rafael Espindola <rafael.espindola at gmail.com>
> Cc: Jim Grosbach <grosbach at apple.com>
> Cc: Eli Friedman <eli.friedman at gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon at gmail.com>
> ---
>  This time, I know for certain that I've done something stupid because
>  a huge number of tests break.  Did I misunderstand how
>  parseIdentifier() is meant to be used?  Am I atleast passing this
>  Simple bool around correctly, and appending " simple" in the low asm
>  layer appropriately?  What am I missing?
>
>  Thanks.
>
>  include/llvm/MC/MCStreamer.h                 | 2 +-
>  lib/CodeGen/AsmPrinter/DwarfCFIException.cpp | 2 +-
>  lib/MC/MCAsmStreamer.cpp                     | 6 ++++--
>  lib/MC/MCParser/AsmParser.cpp                | 8 ++++++--
>  lib/MC/MCStreamer.cpp                        | 2 +-
>  test/MC/AsmParser/cfi-startproc-simple.s     | 7 +++++++
>  6 files changed, 20 insertions(+), 7 deletions(-)
>  create mode 100644 test/MC/AsmParser/cfi-startproc-simple.s
>
> diff --git a/include/llvm/MC/MCStreamer.h b/include/llvm/MC/MCStreamer.h
> index 970c4ed..85f85dc 100644
> --- a/include/llvm/MC/MCStreamer.h
> +++ b/include/llvm/MC/MCStreamer.h
> @@ -551,7 +551,7 @@ namespace llvm {
>
>      virtual void EmitCompactUnwindEncoding(uint32_t CompactUnwindEncoding);
>      virtual void EmitCFISections(bool EH, bool Debug);
> -    void EmitCFIStartProc();
> +    void EmitCFIStartProc(bool Simple);
>      void EmitCFIEndProc();
>      virtual void EmitCFIDefCfa(int64_t Register, int64_t Offset);
>      virtual void EmitCFIDefCfaOffset(int64_t Offset);
> diff --git a/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp b/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
> index fec5ced..13f5fc3 100644
> --- a/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
> +++ b/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp
> @@ -113,7 +113,7 @@ void DwarfCFIException::BeginFunction(const MachineFunction *MF) {
>    if (!shouldEmitPersonality && !shouldEmitMoves)
>      return;
>
> -  Asm->OutStreamer.EmitCFIStartProc();
> +  Asm->OutStreamer.EmitCFIStartProc(false);
>
>    // Indicate personality routine, if any.
>    if (!shouldEmitPersonality)
> diff --git a/lib/MC/MCAsmStreamer.cpp b/lib/MC/MCAsmStreamer.cpp
> index 781e400..9fb3c5d 100644
> --- a/lib/MC/MCAsmStreamer.cpp
> +++ b/lib/MC/MCAsmStreamer.cpp
> @@ -61,7 +61,7 @@ private:
>    bool needsSet(const MCExpr *Value);
>
>    void EmitRegisterName(int64_t Register);
> -  virtual void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame);
> +  virtual void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame, bool Simple);
>    virtual void EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame);
>
>  public:
> @@ -902,13 +902,15 @@ void MCAsmStreamer::EmitCFISections(bool EH, bool Debug) {
>    EmitEOL();
>  }
>
> -void MCAsmStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) {
> +void MCAsmStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame, bool Simple) {
>    if (!UseCFI) {
>      RecordProcStart(Frame);
>      return;
>    }
>
>    OS << "\t.cfi_startproc";
> +  if (Simple)
> +    OS << " simple";
>    EmitEOL();
>  }
>
> diff --git a/lib/MC/MCParser/AsmParser.cpp b/lib/MC/MCParser/AsmParser.cpp
> index fb4bea1..f012cc1 100644
> --- a/lib/MC/MCParser/AsmParser.cpp
> +++ b/lib/MC/MCParser/AsmParser.cpp
> @@ -2736,9 +2736,13 @@ bool AsmParser::ParseDirectiveCFISections() {
>  }
>
>  /// ParseDirectiveCFIStartProc
> -/// ::= .cfi_startproc
> +/// ::= .cfi_startproc [simple]
>  bool AsmParser::ParseDirectiveCFIStartProc() {
> -  getStreamer().EmitCFIStartProc();
> +  StringRef ThisIdentifier;
> +  bool Simple = false;
> +  if (!parseIdentifier(ThisIdentifier) && ThisIdentifier == "simple")
> +    Simple = true;
> +  getStreamer().EmitCFIStartProc(Simple);
>    return false;
>  }
>
> diff --git a/lib/MC/MCStreamer.cpp b/lib/MC/MCStreamer.cpp
> index 6542f42..eab2dc2 100644
> --- a/lib/MC/MCStreamer.cpp
> +++ b/lib/MC/MCStreamer.cpp
> @@ -209,7 +209,7 @@ void MCStreamer::EmitCFISections(bool EH, bool Debug) {
>    EmitDebugFrame = Debug;
>  }
>
> -void MCStreamer::EmitCFIStartProc() {
> +void MCStreamer::EmitCFIStartProc(bool Simple) {
>    MCDwarfFrameInfo *CurFrame = getCurrentFrameInfo();
>    if (CurFrame && !CurFrame->End)
>      report_fatal_error("Starting a frame before finishing the previous one!");
> diff --git a/test/MC/AsmParser/cfi-startproc-simple.s b/test/MC/AsmParser/cfi-startproc-simple.s
> new file mode 100644
> index 0000000..4729e03
> --- /dev/null
> +++ b/test/MC/AsmParser/cfi-startproc-simple.s
> @@ -0,0 +1,7 @@
> +// RUN: llvm-mc -triple x86_64-unknown-unknown -show-encoding %s > %t 2> %t.err
> +// RUN: FileCheck < %t %s
> +// RUN: FileCheck --check-prefix=CHECK-STDERR < %t.err %s
> +
> +.cfi_startproc simple
> +.cfi_endproc
> +// CHECK:
> --
> 1.8.3.2.736.g869de25
>