[PATCH] D48803: Place the BlockAddress type in the address space of the containing function

[Alexander Richardson via Phabricator via llvm-commits]

arichardson marked an inline comment as done.
arichardson added a comment.

I believe all remaining issues were addressed in the latest version.



================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3459
+          if (!ExpectedTy->isPointerTy() ||
+              !ExpectedTy->getPointerElementType()->isIntegerTy(8))
+            return error(
----------------
dylanmckay wrote:
> arichardson wrote:
> > arsenm wrote:
> > > arichardson wrote:
> > > > arsenm wrote:
> > > > > This should not depend on pointee element types
> > > > I've kept this check for now (with a TODO) and only updated the error message.
> > > No, just rip out the type check. It's not needed now, and if it is, it's the verifier's responsibility to check it
> > Will move check to verifier if there isn't one already.
> The error
> 
> ```
> +          if (!ExpectedTy->isPointerTy())
> +            return error(ID.Loc,
> +                         "type of blockaddress must be a pointer and not '" +
> +                             getTypeString(ExpectedTy) + "'");
> ```
> 
> still exists and hasn't been moved to the verifier or removed from this file.
The problem was checking the pointee type , which I've removed. We need to check if it's a pointer since otherwise we can't call `ExpectedTy->getPointerAddressSpace()`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D48803/new/

https://reviews.llvm.org/D48803