[PATCH] D109337: [GlobalOpt][FIX] Do not embed initializers into AS!=0 globals

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert marked an inline comment as done.
jdoerfert added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h:52
+  bool canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const {
+    return AS != AddressSpace::ADDRESS_SPACE_SHARED;
+  }
----------------
tra wrote:
> jdoerfert wrote:
> > tra wrote:
> > > While we should not have globals with ADDRESS_SPACE_LOCAL  or ADDRESS_SPACE_PARAM, it may be worth to return false for them, too.
> > That sounds to me more like an assertion. I'll add one for now, you let me know if that is ok too.
> Assertions are for compiler programming errors, not for diagnosing invalid user-provided input.
> Considering that one can write IR with a global var in an invalid AS I think assertion is not the right choice here.
> An error would be more appropriate, IMO.
TBH, I think such an input is broken and the verifier should trigger, which it does not: https://godbolt.org/z/xr1v361Wo
Assuming the IR is "valid" we should not misuse this interface with an AS that is not allowed for globals, IMHO.
That said, I'm not even sure what "an error" would look like here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109337