arichardson marked an inline comment as done. arichardson added a comment. In D78491#1992618 <https://reviews.llvm.org/D78491#1992618>, @aykevl wrote:
> I can't give an LGTM but at least from my perspective I very much welcome > this change. I am still hitting problems with non-zero address spaces on AVR. > One nit in an inline comment. In case it helps, our changes can be seen here: https://github.com/CTSRD-CHERI/llvm-project. However, we made many of those changes before there was a program address space. For us all pointers are in AS200, so we initially had a getDefaultAS() helper and use that in cases where it should be program/globals address space. That is one of the things I intend to fix while upstreaming. > Do you plan on eventually removing `LLVM_NO_IMPLICIT_ADDRESS_SPACE` when the > transition is finished? If so, I think it's worth documenting this somewhere > - perhaps at llvm/lib/IR/CMakeLists.txt. Yes, ultimately the goal is to remove the default parameters. It might also make sense to switch the define from an opt-out `LLVM_NO_IMPLICIT_ADDRESS_SPACE` to and opt-in `LLVM_NEED_IMPLICIT_ADDRESS_SPACE_ZERO` for individual directories/projects that have not been ported to the explicit APIs yet. ================ Comment at: llvm/include/llvm/IR/DataLayout.h:356 /// Layout pointer alignment - Align getPointerABIAlignment(unsigned AS) const; ---------------- aykevl wrote: > There is no default address space in this declaration? Good catch, I did not mean to change this line. Copy-paste error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78491/new/ https://reviews.llvm.org/D78491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
