tlively added a comment.
I think this is a good direction overall, and I'm glad the code doesn't become
any messier with this change. I do think it would be good to also email
llvm-dev about this change to get general feedback and make sure it doesn't
require a full RFC.
================
Comment at: llvm/lib/AsmParser/LLParser.cpp:1470
+
// For calls we also accept variables in the program address space.
if (IsCall && isa<PointerType>(Ty)) {
----------------
Or maybe this comment can be removed since it is subsumed by the comment you
added just below.
================
Comment at: llvm/lib/AsmParser/LLParser.cpp:1491
getTypeString(Val->getType()) + "' but expected '" +
- getTypeString(SuggestedTy) + "'");
+ getTypeString(Ty) + "'");
return nullptr;
----------------
It would be good to add a test for this error message (if one doesn't already
exist). It's not obvious to me that the expected type in the error message will
always have a correct program address space.
================
Comment at: llvm/lib/IR/DataLayout.cpp:476
return Err;
+ ProgramAddrSpaces.push_back(pa);
break;
----------------
It would be good to check for repeats here. As it is,
`hasNonZeroProgramAddressSpace()` would return `true` for "P0-P0" .
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91428/new/
https://reviews.llvm.org/D91428
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
