[PATCH 1/2] MCAsmParser: full support for gas' '.if{cond} expression' directives

[Saleem Abdulrasool]

On Thu, May 29, 2014 at 3:05 PM, Janne Grunau <j at jannau.net> wrote:

> ---
>  lib/MC/MCParser/AsmParser.cpp       | 37 ++++++++++++++++++-----
>  test/MC/AsmParser/conditional_asm.s | 60
> +++++++++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+), 8 deletions(-)
>
> diff --git a/lib/MC/MCParser/AsmParser.cpp b/lib/MC/MCParser/AsmParser.cpp
> index 168597f..208d758 100644
> --- a/lib/MC/MCParser/AsmParser.cpp
> +++ b/lib/MC/MCParser/AsmParser.cpp
> @@ -352,8 +352,9 @@ private:
>      DK_REFERENCE, DK_WEAK_DEFINITION, DK_WEAK_REFERENCE,
>      DK_WEAK_DEF_CAN_BE_HIDDEN, DK_COMM, DK_COMMON, DK_LCOMM, DK_ABORT,
>      DK_INCLUDE, DK_INCBIN, DK_CODE16, DK_CODE16GCC, DK_REPT, DK_IRP,
> DK_IRPC,
> -    DK_IF, DK_IFNE, DK_IFB, DK_IFNB, DK_IFC, DK_IFEQS, DK_IFNC, DK_IFDEF,
> -    DK_IFNDEF, DK_IFNOTDEF, DK_ELSEIF, DK_ELSE, DK_ENDIF,
> +    DK_IF, DK_IFEQ, DK_IFGE, DK_IFGT, DK_IFLE, DK_IFLT, DK_IFNE, DK_IFB,
> +    DK_IFNB, DK_IFC, DK_IFEQS, DK_IFNC, DK_IFDEF, DK_IFNDEF, DK_IFNOTDEF,
> +    DK_ELSEIF, DK_ELSE, DK_ENDIF,
>      DK_SPACE, DK_SKIP, DK_FILE, DK_LINE, DK_LOC, DK_STABS,
>      DK_CFI_SECTIONS, DK_CFI_STARTPROC, DK_CFI_ENDPROC, DK_CFI_DEF_CFA,
>      DK_CFI_DEF_CFA_OFFSET, DK_CFI_ADJUST_CFA_OFFSET,
> DK_CFI_DEF_CFA_REGISTER,
> @@ -440,8 +441,8 @@ private:
>    bool parseDirectiveInclude(); // ".include"
>    bool parseDirectiveIncbin(); // ".incbin"
>
> -  // ".if" or ".ifne"
> -  bool parseDirectiveIf(SMLoc DirectiveLoc);
> +  // ".if", ".ifeq", ".ifge", ".ifgt" , ".ifle", ".iflt" or ".ifne"
> +  bool parseDirectiveIf(SMLoc DirectiveLoc, DirectiveKind DirKind);
>    // ".ifb" or ".ifnb", depending on ExpectBlank.
>    bool parseDirectiveIfb(SMLoc DirectiveLoc, bool ExpectBlank);
>    // ".ifc" or ".ifnc", depending on ExpectEqual.
> @@ -1236,8 +1237,13 @@ bool AsmParser::parseStatement(ParseStatementInfo
> &Info) {
>    default:
>      break;
>    case DK_IF:
> +  case DK_IFEQ:
> +  case DK_IFGE:
> +  case DK_IFGT:
> +  case DK_IFLE:
> +  case DK_IFLT:
>    case DK_IFNE:
> -    return parseDirectiveIf(IDLoc);
> +    return parseDirectiveIf(IDLoc, DirKind);
>    case DK_IFB:
>      return parseDirectiveIfb(IDLoc, true);
>    case DK_IFNB:
> @@ -3799,9 +3805,8 @@ bool AsmParser::parseDirectiveIncbin() {
>  }
>
>  /// parseDirectiveIf
> -/// ::= .if expression
> -/// ::= .ifne expression
> -bool AsmParser::parseDirectiveIf(SMLoc DirectiveLoc) {
> +/// ::= .if{,eq,ge,gt,le,lt,ne} expression
> +bool AsmParser::parseDirectiveIf(SMLoc DirectiveLoc, DirectiveKind
> DirKind) {
>    TheCondStack.push_back(TheCondState);
>    TheCondState.TheCond = AsmCond::IfCond;
>    if (TheCondState.Ignore) {
> @@ -3816,6 +3821,17 @@ bool AsmParser::parseDirectiveIf(SMLoc
> DirectiveLoc) {
>
>      Lex();
>
> +    if      (DirKind == DK_IFEQ)
>

There shouldn't be the extra space between the if and the (.

I think this might be nicer in a switch statement.


> +      ExprValue = ExprValue == 0;
> +    else if (DirKind == DK_IFGE)
> +      ExprValue = ExprValue >= 0;
> +    else if (DirKind == DK_IFGT)
> +      ExprValue = ExprValue > 0;
> +    else if (DirKind == DK_IFLE)
> +      ExprValue = ExprValue <= 0;
> +    else if (DirKind == DK_IFLT)
> +      ExprValue = ExprValue < 0;
> +
>      TheCondState.CondMet = ExprValue;
>      TheCondState.Ignore = !TheCondState.CondMet;
>    }
> @@ -4118,6 +4134,11 @@ void AsmParser::initializeDirectiveKindMap() {
>    DirectiveKindMap[".bundle_lock"] = DK_BUNDLE_LOCK;
>    DirectiveKindMap[".bundle_unlock"] = DK_BUNDLE_UNLOCK;
>    DirectiveKindMap[".if"] = DK_IF;
> +  DirectiveKindMap[".ifeq"] = DK_IFEQ;
> +  DirectiveKindMap[".ifge"] = DK_IFGE;
> +  DirectiveKindMap[".ifgt"] = DK_IFGT;
> +  DirectiveKindMap[".ifle"] = DK_IFLE;
> +  DirectiveKindMap[".iflt"] = DK_IFLT;
>    DirectiveKindMap[".ifne"] = DK_IFNE;
>    DirectiveKindMap[".ifb"] = DK_IFB;
>    DirectiveKindMap[".ifnb"] = DK_IFNB;
> diff --git a/test/MC/AsmParser/conditional_asm.s
> b/test/MC/AsmParser/conditional_asm.s
> index b9bee33..ecbceb1 100644
> --- a/test/MC/AsmParser/conditional_asm.s
> +++ b/test/MC/AsmParser/conditional_asm.s
> @@ -11,6 +11,66 @@
>      .endif
>  .endif
>
> +# CHECK: .byte 0
> +# CHECK-NOT: .byte 1
> +.ifeq 32 - 32
> +        .byte 0
> +.else
> +        .byte 1
> +.endif
> +
> +# CHECK: .byte 0
> +# CHECK: .byte 1
> +# CHECK-NOT: .byte 2
> +.ifge 32 - 31
> +        .byte 0
> +.endif
> +.ifge 32 - 32
> +        .byte 1
> +.endif
> +.ifge 32 - 33
> +        .byte 2
> +.endif
> +
> +# CHECK: .byte 0
> +# CHECK-NOT: .byte 1
> +# CHECK-NOT: .byte 2
> +.ifgt 32 - 31
> +        .byte 0
> +.endif
> +.ifgt 32 - 32
> +        .byte 1
> +.endif
> +.ifgt 32 - 33
> +        .byte 2
> +.endif
> +
> +# CHECK-NOT: .byte 0
> +# CHECK: .byte 1
> +# CHECK: .byte 2
> +.ifle 32 - 31
> +        .byte 0
> +.endif
> +.ifle 32 - 32
> +        .byte 1
> +.endif
> +.ifle 32 - 33
> +        .byte 2
> +.endif
> +
> +# CHECK-NOT: .byte 0
> +# CHECK-NOT: .byte 1
> +# CHECK: .byte 2
> +.iflt 32 - 31
> +        .byte 0
> +.endif
> +.iflt 32 - 32
> +        .byte 1
> +.endif
> +.iflt 32 - 33
> +        .byte 2
> +.endif
> +
>  # CHECK: .byte 1
>  # CHECK-NOT: .byte 0
>  .ifne 32 - 32
>

Can you please add some tests to cover diagnostics for bad input?


> --
> 1.9.3
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140611/fd314d8d/attachment.html>