[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

Mon Apr 20 16:50:32 PDT 2020

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