llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.57k forks source link

_LP64 macro set incorrectly in clang/lib/Basic/Targets/AArch64.cpp #60743

Open NickLewisL3H opened 1 year ago

NickLewisL3H commented 1 year ago

The _LP64 macro is set in clang/lib/Basic/Targets/AArch64.cpp

n_lewis@a009906:~/llvm-project$ tail +225 clang/lib/Basic/Targets/AArch64.cpp | head -5
  // Target properties.
  if (!getTriple().isOSWindows() && getTriple().isArch64Bit()) {
    Builder.defineMacro("_LP64");
    Builder.defineMacro("__LP64__");
  }

I think this is incorrect for two reasons:

  1. The setting of _LP64 is the responsibility of clang/lib/Frontend/InitPreprocessor.cpp
  2. The AArch64.cpp code does not check that pointers and longs are actually 64-bits. For example AArch64 Morello has 128-bit pointers
jrtc27 commented 1 year ago

Well, the latter is only for the Morello downstream fork as all supported AArch64 variants upstream work for this, but the code does seem unnecessary given the generic code in InitPreprocessor checks precisely for I32 (that one's a bit dubious, should probably just be < 64, but eh, 16-bit ints with 64-bit longs and pointers would be a weird target that most real-world code would hate), L64 and P64. Deleting it should be NFC upstream and fix the issue downstream.

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-driver

llvmbot commented 1 year ago

@llvm/issue-subscribers-backend-aarch64