This is an archive of the discontinued LLVM Phabricator instance.

[IR] Don't consider interposable globals to be dereferenceable
AbandonedPublic

Authored by nikic on Jan 31 2023, 3:02 AM.

Details

Summary

According to D78952, we should not be making assumption about the size (and thus dereferencability) of globals that don't have a definitive initializer.

This is a draft patch to make the implementation comply with the specification, though I haven't fixed all test failures yet:

Failed Tests (17):
  LLVM :: CodeGen/AArch64/GlobalISel/constant-dbg-loc.ll
  LLVM :: CodeGen/AArch64/GlobalISel/localizer-arm64-tti.ll
  LLVM :: CodeGen/AArch64/csr-split.ll
  LLVM :: CodeGen/AMDGPU/GlobalISel/irtranslator-invariant.ll
  LLVM :: CodeGen/PowerPC/GlobalISel/ppc-irtranslator-stackprotect.ll
  LLVM :: CodeGen/PowerPC/aix-cc-abi.ll
  LLVM :: CodeGen/PowerPC/aix-cc-byval.ll
  LLVM :: CodeGen/PowerPC/ctrloops-pseudo.ll
  LLVM :: CodeGen/PowerPC/lower-globaladdr32-aix.ll
  LLVM :: CodeGen/PowerPC/lower-globaladdr64-aix.ll
  LLVM :: CodeGen/PowerPC/out-of-range-dform.ll
  LLVM :: CodeGen/SystemZ/int-cmp-59.ll
  LLVM :: CodeGen/X86/fold-sext-trunc.ll
  LLVM :: CodeGen/X86/pr32610.ll
  LLVM :: CodeGen/X86/pr37916.ll
  LLVM :: CodeGen/X86/remat-constant.ll
  LLVM-Unit :: Analysis/./AnalysisTests/LoadsTest/CanReplacePointersIfEqual

Alternatively, we should change the spec to permit our current assumption of a minimum size

Diff Detail

Event Timeline

nikic created this revision.Jan 31 2023, 3:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2023, 3:02 AM
nikic requested review of this revision.Jan 31 2023, 3:02 AM
arsenm added a subscriber: arsenm.Jan 31 2023, 3:56 AM

I'd expect the minimum size behavior, otherwise what is even the point of having a size associated with the global declaration?

At the C source level, we should be able to assume that if someone writes "extern int x;", that has 4 dereferenceable bytes. (From the standard: "All declarations that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined.") If there's some incentive to change the way we express that, I'm open to proposals, but I don't think it makes sense to just drop that completely.

nikic abandoned this revision.Feb 1 2023, 1:43 AM

Okay, it sounds like the consensus if to change LangRef instead.

nikic added a comment.Feb 1 2023, 2:22 AM

LangRef patch: D143057