[AsmParser] Check that addrspace fits within 24 bits

[luke]

Summary

Address spaces equal or larger than 1 << 24 don't fit and produce an
assertion during debug builds, or worse in release. This causes an error
to be reported during parsing instead.

[luke]

Adding tests that were forgotten

[arichardson]

Minor suggestions inline. Would you mind rebasing this on https://reviews.llvm.org/D138789 (which I will commit once approved). Should make the diff slightly smaller.

llvm/lib/AsmParser/LLParser.cpp

1795–1796

To match the logic in DataLayout.cpp. However, ideally we should have a shared constant somewhere instead of duplicating this knowledge.

[luke]

Rebased upon D138789

[asb]

llvm/lib/AsmParser/LLParser.cpp

1794

Nit: I think "24-bit" would be consistent with similar strings in LLVM.

[luke]

Adjust diagnostic string

[nikic]

llvm/test/Assembler/invalid-addrspace.ll

5 ↗

(On Diff #481252)

Does not match the updated diagnostic.

[luke]

Update test with new diagnostic

[nikic]

LGTM

llvm/lib/AsmParser/LLParser.cpp

 
 /// parseOptionalAddrSpace
 ///   := /*empty*/
 ///   := 'addrspace' '(' uint32 ')'
 bool LLParser::parseOptionalAddrSpace(unsigned &AddrSpace, unsigned DefaultAS) {
   AddrSpace = DefaultAS;
   if (!EatIfPresent(lltok::kw_addrspace))
     return false;
-  return parseToken(lltok::lparen, "expected '(' in address space") ||
+  SMLoc loc = Lex.getLoc();
+  bool HaveError = parseToken(lltok::lparen, "expected '(' in address space") ||
                    parseUInt32(AddrSpace) ||
                    parseToken(lltok::rparen, "expected ')' in address space");
+  if (HaveError)
+    return true;
+  if (AddrSpace >= (1 << 24))
+    return error(loc, "address space too large to fit in 24 bits");
+  return false;
 }
 
 /// parseStringAttribute
 ///   := StringConstant
 ///   := StringConstant '=' StringConstant
 bool LLParser::parseStringAttribute(AttrBuilder &B) {
   std::string Attr = Lex.getStrVal();
   Lex.Lex();