[PATCH] D101338: [MS] Fix crash when calling __cpuid with /EHsc and -mavx2

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 340730.
pengfei added a comment.

Yes, it is a pretty implementation. Thanks Craig.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101338

Files:
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/ms-intrinsics-cpuid.c


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
-// RUN: -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck %s
+// RUN: -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck %s 
--check-prefix=X86
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=17.00 \
-// RUN: -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s
+// RUN: -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s 
--check-prefix=X64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -12,7 +12,12 @@
 void test__cpuid(int *info, int level) {
   __cpuid(info, level);
 }
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
-// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
-// CHECK-SAME:   
"={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
-// CHECK-SAME:   (i32 %{{.*}}, i32 0)
+// X86-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// X86: call { i32, i32, i32, i32 } asm "cpuid",
+// X86-SAME:   "={ax},={bx},={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"
+// X86-SAME:   (i32 %{{.*}}, i32 0)
+
+// X64-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// X64: call { i32, i32, i32, i32 } asm "xchgq %rbx{{.*}}cpuid{{.*}}xchgq 
%rbx{{.*}}",
+// X64-SAME:   "={ax},=r,={cx},={dx},0,2,~{dirflag},~{fpsr},~{flags}"
+// X64-SAME:   (i32 %{{.*}}, i32 0)
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -507,16 +507,25 @@
 |* Misc
 
\**/
 #if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__)
+#define __cpuid_count(__leaf, __count, __eax, __ebx, __ecx, __edx) \
+__asm("cpuid" : "=a"(__eax), "=b" (__ebx), "=c"(__ecx), "=d"(__edx) \
+  : "0"(__leaf), "2"(__count))
+#else
+/* x86-64 uses %rbx as the base register, so preserve it. */
+#define __cpuid_count(__leaf, __count, __eax, __ebx, __ecx, __edx) \
+__asm("xchgq %%rbx,%q1\n" \
+  "cpuid\n" \
+  "xchgq %%rbx,%q1" \
+: "=a"(__eax), "=r" (__ebx), "=c"(__ecx), "=d"(__edx) \
+: "0"(__leaf), "2"(__count))
+#endif
 static __inline__ void __DEFAULT_FN_ATTRS __cpuid(int __info[4], int __level) {
-  __asm__("cpuid"
-  : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
-  : "a"(__level), "c"(0));
+  __cpuid_count(__level, 0, __info[0], __info[1], __info[2], __info[3]);
 }
 static __inline__ void __DEFAULT_FN_ATTRS __cpuidex(int __info[4], int __level,
 int __ecx) {
-  __asm__("cpuid"
-  : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
-  : "a"(__level), "c"(__ecx));
+  __cpuid_count(__level, __ecx, __info[0], __info[1], __info[2], __info[3]);
 }
 static __inline__ void __DEFAULT_FN_ATTRS __halt(void) {
   __asm__ volatile("hlt");


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
-// RUN: -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck %s
+// RUN: -triple i686-windows-msvc -emit-llvm %s -o - | FileCheck %s --check-prefix=X86
 // RUN: %clang_cc1 -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 \
-// RUN: -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s
+// RUN: -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s --check-prefix=X64
 
 // intrin.h needs size_t, but -ffreestanding prevents us from getting it from
 // stddef.h.  Work around it with this typedef.
@@ -12,7 +12,12 @@
 void test__cpuid(int *info, int level) {
   __cpuid(info, level);
 }
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
-// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
-// CHECK-SAME:   "={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
-// CHECK-SAME:   (i32 %{{.*}}, i32 0)
+// X86-LABEL: define {{.*}} 

[PATCH] D101194: [Driver] Push multiarch path setup to individual drivers

[Petr Hosek via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4537c3f51bc: [Driver] Push multiarch path setup to 
individual drivers (authored by phosek).
Herald added subscribers: libcxx-commits, s.egerton, simoncook, arichardson, 
mgorny.
Herald added a project: libc++.
Herald added a reviewer: libc++.

Changed prior to commit:
  https://reviews.llvm.org/D101194?vs=340142=340726#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101194

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h
  libcxx/utils/ci/run-buildbot

Index: libcxx/utils/ci/run-buildbot
===
--- libcxx/utils/ci/run-buildbot
+++ libcxx/utils/ci/run-buildbot
@@ -387,7 +387,7 @@
   -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
   -DLLVM_ENABLE_PROJECTS="clang" \
   -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" \
-  -DLLVM_RUNTIME_TARGETS="x86_64-unknown-linux-gnu"
+  -DLLVM_RUNTIME_TARGETS="x86_64-linux-gnu"
 
 echo "+++ Running the libc++ and libc++abi tests"
 ${NINJA} -C "${BUILD_DIR}" check-runtimes
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -70,6 +70,10 @@
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
   Tool *buildLinker() const override;
+
+  std::string getMultiarchTriple(const Driver ,
+ const llvm::Triple ,
+ StringRef SysRoot) const override;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -26,9 +26,9 @@
 
 /// Following the conventions in https://wiki.debian.org/Multiarch/Tuples,
 /// we remove the vendor field to form the multiarch triple.
-static std::string getMultiarchTriple(const Driver ,
-  const llvm::Triple ,
-  StringRef SysRoot) {
+std::string WebAssembly::getMultiarchTriple(const Driver ,
+const llvm::Triple ,
+StringRef SysRoot) const {
 return (TargetTriple.getArchName() + "-" +
 TargetTriple.getOSAndEnvironmentName()).str();
 }
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -253,6 +253,8 @@
   if (IsAndroid || Distro.IsOpenSUSE())
 ExtraOpts.push_back("--enable-new-dtags");
 
+  addPathIfExists(D, getRuntimePath(), getLibraryPaths());
+
   // The selection of paths to try here is designed to match the patterns which
   // the GCC driver itself uses, as this is part of the GCC-compatible driver.
   // This was determined by running GCC in a fake filesystem, creating all
@@ -260,6 +262,8 @@
   // to the link paths.
   path_list  = getFilePaths();
 
+  addPathIfExists(D, getStdlibPath(), Paths);
+
   const std::string OSLibDir = std::string(getOSLibDir(Triple, Args));
   const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
 
Index: clang/lib/Driver/ToolChains/Gnu.h
===
--- clang/lib/Driver/ToolChains/Gnu.h
+++ clang/lib/Driver/ToolChains/Gnu.h
@@ -310,11 +310,6 @@
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 
-  virtual std::string getMultiarchTriple(const Driver ,
- const llvm::Triple ,
- StringRef SysRoot) const
-  { return TargetTriple.str(); }
-
   /// \name ToolChain Implementation Helper Functions
   /// @{
 
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -97,6 +97,10 @@
 
 protected:
   Tool *buildLinker() const override;
+
+  std::string getMultiarchTriple(const Driver ,
+ const llvm::Triple ,
+ StringRef SysRoot) const override;
 };
 
 } // end namespace toolchains
Index: 

[clang] b4537c3 - [Driver] Push multiarch path setup to individual drivers

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2021-04-26T22:17:26-07:00
New Revision: b4537c3f51bc6c011ddd9c10b80043ac4ce16a01

URL: 
https://github.com/llvm/llvm-project/commit/b4537c3f51bc6c011ddd9c10b80043ac4ce16a01
DIFF: 
https://github.com/llvm/llvm-project/commit/b4537c3f51bc6c011ddd9c10b80043ac4ce16a01.diff

LOG: [Driver] Push multiarch path setup to individual drivers

Different platforms use different rules for multiarch triples so
it's difficult to provide a single method for all platforms. We
instead move the getMultiarchTriple to the ToolChain class and let
individual platforms override it and provide their custom logic.

Differential Revision: https://reviews.llvm.org/D101194

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/lib/Driver/ToolChains/Fuchsia.h
clang/lib/Driver/ToolChains/Gnu.h
clang/lib/Driver/ToolChains/Linux.cpp
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/lib/Driver/ToolChains/WebAssembly.h
libcxx/utils/ci/run-buildbot

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 27391bdafb3e..7ae8a3cd9bbe 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -91,7 +91,7 @@ if(WIN32)
   set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE 
STRING "")
 endif()
 
-foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)
+foreach(target 
aarch64-linux-gnu;armv7-linux-gnueabihf;i386-linux-gnu;x86_64-linux-gnu)
   if(LINUX_${target}_SYSROOT)
 # Set the per-target builtins options.
 list(APPEND BUILTIN_TARGETS "${target}")
@@ -140,111 +140,111 @@ foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unkn
 endforeach()
 
 if(FUCHSIA_SDK)
-  set(FUCHSIA_aarch64_NAME arm64)
-  set(FUCHSIA_i386_NAME x64)
-  set(FUCHSIA_x86_64_NAME x64)
-  set(FUCHSIA_riscv64_NAME riscv64)
-  foreach(target i386;x86_64;aarch64;riscv64)
-set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target}-unknown-fuchsia 
-I${FUCHSIA_SDK}/pkg/fdio/include")
+  set(FUCHSIA_aarch64-fuchsia_NAME arm64)
+  set(FUCHSIA_i386-fuchsia_NAME x64)
+  set(FUCHSIA_x86_64-fuchsia_NAME x64)
+  set(FUCHSIA_riscv64-fuchsia_NAME riscv64)
+  foreach(target i386-fuchsia;x86_64-fuchsia;aarch64-fuchsia;riscv64-fuchsia)
+set(FUCHSIA_${target}_COMPILER_FLAGS "--target=${target} 
-I${FUCHSIA_SDK}/pkg/fdio/include")
 set(FUCHSIA_${target}_LINKER_FLAGS 
"-L${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/lib")
 set(FUCHSIA_${target}_SYSROOT 
"${FUCHSIA_SDK}/arch/${FUCHSIA_${target}_NAME}/sysroot")
   endforeach()
 
-  foreach(target i386;x86_64;aarch64;riscv64)
+  foreach(target i386-fuchsia;x86_64-fuchsia;aarch64-fuchsia;riscv64-fuchsia)
 # Set the per-target builtins options.
-list(APPEND BUILTIN_TARGETS "${target}-unknown-fuchsia")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_SYSTEM_NAME Fuchsia CACHE 
STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_BUILD_TYPE RelWithDebInfo 
CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_ASM_FLAGS 
${FUCHSIA_${target}_COMPILER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_C_FLAGS 
${FUCHSIA_${target}_COMPILER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_CXX_FLAGS 
${FUCHSIA_${target}_COMPILER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_SHARED_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_MODULE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
-set(BUILTINS_${target}-unknown-fuchsia_CMAKE_SYSROOT 
${FUCHSIA_${target}_SYSROOT} CACHE PATH "")
+list(APPEND BUILTIN_TARGETS "${target}")
+set(BUILTINS_${target}_CMAKE_SYSTEM_NAME Fuchsia CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_ASM_FLAGS ${FUCHSIA_${target}_COMPILER_FLAGS} 
CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_C_FLAGS ${FUCHSIA_${target}_COMPILER_FLAGS} 
CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_CXX_FLAGS ${FUCHSIA_${target}_COMPILER_FLAGS} 
CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_SHARED_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_MODULE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_EXE_LINKER_FLAGS 
${FUCHSIA_${target}_LINKER_FLAGS} CACHE STRING "")
+set(BUILTINS_${target}_CMAKE_SYSROOT 

[clang] f2a585e - [NFC] Fix "not used" warning

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2021-04-26T22:09:23-07:00
New Revision: f2a585e6d392f4c8587c1dac4d776037d4b588c5

URL: 
https://github.com/llvm/llvm-project/commit/f2a585e6d392f4c8587c1dac4d776037d4b588c5
DIFF: 
https://github.com/llvm/llvm-project/commit/f2a585e6d392f4c8587c1dac4d776037d4b588c5.diff

LOG: [NFC] Fix "not used" warning

Added: 


Modified: 
clang/lib/Serialization/ASTReaderStmt.cpp
lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
llvm/tools/llvm-dwp/llvm-dwp.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderStmt.cpp 
b/clang/lib/Serialization/ASTReaderStmt.cpp
index 30fb627c2a61..ce6ef36c30da 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1099,10 +1099,9 @@ void ASTStmtReader::VisitCastExpr(CastExpr *E) {
 
 void ASTStmtReader::VisitBinaryOperator(BinaryOperator *E) {
   bool hasFP_Features;
-  BinaryOperator::Opcode opc;
   VisitExpr(E);
   E->setHasStoredFPFeatures(hasFP_Features = Record.readInt());
-  E->setOpcode(opc = (BinaryOperator::Opcode)Record.readInt());
+  E->setOpcode((BinaryOperator::Opcode)Record.readInt());
   E->setLHS(Record.readSubExpr());
   E->setRHS(Record.readSubExpr());
   E->setOperatorLoc(readSourceLocation());

diff  --git a/lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp 
b/lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
index 0a59e24c47a8..164a283b972b 100644
--- a/lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
+++ b/lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
@@ -836,9 +836,8 @@ dataExtractorFromSection(const NormalizedFile 
,
 //inspection" code if possible.
 static uint64_t getCUAbbrevOffset(llvm::DataExtractor abbrevData,
   uint64_t abbrCode) {
-  uint64_t curCode;
   uint64_t offset = 0;
-  while ((curCode = abbrevData.getULEB128()) != abbrCode) {
+  while (abbrevData.getULEB128() != abbrCode) {
 // Tag
 abbrevData.getULEB128();
 // DW_CHILDREN

diff  --git a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp 
b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
index d9b33face569..f20d739c4d41 100644
--- a/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
@@ -502,7 +502,6 @@ LoopIdiomRecognize::isLegalStore(StoreInst *SI) {
   // are stored.  A store of i32 0x01020304 can never be turned into a memset,
   // but it can be turned into memset_pattern if the target supports it.
   Value *SplatValue = isBytewiseValue(StoredVal, *DL);
-  Constant *PatternValue = nullptr;
 
   // Note: memset and memset_pattern on unordered-atomic is yet not supported
   bool UnorderedAtomic = SI->isUnordered() && !SI->isSimple();
@@ -515,10 +514,11 @@ LoopIdiomRecognize::isLegalStore(StoreInst *SI) {
   CurLoop->isLoopInvariant(SplatValue)) {
 // It looks like we can use SplatValue.
 return LegalStoreKind::Memset;
-  } else if (!UnorderedAtomic && HasMemsetPattern && !DisableLIRP::Memset &&
- // Don't create memset_pattern16s with address spaces.
- StorePtr->getType()->getPointerAddressSpace() == 0 &&
- (PatternValue = getMemSetPatternValue(StoredVal, DL))) {
+  }
+  if (!UnorderedAtomic && HasMemsetPattern && !DisableLIRP::Memset &&
+  // Don't create memset_pattern16s with address spaces.
+  StorePtr->getType()->getPointerAddressSpace() == 0 &&
+  getMemSetPatternValue(StoredVal, DL)) {
 // It looks like we can use PatternValue!
 return LegalStoreKind::MemsetPattern;
   }

diff  --git a/llvm/tools/llvm-dwp/llvm-dwp.cpp 
b/llvm/tools/llvm-dwp/llvm-dwp.cpp
index 5cbe1a34392c..790541c08aaf 100644
--- a/llvm/tools/llvm-dwp/llvm-dwp.cpp
+++ b/llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -201,10 +201,9 @@ static void writeStringsAndOffsets(MCStreamer , 
DWPStringPool ,
 }
 
 static uint64_t getCUAbbrev(StringRef Abbrev, uint64_t AbbrCode) {
-  uint64_t CurCode;
   uint64_t Offset = 0;
   DataExtractor AbbrevData(Abbrev, true, 0);
-  while ((CurCode = AbbrevData.getULEB128()) != AbbrCode) {
+  while (AbbrevData.getULEB128() != AbbrCode) {
 // Tag
 AbbrevData.getULEB128();
 // DW_CHILDREN



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

[Georgy Komarov via Phabricator via cfe-commits]

jubnzv updated this revision to Diff 340724.

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

https://reviews.llvm.org/D101259

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
@@ -58,3 +58,7 @@
   (void)__builtin_isinf_sign(0.f);
   (void)__builtin_prefetch(nullptr);
 }
+
+// __builtin_ms_va_list desugared as 'char *'. This test checks whether we are
+// handling this case correctly and not generating false positives.
+void no_false_positive_desugar_ms_va_list(char *in) { char *tmp = in; }
Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -0,0 +1,26 @@
+// Purpose:
+// Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
+// built-in va_list on Windows systems.
+
+// REQUIRES: system-windows
+
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+
+void test_ms_va_list(int a, ...) {
+  __builtin_ms_va_list ap;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type 
va_list; use variadic templates instead
+  __builtin_ms_va_start(ap, a);
+  int b = __builtin_va_arg(ap, int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use va_arg to define 
c-style vararg functions; use variadic templates instead
+  __builtin_ms_va_end(ap);
+}
+
+void test_typedefs(int a, ...) {
+  typedef __builtin_ms_va_list my_va_list1;
+  my_va_list1 ap1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type 
va_list; use variadic templates instead
+
+  using my_va_list2 = __builtin_ms_va_list;
+  my_va_list2 ap2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type 
va_list; use variadic templates instead
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp
@@ -60,8 +60,24 @@
 AST_MATCHER(QualType, isVAList) {
   ASTContext  = Finder->getASTContext();
   QualType Desugar = Node.getDesugaredType(Context);
-  return Context.getBuiltinVaListType().getDesugaredType(Context) == Desugar ||
- Context.getBuiltinMSVaListType().getDesugaredType(Context) == Desugar;
+
+  // __builtin_ms_va_list is internally implemented as 'char *'. This means 
that
+  // the node desugared as 'char *' potentially can be a __builtin_ms_va_list.
+  // So we need to remove all typedefs one by one to check this.
+  if (Desugar == Context.getBuiltinMSVaListType().getDesugaredType(Context)) {
+const auto MSVaListType = Context.getBuiltinMSVaListType();
+QualType Ty;
+do {
+  Ty = Desugar;
+  Desugar = Ty.getSingleStepDesugaredType(Context);
+  if (Desugar == MSVaListType)
+return true;
+} while (Desugar != Ty);
+// No need to check the other cases. We have a regular pointer here.
+return false;
+  }
+
+  return Context.getBuiltinVaListType().getDesugaredType(Context) == Desugar;
 }
 
 AST_MATCHER_P(AdjustedType, hasOriginalType,


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp
@@ -58,3 +58,7 @@
   (void)__builtin_isinf_sign(0.f);
   (void)__builtin_prefetch(nullptr);
 }
+
+// __builtin_ms_va_list desugared as 'char *'. This test checks whether we are
+// handling this case correctly and not generating false positives.
+void no_false_positive_desugar_ms_va_list(char *in) { char *tmp = in; }
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp
@@ -0,0 +1,26 @@
+// Purpose:
+// Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the
+// built-in va_list on Windows systems.
+
+// REQUIRES: system-windows
+
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t
+
+void test_ms_va_list(int a, ...) {
+  

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D100739#2718681 , @rjmccall wrote:

> Here are the options I think the committee might take:
>
> 1. Always select an unaligned allocator and force implementors to dynamically 
> align.  This seems unlikely to me.
> 2. Allow an aligned allocator to be selected.  The issue here is that we 
> cannot know until coroutine splitting whether the frame has a new-extended 
> alignment.  So there are some sub-options:
>
> 2a. Always use an aligned allocator if available, even if the frame ends up 
> not being aligned.  I think this is unlikely.
> 2b. Use the correct allocator for the frame alignment, using the alignment 
> inferred from the immediate coroutine body.  This would force implementations 
> to avoid doing anything prior to coroutine splitting that would increase the 
> frame's alignment beyond the derived limit.  This would be quite annoying for 
> implementors, and it would have strange performance cliffs, but it's 
> theoretically simple.
> 2c. Use the correct allocator for the frame alignment; only that allocator is 
> formally ODR-used.  This would force implementations to not ODR-use the right 
> allocator until coroutine lowering, which is not really reasonable; we should 
> be able to dissuade the committee from picking this option.
> 2d. Use the correct allocator for the frame alignment; both allocators are 
> (allowed to be) ODR-used, but only one would be dynamically used.  This is 
> what would be necessary for the implementation I suggested above.  In reality 
> there won't be any dynamic overhead because we should always be able to fold 
> the branch after allocation.
>
> I think 2d is obvious enough that we could go ahead and implement it pending 
> standardization.  There's still a question of what to do if the promise class 
> only provides an unaligned `operator new`, but the only reasonable behavior 
> is to dynamically align: unlike with `new` expressions, there's no way the 
> promise class could be expected to know/satisfy the appropriate alignment 
> requirement in general, so assuming alignment is not acceptable.  (Neither is 
> rejecting the program if it doesn't provide both — this wouldn't be 
> compatible with existing behavior.)
>
>> *(void**) (frame + size) = rawFrame; this means we always need the extra 
>> space to store the raw frame ptr. If either doing what the patch is 
>> currently doing or add another intrinsic say "llvm.coro.raw.frame.ptr.index" 
>> to do *(void**) (frame + llvm.coro.raw.frame.ptr.index()) = rawFrame;, it is 
>> likely that the extra pointer could reuse some existing paddings in the 
>> frame. There is an example of this in https://reviews.llvm.org/P8260. What 
>> do you think?
>
> You're right that there might be space in the frame we could re-use.  I was 
> thinking that it would be a shame to always add storage to the frame for the 
> raw frame pointer, but maybe the contract of `llvm.coro.raw.frame.ptr.offset` 
> could be that it's only meaningful if the frame has extended alignment.  
> Coroutine splitting would determine if any of the frame members was 
> over-aligned and add a raw-pointer field if so. We'd be stuck allocating 
> space in the frame even when allocation was elided, but stack space is cheap.
>
> It does need to be an offset instead of a type index, though; the frontend 
> will be emitting a GEP and will not know the frame type.

Sounds good to me. Thanks. I'll go ahead with `llvm.coro.raw.frame.ptr.offset`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Here are the options I think the committee might take:

1. Always select an unaligned allocator and force implementors to dynamically 
align.  This seems unlikely to me.
2. Allow an aligned allocator to be selected.  The issue here is that we cannot 
know until coroutine splitting whether the frame has a new-extended alignment.  
So there are some sub-options:

2a. Always use an aligned allocator if available, even if the frame ends up not 
being aligned.  I think this is unlikely.
2b. Use the correct allocator for the frame alignment, using the alignment 
inferred from the immediate coroutine body.  This would force implementations 
to avoid doing anything prior to coroutine splitting that would increase the 
frame's alignment beyond the derived limit.  This would be quite annoying for 
implementors, and it would have strange performance cliffs, but it's 
theoretically simple.
2c. Use the correct allocator for the frame alignment; only that allocator is 
formally ODR-used.  This would force implementations to not ODR-use the right 
allocator until coroutine lowering, which is not really reasonable; we should 
be able to dissuade the committee from picking this option.
2d. Use the correct allocator for the frame alignment; both allocators are 
(allowed to be) ODR-used, but only one would be dynamically used.  This is what 
would be necessary for the implementation I suggested above.  In reality there 
won't be any dynamic overhead because we should always be able to fold the 
branch after allocation.

I think 2d is obvious enough that we could go ahead and implement it pending 
standardization.  There's still a question of what to do if the promise class 
only provides an unaligned `operator new`, but the only reasonable behavior is 
to dynamically align: unlike with `new` expressions, there's no way the promise 
class could be expected to know/satisfy the appropriate alignment requirement 
in general, so assuming alignment is not acceptable.  (Neither is rejecting the 
program if it doesn't provide both — this wouldn't be compatible with existing 
behavior.)

> *(void**) (frame + size) = rawFrame; this means we always need the extra 
> space to store the raw frame ptr. If either doing what the patch is currently 
> doing or add another intrinsic say "llvm.coro.raw.frame.ptr.index" to do 
> *(void**) (frame + llvm.coro.raw.frame.ptr.index()) = rawFrame;, it is likely 
> that the extra pointer could reuse some existing paddings in the frame. There 
> is an example of this in https://reviews.llvm.org/P8260. What do you think?

You're right that there might be space in the frame we could re-use.  I was 
thinking that it would be a shame to always add storage to the frame for the 
raw frame pointer, but maybe the contract of `llvm.coro.raw.frame.ptr.offset` 
could be that it's only meaningful if the frame has extended alignment.  
Coroutine splitting would determine if any of the frame members was 
over-aligned and add a raw-pointer field if so. We'd be stuck allocating space 
in the frame even when allocation was elided, but stack space is cheap.

It does need to be an offset instead of a type index, though; the frontend will 
be emitting a GEP and will not know the frame type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101338: [MS] Fix crash when calling __cpuid with /EHsc and -mavx2

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

If I remember correctly cpuid.h implementation handles this. Can we copy that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101338

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101041: [analyzer] Find better description for tracked symbolic values

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Awesome, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101041

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

This looks like the fix is in the right place and it looks more or less how I 
expected it to look. Our casting procedure hopefully became more correct and 
now we have more correct analysis everywhere. It's still hard to tell whether 
this `ElementRegion` should be there or not, given how nobody still knows why 
they're there and when exactly should they be there; the very representation of 
casted pointers still needs a major rework. But we get there when we get there. 
Thanks a lot for debugging and digging and refactoring along the way.


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

https://reviews.llvm.org/D89055

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101338: [MS] Fix crash when calling __cpuid with /EHsc and -mavx2

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 340718.
pengfei added a comment.

Update for test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101338

Files:
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/ms-intrinsics-cpuid.c


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -12,7 +12,15 @@
 void test__cpuid(int *info, int level) {
   __cpuid(info, level);
 }
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK-LABEL: define {{.*}} @__cpuid(i32* %{{.*}}, i32 %{{.*}})
 // CHECK: call { i32, i32, i32, i32 } asm "cpuid",
 // CHECK-SAME:   
"={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
 // CHECK-SAME:   (i32 %{{.*}}, i32 0)
+
+// CHECK-LABEL: define {{.*}} @__cpuidex(i32* %{{.*}}, i32 %{{.*}}, i32 
%{{.*}})
+// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
+// CHECK-SAME:   
"={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
+// CHECK-SAME:   (i32 %{{.*}}, i32 %{{.*}})
+
+// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK: call void @__cpuid(i32* %{{.*}}, i32 %{{.*}})
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -507,12 +507,12 @@
 |* Misc
 
\**/
 #if defined(__i386__) || defined(__x86_64__)
-static __inline__ void __DEFAULT_FN_ATTRS __cpuid(int __info[4], int __level) {
+static void __attribute__((noinline, __nodebug__)) __cpuid(int __info[4], int 
__level) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
   : "a"(__level), "c"(0));
 }
-static __inline__ void __DEFAULT_FN_ATTRS __cpuidex(int __info[4], int __level,
+static void __attribute__((noinline, __nodebug__)) __cpuidex(int __info[4], 
int __level,
 int __ecx) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -12,7 +12,15 @@
 void test__cpuid(int *info, int level) {
   __cpuid(info, level);
 }
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK-LABEL: define {{.*}} @__cpuid(i32* %{{.*}}, i32 %{{.*}})
 // CHECK: call { i32, i32, i32, i32 } asm "cpuid",
 // CHECK-SAME:   "={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
 // CHECK-SAME:   (i32 %{{.*}}, i32 0)
+
+// CHECK-LABEL: define {{.*}} @__cpuidex(i32* %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
+// CHECK-SAME:   "={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
+// CHECK-SAME:   (i32 %{{.*}}, i32 %{{.*}})
+
+// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK: call void @__cpuid(i32* %{{.*}}, i32 %{{.*}})
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -507,12 +507,12 @@
 |* Misc
 \**/
 #if defined(__i386__) || defined(__x86_64__)
-static __inline__ void __DEFAULT_FN_ATTRS __cpuid(int __info[4], int __level) {
+static void __attribute__((noinline, __nodebug__)) __cpuid(int __info[4], int __level) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
   : "a"(__level), "c"(0));
 }
-static __inline__ void __DEFAULT_FN_ATTRS __cpuidex(int __info[4], int __level,
+static void __attribute__((noinline, __nodebug__)) __cpuidex(int __info[4], int __level,
 int __ecx) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100839: [analyzer] Adjust the reported variable name in retain count checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great, thanks!~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100839

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101327: [Clang][Driver] validate sysregs for -mstack-protector-guard-reg=

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:1425
+  };
+  return std::find(ValidRegs.begin(), ValidRegs.end(), RegName.upper()) !=
+ ValidRegs.end();

We should avoid static constructors/destructors if possible. Just use a plain 
array and `llvm::find(ValidRegs, RegName)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101327

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92815: [PowerPC] [Clang] Enable float128 feature on VSX targets

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92815

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101324: Hurd: Clean up Debian multiarch /usr/include/

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101324

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/test/Driver/hurd.cpp:21
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK: "-L

Since we now unsupport Windows, `{{/|}}` -> `/`

Consider replacing gcc 4.6.0 with the actual gcc version which may be higher, 
just to reflect the truth.
We may want to unsupport too old GCC versions if nobody uses.

But perhaps these cleanups can be done separately.



Comment at: clang/test/Driver/hurd.cpp:49
+// CHECK-STATIC-SAME: {{^}} "-internal-externc-isystem" 
"[[SYSROOT]]/usr/include"
+// CHECK-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-STATIC: "-static"

Drop `{{(.exe)?}}`

I think `"{{.*}}ld{{(.exe)?}}"` can be cleaned as well. Is it `i686-gnu-ld` if 
you add a fake executable `bin/i686-gnu-ld`?
If it is exactly `ld`, will be good to test the exact form. `linux-ld.c` is a 
big messy and probably not a good reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101338: [MS] Fix crash when calling __cpuid with /EHsc and -mavx2

[Pengfei Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: rnk, craig.topper, hans.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a workaround for PR50133.

When we do frame lowering for some complex scenarios, we may use RBX/EBX
register as frame pointer register, which may happen to be clobbered by
inline asm, e.g. __cpuid. Stopping to inline them is a workaround for
these cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101338

Files:
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/ms-intrinsics-cpuid.c


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -9,10 +9,12 @@
 
 #include 
 
-void test__cpuid(int *info, int level) {
-  __cpuid(info, level);
-}
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK-LABEL: define {{.*}} @__cpuid(i32* %{{.*}}, i32 %{{.*}})
 // CHECK: call { i32, i32, i32, i32 } asm "cpuid",
 // CHECK-SAME:   
"={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
 // CHECK-SAME:   (i32 %{{.*}}, i32 0)
+
+// CHECK-LABEL: define {{.*}} @__cpuidex(i32* %{{.*}}, i32 %{{.*}}, i32 
%{{.*}})
+// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
+// CHECK-SAME:   
"={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
+// CHECK-SAME:   (i32 %{{.*}}, i32 %{{.*}})
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -507,12 +507,12 @@
 |* Misc
 
\**/
 #if defined(__i386__) || defined(__x86_64__)
-static __inline__ void __DEFAULT_FN_ATTRS __cpuid(int __info[4], int __level) {
+static void __attribute__((noinline, __nodebug__)) __cpuid(int __info[4], int 
__level) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
   : "a"(__level), "c"(0));
 }
-static __inline__ void __DEFAULT_FN_ATTRS __cpuidex(int __info[4], int __level,
+static void __attribute__((noinline, __nodebug__)) __cpuidex(int __info[4], 
int __level,
 int __ecx) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])


Index: clang/test/CodeGen/ms-intrinsics-cpuid.c
===
--- clang/test/CodeGen/ms-intrinsics-cpuid.c
+++ clang/test/CodeGen/ms-intrinsics-cpuid.c
@@ -9,10 +9,12 @@
 
 #include 
 
-void test__cpuid(int *info, int level) {
-  __cpuid(info, level);
-}
-// CHECK-LABEL: define {{.*}} @test__cpuid(i32* %{{.*}}, i32 %{{.*}})
+// CHECK-LABEL: define {{.*}} @__cpuid(i32* %{{.*}}, i32 %{{.*}})
 // CHECK: call { i32, i32, i32, i32 } asm "cpuid",
 // CHECK-SAME:   "={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
 // CHECK-SAME:   (i32 %{{.*}}, i32 0)
+
+// CHECK-LABEL: define {{.*}} @__cpuidex(i32* %{{.*}}, i32 %{{.*}}, i32 %{{.*}})
+// CHECK: call { i32, i32, i32, i32 } asm "cpuid",
+// CHECK-SAME:   "={ax},={bx},={cx},={dx},{ax},{cx},~{dirflag},~{fpsr},~{flags}"
+// CHECK-SAME:   (i32 %{{.*}}, i32 %{{.*}})
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -507,12 +507,12 @@
 |* Misc
 \**/
 #if defined(__i386__) || defined(__x86_64__)
-static __inline__ void __DEFAULT_FN_ATTRS __cpuid(int __info[4], int __level) {
+static void __attribute__((noinline, __nodebug__)) __cpuid(int __info[4], int __level) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
   : "a"(__level), "c"(0));
 }
-static __inline__ void __DEFAULT_FN_ATTRS __cpuidex(int __info[4], int __level,
+static void __attribute__((noinline, __nodebug__)) __cpuidex(int __info[4], int __level,
 int __ecx) {
   __asm__("cpuid"
   : "=a"(__info[0]), "=b"(__info[1]), "=c"(__info[2]), "=d"(__info[3])
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D100739#2718514 , @ychen wrote:

> Oh, right now C++ coroutine standard is written in the way that the aligned 
> new is not searched by the frontend. The limitation will be lifted in C++23 
> (hopefully).

I see. I am curious about the relationship of compiler implementation and 
language standard now. For example, Clang/LLVM implement coroutine before it 
becomes standard. The point I curious about is that should Clang/LLVM implement 
based on proposals accepted only?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9360430 - Revert "Reapply "[AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed""

[Pushpinder Singh via cfe-commits]

Author: Pushpinder Singh
Date: 2021-04-27T02:23:44Z
New Revision: 93604305bb72201641f31cc50a6e7b2fe65d3af3

URL: 
https://github.com/llvm/llvm-project/commit/93604305bb72201641f31cc50a6e7b2fe65d3af3
DIFF: 
https://github.com/llvm/llvm-project/commit/93604305bb72201641f31cc50a6e7b2fe65d3af3.diff

LOG: Revert "Reapply  "[AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs 
installed""

This reverts commit 15be0c41d2e59fb4599c9aebf21ede498c61f51d.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/tools/CMakeLists.txt

Removed: 
clang/test/Driver/Inputs/amdgpu-arch/amdgpu_arch_different
clang/test/Driver/Inputs/amdgpu-arch/amdgpu_arch_fail
clang/test/Driver/Inputs/amdgpu-arch/amdgpu_arch_gfx906
clang/test/Driver/Inputs/amdgpu-arch/amdgpu_arch_gfx908_gfx908
clang/test/Driver/amdgpu-openmp-system-arch-fail.c
clang/test/Driver/amdgpu-openmp-system-arch.c
clang/tools/amdgpu-arch/AMDGPUArch.cpp
clang/tools/amdgpu-arch/CMakeLists.txt



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index a2ffe1378cb6d..5e580cc4fbb7a 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -67,8 +67,6 @@ def err_drv_no_hip_runtime : Error<
   "cannot find HIP runtime. Provide its path via --rocm-path, or pass "
   "-nogpuinc to build without HIP runtime.">;
 
-def err_drv_undetermined_amdgpu_arch : Error<
-  "Cannot determine AMDGPU architecture: %0. Consider passing it via 
--march.">;
 def err_drv_cuda_version_unsupported : Error<
   "GPU arch %0 is supported by CUDA versions between %1 and %2 (inclusive), "
   "but installation at %3 is %4. Use --cuda-path to specify a 
diff erent CUDA "

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index df3049fe40326..04a05207cc74b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -924,8 +924,6 @@ def rocm_path_EQ : Joined<["--"], "rocm-path=">, 
Group,
   HelpText<"ROCm installation path, used for finding and automatically linking 
required bitcode libraries.">;
 def hip_path_EQ : Joined<["--"], "hip-path=">, Group,
   HelpText<"HIP runtime installation path, used for finding HIP version and 
adding HIP include path.">;
-def amdgpu_arch_tool_EQ : Joined<["--"], "amdgpu-arch-tool=">, Group,
-  HelpText<"Tool used for detecting AMD GPU arch in the system.">;
 def rocm_device_lib_path_EQ : Joined<["--"], "rocm-device-lib-path=">, 
Group,
   HelpText<"ROCm device library path. Alternative to rocm-path.">;
 def : Joined<["--"], "hip-device-lib-path=">, Alias;

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 4da1239dce84e..c0b2b78a1b4b2 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -12,16 +12,9 @@
 #include "clang/Basic/TargetID.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/DriverDiagnostic.h"
-#include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/FileUtilities.h"
-#include "llvm/Support/LineIterator.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/VirtualFileSystem.h"
-#include 
-
-#define AMDGPU_ARCH_PROGRAM_NAME "amdgpu-arch"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -722,78 +715,6 @@ void AMDGPUToolChain::checkTargetID(
   }
 }
 
-llvm::Error
-AMDGPUToolChain::detectSystemGPUs(const ArgList ,
-  SmallVector ) const 
{
-  std::string Program;
-  if (Arg *A = Args.getLastArg(options::OPT_amdgpu_arch_tool_EQ))
-Program = A->getValue();
-  else
-Program = GetProgramPath(AMDGPU_ARCH_PROGRAM_NAME);
-  llvm::SmallString<64> OutputFile;
-  llvm::sys::fs::createTemporaryFile("print-system-gpus", "" /* No Suffix */,
- OutputFile);
-  llvm::FileRemover OutputRemover(OutputFile.c_str());
-  llvm::Optional Redirects[] = {
-  {""},
-  StringRef(OutputFile),
-  {""},
-  };
-
-  std::string ErrorMessage;
-  if (int Result = llvm::sys::ExecuteAndWait(
-  Program.c_str(), {}, {}, Redirects, /* SecondsToWait */ 0,
-  /*MemoryLimit*/ 0, )) {
-if (Result > 0) {
-  ErrorMessage = "Exited with error code " + std::to_string(Result);
-} else if (Result == -1) {
-  ErrorMessage = "Execute failed: " + ErrorMessage;
-} else {
-  ErrorMessage = "Crashed: " + ErrorMessage;
-}
-
-return llvm::createStringError(std::error_code(),
-   Program + ": " + ErrorMessage);
-  

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D100739#2717582 , @rjmccall wrote:

> That's not really what I meant, no. I meant it would be better to find a way 
> to use an allocator that promises to return a well-aligned value when 
> possible. We've talked about this before; that will require the C++ committee 
> to update the design.

I had a question. Does this mean we can't use aligned allocator until the C++ 
committee update the wording? For example, I know Clang/LLVM implement 
coroutine before it becomes the standard. I mean is it possible to use 
aligned-allocator to solve the problem here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D100739#2715964 , @ChuanqiXu wrote:

> In D100739#2713579 , @ychen wrote:
>
>> In D100739#2711698 , @ChuanqiXu 
>> wrote:
>>
 This is an alternative to D97915  which 
 missed proper deallocation of the over-allocated frame. This patch handles 
 both allocations and deallocations.
>>>
>>> If D97915  is not needed, we should 
>>> abandon it.
>>>
>>> For the example shows in D97915 , it says:
>>>
>>>   #include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>   #include 
>>>   
>>>   struct task{
>>> struct alignas(64) promise_type {
>>>   task get_return_object() { return {}; }
>>>   std::experimental::suspend_never initial_suspend() { return {}; }
>>>   std::experimental::suspend_never final_suspend() noexcept { return 
>>> {}; }
>>>   void return_void() {}
>>>   void unhandled_exception() {}
>>> };
>>> using handle = std::experimental::coroutine_handle;
>>>   };
>>>   
>>>   auto switch_to_new_thread() {
>>> struct awaitable {
>>>   bool await_ready() { return false; }
>>>   void await_suspend(task::handle h) {
>>> auto i = reinterpret_cast(());
>>> std::cout << i << std::endl;
>>> assert(i % 64 == 0);
>>>   }
>>>   void await_resume() {}
>>> };
>>> return awaitable{};
>>>   }
>>>   
>>>   task resuming_on_new_thread() {
>>> co_await switch_to_new_thread();
>>>   }
>>>   
>>>   int main() {
>>> resuming_on_new_thread();
>>>   }
>>>
>>> The assertion would fail. If this is the root problem, I think we could 
>>> adjust the align for the promise alloca like:
>>
>> The problem is that any member of the coroutine frame could be overaligned 
>> (thus make the frame overaligned) including promise, local variables, 
>> spills. The problem is *not* specific to promise.
>>
>>>   %promise = alloca %promise_type, align 8
>>>
>>> into
>>>
>>>   %promise = alloca %promise_type, align 128
>>>
>>> In other words, if this the problem we need to solve, I think we could make 
>>> it in a simpler way.
>>
>> This may not fix the problem.
>>
>>> Then I looked into the document you give in the summary. The issue#3 says 
>>> the frontend can't do some work in the process of template instantiation 
>>> due to the frontend doesn't know about the align and size of the coroutine. 
>>> But from the implementation, it looks like not the problem this patch wants 
>>> to solve.
>>
>> I meant to use that as a reference to help describe the problem (but not the 
>> solution). The document itself includes both problem statements (issue#3) 
>> and solutions (frontend-based) which are totally unrelated to this patch. It 
>> looks like it is not that useful in this case so please disregard that.
>>
>>> I am really confused about the problem. Could you please restate your 
>>> problem more in more detail? For example, would it make the alignment 
>>> incorrect like the example above? Or does we want the frontend to get 
>>> alignment information? Then what would be affected? From the title, I can 
>>> guess the size of frame would get bigger. But how big would it be? Who 
>>> would control and determine the final size?
>>
>> understood.
>>
>> There are two kinds of alignments: the alignment of a type/object at 
>> compile-time (ABI specified or user-specified), and the alignment the object 
>> of that type actually gets during runtime. The compiler assumes that the 
>> alignment of a struct is the maximal alignment of all its members. However, 
>> that assumption may not be true at runtime where the memory allocator may 
>> return a memory block that has insufficient alignment which causes some 
>> members aligned incorrectly.
>>
>> For C++ coroutine, right now the default memory allocator could only return 
>> 16 bytes aligned memory block. When any member of the coroutine frame 
>> (promise, local variables, spills etc.) has alignment > 16, the frame 
>> becomes overaligned. This could only be fixed dynamically at runtime: by 
>> over-allocating memory and then adjust the frame start address so that it 
>> aligns correctly.
>>
>> For example, suppose malloc returns 16 bytes aligned address 16, how do we 
>> make it 64 bytes aligned? align 16 up to an address that is 64 bytes aligned 
>> which is 64, so the adjustment amount is 64-16=48
>>
>> Another similar example, suppose malloc returns 16 bytes aligned address 32, 
>> how do we make it 64 bytes aligned? align 32 up to an address that is 64 
>> bytes aligned which is 64, so the adjustment amount is 64-32=32
>>
>> Another similar example, suppose malloc returns 16 bytes aligned address 48, 
>> how do we make it 64 bytes aligned? align 48 up to an address that is 64 
>> bytes aligned which is 64, so the adjustment amount is 64-48=16
>>

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka requested changes to this revision.
vitalybuka added a comment.
This revision now requires changes to proceed.

Is this abandoned patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96203

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: akhuang.
rnk added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

erichkeane wrote:
> rnk wrote:
> > hans wrote:
> > > I worry that this is going to look obscure to most readers passing 
> > > through. Maybe it could be expanded to more explicitly spell out that it 
> > > reduces the size of the debug info?
> > I want to keep it concise, most readers shouldn't need to know what this 
> > is, and they can look up technical terms like "key method". I'll say "debug 
> > info" instead of "type info", though, that should be more obvious.
> FWIW, I just ran into this and did a double/triple take, as it didn't make 
> sense for me to see a 'virtual' function in a 'final' type that didn't 
> inherit to anything looked like nonsense.
> 
> The only way I found out what this meant (googling "key method" did very 
> little for me here) was to do a 'git-blame' then found this review.  The ONLY 
> place that explained what is happening here is the comment you made here: 
> https://reviews.llvm.org/D70340#1752192
Sorry, I went ahead and wrote better comments in 
rG6d78c38986fa0974ea0b37e66f8cb89b256f4e0d.

Re: key functions, this is where the idea is documented:
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable
They control where the vtable is emitted. We have this style rule to take 
advantage of them:
https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
However, the existing rule has to do with RTTI and vtables, which doesn't make 
any sense for Sema.

The idea that class debug info is tied to the vtable "known", but not well 
documented. It is mentioned maybe once in the user manual:
https://clang.llvm.org/docs/UsersManual.html#cmdoption-fstandalone-debug
I couldn't find any GCC documentation about this behavior, so we're doing 
better. :)

@akhuang has been working on the constructor homing feature announced here:
https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/
So maybe in the near future we won't need this hack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92078: [asan] Default to -asan-use-private-alias=1

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka requested changes to this revision.
vitalybuka added a comment.
This revision now requires changes to proceed.

I assume we are not moving forward with this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92078

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6d78c38 - Move Sema's key function around and add more comments

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2021-04-26T18:32:50-07:00
New Revision: 6d78c38986fa0974ea0b37e66f8cb89b256f4e0d

URL: 
https://github.com/llvm/llvm-project/commit/6d78c38986fa0974ea0b37e66f8cb89b256f4e0d
DIFF: 
https://github.com/llvm/llvm-project/commit/6d78c38986fa0974ea0b37e66f8cb89b256f4e0d.diff

LOG: Move Sema's key function around and add more comments

The previous comment was pretty obscure.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bc15bf73bea80..e0ededea6bcae 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -353,9 +353,6 @@ class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
-  /// A key method to reduce duplicate debug info from Sema.
-  virtual void anchor();
-
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
@@ -1527,6 +1524,13 @@ class Sema final {
   /// initialized but before it parses anything.
   void Initialize();
 
+  /// This virtual key function only exists to limit the emission of debug info
+  /// describing the Sema class. GCC and Clang only emit debug info for a class
+  /// with a vtable when the vtable is emitted. Sema is final and not
+  /// polymorphic, but the debug info size savings are so significant that it 
is
+  /// worth adding a vtable just to take advantage of this optimization.
+  virtual void anchor();
+
   const LangOptions () const { return LangOpts; }
   OpenCLOptions () { return OpenCLFeatures; }
   FPOptions () { return CurFPFeatures; }



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101248: [RISCV] [1/2] Add IR intrinsic for Zbm extension

[LevyHsu via Phabricator via cfe-commits]

LevyHsu updated this revision to Diff 340705.
LevyHsu added a comment.

1. clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbm.c
  - All test cases renamed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101248

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbm.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoB.td
  llvm/test/CodeGen/RISCV/rv64zbm-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbm-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbm-intrinsic.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-b -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IB
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zbm -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV64IBM
+
+declare i64 @llvm.riscv.bmator.i64(i64 %a, i64 %b)
+
+define i64 @bmator64(i64 %a, i64 %b) nounwind {
+; RV64IB-LABEL: bmator64:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:bmator a0, a0, a1
+; RV64IB-NEXT:ret
+;
+; RV64IBM-LABEL: bmator64:
+; RV64IBM:   # %bb.0:
+; RV64IBM-NEXT:bmator a0, a0, a1
+; RV64IBM-NEXT:ret
+  %tmp = call i64 @llvm.riscv.bmator.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.bmatxor.i64(i64 %a, i64 %b)
+
+define i64 @bmatxor64(i64 %a, i64 %b) nounwind {
+; RV64IB-LABEL: bmatxor64:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:bmatxor a0, a0, a1
+; RV64IB-NEXT:ret
+;
+; RV64IBM-LABEL: bmatxor64:
+; RV64IBM:   # %bb.0:
+; RV64IBM-NEXT:bmatxor a0, a0, a1
+; RV64IBM-NEXT:ret
+  %tmp = call i64 @llvm.riscv.bmatxor.i64(i64 %a, i64 %b)
+ ret i64 %tmp
+}
+
+declare i64 @llvm.riscv.bmatflip.i64(i64 %a)
+
+define i64 @bmatflip64(i64 %a) nounwind {
+; RV64IB-LABEL: bmatflip64:
+; RV64IB:   # %bb.0:
+; RV64IB-NEXT:bmatflip a0, a0
+; RV64IB-NEXT:ret
+;
+; RV64IBM-LABEL: bmatflip64:
+; RV64IBM:   # %bb.0:
+; RV64IBM-NEXT:bmatflip a0, a0
+; RV64IBM-NEXT:ret
+  %tmp = call i64 @llvm.riscv.bmatflip.i64(i64 %a)
+ ret i64 %tmp
+}
Index: llvm/lib/Target/RISCV/RISCVInstrInfoB.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfoB.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfoB.td
@@ -934,6 +934,12 @@
 def : PatGprGpr;
 } // Predicates = [HasStdExtZbc]
 
+let Predicates = [HasStdExtZbm, IsRV64] in {
+def : PatGprGpr;
+def : PatGprGpr;
+def : PatGpr;
+} // Predicates = [HasStdExtZbm, IsRV64]
+
 let Predicates = [HasStdExtZbr] in {
 def : PatGpr;
 def : PatGpr;
Index: llvm/include/llvm/IR/IntrinsicsRISCV.td
===
--- llvm/include/llvm/IR/IntrinsicsRISCV.td
+++ llvm/include/llvm/IR/IntrinsicsRISCV.td
@@ -89,6 +89,11 @@
   def int_riscv_clmulh : BitManipGPRGPRIntrinsics;
   def int_riscv_clmulr : BitManipGPRGPRIntrinsics;
 
+  // Zbm
+  def int_riscv_bmator   : BitManipGPRGPRIntrinsics;
+  def int_riscv_bmatxor  : BitManipGPRGPRIntrinsics;
+  def int_riscv_bmatflip : BitManipGPRIntrinsics;
+
   // Zbp
   def int_riscv_grev  : BitManipGPRGPRIntrinsics;
   def int_riscv_gorc  : BitManipGPRGPRIntrinsics;
Index: clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbm.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbm.c
@@ -0,0 +1,45 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +experimental-zbm -emit-llvm %s -o - \
+// RUN: | FileCheck %s  -check-prefix=RV64ZBM
+
+// RV64ZBM-LABEL: @bmator(
+// RV64ZBM-NEXT:  entry:
+// RV64ZBM-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
+// RV64ZBM-NEXT:[[B_ADDR:%.*]] = alloca i64, align 8
+// RV64ZBM-NEXT:store i64 [[A:%.*]], i64* [[A_ADDR]], align 8
+// RV64ZBM-NEXT:store i64 [[B:%.*]], i64* [[B_ADDR]], align 8
+// RV64ZBM-NEXT:[[TMP0:%.*]] = load i64, i64* [[A_ADDR]], align 8
+// RV64ZBM-NEXT:[[TMP1:%.*]] = load i64, i64* [[B_ADDR]], align 8
+// RV64ZBM-NEXT:[[TMP2:%.*]] = call i64 @llvm.riscv.bmator.i64(i64 [[TMP0]], i64 [[TMP1]])
+// RV64ZBM-NEXT:ret i64 [[TMP2]]
+//
+long bmator(long a, long b) {
+  return __builtin_riscv_bmator(a, b);
+}
+
+// RV64ZBM-LABEL: @bmatxor(
+// RV64ZBM-NEXT:  entry:
+// RV64ZBM-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
+// RV64ZBM-NEXT:[[B_ADDR:%.*]] = alloca i64, align 8
+// RV64ZBM-NEXT:store i64 [[A:%.*]], i64* [[A_ADDR]], align 8
+// RV64ZBM-NEXT:store i64 [[B:%.*]], i64* [[B_ADDR]], align 8
+// RV64ZBM-NEXT:[[TMP0:%.*]] = load i64, i64* [[A_ADDR]], align 8
+// RV64ZBM-NEXT:[[TMP1:%.*]] = load i64, i64* [[B_ADDR]], align 8
+// 

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul added inline comments.



Comment at: clang/test/Driver/hurd.cpp:4
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \

MaskRay wrote:
> The target should exactly match the hurd triple. Other aliases are supported 
> but should be considered discouraged.
> 
> Driver testing is difficult. Good to test `-DCLANG_DEFAULT_RTLIB=compiler-rt 
> -DCLANG_DEFAULT_CXX_STDLIB=libc++` as well whether you need to fix some 
> options.
> 
> If `hurd.c` has now duplicated testing for some properties, consider dropping 
> it. We should try making tests orthogonal.
> The target should exactly match the hurd triple. Other aliases are supported 
> but should be considered discouraged.

Well, i386-pc-gnu is the GCC triplet for hurd on x86. i386-pc-hurd-gnu is 
llvm-only. Alright anyway.

> If hurd.c has now duplicated testing for some properties, consider dropping 
> it.

Ok I have moved everything from hurd.c to hurd.cpp.



Comment at: clang/test/Driver/hurd.cpp:20
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"

MaskRay wrote:
> You probably missed that I used `"-L` as the first line value (also note that 
> the thing after `CHECK:` is aligned). The ensures the test can catch the case 
> when a "-L" is added to the start of the list.
I noticed that afterwards indeed, fixed so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul updated this revision to Diff 340703.
sthibaul marked 6 inline comments as done.
sthibaul added a comment.

Move all tests to hurd.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/c++/4.6.0/.keep
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/i386-gnu/c++/4.6.0/.keep
  clang/test/Driver/hurd.c
  clang/test/Driver/hurd.cpp

Index: clang/test/Driver/hurd.cpp
===
--- /dev/null
+++ clang/test/Driver/hurd.cpp
@@ -0,0 +1,98 @@
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-hurd-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK: "-cc1"
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0/backward"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK: "-internal-externc-isystem"
+// CHECK-SAME: {{^}} "[[SYSROOT]]/usr/include/i386-gnu"
+// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/include"
+// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK: "-L
+// CHECK-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib/i386-gnu"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib/../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/i386-gnu"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-hurd-gnu -static \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-STATIC %s
+// CHECK-STATIC: "-cc1"
+// CHECK-STATIC: "-static-define"
+// CHECK-STATIC: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-STATIC-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK-STATIC-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"
+// CHECK-STATIC-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0/backward"
+// CHECK-STATIC-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK-STATIC: "-internal-externc-isystem"
+// CHECK-STATIC-SAME: {{^}} "[[SYSROOT]]/usr/include/i386-gnu"
+// CHECK-STATIC-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/include"
+// CHECK-STATIC-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-STATIC: "-static"
+// CHECK-STATIC: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbeginT.o"
+// CHECK-STATIC: "-L
+// CHECK-STATIC-SAME: {{^}}[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/lib/i386-gnu"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/lib/../lib32"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/i386-gnu"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/../lib32"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// CHECK-STATIC-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
+
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-hurd-gnu -shared \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SHARED %s
+// CHECK-SHARED: "-cc1"
+// CHECK-SHARED: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-SHARED-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK-SHARED-SAME: {{^}} 

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/hurd.cpp:4
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \

The target should exactly match the hurd triple. Other aliases are supported 
but should be considered discouraged.

Driver testing is difficult. Good to test `-DCLANG_DEFAULT_RTLIB=compiler-rt 
-DCLANG_DEFAULT_CXX_STDLIB=libc++` as well whether you need to fix some options.

If `hurd.c` has now duplicated testing for some properties, consider dropping 
it. We should try making tests orthogonal.



Comment at: clang/test/Driver/hurd.cpp:20
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"

You probably missed that I used `"-L` as the first line value (also note that 
the thing after `CHECK:` is aligned). The ensures the test can catch the case 
when a "-L" is added to the start of the list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul marked 4 inline comments as done.
sthibaul added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3014
   llvm::opt::ArgStringList ) const 
{
-  addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args);
+  if (!GCCInstallation.isValid())
+return;

MaskRay wrote:
> I'd be glad if we could use `GCCInstallation.isValid()`.
> 
> However, @thakis has a chromium usage where `lib/gcc/$triple/crtbegin.o` is 
> not provided while compile-only  libstdc++ search paths are expected.
> 
> For now, use `GCCInstallation.isValid() ? GCCInstallation.getTriple().str() : 
> ""`
Ok, fixed so



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3018
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+
+  addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args, TripleStr);

MaskRay wrote:
> No need for a blank line
Ok, fixed so



Comment at: clang/test/Driver/hurd.cpp:9
+/// though the installation is i686-gnu.
+// CHECK-SAME: "-internal-isystem" 
"[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+// CHECK-SAME: "-internal-isystem" 
"[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"

MaskRay wrote:
> Windows may use backslashes in some places.
> It is really difficult to tell where need backslashes, so I just add 
> `UNSUPPORTED: system-windows` to linux-cross.cpp
Ok, added so.



Comment at: clang/test/Driver/hurd.cpp:21
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-SAME: "-L[[SYSROOT]]/lib/i386-gnu"

MaskRay wrote:
> `{{^}} ` is important, otherwise a new addition cannot be detected.
Ok, added so.



Comment at: clang/test/Driver/hurd.cpp:5
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning:
+// CHECK: "-cc1"

MaskRay wrote:
> sthibaul wrote:
> > MaskRay wrote:
> > > The warning negative pattern doesn't really work.
> > You mean because it was misplaced?
> > Indeed, fixed so.
> `-###` cannot detect unused warning options: `clang -Wfoobar '-###' a.cc -c` 
> =>  no warning
> 
> It is probably not the test's task to check it.
Ok, dropped it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul updated this revision to Diff 340700.
sthibaul marked 3 inline comments as done.
sthibaul added a comment.

Fix GCCInstallation.isValid use in Generic_GCC::addLibStdCxxIncludePaths
Fix test further


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/c++/4.6.0/.keep
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/i386-gnu/c++/4.6.0/.keep
  clang/test/Driver/hurd.cpp

Index: clang/test/Driver/hurd.cpp
===
--- /dev/null
+++ clang/test/Driver/hurd.cpp
@@ -0,0 +1,28 @@
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK: "-cc1"
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0/backward"
+// CHECK-SAME: {{^}} "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/usr/include/i386-gnu"
+// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/include"
+// CHECK-SAME: {{^}} "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib/i386-gnu"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib/../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/i386-gnu"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib/../lib32"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/lib"
+// CHECK-SAME: {{^}} "-L[[SYSROOT]]/usr/lib"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -595,17 +595,24 @@
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const {
-  // Try generic GCC detection first.
-  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args))
-return;
-
   // We need a detected GCC installation on Linux to provide libstdc++'s
   // headers in odd Linuxish places.
   if (!GCCInstallation.isValid())
 return;
 
-  StringRef LibDir = GCCInstallation.getParentLibPath();
+  // Detect Debian g++-multiarch-incdir.diff.
   StringRef TripleStr = GCCInstallation.getTriple().str();
+  StringRef DebianMultiarch =
+  GCCInstallation.getTriple().getArch() == llvm::Triple::x86
+  ? "i386-linux-gnu"
+  : TripleStr;
+
+  // Try generic GCC detection first.
+  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args,
+   DebianMultiarch))
+return;
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
   const Multilib  = GCCInstallation.getMultilib();
   const GCCVersion  = GCCInstallation.getVersion();
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -26,6 +26,9 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+   llvm::opt::ArgStringList ) const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList ) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -186,6 +186,21 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
+void Hurd::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const {
+  // We need a detected GCC installation on Linux to provide libstdc++'s
+  // headers in odd Linuxish places.
+  if 

[PATCH] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a subscriber: delcypher.
vitalybuka added a comment.

In D101122#2711962 , @joerg wrote:

> There are two approaches that work for reviewing: starting from clang and 
> going down or starting from compiler-rt and going up the layers. I'd prefer 
> to do the latter as it means meaningful testing can be done easier. There are 
> two natural steps here: clang flag+IR generation is one, llvm codegen and 
> compiler-rt is the other. The clang step should include tests that ensure 
> that proper IR is generated for the different flag combination (including 
> documentation for the IR changes). The LLVM step should ensure that the 
> different attributes (either function or module scope) are correctly lowered 
> in the instrumentation pass and unit tests on the compiler-rt side to do 
> functional testing. The unit testing might also be done as a third step if it 
> goes the full way from C to binary code.

Today on chat I advised to start with clang, but now I agree with Joerg. More 
precisely you can go with following 4 patches:

1. LLVM Detailed IR tests of effect of this flag on transformation
2. compiler-rt (tests will have to uses -mllvm -)
3. clang
  - driver tests
  - basic IR tests to see that flag makes a difference of generated code. see 
llvm-project/clang/test/CodeGen/asan-*.cpp
4. compiler-rt:  replace -mllvm copt from patch2  with with clang flag from 
patch3.






Comment at: clang/include/clang/Basic/CodeGenOptions.def:225
  ///< destructors are emitted.
+ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode,
+llvm::AsanDetectStackUseAfterReturnMode, 2,

Mode->Kind to be consistent with the file



Comment at: clang/include/clang/Basic/CodeGenOptions.def:225-228
+ENUM_CODEGENOPT(SanitizeAddressDetectStackUseAfterReturnMode,
+llvm::AsanDetectStackUseAfterReturnMode, 2,
+llvm::AsanDetectStackUseAfterReturnMode::Runtime
+) ///< Set detection mode for stack-use-after-return.

vitalybuka wrote:
> Mode->Kind to be consistent with the file
I believe for consistency we need:
1. Var name (SanitizeAddressDetectStackUseAfterReturn) no Mode/Kind suffix
2. Enum type AsanDetectStackUseAfterReturnKind uses "kind"
3. Actual flag includes no "kind" 
-fsanitize-address-detect-stack-use-after-return=

BTW. @delcypher  -fsanitize-address-destructor-kind was not consistent with the 
rest of file, but I am not sure it's worth of fixing now.





Comment at: clang/include/clang/Driver/Options.td:1555
+  NormalizedValuesScope<"llvm::AsanDetectStackUseAfterReturnMode">,
+  NormalizedValues<["Always", "Runtime", "Never"]>,
+  
MarshallingInfoEnum,
 "Runtime">;

same order as enum



Comment at: 
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h:29
+   ///< (ASAN_OPTIONS=detect_stack_use_after_return=1)
+  Never,   ///< Never detect stack use after return.
+  Invalid, ///< Not a valid detect mode.

Could you order them in acceding order by "strictness", this will make Never == 
0, and the current default == 1.
Never = 0,  (maybe None for consistency?)
Runtime=1,
Always=2,
Invalid=


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101122

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: thakis.
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3014
   llvm::opt::ArgStringList ) const 
{
-  addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args);
+  if (!GCCInstallation.isValid())
+return;

I'd be glad if we could use `GCCInstallation.isValid()`.

However, @thakis has a chromium usage where `lib/gcc/$triple/crtbegin.o` is not 
provided while compile-only  libstdc++ search paths are expected.

For now, use `GCCInstallation.isValid() ? GCCInstallation.getTriple().str() : 
""`



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3018
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+
+  addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args, TripleStr);

No need for a blank line



Comment at: clang/test/Driver/hurd.cpp:9
+/// though the installation is i686-gnu.
+// CHECK-SAME: "-internal-isystem" 
"[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+// CHECK-SAME: "-internal-isystem" 
"[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"

Windows may use backslashes in some places.
It is really difficult to tell where need backslashes, so I just add 
`UNSUPPORTED: system-windows` to linux-cross.cpp



Comment at: clang/test/Driver/hurd.cpp:21
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-SAME: "-L[[SYSROOT]]/lib/i386-gnu"

`{{^}} ` is important, otherwise a new addition cannot be detected.



Comment at: clang/test/Driver/hurd.cpp:5
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning:
+// CHECK: "-cc1"

sthibaul wrote:
> MaskRay wrote:
> > The warning negative pattern doesn't really work.
> You mean because it was misplaced?
> Indeed, fixed so.
`-###` cannot detect unused warning options: `clang -Wfoobar '-###' a.cc -c` => 
 no warning

It is probably not the test's task to check it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2509f9f - [clang] Don't crash when loading invalid VFS for the module dep collector

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2021-04-26T17:05:22-07:00
New Revision: 2509f9fbad0d37e3e5fea934c0ae7af3877ba4ae

URL: 
https://github.com/llvm/llvm-project/commit/2509f9fbad0d37e3e5fea934c0ae7af3877ba4ae
DIFF: 
https://github.com/llvm/llvm-project/commit/2509f9fbad0d37e3e5fea934c0ae7af3877ba4ae.diff

LOG: [clang] Don't crash when loading invalid VFS for the module dep collector

The VFS is null when it's invalid so return early in collectVFSFromYAML.

Added: 
clang/test/VFS/broken-vfs-module-dep.c

Modified: 
llvm/lib/Support/VirtualFileSystem.cpp

Removed: 




diff  --git a/clang/test/VFS/broken-vfs-module-dep.c 
b/clang/test/VFS/broken-vfs-module-dep.c
new file mode 100644
index 0..2336306de8c6d
--- /dev/null
+++ b/clang/test/VFS/broken-vfs-module-dep.c
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: not %clang_cc1 -module-dependency-dir %t -ivfsoverlay 
%S/Inputs/invalid-yaml.yaml %s 2>&1 | FileCheck %s
+
+// CHECK: error: Unexpected token
+// CHECK: error: Unexpected token
+// CHECK: 1 error generated

diff  --git a/llvm/lib/Support/VirtualFileSystem.cpp 
b/llvm/lib/Support/VirtualFileSystem.cpp
index deb0f37bd159d..dd820c15ca68e 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -2127,6 +2127,8 @@ void 
vfs::collectVFSFromYAML(std::unique_ptr Buffer,
   std::unique_ptr VFS = RedirectingFileSystem::create(
   std::move(Buffer), DiagHandler, YAMLFilePath, DiagContext,
   std::move(ExternalFS));
+  if (!VFS)
+return;
   ErrorOr RootResult =
   VFS->lookupPath("/");
   if (!RootResult)



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul marked 2 inline comments as done.
sthibaul added inline comments.



Comment at: clang/test/Driver/hurd.cpp:1
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \

MaskRay wrote:
> I think we need -NEXT patterns to prevent unintentional new paths.
> 
> IMO `linux-cross.cpp` uses a good style and can serve as a reference.
Ok, fixed so.



Comment at: clang/test/Driver/hurd.cpp:5
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning:
+// CHECK: "-cc1"

MaskRay wrote:
> The warning negative pattern doesn't really work.
You mean because it was misplaced?
Indeed, fixed so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul updated this revision to Diff 340689.
sthibaul added a comment.

Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/c++/4.6.0/.keep
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/i386-gnu/c++/4.6.0/.keep
  clang/test/Driver/hurd.cpp

Index: clang/test/Driver/hurd.cpp
===
--- /dev/null
+++ clang/test/Driver/hurd.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK: "-cc1"
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+// CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"
+// CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0/backward"
+// CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/usr/include/i386-gnu"
+// CHECK-SAME: "-internal-externc-isystem" "[[SYSROOT]]/include"
+// CHECK-SAME: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK-NOT: warning:
+// CHECK: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../lib32"
+// CHECK-SAME: "-L[[SYSROOT]]/lib/i386-gnu"
+// CHECK-SAME: "-L[[SYSROOT]]/lib/../lib32"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/i386-gnu"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib/../lib32"
+// CHECK-SAME: "-L[[SYSROOT]]/lib"
+// CHECK-SAME: "-L[[SYSROOT]]/usr/lib"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -595,17 +595,24 @@
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const {
-  // Try generic GCC detection first.
-  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args))
-return;
-
   // We need a detected GCC installation on Linux to provide libstdc++'s
   // headers in odd Linuxish places.
   if (!GCCInstallation.isValid())
 return;
 
-  StringRef LibDir = GCCInstallation.getParentLibPath();
+  // Detect Debian g++-multiarch-incdir.diff.
   StringRef TripleStr = GCCInstallation.getTriple().str();
+  StringRef DebianMultiarch =
+  GCCInstallation.getTriple().getArch() == llvm::Triple::x86
+  ? "i386-linux-gnu"
+  : TripleStr;
+
+  // Try generic GCC detection first.
+  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args,
+   DebianMultiarch))
+return;
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
   const Multilib  = GCCInstallation.getMultilib();
   const GCCVersion  = GCCInstallation.getVersion();
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -26,6 +26,9 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+   llvm::opt::ArgStringList ) const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList ) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -186,6 +186,21 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
+void Hurd::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const {
+  // We need a detected GCC installation on Linux to provide libstdc++'s
+  // headers in odd Linuxish places.
+  if (!GCCInstallation.isValid())
+return;
+
+  StringRef TripleStr = GCCInstallation.getTriple().str();
+  StringRef DebianMultiarch =
+  GCCInstallation.getTriple().getArch() == llvm::Triple::x86 ? "i386-gnu"
+ 

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2a3ca8d9796: BPF: emit debuginfo for Function of 
DeclRefExpr if requested (authored by yonghong-song).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/debug-info-extern-callback.c


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu 
-emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12688,7 +12688,7 @@
 Diag(Var->getLocation(), diag::note_private_extern);
   }
 
-  if (Context.getTargetInfo().allowDebugInfoForExternalVar() &&
+  if (Context.getTargetInfo().allowDebugInfoForExternalRef() &&
   !Var->isInvalidDecl() && !getLangOpts().CPlusPlus)
 ExternalDeclarations.push_back(Var);
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2832,8 +2832,21 @@
 return LV;
   }
 
-  if (const auto *FD = dyn_cast(ND))
-return EmitFunctionDeclLValue(*this, E, FD);
+  if (const auto *FD = dyn_cast(ND)) {
+LValue LV = EmitFunctionDeclLValue(*this, E, FD);
+
+// Emit debuginfo for the function declaration if the target wants to.
+if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
+  if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
+auto *Fn =
+cast(LV.getPointer(*this)->stripPointerCasts());
+if (!Fn->getSubprogram())
+  DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
+  }
+}
+
+return LV;
+  }
 
   // FIXME: While we're emitting a binding from an enclosing scope, all other
   // DeclRefExprs we see should be implicitly treated as if they also refer to
Index: clang/lib/Basic/Targets/BPF.h
===
--- clang/lib/Basic/Targets/BPF.h
+++ clang/lib/Basic/Targets/BPF.h
@@ -76,7 +76,7 @@
 return None;
   }
 
-  bool allowDebugInfoForExternalVar() const override { return true; }
+  bool allowDebugInfoForExternalRef() const override { return true; }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 switch (CC) {
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1538,8 +1538,8 @@
 
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
-  /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  /// Whether target allows debuginfo types for decl only variables/functions.
+  virtual bool allowDebugInfoForExternalRef() const { return false; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12688,7 +12688,7 @@
 

[clang] a2a3ca8 - BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2021-04-26T16:53:25-07:00
New Revision: a2a3ca8d97962d90443ee758e47877e15d7e3832

URL: 
https://github.com/llvm/llvm-project/commit/a2a3ca8d97962d90443ee758e47877e15d7e3832
DIFF: 
https://github.com/llvm/llvm-project/commit/a2a3ca8d97962d90443ee758e47877e15d7e3832.diff

LOG: BPF: emit debuginfo for Function of DeclRefExpr if requested

Commit e3d8ee35e4ad ("reland "[DebugInfo] Support to emit debugInfo
for extern variables"") added support to emit debugInfo for
extern variables if requested by the target. Currently, only
BPF target enables this feature by default.

As BPF ecosystem grows, callback function started to get
support, e.g., recently bpf_for_each_map_elem() is introduced
(https://lwn.net/Articles/846504/) with a callback function as an
argument. In the future we may have something like below as
a demonstration of use case :
extern int do_work(int);
long bpf_helper(void *callback_fn, void *callback_ctx, ...);
long prog_main() {
struct { ... } ctx = { ... };
return bpf_helper(_work, , ...);
}
Basically bpf helper may have a callback function and the
callback function is defined in another file or in the kernel.
In this case, we would like to know the debuginfo types for
do_work(), so the verifier can proper verify the safety of
bpf_helper() call.

For the following example,
extern int do_work(int);
long bpf_helper(void *callback_fn);
long prog() {
return bpf_helper(_work);
}

Currently, there is no debuginfo generated for extern function do_work().
In the IR, we have,
...
define dso_local i64 @prog() local_unnamed_addr #0 !dbg !7 {
entry:
  %call = tail call i64 @bpf_helper(i8* bitcast (i32 (i32)* @do_work to 
i8*)) #2, !dbg !11
  ret i64 %call, !dbg !12
}
...
declare dso_local i32 @do_work(i32) #1
...

This patch added support for the above callback function use case, and
the generated IR looks like below:
...
declare !dbg !17 dso_local i32 @do_work(i32) #1
...
!17 = !DISubprogram(name: "do_work", scope: !1, file: !1, line: 1, type: 
!18, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
!18 = !DISubroutineType(types: !19)
!19 = !{!20, !20}
!20 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

The TargetInfo.allowDebugInfoForExternalVar is renamed to
TargetInfo.allowDebugInfoForExternalRef as now it guards
both extern variable and extern function debuginfo generation.

Differential Revision: https://reviews.llvm.org/D100567

Added: 
clang/test/CodeGen/debug-info-extern-callback.c

Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/Basic/Targets/BPF.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 449c026639b90..1655f932dad23 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1538,8 +1538,8 @@ class TargetInfo : public virtual TransferrableTargetInfo,
 
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
-  /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  /// Whether target allows debuginfo types for decl only variables/functions.
+  virtual bool allowDebugInfoForExternalRef() const { return false; }
 
 protected:
   /// Copy type and layout related info.

diff  --git a/clang/lib/Basic/Targets/BPF.h b/clang/lib/Basic/Targets/BPF.h
index 43e55dfbfb2b2..06b451db189af 100644
--- a/clang/lib/Basic/Targets/BPF.h
+++ b/clang/lib/Basic/Targets/BPF.h
@@ -76,7 +76,7 @@ class LLVM_LIBRARY_VISIBILITY BPFTargetInfo : public 
TargetInfo {
 return None;
   }
 
-  bool allowDebugInfoForExternalVar() const override { return true; }
+  bool allowDebugInfoForExternalRef() const override { return true; }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 switch (CC) {

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index b98865971f8bf..2a272bf29e04e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2832,8 +2832,21 @@ LValue CodeGenFunction::EmitDeclRefLValue(const 
DeclRefExpr *E) {
 return LV;
   }
 
-  if (const auto *FD = dyn_cast(ND))
-return EmitFunctionDeclLValue(*this, E, FD);
+  if (const auto *FD = dyn_cast(ND)) {
+LValue LV = EmitFunctionDeclLValue(*this, E, FD);
+
+// Emit debuginfo for the function declaration if the target wants to.
+if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
+  if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
+auto *Fn =
+cast(LV.getPointer(*this)->stripPointerCasts());
+if (!Fn->getSubprogram())
+  DI->EmitFunctionDecl(FD, FD->getLocation(), 

[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39
+static int go(a) int a;
+{
+  return glob + a;
+}
+
+

dblaikie wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > dblaikie wrote:
> > > > > > hoy wrote:
> > > > > > > dblaikie wrote:
> > > > > > > > hoy wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > hoy wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > > Does this need to be down here? Or would the code 
> > > > > > > > > > > > > > be a well exercised if it was up next to the go 
> > > > > > > > > > > > > > declaration above?
> > > > > > > > > > > > > Yes, it needs to be here. Otherwise it will just like 
> > > > > > > > > > > > > the function `bar` above that doesn't get a 
> > > > > > > > > > > > > uniquefied name. I think moving the definition up to 
> > > > > > > > > > > > > right after the declaration hides the declaration.
> > > > > > > > > > > > Not sure I follow - do you mean that if the go 
> > > > > > > > > > > > declaration and go definition were next to each other, 
> > > > > > > > > > > > this test would (mechanically speaking) not validate 
> > > > > > > > > > > > what the patch? Or that it would be less legible, but 
> > > > > > > > > > > > still mechanically correct?
> > > > > > > > > > > > 
> > > > > > > > > > > > I think it would be (assuming it's still mechanically 
> > > > > > > > > > > > correct) more legible to put the declaration next to 
> > > > > > > > > > > > the definition - the comment describes why the 
> > > > > > > > > > > > declaration is significant/why the definition is weird, 
> > > > > > > > > > > > and seeing all that together would be clearer to me 
> > > > > > > > > > > > than spreading it out/having to look further away to 
> > > > > > > > > > > > see what's going on.
> > > > > > > > > > > When the `go` declaration and `go` definition were next 
> > > > > > > > > > > to each other, the go function won't get a uniqufied name 
> > > > > > > > > > > at all. The declaration will be overwritten by the 
> > > > > > > > > > > definition. Only when the declaration is seen by others, 
> > > > > > > > > > > such the callsite in `baz`, the declaration makes a 
> > > > > > > > > > > difference by having the callsite use a uniqufied name.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > Ah! Interesting, good to know. 
> > > > > > > > > > 
> > > > > > > > > > Is that worth supporting, I wonder? I guess it falls out 
> > > > > > > > > > for free/without significant additional complexity. I worry 
> > > > > > > > > > about the subtlety of the additional declaration changing 
> > > > > > > > > > the behavior here... might be a bit surprising/subtle. But 
> > > > > > > > > > maybe no nice way to avoid it either.
> > > > > > > > > It would be ideal if user never writes code like that. 
> > > > > > > > > Unfortunately it exists with legacy code (such as mysql). I 
> > > > > > > > > think it's worth supporting it from AutoFDO point of view to 
> > > > > > > > > avoid a silent mismatch between debug linkage name and real 
> > > > > > > > > linkage name.
> > > > > > > > Oh, I agree that we shouldn't mismatch debug info and the 
> > > > > > > > actual symbol name - what I meant was whether code like this 
> > > > > > > > should get mangled or not when using unique-internal-linkage 
> > > > > > > > names.
> > > > > > > > 
> > > > > > > > I'm now more curious about this:
> > > > > > > > 
> > > > > > > > > When the `go` declaration and `go` definition were next to 
> > > > > > > > > each other, the go function won't get a uniqufied name at all.
> > > > > > > > 
> > > > > > > > This doesn't seem to happen with the 
> > > > > > > > `__attribute__((overloadable))` attribute, for instance - so 
> > > > > > > > any idea what's different about uniquification that's working 
> > > > > > > > differently than overloadable?
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > $ cat test.c
> > > > > > > > __attribute__((overloadable)) static int go(a) int a; {
> > > > > > > >   return 3 + a;
> > > > > > > > }
> > > > > > > > void baz() {
> > > > > > > >   go(2);
> > > > > > > > }
> > > > > > > > $ clang-tot test.c -emit-llvm -S -o - | grep go
> > > > > > > >   %call = call i32 @_ZL2goi(i32 2)
> > > > > > > > define internal i32 @_ZL2goi(i32 %a) #0 {
> > > > > > > > ```
> > > > > > > Good question. I'm not sure what's exactly going on but it looks 
> > > > > > > like with the overloadable attribute, the old-style definition is 
> > > > > > > treated as having prototype. But if you do this:
> > > > > > > 
> > > > > > > ```
> > > > > > > __attribute__((overloadable)) 
> > > > > > > void baz() {}
> > > > > > > ```
> > > > > > > then there's the error:
> > > > > > > 
> > > > > > > ```
> > > > > > > error: 'overloadable' function 'baz' must have a prototype

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/hurd.cpp:1
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \

I think we need -NEXT patterns to prevent unintentional new paths.

IMO `linux-cross.cpp` uses a good style and can serve as a reference.



Comment at: clang/test/Driver/hurd.cpp:5
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning:
+// CHECK: "-cc1"

The warning negative pattern doesn't really work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101331

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D100739#2717582 , @rjmccall wrote:

> In D100739#2717259 , @ychen wrote:
>
>> In D100739#2717227 , @rjmccall 
>> wrote:
>>
>>> Yes, if you can dynamically choose to use an aligned allocator, that's 
>>> clearly just much better.
>>
>> Right now:
>>
>> Intrinsic::coro_size_aligned :   overaligned frame: over-allocate, adjust 
>> start address;non-overaligned frame: no-op
>> Intrinsic::coro_size :overaligned frame: no-op;  
>>non-overaligned frame: no-op
>>
>> Do you mean to remove `Intrinsic::coro_size_aligned` and make 
>> Intrinsic::coro_size :overaligned frame: over-allocate, 
>> adjust start address;non-overaligned frame: no-op
>>
>> that makes sense to me. Just want to confirm first.
>
> That's not really what I meant, no.  I meant it would be better to find a way 
> to use an allocator that promises to return a well-aligned value when 
> possible.  We've talked about this before; that will require the C++ 
> committee to update the design.
>
> I think the cleanest design for coro_size/align would be that they reflect 
> the unadjusted requirements, and the frontend is expected to emit code which 
> satisfies those requirements.  In the absence of an aligned allocator, that 
> means generating code like:
>
>   if (llvm.coro.alloc()) {
> size_t size = llvm.coro.size(), align = llvm.coro.align();
> if (align > NEW_ALIGN)
>   size += align - NEW_ALIGN + sizeof(void*);
> frame = operator new(size);
> if (align > NEW_ALIGN) {
>   auto rawFrame = frame;
>   frame = (frame + align - 1) & ~(align - 1);
>   *(void**) (frame + size) = rawFrame;
> }
>   }
>
> and then on the deallocation side:
>
>   if (llvm.coro.alloc()) {
> size_t size = llvm.coro.size(), align = llvm.coro.align();
> if (align > NEW_ALIGN)
>   frame = *(void**) (frame + size);
> operator delete(frame);
>   }
>
> That's all quite annoying, but it does extend quite nicely to cover the 
> presence of an aligned allocator when the committee gets around to ratifying 
> that this is what should happen:
>
>   if (llvm.coro.alloc()) {
> size_t size = llvm.coro.size(), align = llvm.coro.align();
> if (align > NEW_ALIGN)
>   frame = operator new(std::align_val_t(align), size);
> else
>   frame = operator new(size);
>   }
>
> and then on the deallocation side:
>
>   if (llvm.coro.alloc()) {
> size_t size = llvm.coro.size(), align = llvm.coro.align();
> if (align > NEW_ALIGN)
>   operator delete(frame, std::align_val_t(align));
> else
>   operator delete(frame);
>   }

`*(void**) (frame + size) = rawFrame;` this means we always need the extra 
space to store the raw frame ptr. If either doing what the patch is currently 
doing or add another intrinsic say "llvm.coro.raw.frame.ptr.index" to do 
`*(void**) (frame + llvm.coro.raw.frame.ptr.index()) = rawFrame;`, it is likely 
that the extra pointer could reuse some existing paddings in the frame.  There 
is an example of this in https://reviews.llvm.org/P8260. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101331: hurd: Detect libstdc++ include paths on Debian Hurd i386

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul created this revision.
sthibaul requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a follow-up of e92d2b80c6c9 
 
("[Driver] Detect libstdc++ include
paths for native gcc (-m32 and -m64) on Debian i386") for the Debian Hurd
case, which has the same multiarch name reduction from i686 to i386.
i386-linux-gnu is actually Linux-only, so this moves the code of that commit
to Linux.cpp, and adds the same to Hurd.cpp


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101331

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Hurd.h
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/c++/4.6.0/.keep
  clang/test/Driver/Inputs/basic_hurd_tree/usr/include/i386-gnu/c++/4.6.0/.keep
  clang/test/Driver/hurd.cpp

Index: clang/test/Driver/hurd.cpp
===
--- /dev/null
+++ clang/test/Driver/hurd.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: --target=i386-pc-gnu \
+// RUN: --sysroot=%S/Inputs/basic_hurd_tree \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning:
+// CHECK: "-cc1"
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+/// Debian specific - the path component after 'include' is i386-gnu even
+/// though the installation is i686-gnu.
+// CHECK: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0"
+// CHECK: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/i386-gnu/c++/4.6.0"
+// CHECK: "-internal-isystem" "[[SYSROOT]]/usr/lib/gcc/i686-gnu/4.6.0/../../../../include/c++/4.6.0/backward"
+// CHECK: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/usr/include/i386-gnu"
+// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/include"
+// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK: "-dynamic-linker" "/lib/ld.so"
+// CHECK: "{{.*}}/usr/lib/gcc/i686-gnu/4.6.0{{/|}}crtbegin.o"
+// CHECK: "-L[[SYSROOT]]/lib/i386-gnu"
+// CHECK: "-L[[SYSROOT]]/lib/../lib32"
+// CHECK: "-L[[SYSROOT]]/usr/lib/i386-gnu"
+// CHECK: "-L[[SYSROOT]]/usr/lib/../lib32"
+// CHECK: "-L[[SYSROOT]]/lib"
+// CHECK: "-L[[SYSROOT]]/usr/lib"
Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -595,17 +595,24 @@
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const {
-  // Try generic GCC detection first.
-  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args))
-return;
-
   // We need a detected GCC installation on Linux to provide libstdc++'s
   // headers in odd Linuxish places.
   if (!GCCInstallation.isValid())
 return;
 
-  StringRef LibDir = GCCInstallation.getParentLibPath();
+  // Detect Debian g++-multiarch-incdir.diff.
   StringRef TripleStr = GCCInstallation.getTriple().str();
+  StringRef DebianMultiarch =
+  GCCInstallation.getTriple().getArch() == llvm::Triple::x86
+  ? "i386-linux-gnu"
+  : TripleStr;
+
+  // Try generic GCC detection first.
+  if (Generic_GCC::addGCCLibStdCxxIncludePaths(DriverArgs, CC1Args,
+   DebianMultiarch))
+return;
+
+  StringRef LibDir = GCCInstallation.getParentLibPath();
   const Multilib  = GCCInstallation.getMultilib();
   const GCCVersion  = GCCInstallation.getVersion();
 
Index: clang/lib/Driver/ToolChains/Hurd.h
===
--- clang/lib/Driver/ToolChains/Hurd.h
+++ clang/lib/Driver/ToolChains/Hurd.h
@@ -26,6 +26,9 @@
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const override;
+  void
+  addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+   llvm::opt::ArgStringList ) const override;
 
   std::string getDynamicLinker(const llvm::opt::ArgList ) const override;
 
Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -186,6 +186,21 @@
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
 }
 
+void Hurd::addLibStdCxxIncludePaths(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const {
+  // We need a detected GCC installation on Linux to provide libstdc++'s
+  // headers in odd Linuxish places.
+  if 

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_interceptors.cpp:28
+#if SANITIZER_POSIX
 #include "sanitizer_common/sanitizer_platform_limits_posix.h"
+#endif

we usually include them unconditionaly and .h itself has guards against 
unsupported platforms


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2844
+  auto *Fn =
+  dyn_cast(LV.getPointer(*this)->stripPointerCasts());
+  if (DI && !Fn->getSubprogram())

dblaikie wrote:
> Oh, please change this to cast, rather than dyn_cast before committing. 
> (since the Fn is unconditionally dereferenced on the next line (well, 
> conditional on DI, but that's not relevant to this)
> 
> Could also move the "if (DI)" further out, like this:
> ```
> if (CGDebugInfo *DI = ...) {
>   auto *Fn = cast...
>   if (!Fn->getSubprogram())
> DI->EmitFunctionDecl(...);
> }
> ```
done as suggested!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 340675.
yonghong-song added a comment.

formatting to mode "DI = CGM.getModuleDebugInfo()" and use cast<...> instead of 
dyn_cast<...> since there should be no possibility of null ptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/debug-info-extern-callback.c


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu 
-emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12667,7 +12667,7 @@
 Diag(Var->getLocation(), diag::note_private_extern);
   }
 
-  if (Context.getTargetInfo().allowDebugInfoForExternalVar() &&
+  if (Context.getTargetInfo().allowDebugInfoForExternalRef() &&
   !Var->isInvalidDecl() && !getLangOpts().CPlusPlus)
 ExternalDeclarations.push_back(Var);
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2834,8 +2834,21 @@
 return LV;
   }
 
-  if (const auto *FD = dyn_cast(ND))
-return EmitFunctionDeclLValue(*this, E, FD);
+  if (const auto *FD = dyn_cast(ND)) {
+LValue LV = EmitFunctionDeclLValue(*this, E, FD);
+
+// Emit debuginfo for the function declaration if the target wants to.
+if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
+  if (CGDebugInfo *DI = CGM.getModuleDebugInfo()) {
+auto *Fn =
+cast(LV.getPointer(*this)->stripPointerCasts());
+if (!Fn->getSubprogram())
+  DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
+  }
+}
+
+return LV;
+  }
 
   // FIXME: While we're emitting a binding from an enclosing scope, all other
   // DeclRefExprs we see should be implicitly treated as if they also refer to
Index: clang/lib/Basic/Targets/BPF.h
===
--- clang/lib/Basic/Targets/BPF.h
+++ clang/lib/Basic/Targets/BPF.h
@@ -76,7 +76,7 @@
 return None;
   }
 
-  bool allowDebugInfoForExternalVar() const override { return true; }
+  bool allowDebugInfoForExternalRef() const override { return true; }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 switch (CC) {
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1538,8 +1538,8 @@
 
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
-  /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  /// Whether target allows debuginfo types for decl only variables/functions.
+  virtual bool allowDebugInfoForExternalRef() const { return false; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ 

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: llvm/cmake/config-ix.cmake:285
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)

jsji wrote:
> Maybe add a warning here about we are forcing it to 0?
As discussed off-list: For a platform where Clang and LLVM is relatively new, I 
don't think this leads to a loss of functionality at the level where a warning 
is warranted. The autodetection is going to set this to 0 on some platforms 
anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rGbdc4ec04d42a: [AIX] Avoid use of mtim.tv_nsec member of stat 
structure on AIX (authored by hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

Files:
  llvm/cmake/config-ix.cmake


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101139: Create install targets for scan-build-py.

[Yu Shan via Phabricator via cfe-commits]

aabbaabb added a comment.

In D101139#2718057 , @phosek wrote:

> In D101139#2713530 , @aabbaabb 
> wrote:
>
>> The python script assumes relative directory while finding things. For 
>> example, for resources folder, it uses os.path.join(this_dir, 'resources') 
>> in report.py, which means resource need to be in the same dir as report.py. 
>> Similarly for the libscanbuild. it assumes the library is always at one 
>> level up from bin folder. Installing them to different directories would 
>> break the script.
>
> We could reorganize things to match the final layout, that's the strategy 
> that https://github.com/llvm/llvm-project/tree/main/clang/tools/scan-build 
> uses as well.
>
> In D101139#2713551 , @aabbaabb 
> wrote:
>
>> libear is built dynamically at runtime from build_libear function in 
>> libear/__init__.py which would be called by libscanbuild/analyze.py. It is 
>> not built statically.
>
> Could we modify the code to avoid building libear at runtime and instead 
> build it with CMake. Is libear even needed when using compilation database?

If i copy libscanbuild/resources to out/share/, libscanbuild to 
out/libscanbuild, bin/* to bin, then resources would be in a different layout 
than the original src. You mean modifying the original source and move 
libscanbuild/resources out to share/resources and update the code reference?

We are not using libear, we are only using analyze-build not scan-build. For 
our usage, I could define a target that only copies analyze-build, not 
scan-build. However, I am not sure whether this is appropriate. On the other 
hand, building libear statically might require significant change to the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101139

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9b0501abc7b5: [Clang] -Wunused-but-set-parameter and 
-Wunused-but-set-variable (authored by mbenfield, committed by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGen/X86/x86_32-xsave.c
  clang/test/CodeGen/X86/x86_64-xsave.c
  clang/test/CodeGen/builtins-arm.c
  clang/test/CodeGen/builtins-riscv.c
  clang/test/FixIt/fixit.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/shift.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables.c
  clang/test/SemaCXX/goto.cpp
  clang/test/SemaCXX/shift.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
  clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
  clang/test/SemaObjC/foreach.m

Index: clang/test/SemaObjC/foreach.m
===
--- clang/test/SemaObjC/foreach.m
+++ clang/test/SemaObjC/foreach.m
@@ -1,4 +1,4 @@
-/* RUN: %clang_cc1 -Wall -fsyntax-only -verify -std=c89 -pedantic %s
+/* RUN: %clang_cc1 -Wall -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -fsyntax-only -verify -std=c89 -pedantic %s
  */
 
 @class NSArray;
Index: clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  // In C++, don't warn for structs. (following gcc's behavior)
+  struct S s;
+  struct S t;
+  s = t;
+
+  int x;
+  x = 0;
+  return x + 5;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
+
+void f2() {
+  // Don't warn for either of these cases.
+  constexpr int x = 2;
+  const int y = 1;
+  char a[x];
+  char b[y];
+}
Index: clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+// In C++, don't warn for a struct (following gcc).
+void f3(struct S s) {
+  struct S t;
+  s = t;
+}
+
+// Make sure this doesn't warn.
+struct A {
+  int i;
+  A(int j) : i(j) {}
+};
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wrange-loop-analysis -triple 

[clang] 9b0501a - [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Arthur Eubanks via cfe-commits]

Author: Michael Benfield
Date: 2021-04-26T15:09:03-07:00
New Revision: 9b0501abc7b515b740fb5ee929817442dd3029a5

URL: 
https://github.com/llvm/llvm-project/commit/9b0501abc7b515b740fb5ee929817442dd3029a5
DIFF: 
https://github.com/llvm/llvm-project/commit/9b0501abc7b515b740fb5ee929817442dd3029a5.diff

LOG: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

These are intended to mimic warnings available in gcc.

-Wunused-but-set-variable is triggered in the case of a variable which
appears on the LHS of an assignment but not otherwise used.

For instance:

  void f() {
int x;
x = 0;
  }

-Wunused-but-set-parameter works similarly, but for function parameters
instead of variables.

In C++, they are triggered only for scalar types; otherwise, they are
triggered for all types. This is gcc's behavior.

-Wunused-but-set-parameter is controlled by -Wextra, while
-Wunused-but-set-variable is controlled by -Wunused. This is slightly
different from gcc's behavior, but seems most consistent with clang's
behavior for -Wunused-parameter and -Wunused-variable.

Reviewed By: aeubanks

Differential Revision: https://reviews.llvm.org/D100581

Added: 
clang/test/Sema/warn-unused-but-set-parameters.c
clang/test/Sema/warn-unused-but-set-variables.c
clang/test/SemaCXX/warn-unused-but-set-parameters-cpp.cpp
clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CodeGen/X86/x86_32-xsave.c
clang/test/CodeGen/X86/x86_64-xsave.c
clang/test/CodeGen/builtins-arm.c
clang/test/CodeGen/builtins-riscv.c
clang/test/FixIt/fixit.cpp
clang/test/Misc/warning-wall.c
clang/test/Sema/shift.c
clang/test/Sema/vector-gcc-compat.c
clang/test/SemaCXX/goto.cpp
clang/test/SemaCXX/shift.cpp
clang/test/SemaCXX/sizeless-1.cpp
clang/test/SemaObjC/foreach.m

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index df2a46cecc5d0..b91cee9a9e5f8 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -724,6 +724,8 @@ def UnusedMemberFunction : 
DiagGroup<"unused-member-function",
 def UnusedLabel : DiagGroup<"unused-label">;
 def UnusedLambdaCapture : DiagGroup<"unused-lambda-capture">;
 def UnusedParameter : DiagGroup<"unused-parameter">;
+def UnusedButSetParameter : DiagGroup<"unused-but-set-parameter">;
+def UnusedButSetVariable : DiagGroup<"unused-but-set-variable">;
 def UnusedResult : DiagGroup<"unused-result">;
 def PotentiallyEvaluatedExpression : 
DiagGroup<"potentially-evaluated-expression">;
 def UnevaluatedExpression : DiagGroup<"unevaluated-expression",
@@ -865,7 +867,8 @@ def Conversion : DiagGroup<"conversion",
  DiagCategory<"Value Conversion Issue">;
 
 def Unused : DiagGroup<"unused",
-   [UnusedArgument, UnusedFunction, UnusedLabel,
+   [UnusedArgument, UnusedButSetVariable,
+UnusedFunction, UnusedLabel,
 // UnusedParameter, (matches GCC's behavior)
 // UnusedTemplate, (clean-up libc++ before enabling)
 // UnusedMemberFunction, (clean-up llvm before 
enabling)
@@ -922,6 +925,7 @@ def Extra : DiagGroup<"extra", [
 SemiBeforeMethodBody,
 MissingMethodReturnType,
 SignCompare,
+UnusedButSetParameter,
 UnusedParameter,
 NullPointerArithmetic,
 EmptyInitStatement,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 639565feb5766..552a9d349e396 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -310,8 +310,12 @@ def note_riscv_repeated_interrupt_attribute : Note<
   "repeated RISC-V 'interrupt' attribute is here">;
 def warn_unused_parameter : Warning<"unused parameter %0">,
   InGroup, DefaultIgnore;
+def warn_unused_but_set_parameter : Warning<"parameter %0 set but not used">,
+  InGroup, DefaultIgnore;
 def warn_unused_variable : Warning<"unused variable %0">,
   InGroup, DefaultIgnore;
+def warn_unused_but_set_variable : Warning<"variable %0 set but not used">,
+  InGroup, DefaultIgnore;
 def warn_unused_local_typedef : Warning<
   "unused %select{typedef|type alias}0 %1">,
   InGroup, DefaultIgnore;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cc008e9869d24..bc15bf73bea80 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2828,6 +2828,13 @@ class Sema final {
   /// ParmVarDecl pointers.
   void 

[PATCH] D101139: Create install targets for scan-build-py.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D101139#2713530 , @aabbaabb wrote:

> The python script assumes relative directory while finding things. For 
> example, for resources folder, it uses os.path.join(this_dir, 'resources') in 
> report.py, which means resource need to be in the same dir as report.py. 
> Similarly for the libscanbuild. it assumes the library is always at one level 
> up from bin folder. Installing them to different directories would break the 
> script.

We could reorganize things to match the final layout, that's the strategy that 
https://github.com/llvm/llvm-project/tree/main/clang/tools/scan-build uses as 
well.

In D101139#2713551 , @aabbaabb wrote:

> libear is built dynamically at runtime from build_libear function in 
> libear/__init__.py which would be called by libscanbuild/analyze.py. It is 
> not built statically.

Could we modify the code to avoid building libear at runtime and instead build 
it with CMake. Is libear even needed when using compilation database?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101139

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101328: [clangd] run clang-format on FindTargetTests.cpp

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 340666.
dgoldman added a comment.

Remove temporary comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101328

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1036,37 +1036,36 @@
 
 TEST_F(FindExplicitReferencesTest, All) {
   std::pair Cases[] =
-  {
-  // Simple expressions.
-  {R"cpp(
+  {// Simple expressions.
+   {R"cpp(
 int global;
 int func();
 void foo(int param) {
   $0^global = $1^param + $2^func();
 }
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"
-   "2: targets = {func}\n"},
-  {R"cpp(
+"0: targets = {global}\n"
+"1: targets = {param}\n"
+"2: targets = {func}\n"},
+   {R"cpp(
 struct X { int a; };
 void foo(X x) {
   $0^x.$1^a = 10;
 }
 )cpp",
-   "0: targets = {x}\n"
-   "1: targets = {X::a}\n"},
-  {R"cpp(
+"0: targets = {x}\n"
+"1: targets = {X::a}\n"},
+   {R"cpp(
 // error-ok: testing with broken code
 int bar();
 int foo() {
   return $0^bar() + $1^bar(42);
 }
 )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
-  // Namespaces and aliases.
-  {R"cpp(
+"0: targets = {bar}\n"
+"1: targets = {bar}\n"},
+   // Namespaces and aliases.
+   {R"cpp(
   namespace ns {}
   namespace alias = ns;
   void foo() {
@@ -1074,19 +1073,19 @@
 using namespace $1^alias;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {alias}\n"},
-  // Using declarations.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {alias}\n"},
+   // Using declarations.
+   {R"cpp(
   namespace ns { int global; }
   void foo() {
 using $0^ns::$1^global;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
-  // Simple types.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   // Simple types.
+   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
@@ -1095,13 +1094,13 @@
static_cast<$4^Struct*>(0);
  }
)cpp",
-   "0: targets = {Struct}\n"
-   "1: targets = {x}, decl\n"
-   "2: targets = {Typedef}\n"
-   "3: targets = {y}, decl\n"
-   "4: targets = {Struct}\n"},
-  // Name qualifiers.
-  {R"cpp(
+"0: targets = {Struct}\n"
+"1: targets = {x}, decl\n"
+"2: targets = {Typedef}\n"
+"3: targets = {y}, decl\n"
+"4: targets = {Struct}\n"},
+   // Name qualifiers.
+   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  void foo() {
$0^a::$1^b::$2^S $3^x;
@@ -1109,16 +1108,16 @@
$6^S::$7^type $8^y;
  }
 )cpp",
-   "0: targets = {a}\n"
-   "1: targets = {a::b}, qualifier = 'a::'\n"
-   "2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-   "3: targets = {x}, decl\n"
-   "4: targets = {a}\n"
-   "5: targets = {a::b}, qualifier = 'a::'\n"
-   "6: targets = {a::b::S}\n"
-   "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
-   "8: targets = {y}, decl\n"},
-  {R"cpp(
+"0: targets = {a}\n"
+"1: targets = {a::b}, qualifier = 'a::'\n"
+"2: targets = {a::b::S}, qualifier = 'a::b::'\n"
+"3: targets = {x}, decl\n"
+"4: targets = {a}\n"
+"5: targets = {a::b}, qualifier = 'a::'\n"
+"6: targets = {a::b::S}\n"
+"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
+"8: targets = {y}, decl\n"},
+   {R"cpp(
  void foo() {
$0^ten: // PRINT "HELLO WORLD!"
goto $1^ten;
@@ -1135,12 +1134,12 @@
 $2^vector $3^vb;
   }
 )cpp",
-   "0: targets = {vector}\n"
-   "1: targets = {vi}, decl\n"
-   "2: targets = {vector}\n"
-   "3: targets = {vb}, decl\n"},
-  // Template type aliases.
-  {R"cpp(
+"0: targets = {vector}\n"
+"1: targets = {vi}, decl\n"
+"2: targets = {vector}\n"
+"3: targets = {vb}, decl\n"},
+   // Template type aliases.
+   {R"cpp(
 template  

[PATCH] D101328: [clangd] run clang-format on FindTargetTests.cpp

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 340665.
dgoldman added a comment.

Actually fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101328

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1036,37 +1036,36 @@
 
 TEST_F(FindExplicitReferencesTest, All) {
   std::pair Cases[] =
-  {
-  // Simple expressions.
-  {R"cpp(
+  {// Simple expressions.
+   {R"cpp(
 int global;
 int func();
 void foo(int param) {
   $0^global = $1^param + $2^func();
 }
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"
-   "2: targets = {func}\n"},
-  {R"cpp(
+"0: targets = {global}\n"
+"1: targets = {param}\n"
+"2: targets = {func}\n"},
+   {R"cpp(
 struct X { int a; };
 void foo(X x) {
   $0^x.$1^a = 10;
 }
 )cpp",
-   "0: targets = {x}\n"
-   "1: targets = {X::a}\n"},
-  {R"cpp(
+"0: targets = {x}\n"
+"1: targets = {X::a}\n"},
+   {R"cpp(
 // error-ok: testing with broken code
 int bar();
 int foo() {
   return $0^bar() + $1^bar(42);
 }
 )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
-  // Namespaces and aliases.
-  {R"cpp(
+"0: targets = {bar}\n"
+"1: targets = {bar}\n"},
+   // Namespaces and aliases.
+   {R"cpp(
   namespace ns {}
   namespace alias = ns;
   void foo() {
@@ -1074,19 +1073,19 @@
 using namespace $1^alias;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {alias}\n"},
-  // Using declarations.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {alias}\n"},
+   // Using declarations.
+   {R"cpp(
   namespace ns { int global; }
   void foo() {
 using $0^ns::$1^global;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
-  // Simple types.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   // Simple types.
+   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
@@ -1095,13 +1094,13 @@
static_cast<$4^Struct*>(0);
  }
)cpp",
-   "0: targets = {Struct}\n"
-   "1: targets = {x}, decl\n"
-   "2: targets = {Typedef}\n"
-   "3: targets = {y}, decl\n"
-   "4: targets = {Struct}\n"},
-  // Name qualifiers.
-  {R"cpp(
+"0: targets = {Struct}\n"
+"1: targets = {x}, decl\n"
+"2: targets = {Typedef}\n"
+"3: targets = {y}, decl\n"
+"4: targets = {Struct}\n"},
+   // Name qualifiers.
+   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  void foo() {
$0^a::$1^b::$2^S $3^x;
@@ -1109,16 +1108,16 @@
$6^S::$7^type $8^y;
  }
 )cpp",
-   "0: targets = {a}\n"
-   "1: targets = {a::b}, qualifier = 'a::'\n"
-   "2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-   "3: targets = {x}, decl\n"
-   "4: targets = {a}\n"
-   "5: targets = {a::b}, qualifier = 'a::'\n"
-   "6: targets = {a::b::S}\n"
-   "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
-   "8: targets = {y}, decl\n"},
-  {R"cpp(
+"0: targets = {a}\n"
+"1: targets = {a::b}, qualifier = 'a::'\n"
+"2: targets = {a::b::S}, qualifier = 'a::b::'\n"
+"3: targets = {x}, decl\n"
+"4: targets = {a}\n"
+"5: targets = {a::b}, qualifier = 'a::'\n"
+"6: targets = {a::b::S}\n"
+"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
+"8: targets = {y}, decl\n"},
+   {R"cpp(
  void foo() {
$0^ten: // PRINT "HELLO WORLD!"
goto $1^ten;
@@ -1135,12 +1134,12 @@
 $2^vector $3^vb;
   }
 )cpp",
-   "0: targets = {vector}\n"
-   "1: targets = {vi}, decl\n"
-   "2: targets = {vector}\n"
-   "3: targets = {vb}, decl\n"},
-  // Template type aliases.
-  {R"cpp(
+"0: targets = {vector}\n"
+"1: targets = {vi}, decl\n"
+"2: targets = {vector}\n"
+"3: targets = {vb}, decl\n"},
+   // Template type aliases.
+   {R"cpp(
 template  

[PATCH] D101328: [clangd] run clang-format on FindTargetTests.cpp

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
dgoldman requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Addressing comments in https://reviews.llvm.org/D98984


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101328

Files:
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -904,6 +904,7 @@
 }
   )cpp";
   EXPECT_DECLS("ObjCProtocolExpr", "@protocol Foo");
+  //
 
   Code = R"cpp(
 @interface Foo


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -904,6 +904,7 @@
 }
   )cpp";
   EXPECT_DECLS("ObjCProtocolExpr", "@protocol Foo");
+  //
 
   Code = R"cpp(
 @interface Foo
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100807: [clang][driver] Use the canonical Darwin arch name when printing out the triple for a Darwin target

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Reverted in ab0df6c0346e 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100807

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ab0df6c - Revert "[clang][driver] Use the provided arch name for a Darwin target triple"

[Alex Lorenz via cfe-commits]

Author: Alex Lorenz
Date: 2021-04-26T14:57:00-07:00
New Revision: ab0df6c0346e515291a381467527621ab0ccf953

URL: 
https://github.com/llvm/llvm-project/commit/ab0df6c0346e515291a381467527621ab0ccf953
DIFF: 
https://github.com/llvm/llvm-project/commit/ab0df6c0346e515291a381467527621ab0ccf953.diff

LOG: Revert "[clang][driver] Use the provided arch name for a Darwin target 
triple"

This reverts commit 6cc62043c8bf4daa27664a2e1674abbe8d0492c6.

This caused a test failure on a M1 mac CI job 
(https://reviews.llvm.org/D100807#2718006),
I will recommit this with a fix.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/aarch64-cpus.c
clang/test/Driver/arm64_32-link.c
clang/test/Driver/darwin-version.c
clang/test/Driver/default-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 177cfd60ffb3..bc59b6beafc7 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -74,12 +74,12 @@ void darwin::setTripleTypeForMachOArchName(llvm::Triple , 
StringRef Str) {
   const llvm::Triple::ArchType Arch = getArchTypeForMachOArchName(Str);
   llvm::ARM::ArchKind ArchKind = llvm::ARM::parseArch(Str);
   T.setArch(Arch);
-  if (Arch != llvm::Triple::UnknownArch)
-T.setArchName(Str);
 
-  if (ArchKind == llvm::ARM::ArchKind::ARMV6M ||
-  ArchKind == llvm::ARM::ArchKind::ARMV7M ||
-  ArchKind == llvm::ARM::ArchKind::ARMV7EM) {
+  if (Str == "x86_64h" || Str == "arm64e")
+T.setArchName(Str);
+  else if (ArchKind == llvm::ARM::ArchKind::ARMV6M ||
+   ArchKind == llvm::ARM::ArchKind::ARMV7M ||
+   ArchKind == llvm::ARM::ArchKind::ARMV7EM) {
 T.setOS(llvm::Triple::UnknownOS);
 T.setObjectFormat(llvm::Triple::MachO);
   }

diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 9a2fc857a0c2..5b9bd5207792 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -33,7 +33,7 @@
 // ARM64E-DARWIN: "-cc1"{{.*}} "-triple" "arm64e{{.*}}" "-target-cpu" 
"apple-a12"
 
 // RUN: %clang -target arm64-apple-darwin -arch arm64_32 -### -c %s 2>&1 | 
FileCheck -check-prefix=ARM64_32-DARWIN %s
-// ARM64_32-DARWIN: "-cc1"{{.*}} "-triple" "arm64_32{{.*}}" "-target-cpu" 
"apple-s4"
+// ARM64_32-DARWIN: "-cc1"{{.*}} "-triple" "aarch64_32{{.*}}" "-target-cpu" 
"apple-s4"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck 
-check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 
2>&1 | FileCheck -check-prefix=CA35 %s

diff  --git a/clang/test/Driver/arm64_32-link.c 
b/clang/test/Driver/arm64_32-link.c
index 6ef0b6f9f2f7..972ae6a234e1 100644
--- a/clang/test/Driver/arm64_32-link.c
+++ b/clang/test/Driver/arm64_32-link.c
@@ -1,4 +1,4 @@
 // RUN: %clang -target x86_64-apple-darwin -arch arm64_32 
-miphoneos-version-min=8.0 %s -### 2>&1 | FileCheck %s
 
-// CHECK: "-cc1"{{.*}} "-triple" "arm64_32-apple-ios8.0.0"
+// CHECK: "-cc1"{{.*}} "-triple" "aarch64_32-apple-ios8.0.0"
 // CHECK: ld{{.*}} "-arch" "arm64_32"

diff  --git a/clang/test/Driver/darwin-version.c 
b/clang/test/Driver/darwin-version.c
index 130a66575a93..3471552c937f 100644
--- a/clang/test/Driver/darwin-version.c
+++ b/clang/test/Driver/darwin-version.c
@@ -60,17 +60,17 @@
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX4 %s
 // RUN: %clang -target i686-apple-darwin9 -mmacosx-version-min=10.4 -c %s -### 
2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX4 %s
-// CHECK-VERSION-OSX4: "i686-apple-macosx10.4.0"
+// CHECK-VERSION-OSX4: "i386-apple-macosx10.4.0"
 // RUN: %clang -target i686-apple-darwin9 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX5 %s
 // RUN: %clang -target i686-apple-darwin9 -mmacosx-version-min=10.5 -c %s -### 
2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX5 %s
-// CHECK-VERSION-OSX5: "i686-apple-macosx10.5.0"
+// CHECK-VERSION-OSX5: "i386-apple-macosx10.5.0"
 // RUN: %clang -target i686-apple-darwin10 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX6 %s
 // RUN: %clang -target i686-apple-darwin9 -mmacosx-version-min=10.6 -c %s -### 
2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX6 %s
-// CHECK-VERSION-OSX6: "i686-apple-macosx10.6.0"
+// CHECK-VERSION-OSX6: "i386-apple-macosx10.6.0"
 // RUN: %clang -target x86_64-apple-darwin14 -c %s -### 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-OSX10 %s
 // RUN: %clang -target x86_64-apple-darwin -mmacosx-version-min=10.10 -c %s 
-### 2>&1 | \
@@ -100,7 +100,7 @@
 
 // Check environment variable gets interpreted correctly
 // RUN: env MACOSX_DEPLOYMENT_TARGET=10.5 IPHONEOS_DEPLOYMENT_TARGET=2.0 \
-// RUN:   %clang -target i686-apple-darwin9 -c %s -### 2>&1 | \
+// RUN:   %clang -target i386-apple-darwin9 -c %s 

[PATCH] D101327: [Clang][Driver] validate sysregs for -mstack-protector-guard-reg=

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 340662.
nickdesaulniers added a comment.

- remove comment about string case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101327

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c

Index: clang/test/Driver/stack-protector-guard.c
===
--- clang/test/Driver/stack-protector-guard.c
+++ clang/test/Driver/stack-protector-guard.c
@@ -46,6 +46,10 @@
 // RUN:   -mstack-protector-guard-reg=sp_el0 \
 // RUN:   -mstack-protector-guard-offset=0 %s 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-AARCH64 %s
+// RUN: %clang -### -target aarch64-linux-gnu -mstack-protector-guard=sysreg \
+// RUN:   -mstack-protector-guard-reg=sp_el2 \
+// RUN:   -mstack-protector-guard-offset=0 %s 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-AARCH64-SP_EL2 %s
 // RUN: %clang -### -target aarch64-linux-gnu \
 // RUN:   -mstack-protector-guard=tls %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-VALUE-AARCH64 %s
@@ -55,5 +59,6 @@
 // RUN: FileCheck -check-prefix=INVALID-REG-AARCH64 %s
 
 // CHECK-AARCH64: "-cc1" {{.*}}"-mstack-protector-guard=sysreg" "-mstack-protector-guard-offset=0" "-mstack-protector-guard-reg=sp_el0"
+// CHECK-AARCH64-SP_EL2: "-cc1" {{.*}}"-mstack-protector-guard=sysreg" "-mstack-protector-guard-offset=0" "-mstack-protector-guard-reg=sp_el2"
 // INVALID-VALUE-AARCH64: error: invalid value 'tls' in 'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' are:sysreg global'
 // INVALID-REG-AARCH64: error: invalid value 'foo' in 'mstack-protector-guard-reg=','for AArch64'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3140,7 +3140,7 @@
   << "for X86, valid arguments to '-mstack-protector-guard-reg=' are:fs gs";
   return;
 }
-if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+if (EffectiveTriple.isAArch64() && !aarch64::isValidSysReg(Value)) {
   D.Diag(diag::err_drv_invalid_value_with_suggestion)
   << A->getOption().getName() << Value << "for AArch64";
   return;
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -27,6 +27,8 @@
 std::string getAArch64TargetCPU(const llvm::opt::ArgList ,
 const llvm::Triple , llvm::opt::Arg *);
 
+bool isValidSysReg(StringRef RegName);
+
 } // end namespace aarch64
 } // end namespace target
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -14,6 +14,8 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
 
+#include 
+
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
@@ -493,3 +495,933 @@
   if (Args.hasArg(options::OPT_mno_neg_immediates))
 Features.push_back("+no-neg-immediates");
 }
+
+/// Checks if a string is a valid sysreg.
+// While it would be good to avoid duplication of
+// llvm::AArch64SysReg::lookupSysRegByName, we don't want to couple CFE to
+// Target/. This list is taken from the generated
+// lib/Target/AArch64/AArch64GenSystemOperands.inc.
+bool aarch64::isValidSysReg(StringRef RegName) {
+  static llvm::SmallVector ValidRegs = {
+  "ACCDATA_EL1",
+  "ACTLR_EL1",
+  "ACTLR_EL2",
+  "ACTLR_EL3",
+  "AFSR0_EL1",
+  "AFSR0_EL12",
+  "AFSR0_EL2",
+  "AFSR0_EL3",
+  "AFSR1_EL1",
+  "AFSR1_EL12",
+  "AFSR1_EL2",
+  "AFSR1_EL3",
+  "AIDR_EL1",
+  "AMAIR_EL1",
+  "AMAIR_EL12",
+  "AMAIR_EL2",
+  "AMAIR_EL3",
+  "AMCFGR_EL0",
+  "AMCGCR_EL0",
+  "AMCNTENCLR0_EL0",
+  "AMCNTENCLR1_EL0",
+  "AMCNTENSET0_EL0",
+  "AMCNTENSET1_EL0",
+  "AMCR_EL0",
+  "AMEVCNTR00_EL0",
+  "AMEVCNTR01_EL0",
+  "AMEVCNTR02_EL0",
+  "AMEVCNTR03_EL0",
+  "AMEVCNTR10_EL0",
+  "AMEVCNTR110_EL0",
+  "AMEVCNTR111_EL0",
+  "AMEVCNTR112_EL0",
+  "AMEVCNTR113_EL0",
+  "AMEVCNTR114_EL0",
+  "AMEVCNTR115_EL0",
+  "AMEVCNTR11_EL0",
+  "AMEVCNTR12_EL0",
+  "AMEVCNTR13_EL0",
+  "AMEVCNTR14_EL0",
+  "AMEVCNTR15_EL0",
+  "AMEVCNTR16_EL0",
+  "AMEVCNTR17_EL0",
+  "AMEVCNTR18_EL0",
+  "AMEVCNTR19_EL0",
+  "AMEVCNTVOFF00_EL2",
+  "AMEVCNTVOFF010_EL2",
+  "AMEVCNTVOFF011_EL2",
+  "AMEVCNTVOFF012_EL2",
+  "AMEVCNTVOFF013_EL2",
+

[PATCH] D100807: [clang][driver] Use the canonical Darwin arch name when printing out the triple for a Darwin target

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D100807#2717778 , @thakis wrote:

> Looks like this breaks tests on mac/arm (by making two unexpectedly pass): 
> http://45.33.8.238/macm1/8249/step_7.txt
>
> Please take a look, and please revert for now if it takes a while to fix.

I will revert it now.

It appears that your bot is running on a Mac M1  
is that correct? Which OS do you have installed? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100807

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101327: [Clang][Driver] validate sysregs for -mstack-protector-guard-reg=

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: echristo, jyknight, rsmith.
Herald added a subscriber: kristof.beyls.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In order to support arbitrary AArch64 sysregs for
-mstack-protector-guard-reg=, copy the generated list from
lib/Target/AArch64/AArch64GenSystemOperands.inc's lookupSysRegByName().
This avoids requiring the AArch64 backend to be linked to validate such
registers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101327

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c

Index: clang/test/Driver/stack-protector-guard.c
===
--- clang/test/Driver/stack-protector-guard.c
+++ clang/test/Driver/stack-protector-guard.c
@@ -46,6 +46,10 @@
 // RUN:   -mstack-protector-guard-reg=sp_el0 \
 // RUN:   -mstack-protector-guard-offset=0 %s 2>&1 | \
 // RUN: FileCheck -check-prefix=CHECK-AARCH64 %s
+// RUN: %clang -### -target aarch64-linux-gnu -mstack-protector-guard=sysreg \
+// RUN:   -mstack-protector-guard-reg=sp_el2 \
+// RUN:   -mstack-protector-guard-offset=0 %s 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-AARCH64-SP_EL2 %s
 // RUN: %clang -### -target aarch64-linux-gnu \
 // RUN:   -mstack-protector-guard=tls %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-VALUE-AARCH64 %s
@@ -55,5 +59,6 @@
 // RUN: FileCheck -check-prefix=INVALID-REG-AARCH64 %s
 
 // CHECK-AARCH64: "-cc1" {{.*}}"-mstack-protector-guard=sysreg" "-mstack-protector-guard-offset=0" "-mstack-protector-guard-reg=sp_el0"
+// CHECK-AARCH64-SP_EL2: "-cc1" {{.*}}"-mstack-protector-guard=sysreg" "-mstack-protector-guard-offset=0" "-mstack-protector-guard-reg=sp_el2"
 // INVALID-VALUE-AARCH64: error: invalid value 'tls' in 'mstack-protector-guard=','valid arguments to '-mstack-protector-guard=' are:sysreg global'
 // INVALID-REG-AARCH64: error: invalid value 'foo' in 'mstack-protector-guard-reg=','for AArch64'
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3140,7 +3140,7 @@
   << "for X86, valid arguments to '-mstack-protector-guard-reg=' are:fs gs";
   return;
 }
-if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+if (EffectiveTriple.isAArch64() && !aarch64::isValidSysReg(Value)) {
   D.Diag(diag::err_drv_invalid_value_with_suggestion)
   << A->getOption().getName() << Value << "for AArch64";
   return;
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -27,6 +27,8 @@
 std::string getAArch64TargetCPU(const llvm::opt::ArgList ,
 const llvm::Triple , llvm::opt::Arg *);
 
+bool isValidSysReg(StringRef RegName);
+
 } // end namespace aarch64
 } // end namespace target
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -14,6 +14,8 @@
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/Host.h"
 
+#include 
+
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
@@ -493,3 +495,933 @@
   if (Args.hasArg(options::OPT_mno_neg_immediates))
 Features.push_back("+no-neg-immediates");
 }
+
+/// Checks if a lowercase string is a valid sysreg.
+// While it would be good to avoid duplication of
+// llvm::AArch64SysReg::lookupSysRegByName, we don't want to couple CFE to
+// Target/. This list is taken from the generated
+// lib/Target/AArch64/AArch64GenSystemOperands.inc.
+bool aarch64::isValidSysReg(StringRef RegName) {
+  static llvm::SmallVector ValidRegs = {
+  "ACCDATA_EL1",
+  "ACTLR_EL1",
+  "ACTLR_EL2",
+  "ACTLR_EL3",
+  "AFSR0_EL1",
+  "AFSR0_EL12",
+  "AFSR0_EL2",
+  "AFSR0_EL3",
+  "AFSR1_EL1",
+  "AFSR1_EL12",
+  "AFSR1_EL2",
+  "AFSR1_EL3",
+  "AIDR_EL1",
+  "AMAIR_EL1",
+  "AMAIR_EL12",
+  "AMAIR_EL2",
+  "AMAIR_EL3",
+  "AMCFGR_EL0",
+  "AMCGCR_EL0",
+  "AMCNTENCLR0_EL0",
+  "AMCNTENCLR1_EL0",
+  "AMCNTENSET0_EL0",
+  "AMCNTENSET1_EL0",
+  "AMCR_EL0",
+  "AMEVCNTR00_EL0",
+  "AMEVCNTR01_EL0",
+  "AMEVCNTR02_EL0",
+  "AMEVCNTR03_EL0",
+  "AMEVCNTR10_EL0",
+  "AMEVCNTR110_EL0",
+  "AMEVCNTR111_EL0",
+  "AMEVCNTR112_EL0",
+  "AMEVCNTR113_EL0",
+  "AMEVCNTR114_EL0",
+  "AMEVCNTR115_EL0",
+  "AMEVCNTR11_EL0",

[PATCH] D98984: [clangd] Improve handling of Objective-C protocols in types

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 340659.
dgoldman marked an inline comment as done.
dgoldman added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98984

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -676,8 +676,7 @@
 @end
 @interface $Class_decl[[Klass]] <$Interface[[Protocol]]>
 @end
-// FIXME: protocol list in ObjCObjectType should be highlighted.
-id $Variable_decl[[x]];
+id<$Interface[[Protocol]]> $Variable_decl[[x]];
   )cpp",
   R"cpp(
 // ObjC: Categories
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -943,14 +943,32 @@
   )cpp";
   EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
 
+  Code = R"cpp(
+@class C;
+@protocol Foo
+@end
+void test([[C]] *p);
+  )cpp";
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@class C;");
+
   Code = R"cpp(
 @class C;
 @protocol Foo
 @end
 void test(C<[[Foo]]> *p);
   )cpp";
-  // FIXME: there's no AST node corresponding to 'Foo', so we're stuck.
-  EXPECT_DECLS("ObjCObjectTypeLoc");
+  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
+
+  Code = R"cpp(
+@class C;
+@protocol Foo
+@end
+@protocol Bar
+@end
+void test(C<[[Foo]], Bar> *p);
+  )cpp";
+  // FIXME: We currently can't disambiguate between multiple protocols.
+  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo", "@protocol Bar");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
@@ -1036,37 +1054,36 @@
 
 TEST_F(FindExplicitReferencesTest, All) {
   std::pair Cases[] =
-  {
-  // Simple expressions.
-  {R"cpp(
+  {// Simple expressions.
+   {R"cpp(
 int global;
 int func();
 void foo(int param) {
   $0^global = $1^param + $2^func();
 }
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"
-   "2: targets = {func}\n"},
-  {R"cpp(
+"0: targets = {global}\n"
+"1: targets = {param}\n"
+"2: targets = {func}\n"},
+   {R"cpp(
 struct X { int a; };
 void foo(X x) {
   $0^x.$1^a = 10;
 }
 )cpp",
-   "0: targets = {x}\n"
-   "1: targets = {X::a}\n"},
-  {R"cpp(
+"0: targets = {x}\n"
+"1: targets = {X::a}\n"},
+   {R"cpp(
 // error-ok: testing with broken code
 int bar();
 int foo() {
   return $0^bar() + $1^bar(42);
 }
 )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
-  // Namespaces and aliases.
-  {R"cpp(
+"0: targets = {bar}\n"
+"1: targets = {bar}\n"},
+   // Namespaces and aliases.
+   {R"cpp(
   namespace ns {}
   namespace alias = ns;
   void foo() {
@@ -1074,19 +1091,19 @@
 using namespace $1^alias;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {alias}\n"},
-  // Using declarations.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {alias}\n"},
+   // Using declarations.
+   {R"cpp(
   namespace ns { int global; }
   void foo() {
 using $0^ns::$1^global;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
-  // Simple types.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   // Simple types.
+   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
@@ -1095,13 +1112,13 @@
static_cast<$4^Struct*>(0);
  }
)cpp",
-   "0: targets = {Struct}\n"
-   "1: targets = {x}, decl\n"
-   "2: targets = {Typedef}\n"
-   "3: targets = {y}, decl\n"
-   "4: targets = {Struct}\n"},
-  // Name qualifiers.
-  {R"cpp(
+"0: targets = {Struct}\n"
+"1: targets = {x}, decl\n"
+"2: targets = {Typedef}\n"
+"3: targets = {y}, decl\n"
+"4: targets = {Struct}\n"},
+   // Name qualifiers.
+   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  

[PATCH] D100807: [clang][driver] Use the canonical Darwin arch name when printing out the triple for a Darwin target

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In D100807#2717778 , @thakis wrote:

> Looks like this breaks tests on mac/arm (by making two unexpectedly pass): 
> http://45.33.8.238/macm1/8249/step_7.txt
>
> Please take a look, and please revert for now if it takes a while to fix.

Thanks, I will take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100807

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101325: [CodeGenOptions] make StackProtectorGuardOffset signed

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:125
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;

krasimir wrote:
> penagos wrote:
> > krasimir wrote:
> > > penagos wrote:
> > > > MyDeveloperDay wrote:
> > > > > I don't really understand what we are saying here? 
> > > > Effectively we are checking that, barring intervening whitespace, we 
> > > > are analyzing 2 consecutive '>' tokens. If so, we treat such sequence 
> > > > as a binary op in lieu of a closing template angle bracket. If there's 
> > > > another more straightforward way of accomplishing this check, I'm open 
> > > > to that, but this seemed to be the most straightforward way at the time.
> > > I'm worried that this may regress template code. How does this account 
> > > for cases where two consecutive `>`-s are really two closing template 
> > > brackets, e.g.,
> > > `std::vector> v;`?
> > > 
> > > In particular, one added test case is ambiguous: `>>` could really be two 
> > > closing template brackets:
> > > https://godbolt.org/z/v19hj9vKn
> > > 
> > > I have to say that my general feeling about trying to disambiguate 
> > > between bitshifts and template closers is: don't try too hard inside 
> > > clang-format as the heuristics are generally quite brittle and make the 
> > > code harder to maintain; in cases where clang-format wrongly detects 
> > > bitshift as templates, users should add parens around the bitshift, which 
> > > IMO improves readability.
> > As this patch currently stands, it does not disambiguate between bitshift 
> > '>>' operators and 2 closing template brackets, so in your snippet, we 
> > would no longer insert a space between the '>' characters (despite arguably 
> > being the better formatting decision in this case).
> > 
> > I agree with your feeling that user guided disambiguation between bitshift 
> > operators and template closing brackets via parens is the ideal solution 
> > and also improves readability, but IMO the approach taken by clang-format 
> > to format the '>' token should be conservative in that any change made 
> > should be non-semantic altering, which is not presently the case. While the 
> > case you mentioned would regress, we would no longer potentially alter 
> > program semantics. Thinking about this more, would it make sense to modify 
> > the actual white-space change generation later on in the analysis to not 
> > break up >> sequences of characters in lieu of annotating the tokens 
> > differently as the proposed patch is currently doing?
> I tried and can't make this misinterpret two consecutive template `>` as a 
> bit shift, IMO because this check is guarded by the `Left->ParentBracket != 
> tok::less` condition. Both `std::vector> v;` and 
> `test> c;` below are handled correctly.
> I'm less worried about regressions in common template cases now.
> Thank you for pointing out altering program semantics, I agree.
> Please add a comment about this tradeoff and and a bit of the reasoning 
> behind it in code for future reference.
I had come to the same conclusion when modifying the conditional here; namely 
the ParentBracket predicate is what catches the case you were alluding to 
earlier.
I've added a brief comment to `parseAngle()` to document the need for the 
change, explaining the conservative nature of the change w.r.t. nested template 
cases; thank you for the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Jinsong Ji via Phabricator via cfe-commits]

jsji accepted this revision as: jsji.
jsji added inline comments.



Comment at: llvm/cmake/config-ix.cmake:285
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)

Maybe add a warning here about we are forcing it to 0?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

[Luis Penagos via Phabricator via cfe-commits]

penagos updated this revision to Diff 340656.
penagos added a comment.

Add justification comment for changes in parseAngle()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100778

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,10 +116,17 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ 
CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7649,6 +7649,8 @@
   verifyFormat("a = 1;", Style);
   verifyFormat("a >>= 1;", Style);
 
+  verifyFormat("test < a | b >> c;");
+  verifyFormat("test> c;");
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,10 +116,17 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
-isKeywordWithCondition(*Line.First))
+(isKeywordWithCondition(*Line.First) ||
+ CurrentToken->getStartOfNonWhitespace() ==
+ CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
+ -1)))
   return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

@nathanchance do you prefer me to revert the patch first as it might take me a 
while to investigate it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like the CI pipeline is failing on the tests.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:68
+  if (Desugar == Context.getBuiltinMSVaListType().getDesugaredType(Context)) {
+const auto MSVaListType = Context.getBuiltinMSVaListType();
+QualType Ty;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101259

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D101323#2717925 , @jsji wrote:

> Have we considered workaround this in `llvm/lib/Support/Unix/Path.inc` 
> instead?

Yes. I think this is a more general fix that would automatically apply in other 
situations where code should be checking for this macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98984: [clangd] Improve handling of Objective-C protocols in types

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 3 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1048
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"

sammccall wrote:
> there are a ton of whitespace changes in this patch, because the version at 
> HEAD isn't clang-format clean :-(
> 
> Feel free to format it first in a separate commit, but I'd prefer not to 
> reformat all the tests mixed in with other changes.
Will do if it hasn't been done already


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98984

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2844
+  auto *Fn =
+  dyn_cast(LV.getPointer(*this)->stripPointerCasts());
+  if (DI && !Fn->getSubprogram())

Oh, please change this to cast, rather than dyn_cast before committing. (since 
the Fn is unconditionally dereferenced on the next line (well, conditional on 
DI, but that's not relevant to this)

Could also move the "if (DI)" further out, like this:
```
if (CGDebugInfo *DI = ...) {
  auto *Fn = cast...
  if (!Fn->getSubprogram())
DI->EmitFunctionDecl(...);
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Have we considered workaround this in `llvm/lib/Support/Unix/Path.inc` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100919: [AArch64] Support customizing stack protector guard

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138
 }
+if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
+  D.Diag(diag::err_drv_invalid_value_with_suggestion)

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the frontend?
> > > I don't think so because 
> > > `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is under lib/ not 
> > > include/. Not sure if I should just remove reg validation?
> > Guidance provided by @echristo and @jyknight was that we should avoid such 
> > linkage requirements on Target/, so instead I'll work on adding a helper to 
> > clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates logic from 
> > `AArch64SysReg::lookupSysRegByName`.
> It looks like there's ~1000 possible sysregs for aarch64 ATM; do we really 
> want to add all of those to clang?
I'm going to post that as a separate commit/review on top of this series, that 
way it doesn't pollute this code review. This is ready to be reviewed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100919

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98984: [clangd] Improve handling of Objective-C protocols in types

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 340653.
dgoldman marked 2 inline comments as done.
dgoldman added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98984

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -676,8 +676,7 @@
 @end
 @interface $Class_decl[[Klass]] <$Interface[[Protocol]]>
 @end
-// FIXME: protocol list in ObjCObjectType should be highlighted.
-id $Variable_decl[[x]];
+id<$Interface[[Protocol]]> $Variable_decl[[x]];
   )cpp",
   R"cpp(
 // ObjC: Categories
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -943,14 +943,32 @@
   )cpp";
   EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
 
+  Code = R"cpp(
+@class C;
+@protocol Foo
+@end
+void test([[C]] *p);
+  )cpp";
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@class C;");
+
   Code = R"cpp(
 @class C;
 @protocol Foo
 @end
 void test(C<[[Foo]]> *p);
   )cpp";
-  // FIXME: there's no AST node corresponding to 'Foo', so we're stuck.
-  EXPECT_DECLS("ObjCObjectTypeLoc");
+  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
+
+  Code = R"cpp(
+@class C;
+@protocol Foo
+@end
+@protocol Bar
+@end
+void test(C<[[Foo]], Bar> *p);
+  )cpp";
+  // FIXME: We currently can't disambiguate between multiple protocols.
+  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo", "@protocol Bar");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
@@ -1036,37 +1054,36 @@
 
 TEST_F(FindExplicitReferencesTest, All) {
   std::pair Cases[] =
-  {
-  // Simple expressions.
-  {R"cpp(
+  {// Simple expressions.
+   {R"cpp(
 int global;
 int func();
 void foo(int param) {
   $0^global = $1^param + $2^func();
 }
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"
-   "2: targets = {func}\n"},
-  {R"cpp(
+"0: targets = {global}\n"
+"1: targets = {param}\n"
+"2: targets = {func}\n"},
+   {R"cpp(
 struct X { int a; };
 void foo(X x) {
   $0^x.$1^a = 10;
 }
 )cpp",
-   "0: targets = {x}\n"
-   "1: targets = {X::a}\n"},
-  {R"cpp(
+"0: targets = {x}\n"
+"1: targets = {X::a}\n"},
+   {R"cpp(
 // error-ok: testing with broken code
 int bar();
 int foo() {
   return $0^bar() + $1^bar(42);
 }
 )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
-  // Namespaces and aliases.
-  {R"cpp(
+"0: targets = {bar}\n"
+"1: targets = {bar}\n"},
+   // Namespaces and aliases.
+   {R"cpp(
   namespace ns {}
   namespace alias = ns;
   void foo() {
@@ -1074,19 +1091,19 @@
 using namespace $1^alias;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {alias}\n"},
-  // Using declarations.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {alias}\n"},
+   // Using declarations.
+   {R"cpp(
   namespace ns { int global; }
   void foo() {
 using $0^ns::$1^global;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
-  // Simple types.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   // Simple types.
+   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
@@ -1095,13 +1112,13 @@
static_cast<$4^Struct*>(0);
  }
)cpp",
-   "0: targets = {Struct}\n"
-   "1: targets = {x}, decl\n"
-   "2: targets = {Typedef}\n"
-   "3: targets = {y}, decl\n"
-   "4: targets = {Struct}\n"},
-  // Name qualifiers.
-  {R"cpp(
+"0: targets = {Struct}\n"
+"1: targets = {x}, decl\n"
+"2: targets = {Typedef}\n"
+"3: targets = {y}, decl\n"
+"4: targets = {Struct}\n"},
+   // Name qualifiers.
+   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; 

[PATCH] D100919: [AArch64] Support customizing stack protector guard

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 340652.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.

- add support for NPOT and negative offsets


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100919

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll

Index: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll
@@ -0,0 +1,73 @@
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=0 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-POSITIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-8 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-OFFSET %s
+; RUN: llc %s --stack-protector-guard=sysreg \
+; RUN:   --stack-protector-guard-reg=sp_el0 \
+; RUN:   --stack-protector-guard-offset=-1 -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Verify that we `mrs` from `SP_EL0` twice, rather than load from
+; __stack_chk_guard.
+define dso_local void @foo(i64 %t) local_unnamed_addr #0 {
+; CHECK-LABEL:   foo:
+; CHECK: // %bb.0: // %entry
+; CHECK-NEXT:stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
+; CHECK-NEXT:mov x29, sp
+; CHECK-NEXT:sub sp, sp, #16 // =16
+; CHECK-NEXT:.cfi_def_cfa w29, 16
+; CHECK-NEXT:.cfi_offset w30, -8
+; CHECK-NEXT:.cfi_offset w29, -16
+; CHECK-NEXT:mrs x8, SP_EL0
+; CHECK-NO-OFFSET:   ldr x8, [x8]
+; CHECK-POSITIVE-OFFSET: ldr x8, [x8, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x8, [x8, #-8]
+; CHECK-NPOT-OFFSET: ldur x8, [x8, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x8, [x8, #-1]
+; CHECK-NEXT:lsl x9, x0, #2
+; CHECK-NEXT:add x9, x9, #15 // =15
+; CHECK-NEXT:and x9, x9, #0xfff0
+; CHECK-NEXT:stur x8, [x29, #-8]
+; CHECK-NEXT:mov x8, sp
+; CHECK-NEXT:sub x0, x8, x9
+; CHECK-NEXT:mov sp, x0
+; CHECK-NEXT:bl baz
+; CHECK-NEXT:ldur x8, [x29, #-8]
+; CHECK-NEXT:mrs x9, SP_EL0
+; CHECK-NO-OFFSET:   ldr x9, [x9]
+; CHECK-POSITIVE-OFFSET: ldr x9, [x9, #8]
+; CHECK-NEGATIVE-OFFSET: ldr x9, [x9, #-8]
+; CHECK-NPOT-OFFSET: ldur x9, [x9, #1]
+; CHECK-NPOT-NEG-OFFSET: ldur x9, [x9, #-1]
+; CHECK-NEXT:cmp x9, x8
+; CHECK-NEXT:b.ne .LBB0_2
+; CHECK-NEXT:  // %bb.1: // %entry
+; CHECK-NEXT:mov sp, x29
+; CHECK-NEXT:ldp x29, x30, [sp], #16 // 16-byte Folded Reload
+; CHECK-NEXT:ret
+; CHECK-NEXT:  .LBB0_2: // %entry
+; CHECK-NEXT:bl __stack_chk_fail
+; CHECK-NOT: __stack_chk_guard
+entry:
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+
+declare dso_local void @baz(i32*) local_unnamed_addr
+
+attributes #0 = { sspstrong }
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -1901,6 +1901,30 @@
   }
 
   Register Reg = MI.getOperand(0).getReg();
+  if (MI.getOpcode() == AArch64::LOAD_STACK_GUARD) {
+TargetOptions Options = MI.getParent()->getParent()->getTarget().Options;
+if (Options.StackProtectorGuard == StackProtectorGuards::SysReg) {
+  const AArch64SysReg::SysReg *SrcReg =
+  AArch64SysReg::lookupSysRegByName(Options.StackProtectorGuardReg);
+  assert(SrcReg && "Unable to encode SysReg");
+  BuildMI(MBB, MI, DL, get(AArch64::MRS))
+  .addReg(Reg, RegState::Define)
+  .addImm(SrcReg->Encoding);
+  if (Options.StackProtectorGuardOffset % 8 == 0)
+BuildMI(MBB, MI, DL, get(AArch64::LDRXui))
+.addReg(Reg, getKillRegState(false))
+.addReg(Reg, RegState::Define)
+.addImm(Options.StackProtectorGuardOffset >> 3);
+  else
+BuildMI(MBB, MI, DL, get(AArch64::LDURXi))
+.addReg(Reg, 

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Looks good - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[David Tenty via Phabricator via cfe-commits]

daltenty accepted this revision as: daltenty.
daltenty added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101323

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77013: [AMDGPU] Add options -mamdgpu-ieee -mno-amdgpu-ieee

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


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

https://reviews.llvm.org/D77013

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 340650.
yonghong-song added a comment.

- use stripPointerCasts() to remove the cast of DeclRefExpr pointer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/debug-info-extern-callback.c


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu 
-emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12667,7 +12667,7 @@
 Diag(Var->getLocation(), diag::note_private_extern);
   }
 
-  if (Context.getTargetInfo().allowDebugInfoForExternalVar() &&
+  if (Context.getTargetInfo().allowDebugInfoForExternalRef() &&
   !Var->isInvalidDecl() && !getLangOpts().CPlusPlus)
 ExternalDeclarations.push_back(Var);
 
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2834,8 +2834,20 @@
 return LV;
   }
 
-  if (const auto *FD = dyn_cast(ND))
-return EmitFunctionDeclLValue(*this, E, FD);
+  if (const auto *FD = dyn_cast(ND)) {
+LValue LV = EmitFunctionDeclLValue(*this, E, FD);
+
+// Emit debuginfo for the function declaration if the target wants to.
+if (getContext().getTargetInfo().allowDebugInfoForExternalRef()) {
+  CGDebugInfo *DI = CGM.getModuleDebugInfo();
+  auto *Fn =
+  dyn_cast(LV.getPointer(*this)->stripPointerCasts());
+  if (DI && !Fn->getSubprogram())
+DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);
+}
+
+return LV;
+  }
 
   // FIXME: While we're emitting a binding from an enclosing scope, all other
   // DeclRefExprs we see should be implicitly treated as if they also refer to
Index: clang/lib/Basic/Targets/BPF.h
===
--- clang/lib/Basic/Targets/BPF.h
+++ clang/lib/Basic/Targets/BPF.h
@@ -76,7 +76,7 @@
 return None;
   }
 
-  bool allowDebugInfoForExternalVar() const override { return true; }
+  bool allowDebugInfoForExternalRef() const override { return true; }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 switch (CC) {
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1538,8 +1538,8 @@
 
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
-  /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  /// Whether target allows debuginfo types for decl only variables/functions.
+  virtual bool allowDebugInfoForExternalRef() const { return false; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-callback.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-callback.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -x c -debug-info-kind=limited -triple bpf-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+extern int do_work1(int);
+long bpf_helper1(void *callback_fn);
+long prog1() {
+  return bpf_helper1(_work1);
+}
+
+extern int do_work2();
+long prog2_1() {
+  return (long)_work2;
+}
+int do_work2() { return 0; }
+long prog2_2() {
+  return (long)_work2;
+}
+
+// CHECK: declare !dbg ![[FUNC1:[0-9]+]] i32 @do_work1
+// CHECK: define dso_local i32 @do_work2() #{{[0-9]+}} !dbg ![[FUNC2:[0-9]+]]
+
+// CHECK: ![[FUNC1]] = !DISubprogram(name: "do_work1"
+// CHECK: ![[FUNC2]] = distinct !DISubprogram(name: "do_work2"
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12667,7 +12667,7 @@
 Diag(Var->getLocation(), diag::note_private_extern);
   }
 
-  

[PATCH] D101325: [CodeGenOptions] make StackProtectorGuardOffset signed

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: xiangzhangllvm, craig.topper, MaskRay, 
LemonBoy.
Herald added subscribers: jansvoboda11, dexonsmith, dang, pengfei, hiraditya.
nickdesaulniers requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

GCC supports negative values for -mstack-protector-guard-offset=, this
should be a signed value. Pre-req to D100919 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101325

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/stack-protector-3.ll

Index: llvm/test/CodeGen/X86/stack-protector-3.ll
===
--- llvm/test/CodeGen/X86/stack-protector-3.ll
+++ llvm/test/CodeGen/X86/stack-protector-3.ll
@@ -4,6 +4,7 @@
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=fs -o - < %s | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-reg=gs -o - < %s | FileCheck --check-prefix=CHECK-GS %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=20 -o - < %s | FileCheck --check-prefix=CHECK-OFFSET %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -stack-protector-guard-offset=-20 -o - < %s | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
 
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
 ; CHECK-TLS-FS-40:   movq%fs:40, %rax
@@ -29,6 +30,15 @@
 ; CHECK-OFFSET-NEXT:  .cfi_def_cfa_offset 32
 ; CHECK-OFFSET-NEXT:  callq   __stack_chk_fail
 
+; CHECK-NEGATIVE-OFFSET:   movl$4294967276, %eax   # imm = 0xFFEC
+; CHECK-NEGATIVE-OFFSET:   movq%fs:(%rax), %rcx
+; CHECK-NEGATIVE-OFFSET:   movq%fs:(%rax), %rax
+; CHECK-NEGATIVE-OFFSET-NEXT:  cmpq16(%rsp), %rax
+; CHECK-NEGATIVE-OFFSET-NEXT:  jne .LBB0_2
+; CHECK-NEGATIVE-OFFSET:   .LBB0_2:
+; CHECK-NEGATIVE-OFFSET-NEXT:  .cfi_def_cfa_offset 32
+; CHECK-NEGATIVE-OFFSET-NEXT:  callq   __stack_chk_fail
+
 ; CHECK-GLOBAL:   movq__stack_chk_guard(%rip), %rax
 ; CHECK-GLOBAL:   movq__stack_chk_guard(%rip), %rax
 ; CHECK-GLOBAL-NEXT:  cmpq16(%rsp), %rax
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -2484,7 +2484,7 @@
 }
 
 static Constant* SegmentOffset(IRBuilder<> ,
-   unsigned Offset, unsigned AddressSpace) {
+   int Offset, unsigned AddressSpace) {
   return ConstantExpr::getIntToPtr(
   ConstantInt::get(Type::getInt32Ty(IRB.getContext()), Offset),
   Type::getInt8PtrTy(IRB.getContext())->getPointerTo(AddressSpace));
@@ -2501,11 +2501,11 @@
 } else {
   unsigned AddressSpace = getAddressSpace();
   // Specially, some users may customize the base reg and offset.
-  unsigned Offset = getTargetMachine().Options.StackProtectorGuardOffset;
+  int Offset = getTargetMachine().Options.StackProtectorGuardOffset;
   // If we don't set -stack-protector-guard-offset value:
   // %fs:0x28, unless we're using a Kernel code model, in which case
   // it's %gs:0x28.  gs:0x14 on i386.
-  if (Offset == (unsigned)-1)
+  if (Offset == INT_MAX)
 Offset = (Subtarget.is64Bit()) ? 0x28 : 0x14;
 
   const auto  = getTargetMachine().Options.StackProtectorGuardReg;
@@ -2576,7 +2576,7 @@
   if (Subtarget.isTargetAndroid()) {
 // %fs:0x48, unless we're using a Kernel code model, in which case it's %gs:
 // %gs:0x24 on i386
-unsigned Offset = (Subtarget.is64Bit()) ? 0x48 : 0x24;
+int Offset = (Subtarget.is64Bit()) ? 0x48 : 0x24;
 return SegmentOffset(IRB, Offset, getAddressSpace());
   }
 
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -80,7 +80,7 @@
 CGOPT(bool, XCOFFTracebackTable)
 CGOPT(std::string, BBSections)
 CGOPT(std::string, StackProtectorGuard)
-CGOPT(unsigned, StackProtectorGuardOffset)
+CGOPT(int, StackProtectorGuardOffset)
 CGOPT(std::string, StackProtectorGuardReg)
 CGOPT(unsigned, TLSSize)
 CGOPT(bool, EmulatedTLS)
@@ -375,9 +375,9 @@
   cl::init("none"));
   CGBINDOPT(StackProtectorGuardReg);
 
-  static cl::opt StackProtectorGuardOffset(
+  static cl::opt StackProtectorGuardOffset(
   "stack-protector-guard-offset", cl::desc("Stack protector guard offset"),
-  cl::init((unsigned)-1));
+  cl::init(INT_MAX));
   CGBINDOPT(StackProtectorGuardOffset);
 
   static cl::opt 

[PATCH] D101324: Hurd: Clean up Debian multiarch /usr/include/

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul created this revision.
sthibaul requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a follow-up of 35dd6470de84 
 for the 
Hurd case, to avoid the
duplication of the i386-gnu path, already provided by
Hurd::getMultiarchTriple.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101324

Files:
  clang/lib/Driver/ToolChains/Hurd.cpp


Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -170,11 +170,13 @@
 
   AddMultilibIncludeArgs(DriverArgs, CC1Args);
 
-  if (getTriple().getArch() == llvm::Triple::x86) {
-std::string Path = SysRoot + "/usr/include/i386-gnu";
-if (D.getVFS().exists(Path))
-  addExternCSystemInclude(DriverArgs, CC1Args, Path);
-  }
+  // On systems using multiarch, add /usr/include/$triple before
+  // /usr/include.
+  std::string MultiarchIncludeDir = getMultiarchTriple(D, getTriple(), 
SysRoot);
+  if (!MultiarchIncludeDir.empty() &&
+  D.getVFS().exists(SysRoot + "/usr/include/" + MultiarchIncludeDir))
+addExternCSystemInclude(DriverArgs, CC1Args,
+SysRoot + "/usr/include/" + MultiarchIncludeDir);
 
   // Add an include of '/include' directly. This isn't provided by default by
   // system GCCs, but is often used with cross-compiling GCCs, and harmless to


Index: clang/lib/Driver/ToolChains/Hurd.cpp
===
--- clang/lib/Driver/ToolChains/Hurd.cpp
+++ clang/lib/Driver/ToolChains/Hurd.cpp
@@ -170,11 +170,13 @@
 
   AddMultilibIncludeArgs(DriverArgs, CC1Args);
 
-  if (getTriple().getArch() == llvm::Triple::x86) {
-std::string Path = SysRoot + "/usr/include/i386-gnu";
-if (D.getVFS().exists(Path))
-  addExternCSystemInclude(DriverArgs, CC1Args, Path);
-  }
+  // On systems using multiarch, add /usr/include/$triple before
+  // /usr/include.
+  std::string MultiarchIncludeDir = getMultiarchTriple(D, getTriple(), SysRoot);
+  if (!MultiarchIncludeDir.empty() &&
+  D.getVFS().exists(SysRoot + "/usr/include/" + MultiarchIncludeDir))
+addExternCSystemInclude(DriverArgs, CC1Args,
+SysRoot + "/usr/include/" + MultiarchIncludeDir);
 
   // Add an include of '/include' directly. This isn't provided by default by
   // system GCCs, but is often used with cross-compiling GCCs, and harmless to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

Looks like stripPointerCasts() is enough to make it work (no null Function 
pointer any more). This is consistent with

  llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
  StringRef MangledName, llvm::Type *Ty, GlobalDecl GD, bool ForVTable,
  bool DontDefer, bool IsThunk, llvm::AttributeList ExtraAttrs,
  ForDefinition_t IsForDefinition) {
const Decl *D = GD.getDecl();
  ...
// Make sure the result is of the requested type.
if (!IsIncompleteFunction) {
  assert(F->getFunctionType() == Ty);
  return F;
} 
  
llvm::Type *PTy = llvm::PointerType::getUnqual(Ty);
return llvm::ConstantExpr::getBitCast(F, PTy);
  }

It either returns a Function, or a Function-Casting-To-ConstantExpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100874: [OpenMP] Use irbuilder as default for masked and master construct

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 340641.
cchen added a comment.
This revision is now accepted and ready to land.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100874

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/masked_codegen.cpp
  clang/test/OpenMP/master_codegen.cpp

Index: clang/test/OpenMP/master_codegen.cpp
===
--- clang/test/OpenMP/master_codegen.cpp
+++ clang/test/OpenMP/master_codegen.cpp
@@ -1,10 +1,7 @@
-// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,NORMAL
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,NORMAL
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
-// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefixes=ALL,IRBUILDER
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -emit-llvm %s -fexceptions -fcxx-exceptions -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
@@ -15,53 +12,52 @@
 #ifndef HEADER
 #define HEADER
 
-// ALL:   [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
+// CHECK:   [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
 
-// ALL:   define {{.*}}void [[FOO:@.+]]()
+// CHECK:   define {{.*}}void [[FOO:@.+]]()
 
 void foo() { extern void mayThrow(); mayThrow(); }
 
-// ALL-LABEL: @main
+// CHECK-LABEL: @main
 // TERM_DEBUG-LABEL: @main
 int main() {
-  // ALL:  			[[A_ADDR:%.+]] = alloca i8
+  // CHECK:  			[[A_ADDR:%.+]] = alloca i8
   char a;
 
-// ALL:   			[[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// ALL:   			[[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// ALL-NEXT:  			[[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
-// ALL-NEXT:  			br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
-// ALL:   			[[THEN]]
-// ALL-NEXT:  			store i8 2, i8* [[A_ADDR]]
-// ALL-NEXT:  			call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// ALL-NEXT:  			br label {{%?}}[[EXIT]]
-// ALL:   			[[EXIT]]
+// CHECK:   			[[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// CHECK:   			[[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// CHECK-NEXT:  			[[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
+// CHECK-NEXT:  			br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
+// CHECK:   			[[THEN]]
+// CHECK-NEXT:  			store i8 2, i8* [[A_ADDR]]
+// CHECK-NEXT:  			call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
+// CHECK-NEXT:  			br label {{%?}}[[EXIT]]
+// CHECK:   			[[EXIT]]
 #pragma omp master
   a = 2;
-// IRBUILDER: 			[[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
-// ALL:   			[[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// ALL-NEXT:  			[[IS_MASTER:%.+]] = icmp ne i32 [[RES]], 0
-// ALL-NEXT:  			br i1 [[IS_MASTER]], label {{%?}}[[THEN:.+]], label {{%?}}[[EXIT:.+]]
-// ALL:   			[[THEN]]
-// IRBUILDER-NEXT:  call {{.*}}void [[FOO]]()
-// NORMAL-NEXT:  		invoke {{.*}}void [[FOO]]()
-// ALL:   			call {{.*}}void @__kmpc_end_master([[IDENT_T_TY]]* [[DEFAULT_LOC]], i32 [[GTID]])
-// ALL-NEXT:  			br label {{%?}}[[EXIT]]
-// ALL:   			[[EXIT]]
+// CHECK: 			[[GTID:%.+]] = call {{.*}}i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:@.+]])
+// CHECK:   			[[RES:%.+]] = call {{.*}}i32 @__kmpc_master([[IDENT_T_TY]]* 

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

In D94355#2717813 , @gulfem wrote:

> Thanks for the report @nathanchance, and I'm looking at it now.

Thanks!

> Is this failure from one of the bots?

No, this was discovered by a user of my LLVM build script:

https://github.com/ClangBuiltLinux/tc-build

https://github.com/kdrag0n/proton-clang-build/runs/2371499466?check_suite_focus=true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101323: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: daltenty, Xiangling_L, jsji.
Herald added a subscriber: mgorny.
hubert.reinterpretcast requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The value observed for the `mtim.tv_nsec` member is erroneous in some AIX 
environments. Avoid using this member by forcing 
`HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC` to `0`.

This resolves "mtime changed" errors such as the one 
http://lab.llvm.org:8014/#/builders/126/builds/330/steps/5/logs/FAIL__Clang__test_c
 has.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101323

Files:
  llvm/cmake/config-ix.cmake


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )


Index: llvm/cmake/config-ix.cmake
===
--- llvm/cmake/config-ix.cmake
+++ llvm/cmake/config-ix.cmake
@@ -280,8 +280,14 @@
 
 CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
 "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
-CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
-"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
+# environments.
+  set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
+else()
+  CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
+  "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
+endif()
 
 check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
 if( LLVM_USING_GLIBC )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94355: [Passes] Add relative lookup table converter pass

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

> This patch breaks a two stage build with LTO:

Thanks for the report @nathanchance, and I'm looking at it now.
Is this failure from one of the bots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94355

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

yeah you have to manually go to "Edit Revision" and update the message.

I'll just amend it locally though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Michael Benfield via Phabricator via cfe-commits]

mbenfield added a comment.

In D100581#2717182 , @aeubanks wrote:

> I can land this for you, but could you update the description to be a bit 
> more descriptive? Explaining what the warnings do.

Are you looking at the updated message in the commit, or the old summary 
displayed in Phabricator? It seems Phabricator doesn't change the summary when 
I update the commit description. Let me know whether this is what you thought, 
or if the current commit message is not adequate. The current commit message is

[Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

These are intended to mimic warnings available in gcc.

-Wunused-but-set-variable is triggered in the case of a variable which
appears on the LHS of an assignment but not otherwise used.

For instance:

  void f() {
  int x;
  x = 0;
  }

-Wunused-but-set-parameter works similarly, but for function parameters
instead of variables.

In C++, they are triggered only for scalar types; otherwise, they are
triggered for all types. This is gcc's behavior.

-Wunused-but-set-parameter is controlled by -Wextra, while
-Wunused-but-set-variable is controlled by -Wunused. This is slightly
different from gcc's behavior, but seems most consistent with clang's
behavior for -Wunused-parameter and -Wunused-variable.

Differential Revision: https://reviews.llvm.org/D100581


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101322: [clang-format] Add comment justification for https://reviews.llvm.org/D100778

[Luis Penagos via Phabricator via cfe-commits]

penagos added a comment.

Please ignore this revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101322

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100807: [clang][driver] Use the canonical Darwin arch name when printing out the triple for a Darwin target

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on mac/arm (by making two unexpectedly pass): 
http://45.33.8.238/macm1/8249/step_7.txt

Please take a look, and please revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100807

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101322: [clang-format] Add comment justification for https://reviews.llvm.org/D100778

[Luis Penagos via Phabricator via cfe-commits]

penagos created this revision.
penagos requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101322

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,7 +116,11 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
 (isKeywordWithCondition(*Line.First) ||


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -116,7 +116,11 @@
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
 // Try to do a better job at looking for ">>" within the condition of
-// a statement.
+// a statement. Conservatively insert spaces between consecutive ">"
+// tokens to prevent splitting right bitshift operators and potentially
+// altering program semantics. This check is overly conservative and
+// will prevent spaces from being inserted in select nested template
+// parameter cases, but should not alter program semantics.
 if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
 Left->ParentBracket != tok::less &&
 (isKeywordWithCondition(*Line.First) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Sorry for the delay. I think all my points have been resolved beside the 
insertion SourceLoc.




Comment at: clang/lib/Interpreter/IncrementalParser.cpp:184
+  SourceLocation NewLoc = SM.getLocForStartOfFile(SM.getMainFileID())
+  .getLocWithOffset(InputCount + 2);
+

v.g.vassilev wrote:
> teemperor wrote:
> > The `+ 2` here is probably not what you want. This will just give you a 
> > pointer into Clang's source buffers and will eventually point to random 
> > source buffers (or worse) once InputCount is large enough.
> > 
> > I feel like the proper solution is to just use the StartOfFile Loc and 
> > don't add any offset to it. I think Clang should still be able to figure 
> > out a reasonable ordering for overload candidates etc. 
> > 
> > (I thought I already commented on this line before, but I can't see my 
> > comment or any replies on Phab so I'm just commenting again).
> > The `+ 2` here is probably not what you want. This will just give you a 
> > pointer into Clang's source buffers and will eventually point to random 
> > source buffers (or worse) once InputCount is large enough.
> 
> Indeed.
> 
> > 
> > I feel like the proper solution is to just use the StartOfFile Loc and 
> > don't add any offset to it. I think Clang should still be able to figure 
> > out a reasonable ordering for overload candidates etc. 
> 
> That particular part of the input processing has been causing a lot of 
> troubles for cling over the years. If we use the StartOfFile each new line 
> will appear before the previous which can be problematic for as you say 
> diagnostics but also template instantiations.
> 
> Cling ended up solving this by introducing a virtual file with impossible to 
> allocate size of `1U << 15U` 
> (https://github.com/vgvassilev/cling/blob/da1bb78f3dea4d2bf19b383aeb1872e9f2b117ad/lib/Interpreter/CIFactory.cpp#L1516-L1527
>  and 
> https://github.com/vgvassilev/cling/blob/da1bb78f3dea4d2bf19b383aeb1872e9f2b117ad/lib/Interpreter/IncrementalParser.cpp#L394)
>  Then we are save to get Loc + 1 (I do not remember how that + 2 came 
> actually) and you should be still safe.
> 
> I wonder if that's something we should do here? 
> 
> > 
> > (I thought I already commented on this line before, but I can't see my 
> > comment or any replies on Phab so I'm just commenting again).
> 
> 
I think my point then is: If we would change Clang's behaviour to consider the 
insertion order in the include tree when deciding the SourceLocation order, 
wouldn't that fix the problem? IIUC that's enough to make this case work the 
intended way without requiring some fake large source file. It would also make 
this work in other projects such as LLDB.

So IMHO we could just use StartOfFile as the loc here and then consider the 
wrong ordering as a Clang bug (and add a FIXME for that here).


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

https://reviews.llvm.org/D96033

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92639: [analyzer] Add control flow arrows to the analyzer's HTML reports

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D92639#2717134 , @vsavchenko wrote:

> In D92639#2716973 , @ASDenysPetrov 
> wrote:
>
>> @vsavchenko 
>> I make some tests and fixes. Please, consider.
>
> OMG, that's so awesome! Thank you so much!

Never mind :-) Apply the changes and I will approve the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92639

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99386: [compiler-rt][hwasan] Add definition for Symbolizer::SymbolizeFrame

[Leonard Chan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa786f2badc41: [compiler-rt][hwasan] Add definition for 
Symbolizer::SymbolizeFrame (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99386

Files:
  compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp


Index: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
===
--- compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -54,6 +54,10 @@
   return false;
 }
 
+// This is mainly used by hwasan for online symbolization. This isn't needed
+// since hwasan can always just dump stack frames for offline symbolization.
+bool Symbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) { return false; }
+
 // This is used in some places for suppression checking, which we
 // don't really support for Fuchsia.  It's also used in UBSan to
 // identify a PC location to a function name, so we always fill in


Index: compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
===
--- compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -54,6 +54,10 @@
   return false;
 }
 
+// This is mainly used by hwasan for online symbolization. This isn't needed
+// since hwasan can always just dump stack frames for offline symbolization.
+bool Symbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) { return false; }
+
 // This is used in some places for suppression checking, which we
 // don't really support for Fuchsia.  It's also used in UBSan to
 // identify a PC location to a function name, so we always fill in
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2843-2844
+  CGDebugInfo *DI = CGM.getModuleDebugInfo();
+  auto *Fn = dyn_cast(LV.getPointer(*this));
+  if (DI && Fn && !Fn->getSubprogram())
+DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn);

yonghong-song wrote:
> dblaikie wrote:
> > It looks like there isn't any test coverage for the case where Fn is null 
> > here (I added an assertion that Fn is non-null and it didn't fire when 
> > running check-clang) - please add some to this patch. (I'd like to then 
> > look at those cases to better understand when they come up and whether 
> > there's a different/better way to phrase this code to handle those cases)
> Sorry. It is my fault. Tested with a slight different test case than this one 
> in my actual test setup. Now the new test should reflect null Fn. Thanks!
Thanks for the updated test case!

I think for generality, either this code shouldn't use the `LV` value - instead 
calling `GetOrCreateLLVMFunction` (though that's probably not great - since 
that would involve things like going through ConvertType and other shenanigans 
in `GetAddrOfFunction` - I guess if pieces of `GetAddrToFunction`, 
`EmitFunctionDeclPointer`, and `EmitFunctionDeclPointer` were split up maybe it 
could be done - or changet he return value to include the raw llvm::Function 
result in addition to the LValue (& keep the existing function name returning 
just the LValue for other callers)) - or doing more work to unwrap the Constant 
- maybe it would be enough to use "stripPointerCasts" & that'd get you 
something you can cast to llvm::Function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99386: [compiler-rt][hwasan] Add definition for Symbolizer::SymbolizeFrame

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99386

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99381: [compiler-rt][hwasan] Add Fuchsia-specific sanitizer platform limits

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping* @mcgrathr Given my explanation before, does it still make sense to 
refactor in this case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99381

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

> That's odd. Does your `CMakeCache.txt` file have 
> `CLANG_ROUND_TRIP_CC1_ARGS:BOOL=OFF` by any chance? It's possible it didn't 
> get switched to `ON` when D97462  landed.
>
> With the way the tests are written now, I'd expect them to fail in assert 
> build without the code generating `-fcxx-abi=`.

Ah, I did not have that flag ON by default (incrementally working off an old 
invocation), and turning this on without the `GenerateArg` bit causes 
`cxx-abi-switch.cpp` to fail. Thanks for catching this!

In D85802#2713395 , @phosek wrote:

> I don't see any checks to ensure that the selected ABI is compatible with the 
> specified target, is that something you plan on implementing in a follow up 
> change?

Added a check for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 340620.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85802

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TargetCXXABI.def
  clang/include/clang/Basic/TargetCXXABI.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/cxx-abi-switch.cpp
  clang/test/Frontend/invalid-cxx-abi.cpp

Index: clang/test/Frontend/invalid-cxx-abi.cpp
===
--- /dev/null
+++ clang/test/Frontend/invalid-cxx-abi.cpp
@@ -0,0 +1,34 @@
+// These should succeed.
+// RUN: %clang -c -fc++-abi=itanium %s
+// RUN: %clang -c -fc++-abi=arm -target arm64 %s
+// RUN: %clang -c -fc++-abi=ios -target arm64 %s
+// RUN: %clang -c -fc++-abi=aarch64 -target arm64 %s
+// RUN: %clang -c -fc++-abi=mips -target mips %s
+// RUN: %clang -c -fc++-abi=webassembly -target wasm64 %s
+// RUN: %clang -c -fc++-abi=fuchsia -target x86_64 %s
+// RUN: %clang -S -fc++-abi=xl -target powerpc-unknown-aix %s -o /dev/null
+// RUN: %clang -c -fc++-abi=microsoft -target x86_64-windows-msvc %s
+// RUN: %clang_cc1 -fc++-abi=itanium %s
+// RUN: %clang_cc1 -fc++-abi=arm -triple arm64 %s
+// RUN: %clang_cc1 -fc++-abi=ios -triple arm64 %s
+// RUN: %clang_cc1 -fc++-abi=aarch64 -triple arm64 %s
+// RUN: %clang_cc1 -fc++-abi=mips -triple mips %s
+// RUN: %clang_cc1 -fc++-abi=webassembly -triple wasm64 %s
+// RUN: %clang_cc1 -fc++-abi=fuchsia -triple x86_64 %s
+// RUN: %clang_cc1 -S -fc++-abi=xl -triple powerpc-unknown-aix %s -o /dev/null
+// RUN: %clang_cc1 -fc++-abi=microsoft -triple x86_64-windows-msvc %s
+
+// RUN: not %clang -c -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
+// RUN: not %clang -c -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
+// RUN: not %clang_cc1 -fc++-abi=InvalidABI %s 2>&1 | FileCheck %s -check-prefix=INVALID
+// RUN: not %clang_cc1 -fc++-abi=Fuchsia %s 2>&1 | FileCheck %s -check-prefix=CASE-SENSITIVE
+// INVALID: error: Invalid C++ ABI name 'InvalidABI'
+// CASE-SENSITIVE: error: Invalid C++ ABI name 'Fuchsia'
+
+// The flag is propgated from the driver to cc1.
+// RUN: %clang -fc++-abi=InvalidABI %s -### 2>&1 | FileCheck %s -check-prefix=CC1-FLAG
+// CC1-FLAG: -fc++-abi=InvalidABI
+
+// Some C++ ABIs are not supported on some platforms.
+// RUN: not %clang_cc1 -c -fc++-abi=fuchsia -triple i386 %s 2>&1 | FileCheck %s -check-prefix=UNSUPPORTED-FUCHSIA
+// UNSUPPORTED-FUCHSIA: error: C++ ABI 'fuchsia' is not supported on target triple 'i386'
Index: clang/test/CodeGenCXX/cxx-abi-switch.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx-abi-switch.cpp
@@ -0,0 +1,27 @@
+// Assert that the ABI switch uses the proper codegen. Fuchsia uses the
+// "return this" ABI on constructors and destructors by default, so we can just
+// check that ABI is used regardless of the target.
+//
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-unknown-linux-gnu -fc++-abi=fuchsia | FileCheck %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=aarch64-unknown-linux-gnu -fc++-abi=fuchsia | FileCheck %s
+
+class A {
+public:
+  virtual ~A();
+  int x_;
+};
+
+class B : public A {
+public:
+  B(int *i);
+  virtual ~B();
+  int *i_;
+};
+
+B::B(int *i) : i_(i) {}
+B::~B() {}
+
+// CHECK: define{{.*}} %class.B* @_ZN1BC2EPi(%class.B* {{[^,]*}} returned {{[^,]*}} %this, i32* %i)
+// CHECK: define{{.*}} %class.B* @_ZN1BC1EPi(%class.B* {{[^,]*}} returned {{[^,]*}} %this, i32* %i)
+// CHECK: define{{.*}} %class.B* @_ZN1BD2Ev(%class.B* {{[^,]*}} returned {{[^,]*}} %this)
+// CHECK: define{{.*}} %class.B* @_ZN1BD1Ev(%class.B* {{[^,]*}} returned {{[^,]*}} %this)
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3506,6 +3506,10 @@
   if (Opts.getSignReturnAddressKey() ==
   LangOptions::SignReturnAddressKeyKind::BKey)
 GenerateArg(Args, OPT_msign_return_address_key_EQ, "b_key", SA);
+
+  if (Opts.CXXABI)
+GenerateArg(Args, OPT_fcxx_abi_EQ, TargetCXXABI::getSpelling(*Opts.CXXABI),
+SA);
 }
 
 bool CompilerInvocation::ParseLangArgs(LangOptions , ArgList ,
@@ -3989,6 +3993,58 @@
 }
   }
 
+  // Check that the kind provided by the fc++-abi flag is supported on this
+  // target. Users who want to experiment using different ABIs on specific
+  // platforms can change this freely, but this function should be conservative
+  // enough such that not all ABIs are allowed on all platforms. For 

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:152
 //
 // TODO: Make previous parameters part of the signature for Objective-C
 // methods.

sammccall wrote:
> is this TODO handled now?
No, this does not change any behavior for parameters (we don't show the full 
ObjcMethodDecl signature including previous parameter types if you complete it 
from the middle)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 340619.
dgoldman marked 4 inline comments as done.
dgoldman added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100798

Files:
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2783,6 +2783,61 @@
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, ObjectiveCMethodDeclaration) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Qualifier("- (int)")));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c secondArgument:(id)object")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationPrefixTyped) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c;
+  @end
+  @implementation Foo
+  - (int)valueFor^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("valueForCharacter:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(char)c")));
+}
+
+TEST(CompletionTest, ObjectiveCMethodDeclarationFromMiddle) {
+  auto Results = completions(R"objc(
+  @interface Foo
+  - (int)valueForCharacter:(char)c secondArgument:(id)object;
+  @end
+  @implementation Foo
+  - (int)valueForCharacter:(char)c second^
+  @end
+)objc",
+ /*IndexSymbols=*/{},
+ /*Opts=*/{}, "Foo.m");
+
+  auto C = Results.Completions;
+  EXPECT_THAT(C, ElementsAre(Named("secondArgument:")));
+  EXPECT_THAT(C, ElementsAre(Kind(CompletionItemKind::Method)));
+  EXPECT_THAT(C, ElementsAre(Signature("(id)object")));
+}
+
 TEST(CompletionTest, CursorInSnippets) {
   clangd::CodeCompleteOptions Options;
   Options.EnableSnippets = true;
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -114,6 +114,7 @@
   }
   unsigned SnippetArg = 0;
   bool HadObjCArguments = false;
+  bool HadInformativeChunks = false;
   for (const auto  : CCS) {
 // Informative qualifier chunks only clutter completion results, skip
 // them.
@@ -129,10 +130,14 @@
   //   reclassified as qualifiers.
   //
   // Objective-C:
-  //   Objective-C methods may have multiple typed-text chunks, so we must
-  //   treat them carefully. For Objective-C methods, all typed-text chunks
-  //   will end in ':' (unless there are no arguments, in which case we
-  //   can safely treat them as C++).
+  //   Objective-C methods expressions may have multiple typed-text chunks,
+  //   so we must treat them carefully. For Objective-C methods, all
+  //   typed-text and informative chunks will end in ':' (unless there are
+  //   no arguments, in which case we can safely treat them as C++).
+  //
+  //   Completing a method declaration itself (not a method expression) is
+  //   similar except that we use the `RequiredQualifiers` to store the
+  //   text before the selector, e.g. `- (void)`.
   if (!llvm::StringRef(Chunk.Text).endswith(":")) { // Treat as C++.
 if (RequiredQualifiers)
   *RequiredQualifiers = std::move(*Signature);
@@ -147,6 +152,28 @@
 // methods.
 if (!HadObjCArguments) {
   HadObjCArguments = true;
+  // If we have no previous informative chunks (informative selector
+  // fragments in practice), we treat any previous chunks as
+  // `RequiredQualifiers` so they will be added as a prefix during the
+  // completion.
+  //
+  // e.g. to complete `- (void)doSomething:(id)argument`:
+  // - Completion name: `doSomething:`
+  // - RequiredQualifiers: `- (void)`
+  // - Snippet/Signature suffix: `(id)argument`
+  //
+  // This differs from the case when we're completing a method
+  //