[PATCH] D48803: Place the BlockAddress type in the address space of the containing function
Alexander Richardson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 28 06:01:46 PDT 2021
arichardson added a comment.
Thanks for the review, will update shortly.
================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3471-3473
+ // TODO: can this ever happen? I wasn't able to think of a test case
+ // where this would be triggered.
+ FwdDeclAS = M->getDataLayout().getProgramAddressSpace();
----------------
arsenm wrote:
> I prefer to not add code that you can't test and should be unreachable
Sounds good, will change to llvm_unreachable("").
================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3459
+ if (!ExpectedTy->isPointerTy() ||
+ !ExpectedTy->getPointerElementType()->isIntegerTy(8))
+ return error(
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D48803/new/
https://reviews.llvm.org/D48803
More information about the llvm-commits
mailing list