[PATCH] D145214: [TSAN] add support for riscv64

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: compiler-rt/CMakeLists.txt:529
 # natively, but supports --as-needed/--no-as-needed for GNU ld compatibility.
-if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc")
+if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc|riscv")
   list(APPEND SANITIZER_COMMON_LINK_LIBS -Wl,--as-needed atomic 
-Wl,--no-as-needed)

This is only to work around a long-standing bug in GCC's atomics implementation 
for RISC-V. If you build compiler-rt with Clang you don't need this, and in 
theory future versions of GCC (possibly trunk already fixes this, I'm not sure 
of the status of it).



Comment at: compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp:60
 template T func_xchg(volatile T *v, T op) {
-  T res = __sync_lock_test_and_set(v, op);
+  T res = __atomic_test_and_set(v, op);
   // __sync_lock_test_and_set does not contain full barrier.

?



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform.h:971
 };
-const uptr indicator = 0x0e00ull;
+const uptr indicator = 0x0f00ull;
 const uptr ind_lsb = 1ull << LeastSignificantSetBitIndex(indicator);

?



Comment at: compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:270-271
+#elif SANITIZER_RISCV64
+  // the bottom half of vma is allocated for userspace
+  vmaSize = vmaSize + 1;
+# if !SANITIZER_GO

Why don't other architectures need the +1?



Comment at: compiler-rt/lib/tsan/tests/CMakeLists.txt:59
+  if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "riscv")
+list(APPEND TSAN_UNITTEST_LINK_FLAGS -latomic)
+  endif()

Same as before



Comment at: compiler-rt/test/tsan/test.h:77
+#elif defined(__riscv) && __riscv_xlen == 64
+const int kPCInc = 2;
 #else

What's this the length of? RVC is optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145214

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

[PATCH] D145214: [TSAN] add support for riscv64

[Alex Fan via Phabricator via cfe-commits]

alexfanqi created this revision.
Herald added subscribers: luke, Enna1, VincentWu, vkmr, frasercrmck, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
fedor.sergeev, arichardson.
Herald added a project: All.
alexfanqi updated this revision to Diff 502351.
alexfanqi added a comment.
alexfanqi retitled this revision from "[TSAN] add RISCV support" to "[TSAN] add 
support for riscv64".
alexfanqi edited the summary of this revision.
alexfanqi added reviewers: MaskRay, dvyukov, asb, StephenFan.
alexfanqi published this revision for review.
Herald added subscribers: Sanitizers, cfe-commits, pcwang-thead, eopXD.
Herald added projects: clang, Sanitizers.

fix tsan_rtl_riscv64.S and test failures

remaining test failure:
ThreadSanitizer-riscv64 :: custom_mutex5.cpp   
The debuginfo line reported is slightly off. 
custom_mutex5.cpp:14 -> 15
custom_mutex5.cpp:22 -> 23

ThreadSanitizer-riscv64 :: mmap_lots.cpp
flaky, the last 1000 sized mmap can get allocated to 0x-0x1000


Implements for sv39 and sv48 VMA layout.

Userspace only has access to the bottom half of vma range. The top half is used 
by kernel.
There is no dedicated vsyscall or heap segment.
PIE program is allocated to start at TASK_SIZE/3*2. Maximum ASLR is 
ARCH_MMAP_RND_BITS_MAX+PAGE_SHIFT=24+12=36
Loader, vdso and other libraries are allocated below stack from the top.

Both riscv32 and riscv64 need libatomic for 1,2 bytes atomic ops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145214

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
  compiler-rt/lib/tsan/rtl/CMakeLists.txt
  compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
  compiler-rt/lib/tsan/rtl/tsan_interface_atomic.cpp
  compiler-rt/lib/tsan/rtl/tsan_platform.h
  compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
  compiler-rt/lib/tsan/rtl/tsan_rtl.h
  compiler-rt/lib/tsan/rtl/tsan_rtl_riscv64.S
  compiler-rt/lib/tsan/tests/CMakeLists.txt
  compiler-rt/test/tsan/map32bit.cpp
  compiler-rt/test/tsan/mmap_large.cpp
  compiler-rt/test/tsan/test.h

Index: compiler-rt/test/tsan/test.h
===
--- compiler-rt/test/tsan/test.h
+++ compiler-rt/test/tsan/test.h
@@ -73,6 +73,8 @@
 const int kPCInc = 1;
 #elif defined(__sparc__) || defined(__mips__)
 const int kPCInc = 8;
+#elif defined(__riscv) && __riscv_xlen == 64
+const int kPCInc = 2;
 #else
 const int kPCInc = 4;
 #endif
Index: compiler-rt/test/tsan/mmap_large.cpp
===
--- compiler-rt/test/tsan/mmap_large.cpp
+++ compiler-rt/test/tsan/mmap_large.cpp
@@ -17,7 +17,8 @@
 int main() {
 #ifdef __x86_64__
   const size_t kLog2Size = 39;
-#elif defined(__mips64) || defined(__aarch64__) || defined(__loongarch_lp64)
+#elif defined(__mips64) || defined(__aarch64__) || \
+defined(__loongarch_lp64) || (defined(__riscv) && __riscv_xlen == 64)
   const size_t kLog2Size = 32;
 #elif defined(__powerpc64__)
   const size_t kLog2Size = 39;
Index: compiler-rt/test/tsan/map32bit.cpp
===
--- compiler-rt/test/tsan/map32bit.cpp
+++ compiler-rt/test/tsan/map32bit.cpp
@@ -13,6 +13,7 @@
 // XFAIL: target=powerpc64{{.*}}
 // XFAIL: target=s390x{{.*}}
 // XFAIL: target=loongarch64{{.*}}
+// XFAIL: target=riscv64{{.*}}
 
 // MAP_32BIT doesn't exist on OS X and NetBSD.
 // UNSUPPORTED: darwin,target={{.*netbsd.*}}
Index: compiler-rt/lib/tsan/tests/CMakeLists.txt
===
--- compiler-rt/lib/tsan/tests/CMakeLists.txt
+++ compiler-rt/lib/tsan/tests/CMakeLists.txt
@@ -55,6 +55,9 @@
 else()
   list(APPEND TSAN_UNITTEST_LINK_FLAGS -fsanitize=thread)
   list(APPEND TSAN_UNITTEST_LINK_FLAGS -lm)
+  if("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "riscv")
+list(APPEND TSAN_UNITTEST_LINK_FLAGS -latomic)
+  endif()
   list(APPEND TSAN_UNITTEST_LINK_FLAGS ${COMPILER_RT_TEST_LIBDISPATCH_CFLAGS})
 endif()
 
Index: compiler-rt/lib/tsan/rtl/tsan_rtl_riscv64.S
===
--- /dev/null
+++ compiler-rt/lib/tsan/rtl/tsan_rtl_riscv64.S
@@ -0,0 +1,203 @@
+#include "sanitizer_common/sanitizer_asm.h"
+
+.section .text
+
+.comm _ZN14__interception11real_setjmpE,8,8
+.globl ASM_SYMBOL_INTERCEPTOR(setjmp)
+ASM_TYPE_FUNCTION(ASM_SYMBOL_INTERCEPTOR(setjmp))
+ASM_SYMBOL_INTERCEPTOR(setjmp):
+  CFI_STARTPROC
+
+  // Save frame pointer and return address register
+  addi sp, sp, -32
+  sd ra, 24(sp)
+  sd s0, 16(sp)
+  CFI_DEF_CFA_OFFSET (32)
+  CFI_OFFSET (1, -8)
+  CFI_OFFSET (8, -16)
+
+  // Adjust the SP for previous frame
+  addi s0, sp, 32
+  CFI_DEF_CFA_REGISTER (8)

[PATCH] D129507: [OffloadPackager] Add option to extract files from images

[Alexey Bader via Phabricator via cfe-commits]

bader added inline comments.



Comment at: clang/test/Driver/offload-packager.c:2-3
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+// UNSUPPORTED: system-windows

Are nvptx and amdgpu target required for this test?
Latest version of the test invokes clang only for x86 target and 
clang-offload-packager just adds triple as metadata string without using llvm 
target. Right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129507

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

[PATCH] D143328: [analyzer] Remove the loop from the exploded graph caused by missing information in program points

[Domján Dániel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd65379c8d476: [analyzer] Remove the loop from the exploded 
graph caused by missing… (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143328

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
  clang/test/Analysis/PR60412.cpp
  clang/test/Analysis/analysis-after-multiple-dtors.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -89,11 +89,14 @@
 
 CallEventManager  = Eng.getStateManager().getCallEventManager();
 CallEventRef<> Call = [=, ]() -> CallEventRef {
+  CFGBlock::ConstCFGElementRef ElemRef = {SFC->getCallSiteBlock(),
+  SFC->getIndex()};
   if (std::is_base_of::value)
-return CEMgr.getCall(E, State, SFC);
+return CEMgr.getCall(E, State, SFC, ElemRef);
   if (std::is_same::value)
 return CEMgr.getCXXConstructorCall(cast(E),
-   /*Target=*/nullptr, State, SFC);
+   /*Target=*/nullptr, State, SFC,
+   ElemRef);
   llvm_unreachable("Only these expressions are supported for now.");
 }();
 
Index: clang/test/Analysis/analysis-after-multiple-dtors.cpp
===
--- /dev/null
+++ clang/test/Analysis/analysis-after-multiple-dtors.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct Test {
+  Test() {}
+  ~Test();
+};
+
+int foo() {
+  struct a {
+  // The dtor invocation of 'b' and 'c' used to create
+  // a loop in the egraph and the analysis stopped after
+  // this point.
+Test b, c;
+  } d;
+  return 1;
+}
+
+int main() {
+  if (foo()) {
+  }
+
+  int x;
+  int y = x;
+  // expected-warning@-1{{Assigned value is garbage or undefined}}
+  (void)y;
+}
Index: clang/test/Analysis/PR60412.cpp
===
--- /dev/null
+++ clang/test/Analysis/PR60412.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.deadcode.UnreachableCode -verify %s
+// expected-no-diagnostics
+
+struct Test {
+  Test() {}
+  ~Test();
+};
+
+int foo() {
+  struct a {
+Test b, c;
+  } d;
+  return 1;
+}
+
+int main() {
+  if (foo()) return 1; // <- this used to warn as unreachable
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -148,8 +148,8 @@
   ExplodedNode *Pred,
   ExplodedNodeSet ) {
   CallEventManager  = getStateManager().getCallEventManager();
-  CallEventRef Msg =
-CEMgr.getObjCMethodCall(ME, Pred->getState(), Pred->getLocationContext());
+  CallEventRef Msg = CEMgr.getObjCMethodCall(
+  ME, Pred->getState(), Pred->getLocationContext(), getCFGElementRef());
 
   // There are three cases for the receiver:
   //   (1) it is definitely nil,
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -610,8 +610,8 @@
   // Get the call in its initial state. We use this as a template to perform
   // all the checks.
   CallEventManager  = getStateManager().getCallEventManager();
-  CallEventRef<> CallTemplate
-= CEMgr.getSimpleCall(CE, Pred->getState(), Pred->getLocationContext());
+  CallEventRef<> CallTemplate = CEMgr.getSimpleCall(
+  CE, Pred->getState(), Pred->getLocationContext(), getCFGElementRef());
 
   // Evaluate the function call.  We try each of the checkers
   // to see if the can evaluate the function call.
@@ -837,7 +837,8 @@
   State = bindReturnValue(Call, 

[clang] d65379c - [analyzer] Remove the loop from the exploded graph caused by missing information in program points

[via cfe-commits]

Author: isuckatcs
Date: 2023-03-04T02:01:45+01:00
New Revision: d65379c8d4768ddae143672f5d4827acafe22553

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

LOG: [analyzer] Remove the loop from the exploded graph caused by missing 
information in program points

This patch adds CFGElementRef to ProgramPoints
and helps the analyzer to differentiate between
two otherwise identically looking ProgramPoints.

Fixes #60412

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

Added: 
clang/test/Analysis/PR60412.cpp
clang/test/Analysis/analysis-after-multiple-dtors.cpp

Modified: 
clang/include/clang/Analysis/ProgramPoint.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/ProgramPoint.h 
b/clang/include/clang/Analysis/ProgramPoint.h
index 6dba0582c8ddb..b9339570e1ae7 100644
--- a/clang/include/clang/Analysis/ProgramPoint.h
+++ b/clang/include/clang/Analysis/ProgramPoint.h
@@ -95,35 +95,33 @@ class ProgramPoint {
 
   llvm::PointerIntPair Tag;
 
+  CFGBlock::ConstCFGElementRef ElemRef = {nullptr, 0};
+
 protected:
   ProgramPoint() = default;
-  ProgramPoint(const void *P,
-   Kind k,
-   const LocationContext *l,
-   const ProgramPointTag *tag = nullptr)
-: Data1(P),
-  Data2(nullptr, (((unsigned) k) >> 0) & 0x3),
-  L(l, (((unsigned) k) >> 2) & 0x3),
-  Tag(tag, (((unsigned) k) >> 4) & 0x3) {
-assert(getKind() == k);
-assert(getLocationContext() == l);
-assert(getData1() == P);
-  }
-
-  ProgramPoint(const void *P1,
-   const void *P2,
-   Kind k,
-   const LocationContext *l,
-   const ProgramPointTag *tag = nullptr)
-: Data1(P1),
-  Data2(P2, (((unsigned) k) >> 0) & 0x3),
-  L(l, (((unsigned) k) >> 2) & 0x3),
-  Tag(tag, (((unsigned) k) >> 4) & 0x3) {}
+  ProgramPoint(const void *P, Kind k, const LocationContext *l,
+   const ProgramPointTag *tag = nullptr,
+   CFGBlock::ConstCFGElementRef ElemRef = {nullptr, 0})
+  : Data1(P), Data2(nullptr, (((unsigned)k) >> 0) & 0x3),
+L(l, (((unsigned)k) >> 2) & 0x3), Tag(tag, (((unsigned)k) >> 4) & 0x3),
+ElemRef(ElemRef) {
+assert(getKind() == k);
+assert(getLocationContext() == l);
+assert(getData1() == P);
+  }
+
+  ProgramPoint(const void *P1, const void *P2, Kind k, const LocationContext 
*l,
+   const ProgramPointTag *tag = nullptr,
+   CFGBlock::ConstCFGElementRef ElemRef = {nullptr, 0})
+  : Data1(P1), Data2(P2, (((unsigned)k) >> 0) & 0x3),
+L(l, (((unsigned)k) >> 2) & 0x3), Tag(tag, (((unsigned)k) >> 4) & 0x3),
+ElemRef(ElemRef) {}
 
 protected:
   const void *getData1() const { return Data1; }
   const void *getData2() const { return Data2.getPointer(); }
   void setData2(const void *d) { Data2.setPointer(d); }
+  CFGBlock::ConstCFGElementRef getElementRef() const { return ElemRef; }
 
 public:
   /// Create a new ProgramPoint object that is the same as the original
@@ -190,17 +188,13 @@ class ProgramPoint {
   }
 
   bool operator==(const ProgramPoint & RHS) const {
-return Data1 == RHS.Data1 &&
-   Data2 == RHS.Data2 &&
-   L == RHS.L &&
-   Tag == RHS.Tag;
+return Data1 == RHS.Data1 && Data2 == RHS.Data2 && L == RHS.L &&
+   Tag == RHS.Tag && ElemRef == RHS.ElemRef;
   }
 
   bool operator!=(const ProgramPoint ) const {
-return Data1 != RHS.Data1 ||
-   Data2 != RHS.Data2 ||
-   L != RHS.L ||
-   Tag != RHS.Tag;
+return Data1 != RHS.Data1 || Data2 != RHS.Data2 || L != RHS.L ||
+   Tag != RHS.Tag || ElemRef != RHS.ElemRef;
   }
 
   void Profile(llvm::FoldingSetNodeID& ID) const {
@@ -209,6 +203,8 @@ class ProgramPoint {
 ID.AddPointer(getData2());
 ID.AddPointer(getLocationContext());
 ID.AddPointer(getTag());
+ID.AddPointer(ElemRef.getParent());
+ID.AddInteger(ElemRef.getIndexInBlock());
   }
 
   void printJson(llvm::raw_ostream , const char *NL = "\n") const;
@@ -266,6 +262,7 @@ class BlockExit : public ProgramPoint {
   }
 };
 
+// FIXME: Eventually we want to take a CFGElementRef 

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 502308.
cjdb added a comment.

tidies up some stuff that I overlooked


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145284

Files:
  clang/include/clang/Basic/DiagnosticOptions.def
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -230,7 +230,12 @@
   CompilerInvocation::GetResourcesPath(Argv0, MainAddr);
 
   // Create the actual diagnostics engine.
-  Clang->createDiagnostics();
+  auto diagnostics_format_option =
+  llvm::find(Argv, StringRef("-fdiagnostics-format"));
+  bool IsSarifFile =
+  std::distance(diagnostics_format_option, Argv.end()) > 1 &&
+  *std::next(diagnostics_format_option) == StringRef("sarif-file");
+  Clang->createDiagnostics(nullptr, true, IsSarifFile);
   if (!Clang->hasDiagnostics())
 return 1;
 
Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- clang/test/Driver/fdiagnostics-format-sarif.cpp
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-stderr %s -### 2>&1 | FileCheck %s --check-prefix=WARN
 // WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
 
-// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-file %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
 // WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -425,7 +425,9 @@
   std::unique_ptr ScopedToolAction(create());
 
   // Create the compiler's actual diagnostics engine.
-  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false,
+ Invocation->DiagnosticOpts->getFormat() ==
+ DiagnosticOptions::SARIFFile);
   if (!Compiler.hasDiagnostics())
 return false;
 
@@ -684,7 +686,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,7 +815,8 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -838,7 +839,8 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
===
--- clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -22,19 +22,37 @@
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 
 namespace clang {
 
 SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(raw_ostream ,
DiagnosticOptions *Diags)
-: OS(OS), DiagOpts(Diags) {}
+: OS(), DiagOpts(Diags) {}
+
+static std::unique_ptr
+OpenDiagnosticFile(StringRef TargetPath, std::error_code ) {
+  assert(!TargetPath.empty());
+  return std::make_unique(
+  std::string(TargetPath) + ".sarif", EC);
+}
+
+SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(StringRef TargetPath,
+   DiagnosticOptions *Diags)
+: EC(std::error_code()), OS(OpenDiagnosticFile(TargetPath, *EC)),
+  

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 502305.
cjdb edited the summary of this revision.
cjdb added a comment.

adds dependency


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145284

Files:
  clang/include/clang/Basic/DiagnosticOptions.def
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -230,7 +230,12 @@
   CompilerInvocation::GetResourcesPath(Argv0, MainAddr);
 
   // Create the actual diagnostics engine.
-  Clang->createDiagnostics();
+  auto diagnostics_format_option =
+  llvm::find(Argv, StringRef("-fdiagnostics-format"));
+  bool IsSarifFile =
+  std::distance(diagnostics_format_option, Argv.end()) > 1 &&
+  *std::next(diagnostics_format_option) == StringRef("sarif-file");
+  Clang->createDiagnostics(nullptr, true, IsSarifFile);
   if (!Clang->hasDiagnostics())
 return 1;
 
Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- clang/test/Driver/fdiagnostics-format-sarif.cpp
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-stderr %s -### 2>&1 | FileCheck %s --check-prefix=WARN
 // WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
 
-// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-file %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
 // WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -425,7 +425,9 @@
   std::unique_ptr ScopedToolAction(create());
 
   // Create the compiler's actual diagnostics engine.
-  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false,
+ Invocation->DiagnosticOpts->getFormat() ==
+ DiagnosticOptions::SARIFFile);
   if (!Compiler.hasDiagnostics())
 return false;
 
@@ -684,7 +686,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,7 +815,8 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -838,7 +839,8 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
===
--- clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -22,19 +22,37 @@
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 
 namespace clang {
 
 SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(raw_ostream ,
DiagnosticOptions *Diags)
-: OS(OS), DiagOpts(Diags) {}
+: OS(), DiagOpts(Diags) {}
+
+static std::unique_ptr
+OpenDiagnosticFile(StringRef TargetPath, std::error_code ) {
+  assert(!TargetPath.empty());
+  return std::make_unique(
+  std::string(TargetPath) + ".sarif", EC);
+}
+
+SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(StringRef TargetPath,
+   DiagnosticOptions *Diags)
+: EC(std::error_code()), 

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie.
Herald added a project: All.
cjdb requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

The original `-fdiagnostics-format=sarif` wrote directly to standard
error. This commit renames that mode to `sarif-stderr` and introduces a
`sarif-file` mode that allows the compiler to write `.sarif` files to
the current working directory. It also removes `SARIF` as an option so
that there's a single canonical approach for each option.

IMPORTANT: There are two different designs for writing SARIF to file,
so I've pulled the common parts of the design out into their own patch.
This ensures it only gets reviewed once, and so that the divergence is
apparent in the other two patches. Once a design is settled on, this
patch will be joined with the selected design.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145284

Files:
  clang/include/clang/Basic/DiagnosticOptions.def
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CompilerInstance.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -230,7 +230,12 @@
   CompilerInvocation::GetResourcesPath(Argv0, MainAddr);
 
   // Create the actual diagnostics engine.
-  Clang->createDiagnostics();
+  auto diagnostics_format_option =
+  llvm::find(Argv, StringRef("-fdiagnostics-format"));
+  bool IsSarifFile =
+  std::distance(diagnostics_format_option, Argv.end()) > 1 &&
+  *std::next(diagnostics_format_option) == StringRef("sarif-file");
+  Clang->createDiagnostics(nullptr, true, IsSarifFile);
   if (!Clang->hasDiagnostics())
 return 1;
 
Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- clang/test/Driver/fdiagnostics-format-sarif.cpp
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-stderr %s -### 2>&1 | FileCheck %s --check-prefix=WARN
 // WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
 
-// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-file %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
 // WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -425,7 +425,9 @@
   std::unique_ptr ScopedToolAction(create());
 
   // Create the compiler's actual diagnostics engine.
-  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+  Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false,
+ Invocation->DiagnosticOpts->getFormat() ==
+ DiagnosticOptions::SARIFFile);
   if (!Compiler.hasDiagnostics())
 return false;
 
@@ -684,7 +686,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,7 +815,8 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -838,7 +839,8 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
-  case DiagnosticOptions::SARIF:
+  case DiagnosticOptions::SARIFStderr:
+  case DiagnosticOptions::SARIFFile:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
===
--- clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -22,19 +22,37 @@
 

[PATCH] D145093: Add map info for dereference pointer.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

jyu2 wrote:
> ABataev wrote:
> > What if we have something like:
> > ```
> > typedef int (T)[3];
> > 
> > T** p;
> > map(**p)
> > ```
> > ? Shall it work? I.e. mapping of the whole array by dereferening the 
> > pointer to the array.
> No, it is not work currently.  What about *(*(p+a)+b)...
My question - shall it work? Mapping  (**a)[:3] should result in the same in 
the same code for **a if a is
```
typedef int(T)[3];
T** a;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D145093: Add map info for dereference pointer.

[Jennifer Yu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

ABataev wrote:
> What if we have something like:
> ```
> typedef int (T)[3];
> 
> T** p;
> map(**p)
> ```
> ? Shall it work? I.e. mapping of the whole array by dereferening the pointer 
> to the array.
No, it is not work currently.  What about *(*(p+a)+b)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> the programmer is indicating the function has some special purpose, and is 
> not expected to be garbage collected.

There are use cases that GC semantics are desired. They may place the address 
function in a variable.
I don't think changing the behavior by default is fine. Using an option may be 
fine, but it'd be nicer that GCC commits to do this as well.


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

https://reviews.llvm.org/D145173

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

[PATCH] D145238: [NVPTX] Expose LDU builtins

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Nice. Thank you!




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18104
 CodeGenFunction::EmitNVPTXBuiltinExpr(unsigned BuiltinID, const CallExpr *E) {
-  auto MakeLdg = [&](unsigned IntrinsicID) {
+  auto HasHalfSupport = [&](unsigned BuiltinID) {
+auto  = getContext();

I'd add a comment describing a meaning of the fields in the returned pair.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18116
+case NVPTX::BI__nvvm_ldu_h:
+  BuiltinName = "__nvvm_ldu_h";
+  break;

Can we use the standard `StringRef Name = 
getContext().BuiltinInfo.getName(BuiltinID);` to figure out the builtin name?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18261-18263
+  std::string ErrMsg{HalfSupport.second};
+  CGM.Error(E->getExprLoc(),
+ErrMsg.append(" requires native half type support.").c_str());

Nit: this would be a bit more readable:
```
std::string BuiltinName{HalfSupport.second};
CGM.Error(E->getExprLoc(),  BuiltinName + " requires native half type 
support.")` 
```
You may also consider changing returned `BuiltinName` to be `std::string`, so 
you would not need an explicit temp var. 



Comment at: clang/test/CodeGen/builtins-nvptx-native-half-type-err.c:3
+//
+// RUN: not %clang_cc1 -DLDG -fsyntax-only -ffp-contract=off -triple 
nvptx-unknown-unknown -target-cpu \
+// RUN:   sm_75 -target-feature +ptx70 -fcuda-is-device -x cuda -emit-llvm -o 
- %s 2>&1 \

I think we could've done it all in one run if we were to do both ldg and ldu in 
one function.



Comment at: llvm/test/CodeGen/NVPTX/ldu-ldg.ll:33
+; ldu.global.u16
+  %val = tail call i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, 
i32 4)
+  ret i16 %val

Should alignment be 2 ?



Comment at: llvm/test/CodeGen/NVPTX/ldu-ldg.ll:60
+define double @test_ldu_f64(ptr addrspace(1) %ptr) {
+; ldu.global.u64
+  %val = tail call double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) 
%ptr, i32 8)

Hmm. I wonder why we end up with `u64` here and not `b64`. Not that it matters 
in this case, but it is a discrepancy vs. `f16`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

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

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Julian Lettner via Phabricator via cfe-commits]

yln accepted this revision.
yln added inline comments.



Comment at: 
compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 

usama54321 wrote:
> yln wrote:
> > usama54321 wrote:
> > > yln wrote:
> > > > zixuw wrote:
> > > > > dmaclach wrote:
> > > > > > yln wrote:
> > > > > > > This should work, right?
> > > > > > No.. darwin should fail with the `-static-libsan` flag. This is the 
> > > > > > test that was failing and caused the rollback.
> > > > > I think @yln is suggesting using `REQUIRES: asan-static-runtime` 
> > > > > instead of `XFAIL: darwin`. I wasn't aware of that conditional but 
> > > > > yeah that should be better if it works.
> > > > I meant using `// REQUIRES: asan-static-runtime ` instead of `XFAIL: 
> > > > darwin` since it seems that we already have a lit feature for it.
> > > I think UNSUPPORTED: darwin makes the most sense here. I don't think lit 
> > > understands that REQUIRES: asan-static-runtime should result in skipping 
> > > the test on Darwin as it does not know about this dependency.
> > > I don't think lit understands that REQUIRES: asan-static-runtime should 
> > > result in skipping the test on Darwin as it does not know about this 
> > > dependency.
> > 
> > Actually, this was exactly my point.  We have other tests already marked 
> > with `REQUIRES: asan-static-runtime` and we should double check our changes 
> > don't affect these as well.
> > 
> > If LIT doesn't model this dependency yet, then we should make sure it does! 
> >  And this test can act as a good "canary in the coal mine".
> > 
> > Please use `REQUIRES: asan-static-runtime` and make sure we understand and 
> > deal with any fallout.
> Sorry for my earlier comment. @yln is correct, and we should use REQUIRES: 
> asan-static-runtime. I double checked, and this already works as expected on 
> darwin, i.e. these tests are unsupported on darwin. So you should not need to 
> do anything apart from adding the REQUIRES in the test.
> [...] I double checked, and this already works as expected on darwin, i.e. 
> these tests are unsupported on darwin.

Thanks Usama!

Patch LGTM assuming we switch to using `REQUIRES: asan-static-runtime`.  Thanks 
for seeing this through! :)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

agozillon wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > This test looks correct to me, but please note that:
> > > 
> > > ```
> > > ! Check that flang -fc1 is invoked when in --driver-mode=flang 
> > > ```
> > > 
> > > yet (`%clang` instead of `%flang`)
> > > 
> > > ```
> > > ! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
> > > --check-prefixes=CHECK-OPENMP %s
> > > ```
> > > 
> > > I'm not really sure whether we should be testing Flang-specific logic in 
> > > Clang. Having said that, Flang does use `clangDriver` to implement its 
> > > driver :) 
> > > 
> > > You could consider using 
> > > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
> > >  instead (or add an OpenMP specific file there).
> > > 
> > > Not a blocker.
> > Yes, I wasn't so sure either as it felt a little weird to test Flang 
> > components inside of Clang, but as you said it is a Clang toolchain (that 
> > this test is checking) and borrows from the clangDriver! 
> > 
> > I borrowed this test from other similar tests in the same folder that test 
> > other flang specific driver logic in a similar manner, but I am more than 
> > happy to add an additional flang specific driver test as you mention! 
> On further looking into the frontend-forwarding test, I don't know if it is 
> suitable for fopenmp-is-device as it is an fc1 option and won't be forwarded 
> from the flang-new frontend down to fc1 at the moment! 
> 
> I think this test will be more suitable when additional flags like the 
> fopenmp-targets (or similar flags) that are used in this test are added to 
> the Flang driver. As they spawn/propagate the openmp-is-device flag. However, 
> perhaps I am incorrect.
> On further looking into the frontend-forwarding test, I don't know if it is 
> suitable for fopenmp-is-device as it is an fc1 option and won't be forwarded 
> from the flang-new frontend down to fc1 at the moment!

It should be - that's what the logic in Flang.cpp does, no? And if it doesn't, 
is it a deliberate design decision?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D143745: Make section attribute and -ffunction-sections play nicely

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

See D145173  for a different tactic to solve 
this.


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

https://reviews.llvm.org/D143745

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D145262#4168058 , @jaredgrubb 
wrote:

> I wasn't sure about testing (this is my first patch!) and the test-case I did 
> was in `clang/test/Format` .. I can look at `clang/unittests/Format` and see 
> how to model something like it.
>
> If I do that, would that be in-addition-to or instead-of the test-case I 
> included already?

Instead of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145262

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

[PATCH] D145271: [MSVC compatibility][DLLEXPORT/DLLIMPORT] Allow dllexport/dllimport for local classes

[Paul Robinson via Phabricator via cfe-commits]

probinson added a subscriber: cfe-commits.
probinson added a project: clang.
probinson added a comment.

I've looked at this but I'd like someone more in tune with MSVC behavior to 
review as well.


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

https://reviews.llvm.org/D145271

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

Would be nice to have feedback on this! So much work was put into it! @rnk, 
@efriedma


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

https://reviews.llvm.org/D137107

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

[PATCH] D145077: [clang][DebugInfo] Support DW_AT_LLVM_preferred_name attribute

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D145077#4168209 , @dblaikie wrote:

> In D145077#4162110 , @aprantl wrote:
>
>> I originally was hoping we wouldn't have to introduce a new attribute for 
>> this, but it looks like there are legitimate concerns about the 
>> alternatives, so in that sense this looks good!
>
> Out of curiosity, what were the other existing attributes that were 
> considered? (would be curious to have them written down here to explain/add 
> strength to the motivation to add a new extension attribute)

Oh, I guess this discussion might go better in the llvm side that actually 
introduces the attribute D145076 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145077

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

[PATCH] D145077: [clang][DebugInfo] Support DW_AT_LLVM_preferred_name attribute

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D145077#4162110 , @aprantl wrote:

> I originally was hoping we wouldn't have to introduce a new attribute for 
> this, but it looks like there are legitimate concerns about the alternatives, 
> so in that sense this looks good!

Out of curiosity, what were the other existing attributes that were considered? 
(would be curious to have them written down here to explain/add strength to the 
motivation to add a new extension attribute)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145077

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

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

[Joshua Batista via Phabricator via cfe-commits]

bob80905 created this revision.
bob80905 added reviewers: python3kgae, beanz, fhahn.
Herald added subscribers: luke, Anastasia, StephenFan, frasercrmck, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb.
Herald added a project: All.
bob80905 requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead.
Herald added a project: clang.

Add codegen for llvm exp/exp2 elementwise builtin
The exp/exp2 elementwise builtins are necessary for HLSL codegen.
Tests were added to make sure that the expected errors are encountered when 
these functions are given inputs of incompatible types.
The new builtins are restricted to floating point types only.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145270

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/aarch64-sve-vector-exp-ops.c
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/Sema/riscv-sve-vector-exp-ops.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -67,6 +67,20 @@
   static_assert(!is_const::value);
 }
 
+void test_builtin_elementwise_exp() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
+
+void test_builtin_elementwise_exp2() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
+
 void test_builtin_elementwise_sin() {
   const float a = 42.0;
   float b = 42.3;
Index: clang/test/Sema/riscv-sve-vector-exp-ops.c
===
--- /dev/null
+++ clang/test/Sema/riscv-sve-vector-exp-ops.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +v -target-feature +zfh -target-feature +experimental-zvfh \
+// RUN:   -disable-O0-optnone -o - -fsyntax-only %s -verify 
+// REQUIRES: riscv-registered-target
+
+#include 
+
+
+vfloat32mf2_t test_exp_vv_i8mf8(vfloat32mf2_t v) {
+
+  return __builtin_elementwise_exp(v);
+  // expected-error@-1 {{1st argument must be a vector, integer or floating point type}}
+}
+
+vfloat32mf2_t test_exp2_vv_i8mf8(vfloat32mf2_t v) {
+
+  return __builtin_elementwise_exp2(v);
+  // expected-error@-1 {{1st argument must be a vector, integer or floating point type}}
+}
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -311,6 +311,49 @@
   // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
 }
 
+void test_builtin_elementwise_exp(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_exp(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_exp();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_exp(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_exp(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_exp(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_exp(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
+
+void test_builtin_elementwise_exp2(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_exp2(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_exp2();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_exp2(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_exp2(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_exp2(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_exp2(uv);
+  // expected-error@-1 {{1st 

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5e035651fd3a: [clang/Diagnostic] Use `optional` to 
disambiguate between a `StoredDiagMessage`… (authored by akyrtzi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145256

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/Basic/Diagnostic.cpp
  clang/unittests/Basic/DiagnosticTest.cpp


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,26 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,26 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}

[clang] 5e03565 - [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Argyrios Kyrtzidis via cfe-commits]

Author: Argyrios Kyrtzidis
Date: 2023-03-03T12:48:48-08:00
New Revision: 5e035651fd3acbb2645abbe80cae332d90eac78a

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

LOG: [clang/Diagnostic] Use `optional` to disambiguate between a 
`StoredDiagMessage` that is not set vs set as empty string

"But when would you have a completely empty diagnostic message", you ask dear 
reader?
That is when there is an empty "#warning" in code.

rdar://106155415

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

Added: 


Modified: 
clang/include/clang/Basic/Diagnostic.h
clang/lib/Basic/Diagnostic.cpp
clang/unittests/Basic/DiagnosticTest.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index b9ba459d1358b..5606a22fe9d68 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@ inline DiagnosticBuilder 
DiagnosticsEngine::Report(unsigned DiagID) {
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}

diff  --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index dbe62ecb50d33..3474acbced195 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@ static const char *getTokenDescForDiagnostic(tok::TokenKind 
Kind) {
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 

diff  --git a/clang/unittests/Basic/DiagnosticTest.cpp 
b/clang/unittests/Basic/DiagnosticTest.cpp
index f4ce7e187f8f2..7469019391716 100644
--- a/clang/unittests/Basic/DiagnosticTest.cpp
+++ b/clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,26 @@ TEST(DiagnosticTest, diagnosticError) {
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }



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

[PATCH] D145269: Dump functions attribute right before their return type

[Giuliano Belinassi via Phabricator via cfe-commits]

giulianobelinassi created this revision.
Herald added a project: All.
giulianobelinassi requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

GCC rejects the following kind of dumps:

int f(void) __attribute__((unused))
{

  return 0;

}

because of the following reason: Assume the function is K declared:

int f(i) __attribute__((unused))

  int i;

{

  return 0;

}

Now, to which symbol should the __attribute__((unused)) be applied?
Depending of the parsing strategy, this will be either applied to
f or i, but likely this will be applied to f as the compiler parses
from left-to-right.

GCC, therefore, rejects this kind of declarations altogether. Clang
even warns that such declarations won't be accepted by GCC. Since
an user may be using clang dumps as a library to do static analysis
and outputing code that will be fed into GCC, then instead of dumping:

int f(void) __attribute__((unused));

this patch modifies such cases to output to:

int __attribute__((unused)) f(void);

causing GCC to also accept such dumps.

Signed-off-by: Giuliano Belinassi 

Depends on: D141714 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145269

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/test/AST/ast-print-attr-knr.c
  clang/test/AST/ast-print-attr.c
  clang/test/AST/ast-print-no-sanitize.cpp
  clang/test/AST/attr-print-emit.cpp
  clang/test/Analysis/std-c-library-functions-arg-weakdeps.c
  clang/test/OpenMP/assumes_codegen.cpp
  clang/test/OpenMP/assumes_print.cpp
  clang/test/OpenMP/assumes_template_print.cpp
  clang/test/OpenMP/declare_simd_ast_print.c
  clang/test/OpenMP/declare_simd_ast_print.cpp
  clang/test/Sema/attr-print.c
  clang/test/SemaCXX/attr-no-sanitize.cpp
  clang/test/SemaCXX/attr-print.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp

Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-attr-print.cpp
@@ -30,16 +30,16 @@
 // CHECK: int c11_alignas _Alignas(alignof(int));
 _Alignas(int) int c11_alignas;
 
-// CHECK: void foo() __attribute__((const));
+// CHECK: void __attribute__((const)) foo();
 void foo() __attribute__((const));
 
-// CHECK: void bar() __attribute__((__const));
-void bar() __attribute__((__const));
+// CHECK: void __attribute__((__const)) bar();
+void __attribute__((__const)) bar();
 
 // FIXME: It's unfortunate that the string literal prints with the below three
 // cases given that the string is only exposed via the [[nodiscard]] spelling.
-// CHECK: int f1() __attribute__((warn_unused_result("")));
-int f1() __attribute__((warn_unused_result));
+// CHECK: int __attribute__((warn_unused_result(""))) f1();
+int __attribute__((warn_unused_result)) f1();
 
 // CHECK: {{\[}}[clang::warn_unused_result("")]];
 int f2 [[clang::warn_unused_result]] ();
@@ -52,24 +52,24 @@
 // CHECK: {{\[}}[noreturn]];
 void f4 [[noreturn]] ();
 
-// CHECK: __attribute__((gnu_inline));
-inline void f6() __attribute__((gnu_inline));
+// CHECK: void __attribute__((gnu_inline))
+inline void __attribute__((gnu_inline)) f6();
 
 // CHECK: {{\[}}[gnu::gnu_inline]];
 inline void f7 [[gnu::gnu_inline]] ();
 
 // arguments printing
-// CHECK: __attribute__((format(printf, 2, 3)));
-void f8 (void *, const char *, ...) __attribute__ ((format (printf, 2, 3)));
+// CHECK: void __attribute__((format(printf, 2, 3)))
+void __attribute__ ((format (printf, 2, 3))) f8 (void *, const char *, ...);
 
 // CHECK: int m __attribute__((aligned(4)))
 // CHECK: int n alignas(4
-// CHECK: static int f() __attribute__((pure))
+// CHECK: static int __attribute__((pure)) f()
 // CHECK: static int g() {{\[}}[gnu::pure]]
 template  struct S {
   int __attribute__((aligned(4))) m;
   alignas(4) int n;
-  __attribute__((pure)) static int f() {
+  static int __attribute__((pure)) f() {
 return 0;
   }
   [[gnu::pure]] static int g() {
@@ -79,7 +79,7 @@
 
 // CHECK: int m __attribute__((aligned(4)))
 // CHECK: int n alignas(4
-// CHECK: static int f() __attribute__((pure))
+// CHECK: static int __attribute__((pure)) f()
 // CHECK: static int g() {{\[}}[gnu::pure]]
 template struct S;
 
Index: clang/test/SemaCXX/attr-print.cpp
===
--- clang/test/SemaCXX/attr-print.cpp
+++ clang/test/SemaCXX/attr-print.cpp
@@ -7,14 +7,14 @@
 // CHECK: int __declspec(align(4)) y;
 int __declspec(align(4)) y;
 
-// CHECK: void foo() __attribute__((const));
+// CHECK: void __attribute__((const)) foo();
 void foo() __attribute__((const));
 
-// CHECK: void bar() __attribute__((__const));
+// CHECK: void __attribute__((__const)) bar();
 void bar() __attribute__((__const));
 
 // FIXME: Print this with correct format.
-// CHECK: void foo1() __attribute__((noinline)) __attribute__((pure));
+// CHECK: void 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb added a comment.

I wasn't sure about testing (this is my first patch!) and the test-case I did 
was in `clang/test/Format` .. I can look at `clang/unittests/Format` and see 
how to model something like it.

If I do that, would that be in-addition-to or instead-of the test-case I 
included already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb added a comment.

For background, the current clang-format results in the following 
(`-style="{BasedOnStyle: LLVM, ColumnLimit: 0, AttributeMacros: [MACRO]}`):

  MACRO MACRO(A)
  @interface Foo
  @end
  
  MACRO(A)
  MACRO
  @interface Foo
  @end

This patch improves it (removes the indention and makes both cases have same 
wrapping):

  MACRO MACRO(A)
  @interface Foo
  @end
  
  MACRO(A) MACRO
  @interface Foo
  @end


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145262

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

[PATCH] D145264: [OpenMP][MLIR] Lower and apply RTLModuleFlagsAttr

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

This builds on the following phabricator patches that are in review at the 
moment, so there are components that are used here that are added from these:

https://reviews.llvm.org/D144864 : add fopenmp-is-device and an mlir is-device 
attribute for the OpenMP dialect
https://reviews.llvm.org/D144896 : the RTLModuleFlagsAttr that is in use within 
this patch for lowering and application
https://reviews.llvm.org/D144883 : an infrastructure addition for the MLIR LLVM 
translation interface

For the moment, this patch is primarily to give context to the usage of the 
RTLModuleFlagsAttr mlir attribute and give some further understanding for a 
reviewer, however, please feel free to review it if you have time as I will 
eventually ask for some revision of it after all the components it depends on 
have been progressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145264

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

And please add an entry to the `ReleaseNotes.rst`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D145264: [OpenMP][MLIR] Lower and apply RTLModuleFlagsAttr

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon created this revision.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, awarzynski, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, thopre, guansong, yaxunl.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
agozillon requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, stephenneuendorffer, 
nicolasvasilache, jdoerfert, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: nicolasvasilache.
Herald added projects: clang, MLIR.

This patch adds the lowering for the RTLModuleFlagsAttr to LLVM-IR,
which lowers each individual arguent of the attribute to globals. AS well
as the application of it by flags in the frontend which have been activated
for flang fc1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145264

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/rtl-flags.f90
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1351,6 +1351,40 @@
   return success();
 }
 
+/// Lowers the RTLModuleFlagsAttr which is applied to the module on the device
+/// pass when offloading, this attribute contains OpenMP RTL globals that can
+/// be passed as flags to the frontend, otherwise they are set to default
+LogicalResult
+convertRTLModuleFlagsAttr(Operation *op,
+  mlir::omp::RTLModuleFlagsAttr attribute,
+  LLVM::ModuleTranslation ) {
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+
+  ompBuilder->createGlobalFlag(
+  attribute.getDebugKind() /*LangOpts().OpenMPTargetDebug*/,
+  "__omp_rtl_debug_kind");
+  ompBuilder->createGlobalFlag(
+  attribute
+  .getAssumeTeamsOversubscription() /*LangOpts().OpenMPTeamSubscription*/
+  ,
+  "__omp_rtl_assume_teams_oversubscription");
+  ompBuilder->createGlobalFlag(
+  attribute
+  .getAssumeThreadsOversubscription() /*LangOpts().OpenMPThreadSubscription*/
+  ,
+  "__omp_rtl_assume_threads_oversubscription");
+  ompBuilder->createGlobalFlag(
+  attribute.getAssumeNoThreadState() /*LangOpts().OpenMPNoThreadState*/,
+  "__omp_rtl_assume_no_thread_state");
+  ompBuilder->createGlobalFlag(
+  attribute
+  .getAssumeNoNestedParallelism() /*LangOpts().OpenMPNoNestedParallelism*/
+  ,
+  "__omp_rtl_assume_no_nested_parallelism");
+
+  return success();
+}
+
 namespace {
 
 /// Implementation of the dialect interface that converts operations belonging
@@ -1365,10 +1399,34 @@
   LogicalResult
   convertOperation(Operation *op, llvm::IRBuilderBase ,
LLVM::ModuleTranslation ) const final;
+
+  LogicalResult
+  amendOperation(Operation *op, NamedAttribute attribute,
+ LLVM::ModuleTranslation ) const final;
 };
 
 } // namespace
 
+/// Given an OpenMP MLIR attribute, create the corresponding LLVM-IR, runtime
+/// calls, or operation amendments
+LogicalResult OpenMPDialectLLVMIRTranslationInterface::amendOperation(
+Operation *op, NamedAttribute attribute,
+LLVM::ModuleTranslation ) const {
+
+  return llvm::TypeSwitch(attribute.getValue())
+  .Case([&](mlir::omp::RTLModuleFlagsAttr rtlAttr) {
+return convertRTLModuleFlagsAttr(op, rtlAttr, moduleTranslation);
+  })
+  .Default([&](Attribute attr) {
+// fall through for omp attributes that do not require lowering and/or
+// have no concrete definition and thus no type to define a case on
+// e.g. omp.is_device
+return success();
+  });
+
+  return failure();
+}
+
 /// Given an OpenMP MLIR operation, create the corresponding LLVM IR
 /// (including OpenMP runtime calls).
 LogicalResult OpenMPDialectLLVMIRTranslationInterface::convertOperation(
Index: flang/test/Lower/OpenMP/rtl-flags.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/rtl-flags.f90
@@ -0,0 +1,76 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEFAULT-DEVICE-FIR
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=DEFAULT-HOST-FIR
+!RUN: %flang_fc1 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added reviewers: rymiel, owenpan, MyDeveloperDay.
HazardyKnusperkeks added a comment.

Our tests reside in `clang/unittests/Format`, you want to look into the 
`TokenAnnotatorTests`. In fact **I** can't read the tests you wrote there.

And could you please provide an example what you want to format and where the 
misformatting (or apparently misannotation) happens?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145262

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Please reupload with the complete context.

Please add tests.




Comment at: clang/include/clang/Format/Format.h:827
+  /// \code
+  ///   someFunction(
+  ///   int argument1,

That's not a valid declaration (missing return type), so not a good example.



Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+

Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)



Comment at: clang/include/clang/Format/Format.h:831
+  /// \endcode
+  bool AlwaysBreakBeforeFunctionParameters;
+

HazardyKnusperkeks wrote:
> Please sort alphabetically. (So above `AlwaysBreakBeforeMultilineStrings`.)
You are missing the \since 17.



Comment at: clang/lib/Format/Format.cpp:1510
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
+  GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;

Only set it in getLLVMStyle, which you are missing right now. Every other style 
will inherit it.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4740-4742
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren)) {
+if (Left.Previous) {

Merge the `if`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Please add unit tests to relevant files in `clang/unittests/Format/`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Jared Grubb via Phabricator via cfe-commits]

jaredgrubb created this revision.
jaredgrubb added reviewers: HazardyKnusperkeks, djasper, egorzhdan.
jaredgrubb added a project: clang-format.
Herald added a project: All.
jaredgrubb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I noticed that clang-format was inserting some strange indentation whenever I 
used custom "attribute-like macros"
(things like `FOO_EXTERN` to wrap attribute-visible-default, or macros with 
parentheses like `NS_SWIFT_NAME(...)`).

There are two parts to this fix:

- tokenize the paren after an `AttributeMacro` as a `TT_AttributeParen`
- treat a `AttributeMacro`-without-paren the same as one with a paren (eg, the 
`FOO_EXTERN` case)

I added a new test-case to differentiate a macro that is or is-not a 
`AttributeMacro`; also handled whether the
`ColumnLimit` is set to infinite (0) or a finite value, as part of this patch 
is in `ContinuationIndenter`.

There may be other places that need to handle `TT_AttributeMacro` better, but 
this is at least a step in the right
direction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145262

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/test/Format/objc-definitions.m

Index: clang/test/Format/objc-definitions.m
===
--- /dev/null
+++ clang/test/Format/objc-definitions.m
@@ -0,0 +1,58 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AttributeMacros: [MACRO]}" \
+// RUN:   | FileCheck -strict-whitespace %s --check-prefixes=CHECK-COMMON,CHECK-ATTRMACRO
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AttributeMacros: [MACRO], ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s --check-prefixes=CHECK-COMMON,CHECK-ATTRMACRO
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM}" \
+// RUN:   | FileCheck -strict-whitespace %s --check-prefixes=CHECK-COMMON,CHECK-PLAIN
+// RUN: grep -Ev "// *[A-Z-]+:" %s \
+// RUN:   | clang-format -style="{BasedOnStyle: LLVM, ColumnLimit: 0}" \
+// RUN:   | FileCheck -strict-whitespace %s --check-prefixes=CHECK-COMMON,CHECK-PLAIN-COL0
+
+// CHECK-COMMON: {{^@interface Foo$}}
+// CHECK-COMMON-NEXT: {{^@end$}}
+@interface Foo
+@end
+
+// CHECK-COMMON: {{^MACRO$}}
+// CHECK-COMMON-NEXT: {{^@interface Foo$}}
+// CHECK-COMMON-NEXT: {{^@end$}}
+MACRO
+@interface Foo
+@end
+
+// CHECK-COMMON: {{^MACRO\(A\)$}}
+// CHECK-COMMON-NEXT: {{^@interface Foo$}}
+// CHECK-COMMON-NEXT: {{^@end$}}
+MACRO(A)
+@interface Foo
+@end
+
+// CHECK-ATTRMACRO: {{^MACRO MACRO\(A\)$}}
+// CHECK-ATTRMACRO-NEXT: {{^@interface Foo$}}
+// CHECK-ATTRMACRO-NEXT: {{^@end$}}
+// CHECK-PLAIN: {{^MACRO MACRO\(A\) @interface Foo$}}
+// CHECK-PLAIN-NEXT: {{^@end$}}
+// COM: The leading indentation in the next case is existing behavior but probably not desired.
+// CHECK-PLAIN-COL0: {{^MACRO MACRO\(A\)$}}
+// CHECK-PLAIN-COL0-NEXT: {{^@interface Foo$}}
+// CHECK-PLAIN-COL0-NEXT: {{^@end$}}
+MACRO MACRO(A)
+@interface Foo
+@end
+
+// CHECK-ATTRMACRO: {{^MACRO\(A\) MACRO$}}
+// CHECK-ATTRMACRO-NEXT: {{^@interface Foo$}}
+// CHECK-ATTRMACRO-NEXT: {{^@end$}}
+// CHECK-PLAIN: {{^MACRO\(A\) MACRO @interface Foo$}}
+// CHECK-PLAIN-NEXT: {{^@end$}}
+// COM: The leading indentation in the next case is existing behavior but probably not desired.
+// CHECK-PLAIN-COL0: {{^MACRO\(A\)$}}
+// CHECK-PLAIN-COL0: {{^MACRO$}}
+// CHECK-PLAIN-COL0-NEXT: {{^@interface Foo$}}
+// CHECK-PLAIN-COL0-NEXT: {{^@end$}}
+MACRO(A) MACRO
+@interface Foo
+@end
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -369,7 +369,7 @@
 // Infer the role of the l_paren based on the previous token if we haven't
 // detected one yet.
 if (PrevNonComment && OpeningParen.is(TT_Unknown)) {
-  if (PrevNonComment->is(tok::kw___attribute)) {
+  if (PrevNonComment->isOneOf(tok::kw___attribute, TT_AttributeMacro)) {
 OpeningParen.setType(TT_AttributeParen);
   } else if (PrevNonComment->isOneOf(TT_TypenameMacro, tok::kw_decltype,
  tok::kw_typeof,
@@ -4889,8 +4889,10 @@
   }
 
   // Ensure wrapping after __attribute__((XX)) and @interface etc.
-  if (Left.is(TT_AttributeParen) && Right.is(TT_ObjCDecl))
+  if (Left.isOneOf(TT_AttributeMacro, TT_AttributeParen) &&
+  Right.is(TT_ObjCDecl)) {
 return true;
+  }
 
   if (Left.is(TT_LambdaLBrace)) {
 if (IsFunctionArgument(Left) &&
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1228,8 +1228,9 @@

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

See also 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Please see 
https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added inline comments.



Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

agozillon wrote:
> awarzynski wrote:
> > This test looks correct to me, but please note that:
> > 
> > ```
> > ! Check that flang -fc1 is invoked when in --driver-mode=flang 
> > ```
> > 
> > yet (`%clang` instead of `%flang`)
> > 
> > ```
> > ! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
> > --check-prefixes=CHECK-OPENMP %s
> > ```
> > 
> > I'm not really sure whether we should be testing Flang-specific logic in 
> > Clang. Having said that, Flang does use `clangDriver` to implement its 
> > driver :) 
> > 
> > You could consider using 
> > https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
> >  instead (or add an OpenMP specific file there).
> > 
> > Not a blocker.
> Yes, I wasn't so sure either as it felt a little weird to test Flang 
> components inside of Clang, but as you said it is a Clang toolchain (that 
> this test is checking) and borrows from the clangDriver! 
> 
> I borrowed this test from other similar tests in the same folder that test 
> other flang specific driver logic in a similar manner, but I am more than 
> happy to add an additional flang specific driver test as you mention! 
On further looking into the frontend-forwarding test, I don't know if it is 
suitable for fopenmp-is-device as it is an fc1 option and won't be forwarded 
from the flang-new frontend down to fc1 at the moment! 

I think this test will be more suitable when additional flags like the 
fopenmp-targets (or similar flags) that are used in this test are added to the 
Flang driver. As they spawn/propagate the openmp-is-device flag. However, 
perhaps I am incorrect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

[Michael Francis via Phabricator via cfe-commits]

francii updated this revision to Diff 502204.
francii added a comment.

Fix missing bracket


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145021

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ibm-profiling.c
  clang/test/Driver/zos-profiling-error.c

Index: clang/test/Driver/zos-profiling-error.c
===
--- clang/test/Driver/zos-profiling-error.c
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: %clang 2>&1 -### --target=s390x-none-zos -pg -S %s | FileCheck -check-prefix=FAIL-PG-NAME %s
-// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
Index: clang/test/Driver/ibm-profiling.c
===
--- /dev/null
+++ clang/test/Driver/ibm-profiling.c
@@ -0,0 +1,27 @@
+// Check that -pg throws an error on z/OS.
+// RUN: %clang -### 2>&1 --target=s390x-none-zos -S -pg %s | FileCheck -check-prefix=FAIL-PG-NAME %s
+// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
+
+// Check that -p is still used when not linking on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 -S -p -S %s \
+// RUN:   | FileCheck --check-prefix=CHECK3 %s
+// CHECK3-NOT: warning: argument unused during compilation: '-p'
+
+// Check precedence: -pg is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -pg -p %s \
+// RUN:| FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK: "[[SYSROOT]]/usr/lib{{/|}}mcrt0.o"
+// CHECK: "-L[[SYSROOT]]/lib/profiled"
+// CHECK: "-L[[SYSROOT]]/usr/lib/profiled"
+
+// Check precedence: -p is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -p -pg %s \
+// RUN:| FileCheck --check-prefix=CHECK2 %s
+// CHECK2: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK2: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK2: "[[SYSROOT]]/usr/lib{{/|}}gcrt0.o"
+// CHECK2: "-L[[SYSROOT]]/lib/profiled"
+// CHECK2: "-L[[SYSROOT]]/usr/lib/profiled"
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6322,20 +6322,26 @@
 << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
-if (TC.getTriple().isOSAIX()) {
-  CmdArgs.push_back("-pg");
-} else if (!TC.getTriple().isOSOpenBSD()) {
+
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
+if (TC.getTriple().isOSzOS()) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
-if (TC.getTriple().isOSzOS()) {
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
+if (!(TC.getTriple().isOSAIX() || TC.getTriple().isOSOpenBSD())) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg)) {
+if (A->getOption().matches(options::OPT_p)) {
+  A->claim();
+  if (TC.getTriple().isOSAIX() && !Args.hasArgNoClaim(options::OPT_pg))
+CmdArgs.push_back("-pg");
+}
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -164,11 +164,12 @@
   }
 
   auto getCrt0Basename = [, IsArch32Bit] {
+Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg);
 // Enable gprofiling when "-pg" is specified.
-if (Args.hasArg(options::OPT_pg))
+if (A->getOption().matches(options::OPT_pg))
   return IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o";
 // Enable profiling when "-p" is specified.
-else if (Args.hasArg(options::OPT_p))
+else if (A->getOption().matches(options::OPT_p))
   return IsArch32Bit ? "mcrt0.o" : "mcrt0_64.o";
 else
   return IsArch32Bit ? "crt0.o" : "crt0_64.o";
@@ -271,7 +272,7 @@
 
 CmdArgs.push_back("-lc");
 
-if (Args.hasArg(options::OPT_p, options::OPT_pg)) {
+if (Args.hasArgNoClaim(options::OPT_p, options::OPT_pg)) {
   CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
"/lib/profiled"));
   

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

[Michael Francis via Phabricator via cfe-commits]

francii added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6327
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
+  if (Arg *A = Args.getLastArg(options::OPT_p)) {
 if (TC.getTriple().isOSAIX()) {

daltenty wrote:
> Actually, a question here. Does this now result in the arg being claimed even 
> if nothing was done it? Since there are case we don't go into the error path.
I've updated the logic of this patch entirely. With these changes, using `-p` 
and `-pg` together now gives precedence to the option that is passed last.

Now, whenever we check for `-p`, we **never** do a blind claim. We instead 
manually claim `-p` in the following instances:
1. The user passes `-p` without also passing `-pg`: We manually claim `-p`, and 
push `-pg` to `CmdArgs`.
2. The user passes both `-p` and `-pg`, but `-p` is given precedence: We 
manually claim `-p`, and do nothing (since `-pg` is already in `CmdArgs`).

As a result, `-p` will only warn that it goes unused if it is passed before 
`-pg` (i.e. `clang -p -pg`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145021

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

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi updated this revision to Diff 502201.
akyrtzi added a comment.

Avoid passing a new `IgnoringDiagConsumer` for the test since it's unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145256

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/Basic/Diagnostic.cpp
  clang/unittests/Basic/DiagnosticTest.cpp


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,26 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,26 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions);
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}
___
cfe-commits mailing 

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145256

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

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 502190.
cjdb added a comment.

fixes string goof


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145201

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/lib/Basic/Sarif.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/test/Frontend/sarif-diagnostics.hpp

Index: clang/test/Frontend/sarif-diagnostics.hpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.hpp
@@ -0,0 +1 @@
+Test test;
Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -27,6 +27,8 @@
 x + y;
 }
 
+#include "sarif-diagnostics.hpp"
+
 // RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t.txt 2>&1 || true
 // RUN: FileCheck -dump-input=always %s --input-file=%t.txt --check-prefixes=STDERR,SARIF
 
@@ -46,6 +48,17 @@
 // SARIF:   "roles":[
 // SARIF: "resultFile"
 // SARIF:   ]
+// SARIF: },
+// SARIF: {
+// SARIF:   "length":{{[0-9]+}},
+// SARIF:   "location":{
+// SARIF: "index":1,
+// SARIF: "uri":"file://{{[^"]+test/Frontend/sarif-diagnostics.hpp}}"
+// SARIF:   },
+// SARIF:   "mimeType":"text/plain",
+// SARIF:   "roles":[
+// SARIF: "resultFile"
+// SARIF:   ]
 // SARIF: }
 // SARIF:   ],
 // SARIF:   "columnKind":"unicodeCodePoints",
@@ -172,7 +185,7 @@
 // SARIF:   "physicalLocation":{
 // SARIF: "artifactLocation":{
 // SARIF:   "index":0,
-// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF:   "uri":{{"file://[^"]+clang/test/Frontend/sarif-diagnostics.cpp"}}
 // SARIF: },
 // SARIF: "region":{
 // SARIF:   "endColumn":10,
@@ -195,7 +208,7 @@
 // SARIF:   "physicalLocation":{
 // SARIF: "artifactLocation":{
 // SARIF:   "index":0,
-// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF:   "uri":{{"file://[^"]+clang/test/Frontend/sarif-diagnostics.cpp"}}
 // SARIF: },
 // SARIF: "region":{
 // SARIF:   "endColumn":12,
@@ -284,6 +297,52 @@
 // SARIF:   },
 // SARIF:   "ruleId":"4567",
 // SARIF:   "ruleIndex":8
+// SARIF: },
+// SARIF: {
+// SARIF:   "level":"note",
+// SARIF:   "locations":[
+// SARIF: {
+// SARIF:   "physicalLocation":{
+// SARIF: "artifactLocation":{
+// SARIF:   "index":0,
+// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF: },
+// SARIF: "region":{
+// SARIF:   "endColumn":10,
+// SARIF:   "startColumn":10,
+// SARIF:   "startLine":30
+// SARIF: }
+// SARIF:   }
+// SARIF: }
+// SARIF:   ],
+// SARIF:   "message":{
+// SARIF: "text":"in file included from {{[^"]+test/Frontend/sarif-diagnostics.cpp:30:}}\n"
+// SARIF:   },
+// SARIF:   "ruleId":"-1",
+// SARIF:   "ruleIndex":9
+// SARIF: },
+// SARIF: {
+// SARIF:   "level":"error",
+// SARIF:   "locations":[
+// SARIF: {
+// SARIF:   "physicalLocation":{
+// SARIF: "artifactLocation":{
+// SARIF:   "index":1,
+// SARIF:   "uri":"file:///{{[^"]+/test/Frontend/sarif-diagnostics.hpp}}"
+// SARIF: },
+// SARIF: "region":{
+// SARIF:   "endColumn":1,
+// SARIF:   "startColumn":1,
+// SARIF:   "startLine":1
+// SARIF: }
+// SARIF:   }
+// SARIF: }
+// SARIF:   ],
+// SARIF:   "message":{
+// SARIF: "text":"unknown type name 'Test'"
+// SARIF:   },
+// SARIF:   "ruleId":"4657",
+// SARIF:   "ruleIndex":10
 // SARIF: }
 // SARIF:   ],
 // SARIF:   "tool":{
@@ -400,6 +459,30 @@
 // SARIF:   },
 // SARIF:   "id":"4567",
 // SARIF:   "name":""
+// SARIF: },
+// SARIF: {
+// SARIF:   "defaultConfiguration":{
+// SARIF: "enabled":true,
+// SARIF: "level":"note",
+// SARIF: "rank":-1
+// SARIF:   },
+// SARIF:   

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

That test is kinda problematic because it seems that the artifacts aren't 
ordered. I think we should change this from a




Comment at: clang/lib/Basic/Sarif.cpp:314-317
+  llvm::sort(*Artifacts, [](const json::Value , const json::Value ) {
+return x.getAsObject()->getNumber("index") <
+   y.getAsObject()->getNumber("index");
+  });

I'm wondering if I should instead copy `CurrentArtifacts` to a vector and sort 
prior to insertion, rather than in post.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214
 void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) 
{
-  assert(false && "Not implemented in SARIF mode");
+  SarifRule Rule = SarifRule::create().setRuleId(std::to_string(-1));
+  Rule = addDiagnosticLevelToRule(Rule, DiagnosticsEngine::Level::Note);

aaron.ballman wrote:
> Why do we want -1 as the rule ID and... can we use `"-1"` instead of doing a 
> string conversion?
lol at obvious C++ goof.

Re -1, there doesn't seem to be a diagnostic associated with this note, so I 
picked a value that I know isn't in use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145201

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

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb updated this revision to Diff 502186.
cjdb added a comment.
Herald added a subscriber: mgrang.

sorts artifacts so that they're output in index order


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145201

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/lib/Basic/Sarif.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/test/Frontend/sarif-diagnostics.hpp

Index: clang/test/Frontend/sarif-diagnostics.hpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.hpp
@@ -0,0 +1 @@
+Test test;
Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -27,6 +27,8 @@
 x + y;
 }
 
+#include "sarif-diagnostics.hpp"
+
 // RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t.txt 2>&1 || true
 // RUN: FileCheck -dump-input=always %s --input-file=%t.txt --check-prefixes=STDERR,SARIF
 
@@ -46,6 +48,17 @@
 // SARIF:   "roles":[
 // SARIF: "resultFile"
 // SARIF:   ]
+// SARIF: },
+// SARIF: {
+// SARIF:   "length":{{[0-9]+}},
+// SARIF:   "location":{
+// SARIF: "index":1,
+// SARIF: "uri":"file://{{[^"]+test/Frontend/sarif-diagnostics.hpp}}"
+// SARIF:   },
+// SARIF:   "mimeType":"text/plain",
+// SARIF:   "roles":[
+// SARIF: "resultFile"
+// SARIF:   ]
 // SARIF: }
 // SARIF:   ],
 // SARIF:   "columnKind":"unicodeCodePoints",
@@ -172,7 +185,7 @@
 // SARIF:   "physicalLocation":{
 // SARIF: "artifactLocation":{
 // SARIF:   "index":0,
-// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF:   "uri":{{"file://[^"]+clang/test/Frontend/sarif-diagnostics.cpp"}}
 // SARIF: },
 // SARIF: "region":{
 // SARIF:   "endColumn":10,
@@ -195,7 +208,7 @@
 // SARIF:   "physicalLocation":{
 // SARIF: "artifactLocation":{
 // SARIF:   "index":0,
-// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF:   "uri":{{"file://[^"]+clang/test/Frontend/sarif-diagnostics.cpp"}}
 // SARIF: },
 // SARIF: "region":{
 // SARIF:   "endColumn":12,
@@ -284,6 +297,52 @@
 // SARIF:   },
 // SARIF:   "ruleId":"4567",
 // SARIF:   "ruleIndex":8
+// SARIF: },
+// SARIF: {
+// SARIF:   "level":"note",
+// SARIF:   "locations":[
+// SARIF: {
+// SARIF:   "physicalLocation":{
+// SARIF: "artifactLocation":{
+// SARIF:   "index":0,
+// SARIF:   "uri":{{"file://[^"]+/clang/test/Frontend/sarif-diagnostics.cpp"}}
+// SARIF: },
+// SARIF: "region":{
+// SARIF:   "endColumn":10,
+// SARIF:   "startColumn":10,
+// SARIF:   "startLine":30
+// SARIF: }
+// SARIF:   }
+// SARIF: }
+// SARIF:   ],
+// SARIF:   "message":{
+// SARIF: "text":"in file included from {{[^"]+test/Frontend/sarif-diagnostics.cpp:30:}}\n"
+// SARIF:   },
+// SARIF:   "ruleId":"-1",
+// SARIF:   "ruleIndex":9
+// SARIF: },
+// SARIF: {
+// SARIF:   "level":"error",
+// SARIF:   "locations":[
+// SARIF: {
+// SARIF:   "physicalLocation":{
+// SARIF: "artifactLocation":{
+// SARIF:   "index":1,
+// SARIF:   "uri":"file:///{{[^"]+/test/Frontend/sarif-diagnostics.hpp}}"
+// SARIF: },
+// SARIF: "region":{
+// SARIF:   "endColumn":1,
+// SARIF:   "startColumn":1,
+// SARIF:   "startLine":1
+// SARIF: }
+// SARIF:   }
+// SARIF: }
+// SARIF:   ],
+// SARIF:   "message":{
+// SARIF: "text":"unknown type name 'Test'"
+// SARIF:   },
+// SARIF:   "ruleId":"4657",
+// SARIF:   "ruleIndex":10
 // SARIF: }
 // SARIF:   ],
 // SARIF:   "tool":{
@@ -400,6 +459,30 @@
 // SARIF:   },
 // SARIF:   "id":"4567",
 // SARIF:   "name":""
+// SARIF: },
+// SARIF: {
+// SARIF:   "defaultConfiguration":{
+// SARIF: "enabled":true,
+// SARIF: "level":"note",
+// SARIF: "rank":-1

[PATCH] D145228: [clangd] Add clangd headers to install targets

[Shoaib Meenai via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145228

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

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

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

"But when would you have a completely empty diagnostic message", you ask dear 
reader?
That is when there is an empty "#warning" in code.

rdar://106155415


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145256

Files:
  clang/include/clang/Basic/Diagnostic.h
  clang/lib/Basic/Diagnostic.cpp
  clang/unittests/Basic/DiagnosticTest.cpp


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,27 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+  new IgnoringDiagConsumer());
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const DiagnosticsEngine *DiagObj;
-  StringRef StoredDiagMessage;
+  std::optional StoredDiagMessage;
 
 public:
   explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {}


Index: clang/unittests/Basic/DiagnosticTest.cpp
===
--- clang/unittests/Basic/DiagnosticTest.cpp
+++ clang/unittests/Basic/DiagnosticTest.cpp
@@ -9,6 +9,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticLex.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -128,4 +129,27 @@
   EXPECT_EQ(*Value, std::make_pair(20, 1));
   EXPECT_EQ(Value->first, 20);
 }
+
+TEST(DiagnosticTest, storedDiagEmptyWarning) {
+  DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions,
+  new IgnoringDiagConsumer());
+
+  class CaptureDiagnosticConsumer : public DiagnosticConsumer {
+  public:
+SmallVector StoredDiags;
+
+void HandleDiagnostic(DiagnosticsEngine::Level level,
+  const Diagnostic ) override {
+  StoredDiags.push_back(StoredDiagnostic(level, Info));
+}
+  };
+
+  CaptureDiagnosticConsumer CaptureConsumer;
+  Diags.setClient(, /*ShouldOwnClient=*/false);
+  Diags.Report(diag::pp_hash_warning) << "";
+  ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1);
+
+  // Make sure an empty warning can round-trip with \c StoredDiagnostic.
+  Diags.Report(CaptureConsumer.StoredDiags.front());
+}
 }
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -793,8 +793,8 @@
 /// array.
 void Diagnostic::
 FormatDiagnostic(SmallVectorImpl ) const {
-  if (!StoredDiagMessage.empty()) {
-OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end());
+  if (StoredDiagMessage.has_value()) {
+OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end());
 return;
   }
 
Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -1565,7 +1565,7 @@
 /// currently in-flight diagnostic.
 class Diagnostic {
   const 

[PATCH] D142328: [clang][Interp] Fix compound assign operator types

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

All the interpreter work is experimental so it doesn't really need to be in the 
release branch. This should all be fine, thanks for asking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142328

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

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Usama Hameed via Phabricator via cfe-commits]

usama54321 added inline comments.



Comment at: 
compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 

yln wrote:
> usama54321 wrote:
> > yln wrote:
> > > zixuw wrote:
> > > > dmaclach wrote:
> > > > > yln wrote:
> > > > > > This should work, right?
> > > > > No.. darwin should fail with the `-static-libsan` flag. This is the 
> > > > > test that was failing and caused the rollback.
> > > > I think @yln is suggesting using `REQUIRES: asan-static-runtime` 
> > > > instead of `XFAIL: darwin`. I wasn't aware of that conditional but yeah 
> > > > that should be better if it works.
> > > I meant using `// REQUIRES: asan-static-runtime ` instead of `XFAIL: 
> > > darwin` since it seems that we already have a lit feature for it.
> > I think UNSUPPORTED: darwin makes the most sense here. I don't think lit 
> > understands that REQUIRES: asan-static-runtime should result in skipping 
> > the test on Darwin as it does not know about this dependency.
> > I don't think lit understands that REQUIRES: asan-static-runtime should 
> > result in skipping the test on Darwin as it does not know about this 
> > dependency.
> 
> Actually, this was exactly my point.  We have other tests already marked with 
> `REQUIRES: asan-static-runtime` and we should double check our changes don't 
> affect these as well.
> 
> If LIT doesn't model this dependency yet, then we should make sure it does!  
> And this test can act as a good "canary in the coal mine".
> 
> Please use `REQUIRES: asan-static-runtime` and make sure we understand and 
> deal with any fallout.
Sorry for my earlier comment. @yln is correct, and we should use REQUIRES: 
asan-static-runtime. I double checked, and this already works as expected on 
darwin, i.e. these tests are unsupported on darwin. So you should not need to 
do anything apart from adding the REQUIRES in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

Thank you very much @awarzynski I will wait for further approval!




Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

awarzynski wrote:
> This test looks correct to me, but please note that:
> 
> ```
> ! Check that flang -fc1 is invoked when in --driver-mode=flang 
> ```
> 
> yet (`%clang` instead of `%flang`)
> 
> ```
> ! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
> --check-prefixes=CHECK-OPENMP %s
> ```
> 
> I'm not really sure whether we should be testing Flang-specific logic in 
> Clang. Having said that, Flang does use `clangDriver` to implement its driver 
> :) 
> 
> You could consider using 
> https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
>  instead (or add an OpenMP specific file there).
> 
> Not a blocker.
Yes, I wasn't so sure either as it felt a little weird to test Flang components 
inside of Clang, but as you said it is a Clang toolchain (that this test is 
checking) and borrows from the clangDriver! 

I borrowed this test from other similar tests in the same folder that test 
other flang specific driver logic in a similar manner, but I am more than happy 
to add an additional flang specific driver test as you mention! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `[any]` to `(DRE.data() + any)`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D143128#4167626 , @jkorous wrote:

> This is an interesting topic. In the abstract I see the question as: "Should 
> the Fix-Its prioritize how the code will fit the desired end state 
> (presumably modern idiomatic C++) or carefully respect the state of the code 
> as is now?"
>
> The only thing I feel pretty strongly about is that no matter what philosophy 
> we decide to use here we should apply it consistently to all our Fix-Its 
> (which might or might not already be the case).
>
> And FWIW I can also imagine at some point in the future we might either have 
> two dialects of the Fix-Its or that a separate modernizer tool (completely 
> independent of Safe Buffers) could suggest transformations like:
> "Would you like to change `()[any]` to `(DRE.data() + any)`?"

Fantastic topic! :)  In our experience at Google, we've generally followed the 
philosophy of leaving the code at least as good as it was before we touched it. 
So, if the idiom is archaic, but we're just adjusting it, that's fine (as in 
this example), but our tools shouldn't generate non-idiomatic (or anti-idiomic) 
code.  We've also often taken the path of "leave cleanups to a separate pass", 
especially when we already have say, a clang tidy check, that does that kind of 
clean up running regularly.  But, this one is more a judgment call. I'd 
probably lean towards "make the code better once you're at it", but certainly 
see the conservative argument.


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

https://reviews.llvm.org/D143128

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

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D145178#4167103 , @aaron.ballman 
wrote:

> LGTM, this is incremental progress. Hopefully we won't be adding too many 
> more RUN lines to this file though (sticking too many tests into one file is 
> also tech debt).

Thanks! 100% of the RUN lines I intend to add are bug regressions, but there 
will be more //RUN// lines in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145178

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4052-4060
+  // LLVM utilities like blocks with terminators.
+  auto *UI = Builder.CreateUnreachable();
+  Instruction *ThenTI = UI, *ElseTI = nullptr;
+  if (IfCond) {
+SplitBlockAndInsertIfThenElse(IfCond, UI, , );
+ThenTI->getParent()->setName("omp_if.then");
+ElseTI->getParent()->setName("omp_if.else");

kiranchandramohan wrote:
> There is some recent recommendation to not insert artificial instructions and 
> remove them. The preferred approach is to use `splitBB` function(s) inside 
> the OpenMPIRBuilder. This function works on blocks without terminators. You 
> can consult the `IfCondition` code in `createParallel`.
Thanks a lot for taking the time to review this lengthy patch.

This one seems a bit tricky to do. At first glance `createParallel` seems to be 
doing something different where its calling different runtime functions based 
on the `IfCondition` instead of much in the way of Control Flow changes.

The `unreachable` inst helps out a lot here as it makes it really easy to keep 
trace of the resume point for adding instructions after the `BodyGen` codes are 
generated.

I am still looking into finding a way to do this elegantly without having to 
use the `unreachable` instruction, but would it be a major blocker if we had to 
keep it?

I have addressed all the other changes you requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D145098: [clang][deps] Preserve input ordering in the full output

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D145098#4167163 , @Jake-Egan wrote:

> Hi, this new test fails on AIX, could you take a look please?
> https://lab.llvm.org/buildbot/#/builders/214/builds/6148/steps/6/logs/FAIL__Clang__modules-full-output-tu-order_c

`-fno-integrated-as` strikes again. Thanks for reporting this, should be fixed 
in 3ac8d32 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145098

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

[clang] 3ac8d32 - [clang][deps] Fix test failing on AIX

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-03-03T10:22:39-08:00
New Revision: 3ac8d322100bc72aaacb67f13eb21d9b644f9930

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

LOG: [clang][deps] Fix test failing on AIX

Introduced in 86405450, caused by AIX defaulting to `-fno-integrated-as`.

Added: 


Modified: 
clang/test/ClangScanDeps/modules-full-output-tu-order.c

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-full-output-tu-order.c 
b/clang/test/ClangScanDeps/modules-full-output-tu-order.c
index dd5002809daa9..eb96d2e99c622 100644
--- a/clang/test/ClangScanDeps/modules-full-output-tu-order.c
+++ b/clang/test/ClangScanDeps/modules-full-output-tu-order.c
@@ -8,12 +8,12 @@
   {
 "directory": "DIR",
 "file": "DIR/tu.c",
-"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c -o 
DIR/tu1.o"
+"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c 
-DONE -o DIR/tu1.o"
   },
   {
 "directory": "DIR",
 "file": "DIR/tu.c",
-"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c -o 
DIR/tu2.o"
+"command": "clang -fmodules -fmodules-cache-path=DIR/cache -c DIR/tu.c 
-DTWO -o DIR/tu2.o"
   }
 ]
 
@@ -32,8 +32,8 @@
 // CHECK-NEXT:   "clang-context-hash": "{{.*}}",
 // CHECK-NEXT:   "clang-module-deps": [],
 // CHECK-NEXT:   "command-line": [
-// CHECK:  "-o",
-// CHECK-NEXT: "[[PREFIX]]/tu1.o",
+// CHECK:  "-D"
+// CHECK-NEXT: "ONE"
 // CHECK:],
 // CHECK-NEXT:   "executable": "clang",
 // CHECK-NEXT:   "file-deps": [
@@ -49,8 +49,8 @@
 // CHECK-NEXT:   "clang-context-hash": "{{.*}}",
 // CHECK-NEXT:   "clang-module-deps": [],
 // CHECK-NEXT:   "command-line": [
-// CHECK:  "-o",
-// CHECK-NEXT: "[[PREFIX]]/tu2.o",
+// CHECK:  "-D"
+// CHECK-NEXT: "TWO"
 // CHECK:],
 // CHECK-NEXT:   "executable": "clang",
 // CHECK-NEXT:   "file-deps": [



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

[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `[any]` to `(DRE.data() + any)`

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

This is an interesting topic. In the abstract I see the question as should the 
Fix-Its prioritize how the code will fit the desired end state (presumably 
modern idiomatic C++) or carefully respect the state of the code as is now.

The only thing I feel pretty strongly about is that no matter what philosophy 
we decide to use here we should apply it consistently to all our Fix-Its (which 
might or might not already be the case).

And FWIW I can also imagine at some point in the future we might either have 
two dialects of the Fix-Its or that a separate modernizer tool (completely 
independent of Safe Buffers) could suggest transformations like:
"Would you like to change `()[any]` to `(DRE.data() + any)`?"


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

https://reviews.llvm.org/D143128

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 502175.
TIFitis marked 7 inline comments as done.
TIFitis added a comment.

Addressed reviewer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/include/mlir/Target/LLVMIR/Dialect/OpenMPCommon.h
  mlir/lib/Target/LLVMIR/CMakeLists.txt
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMPCommon.cpp
  mlir/test/Target/LLVMIR/omptarget-llvm.mlir

Index: mlir/test/Target/LLVMIR/omptarget-llvm.mlir
===
--- /dev/null
+++ mlir/test/Target/LLVMIR/omptarget-llvm.mlir
@@ -0,0 +1,205 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+
+llvm.func @_QPopenmp_target_data() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
+  omp.target_data   map((tofrom -> %1 : !llvm.ptr)) {
+%2 = llvm.mlir.constant(99 : i32) : i32
+llvm.store %2, %1 : !llvm.ptr
+omp.terminator
+  }
+  llvm.return
+}
+
+// CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
+// CHECK-LABEL: define void @_QPopenmp_target_data() {
+// CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
+// CHECK: %[[VAL_3:.*]] = alloca i32, i64 1, align 4
+// CHECK: br label %[[VAL_4:.*]]
+// CHECK:   entry:; preds = %[[VAL_5:.*]]
+// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_6]], align 8
+// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_7]], align 8
+// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %[[VAL_8]], align 4
+// CHECK: %[[VAL_9:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_10:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: %[[VAL_11:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr %[[VAL_11]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: br label %[[VAL_12:.*]]
+// CHECK:   omp.data.region:  ; preds = %[[VAL_4]]
+// CHECK: store i32 99, ptr %[[VAL_3]], align 4
+// CHECK: br label %[[VAL_13:.*]]
+// CHECK:   omp.region.cont:  ; preds = %[[VAL_12]]
+// CHECK: %[[VAL_14:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_15:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: %[[VAL_16:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_14]], ptr %[[VAL_15]], ptr %[[VAL_16]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: ret void
+
+// -
+
+llvm.func @_QPopenmp_target_data_loop(%1 : !llvm.ptr>) {
+  %2 = llvm.mlir.constant(1 : i64) : i64
+  %3 = llvm.alloca %2 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array, uniq_name = "_QFopenmp_target_data_loopEi"} : (i64) -> !llvm.ptr
+  omp.target_data   map((from -> %1 : !llvm.ptr>)) {
+%4 = llvm.mlir.constant(1 : i32) : i32
+%5 = llvm.sext %4 : i32 to i64
+%6 = llvm.mlir.constant(1024 : i32) : i32
+%7 = llvm.sext %6 : i32 to i64
+%8 = llvm.mlir.constant(1 : index) : i64
+%9 = llvm.trunc %5 : i64 to i32
+%10 = llvm.sub %7, %5  : i64
+%11 = llvm.add %10, %8  : i64
+llvm.br ^bb1(%5, %9, %11 : i64, i32, i64)
+  ^bb1(%12: i64, %13: i32, %14: i64):  // 2 preds: ^bb0, ^bb2
+%15 = llvm.mlir.constant(0 : index) : i64
+%16 = llvm.icmp "sgt" %14, %15 : i64
+llvm.cond_br %16, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+llvm.store %13, %3 : !llvm.ptr
+%17 = llvm.load %3 : !llvm.ptr
+%18 = llvm.load %3 : !llvm.ptr
+%19 = llvm.sext %18 : i32 to i64
+%20 = 

[PATCH] D145187: [Fuchsia] Add other necessary components to LLDB install.

[Daniel Thornburgh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc65fb80b73aa: [Fuchsia] Add other necessary components to 
LLDB install. (authored by mysterymath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145187

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -343,8 +343,8 @@
 
 set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
 if(FUCHSIA_ENABLE_LLDB)
-  list(APPEND _FUCHSIA_ENABLE_PROJECTS "lldb")
-  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS "lldb")
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS lldb liblldb lldb-server 
lldb-argdumper)
 endif()
 
 set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -343,8 +343,8 @@
 
 set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
 if(FUCHSIA_ENABLE_LLDB)
-  list(APPEND _FUCHSIA_ENABLE_PROJECTS "lldb")
-  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS "lldb")
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS lldb liblldb lldb-server lldb-argdumper)
 endif()
 
 set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c65fb80 - [Fuchsia] Add other necessary components to LLDB install.

[Daniel Thornburgh via cfe-commits]

Author: Daniel Thornburgh
Date: 2023-03-03T10:07:08-08:00
New Revision: c65fb80b73aa5087a4bba79e61d96b4d652e1b54

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

LOG: [Fuchsia] Add other necessary components to LLDB install.

Reviewed By: phosek

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 54a64a84f231b..016fa9eb5f9d2 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -343,8 +343,8 @@ set(_FUCHSIA_DISTRIBUTION_COMPONENTS
 
 set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
 if(FUCHSIA_ENABLE_LLDB)
-  list(APPEND _FUCHSIA_ENABLE_PROJECTS "lldb")
-  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS "lldb")
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+  list(APPEND _FUCHSIA_DISTRIBUTION_COMPONENTS lldb liblldb lldb-server 
lldb-argdumper)
 endif()
 
 set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")



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

[PATCH] D142800: [Clang][Diagnostic] Add `-Wcomparison-op-parentheses` to warn on chained comparisons

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

In D142800#4165090 , @aaron.ballman 
wrote:

> In D142800#4104241 , @hazohelet 
> wrote:
>
>>> Also, why are these diagnostics off by default? Do we have some idea as to 
>>> the false positive rate?
>>
>> As for the false positive rate, I have checked for instances of this warning 
>> in the codebases for 'oneapi-src/oneTBB', 'rui314/mold', and 
>> 'microsoft/lightgbm', but did not find any new cases.
>
> Thank you for doing the extra testing! Sorry for the delayed response, this 
> review fell off my radar for a bit.
>
>> I also ran a test on  'tensorflow/tensorflow' using gcc '-Wparentheses' and 
>> found six lines of code that trigger the new diagnostic. They all relate to 
>> checking whether `x` and `y` have the same sign using `x > 0 == y > 0` and 
>> alike. I tried to build with tensorflow using clang, but I stumbled upon 
>> some errors (for my poor knowledge of bazel configuration), so here I am 
>> using gcc.
>
> Thank you for reporting this, that's really good information.
>
>> I set the diagnostic disabled by default for compatibility with gcc. 
>> Considering the test against tensorflow above, it would be too noisy if we 
>> turned on the suggest-parentheses diagnostic by default (From my best guess, 
>> it would generate at least 18 new instances of warning on the tensorflow 
>> build for the six lines).
>> However, in my opinion, it is reasonable enough to have the diagnostic on 
>> chained relational operators enabled by default. The following is an excerpt 
>> from the 'Existing Code in C++' section of the proposal document of the 
>> introduction of chaining comparison for C++17.
>>
>>> Overall, what we found was:
>>>
>>> - Zero instances of chained arithmetic comparisons that are correct today. 
>>> That is, intentionally using the current standard behavior.
>>> - Four instances of currently-erroneous arithmetic chaining, of the 
>>> assert(0 <= ratio <= 1.0); variety. These are bugs that compile today but 
>>> don’t do what the programmer intended, but with this proposal would change 
>>> in meaning to become correct.
>>> - Many instances of using successive comparison operators in DSLs that 
>>> overloaded these operators to give meaning unrelated to comparisons.
>>
>> Note that the 'chaining comparisons' in the document are
>>
>>> - all `==`, such as `a == b == c == d`;
>>> - all `{<, <=}`, such as `a < b <= c < d`; and
>>> - all `{>, >=}` (e.g., `a >= b > c > d`).
>>
>> URL: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0893r0.html
>>
>> Although this paper is five years old now, I think we can conclude chaining 
>> relational operators are bug-prone and should be diagnosed by default.
>> Also, reading the document above, I think it would be reasonable to suggest 
>> adding '&&' in `a == b == c` case, too.
>
> I tend to agree -- I was searching around sourcegraph 
> (https://sourcegraph.com/search?q=context:global+lang:C+lang:C%2B%2B+%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29%5Cs*%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29=regexp=1=repo)
>  and I can't find a whole lot of evidence for chained operators outside of 
> comments.

Thanks for the review and the sourcegraph search!
I added a new warning flag `-Wchaining-comparisons` that is enabled by default. 
I added this flag to `-Wparentheses` for gcc compatibility.


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

https://reviews.llvm.org/D142800

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

[PATCH] D142800: [Clang][Diagnostic] Add `-Wcomparison-op-parentheses` to warn on chained comparisons

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet updated this revision to Diff 502166.
hazohelet added a comment.

Update the differential

- Revise warning messages based on ideas from @aaron.ballman.
- Introduce a new warning flag `-Wchaining-comparisons` that is enabled by 
default, to warn about chaining relationals or equal operators.
- Adjust existing test files to conform with the new warning settings.


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

https://reviews.llvm.org/D142800

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/cxx-uninitialized-object.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Sema/bool-compare.c
  clang/test/Sema/chaining-comparisons.c
  clang/test/Sema/comparison-op-parentheses.c
  clang/test/SemaCXX/bool-compare.cpp
  clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
  clang/test/SemaTemplate/dependent-template-recover.cpp
  clang/test/SemaTemplate/typo-dependent-name.cpp
  clang/test/SemaTemplate/typo-template-name.cpp
  libcxx/test/support/test_comparisons.h

Index: libcxx/test/support/test_comparisons.h
===
--- libcxx/test/support/test_comparisons.h
+++ libcxx/test/support/test_comparisons.h
@@ -164,7 +164,7 @@
 bool less= order == Order::less;
 bool greater = order == Order::greater;
 
-return (t1 <=> t2 == order) && testComparisonsComplete(t1, t2, equal, less, greater);
+return ((t1 <=> t2) == order) && testComparisonsComplete(t1, t2, equal, less, greater);
 }
 
 template 
Index: clang/test/SemaTemplate/typo-template-name.cpp
===
--- clang/test/SemaTemplate/typo-template-name.cpp
+++ clang/test/SemaTemplate/typo-template-name.cpp
@@ -37,6 +37,8 @@
 // These are valid expressions.
 foo(0);
+  // expected-warning@-1 {{boolean value derived from '<' is compared as the operand of '>'; did you mean 'foo < int() && int() > (0)'?}}
+  // expected-note@-2 {{split the comparison into two}}
 foo(false);
 fooinner < other > ::z;
+  // expected-warning@-1 {{boolean value derived from '<' is compared as the operand of '>'; did you mean 'this->inner < other && other > ::z'?}}
+  // expected-note@-2 {{split the comparison into two}}
   }
 };
 
Index: clang/test/SemaTemplate/dependent-template-recover.cpp
===
--- clang/test/SemaTemplate/dependent-template-recover.cpp
+++ clang/test/SemaTemplate/dependent-template-recover.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-chaining-comparisons %s
 template
 struct X {
   void f(T* t) {
Index: clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
===
--- clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
+++ clang/test/SemaCXX/cxx2a-adl-only-template-id.cpp
@@ -28,6 +28,8 @@
   f<0>(q);
   int f;
   f<0>(q); // expected-error {{invalid operands to binary expression}}
+  // expected-warning@-1 {{boolean value derived from '<' is compared as the operand of '>'; did you mean 'f < 0 && 0 > (q)'?}}
+  // expected-note@-2 {{split the comparison into two}}
 }
 
 void disambig() {
Index: clang/test/SemaCXX/bool-compare.cpp
===
--- clang/test/SemaCXX/bool-compare.cpp
+++ clang/test/SemaCXX/bool-compare.cpp
@@ -98,7 +98,9 @@
   if ((ayy' is compared as the operand of '<'; did you mean 'a > y && y < z'?}}
+  // expected-note@-2 {{split the comparison into two}}
   if ((a z)  {} // no warning
   if((a(zy)  {} // no warning
+  if (zy)  {}
+  // expected-warning@-1 {{boolean value derived from '<' is compared as the operand of '>'; did you mean 'z < a && a > y'?}}
+  // expected-note@-2 {{split the comparison into two}}
   if (z > (a(a&1 | FileCheck %s
+
+// off-no-diagnostics
+
+void comparison_op_parentheses(int a, int b, int c) {
+  (void)(a ==
+ b > c);
+  // expected-warning@-1 {{boolean value derived from '>' is compared as the operand of '=='}}
+  // expected-note@-2 {{place parentheses around the '>' expression to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:14-[[@LINE-3]]:14}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:19-[[@LINE-4]]:19}:")"
+
+  (void)(a !=b == c);
+  // expected-warning@-1 {{boolean value derived from '!=' is compared as the operand of '=='}}
+  // expected-note@-2 {{place parentheses around the '!=' expression to silence this warning}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+
+  (void)(a !=
+ b < c);
+  // expected-warning@-1 {{boolean value derived from '<' is compared as the operand of '!='}}
+  // expected-note@-2 {{place parentheses around the '<' 

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrzej Warzynski via Phabricator via cfe-commits]

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

> Thank you very much @awarzynski for suggesting them that was a great help.

I'm happy that I could help :) The driver logic LGTM, but please wait for 
either @jdoerfert and/or @kiranchandramohan to also approve. Thanks for 
contributing!




Comment at: clang/test/Driver/flang/flang-omp.f90:1
+! Check that flang -fc1 is invoked when in --driver-mode=flang 
+! and the relevant openmp and openmp offload flags are utilised

This test looks correct to me, but please note that:

```
! Check that flang -fc1 is invoked when in --driver-mode=flang 
```

yet (`%clang` instead of `%flang`)

```
! RUN: %clang --driver-mode=flang -### -fopenmp %s 2>&1 | FileCheck 
--check-prefixes=CHECK-OPENMP %s
```

I'm not really sure whether we should be testing Flang-specific logic in Clang. 
Having said that, Flang does use `clangDriver` to implement its driver :) 

You could consider using 
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/frontend-forwarding.f90
 instead (or add an OpenMP specific file there).

Not a blocker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D144309: [HLSL] add max/min library functions

[Joshua Batista via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ac0551e77f4: [HLSL] add max/min library functions (authored 
by bob80905).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144309

Files:
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/builtins/max.hlsl
  clang/test/CodeGenHLSL/builtins/min.hlsl

Index: clang/test/CodeGenHLSL/builtins/min.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/min.hlsl
@@ -0,0 +1,223 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -O3 -o - | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes \
+// RUN:   -D__HLSL_ENABLE_16_BIT -o - | FileCheck %s --check-prefix=NO_HALF
+
+#ifdef __HLSL_ENABLE_16_BIT
+// CHECK: define noundef i16 @
+// CHECK: call i16 @llvm.smin.i16(
+// NO_HALF: define noundef i16 @"?test_min_short@@YAFFF@Z"(
+// NO_HALF: call i16 @llvm.smin.i16(
+int16_t test_min_short ( int16_t p0, int16_t p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <2 x i16> @
+// CHECK: call <2 x i16> @llvm.smin.v2i16(
+// NO_HALF: define noundef <2 x i16> @"?test_min_short2@@YAT?$__vector@F$01@__clang@@T12@0@Z"(
+// NO_HALF: call <2 x i16> @llvm.smin.v2i16(
+int16_t2 test_min_short2 ( int16_t2 p0, int16_t2 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <3 x i16> @
+// CHECK: call <3 x i16> @llvm.smin.v3i16
+// NO_HALF: define noundef <3 x i16> @"?test_min_short3@@YAT?$__vector@F$02@__clang@@T12@0@Z"(
+// NO_HALF: call <3 x i16> @llvm.smin.v3i16(
+int16_t3 test_min_short3 ( int16_t3 p0, int16_t3 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <4 x i16> @
+// CHECK: call <4 x i16> @llvm.smin.v4i16
+// NO_HALF: define noundef <4 x i16> @"?test_min_short4@@YAT?$__vector@F$03@__clang@@T12@0@Z"(
+// NO_HALF: call <4 x i16> @llvm.smin.v4i16(
+int16_t4 test_min_short4 ( int16_t4 p0, int16_t4 p1 ) {
+  return min ( p0, p1 );
+}
+
+
+// CHECK: define noundef i16 @
+// CHECK: call i16 @llvm.umin.i16(
+// NO_HALF: define noundef i16 @"?test_min_ushort@@YAGGG@Z"(
+// NO_HALF: call i16 @llvm.umin.i16(
+uint16_t test_min_ushort ( uint16_t p0, uint16_t p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <2 x i16> @
+// CHECK: call <2 x i16> @llvm.umin.v2i16
+// NO_HALF: define noundef <2 x i16> @"?test_min_ushort2@@YAT?$__vector@G$01@__clang@@T12@0@Z"(
+// NO_HALF: call <2 x i16> @llvm.umin.v2i16(
+uint16_t2 test_min_ushort2 ( uint16_t2 p0, uint16_t2 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <3 x i16> @
+// CHECK: call <3 x i16> @llvm.umin.v3i16
+// NO_HALF: define noundef <3 x i16> @"?test_min_ushort3@@YAT?$__vector@G$02@__clang@@T12@0@Z"(
+// NO_HALF: call <3 x i16> @llvm.umin.v3i16(
+uint16_t3 test_min_ushort3 ( uint16_t3 p0, uint16_t3 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <4 x i16> @
+// CHECK: call <4 x i16> @llvm.umin.v4i16
+// NO_HALF: define noundef <4 x i16> @"?test_min_ushort4@@YAT?$__vector@G$03@__clang@@T12@0@Z"(
+// NO_HALF: call <4 x i16> @llvm.umin.v4i16(
+uint16_t4 test_min_ushort4 ( uint16_t4 p0, uint16_t4 p1 ) {
+  return min ( p0, p1 );
+}
+#endif
+
+// CHECK: define noundef i32 @
+// CHECK: call i32 @llvm.smin.i32(
+int test_min_int ( int p0, int p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <2 x i32> @
+// CHECK: call <2 x i32> @llvm.smin.v2i32
+int2 test_min_int2 ( int2 p0, int2 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <3 x i32> @
+// CHECK: call <3 x i32> @llvm.smin.v3i32
+int3 test_min_int3 ( int3 p0, int3 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <4 x i32> @
+// CHECK: call <4 x i32> @llvm.smin.v4i32
+int4 test_min_int4 ( int4 p0, int4 p1) {
+  return min ( p0, p1 );
+}
+
+// CHECK: define noundef i32 @
+// CHECK: call i32 @llvm.umin.i32(
+int test_min_uint ( uint p0, uint p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <2 x i32> @
+// CHECK: call <2 x i32> @llvm.umin.v2i32
+uint2 test_min_uint2 ( uint2 p0, uint2 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <3 x i32> @
+// CHECK: call <3 x i32> @llvm.umin.v3i32
+uint3 test_min_uint3 ( uint3 p0, uint3 p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <4 x i32> @
+// CHECK: call <4 x i32> @llvm.umin.v4i32
+uint4 test_min_uint4 ( uint4 p0, uint4 p1) {
+  return min ( p0, p1 );
+}
+
+// CHECK: define noundef i64 @
+// CHECK: call i64 @llvm.smin.i64(
+int64_t test_min_long ( int64_t p0, int64_t p1 ) {
+  return min ( p0, p1 );
+}
+// CHECK: define noundef <2 x i64> @
+// CHECK: call <2 x i64> @llvm.smin.v2i64
+int64_t2 test_min_long2 ( int64_t2 p0, int64_t2 p1 ) {
+  

[clang] 7ac0551 - [HLSL] add max/min library functions

[Joshua Batista via cfe-commits]

Author: Joshua Batista
Date: 2023-03-03T09:31:50-08:00
New Revision: 7ac0551e77f4adab18f3ac6ae428d4c09f9b6c49

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

LOG: [HLSL] add max/min library functions

This change exposes the max and min library functions for HLSL, excluding long, 
and long long doubles.
The max / min functions are supported for all scalar, vector, and matrix types.
Long and long long double support is missing in this patch because those types
don't exist in HLSL.

The full documentation of the HLSL max / min functions are available here:
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-max
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-min

Reviewed By: python3kgae

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

Added: 
clang/test/CodeGenHLSL/builtins/max.hlsl
clang/test/CodeGenHLSL/builtins/min.hlsl

Modified: 
clang/lib/Headers/hlsl/hlsl_intrinsics.h

Removed: 




diff  --git a/clang/lib/Headers/hlsl/hlsl_intrinsics.h 
b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
index 570552367215b..1a34e1626e5a6 100644
--- a/clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ b/clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -308,5 +308,173 @@ double3 log10(double3);
 __attribute__((clang_builtin_alias(__builtin_elementwise_log10)))
 double4 log10(double4);
 
+// max builtins
+#ifdef __HLSL_ENABLE_16_BIT
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+half max(half, half);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+half2 max(half2, half2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+half3 max(half3, half3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+half4 max(half4, half4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int16_t max(int16_t, int16_t);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int16_t2 max(int16_t2, int16_t2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int16_t3 max(int16_t3, int16_t3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int16_t4 max(int16_t4, int16_t4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint16_t max(uint16_t, uint16_t);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint16_t2 max(uint16_t2, uint16_t2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint16_t3 max(uint16_t3, uint16_t3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint16_t4 max(uint16_t4, uint16_t4);
+#endif
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max))) int max(int,
+int);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int2 max(int2, int2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int3 max(int3, int3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int4 max(int4, int4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint max(uint, uint);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint2 max(uint2, uint2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint3 max(uint3, uint3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint4 max(uint4, uint4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int64_t max(int64_t, int64_t);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int64_t2 max(int64_t2, int64_t2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int64_t3 max(int64_t3, int64_t3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+int64_t4 max(int64_t4, int64_t4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint64_t max(uint64_t, uint64_t);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint64_t2 max(uint64_t2, uint64_t2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint64_t3 max(uint64_t3, uint64_t3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+uint64_t4 max(uint64_t4, uint64_t4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max))) float
+max(float, float);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+float2 max(float2, float2);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+float3 max(float3, float3);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+float4 max(float4, float4);
+
+__attribute__((clang_builtin_alias(__builtin_elementwise_max))) double
+max(double, double);
+__attribute__((clang_builtin_alias(__builtin_elementwise_max)))
+double2 max(double2, double2);

[clang] 34d6a6e - Fix bots by adding a triple to the test

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-03-03T12:26:52-05:00
New Revision: 34d6a6e23a6d8d78c86fad06bc219107ce6b5780

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

LOG: Fix bots by adding a triple to the test

Resolves the issue found by:
https://lab.llvm.org/buildbot/#/builders/245/builds/5384

Added: 


Modified: 
clang/test/C/C2x/n2838.c

Removed: 




diff  --git a/clang/test/C/C2x/n2838.c b/clang/test/C/C2x/n2838.c
index f110fef9d0f4..cd20ea59884b 100644
--- a/clang/test/C/C2x/n2838.c
+++ b/clang/test/C/C2x/n2838.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -std=c2x %s
+// RUN: %clang_cc1 -triple x86_64 -verify -std=c2x %s
 
 /* WG14 N2838: yes
  * Types and sizes



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

[PATCH] D144454: Add builtin for llvm set rounding

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, so if we're adding this builtin, and it's not just imitating a stdlib 
function, we should definitely document it as a language extension.  There's a 
section in the manual about controlling FP modes which is probably an 
appropriate place for this.

I assume we need to document that the builtin has undefined behavior if it 
receives a value outside of the standard-defined set.  (We can't guarantee that 
it's a no-op or else we'll never be able to extend the standard-defined set.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144454

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

[PATCH] D145093: Add map info for dereference pointer.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7489-7493
+  if (UO && UO->getOpcode() == UO_Deref)
+if (isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()) ||
+isa(Last->getAssociatedExpression()))
+  IsVarDerefAssoWithArray = true;

What if we have something like:
```
typedef int (T)[3];

T** p;
map(**p)
```
? Shall it work? I.e. mapping of the whole array by dereferening the pointer to 
the array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D145093: Add map info for dereference pointer.

[Jennifer Yu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7487-7497
+  auto TNext = Next;
+  bool IsVarDerefAssoWithArray = false;
+  if (UO && UO->getOpcode() == UO_Deref)
+for (; TNext != CE; TNext = std::next(TNext))
+  if (isa(TNext->getAssociatedExpression()) ||
+  isa(TNext->getAssociatedExpression()) ||
+  isa(TNext->getAssociatedExpression()) ||

ABataev wrote:
> jyu2 wrote:
> > ABataev wrote:
> > > jyu2 wrote:
> > > > ABataev wrote:
> > > > > Why need a loop here? Can you somehow merge analysis for (*p) 
> > > > > expression with the pointer subscript analysis?
> > > > What about if you have three dereference pointers like (***a)[0:3]  or 
> > > > four pointers...
> > > Why you can't iterate through the required components just like for the 
> > > array subscrit expression?
> > Because currently, for unarray operator the map info is skipped following 
> > the rule:
> > 
> > bool IsNonDerefPointer = IsPointer && !UO && !BO && !IsNonContiguous;
> > 
> > So I need to check if it is dereference to array some how to not skip it.
> 1. Same problem in Sema too.
> 2. Can you try to fix it to avoid those loops?
Thanks.  I removed loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

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

[PATCH] D145093: Add map info for dereference pointer.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 502144.
jyu2 added a comment.

Thanks Alexey for the code review.  I removed loop by using last expression in 
the component for checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145093

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_deref_array_codegen.cpp
  openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp

Index: openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp
@@ -0,0 +1,33 @@
+// RUN: %libomptarget-compilexx-generic -fopenmp-version=51
+// RUN: %libomptarget-run-generic 2>&1 \
+// RUN: | %fcheck-generic
+
+#include 
+#include 
+
+void foo(int **t1d) {
+  int ***t2d = 
+  int t3d = 
+  *t1d = (int *)malloc(3 * sizeof(int));
+  int j;
+
+  for (j = 0; j < 3; j++)
+(*t1d)[j] = 0;
+#pragma omp target map(tofrom : (*t1d)[0 : 3])
+  { (*t1d)[1] = 1; }
+  // CHECK: 1
+  printf("%d\n", (*t1d)[1]);
+#pragma omp target map(tofrom : (**t2d)[0 : 3])
+  { (**t2d)[1] = 2; }
+  // CHECK: 2
+  printf("%d\n", (**t2d)[1]);
+#pragma omp target map(tofrom : (***t3d)[0 : 3])
+  { (***t3d)[1] = 3; }
+  // CHECK: 3
+  printf("%d\n", (***t3d)[1]);
+}
+
+int main() {
+  int *data = 0;
+  foo();
+}
Index: clang/test/OpenMP/target_map_deref_array_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_map_deref_array_codegen.cpp
@@ -0,0 +1,131 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+extern void *malloc (int __size) throw () __attribute__ ((__malloc__));
+
+void foo(int **t1d)
+{
+  *t1d = (int *) malloc(3 * sizeof(int));
+  for (int j=0; j < 3; j++)
+(*t1d)[j] = 1;
+  #pragma omp target map(to: (*t1d)[0:3])
+(*t1d)[2] = 2;
+}
+
+#endif
+
+// CHECK: @.offload_sizes = private unnamed_addr constant [2 x i64] [i64 8, i64 12]
+// CHECK: @.offload_maptypes = private unnamed_addr constant [2 x i64] [i64 33, i64 17]
+// CHECK-LABEL: define {{[^@]+}}@_Z3fooPPi
+// CHECK-SAME: (ptr noundef [[T1D:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[T1D_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:store ptr [[T1D]], ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:[[CALL:%.*]] = call noalias noundef ptr @_Z6malloci(i32 noundef signext 12) #[[ATTR3:[0-9]+]]
+// CHECK-NEXT:[[TMP0:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:store ptr [[CALL]], ptr [[TMP0]], align 8
+// CHECK-NEXT:store i32 0, ptr [[J]], align 4
+// CHECK-NEXT:br label [[FOR_COND:%.*]]
+// CHECK:   for.cond:
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:[[CMP:%.*]] = icmp slt i32 [[TMP1]], 3
+// CHECK-NEXT:br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:   for.body:
+// CHECK-NEXT:[[TMP2:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:[[TMP3:%.*]] = load ptr, ptr [[TMP2]], align 8
+// CHECK-NEXT:[[TMP4:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:[[IDXPROM:%.*]] = sext i32 [[TMP4]] to i64
+// CHECK-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP3]], i64 [[IDXPROM]]
+// CHECK-NEXT:store i32 1, ptr [[ARRAYIDX]], align 4
+// CHECK-NEXT:br label [[FOR_INC:%.*]]
+// CHECK:   for.inc:
+// CHECK-NEXT:[[TMP5:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:[[INC:%.*]] = add nsw i32 [[TMP5]], 1
+// CHECK-NEXT:store i32 [[INC]], ptr [[J]], align 4
+// CHECK-NEXT:br label [[FOR_COND]], !llvm.loop [[LOOP4:![0-9]+]]
+// CHECK:   for.end:
+// CHECK-NEXT:[[TMP6:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:[[TMP7:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// 

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 502153.
balazske added a comment.

rebase, reformatted code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143751

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -65,47 +65,68 @@
   class Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
-  /// to us. If he doesn't, he performs additional invalidations.
-  enum InvalidationKind { NoEvalCall, EvalCallAsPure };
+  /// to us.
+  enum InvalidationKind {
+/// No \c eval::Call for the function, it can be modeled elsewhere.
+/// This checker checks only pre and post conditions.
+NoEvalCall,
+/// The function is modeled completely in this checker.
+EvalCallAsPure
+  };
 
-  // The universal integral type to use in value range descriptions.
-  // Unsigned to make sure overflows are well-defined.
+  /// Given a range, should the argument stay inside or outside this range?
+  enum RangeKind { OutOfRange, WithinRange };
+
+  static RangeKind negateKind(RangeKind K) {
+switch (K) {
+case OutOfRange:
+  return WithinRange;
+case WithinRange:
+  return OutOfRange;
+}
+llvm_unreachable("Unknown range kind");
+  }
+
+  /// The universal integral type to use in value range descriptions.
+  /// Unsigned to make sure overflows are well-defined.
   typedef uint64_t RangeInt;
 
-  /// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
-  /// a non-negative integer, which less than 5 and not equal to 2. For
-  /// `ComparesToArgument', holds information about how exactly to compare to
-  /// the argument.
+  /// Describes a single range constraint. Eg. {{0, 1}, {3, 4}} is
+  /// a non-negative integer, which less than 5 and not equal to 2.
   typedef std::vector> IntRangeVector;
 
   /// A reference to an argument or return value by its number.
   /// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNo;
+  /// Special argument number for specifying the return value.
   static const ArgNo Ret;
 
   using DescString = SmallString<96>;
+
   /// Returns the string representation of an argument index.
   /// E.g.: (1) -> '1st arg', (2) - > '2nd arg'
   static SmallString<8> getArgDesc(ArgNo);
   /// Append textual description of a numeric range [RMin,RMax] to the string
-  /// 'Out'.
+  /// \p Out.
   static void appendInsideRangeDesc(llvm::APSInt RMin, llvm::APSInt RMax,
 QualType ArgT, BasicValueFactory ,
 DescString );
   /// Append textual description of a numeric range out of [RMin,RMax] to the
-  /// string 'Out'.
+  /// string \p Out.
   static void appendOutOfRangeDesc(llvm::APSInt RMin, llvm::APSInt RMax,
QualType ArgT, BasicValueFactory ,
DescString );
 
   class ValueConstraint;
 
-  // Pointer to the ValueConstraint. We need a copyable, polymorphic and
-  // default initialize able type (vector needs that). A raw pointer was good,
-  // however, we cannot default initialize that. unique_ptr makes the Summary
-  // class non-copyable, therefore not an option. Releasing the copyability
-  // requirement would render the initialization of the Summary map infeasible.
+  /// Pointer to the ValueConstraint. We need a copyable, polymorphic and
+  /// default initializable type (vector needs that). A raw pointer was good,
+  /// however, we cannot default initialize that. unique_ptr makes the Summary
+  /// class non-copyable, therefore not an option. Releasing the copyability
+  /// requirement would render the initialization of the Summary map infeasible.
+  /// Mind that a pointer to a new value constraint is created when the negate
+  /// function is used.
   using ValueConstraintPtr = std::shared_ptr;
 
   /// Polymorphic base class that represents a constraint on a given argument
@@ -122,35 +143,12 @@
   public:
 ValueConstraint(ArgNo ArgN) : ArgN(ArgN) {}
 virtual ~ValueConstraint() {}
+
 /// Apply the effects of the constraint on the given program state. If null
 /// is returned then the constraint is not feasible.
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ,
   CheckerContext ) const = 0;
-virtual ValueConstraintPtr negate() const {
-  llvm_unreachable("Not implemented");
-};
-
-/// Check whether the constraint is malformed or not. It is malformed 

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Julian Lettner via Phabricator via cfe-commits]

yln added inline comments.



Comment at: 
compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 

usama54321 wrote:
> yln wrote:
> > zixuw wrote:
> > > dmaclach wrote:
> > > > yln wrote:
> > > > > This should work, right?
> > > > No.. darwin should fail with the `-static-libsan` flag. This is the 
> > > > test that was failing and caused the rollback.
> > > I think @yln is suggesting using `REQUIRES: asan-static-runtime` instead 
> > > of `XFAIL: darwin`. I wasn't aware of that conditional but yeah that 
> > > should be better if it works.
> > I meant using `// REQUIRES: asan-static-runtime ` instead of `XFAIL: 
> > darwin` since it seems that we already have a lit feature for it.
> I think UNSUPPORTED: darwin makes the most sense here. I don't think lit 
> understands that REQUIRES: asan-static-runtime should result in skipping the 
> test on Darwin as it does not know about this dependency.
> I don't think lit understands that REQUIRES: asan-static-runtime should 
> result in skipping the test on Darwin as it does not know about this 
> dependency.

Actually, this was exactly my point.  We have other tests already marked with 
`REQUIRES: asan-static-runtime` and we should double check our changes don't 
affect these as well.

If LIT doesn't model this dependency yet, then we should make sure it does!  
And this test can act as a good "canary in the coal mine".

Please use `REQUIRES: asan-static-runtime` and make sure we understand and deal 
with any fallout.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon added a comment.

The new update adds the suggested new tests! Thank you very much @awarzynski 
for suggesting them that was a great help. Hopefully the new additions cover 
what you have requested, if not I am of course happy to change or add more 
tests as needed.




Comment at: clang/lib/Driver/ToolChains/Flang.cpp:121-125
+  if (IsOpenMPDevice) {
+// -fopenmp-is-device is passed along to tell the frontend that it is
+// generating code for a device, so that only the relevant declarations are
+// emitted.
+CmdArgs.push_back("-fopenmp-is-device");

awarzynski wrote:
> Can you add a test for this section?
I have added a new test file flang-omp.f90 which aims to test that the 
appropriate flags are generated when openmp related flags are used it currently 
checks for -fopenmp/fopenmp-is-device



Comment at: flang/test/Lower/OpenMP/omp-is-device.f90:1
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s 
--check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST

awarzynski wrote:
> What happens if `-fopenmp-is-device` is used without `-fopnemp`?
I think I now cover this in the test with the *-DEVICE-FLAG-ONLY tests, 
currently it is silently ignored and nothing is applied to the module.



Comment at: flang/tools/bbc/bbc.cpp:127
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),

awarzynski wrote:
> This option is not tested
I believe I now test this in the omp-is-device.f90 test!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D145251: [clang] Treat function parameter scope as an immediate function context

[Mariya Podchishchaeva via Phabricator via cfe-commits]

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

This results in expressions that appear in default function argument not
being checked for being actual constant expressions.
This aligns clang's behavior with the standard and fixes one of the
examples from https://wg21.link/P1073R3.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145251

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.const/p6-2a.cpp


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared 
here}} \
- expected-note {{pointer 
to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+static_assert(r == 42);
 } // namespace P1073R3
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5918,7 +5918,10 @@
"default argument expression has capturing blocks?");
   }
   EnterExpressionEvaluationContext EvalContext(
-  *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+  *this,
+  FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+  Param);
   ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
   SkipImmediateInvocations;
   MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables*/ true);
@@ -6006,7 +6009,11 @@
 // If we are instantiating a template we won't have to
 // retransform immediate calls.
 EnterExpressionEvaluationContext EvalContext(
-*this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+*this,
+FD->isConsteval()
+? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+Param);
 
 if (Param->hasUninstantiatedDefaultArg()) {
   if (InstantiateDefaultArgument(CallLoc, FD, Param))


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared here}} \
- expected-note {{pointer to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval declaration is not a constant expression}}
+static_assert(r == 42);
 } // 

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

ping


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

https://reviews.llvm.org/D142907

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

[clang] 637ce0f - [C2x] Claim support for WG14 N2838

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-03-03T11:57:21-05:00
New Revision: 637ce0f7139015735d7e50cf12ad312a382f6283

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

LOG: [C2x] Claim support for WG14 N2838

This paper clarifies that complete object types need to be smaller than
SIZE_MAX. We already conformed to that requirement, so this adds some
test coverage to prove it.

Added: 
clang/test/C/C2x/n2838.c

Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/test/C/C2x/n2838.c b/clang/test/C/C2x/n2838.c
new file mode 100644
index 0..f110fef9d0f41
--- /dev/null
+++ b/clang/test/C/C2x/n2838.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -verify -std=c2x %s
+
+/* WG14 N2838: yes
+ * Types and sizes
+ */
+
+char buffer4[0x''''1wb]; /* expected-error {{array is too 
large (295147905179352825841 elements)}} */
+char buffer3[0x'''wb];   /* expected-error {{array is too 
large (18446744073709551615 elements)}} */
+char buffer2[0x7FFF'''wb];   /* expected-error {{array is too 
large (9223372036854775807 elements)}} */
+char buffer1[0x1FFF'''wb];   /* array is juust right */
+
+/* The largest object we can create is still smaller than SIZE_MAX. */
+static_assert(0x1FFF'''wb <= __SIZE_MAX__);

diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index dcc02e8b05145..20eec37fce3de 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -972,7 +972,7 @@ C2x implementation status
 
   Types and sizes
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2838.htm;>N2838
-  Unknown
+  Yes
 
 
   Clarifying integer terms



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

[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 502143.
balazske added a comment.

changed comments, small variable rename


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143751

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -65,47 +65,68 @@
   class Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
-  /// to us. If he doesn't, he performs additional invalidations.
-  enum InvalidationKind { NoEvalCall, EvalCallAsPure };
+  /// to us.
+  enum InvalidationKind {
+/// No \c eval::Call for the function, it can be modeled elsewhere.
+/// This checker checks only pre and post conditions.
+NoEvalCall,
+/// The function is modeled completely in this checker.
+EvalCallAsPure
+  };
 
-  // The universal integral type to use in value range descriptions.
-  // Unsigned to make sure overflows are well-defined.
+  /// Given a range, should the argument stay inside or outside this range?
+  enum RangeKind { OutOfRange, WithinRange };
+
+  static RangeKind negateKind(RangeKind K) {
+switch (K) {
+case OutOfRange:
+  return WithinRange;
+case WithinRange:
+  return OutOfRange;
+}
+llvm_unreachable("Unknown range kind");
+  }
+
+  /// The universal integral type to use in value range descriptions.
+  /// Unsigned to make sure overflows are well-defined.
   typedef uint64_t RangeInt;
 
-  /// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
-  /// a non-negative integer, which less than 5 and not equal to 2. For
-  /// `ComparesToArgument', holds information about how exactly to compare to
-  /// the argument.
+  /// Describes a single range constraint. Eg. {{0, 1}, {3, 4}} is
+  /// a non-negative integer, which less than 5 and not equal to 2.
   typedef std::vector> IntRangeVector;
 
   /// A reference to an argument or return value by its number.
   /// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
   /// obviously uint32_t should be enough for all practical purposes.
   typedef uint32_t ArgNo;
+  /// Special argument number for specifying the return value.
   static const ArgNo Ret;
 
   using DescString = SmallString<96>;
+
   /// Returns the string representation of an argument index.
   /// E.g.: (1) -> '1st arg', (2) - > '2nd arg'
   static SmallString<8> getArgDesc(ArgNo);
   /// Append textual description of a numeric range [RMin,RMax] to the string
-  /// 'Out'.
+  /// \p Out.
   static void appendInsideRangeDesc(llvm::APSInt RMin, llvm::APSInt RMax,
 QualType ArgT, BasicValueFactory ,
 DescString );
   /// Append textual description of a numeric range out of [RMin,RMax] to the
-  /// string 'Out'.
+  /// string \p Out.
   static void appendOutOfRangeDesc(llvm::APSInt RMin, llvm::APSInt RMax,
QualType ArgT, BasicValueFactory ,
DescString );
 
   class ValueConstraint;
 
-  // Pointer to the ValueConstraint. We need a copyable, polymorphic and
-  // default initialize able type (vector needs that). A raw pointer was good,
-  // however, we cannot default initialize that. unique_ptr makes the Summary
-  // class non-copyable, therefore not an option. Releasing the copyability
-  // requirement would render the initialization of the Summary map infeasible.
+  /// Pointer to the ValueConstraint. We need a copyable, polymorphic and
+  /// default initializable type (vector needs that). A raw pointer was good,
+  /// however, we cannot default initialize that. unique_ptr makes the Summary
+  /// class non-copyable, therefore not an option. Releasing the copyability
+  /// requirement would render the initialization of the Summary map infeasible.
+  /// Mind that a pointer to a new value constraint is created when the negate
+  /// function is used.
   using ValueConstraintPtr = std::shared_ptr;
 
   /// Polymorphic base class that represents a constraint on a given argument
@@ -122,35 +143,12 @@
   public:
 ValueConstraint(ArgNo ArgN) : ArgN(ArgN) {}
 virtual ~ValueConstraint() {}
+
 /// Apply the effects of the constraint on the given program state. If null
 /// is returned then the constraint is not feasible.
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
   const Summary ,
   CheckerContext ) const = 0;
-virtual ValueConstraintPtr negate() const {
-  llvm_unreachable("Not implemented");
-};
-
-/// Check whether the constraint is malformed or not. 

[clang] 4679d7a - [NFC][ARM][AArch64] Cleanup TargetParser includes

[Archibald Elliott via cfe-commits]

Author: Archibald Elliott
Date: 2023-03-03T16:24:55Z
New Revision: 4679d7a26a5f42f1128fac6fdf49488d13066c52

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

LOG: [NFC][ARM][AArch64] Cleanup TargetParser includes

llvm/TargetParser/TargetParser.h now only includes AMDGPU-specific
functionality, the ARM- and AArch64-specific functionality is in other
headers.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
llvm/lib/Target/ARM/ARMAsmPrinter.cpp
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMMCTargetDesc.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMTargetStreamer.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 519da92b09417..0b2ee72f7868e 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -14,7 +14,6 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/TargetParser/Host.h"
-#include "llvm/TargetParser/TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 0967ffe047579..e75f8a474410c 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -14,7 +14,6 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/TargetParser/ARMTargetParser.h"
 #include "llvm/TargetParser/Host.h"
-#include "llvm/TargetParser/TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.h 
b/clang/lib/Driver/ToolChains/Arch/ARM.h
index ef56c8b065df7..18a358c55c1b4 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -13,7 +13,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Option/Option.h"
 #include "llvm/TargetParser/ARMTargetParser.h"
-#include "llvm/TargetParser/TargetParser.h"
 #include "llvm/TargetParser/Triple.h"
 #include 
 #include 

diff  --git a/llvm/lib/Target/AArch64/AArch64Subtarget.cpp 
b/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
index 6241b145f9b50..843c3b78f8758 100644
--- a/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -25,7 +25,6 @@
 #include "llvm/CodeGen/MachineScheduler.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
-#include "llvm/TargetParser/TargetParser.h"
 
 using namespace llvm;
 

diff  --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp 
b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 8393fb1aa9aba..4f2833c1c4230 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -50,7 +50,6 @@
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/AArch64TargetParser.h"
-#include "llvm/TargetParser/TargetParser.h"
 #include 
 #include 
 #include 

diff  --git a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp 
b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
index 7cd7ee7543a0e..5ed8f900fb88f 100644
--- a/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
+++ b/llvm/lib/Target/ARM/ARMAsmPrinter.cpp
@@ -46,7 +46,6 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
-#include "llvm/TargetParser/TargetParser.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"

diff  --git a/llvm/lib/Target/ARM/ARMSubtarget.cpp 
b/llvm/lib/Target/ARM/ARMSubtarget.cpp
index 9cc9056f408b7..53c03a6b28d7e 100644
--- a/llvm/lib/Target/ARM/ARMSubtarget.cpp
+++ b/llvm/lib/Target/ARM/ARMSubtarget.cpp
@@ -36,7 +36,6 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/TargetParser/ARMTargetParser.h"
-#include "llvm/TargetParser/TargetParser.h"
 #include "llvm/TargetParser/Triple.h"
 
 using namespace llvm;

diff  --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp 
b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index d694385098583..bf419511b 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -35,7 +35,6 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
-#include 

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 502139.
agozillon added a comment.

- [Flang][MLIR][Test] Add newline at end of test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,19 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-DEVICE
+!RUN: bbc -fopenmp -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-HOST
+!RUN: bbc -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-DEVICE-FLAG-ONLY
+
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = 

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Went with unsupported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

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

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 502138.
dmaclach marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/asan/TestCases/replaceable_new_delete.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp

Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
===
--- /dev/null
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
@@ -0,0 +1,35 @@
+// Ensure that operator new/delete are still replaceable using static-libsan.
+
+// FIXME: Weak symbols aren't supported on Windows, although some code in
+// compiler-rt already exists to solve this problem. We should probably define
+// the new/delete interceptors as "weak" using those workarounds as well.
+// UNSUPPORTED: target={{.*windows.*}}
+
+// darwin only supports `shared-libsan`.
+// UNSUPPORTED: darwin
+
+// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
+
+#include 
+#include 
+#include 
+
+void *operator new[](size_t size) {
+  fprintf(stderr, "replaced new\n");
+  return malloc(size);
+}
+
+void operator delete[](void *ptr) noexcept {
+  fprintf(stderr, "replaced delete\n");
+  return free(ptr);
+}
+
+int main(int argc, char **argv) {
+  // CHECK: replaced new
+  char *x = new char[5];
+  // CHECK: replaced delete
+  delete[] x;
+  // CHECK: ERROR: AddressSanitizer
+  *x = 13;
+  return 0;
+}
Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
===
--- compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
@@ -1,4 +1,4 @@
-// Ensure that operator new/delete are still replaceable.
+// Ensure that operator new/delete are still replaceable using shared-libsan.
 
 // FIXME: Weak symbols aren't supported on Windows, although some code in
 // compiler-rt already exists to solve this problem. We should probably define
@@ -6,7 +6,6 @@
 // UNSUPPORTED: target={{.*windows.*}}
 
 // RUN: %clangxx %s -o %t -fsanitize=address -shared-libsan && not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
 
 #include 
 #include 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs  = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer 

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang-c/Index.h:329
+ * CXIndexOptions Opts = { sizeof(CXIndexOptions),
+ * clang_getDefaultGlobalOptions() };
+ * \endcode

vedgy wrote:
> aaron.ballman wrote:
> > vedgy wrote:
> > > vedgy wrote:
> > > > aaron.ballman wrote:
> > > > > vedgy wrote:
> > > > > > When I almost finished the requested changes, I remembered that the 
> > > > > > return value of `clang_getDefaultGlobalOptions()` depends on 
> > > > > > environment variables, and thus `0` is not necessarily the default. 
> > > > > > Adjusted the changes and updated this revision.
> > > > > > 
> > > > > > Does the extra requirement to non-zero initialize this second 
> > > > > > member sway your opinion on the usefulness of the helper function 
> > > > > > `inline CXIndexOptions clang_getDefaultIndexOptions()`? Note that 
> > > > > > there may be same (environment) or other important reasons why 
> > > > > > future new options couldn't be zeroes by default.
> > > > > Thinking out loud a bit... (potentially bad idea incoming)
> > > > > 
> > > > > What if we dropped `clang_getDefaultGlobalOptions()` and instead made 
> > > > > a change to `CXGlobalOptFlags`:
> > > > > ```
> > > > > typedef enum {
> > > > >   /**
> > > > >* Used to indicate that the default CXIndex options are used. By 
> > > > > default, no
> > > > >* global options will be used. However, environment variables may 
> > > > > change which
> > > > >* global options are in effect at runtime.
> > > > >*/
> > > > >   CXGlobalOpt_Default = 0x0,
> > > > > 
> > > > >   /**
> > > > >* Used to indicate that threads that libclang creates for indexing
> > > > >* purposes should use background priority.
> > > > >*
> > > > >* Affects #clang_indexSourceFile, #clang_indexTranslationUnit,
> > > > >* #clang_parseTranslationUnit, #clang_saveTranslationUnit.
> > > > >*/
> > > > >   CXGlobalOpt_ThreadBackgroundPriorityForIndexing = 0x1,
> > > > > 
> > > > >   /**
> > > > >* Used to indicate that threads that libclang creates for editing
> > > > >* purposes should use background priority.
> > > > >*
> > > > >* Affects #clang_reparseTranslationUnit, #clang_codeCompleteAt,
> > > > >* #clang_annotateTokens
> > > > >*/
> > > > >   CXGlobalOpt_ThreadBackgroundPriorityForEditing = 0x2,
> > > > > 
> > > > >   /**
> > > > >* Used to indicate that all threads that libclang creates should 
> > > > > use
> > > > >* background priority.
> > > > >*/
> > > > >   CXGlobalOpt_ThreadBackgroundPriorityForAll =
> > > > >   CXGlobalOpt_ThreadBackgroundPriorityForIndexing |
> > > > >   CXGlobalOpt_ThreadBackgroundPriorityForEditing,
> > > > > 
> > > > >   /**
> > > > >* Used to indicate that no global options should be used, even
> > > > >* in the presence of environment variables.
> > > > >*/
> > > > >   CXGlobalOpt_None = 0x
> > > > > } CXGlobalOptFlags;
> > > > > ```
> > > > > so that when the user passes `0` they get the previous behavior.
> > > > > 
> > > > > `clang_CXIndex_setGlobalOptions()` would remain deprecated. 
> > > > > `clang_CXIndex_getGlobalOptions()` would be interesting though -- 
> > > > > would it return `CXGlobalOpt_None` or `CXGlobalOpt_Default` in the 
> > > > > event the index was created without any global options? Hmmm.
> > > > > 
> > > > > Err, actually, I suppose this won't work too well because then code 
> > > > > silently changes behavior if it does 
> > > > > `clang_CXIndex_setGlobalOptions(CXGlobalOpt_None);` because that 
> > > > > would change from "do what the environment says" to "ignore the 
> > > > > environment". But I have to wonder whether anyone actually *does* 
> > > > > that or not... my intuition is that folks would not call 
> > > > > `clang_CXIndex_setGlobalOptions()` at all unless they were setting an 
> > > > > option to a non-none value. We could rename `CXGlobalOpt_None` to 
> > > > > `CXGlobalOpt_Nothing` (or something along those lines) to force a 
> > > > > compilation error, but that's a bit of a nuclear option for what's 
> > > > > supposed to be a stable API.
> > > > > 
> > > > > So I'm on the fence, I guess. I'd still prefer for zero to give 
> > > > > sensible defaults and I don't think there's enough use of the global 
> > > > > options + environment variables to matter. But I also don't like 
> > > > > silently breaking code, so my idea above may be a nonstarter.
> > > > > 
> > > > > I suppose another possible idea is: deprecate the notion of global 
> > > > > options enum and setter/getter entirely, add two new fields to 
> > > > > `CXIndexOptions`:
> > > > > ```
> > > > > typedef enum {
> > > > >   CXChoice_Default = 0,
> > > > >   CXChoice_Enabled = 1,
> > > > >   CXChoice_Disabled = 2
> > > > > } CXChoice;
> > > > > 
> > > > > ...
> > > > > unsigned ThreadPriorityBackgroundForIndexing;
> > > > > unsigned 

[PATCH] D145229: [analyzer] Improve the documentation of the alpha.security.taint.TaintPropagation checker

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:78-80
+The ``SuppressAddressSpaces`` option suppresses
 warnings for null dereferences of all pointers with address spaces. You can
 disable this behavior with the option

Why is this paragraph (and the one above it) wrapped inconsistently? If we are 
touching these docs, perhaps we could re-wrap them to e.g 80 characters / line.



Comment at: clang/docs/analyzer/checkers.rst:2385
 
   - network originating data
+  - files or standard input

This old word choice is awkward, consider replacing it with e.g. "data from the 
network".



Comment at: clang/docs/analyzer/checkers.rst:2388
   - environment variables
   - database originating data
 

Why not just "databases"?



Comment at: clang/docs/analyzer/checkers.rst:2404
+  }
+  strcat(cmd, filename);
+  system(cmd); // Warning: Untrusted data is passed to a system call

If the filename is too long (more than 1014 characters), this is a buffer 
overflow. I admit that having a secondary unrelated vulnerability makes the 
example more realistic :), but I think we should still avoid it. (This also 
appears in other variants of the example code, including the "No vulnerability 
anymore" one.)



Comment at: clang/docs/analyzer/checkers.rst:2426
+
+  if (access(filename,F_OK)){//sanitizing user input
+printf("File does not exist\n");

Nitpick: the comment formatting is inconsistent: for example, this is lowercase 
while most others start with an uppercase letter, or half of the comments have 
a space after the `//` while the others don't.



Comment at: clang/docs/analyzer/checkers.rst:2457-2461
+  if (access(filename,F_OK)){//sanitizing user input
+printf("File does not exist\n");
+return -1;
+  }
+  csa_sanitize(filename); // Indicating to CSA that filename variable is safe 
to be used after this point

Separating the actual sanitization and the function that's magically recognized 
by the taint checker doesn't seem to be a good design pattern. Here 
`csa_sanitize()` is just a synonym for the "silence this checker here" marker, 
which is //very// confusing, because if someone is not familiar with this 
locally introduced no-op function, they will think that it's performing actual 
sanitization! At the very least we should rename this magical no-op to 
`csa_mark_sanitized()` or something similar.

The root issue is that in this example we would like to use a verifier function 
(that determines whether the tainted data is safe) instead of a sanitizer 
function (that can convert //any// tainted data into safe data) and our taint 
handling engine is not prepared to handle conditional Filter effects like "this 
function removes taint from its first argument //if its return value is true//".

I think it would be much better to switch to a different example where the 
"natural" solution is more aligned with the limited toolbox provided by our 
taint framework (i.e. it's possible define a filter function that actually 
removes problematic parts of the untrusted input).



Comment at: clang/docs/analyzer/checkers.rst:2505
 
-There are built-in sources, propagations and sinks defined in code inside 
``GenericTaintChecker``.
-These operations are handled even if no external taint configuration is 
provided.
+There are built-in sources, propagations and sinks even if no external taint 
configuration is provided.
 

Perhaps explicitly mention that there are no built-in filters.



Comment at: clang/docs/analyzer/checkers.rst:2535
+
+* The taintedness property is not propagated through function calls which are 
unkown (or too complex) to the analyzer, unless there is a specific
+propagation rule built-in to the checker or given in the YAML configuration 
file. This causes potential true positive findings to be lost.

Spellcheck: "unknown"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145229

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

[Andrew Gozillon via Phabricator via cfe-commits]

agozillon updated this revision to Diff 502134.
agozillon added a comment.

- [MLIR][OpenMP] Fix attribute helpers to apply to more than builtin.module
- [Flang][mlir] Adding space at the end of the omp-is-device test file
- [Clang][Flang][Driver][Test] Add flang-omp.f90 to test 
fopenmp/fopenmp-is-device for flang driver-mode
- [Flang][MLIR][OpenMP] Add additional tests to omp-is-device.f90 test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/test/Driver/flang/flang-omp.f90
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,19 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=BBC-DEVICE
+!RUN: bbc -fopenmp -emit-fir 

[PATCH] D127259: [CodeGen] guarantee templated static variables are initialized in the reverse instantiation order

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:19907
+  if (Iter != I.end()) {
+SemaRef.PendingInstantiations.push_back(*Iter);
+I.erase(Iter);

Doesn't this invalidate the iterators? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127259

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

[PATCH] D145098: [clang][deps] Preserve input ordering in the full output

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added a comment.

Hi, this new test fails on AIX, could you take a look please?
https://lab.llvm.org/buildbot/#/builders/214/builds/6148/steps/6/logs/FAIL__Clang__modules-full-output-tu-order_c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145098

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

[PATCH] D144546: [clang][dataflow] Fix assert for CXXConstructExpr argument number

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

By looking at the title, I get the impression that this fixes an assertion 
violation.
I also observed that this commit is part of `main` but not part of 
`release/16.x`, hence the `clang-16` would be released without this fix.

I want to raise awareness of backporting crash fixes to llvm releases. IMO 
that's a good practice.
So my question is, should we backport this patch to the release branch?

If so, could you please check if there are more commits like this for the 
dataflow library @ymandel?

I used `git log release/16.x..main --oneline 
clang/lib/{Analysis,AST,ASTMatchers,StaticAnalyzer} 
clang/include/clang/{Analysis,AST,ASTMatchers,StaticAnalyzer}  | grep -i 
'crash\|fix\|assert'` to check for relevant commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144546

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

[PATCH] D142384: [C++20] Fix a crash with modules.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

@usaxena95 @ilya-biryukov, @hans, I wonder if we should backport this change to 
`release/16.x`. Otherwise, clang-16 won't have this fix. WDYT?
I'm also a bit worried that we don't have tests. And that we have "unexpected" 
sideeffects like what you mentioned @hans.

Given all these, I would probably vote for not backporting this, but this is 
the chance if we want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142384

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

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

@amyk thank you so much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144967

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

[PATCH] D142328: [clang][Interp] Fix compound assign operator types

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Hi @tbaeder, I was looking for commits that mentions "fix" "assertion" or 
"crash" in the title, that are part of the `main` branch but not backported to 
`release/16.x` to be eventually released as `clang-16`.
I wonder what's the status of this and the related patches given that the 
commit that the summary mentions (490e8214fca48824beda8b508d6d6bbbf3d8d9a7 
) is 
present in `release/16.x`. I can also see that quite a few revert commits are 
also related to `Interp`.
It's probably the case that the mentioned change was removed from `clang-16` by 
f6ea1af9a4b71d27de2dde629224af1220c5c85b 
.

Could you please confirm that the changes around `Interp` are consistent and no 
patches are missing from the `release/16.x` just to be sure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142328

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

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like there's a valid precommit CI failure that needs to be addressed:

  
  Failed Tests (1):
Clang :: Frontend/sarif-diagnostics.cpp




Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214
 void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) 
{
-  assert(false && "Not implemented in SARIF mode");
+  SarifRule Rule = SarifRule::create().setRuleId(std::to_string(-1));
+  Rule = addDiagnosticLevelToRule(Rule, DiagnosticsEngine::Level::Note);

Why do we want -1 as the rule ID and... can we use `"-1"` instead of doing a 
string conversion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145201

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

[PATCH] D145244: [clang-format] Add ability to trace tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek updated this revision to Diff 502123.
klimek added a comment.

Remove superfluous include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145244

Files:
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatDebug.cpp
  clang/lib/Format/FormatDebug.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp

Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "UnwrappedLineFormatter.h"
+#include "FormatDebug.h"
 #include "FormatToken.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "WhitespaceManager.h"
@@ -1252,17 +1253,43 @@
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
bool NewLine, unsigned *Count, QueueType *Queue) {
-if (NewLine && !Indenter->canBreak(PreviousNode->State))
+bool MustBreak = false;
+bool MustNotBreak = false;
+LLVM_DEBUG({
+  if (internal::DebugForceMustBreak().match(
+  PreviousNode->State.NextToken->TokenText)) {
+MustBreak = true;
+  }
+  if (internal::DebugForceMustNotBreak().match(
+  PreviousNode->State.NextToken->TokenText)) {
+MustNotBreak = true;
+  }
+});
+
+if (NewLine && (!Indenter->canBreak(PreviousNode->State) || MustNotBreak) &&
+!MustBreak) {
+  DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+   "Cannot break before");
   return;
-if (!NewLine && Indenter->mustBreak(PreviousNode->State))
+}
+if (!NewLine && (Indenter->mustBreak(PreviousNode->State) || MustBreak) &&
+!MustNotBreak) {
+  DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+   "Must break before");
   return;
+}
 
 StateNode *Node = new (Allocator.Allocate())
 StateNode(PreviousNode->State, NewLine, PreviousNode);
 if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
   return;
 
-Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
+unsigned AddPenalty = Indenter->addTokenToState(Node->State, NewLine, true);
+DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Penalty for " << (!NewLine ? "not " : "")
+<< "breaking before token is "
+<< AddPenalty);
+Penalty += AddPenalty;
 
 Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
 ++(*Count);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -13,6 +13,8 @@
 //===--===//
 
 #include "TokenAnnotator.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
 #include "FormatToken.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
@@ -3244,6 +3246,7 @@
 
 const auto  = Prev->Children;
 if (!Children.empty() && Children.back()->Last->is(TT_LineComment)) {
+  DEBUG_FORMAT_TRACE_TOKEN(*Current, "MustBreakBefore");
   Current->MustBreakBefore = true;
 } else {
   Current->MustBreakBefore =
@@ -3252,6 +3255,8 @@
   Current->is(TT_FunctionDeclarationName)) {
 Current->MustBreakBefore = mustBreakForReturnType(Line);
   }
+  DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
 }
 
 Current->CanBreakBefore =
Index: clang/lib/Format/FormatDebug.h
===
--- /dev/null
+++ clang/lib/Format/FormatDebug.h
@@ -0,0 +1,41 @@
+//===--- FormatDebug.h - Format C++ code *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file declares utilities used in clang-format in debug mode.
+///
+//===--===//
+#ifndef LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+#define LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace format {
+namespace internal {
+
+#ifndef NDEBUG
+

[PATCH] D145244: [clang-format] Add ability to trace tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek created this revision.
klimek added a reviewer: sammccall.
Herald added a project: All.
klimek requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145244

Files:
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatDebug.cpp
  clang/lib/Format/FormatDebug.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp

Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "UnwrappedLineFormatter.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
 #include "FormatToken.h"
 #include "NamespaceEndCommentsFixer.h"
 #include "WhitespaceManager.h"
@@ -1252,17 +1254,43 @@
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
bool NewLine, unsigned *Count, QueueType *Queue) {
-if (NewLine && !Indenter->canBreak(PreviousNode->State))
+bool MustBreak = false;
+bool MustNotBreak = false;
+LLVM_DEBUG({
+  if (internal::DebugForceMustBreak().match(
+  PreviousNode->State.NextToken->TokenText)) {
+MustBreak = true;
+  }
+  if (internal::DebugForceMustNotBreak().match(
+  PreviousNode->State.NextToken->TokenText)) {
+MustNotBreak = true;
+  }
+});
+
+if (NewLine && (!Indenter->canBreak(PreviousNode->State) || MustNotBreak) &&
+!MustBreak) {
+  DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+   "Cannot break before");
   return;
-if (!NewLine && Indenter->mustBreak(PreviousNode->State))
+}
+if (!NewLine && (Indenter->mustBreak(PreviousNode->State) || MustBreak) &&
+!MustNotBreak) {
+  DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+   "Must break before");
   return;
+}
 
 StateNode *Node = new (Allocator.Allocate())
 StateNode(PreviousNode->State, NewLine, PreviousNode);
 if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
   return;
 
-Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
+unsigned AddPenalty = Indenter->addTokenToState(Node->State, NewLine, true);
+DEBUG_FORMAT_TRACE_TOKEN(*PreviousNode->State.NextToken,
+ "Penalty for " << (!NewLine ? "not " : "")
+<< "breaking before token is "
+<< AddPenalty);
+Penalty += AddPenalty;
 
 Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
 ++(*Count);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -13,6 +13,8 @@
 //===--===//
 
 #include "TokenAnnotator.h"
+#include "ContinuationIndenter.h"
+#include "FormatDebug.h"
 #include "FormatToken.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
@@ -3244,6 +3246,7 @@
 
 const auto  = Prev->Children;
 if (!Children.empty() && Children.back()->Last->is(TT_LineComment)) {
+  DEBUG_FORMAT_TRACE_TOKEN(*Current, "MustBreakBefore");
   Current->MustBreakBefore = true;
 } else {
   Current->MustBreakBefore =
@@ -3252,6 +3255,8 @@
   Current->is(TT_FunctionDeclarationName)) {
 Current->MustBreakBefore = mustBreakForReturnType(Line);
   }
+  DEBUG_FORMAT_TRACE_TOKEN(*Current, (Current->MustBreakBefore ? "" : "!")
+ << "MustBreakBefore");
 }
 
 Current->CanBreakBefore =
Index: clang/lib/Format/FormatDebug.h
===
--- /dev/null
+++ clang/lib/Format/FormatDebug.h
@@ -0,0 +1,41 @@
+//===--- FormatDebug.h - Format C++ code *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file declares utilities used in clang-format in debug mode.
+///
+//===--===//
+#ifndef LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+#define LLVM_CLANG_LIB_FORMAT_FORMATDEBUG_H
+
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace format {

[PATCH] D145178: [clang][NFC] reformats the SARIF diagnostic test so it's human readable

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, this is incremental progress. Hopefully we won't be adding too many more 
RUN lines to this file though (sticking too many tests into one file is also 
tech debt).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145178

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

[PATCH] D65956: clang: Diag running out of file handles while looking for files

[Axel Naumann via Phabricator via cfe-commits]

karies added a comment.
Herald added a project: All.

Similar to the concern raised at 
https://github.com/llvm/llvm-project/commit/50fcf7285eeb001d751eadac5d001a0e216fd701
 we have received user reports about this patch: with `-Ino-access-permissions 
-Iall-good`, clang will throw an error (EACCES) even though header search goes 
on and will find the header in `all-good`. That seems a misleading an 
unnecessary error, especially as the header *is* found later, yet compilation 
"fails" because of this diagnostic.

I would propose to collect these errors, and where originally (before this 
patch), cling would complain "file not found" we could diagnose "while 
searching for this header, the following errors have been seen" and reporting 
the uniq-ed set of collected diags - but *only* in the case where the file 
cannot be found. This would prevent spurious diags during iteration through the 
SearchDirs when HeaderSearch actually finds the file. And I am fully aware that 
it's pointless to propose something without providing a differential :-/

FYI here's what we do right now to handle the EACCES case:

  patch
  --- a/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
  +++ b/interpreter/llvm/src/tools/clang/lib/Lex/HeaderSearch.cpp
  @@ -367,7 +367,9 @@ Optional 
HeaderSearch::getFileAndSuggestModule(
   std::error_code EC = llvm::errorToErrorCode(File.takeError());
   if (EC != llvm::errc::no_such_file_or_directory &&
   EC != llvm::errc::invalid_argument &&
  -EC != llvm::errc::is_a_directory && EC != 
llvm::errc::not_a_directory) {
  +EC != llvm::errc::is_a_directory &&
  +EC != llvm::errc::not_a_directory &&
  +EC != llvm::errc::permission_denied) {
 Diags.Report(IncludeLoc, diag::err_cannot_open_file)
 << FileName << EC.message();
   }


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65956

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

[PATCH] D145238: [NVPTX] Expose LDU builtins

[Jakub Chlanda via Phabricator via cfe-commits]

jchlanda created this revision.
jchlanda added a reviewer: tra.
Herald added subscribers: mattd, gchakrabarti, asavonic.
Herald added a project: All.
jchlanda requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jholewinski.
Herald added projects: clang, LLVM.

Also check if native half types are supported to give more descriptive error 
message, without it clang only reports incorrect intrinsic return type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145238

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
  clang/test/CodeGen/builtins-nvptx-native-half-type.c
  clang/test/CodeGen/builtins-nvptx.c
  llvm/test/CodeGen/NVPTX/ldu-ldg.ll

Index: llvm/test/CodeGen/NVPTX/ldu-ldg.ll
===
--- llvm/test/CodeGen/NVPTX/ldu-ldg.ll
+++ llvm/test/CodeGen/NVPTX/ldu-ldg.ll
@@ -3,7 +3,13 @@
 
 
 declare i8 @llvm.nvvm.ldu.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i32 @llvm.nvvm.ldu.global.i.i32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 %align)
+declare <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 %align)
 
 declare i8 @llvm.nvvm.ldg.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
@@ -21,6 +27,13 @@
   ret i8 %val
 }
 
+; CHECK: test_ldu_i16
+define i16 @test_ldu_i16(ptr addrspace(1) %ptr) {
+; ldu.global.u16
+  %val = tail call i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 4)
+  ret i16 %val
+}
+
 ; CHECK: test_ldu_i32
 define i32 @test_ldu_i32(ptr addrspace(1) %ptr) {
 ; ldu.global.u32
@@ -28,6 +41,41 @@
   ret i32 %val
 }
 
+; CHECK: test_ldu_i64
+define i64 @test_ldu_i64(ptr addrspace(1) %ptr) {
+; ldu.global.u64
+  %val = tail call i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret i64 %val
+}
+
+; CHECK: test_ldu_f32
+define float @test_ldu_f32(ptr addrspace(1) %ptr) {
+; ldu.global.u64
+  %val = tail call float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 4)
+  ret float %val
+}
+
+; CHECK: test_ldu_f64
+define double @test_ldu_f64(ptr addrspace(1) %ptr) {
+; ldu.global.u64
+  %val = tail call double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret double %val
+}
+
+; CHECK: test_ldu_f16
+define half @test_ldu_f16(ptr addrspace(1) %ptr) {
+; ldu.global.b16
+  %val = tail call half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ret half %val
+}
+
+; CHECK: test_ldu_v2f16
+define <2 x half> @test_ldu_v2f16(ptr addrspace(1) %ptr) {
+; ldu.global.b32
+  %val = tail call <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ret <2 x half> %val
+}
+
 ; CHECK: test_ldg_i8
 define i8 @test_ldg_i8(ptr addrspace(1) %ptr) {
 ; ld.global.nc.u8
Index: clang/test/CodeGen/builtins-nvptx.c
===
--- clang/test/CodeGen/builtins-nvptx.c
+++ clang/test/CodeGen/builtins-nvptx.c
@@ -652,6 +652,97 @@
   __nvvm_ldg_d2((const double2 *)p);
 }
 
+// CHECK-LABEL: nvvm_ldu
+__device__ void nvvm_ldu(const void *p) {
+  // CHECK: call i8 @llvm.nvvm.ldu.global.i.i8.p0(ptr {{%[0-9]+}}, i32 1)
+  // CHECK: call i8 @llvm.nvvm.ldu.global.i.i8.p0(ptr {{%[0-9]+}}, i32 1)
+  __nvvm_ldu_c((const char *)p);
+  __nvvm_ldu_uc((const unsigned char *)p);
+
+  // CHECK: call i16 @llvm.nvvm.ldu.global.i.i16.p0(ptr {{%[0-9]+}}, i32 2)
+  // CHECK: call i16 @llvm.nvvm.ldu.global.i.i16.p0(ptr {{%[0-9]+}}, i32 2)
+  __nvvm_ldu_s((const short *)p);
+  __nvvm_ldu_us((const unsigned short *)p);
+
+  // CHECK: call i32 @llvm.nvvm.ldu.global.i.i32.p0(ptr {{%[0-9]+}}, i32 4)
+  // CHECK: call i32 @llvm.nvvm.ldu.global.i.i32.p0(ptr {{%[0-9]+}}, i32 4)
+  __nvvm_ldu_i((const int *)p);
+  __nvvm_ldu_ui((const unsigned int *)p);
+
+  // LP32: call i32 @llvm.nvvm.ldu.global.i.i32.p0(ptr {{%[0-9]+}}, i32 4)
+  // LP32: call i32 @llvm.nvvm.ldu.global.i.i32.p0(ptr {{%[0-9]+}}, i32 4)
+  // LP64: call i64 @llvm.nvvm.ldu.global.i.i64.p0(ptr {{%[0-9]+}}, i32 8)
+  // LP64: call i64 @llvm.nvvm.ldu.global.i.i64.p0(ptr {{%[0-9]+}}, i32 8)
+  __nvvm_ldu_l((const long *)p);
+  __nvvm_ldu_ul((const unsigned long *)p);
+
+  // CHECK: call float @llvm.nvvm.ldu.global.f.f32.p0(ptr {{%[0-9]+}}, i32 4)
+  __nvvm_ldu_f((const float *)p);
+  // CHECK: call double @llvm.nvvm.ldu.global.f.f64.p0(ptr {{%[0-9]+}}, i32 8)
+  __nvvm_ldu_d((const double *)p);
+
+  // CHECK: call <2 x i8> 

[PATCH] D145227: [LLVM][OHOS] Clang toolchain and targets

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

I'm not familiar with the details of toolchain config, but added some general 
comments.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1404
+  // OHOS-specific defaults for PIC/PIE
+  if (Triple.isOHOSFamily()) {
+switch (Triple.getArch()) {

Collapse this into `if isohos && triple.getarch is...`.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1702
+  if (TC.getTriple().isOHOSFamily() && UNW != ToolChain::UNW_None) {
+CmdArgs.push_back("-l:libunwind.a");
+return;

Please add a comment explaining this. Even if it is just "OHOS is only 
compatible with libunwind". At least then we know it's nothing more mysterious.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:645
 
-  if (WantPthread && !isAndroid)
+  // We don't need libpthread neither for bionic nor for musl
+  if (WantPthread && !isAndroid && !isOHOSFamily)

And musl is what OHOS uses? Please add that to the comment if so "for musl, 
which OHOS uses...".



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2318
+  "mipsel-linux-gnu", "mips-img-linux-gnu", "mipsisa32r6el-linux-gnu",
+  "mipsel-linux-ohos"};
 

No riscv related changes needed in this file?



Comment at: clang/lib/Driver/ToolChains/OHOS.cpp:164
+  // For compatibility with arm-liteos sysroot
+  // FIXME: Remove this when we'll use arm-liteos sysroot produced by build.py.
+  addPathIfExists(

Keep FIXMEs that refer to stuff outside the project downstream please :)



Comment at: clang/lib/Driver/ToolChains/OHOS.cpp:372
+// FIXME: gnu or both???
+CmdArgs.push_back("--hash-style=both");
+  }

Maybe this helps? 
https://github.com/llvm/llvm-project/blob/e00c73c856a325008afead10cfb3e9d0fc4a1e41/clang/lib/Driver/ToolChains/Linux.cpp#L235

(no idea myself)



Comment at: clang/test/Driver/ohos.c:240
+
+// CHECK-OHOS-PTHREAD-NOT: -lpthread
+

This one is checking that we do not link to a pthread library, because when 
using musl, it already provides one?

Just checking you're not accepting the option but doing the opposite here. 
Worth adding a comment to explain the reasoning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145227

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

[PATCH] D125171: Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin updated this revision to Diff 502112.
jrmolin added a comment.

Finally figured out how to run the latest `git-clang-format` on the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp

Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4735,6 +4735,17 @@
 return true;
   }
 
+  // If AlwaysBreakBeforeFunctionParameters is true, we want to break before
+  // the next parameter, if there is one.
+  if (Left.is(tok::l_paren) && Style.AlwaysBreakBeforeFunctionParameters &&
+  !Right.is(tok::r_paren)) {
+if (Left.Previous) {
+  const FormatToken  = *Left.Previous;
+  if (TwoPrevious.is(TT_FunctionDeclarationName))
+return true;
+}
+  }
+
   // If the last token before a '}', ']', or ')' is a comma or a trailing
   // comment, the intention is to insert a line break after it in order to make
   // shuffling around entries easier. Import statements, especially in
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -870,6 +870,8 @@
Style.AlwaysBreakAfterReturnType);
 IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
Style.AlwaysBreakBeforeMultilineStrings);
+IO.mapOptional("AlwaysBreakBeforeFunctionParameters",
+   Style.AlwaysBreakBeforeFunctionParameters);
 IO.mapOptional("AlwaysBreakTemplateDeclarations",
Style.AlwaysBreakTemplateDeclarations);
 IO.mapOptional("AttributeMacros", Style.AttributeMacros);
@@ -1505,6 +1507,7 @@
   FormatStyle::SIS_WithoutElse;
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
+  GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   GoogleStyle.DerivePointerAlignment = true;
   GoogleStyle.IncludeStyle.IncludeCategories = {{"^", 2, 0, false},
@@ -1577,6 +1580,7 @@
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
+GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
 GoogleStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
 GoogleStyle.ColumnLimit = 100;
 GoogleStyle.SpaceAfterCStyleCast = true;
@@ -1588,6 +1592,7 @@
 // TODO: still under discussion whether to switch to SLS_All.
 GoogleStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
+GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
 GoogleStyle.BreakBeforeTernaryOperators = false;
 // taze:, triple slash directives (`/// <...`), tslint:, and @see, which is
 // commonly followed by overlong URLs.
@@ -1613,6 +1618,7 @@
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
+GoogleStyle.AlwaysBreakBeforeFunctionParameters = false;
 GoogleStyle.ColumnLimit = 100;
 // "Regroup" doesn't work well for ObjC yet (main header heuristic,
 // relationship between ObjC standard library headers and other heades,
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -354,6 +354,12 @@
 auto LambdaBodyLength = getLengthToMatchingParen(Current, State.Stack);
 return LambdaBodyLength > getColumnLimit(State);
   }
+  // Check if we want to break before function parameters in declarations
+  if (startsNextParameter(Current, Style) &&
+  Style.AlwaysBreakBeforeFunctionParameters &&
+  State.Line->MustBeDeclaration) {
+return true;
+  }
   if (Current.MustBreakBefore ||
   (Current.is(TT_InlineASMColon) &&
(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always ||
@@ -1055,7 +1061,9 @@
 // If we are breaking after '(', '{', '<', or this is the break after a ':'
 // to start a member initializater list in a constructor, this should not
 // be considered bin packing unless the relevant AllowAll option is false or
-// this is a dict/object literal.
+// this is a dict/object literal. Break if
+// AlwaysBreakBeforeFunctionParameters is true and it's a function
+// declaration.
 bool 

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Kiran Chandramohan via Phabricator via cfe-commits]

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

Thanks for making the change.
I am still going through the patch, I have a few comments.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1574-1575
+  const LocationDescription , OpenMPIRBuilder::InsertPointTy CodeGenIP,
+  bool HasRegion, SmallVector ,
+  SmallVector , struct MapperAllocas ,
+  Function *MapperFunc, int64_t DeviceID, Value *IfCond,

SmallVector -> SmallVectorImpl 
https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4052-4060
+  // LLVM utilities like blocks with terminators.
+  auto *UI = Builder.CreateUnreachable();
+  Instruction *ThenTI = UI, *ElseTI = nullptr;
+  if (IfCond) {
+SplitBlockAndInsertIfThenElse(IfCond, UI, , );
+ThenTI->getParent()->setName("omp_if.then");
+ElseTI->getParent()->setName("omp_if.else");

There is some recent recommendation to not insert artificial instructions and 
remove them. The preferred approach is to use `splitBB` function(s) inside the 
OpenMPIRBuilder. This function works on blocks without terminators. You can 
consult the `IfCondition` code in `createParallel`.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:4983
+  CallInst *TargetDataCall = dyn_cast(>back());
+  BB->dump();
+  EXPECT_NE(TargetDataCall, nullptr);

Is this a debugging leftover?



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:4990
+  EXPECT_TRUE(TargetDataCall->getOperand(2)->getType()->isIntegerTy(32));
+  EXPECT_TRUE(TargetDataCall->getOperand(8)->getType()->isPointerTy());
+}

Call verifyModule to ensure the IR is well formed.



Comment at: mlir/include/mlir/Target/LLVMIR/Dialect/OpenMPCommon.h:13-14
+
+#ifndef MLIR_TARGET_LLVMIR_DIALECT_UTILS_H
+#define MLIR_TARGET_LLVMIR_DIALECT_UTILS_H
+

Nit: The header guards would need changing.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1471
+mapTypes = enterDataOp.getMapTypes();
+mapperFunc = ompBuilder->getOrCreateRuntimeFunctionPtr(
+llvm::omp::OMPRTL___tgt_target_data_begin_mapper);

Ideally we would not want to create an OpenMP runtime calls in the Translation. 
This is the job of the OpenMPIRbuilder. I would recommend passing a boolean to 
the OpenMPIRBuilder and let the IRBuilder pick up the runtime function.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1492
+mapTypes = exitDataOp.getMapTypes();
+mapperFunc = ompBuilder->getOrCreateRuntimeFunctionPtr(
+llvm::omp::OMPRTL___tgt_target_data_end_mapper);

Same comment as above.



Comment at: mlir/test/Target/LLVMIR/omptarget-llvm.mlir:1
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+

In MLIR it is preferred to test the minimal set of functionalities. So you will 
have to minimize the CHECK lines and write them manually. Focus on Checking the 
runtime call and any important IR inserted by the IRBuilder. Keep the contents 
of the region to a minimum.
https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 502110.
DmitryPolukhin added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/test/Inputs/did-change-configuration-params.args
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp

Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
@@ -299,6 +300,31 @@
   }
 }
 
+void addExpandedResponseFiles(std::vector ,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem ) {
+  bool SeenRSPFile = false;
+  llvm::SmallVector Argv;
+  Argv.reserve(CommandLine.size());
+  for (auto  : CommandLine) {
+Argv.push_back(Arg.c_str());
+if (!Arg.empty())
+  SeenRSPFile |= Arg.front() == '@';
+  }
+  if (!SeenRSPFile)
+return;
+  llvm::BumpPtrAllocator Alloc;
+  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+  llvm::Error Err =
+  ECtx.setVFS().setCurrentDir(WorkingDir).expandResponseFiles(Argv);
+  if (Err)
+llvm::errs() << Err;
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  CommandLine = std::move(ExpandedArgv);
+}
+
 } // namespace tooling
 } // namespace clang
 
@@ -684,7 +710,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
@@ -48,28 +49,9 @@
 
 private:
   std::vector expand(std::vector Cmds) const {
-for (auto  : Cmds) {
-  bool SeenRSPFile = false;
-  llvm::SmallVector Argv;
-  Argv.reserve(Cmd.CommandLine.size());
-  for (auto  : Cmd.CommandLine) {
-Argv.push_back(Arg.c_str());
-if (!Arg.empty())
-  SeenRSPFile |= Arg.front() == '@';
-  }
-  if (!SeenRSPFile)
-continue;
-  llvm::BumpPtrAllocator Alloc;
-  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
-  llvm::Error Err = ECtx.setVFS(FS.get())
-.setCurrentDir(Cmd.Directory)
-.expandResponseFiles(Argv);
-  if (Err)
-llvm::errs() << Err;
-  // Don't assign directly, Argv aliases CommandLine.
-  std::vector ExpandedArgv(Argv.begin(), Argv.end());
-  Cmd.CommandLine = std::move(ExpandedArgv);
-}
+for (auto  : Cmds)
+  tooling::addExpandedResponseFiles(Cmd.CommandLine, Cmd.Directory,
+Tokenizer, *FS);
 return Cmds;
   }
 
Index: clang/include/clang/Tooling/Tooling.h
===
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -506,6 +506,12 @@
 void addTargetAndModeForProgramName(std::vector ,
 StringRef InvokedAs);
 
+/// Helper function that expands response files in command line.
+void addExpandedResponseFiles(std::vector ,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem );
+
 /// Creates a \c CompilerInvocation.
 CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
   ArrayRef CC1Args,
Index: clang-tools-extra/clangd/unittests/SyncAPI.cpp
===
--- clang-tools-extra/clangd/unittests/SyncAPI.cpp
+++ 

[PATCH] D144179: [Clang] Added functionality to provide config file name via env variable

[mgabka via Phabricator via cfe-commits]

mgabka added a comment.

In D144179#4146599 , @MaskRay wrote:

> This looks like introducing a footgun (when something behaves differently 
> from an upstream Clang, it would be difficult for toolchain maintainers to 
> know why).
> Why can't your user specify `CLANG_CONFIG_FILE_SYSTEM_DIR`?

hi @MaskRay,
The reason we wanted to suggest use of environment variable is that the 
CLANG_CONFIG_FILE_SYSTEM_DIR is only defined at compilation time, after 
discussing it once again we would rather lean towards introducing an 
environment variable with similar semantics as CLANG_CONFIG_FILE_SYSTEM_DIR or 
rather ``CLANG_CONFIG_FILE_USER_DIR``, the motivation here is that it will 
allow to specify the directory to search for config files in a dynamic way, 
without need to recompile the compiler.
It is for user convenience in situations when they are using a system wide 
installation in a location where they do not have access right, and the 
``CLANG_CONFIG_FILE_SYSTEM_DIR`` and ``CLANG_CONFIG_FILE_USER_DIR`` were not 
defined at build time.

We realised that environment variables are already used in this area, for 
example CLANG_NO_DEFAULT_CONFIG, so adding another one is not breaking existing 
convention.

What do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144179

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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added a comment.

@kiranchandramohan I've added the MLIR test as requested. Please let me know if 
any other changes are required :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D144967: [PowerPC] Recognize long CPU name for -mtune in Clang

[Amy Kwan via Phabricator via cfe-commits]

amyk added a comment.

In D144967#4164858 , @nathanchance 
wrote:

> Could this be merged into `main` and backported to `release/16.x`? If this 
> makes 16.0.0 final, I think the kernel can avoid working around this issue 
> altogether, as `-mtune` was only wired up to do something on PowerPC in 
> during the 16 development cycle; in prior versions, it was ignored so any 
> value was accepted.

@nathanchance Just wanted to let you know that this patch is now backported 
into `release/16.x`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144967

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