[mips] [IAS] Fix parsing of memory offset expressions with parenthesis depth >1.

[tomatabacu]

Summary

In an expression such as "(((a+b)+c)+d)", parseParenExpression() would only parse the "a+b)+c", which would result in an error later on in the parser.
This means that we can only parse one level of inner parentheses.

In order to fix this, I added a new function called parseParenExprOfDepth(), which parses a specified number of trailing parenthesis expressions
(except for the outermost parenthesis), and changed MipsAsmParser to use it in parseMemOffset instead of parseParenExpression().

[tomatabacu]

Ping.

[dsanders]

I already know why we need this in the short term (to parse %hi,%lo,etc. correctly) but it's not evident from the patch. Could you add some test-cases that depend on the MIPS-specific expression parsing?

[tomatabacu]

Added test case with MIPS-specific expression.

[dsanders]

LGTM. This seems like a reasonable compromise between the ideal solution (adding the ability to extend the expression parser to support MIPS-specific operators such as '%lo(X)') and getting the most common expressions working.

[tomatabacu]

Rafael, are the common-code additions ok with you ?

[• rafael]

LGTM

include/llvm/MC/MCParser/MCAsmParser.h

public:
   /// \param Res - The value of the absolute expression. The result is undefined
   /// on error.
   /// \return - False on success.
   virtual bool parseAbsoluteExpression(int64_t &Res) = 0;
 
   /// \brief Ensure that we have a valid section set in the streamer. Otherwise,
   /// report an error and switch to .text.
   virtual void checkForValidSection() = 0;
+
+  /// \brief Parse an arbitrary expression of a specified parenthesis depth,
+  /// assuming that the initial '(' characters have already been consumed.
+  ///
+  /// \param ParenDepth - Specifies how many trailing expressions outside the
+  /// current parentheses we have to parse.
+  /// \param Res - The value of the expression. The result is undefined
+  /// on error.
+  /// \return - False on success.
+  virtual bool parseParenExprOfDepth(unsigned ParenDepth, const MCExpr *&Res,
+                                     SMLoc &EndLoc) = 0;
 };
 
 /// \brief Create an MCAsmParser instance.
 MCAsmParser *createMCAsmParser(SourceMgr &, MCContext &, MCStreamer &,
                                const MCAsmInfo &);
 
 } // End llvm namespace
 
 #endif

lib/MC/MCParser/AsmParser.cpp

bool parseMSInlineAsm(void *AsmLoc, std::string &AsmString,
                         SmallVectorImpl<std::string> &Clobbers,
                         const MCInstrInfo *MII, const MCInstPrinter *IP,
                         MCAsmParserSemaCallback &SI) override;
 
   bool parseExpression(const MCExpr *&Res);
   bool parseExpression(const MCExpr *&Res, SMLoc &EndLoc) override;
   bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) override;
   bool parseParenExpression(const MCExpr *&Res, SMLoc &EndLoc) override;
+  bool parseParenExprOfDepth(unsigned ParenDepth, const MCExpr *&Res,
+                             SMLoc &EndLoc) override;
   bool parseAbsoluteExpression(int64_t &Res) override;
 
   /// \brief Parse an identifier or string (as a quoted identifier)
   /// and set \p Res to the identifier contents.
   bool parseIdentifier(StringRef &Res) override;
   void eatToEndOfStatement() override;
 
   void checkForValidSection() override;
bool AsmParser::parseExpression(const MCExpr *&Res, SMLoc &EndLoc) {
   return false;
 }
 
 bool AsmParser::parseParenExpression(const MCExpr *&Res, SMLoc &EndLoc) {
   Res = nullptr;
   return parseParenExpr(Res, EndLoc) || parseBinOpRHS(1, Res, EndLoc);
 }
 
+bool AsmParser::parseParenExprOfDepth(unsigned ParenDepth, const MCExpr *&Res,
+                                      SMLoc &EndLoc) {
+  if (parseParenExpr(Res, EndLoc))
+    return true;
+
+  for (; ParenDepth > 0; --ParenDepth) {
+    if (parseBinOpRHS(1, Res, EndLoc))
+      return true;
+
+    // We don't Lex() the last RParen.
+    // This is the same behavior as parseParenExpression().
+    if (ParenDepth - 1 > 0) {
+      if (Lexer.isNot(AsmToken::RParen))
+        return TokError("expected ')' in parentheses expression");
+      EndLoc = Lexer.getTok().getEndLoc();
+      Lex();
+    }
+  }
+  return false;
+}
+
 bool AsmParser::parseAbsoluteExpression(int64_t &Res) {
   const MCExpr *Expr;
 
   SMLoc StartLoc = Lexer.getLoc();
   if (parseExpression(Expr))
     return true;
 
   if (!Expr->EvaluateAsAbsolute(Res))

lib/Target/Mips/AsmParser/MipsAsmParser.cpp

bool MipsAsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
   assert(Operands.size() == 0);
   return (RegNo == (unsigned)-1);
 }
 
 bool MipsAsmParser::parseMemOffset(const MCExpr *&Res, bool isParenExpr) {
   MCAsmParser &Parser = getParser();
   SMLoc S;
   bool Result = true;
+  unsigned NumOfLParen = 0;
 
-  while (getLexer().getKind() == AsmToken::LParen)
+  while (getLexer().getKind() == AsmToken::LParen) {
     Parser.Lex();
+    ++NumOfLParen;
+  }
 
   switch (getLexer().getKind()) {
   default:
     return true;
   case AsmToken::Identifier:
   case AsmToken::LParen:
   case AsmToken::Integer:
   case AsmToken::Minus:
   case AsmToken::Plus:
     if (isParenExpr)
-      Result = getParser().parseParenExpression(Res, S);
+      Result = getParser().parseParenExprOfDepth(NumOfLParen, Res, S);
     else
       Result = (getParser().parseExpression(Res));
     while (getLexer().getKind() == AsmToken::RParen)
       Parser.Lex();
     break;
   case AsmToken::Percent:
     Result = parseRelocOperand(Res);
   }

test/MC/Mips/expr1.s

 # 32R2-EL:                             #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_Mips_LO16
 # 32R2-EL: lw   $4, 56($4)             # encoding: [0x38,0x00,0x84,0x8c]
 # 32R2-EL: lw   $4, %lo(foo+8)($4)     # encoding: [0x08'A',A,0x84,0x8c]
 # 32R2-EL:                             #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_Mips_LO16
 # 32R2-EL: lw   $4, %lo(foo+8)($4)     # encoding: [0x08'A',A,0x84,0x8c]
 # 32R2-EL:                             #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_Mips_LO16
 # 32R2-EL: lw   $4, %lo(foo+8)($4)     # encoding: [0x08'A',A,0x84,0x8c]
 # 32R2-EL:                             #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_Mips_LO16
+# 32R2-EL: lw   $4, 10($4)             # encoding: [0x0a,0x00,0x84,0x8c]
+# 32R2-EL: lw   $4, 15($4)             # encoding: [0x0f,0x00,0x84,0x8c]
+# 32R2-EL: lw   $4, 21($4)             # encoding: [0x15,0x00,0x84,0x8c]
+# 32R2-EL: lw   $4, 28($4)             # encoding: [0x1c,0x00,0x84,0x8c]
 # 32R2-EL: .space  64
 
 # MM-32R2-EL: .text
 # MM-32R2-EL: .globl  foo
 # MM-32R2-EL: foo:
 # MM-32R2-EL: lw   $4, %lo(foo)($4)    # encoding: [0x84'A',0xfc'A',0x00,0x00]
 # MM-32R2-EL:                          #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_MICROMIPS_LO16
 # MM-32R2-EL: lw   $4, 56($4)          # encoding: [0x84,0xfc,0x38,0x00]
 # MM-32R2-EL: lw   $4, %lo(foo+8)($4)  # encoding: [0x84'A',0xfc'A',0x08,0x00]
 # MM-32R2-EL:                          #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_MICROMIPS_LO16
 # MM-32R2-EL: lw   $4, %lo(foo+8)($4)  # encoding: [0x84'A',0xfc'A',0x08,0x00]
 # MM-32R2-EL:                          #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_MICROMIPS_LO16
 # MM-32R2-EL: lw   $4, %lo(foo+8)($4)  # encoding: [0x84'A',0xfc'A',0x08,0x00]
 # MM-32R2-EL:                          #   fixup A - offset: 0, value: foo@ABS_LO, kind: fixup_MICROMIPS_LO16
+# MM-32R2-EL: lw   $4, 10($4)          # encoding: [0x84,0xfc,0x0a,0x00]
+# MM-32R2-EL: lw   $4, 15($4)          # encoding: [0x84,0xfc,0x0f,0x00]
+# MM-32R2-EL: lw   $4, 21($4)          # encoding: [0x84,0xfc,0x15,0x00]
+# MM-32R2-EL: lw   $4, 28($4)          # encoding: [0x84,0xfc,0x1c,0x00]
 # MM-32R2-EL: .space  64
 
   .globl  foo
   .ent  foo
 foo:
   lw  $4,%lo(foo)($4)
   lw  $4,((10 + 4) * 4)($4)
   lw  $4,%lo (2 * 4) + foo($4)
   lw  $4,%lo((2 * 4) + foo)($4)
   lw  $4,(((%lo ((2 * 4) + foo))))($4)
+  lw  $4, (((1+2)+3)+4)($4)
+  lw  $4, ((((1+2)+3)+4)+5)($4)
+  lw  $4, (((((1+2)+3)+4)+5)+6)($4)
+  lw  $4, ((((((1+2)+3)+4)+5)+6)+7)($4)
   .space  64
   .end  foo