[llvm] ac2bec5 - OpaquePtr: Support i32** with --force-opaque-pointers

[Duncan P. N. Exon Smith via llvm-commits]

Author: Duncan P. N. Exon Smith
Date: 2021-06-29T14:10:29-07:00
New Revision: ac2bec5addd2f96e976242bc8e0d93725fe3d2fd

URL: https://github.com/llvm/llvm-project/commit/ac2bec5addd2f96e976242bc8e0d93725fe3d2fd
DIFF: https://github.com/llvm/llvm-project/commit/ac2bec5addd2f96e976242bc8e0d93725fe3d2fd.diff

LOG: OpaquePtr: Support i32** with --force-opaque-pointers

4506f614cb6983a16d117cf77a968608e66d7a5c fixed parsing of textual IR to
reject `ptr*`, but broke the auto-conversion of `i32**` to `ptr` with
`--force-opaque-pointers`.

Get that working again by refactoring LLParser::parseType to only send
`ptr`-spelled pointers into the type suffix logic when it's the return
of a function type. This also rejects `ptr addrspace(3) addrspace(2)`,
which 1e6303e60ca5af4fbe7ca728572fd65666a98271 invadvertently started
accepting. Just the default top-level error message for the
double-addrspace since I had trouble thinking of something nice;
probably it's fine as is (it doesn't look valid the way that `ptr*`
does).

Differential Revision: https://reviews.llvm.org/D105146

Added: 
    llvm/test/Assembler/invalid-opaque-ptr-double-addrspace.ll

Modified: 
    llvm/lib/AsmParser/LLParser.cpp
    llvm/test/Assembler/opaque-ptr.ll
    llvm/test/Other/force-opaque-ptrs.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 30057866fb3ed..f9f73d2a4ffd4 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -2586,11 +2586,24 @@ bool LLParser::parseType(Type *&Result, const Twine &Msg, bool AllowVoid) {
   }
   }
 
+  // Handle (explicit) opaque pointer types (not --force-opaque-pointers).
+  //
+  // Type ::= ptr ('addrspace' '(' uint32 ')')?
   if (Result->isOpaquePointerTy()) {
     unsigned AddrSpace;
     if (parseOptionalAddrSpace(AddrSpace))
       return true;
     Result = PointerType::get(getContext(), AddrSpace);
+
+    // Give a nice error for 'ptr*'.
+    if (Lex.getKind() == lltok::star)
+      return tokError("ptr* is invalid - use ptr instead");
+
+    // Fall through to parsing the type suffixes only if this 'ptr' is a
+    // function return. Otherwise, return success, implicitly rejecting other
+    // suffixes.
+    if (Lex.getKind() != lltok::lparen)
+      return false;
   }
 
   // parse the type suffixes.
@@ -2608,8 +2621,6 @@ bool LLParser::parseType(Type *&Result, const Twine &Msg, bool AllowVoid) {
         return tokError("basic block pointers are invalid");
       if (Result->isVoidTy())
         return tokError("pointers to void are invalid - use i8* instead");
-      if (Result->isOpaquePointerTy())
-        return tokError("ptr* is invalid - use ptr instead");
       if (!PointerType::isValidElementType(Result))
         return tokError("pointer to this type is invalid");
       Result = PointerType::getUnqual(Result);

diff  --git a/llvm/test/Assembler/invalid-opaque-ptr-double-addrspace.ll b/llvm/test/Assembler/invalid-opaque-ptr-double-addrspace.ll
new file mode 100644
index 0000000000000..733c43d8f3c1c
--- /dev/null
+++ b/llvm/test/Assembler/invalid-opaque-ptr-double-addrspace.ll
@@ -0,0 +1,4 @@
+; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s
+
+; CHECK: expected top-level entity
+ at g1 = external global ptr addrspace(3) addrspace(4)

diff  --git a/llvm/test/Assembler/opaque-ptr.ll b/llvm/test/Assembler/opaque-ptr.ll
index 9167ca9e45866..5ee57fae18f06 100644
--- a/llvm/test/Assembler/opaque-ptr.ll
+++ b/llvm/test/Assembler/opaque-ptr.ll
@@ -4,6 +4,13 @@
 ; CHECK: @global = external global ptr
 @global = external global ptr
 
+; CHECK: @fptr1 = external global ptr ()*
+; CHECK: @fptr2 = external global ptr () addrspace(1)*
+; CHECK: @fptr3 = external global ptr () addrspace(1)* addrspace(2)*
+ at fptr1 = external global ptr ()*
+ at fptr2 = external global ptr () addrspace(1)*
+ at fptr3 = external global ptr () addrspace(1)* addrspace(2)*
+
 ; CHECK: define ptr @f(ptr %a) {
 ; CHECK:     %b = bitcast ptr %a to ptr
 ; CHECK:     ret ptr %b

diff  --git a/llvm/test/Other/force-opaque-ptrs.ll b/llvm/test/Other/force-opaque-ptrs.ll
index 442ef0ac9eb0a..779c5a158bf5a 100644
--- a/llvm/test/Other/force-opaque-ptrs.ll
+++ b/llvm/test/Other/force-opaque-ptrs.ll
@@ -32,3 +32,15 @@ define void @f(i32* %p) {
 
 @g.fwd = global i32 0
 declare void @fn.fwd(i32)
+
+define void @f2(i32** %p) {
+; CHECK-LABEL: define {{[^@]+}}@f2(
+; CHECK-SAME: ptr {{%.*}}) {
+  unreachable
+}
+
+define void @f3(i32 addrspace(1)* addrspace(2)* %p) {
+; CHECK-LABEL: define {{[^@]+}}@f3(
+; CHECK-SAME: ptr addrspace(2) {{%.*}}) {
+  unreachable
+}