[PATCH] D86398: [X86] Enable constexpr on _cast fp<-> uint intrinsics (PR31446)

[Simon Pilgrim via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8e0e5db4860: [X86] Enable constexpr on _cast fp<-> 
uint intrinsics (PR31446) (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86398

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Headers/ia32intrin.h
  clang/test/CodeGen/x86-builtins.c

Index: clang/test/CodeGen/x86-builtins.c
===
--- clang/test/CodeGen/x86-builtins.c
+++ clang/test/CodeGen/x86-builtins.c
@@ -1,45 +1,43 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=CHECK-64
-// RUN: %clang_cc1 -ffreestanding %s -triple=i386-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=CHECK-32
+// RUN: %clang_cc1 -x c -ffreestanding %s -triple=x86_64-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c -ffreestanding %s -triple=i386-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -ffreestanding %s -triple=x86_64-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -ffreestanding %s -triple=i386-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
 
 #include 
 
 unsigned int test_castf32_u32 (float __A){
-  // CHECK-64-LABEL: @test_castf32_u32
-  // CHECK-64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
-  // CHECK-64: %{{.*}} = load i32, i32* %{{.*}}, align 4
-  // CHECK-32-LABEL: @test_castf32_u32
-  // CHECK-32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
-  // CHECK-32: %{{.*}} = load i32, i32* %{{.*}}, align 4
+  // CHECK-LABEL: test_castf32_u32
+  // CHECK: bitcast float* %{{.*}} to i32*
+  // CHECK: %{{.*}} = load i32, i32* %{{.*}}, align 4
   return _castf32_u32(__A);
 }
 
 unsigned long long test_castf64_u64 (double __A){
-  // CHECK-64-LABEL: @test_castf64_u64
-  // CHECK-64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %{{.*}}, i8* align 8 %{{.*}}, i64 8, i1 false)
-  // CHECK-64: %{{.*}} = load i64, i64* %{{.*}}, align 8
-  // CHECK-32-LABEL: @test_castf64_u64
-  // CHECK-32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %{{.*}}, i8* align 8 %{{.*}}, i32 8, i1 false)
-  // CHECK-32: %{{.*}} = load i64, i64* %{{.*}}, align 8
+  // CHECK-LABEL: test_castf64_u64
+  // CHECK: bitcast double* %{{.*}} to i64*
+  // CHECK: %{{.*}} = load i64, i64* %{{.*}}, align 8
   return _castf64_u64(__A);
 }
 
 float test_castu32_f32 (unsigned int __A){
-  // CHECK-64-LABEL: @test_castu32_f32
-  // CHECK-64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
-  // CHECK-64: %{{.*}} = load float, float* %{{.*}}, align 4
-  // CHECK-32-LABEL: @test_castu32_f32
-  // CHECK-32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
-  // CHECK-32: %{{.*}} = load float, float* %{{.*}}, align 4
+  // CHECK-LABEL: test_castu32_f32
+  // CHECK: bitcast i32* %{{.*}} to float*
+  // CHECK: %{{.*}} = load float, float* %{{.*}}, align 4
   return _castu32_f32(__A);
 }
 
 double test_castu64_f64 (unsigned long long __A){
-  // CHECK-64-LABEL: @test_castu64_f64
-  // CHECK-64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %{{.*}}, i8* align 8 %{{.*}}, i64 8, i1 false)
-  // CHECK-64: %{{.*}} = load double, double* %{{.*}}, align 8
-  // CHECK-32-LABEL: @test_castu64_f64
-  // CHECK-32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %{{.*}}, i8* align 8 %{{.*}}, i32 8, i1 false)
-  // CHECK-32: %{{.*}} = load double, double* %{{.*}}, align 8
+  // CHECK-LABEL: test_castu64_f64
+  // CHECK: bitcast i64* %{{.*}} to double*
+  // CHECK: %{{.*}} = load double, double* %{{.*}}, align 8
   return _castu64_f64(__A);
 }
 
+// Test constexpr handling.
+#if defined(__cplusplus) && (__cplusplus >= 201103L)
+char cast_f32_u32_0[_castf32_u32(-0.0f) == 0x8000 ? 1 : -1];
+char cast_u32_f32_0[_castu32_f32(0x3F80) == +1.0f ? 1 : -1];
+
+char castf64_u64_0[_castf64_u64(-0.0) == 0x8000 ? 1 : -1];
+char castu64_f64_0[_castu64_f64(0xBFF0ULL) == -1.0 ? 1 : -1];
+#endif
Index: clang/lib/Headers/ia32intrin.h
===
--- clang/lib/Headers/ia32intrin.h
+++ clang/lib/Headers/ia32intrin.h
@@ -16,12 +16,13 @@
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
-#define __DEFAULT_FN_ATTRS_CAST __attribute__((__always_inline__))
 #define __DEFAULT_FN_ATTRS_SSE42 __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
 
 #if defined(__cplusplus) && (__cplusplus >= 20

[clang] f8e0e5d - [X86] Enable constexpr on _cast fp<-> uint intrinsics (PR31446)

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-23T10:27:46+01:00
New Revision: f8e0e5db48601cb0d019405703ccaa2378f503e0

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

LOG: [X86] Enable constexpr on _cast fp<-> uint intrinsics (PR31446)

As suggested by @rsmith on PR47267, by replacing the builtin_memcpy bitcast 
pattern with builtin_bit_cast we can use _castf32_u32, _castu32_f32, 
_castf64_u64 and _castu64_f64 inside constant expresssions (constexpr). 
Although __builtin_bit_cast was added for c++20 it works on all clang c/c++ 
modes.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Headers/ia32intrin.h
clang/test/CodeGen/x86-builtins.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ba2b540639ad..b4e594a1c4b4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -179,6 +179,9 @@ X86 Support in Clang
 - The x86 intrinsics ``__bswap``, ``__bswapd``, ``__bswap64`` and ``__bswapq``
   may now be used within constant expressions.
 
+- The x86 intrinsics ``_castf32_u32``, ``_castf64_u64``, ``_castu32_f32`` and
+  ``_castu64_f64`` may now be used within constant expressions.
+
 Internal API Changes
 
 

diff  --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index 1d17a565378a..f01a3adfc3b2 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -16,12 +16,13 @@
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
-#define __DEFAULT_FN_ATTRS_CAST __attribute__((__always_inline__))
 #define __DEFAULT_FN_ATTRS_SSE42 __attribute__((__always_inline__, 
__nodebug__, __target__("sse4.2")))
 
 #if defined(__cplusplus) && (__cplusplus >= 201103L)
+#define __DEFAULT_FN_ATTRS_CAST __attribute__((__always_inline__)) constexpr
 #define __DEFAULT_FN_ATTRS_CONSTEXPR __DEFAULT_FN_ATTRS constexpr
 #else
+#define __DEFAULT_FN_ATTRS_CAST __attribute__((__always_inline__))
 #define __DEFAULT_FN_ATTRS_CONSTEXPR __DEFAULT_FN_ATTRS
 #endif
 
@@ -218,9 +219,7 @@ __writeeflags(unsigned int __f)
  */
 static __inline__ unsigned int __DEFAULT_FN_ATTRS_CAST
 _castf32_u32(float __A) {
-  unsigned int D;
-  __builtin_memcpy(&D, &__A, sizeof(__A));
-  return D;
+  return __builtin_bit_cast(unsigned int, __A);
 }
 
 /** Cast a 64-bit float value to a 64-bit unsigned integer value
@@ -235,9 +234,7 @@ _castf32_u32(float __A) {
  */
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS_CAST
 _castf64_u64(double __A) {
-  unsigned long long D;
-  __builtin_memcpy(&D, &__A, sizeof(__A));
-  return D;
+  return __builtin_bit_cast(unsigned long long, __A);
 }
 
 /** Cast a 32-bit unsigned integer value to a 32-bit float value
@@ -252,9 +249,7 @@ _castf64_u64(double __A) {
  */
 static __inline__ float __DEFAULT_FN_ATTRS_CAST
 _castu32_f32(unsigned int __A) {
-  float D;
-  __builtin_memcpy(&D, &__A, sizeof(__A));
-  return D;
+  return __builtin_bit_cast(float, __A);
 }
 
 /** Cast a 64-bit unsigned integer value to a 64-bit float value
@@ -269,9 +264,7 @@ _castu32_f32(unsigned int __A) {
  */
 static __inline__ double __DEFAULT_FN_ATTRS_CAST
 _castu64_f64(unsigned long long __A) {
-  double D;
-  __builtin_memcpy(&D, &__A, sizeof(__A));
-  return D;
+  return __builtin_bit_cast(double, __A);
 }
 
 /** Adds the unsigned integer operand to the CRC-32C checksum of the

diff  --git a/clang/test/CodeGen/x86-builtins.c 
b/clang/test/CodeGen/x86-builtins.c
index fa2530b2cd15..6604e1f2c7d2 100644
--- a/clang/test/CodeGen/x86-builtins.c
+++ b/clang/test/CodeGen/x86-builtins.c
@@ -1,45 +1,43 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-unknown -emit-llvm 
-o - -Wall -Werror | FileCheck %s -check-prefix=CHECK-64
-// RUN: %clang_cc1 -ffreestanding %s -triple=i386-unknown-unknown -emit-llvm 
-o - -Wall -Werror | FileCheck %s -check-prefix=CHECK-32
+// RUN: %clang_cc1 -x c -ffreestanding %s -triple=x86_64-unknown-unknown 
-emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c -ffreestanding %s -triple=i386-unknown-unknown 
-emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -ffreestanding %s 
-triple=x86_64-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -ffreestanding %s 
-triple=i386-unknown-unknown -emit-llvm -o - -Wall -Werror | FileCheck %s
 
 #include 
 
 unsigned int test_castf32_u32 (float __A){
-  // CHECK-64-LABEL: @test_castf32_u32
-  // CHECK-64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* 
align 4 %{{.*}}, i64 4, i1 false)
-  // CHECK-64: %{{.*}} = load i32, i32* %{{.*}}, align 4
-  // CHECK-

[clang] f76adc2 - [docs] Add an initial (non-exhaustive) list of intrinsics that can be used in constant expressions

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-23T10:55:14+01:00
New Revision: f76adc2603f6cc466dd809142388ffb56a1c3e31

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

LOG: [docs] Add an initial (non-exhaustive) list of intrinsics that can be used 
in constant expressions

As suggested by @rsmith on D86398 - we should try to document the intrinsics 
that can be used in constexpr

Added: 


Modified: 
clang/docs/LanguageExtensions.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index a431239f887b..970c75ffafee 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3603,3 +3603,76 @@ and alignment as the smallest basic type that can 
contain them. Types that are l
 than 64 bits are handled in the same way as _int128 is handled; they are 
conceptually
 treated as struct of register size chunks. They number of chunks are the 
smallest
 number that can contain the types which does not necessarily mean a power-of-2 
size.
+
+Intrinsics Support within Constant Expressions
+==
+
+The following builtin intrinsics can be used in constant expressions:
+
+* ``__builtin_bitreverse8``
+* ``__builtin_bitreverse16``
+* ``__builtin_bitreverse32``
+* ``__builtin_bitreverse64``
+* ``__builtin_bswap16``
+* ``__builtin_bswap32``
+* ``__builtin_bswap64``
+* ``__builtin_clrsb``
+* ``__builtin_clrsbl``
+* ``__builtin_clrsbll``
+* ``__builtin_clz``
+* ``__builtin_clzl``
+* ``__builtin_clzll``
+* ``__builtin_clzs``
+* ``__builtin_ctz``
+* ``__builtin_ctzl``
+* ``__builtin_ctzll``
+* ``__builtin_ctzs``
+* ``__builtin_ffs``
+* ``__builtin_ffsl``
+* ``__builtin_ffsll``
+* ``__builtin_fpclassify``
+* ``__builtin_inf``
+* ``__builtin_isinf``
+* ``__builtin_isinf_sign``
+* ``__builtin_isfinite``
+* ``__builtin_isnan``
+* ``__builtin_isnormal``
+* ``__builtin_nan``
+* ``__builtin_nans``
+* ``__builtin_parity``
+* ``__builtin_parityl``
+* ``__builtin_parityll``
+* ``__builtin_popcount``
+* ``__builtin_popcountl``
+* ``__builtin_popcountll``
+* ``__builtin_rotateleft8``
+* ``__builtin_rotateleft16``
+* ``__builtin_rotateleft32``
+* ``__builtin_rotateleft64``
+* ``__builtin_rotateright8``
+* ``__builtin_rotateright16``
+* ``__builtin_rotateright32``
+* ``__builtin_rotateright64``
+
+The following x86-specific intrinsics can be used in constant expressions:
+
+* ``_bit_scan_forward``
+* ``_bit_scan_reverse``
+* ``__bsfd``
+* ``__bsfq``
+* ``__bsrd``
+* ``__bsrq``
+* ``__bswap``
+* ``__bswapd``
+* ``__bswap64``
+* ``__bswapq``
+* ``_castf32_u32``
+* ``_castf64_u64``
+* ``_castu32_f32``
+* ``_castu64_f64``
+* ``_mm_popcnt_u32``
+* ``_mm_popcnt_u64``
+* ``_popcnt32``
+* ``_popcnt64``
+* ``__popcntd``
+* ``__popcntq``
\ No newline at end of file



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

[PATCH] D86029: [analyzer] Add modeling for unque_ptr::get()

[Nithin VR via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG55208f5a2126: [analyzer] Add modeling for unque_ptr::get() 
(authored by vrnithinkumar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86029

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp
  clang/test/Analysis/smart-ptr.cpp

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -7,6 +7,7 @@
 
 void clang_analyzer_warnIfReached();
 void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(bool);
 
 void derefAfterMove(std::unique_ptr P) {
   std::unique_ptr Q = std::move(P);
@@ -252,3 +253,26 @@
   P->foo(); // No warning.
   PValid->foo(); // No warning.
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr P;
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromMultipleGetOnUnknownPtr(std::unique_ptr P) {
+  A *X = P.get();
+  A *Y = P.get();
+  clang_analyzer_eval(X == Y); // expected-warning{{TRUE}}
+  if (!X) {
+Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+  }
+}
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -116,3 +116,18 @@
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr P; // FIXME: add note "Default constructed smart pointer 'P' is null"
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+  // expected-note@-1 {{Called C++ object pointer is null}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
+  P.get()->foo(); // No warning.
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -56,13 +56,15 @@
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+  void handleGet(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, &SmartPtrModeling::handleReset},
   {{"release"}, &SmartPtrModeling::handleRelease},
-  {{"swap", 1}, &SmartPtrModeling::handleSwap}};
+  {{"swap", 1}, &SmartPtrModeling::handleSwap},
+  {{"get"}, &SmartPtrModeling::handleGet}};
 };
 } // end of anonymous namespace
 
@@ -345,6 +347,33 @@
   }));
 }
 
+void SmartPtrModeling::handleGet(const CallEvent &Call,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  const auto *IC = dyn_cast(&Call);
+  if (!IC)
+return;
+
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
+return;
+
+  SVal InnerPointerVal;
+  if (const auto *InnerValPtr = State->get(ThisRegion)) {
+InnerPointerVal = *InnerValPtr;
+  } else {
+const auto *CallExpr = Call.getOriginExpr();
+InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
+CallExpr, C.getLocationContext(), Call.getResultType(), C.blockCount());
+State = State->set(ThisRegion, InnerPointerVal);
+  }
+
+  State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+  InnerPointerVal);
+  // TODO: Add NoteTag, for how the raw pointer got using 'get' method.
+  C.addTransition(State);
+}
+
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker();
   Checker->ModelSmartPtrDereference =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 55208f5 - [analyzer] Add modeling for unque_ptr::get()

[Nithin Vadukkumchery Rajendrakumar via cfe-commits]

Author: Nithin Vadukkumchery Rajendrakumar
Date: 2020-08-23T14:50:26+02:00
New Revision: 55208f5a2126e491c41ba9ff6542551bfc090e86

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

LOG: [analyzer] Add modeling for unque_ptr::get()

Summary: Implemented  modeling for get() method in SmartPtrModeling

Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun

Reviewed By: NoQ, xazax.hun

Subscribers: martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D86029

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 15f56728eaa9..0b084accbfbe 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -56,13 +56,15 @@ class SmartPtrModeling
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+  void handleGet(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) 
const;
   CallDescriptionMap SmartPtrMethodHandlers{
   {{"reset"}, &SmartPtrModeling::handleReset},
   {{"release"}, &SmartPtrModeling::handleRelease},
-  {{"swap", 1}, &SmartPtrModeling::handleSwap}};
+  {{"swap", 1}, &SmartPtrModeling::handleSwap},
+  {{"get"}, &SmartPtrModeling::handleGet}};
 };
 } // end of anonymous namespace
 
@@ -345,6 +347,33 @@ void SmartPtrModeling::handleSwap(const CallEvent &Call,
   }));
 }
 
+void SmartPtrModeling::handleGet(const CallEvent &Call,
+ CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  const auto *IC = dyn_cast(&Call);
+  if (!IC)
+return;
+
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
+return;
+
+  SVal InnerPointerVal;
+  if (const auto *InnerValPtr = State->get(ThisRegion)) {
+InnerPointerVal = *InnerValPtr;
+  } else {
+const auto *CallExpr = Call.getOriginExpr();
+InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
+CallExpr, C.getLocationContext(), Call.getResultType(), 
C.blockCount());
+State = State->set(ThisRegion, InnerPointerVal);
+  }
+
+  State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+  InnerPointerVal);
+  // TODO: Add NoteTag, for how the raw pointer got using 'get' method.
+  C.addTransition(State);
+}
+
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker();
   Checker->ModelSmartPtrDereference =

diff  --git a/clang/test/Analysis/smart-ptr-text-output.cpp 
b/clang/test/Analysis/smart-ptr-text-output.cpp
index 1132a37fa667..9af6f251e01d 100644
--- a/clang/test/Analysis/smart-ptr-text-output.cpp
+++ b/clang/test/Analysis/smart-ptr-text-output.cpp
@@ -116,3 +116,18 @@ void noNoteTagsForNonMatchingBugType() {
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of 
type 'std::unique_ptr' [cplusplus.Move]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P' of type 
'std::unique_ptr'}}
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr P; // FIXME: add note "Default constructed smart pointer 
'P' is null"
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null 
[core.CallAndMessage]}}
+  // expected-note@-1 {{Called C++ object pointer is null}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
+  P.get()->foo(); // No warning.
+}

diff  --git a/clang/test/Analysis/smart-ptr.cpp 
b/clang/test/Analysis/smart-ptr.cpp
index bcf1e569d690..17f6718c6605 100644
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -7,6 +7,7 @@
 
 void clang_analyzer_warnIfReached();
 void clang_analyzer_numTimesReached();
+void clang_analyzer_eval(bool);
 
 void derefAfterMove(std::unique_ptr P) {
   std::unique_ptr Q = std::move(P);
@@ -252,3 +253,26 @@ void derefOnSwappedValidPtr() {
   P->foo(); // No warning.
   PValid->foo(); // No warning.
 }
+
+void derefOnRawPtrFromGetOnNullPtr() {
+  std::unique_ptr P;
+  P.get()->foo(); // expected-warning {{Called C++ object pointer is null 
[core.CallAndMessage]}}
+}
+
+void derefOnRawPtrFromGetOnValidPtr() {
+  std::unique_ptr P(new A());
+  P.get()->foo(); // No warning.
+}
+
+void de

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D79219#2231927 , @mati865 wrote:

> @phosek in MSYS2 (targeting x86_64-w64-windows-gnu) Zlib works properly for 
> LLVM 10 but with master I'm now seeing:
>
>   -- Constructing LLVMBuild project information
>   -- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a
>   CMake Error at lib/Support/CMakeLists.txt:9 (string):
> string sub-command REGEX, mode REPLACE: regex "^(lib|)" matched an empty
> string.
>   Call Stack (most recent call first):
> lib/Support/CMakeLists.txt:226 (get_system_libname)
>
> `-- DEBUG zlib_library=D:/msys64/mingw64/lib/libz.dll.a` was printed by my 
> change to help debugging it.
> FYI `zlib_library` is set here: 
> https://github.com/llvm/llvm-project/blob/8e06bf6b3a2e8d25e56cd52dca0cf3ff1b37b5d1/llvm/lib/Support/CMakeLists.txt#L218

Looks like that's an issue introduced by D86134 
 or D86245 . 
Can you print the value of `${CMAKE_FIND_LIBRARY_PREFIXES}` and 
`${CMAKE_FIND_LIBRARY_SUFFIXES}` in your build?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

@phosek

> Looks like that's an issue introduced by D86134 
>  or D86245 .

Indeed, I apologize for bothering you. Should I move the discussion to one of 
patches created by  @haampie?

Continuing the discussion with `if( CMAKE_FIND_LIBRARY_PREFIXES )` and `if( 
CMAKE_FIND_LIBRARY_SUFFIXES )` from https://reviews.llvm.org/D86245 changed to 
false `llvm-config --system-libs` prints `-llibz.dll.a` so we need those 
replaces to bring back correct `-lz` value.

> Can you print the value of ${CMAKE_FIND_LIBRARY_PREFIXES} and 
> ${CMAKE_FIND_LIBRARY_SUFFIXES} in your build?

  -- CMAKE_FIND_LIBRARY_PREFIXES=lib;
  -- CMAKE_FIND_LIBRARY_SUFFIXES=.dll.a;.a;.lib

Which matches 
https://gitlab.kitware.com/cmake/cmake/-/blob/269d1a86249ea037a2884133daffc8c44a38d926/Modules/Platform/Windows-GNU.cmake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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

[clang] a1dc3d2 - [X86] Enable constexpr on ROTL/ROTR intrinsics (PR31446)

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-08-23T16:11:58+01:00
New Revision: a1dc3d241ba00042b6160287f887d1019e36bae0

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

LOG: [X86] Enable constexpr on ROTL/ROTR intrinsics (PR31446)

This enables constexpr rotate intrinsics defined in ia32intrin.h, including the 
MS specific builtins.

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/lib/AST/ExprConstant.cpp
clang/lib/Headers/ia32intrin.h
clang/test/CodeGen/rot-intrinsics.c

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 970c75ffafee..c89f924c58ba 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3675,4 +3675,18 @@ The following x86-specific intrinsics can be used in 
constant expressions:
 * ``_popcnt32``
 * ``_popcnt64``
 * ``__popcntd``
-* ``__popcntq``
\ No newline at end of file
+* ``__popcntq``
+* ``__rolb``
+* ``__rolw``
+* ``__rold``
+* ``__rolq``
+* ``__rorb``
+* ``__rorw``
+* ``__rord``
+* ``__rorq``
+* ``_rotl``
+* ``_rotr``
+* ``_rotwl``
+* ``_rotwr``
+* ``_lrotl``
+* ``_lrotr``

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b4e594a1c4b4..c036f66d60bf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -182,6 +182,12 @@ X86 Support in Clang
 - The x86 intrinsics ``_castf32_u32``, ``_castf64_u64``, ``_castu32_f32`` and
   ``_castu64_f64`` may now be used within constant expressions.
 
+- The x86 intrinsics ``__rolb``, ``__rolw``, ``__rold``, ``__rolq`, ``_rotl``,
+  ``_rotwl`` and ``_lrotl`` may now be used within constant expressions.
+
+- The x86 intrinsics ``__rorb``, ``__rorw``, ``__rord``, ``__rorq`, ``_rotr``,
+  ``_rotwr`` and ``_lrotr`` may now be used within constant expressions.
+
 Internal API Changes
 
 

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index c23233ab8c8c..014c48e6f08f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -11361,7 +11361,12 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
   case Builtin::BI__builtin_rotateleft8:
   case Builtin::BI__builtin_rotateleft16:
   case Builtin::BI__builtin_rotateleft32:
-  case Builtin::BI__builtin_rotateleft64: {
+  case Builtin::BI__builtin_rotateleft64:
+  case Builtin::BI_rotl8: // Microsoft variants of rotate right
+  case Builtin::BI_rotl16:
+  case Builtin::BI_rotl:
+  case Builtin::BI_lrotl:
+  case Builtin::BI_rotl64: {
 APSInt Val, Amt;
 if (!EvaluateInteger(E->getArg(0), Val, Info) ||
 !EvaluateInteger(E->getArg(1), Amt, Info))
@@ -11373,7 +11378,12 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const 
CallExpr *E,
   case Builtin::BI__builtin_rotateright8:
   case Builtin::BI__builtin_rotateright16:
   case Builtin::BI__builtin_rotateright32:
-  case Builtin::BI__builtin_rotateright64: {
+  case Builtin::BI__builtin_rotateright64:
+  case Builtin::BI_rotr8: // Microsoft variants of rotate right
+  case Builtin::BI_rotr16:
+  case Builtin::BI_rotr:
+  case Builtin::BI_lrotr:
+  case Builtin::BI_rotr64: {
 APSInt Val, Amt;
 if (!EvaluateInteger(E->getArg(0), Val, Info) ||
 !EvaluateInteger(E->getArg(1), Amt, Info))

diff  --git a/clang/lib/Headers/ia32intrin.h b/clang/lib/Headers/ia32intrin.h
index f01a3adfc3b2..00138effd505 100644
--- a/clang/lib/Headers/ia32intrin.h
+++ b/clang/lib/Headers/ia32intrin.h
@@ -373,43 +373,43 @@ _wbinvd(void) {
   __builtin_ia32_wbinvd();
 }
 
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
+static __inline__ unsigned char __DEFAULT_FN_ATTRS_CONSTEXPR
 __rolb(unsigned char __X, int __C) {
   return __builtin_rotateleft8(__X, __C);
 }
 
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
+static __inline__ unsigned char __DEFAULT_FN_ATTRS_CONSTEXPR
 __rorb(unsigned char __X, int __C) {
   return __builtin_rotateright8(__X, __C);
 }
 
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
+static __inline__ unsigned short __DEFAULT_FN_ATTRS_CONSTEXPR
 __rolw(unsigned short __X, int __C) {
   return __builtin_rotateleft16(__X, __C);
 }
 
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
+static __inline__ unsigned short __DEFAULT_FN_ATTRS_CONSTEXPR
 __rorw(unsigned short __X, int __C) {
   return __builtin_rotateright16(__X, __C);
 }
 
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
+static __inline__ unsigned int __DEFAULT_FN_ATTRS_CONSTEXPR
 __rold(unsigned int __X, int __C) {
   return __builtin_rotateleft32(__X, __C);
 }
 
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
+static __inline__ unsigned int __DEFAULT_FN_ATTRS_CONSTEXPR
 __rord(unsigned int __X, int __C) {
   return __builtin_rotateright32(__X, __C);
 }
 
 #i

Re: [clang] f76adc2 - [docs] Add an initial (non-exhaustive) list of intrinsics that can be used in constant expressions

[Richard Smith via cfe-commits]

Thank you!

On Sun, 23 Aug 2020 at 02:55, Simon Pilgrim via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Simon Pilgrim
> Date: 2020-08-23T10:55:14+01:00
> New Revision: f76adc2603f6cc466dd809142388ffb56a1c3e31
>
> URL:
> https://github.com/llvm/llvm-project/commit/f76adc2603f6cc466dd809142388ffb56a1c3e31
> DIFF:
> https://github.com/llvm/llvm-project/commit/f76adc2603f6cc466dd809142388ffb56a1c3e31.diff
>
> LOG: [docs] Add an initial (non-exhaustive) list of intrinsics that can be
> used in constant expressions
>
> As suggested by @rsmith on D86398 - we should try to document the
> intrinsics that can be used in constexpr
>
> Added:
>
>
> Modified:
> clang/docs/LanguageExtensions.rst
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/docs/LanguageExtensions.rst
> b/clang/docs/LanguageExtensions.rst
> index a431239f887b..970c75ffafee 100644
> --- a/clang/docs/LanguageExtensions.rst
> +++ b/clang/docs/LanguageExtensions.rst
> @@ -3603,3 +3603,76 @@ and alignment as the smallest basic type that can
> contain them. Types that are l
>  than 64 bits are handled in the same way as _int128 is handled; they are
> conceptually
>  treated as struct of register size chunks. They number of chunks are the
> smallest
>  number that can contain the types which does not necessarily mean a
> power-of-2 size.
> +
> +Intrinsics Support within Constant Expressions
> +==
> +
> +The following builtin intrinsics can be used in constant expressions:
> +
> +* ``__builtin_bitreverse8``
> +* ``__builtin_bitreverse16``
> +* ``__builtin_bitreverse32``
> +* ``__builtin_bitreverse64``
> +* ``__builtin_bswap16``
> +* ``__builtin_bswap32``
> +* ``__builtin_bswap64``
> +* ``__builtin_clrsb``
> +* ``__builtin_clrsbl``
> +* ``__builtin_clrsbll``
> +* ``__builtin_clz``
> +* ``__builtin_clzl``
> +* ``__builtin_clzll``
> +* ``__builtin_clzs``
> +* ``__builtin_ctz``
> +* ``__builtin_ctzl``
> +* ``__builtin_ctzll``
> +* ``__builtin_ctzs``
> +* ``__builtin_ffs``
> +* ``__builtin_ffsl``
> +* ``__builtin_ffsll``
> +* ``__builtin_fpclassify``
> +* ``__builtin_inf``
> +* ``__builtin_isinf``
> +* ``__builtin_isinf_sign``
> +* ``__builtin_isfinite``
> +* ``__builtin_isnan``
> +* ``__builtin_isnormal``
> +* ``__builtin_nan``
> +* ``__builtin_nans``
> +* ``__builtin_parity``
> +* ``__builtin_parityl``
> +* ``__builtin_parityll``
> +* ``__builtin_popcount``
> +* ``__builtin_popcountl``
> +* ``__builtin_popcountll``
> +* ``__builtin_rotateleft8``
> +* ``__builtin_rotateleft16``
> +* ``__builtin_rotateleft32``
> +* ``__builtin_rotateleft64``
> +* ``__builtin_rotateright8``
> +* ``__builtin_rotateright16``
> +* ``__builtin_rotateright32``
> +* ``__builtin_rotateright64``
> +
> +The following x86-specific intrinsics can be used in constant expressions:
> +
> +* ``_bit_scan_forward``
> +* ``_bit_scan_reverse``
> +* ``__bsfd``
> +* ``__bsfq``
> +* ``__bsrd``
> +* ``__bsrq``
> +* ``__bswap``
> +* ``__bswapd``
> +* ``__bswap64``
> +* ``__bswapq``
> +* ``_castf32_u32``
> +* ``_castf64_u64``
> +* ``_castu32_f32``
> +* ``_castu64_f64``
> +* ``_mm_popcnt_u32``
> +* ``_mm_popcnt_u64``
> +* ``_popcnt32``
> +* ``_popcnt64``
> +* ``__popcntd``
> +* ``__popcntq``
> \ No newline at end of file
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 8 inline comments as done.
Mordante added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Mordante wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Mordante wrote:
> > > > > aaron.ballman wrote:
> > > > > > Hmm, I'm on the fence about specifying `201803` for these 
> > > > > > attributes. Given that this is only the start of supporting the 
> > > > > > attribute, do we want to claim it already matches the standard's 
> > > > > > behavior? Or do we just want to return `1` to signify that we 
> > > > > > understand this attribute but we don't yet fully support it in 
> > > > > > common cases (such as on labels in switch statements, etc)?
> > > > > > 
> > > > > > As another question, should we consider adding a C2x spelling 
> > > > > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this 
> > > > > > functionality to C?
> > > > > I was also somewhat on the fence, for now I'll change it to specify 
> > > > > `1`.
> > > > > 
> > > > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > > > there's a proposal to add this to C2x?
> > > > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > > > there's a proposal to add this to C2x?
> > > > 
> > > > There isn't one currently because there is no implementation experience 
> > > > with the attributes in C. This is a way to get that implementation 
> > > > experience so it's easier to propose the feature to WG14.
> > > > I was also somewhat on the fence, for now I'll change it to specify `1`.
> > > 
> > > There seems to be an issue using the `1` so I used `2` instead. (Didn't 
> > > want to look closely at the issue.)
> > > 
> > > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > > there's a proposal to add this to C2x?
> > > 
> > > There isn't one currently because there is no implementation experience 
> > > with the attributes in C. This is a way to get that implementation 
> > > experience so it's easier to propose the feature to WG14.
> > 
> > Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I 
> > didn't implement it yet. I intend to look at it later. I first want the 
> > initial part done to see whether this is the right approach.
> What issues are you running into? 1 is the default value when you don't 
> specify a value specifically.
It give the error "clang/include/clang/Basic/Attr.td:265:7: error: Standard 
attributes must have valid version information."



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > Mordante wrote:
> > > > aaron.ballman wrote:
> > > > > Mordante wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Hmm, I'm on the fence about specifying `201803` for these 
> > > > > > > attributes. Given that this is only the start of supporting the 
> > > > > > > attribute, do we want to claim it already matches the standard's 
> > > > > > > behavior? Or do we just want to return `1` to signify that we 
> > > > > > > understand this attribute but we don't yet fully support it in 
> > > > > > > common cases (such as on labels in switch statements, etc)?
> > > > > > > 
> > > > > > > As another question, should we consider adding a C2x spelling 
> > > > > > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this 
> > > > > > > functionality to C?
> > > > > > I was also somewhat on the fence, for now I'll change it to specify 
> > > > > > `1`.
> > > > > > 
> > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > whether there's a proposal to add this to C2x?
> > > > > > I'll have a look at the C2x changes. Just curious do you know 
> > > > > > whether there's a proposal to add this to C2x?
> > > > > 
> > > > > There isn't one currently because there is no implementation 
> > > > > experience with the attributes in C. This is a way to get that 
> > > > > implementation experience so it's easier to propose the feature to 
> > > > > WG14.
> > > > > I was also somewhat on the fence, for now I'll change it to specify 
> > > > > `1`.
> > > > 
> > > > There seems to be an issue using the `1` so I used `2` instead. (Didn't 
> > > > want to look closely at the issue.)
> > > > 
> > > > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > > > there's a proposal to add this to C2x?
> > > > 
> > > > There isn't one currently because there is no implementation experience 
> > > > with the attributes in C. This is a way to get that implementation 
> > > > experience so it's easier to propose the feature to WG14.
> > > 
> > > Nice to know. It seems the C2x wasn't at str

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 287264.
Mordante marked 2 inline comments as done.
Mordante added a comment.

Addresses review comments:

- Adds support for c2x using `[[clang::likely]]` and `[[clang::unlikely]]`
- Remove warnings about conflicting likelihoods, except those required by the 
Standard
- The attributes can now be placed inside the compound statement in of the 
`ThenStmt` and `ElseStmt`

The current implementation matches GCC's behaviour. This behaviour aligns best 
with my interpretation of the standard and GCC seems to have the most mature 
implementation. I would like to get the current implementation in Clang and 
proceed to work on the missing pieces. In parallel we can align with the other 
vendors and, if needed, adjust our implementation.


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

https://reviews.llvm.org/D85091

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/Preprocessor/has_attribute.cpp
  clang/test/Sema/attr-likelihood.c
  clang/test/SemaCXX/attr-likelihood.cpp
  clang/www/cxx_status.html
  llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp

Index: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===
--- llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -24,7 +24,6 @@
 #include "llvm/IR/Metadata.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/MisExpect.h"
@@ -48,10 +47,10 @@
 // 'select' instructions. It may be worthwhile to hoist these values to some
 // shared space, so they can be used directly by other passes.
 
-static cl::opt LikelyBranchWeight(
+cl::opt llvm::LikelyBranchWeight(
 "likely-branch-weight", cl::Hidden, cl::init(2000),
 cl::desc("Weight of the branch likely to be taken (default = 2000)"));
-static cl::opt UnlikelyBranchWeight(
+cl::opt llvm::UnlikelyBranchWeight(
 "unlikely-branch-weight", cl::Hidden, cl::init(1),
 cl::desc("Weight of the branch unlikely to be taken (default = 1)"));
 
Index: llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
===
--- llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
+++ llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
@@ -17,6 +17,7 @@
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
@@ -31,6 +32,8 @@
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &);
 };
 
+extern cl::opt LikelyBranchWeight;
+extern cl::opt UnlikelyBranchWeight;
 }
 
 #endif
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -987,7 +987,7 @@
 
   [[likely]] and [[unlikely]] attributes
   https://wg21.link/p0479r5";>P0479R5
-  No
+  Clang 12 (partial)
 
 
   typename optional in more contexts
Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -0,0 +1,132 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -DPEDANTIC -pedantic -fsyntax-only -verify
+
+#if PEDANTIC
+void g() {
+  if (true)
+[[likely]] {} // expected-warning {{use of the 'likely' attribute is a C++20 extension}}
+  else
+[[unlikely]] {} // expected-warning {{use of the 'unlikely' attribute is a C++20 extension}}
+}
+#else
+void a() {
+  if (true)
+[[likely]]   // expected-note {{conflicting attribute is here}}
+[[unlikely]] // expected-error {{unlikely and likely attributes are not compatible}}
+;
+}
+
+void b() {
+  if (true)
+[[unlikely]] // expected-note {{conflicting attribute is here}}
+[[likely]]   // expected-error {{likely and unlikely attributes are not compatible}}
+;
+}
+
+void c() {
+  if (true)
+[[likely]];
+}
+
+void d() {
+  if (true)
+[[unlikely]];
+}
+
+void g() {
+  if (true)
+[[likely]] {}
+  else
+[[unlikely]] {}
+}
+
+void h() {
+  if (true)
+[[likely]] {}
+  else {
+  }
+}
+
+void i() {
+  if (true)
+[[unlikely]] {}
+  else {
+  }
+}
+
+void j() {
+  if (true) {
+  } else
+[[likely]] {}
+}
+
+void k() {
+  if (true) {
+  } else
+[[likely]] {}
+}
+
+void l() {
+  if (true)
+[[likely]] {}
+  else
+[[unlikely]] if (false) [[likely]] {}
+}
+
+void m() {
+  [[likely]] int x = 42

[PATCH] D86193: [CSSPGO] Pseudo probe instrumentation for basic blocks.

[Wei Mi via Phabricator via cfe-commits]

wmi added a comment.

Thanks for the patch! A few questions:

> probe blocks some CFG transformations that can mess up profile correlation.

Can you enumerate some CFG transformations which be blocked? Is it possible 
that some CFG transformations being blocked are actually beneficial for later 
optimizations?

Are the intrinsic probes counted when computing bb size and function size?

And could you split the patches into small parts for easier review. For 
example,  Add the intrinsic support in IR and MIR. SampleProfileProbe pass. 
-fpseudo-probe-for-profiling support. changes in various passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86193

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Alan Phipps via Phabricator via cfe-commits]

alanphipps marked 13 inline comments as done.
alanphipps added a comment.
Herald added a subscriber: wenlei.

In D84467#2180421 , @vsk wrote:

> I haven't taken a close look at the tests yet, but plan on getting to it soon.
>
> I'm not sure whether this is something you've already tried, but for this 
> kind of change, it can be helpful to verify that a stage2 clang self-host 
> with assertions enabled completes successfully. For coverage specifically, 
> it's possible to do that by setting '-DLLVM_BUILD_INSTRUMENTED_COVERAGE=On' 
> during the stage2 cmake step.

Thank you for recommending this.  I was able to do an instrumented stage2 build 
successfully, with assertions enabled, and it built fine, and no problems 
running LIT.  I have to say that it was also cool to see branch coverage on the 
source code I added to support branch coverage!




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:742
+// function's SourceRegions) because it doesn't apply to any other source
+// code other than the Condition.
+if (CodeGenFunction::isLeafCondition(C)) {

vsk wrote:
> alanphipps wrote:
> > vsk wrote:
> > > Is there some alternative to pushing and then immediately popping branch 
> > > regions? Did you try adding the region directly to the function's 
> > > SourceRegions, and find that there was some issue with the more direct 
> > > approach?
> > The push/pop combo does look strange, and you're right -- I can just add it 
> > directly to SourceRegions, but the algorithm in popRegions() will do 
> > additional things like adjust the start/end location if the region spans a 
> > nested file depth. I think that's still useful to do.  Another option is I 
> > could factor that algorithm out of popRegions() into a new function and 
> > also call that from createBranchRegions().
> If it's possible/straightforward to factor the region start/end adjustment 
> code out of popRegions(), that would be really nice. If not, the current 
> approach looks reasonable.
I looked at it again, and I decided to table the refactor for now.  The 
adjustment code is coupled to SourceRegions as is the rest of popRegions(), and 
it may be necessary to rework the whole of the function to get a nicer refactor.



Comment at: llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp:659
 
+void SourceCoverageViewHTML::renderBranchView(raw_ostream &OS,
+  BranchView &BRV,

vsk wrote:
> alanphipps wrote:
> > vsk wrote:
> > > Wdyt of displaying branch-taken counts using tooltips? This would match 
> > > the way region execution counts are shown, which could be nice, because 
> > > the information wouldn't interrupt the flow of source code.
> > I think using tooltips is a great idea, and I did look into it.  
> > Ultimately, I decided to defer that work to a future patch to allow for 
> > some time to research a good way to properly distinguish the branch-taken 
> > counts from the region counts.
> Sounds reasonable. Can the branch coverage text/html output should be opt-in 
> via a cl::opt until it's finalized? That should make it possible to iterate 
> on the format without changing what users see.
Yep, I added two options:

--show-branch-summary, which shows the total branch coverage in the summary 
report, and this is ON by default.
--show-branches={count,percent}, which shows source-level coverage on a given 
file, which is OFF by default. So it's opt-in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84467

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Alan Phipps via Phabricator via cfe-commits]

alanphipps updated this revision to Diff 287269.
alanphipps marked 2 inline comments as done.
alanphipps added a comment.

Updating for formatting and comments (and some test adjustments after rebase).  
Bypassing logical-NOT operators in CodeGenFunction::isLeafCondition(), which 
checks the condition for the presence of logical-AND and logical-OR. (this can 
be further expanded, if necessary)


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

https://reviews.llvm.org/D84467

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/test/CoverageMapping/branch-constfolded.cpp
  clang/test/CoverageMapping/branch-logical-mixed.cpp
  clang/test/CoverageMapping/branch-macros.cpp
  clang/test/CoverageMapping/branch-mincounters.cpp
  clang/test/CoverageMapping/branch-templates.cpp
  llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/test/tools/llvm-cov/branch-c-general.c
  llvm/test/tools/llvm-cov/branch-export-json.test
  llvm/test/tools/llvm-cov/branch-export-lcov.test
  llvm/test/tools/llvm-cov/branch-logical-mixed.cpp
  llvm/test/tools/llvm-cov/branch-noShowBranch.test
  llvm/tools/llvm-cov/CodeCoverage.cpp
  llvm/tools/llvm-cov/CoverageExporterJson.cpp
  llvm/tools/llvm-cov/CoverageExporterLcov.cpp
  llvm/tools/llvm-cov/CoverageReport.cpp
  llvm/tools/llvm-cov/CoverageSummaryInfo.cpp
  llvm/tools/llvm-cov/CoverageViewOptions.h
  llvm/tools/llvm-cov/SourceCoverageView.cpp
  llvm/tools/llvm-cov/SourceCoverageView.h
  llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
  llvm/tools/llvm-cov/SourceCoverageViewText.cpp

Index: llvm/tools/llvm-cov/SourceCoverageViewText.cpp
===
--- llvm/tools/llvm-cov/SourceCoverageViewText.cpp
+++ llvm/tools/llvm-cov/SourceCoverageViewText.cpp
@@ -223,14 +223,13 @@
   /*ShowTitle=*/false, ViewDepth + 1);
 }
 
-void SourceCoverageViewText::renderBranchView(raw_ostream &OS,
-  BranchView &BRV,
+void SourceCoverageViewText::renderBranchView(raw_ostream &OS, BranchView &BRV,
   unsigned ViewDepth) {
   // Render the child subview.
   if (getOptions().Debug)
 errs() << "Branch at line " << BRV.getLine() << '\n';
 
-  for (const auto R : BRV.Regions) {
+  for (const auto &R : BRV.Regions) {
 double TruePercent = 0.0;
 double FalsePercent = 0.0;
 unsigned Total = R.ExecutionCount + R.FalseExecutionCount;
@@ -249,8 +248,9 @@
 }
 
 colored_ostream(OS, raw_ostream::RED,
-getOptions().Colors && !R.ExecutionCount,
-/*Bold=*/false, /*BG=*/true) << "True";
+getOptions().Colors && !R.ExecutionCount,
+/*Bold=*/false, /*BG=*/true)
+<< "True";
 
 if (getOptions().ShowBranchCounts)
   OS << ": " << formatCount(R.ExecutionCount) << ", ";
@@ -258,8 +258,9 @@
   OS << ": " << format("%0.2f", TruePercent) << "%, ";
 
 colored_ostream(OS, raw_ostream::RED,
-getOptions().Colors && !R.FalseExecutionCount,
-/*Bold=*/false, /*BG=*/true) << "False";
+getOptions().Colors && !R.FalseExecutionCount,
+/*Bold=*/false, /*BG=*/true)
+<< "False";
 
 if (getOptions().ShowBranchCounts)
   OS << ": " << formatCount(R.FalseExecutionCount);
Index: llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
===
--- llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
+++ llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
@@ -656,8 +656,7 @@
   OS << EndExpansionDiv;
 }
 
-void SourceCoverageViewHTML::renderBranchView(raw_ostream &OS,
-  BranchView &BRV,
+void SourceCoverageViewHTML::renderBranchView(raw_ostream &OS, BranchView &BRV,
   unsigned ViewDepth) {
   // Render the child subview.
   if (getOptions().Debug)
@@ -665,9 +664,8 @@
 
   OS << BeginExpansionDiv;
   OS << BeginPre;
-  for (const auto R : BRV.Regions)
-  {
-// Calculate TruePercent and False Percent
+  for (const auto &R : BRV.Regions) {
+// Calculate TruePercent and False Percent.
 double TruePercent = 0.0;
 double FalsePercent = 0.0;
 unsigned Total = R.ExecutionCount + R.FalseExecutionCount;
@@ -677,21 +675,24 @@
   FalsePercent = ((double)(R.FalseExecutionCount) / (double)Total) * 100.0;
 }
 
-// Display Line + Column
+// Display Line + Column.
 std::string LineNoStr = utostr(uint64_t(R.LineStart));
 std::string ColNoStr = utostr(uint64_t(R.ColumnStart));
 std::string TargetName = "L" + LineNoStr;

[PATCH] D86412: [clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.

[dana koch via Phabricator via cfe-commits]

3405691582 updated this revision to Diff 287263.
3405691582 edited the summary of this revision.
3405691582 added a comment.

Those platforms still use the legacy mechanism in `InitHeaderSearch.cpp`. For 
platforms which have moved include handling to the driver, the mechanism in 
`InitHeaderSearch::AddDefaultIncludePaths` becomes a noop.

This reminds me of a couple of things: we should also ensure that include 
management gets delegated in the same way -- it doesn't cause an immediate 
problem, which is why I missed it here -- so I've updated the diff and summary 
here to do that. Furthermore, as I understand it, the lack of `/usr/include` is 
only applicable when something is invoking clang programatically rather than 
with the frontend.

(In the concrete, this is exposed with the Swift programming language and its 
clang-based C interop.)


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

https://reviews.llvm.org/D86412

Files:
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/lib/Frontend/InitHeaderSearch.cpp


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -270,6 +270,7 @@
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::OpenBSD:
 llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
@@ -423,6 +424,7 @@
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
+  case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::AIX:
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -65,6 +65,12 @@
 return ToolChain::CST_Libcxx;
   }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const override;
+
+  bool HasNativeLLVMSupport() const override;
+
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -10,10 +10,12 @@
 #include "Arch/Mips.h"
 #include "Arch/Sparc.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -278,3 +280,38 @@
   options::OPT_fno_use_init_array, false))
 CC1Args.push_back("-fno-use-init-array");
 }
+
+void OpenBSD::AddClangSystemIncludeArgs(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+
+  if (DriverArgs.hasArg(clang::driver::options::OPT_nostdinc))
+return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(D.ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+return;
+
+  // Check for configure-time C include directories.
+  StringRef CIncludeDirs(C_INCLUDE_DIRS);
+  if (CIncludeDirs != "") {
+SmallVector dirs;
+CIncludeDirs.split(dirs, ":");
+for (StringRef dir : dirs) {
+  StringRef Prefix =
+  llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+  addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
+}
+return;
+  }
+
+  addExternCSystemInclude(DriverArgs, CC1Args, D.SysRoot + "/usr/include");
+}
+
+bool OpenBSD::HasNativeLLVMSupport() const { return true; }


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -270,6 +270,7 @@
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::OpenBSD:
 llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
@@ -423,6 +424,7 @@
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
+  case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::AIX:
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/O

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Caroline via Phabricator via cfe-commits]

CarolineConcatto updated this revision to Diff 287270.
CarolineConcatto added a comment.

Address review comments

Replace namespace flang for Fortran::frontend 
and fix style


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/lib/Tooling/Tooling.cpp
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/unittests/Driver/SanitizerArgsTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  flang/CMakeLists.txt
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/FrontendTool/Utils.h
  flang/lib/CMakeLists.txt
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/CMakeLists.txt
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/CMakeLists.txt
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-error-diagnostic.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/lit.cfg.py
  flang/test/lit.site.cfg.py.in
  flang/tools/CMakeLists.txt
  flang/tools/flang-driver/CMakeLists.txt
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/CMakeLists.txt
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/CompilerInstanceTest.cpp
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -612,7 +612,19 @@
 unsigned Flags = getInfo(Id).Flags;
 if (FlagsToInclude && !(Flags & FlagsToInclude))
   continue;
-if (Flags & FlagsToExclude)
+// If `Flags` is in both the Exclude set and in the Include set - display
+// it.
+if ((Flags & FlagsToExclude) && !(Flags & FlagsToInclude))
+  continue;
+
+// If `Flags` is empty (i.e. it's an option without any flags) then this is
+// a Clang-only option. If:
+// * we _are not_ in Flang Mode (FlagsToExclude contains FlangMode), then
+//   display it.
+// * we _are_ in Flang mode (FlagsToExclude does not contain FlangMode),
+//  don't display it.
+if (!Flags &&
+(FlagsToExclude & /*clang::driver::options::FlangMode*/ (1 << 14)))
   continue;
 
 // If an alias doesn't have a help text, show a help text for the aliased
Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -0,0 +1,52 @@
+//===- unittests/Frontend/CompilerInstanceTest.cpp - CI tests -===//
+//
+// 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
+//
+//===--===//
+
+#include "flang/Frontend/CompilerInstance.h"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include 
+using namespace llvm;
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  // 1. Set-up a basic DiagnosticConsumer
+  std::string diagnosticOutput;
+  llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
+  auto diagPrinter = std::make_unique(
+  diagnosticsOS, new clang::DiagnosticOptions());
+
+  // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
+  CompilerInstance compInst;
+
+  // 3. Set-up DiagnosticOptions
+  auto diagOpts = new clang::DiagnosticOptions();
+  // Tell the diagnostics engine to emit the diagnostic log to STDERR. This
+  // ensures that a chained diagnostic consumer is created so that the test can
+  // exercise the unowned diagnostic consumer in a chained consumer.
+  diagOpts->DiagnosticLogFile = "-";
+
+  // 4. Create a DiagnosticEngine with an unowned consumer
+  IntrusiveRefCntPtr diags =
+  compInst.CreateDiagnostics(diagOpts, diagPrinter.get(),
+  /*ShouldOwnClient=*/false);
+
+  // 5. Report a diagnostic
+  diags->Report(clang::diag::err_

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Caroline via Phabricator via cfe-commits]

CarolineConcatto marked 17 inline comments as done.
CarolineConcatto added a comment.

Hey Folks,
Fist of all, thank you very much for your review. 
I have answered some questions and update some of the requests. 
There are some that I did not had time to go inside yet.
I have the week of 24th off. IN my return I will tackle all the reviews as:
 out of build tree 
 and some question about the driver




Comment at: flang/include/flang/Frontend/CompilerInstance.h:11
+
+#include "flang/Frontend/CompilerInvocation.h"
+

tskeith wrote:
> Why is this called "Frontend" rather than "Driver"?
The way we think is that the driver is split in:
**the driver** libDriver -> that implements the driver. ATM it is inside 
clang/lib/Driver. And it can be clang( called inside 
clang/tools/driver/driver.cpp) or flang-new(called inside 
flang/tools/driver/driver.cpp) according to the mode the driver is called.
**the frontend driver**  -> it can be:
 clang frontend driver: clang -cc1  that calls libclangfrontend 
 flang frontend driver: flang-new -fc1 that calls libflangfrontend. 
the -xc1 has its functions/methods implemented inside the driver Frontend 

So this folder is called Frontend because it belongs to the part that 
implements the driver frontend of the compiler and we would like to leave this 
way so it has the same design as clang.
 



Comment at: flang/include/flang/Frontend/CompilerInstance.h:21
+  /// The options used in this compiler instance.
+  std::shared_ptr Invocation;
+

tskeith wrote:
> Data members, local variables, etc. should start with lower case.
> Non-public data members should end with underscore.
> 
> Please follow the style in flang/documentation/C++style.md.
I believe I have changed the style of the patch to be flang style.



Comment at: flang/include/flang/Frontend/FrontendOptions.h:31
+  Language Lang;
+  unsigned Fmt : 3;
+  unsigned Preprocessed : 1;

tskeith wrote:
> Why isn't the type `Format`?
This is to not mistake with: enum Format { Source, ModuleMap, Precompiled };



Comment at: flang/include/flang/Frontend/FrontendOptions.h:32
+  unsigned Fmt : 3;
+  unsigned Preprocessed : 1;
+

tskeith wrote:
> Why isn't the type `bool`?
Most of these things are going to be set by option::driver.  That is the reason 
it is as it is.



Comment at: flang/include/flang/Frontend/FrontendOptions.h:65
+  /// Show the -version text.
+  unsigned ShowVersion : 1;
+

tskeith wrote:
> Why aren't the types `bool`?
Most of these things are going to be set by option::driver. That is the reason 
it is as it is.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:46
+  InputKind DashX(Language::Unknown);
+  return DashX;
+}

tskeith wrote:
> Why not `return InputKind{Language::Unknown};`?
Just because the next lines also return DashX.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:20
+
+using namespace flang;
+namespace flang {

tskeith wrote:
> What is this for?
Because of CompilerInstance *Flang.



Comment at: flang/test/lit.cfg.py:40
+# exclude the tests for flang_new driver while there are two drivers
+if config.include_flang_new_driver_test == "OFF":
+  config.excludes = ['Inputs', 'CMakeLists.txt', 'README.txt', 'LICENSE.txt', 
'Flang-Driver']

richard.barton.arm wrote:
> richard.barton.arm wrote:
> > I think it would be cleaner to define config.excludes unconditionally then 
> > append the Flang-Driver dir if our condition passes.
> I _think_ the pattern to follow to exclude tests for something you haven't 
> built is to use lit features.
> 
> So you would add a feature like:
> `config.available_features.add("new-driver")`
> 
> then each test that only works on the new driver would be prefixed with a 
> statement:
> 
> `REQUIRES: new-driver`
> 
> This means that the tests can go in the test/Driver directory and you don't 
> need to create a new directory for these tests or this hack. The additional 
> benefit would be that all the existing tests for the throwaway driver can be 
> re-used on the new Driver to test it. There are not many of those though and 
> we are using a different driver name so they can't be shared either. Still 
> think it would be a worthwhile thing to do because when looking at the test 
> itself it is clear why it is not being run whereas with this hack it is 
> hidden away.
> 
>  Sorry for not thinking this first time around.
I like to have the test at a different folder for now so it is clear that the 
tests inside this folder all belongs to the new driver. So I don't need to open 
the test to know.
I can implement the requires, but in the future will not need the REQUIRES for 
the driver test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llv

[PATCH] D86412: [clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.

[Brad Smith via Phabricator via cfe-commits]

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

I was wondering why we had not seen any issues like this but your followup post 
clarified the situation.

Thank you.

LGTM.


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

https://reviews.llvm.org/D86412

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

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

[Mikhail Ramalho via Phabricator via cfe-commits]

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

In D86223#2231991 , @steakhal wrote:

> In D86223#2231959 , @mikhail.ramalho 
> wrote:
>
>> I don't mind having it for release builds as well, why are you applying it 
>> only for debug builds?
>
> It might introduce a slight overhead since Z3 will parse longer the symbol 
> names.

That overhead should be negligible, in the worst case you are adding just a few 
extra characters to the variable's name.

I rather have it for release builds as well so we don't introduce different 
outputs depending on the build options, and we can improve debugging for 
release builds as well.

Also as a bonus, we don't have to change the test scripts for a single test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86223

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

[PATCH] D86412: [clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.

[dana koch via Phabricator via cfe-commits]

3405691582 added a comment.

Thanks! please commit on my be half as appropriate.


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

https://reviews.llvm.org/D86412

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

[clang] 2b37174 - [clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2020-08-23T20:08:40-04:00
New Revision: 2b37174b9a5db235e493cb72e4454cc08a1b1791

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

LOG: [clang][Driver] Implement AddClangSystemIncludeArgs and 
HasNativeLLVMSupport for the OpenBSD clang driver.

If not overridden, AddClangSystemIncludeArgs's implementation is empty, so by
default, no system include args are added to the Clang driver. This means that
invoking Clang without the frontend must include a manual -I/usr/include flag,
which is inconsistent behavior. Therefore, override and implement this method
to match. Some boilerplate is also borrowed for handling of the other driver
flags.

While we are here, also override and enable HasNativeLLVMSupport.

Patch by: 3405691582 (dana koch)

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/OpenBSD.cpp
clang/lib/Driver/ToolChains/OpenBSD.h
clang/lib/Frontend/InitHeaderSearch.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp 
b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index 9c1a9c5f8228..b0174ac62b58 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -10,10 +10,12 @@
 #include "Arch/Mips.h"
 #include "Arch/Sparc.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -278,3 +280,38 @@ void OpenBSD::addClangTargetOptions(const ArgList 
&DriverArgs,
   options::OPT_fno_use_init_array, false))
 CC1Args.push_back("-fno-use-init-array");
 }
+
+void OpenBSD::AddClangSystemIncludeArgs(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+
+  if (DriverArgs.hasArg(clang::driver::options::OPT_nostdinc))
+return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(D.ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+return;
+
+  // Check for configure-time C include directories.
+  StringRef CIncludeDirs(C_INCLUDE_DIRS);
+  if (CIncludeDirs != "") {
+SmallVector dirs;
+CIncludeDirs.split(dirs, ":");
+for (StringRef dir : dirs) {
+  StringRef Prefix =
+  llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+  addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
+}
+return;
+  }
+
+  addExternCSystemInclude(DriverArgs, CC1Args, D.SysRoot + "/usr/include");
+}
+
+bool OpenBSD::HasNativeLLVMSupport() const { return true; }

diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.h 
b/clang/lib/Driver/ToolChains/OpenBSD.h
index 897eee57ab68..9924aa22e9d9 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.h
+++ b/clang/lib/Driver/ToolChains/OpenBSD.h
@@ -65,6 +65,12 @@ class LLVM_LIBRARY_VISIBILITY OpenBSD : public Generic_ELF {
 return ToolChain::CST_Libcxx;
   }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const override;
+
+  bool HasNativeLLVMSupport() const override;
+
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 

diff  --git a/clang/lib/Frontend/InitHeaderSearch.cpp 
b/clang/lib/Frontend/InitHeaderSearch.cpp
index 16f1f1670e8d..bc31445d6d08 100644
--- a/clang/lib/Frontend/InitHeaderSearch.cpp
+++ b/clang/lib/Frontend/InitHeaderSearch.cpp
@@ -270,6 +270,7 @@ void InitHeaderSearch::AddDefaultCIncludePaths(const 
llvm::Triple &triple,
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::OpenBSD:
 llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
@@ -423,6 +424,7 @@ void InitHeaderSearch::AddDefaultIncludePaths(const 
LangOptions &Lang,
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
+  case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::AIX:



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

[PATCH] D86412: [clang][Driver] Implement AddClangSystemIncludeArgs and HasNativeLLVMSupport for the OpenBSD clang driver.

[Brad Smith via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b37174b9a5d: [clang][Driver] Implement 
AddClangSystemIncludeArgs and HasNativeLLVMSupport… (authored by brad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86412

Files:
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/lib/Frontend/InitHeaderSearch.cpp


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -270,6 +270,7 @@
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::OpenBSD:
 llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
@@ -423,6 +424,7 @@
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
+  case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::AIX:
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -65,6 +65,12 @@
 return ToolChain::CST_Libcxx;
   }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const override;
+
+  bool HasNativeLLVMSupport() const override;
+
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -10,10 +10,12 @@
 #include "Arch/Mips.h"
 #include "Arch/Sparc.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -278,3 +280,38 @@
   options::OPT_fno_use_init_array, false))
 CC1Args.push_back("-fno-use-init-array");
 }
+
+void OpenBSD::AddClangSystemIncludeArgs(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+
+  if (DriverArgs.hasArg(clang::driver::options::OPT_nostdinc))
+return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+SmallString<128> Dir(D.ResourceDir);
+llvm::sys::path::append(Dir, "include");
+addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+return;
+
+  // Check for configure-time C include directories.
+  StringRef CIncludeDirs(C_INCLUDE_DIRS);
+  if (CIncludeDirs != "") {
+SmallVector dirs;
+CIncludeDirs.split(dirs, ":");
+for (StringRef dir : dirs) {
+  StringRef Prefix =
+  llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+  addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
+}
+return;
+  }
+
+  addExternCSystemInclude(DriverArgs, CC1Args, D.SysRoot + "/usr/include");
+}
+
+bool OpenBSD::HasNativeLLVMSupport() const { return true; }


Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -270,6 +270,7 @@
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
+  case llvm::Triple::OpenBSD:
 llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
@@ -423,6 +424,7 @@
   case llvm::Triple::Emscripten:
   case llvm::Triple::Linux:
   case llvm::Triple::Hurd:
+  case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::AIX:
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -65,6 +65,12 @@
 return ToolChain::CST_Libcxx;
   }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const override;
+
+  bool HasNativeLLVMSupport() const override;
+
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenB

[clang] bf3577e - [clang][Driver] Implement addLibCxxIncludePaths and getCompilerRT for the OpenBSD clang driver.

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2020-08-23T20:44:29-04:00
New Revision: bf3577ef64c300ba7841a90a4e09e1e305271976

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

LOG: [clang][Driver] Implement addLibCxxIncludePaths and getCompilerRT for the 
OpenBSD clang driver.

Added: 


Modified: 
clang/lib/Driver/ToolChains/OpenBSD.cpp
clang/lib/Driver/ToolChains/OpenBSD.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp 
b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index b0174ac62b58..4f2d04058d24 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -258,29 +258,6 @@ OpenBSD::OpenBSD(const Driver &D, const llvm::Triple 
&Triple,
   getFilePaths().push_back(getDriver().SysRoot + "/usr/lib");
 }
 
-void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
-  ArgStringList &CmdArgs) const {
-  bool Profiling = Args.hasArg(options::OPT_pg);
-
-  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
-  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
-}
-
-Tool *OpenBSD::buildAssembler() const {
-  return new tools::openbsd::Assembler(*this);
-}
-
-Tool *OpenBSD::buildLinker() const { return new tools::openbsd::Linker(*this); 
}
-
-void OpenBSD::addClangTargetOptions(const ArgList &DriverArgs,
-ArgStringList &CC1Args,
-Action::OffloadKind) const {
-  // Support for .init_array is still new (Aug 2016).
-  if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
-  options::OPT_fno_use_init_array, false))
-CC1Args.push_back("-fno-use-init-array");
-}
-
 void OpenBSD::AddClangSystemIncludeArgs(
 const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
@@ -314,4 +291,41 @@ void OpenBSD::AddClangSystemIncludeArgs(
   addExternCSystemInclude(DriverArgs, CC1Args, D.SysRoot + "/usr/include");
 }
 
+void OpenBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args) const {
+  addSystemInclude(DriverArgs, CC1Args,
+   getDriver().SysRoot + "/usr/include/c++/v1");
+}
+
+void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
+  ArgStringList &CmdArgs) const {
+  bool Profiling = Args.hasArg(options::OPT_pg);
+
+  CmdArgs.push_back(Profiling ? "-lc++_p" : "-lc++");
+  CmdArgs.push_back(Profiling ? "-lc++abi_p" : "-lc++abi");
+}
+
+std::string OpenBSD::getCompilerRT(const ArgList &Args,
+   StringRef Component,
+   FileType Type) const {
+  SmallString<128> Path(getDriver().SysRoot);
+  llvm::sys::path::append(Path, "/usr/lib/libcompiler_rt.a");
+  return std::string(Path.str());
+}
+
+void OpenBSD::addClangTargetOptions(const ArgList &DriverArgs,
+ArgStringList &CC1Args,
+Action::OffloadKind) const {
+  // Support for .init_array is still new (Aug 2016).
+  if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
+  options::OPT_fno_use_init_array, false))
+CC1Args.push_back("-fno-use-init-array");
+}
+
+Tool *OpenBSD::buildAssembler() const {
+  return new tools::openbsd::Assembler(*this);
+}
+
+Tool *OpenBSD::buildLinker() const { return new tools::openbsd::Linker(*this); 
}
+
 bool OpenBSD::HasNativeLLVMSupport() const { return true; }

diff  --git a/clang/lib/Driver/ToolChains/OpenBSD.h 
b/clang/lib/Driver/ToolChains/OpenBSD.h
index 9924aa22e9d9..09595faf9d6b 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.h
+++ b/clang/lib/Driver/ToolChains/OpenBSD.h
@@ -54,6 +54,8 @@ class LLVM_LIBRARY_VISIBILITY OpenBSD : public Generic_ELF {
   OpenBSD(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
 
+  bool HasNativeLLVMSupport() const override;
+
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
   bool isPIEDefault() const override { return true; }
@@ -69,11 +71,14 @@ class LLVM_LIBRARY_VISIBILITY OpenBSD : public Generic_ELF {
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
-  bool HasNativeLLVMSupport() const override;
-
+  void addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) const override;
 
+  std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef 
Compo

[PATCH] D85596: [Docs] Link to --print-supported-cpus option

[Travis Finkenauer via Phabricator via cfe-commits]

tmfink added inline comments.



Comment at: clang/docs/CommandGuide/clang.rst:346
 
-  Aliases of --print-supported-cpus
+  Aliases of :option:`--print-supported-cpus`
 

nickdesaulniers wrote:
> Would you mind adding a `.` for punctuation while you're here?
This is not a complete sentence (no predicate/verb), so from a grammar point of 
view, a period does not make sense.

It seems like the other options use complete sentences (with a period).

I'll reword this description to match the style of the other options (so a 
period will make sense).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85596

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

[PATCH] D86424: [clang] Do not consider the template arguments of bases to be bases themselves

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.
nridge requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86424

Files:
  clang/lib/Index/IndexTypeSourceInfo.cpp
  clang/unittests/Index/IndexTests.cpp


Index: clang/unittests/Index/IndexTests.cpp
===
--- clang/unittests/Index/IndexTests.cpp
+++ clang/unittests/Index/IndexTests.cpp
@@ -334,6 +334,20 @@
  WrittenAt(Position(3, 20);
 }
 
+TEST(IndexTest, RelationBaseOf) {
+  std::string Code = R"cpp(
+class A {};
+template  class B {};
+class C : B {};
+  )cpp";
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(std::make_unique(Index), Code);
+  // A should not be the base of anything.
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("A"), HasRole(SymbolRole::Reference),
+ Not(HasRole(SymbolRole::RelationBaseOf);
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: clang/lib/Index/IndexTypeSourceInfo.cpp
===
--- clang/lib/Index/IndexTypeSourceInfo.cpp
+++ clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -160,6 +160,25 @@
 return true;
   }
 
+  bool TraverseTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) 
{
+if (!WalkUpFromTemplateSpecializationTypeLoc(TL))
+  return false;
+if (!TraverseTemplateName(TL.getTypePtr()->getTemplateName()))
+  return false;
+
+// The relations we have to `Parent` do not apply to our template 
arguments,
+// so clear them while visiting the args.
+SmallVector SavedRelations = Relations;
+// Relations.clear();
+for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+  if (!TraverseTemplateArgumentLoc(TL.getArgLoc(I)))
+return false;
+}
+Relations = SavedRelations;
+
+return true;
+  }
+
   bool 
VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc 
TL) {
 auto *T = TL.getTypePtr();
 if (!T)


Index: clang/unittests/Index/IndexTests.cpp
===
--- clang/unittests/Index/IndexTests.cpp
+++ clang/unittests/Index/IndexTests.cpp
@@ -334,6 +334,20 @@
  WrittenAt(Position(3, 20);
 }
 
+TEST(IndexTest, RelationBaseOf) {
+  std::string Code = R"cpp(
+class A {};
+template  class B {};
+class C : B {};
+  )cpp";
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(std::make_unique(Index), Code);
+  // A should not be the base of anything.
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("A"), HasRole(SymbolRole::Reference),
+ Not(HasRole(SymbolRole::RelationBaseOf);
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: clang/lib/Index/IndexTypeSourceInfo.cpp
===
--- clang/lib/Index/IndexTypeSourceInfo.cpp
+++ clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -160,6 +160,25 @@
 return true;
   }
 
+  bool TraverseTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+if (!WalkUpFromTemplateSpecializationTypeLoc(TL))
+  return false;
+if (!TraverseTemplateName(TL.getTypePtr()->getTemplateName()))
+  return false;
+
+// The relations we have to `Parent` do not apply to our template arguments,
+// so clear them while visiting the args.
+SmallVector SavedRelations = Relations;
+// Relations.clear();
+for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+  if (!TraverseTemplateArgumentLoc(TL.getArgLoc(I)))
+return false;
+}
+Relations = SavedRelations;
+
+return true;
+  }
+
   bool VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc TL) {
 auto *T = TL.getTypePtr();
 if (!T)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

@vsavchenko @NoQ 
The patch has been updated as required. Is there anything I need to do with the 
patch?




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:56
   /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
+  /// Use `getEndPath` to customize the note associated with the report
   /// end instead.

vsavchenko wrote:
> Maybe it is better to create a reference to this function using the [[ 
> https://www.doxygen.nl/manual/commands.html#cmdref | \ref ]] command?
Yes, using `\ref` looks better. I will submit it again. To be similar to the 
original code, I will use `@ref getEndPath` instead.


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

https://reviews.llvm.org/D85105

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

[PATCH] D86426: [Docs] Fix --print-supported-cpus option rendering

[Travis Finkenauer via Phabricator via cfe-commits]

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

Adds link/code sample to avoid rendering two dashes as non-ASCII "en dash".
Also make wording a complete sentence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86426

Files:
  clang/docs/CommandGuide/clang.rst


Index: clang/docs/CommandGuide/clang.rst
===
--- clang/docs/CommandGuide/clang.rst
+++ clang/docs/CommandGuide/clang.rst
@@ -338,12 +338,12 @@
 .. option:: --print-supported-cpus
 
   Print out a list of supported processors for the given target (specified
-  through --target= or -arch ). If no target is
-  specified, the system default target will be used.
+  through ``--target=`` or :option:`-arch` ). 
If no
+  target is specified, the system default target will be used.
 
 .. option:: -mcpu=?, -mtune=?
 
-  Aliases of --print-supported-cpus
+  Acts as an alias for :option:`--print-supported-cpus`.
 
 .. option:: -march=
 


Index: clang/docs/CommandGuide/clang.rst
===
--- clang/docs/CommandGuide/clang.rst
+++ clang/docs/CommandGuide/clang.rst
@@ -338,12 +338,12 @@
 .. option:: --print-supported-cpus
 
   Print out a list of supported processors for the given target (specified
-  through --target= or -arch ). If no target is
-  specified, the system default target will be used.
+  through ``--target=`` or :option:`-arch` ). If no
+  target is specified, the system default target will be used.
 
 .. option:: -mcpu=?, -mtune=?
 
-  Aliases of --print-supported-cpus
+  Acts as an alias for :option:`--print-supported-cpus`.
 
 .. option:: -march=
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86427: Fix some spelling errors

[YangZhihui via Phabricator via cfe-commits]

YangZhihui created this revision.
YangZhihui added reviewers: peter.smith, plotfi.
YangZhihui added a project: clang.
Herald added subscribers: cfe-commits, dang.
YangZhihui requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86427

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -698,7 +698,7 @@
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, 
Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_interface_stubs : Flag<["-"], "emit-interface-stubs">, 
Flags<[CC1Option]>, Group,
-  HelpText<"Generate Inteface Stub Files.">;
+  HelpText<"Generate Interface Stub Files.">;
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
@@ -1809,7 +1809,7 @@
"This uses a loose heuristic which considers functions vulnerable 
if they "
"contain a char (or 8bit integer) array or constant sized calls to 
alloca "
", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
-   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "variable sized calls to alloca are considered vulnerable. A 
function with "
"a stack protector has a guard value added to the stack frame that 
is "
"checked on function exit. The guard value must be positioned in 
the "
"stack frame such that a buffer overflow from a vulnerable variable 
will "


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -698,7 +698,7 @@
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_interface_stubs : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, Group,
-  HelpText<"Generate Inteface Stub Files.">;
+  HelpText<"Generate Interface Stub Files.">;
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
@@ -1809,7 +1809,7 @@
"This uses a loose heuristic which considers functions vulnerable if they "
"contain a char (or 8bit integer) array or constant sized calls to alloca "
", which are of greater size than ssp-buffer-size (default: 8 bytes). All "
-   "variable sized calls to alloca are considered vulnerable. A function with"
+   "variable sized calls to alloca are considered vulnerable. A function with "
"a stack protector has a guard value added to the stack frame that is "
"checked on function exit. The guard value must be positioned in the "
"stack frame such that a buffer overflow from a vulnerable variable will "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86426: [Docs] Fix --print-supported-cpus option rendering

[Travis Finkenauer via Phabricator via cfe-commits]

tmfink abandoned this revision.
tmfink added a comment.

Accidentally created new review instead of updating 
https://reviews.llvm.org/D85596

Please ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86426

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

[PATCH] D85596: [Docs] Link to --print-supported-cpus option

[Travis Finkenauer via Phabricator via cfe-commits]

tmfink updated this revision to Diff 287290.
tmfink added a comment.

Fix rendering of --print-supported-cpus option and make alias documentation a
complete sentence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85596

Files:
  clang/docs/CommandGuide/clang.rst


Index: clang/docs/CommandGuide/clang.rst
===
--- clang/docs/CommandGuide/clang.rst
+++ clang/docs/CommandGuide/clang.rst
@@ -338,12 +338,12 @@
 .. option:: --print-supported-cpus
 
   Print out a list of supported processors for the given target (specified
-  through --target= or -arch ). If no target is
-  specified, the system default target will be used.
+  through ``--target=`` or :option:`-arch` ). 
If no
+  target is specified, the system default target will be used.
 
 .. option:: -mcpu=?, -mtune=?
 
-  Aliases of --print-supported-cpus
+  Acts as an alias for :option:`--print-supported-cpus`.
 
 .. option:: -march=
 


Index: clang/docs/CommandGuide/clang.rst
===
--- clang/docs/CommandGuide/clang.rst
+++ clang/docs/CommandGuide/clang.rst
@@ -338,12 +338,12 @@
 .. option:: --print-supported-cpus
 
   Print out a list of supported processors for the given target (specified
-  through --target= or -arch ). If no target is
-  specified, the system default target will be used.
+  through ``--target=`` or :option:`-arch` ). If no
+  target is specified, the system default target will be used.
 
 .. option:: -mcpu=?, -mtune=?
 
-  Aliases of --print-supported-cpus
+  Acts as an alias for :option:`--print-supported-cpus`.
 
 .. option:: -march=
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86424: [clang] Do not consider the template arguments of bases to be bases themselves

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 287291.
nridge added a comment.

Fix a typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86424

Files:
  clang/lib/Index/IndexTypeSourceInfo.cpp
  clang/unittests/Index/IndexTests.cpp


Index: clang/unittests/Index/IndexTests.cpp
===
--- clang/unittests/Index/IndexTests.cpp
+++ clang/unittests/Index/IndexTests.cpp
@@ -334,6 +334,20 @@
  WrittenAt(Position(3, 20);
 }
 
+TEST(IndexTest, RelationBaseOf) {
+  std::string Code = R"cpp(
+class A {};
+template  class B {};
+class C : B {};
+  )cpp";
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(std::make_unique(Index), Code);
+  // A should not be the base of anything.
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("A"), HasRole(SymbolRole::Reference),
+ Not(HasRole(SymbolRole::RelationBaseOf);
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: clang/lib/Index/IndexTypeSourceInfo.cpp
===
--- clang/lib/Index/IndexTypeSourceInfo.cpp
+++ clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -160,6 +160,25 @@
 return true;
   }
 
+  bool TraverseTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) 
{
+if (!WalkUpFromTemplateSpecializationTypeLoc(TL))
+  return false;
+if (!TraverseTemplateName(TL.getTypePtr()->getTemplateName()))
+  return false;
+
+// The relations we have to `Parent` do not apply to our template 
arguments,
+// so clear them while visiting the args.
+SmallVector SavedRelations = Relations;
+Relations.clear();
+for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+  if (!TraverseTemplateArgumentLoc(TL.getArgLoc(I)))
+return false;
+}
+Relations = SavedRelations;
+
+return true;
+  }
+
   bool 
VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc 
TL) {
 auto *T = TL.getTypePtr();
 if (!T)


Index: clang/unittests/Index/IndexTests.cpp
===
--- clang/unittests/Index/IndexTests.cpp
+++ clang/unittests/Index/IndexTests.cpp
@@ -334,6 +334,20 @@
  WrittenAt(Position(3, 20);
 }
 
+TEST(IndexTest, RelationBaseOf) {
+  std::string Code = R"cpp(
+class A {};
+template  class B {};
+class C : B {};
+  )cpp";
+  auto Index = std::make_shared();
+  tooling::runToolOnCode(std::make_unique(Index), Code);
+  // A should not be the base of anything.
+  EXPECT_THAT(Index->Symbols,
+  Contains(AllOf(QName("A"), HasRole(SymbolRole::Reference),
+ Not(HasRole(SymbolRole::RelationBaseOf);
+}
+
 } // namespace
 } // namespace index
 } // namespace clang
Index: clang/lib/Index/IndexTypeSourceInfo.cpp
===
--- clang/lib/Index/IndexTypeSourceInfo.cpp
+++ clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -160,6 +160,25 @@
 return true;
   }
 
+  bool TraverseTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+if (!WalkUpFromTemplateSpecializationTypeLoc(TL))
+  return false;
+if (!TraverseTemplateName(TL.getTypePtr()->getTemplateName()))
+  return false;
+
+// The relations we have to `Parent` do not apply to our template arguments,
+// so clear them while visiting the args.
+SmallVector SavedRelations = Relations;
+Relations.clear();
+for (unsigned I = 0, E = TL.getNumArgs(); I != E; ++I) {
+  if (!TraverseTemplateArgumentLoc(TL.getArgLoc(I)))
+return false;
+}
+Relations = SavedRelations;
+
+return true;
+  }
+
   bool VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc TL) {
 auto *T = TL.getTypePtr();
 if (!T)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86427: Fix some spelling errors

[Puyan Lotfi via Phabricator via cfe-commits]

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

LGTM. Go ahead and land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86089: [flang][driver]Add experimental flang driver and frontend with help screen

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added inline comments.



Comment at: flang/tools/flang-driver/fc1_main.cpp:1
+//===-- fc1_main.cpp - Flang FC1 Compiler Frontend 
===//
+//

This file-name seems odd considering LLVM style. How about just `Flang.cpp` or 
`FlangMain.cpp` ? or other


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86089

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

 The most immediate problem is divergence analysis, which is extremely 
 complex and difficult to get right. If I had tried to fight the accidental 
 complexity that comes with attempting to write such an algorithm as C++ 
 templates in addition to the inherent complexity of the algorithm at the 
 same time, I'm not sure I would have been able to produce anything 
 workable at all.

 Frankly, I suspect that our dominator tree implementation also suffer 
 because of this, though at least dominator trees are much more well 
 studied in the academic literature, so that helps keep the inherent 
 complexity under control.
>>>
>>> I'm totally open to discussing making APIs more usable, for sure - though 
>>> I'm thinking it's likely a concept (like containers in the C++ standard 
>>> library) might be the better direction.
>>>
>>> Perhaps some code samples showing how one would interact (probably not 
>>> whole algorithms - maybe something simple like generating a dot diagram for 
>>> a graph) with these things given different APIs (traits, concepts, and 
>>> runtime polymorphism) - and implementations of each kind too.
>>
>> Take a look here for example: 
>> https://github.com/nhaehnle/llvm-project/blob/715450fa7f968ceefaf9c3b04b47066866c97206/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L499
>>  -- this is obviously still fairly simple, but it's an example of printing 
>> out the results of an analysis in a way that's generic over the underlying 
>> CFG and SSA form.
>
> I'm having trouble following this example - I'm not sure what the CfgPrinter 
> abstraction is/why it's first-class, and why this "print" function is calling 
> what look like mutation operations like "appendBlocks". I guess perhaps the 
> question is - what's it printing from and what's it printing to?
>
> Ah, I see, the "append" functions are accessors, of a sort. Returning a 
> container might be more clear than using an out parameter - alternatively, a 
> functor parameter (ala std::for_each) that is called for each element, that 
> can then be used to populate an existing container if desired, or to do 
> immediate processing without the need for an intermediate container.

The code is trying to strike a balance here in terms of performance. Since 
dynamic polymorphism is used, a functor-based traversal can't be inlined and so 
the number of indirect function calls increases quite a bit. There are a number 
of use cases where you really do want to just append successors or predecessors 
to a vectors, like during a graph traversal. An example graph traversal is 
here: 
https://github.com/nhaehnle/llvm-project/blob/controlflow-wip-v7/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L329

> Though the printer abstraction still strikes me as a bit strange - especially 
> since it doesn't seem to be printing itself. This function was passed a 
> printer and a stream - the printer prints to the stream (perhaps it'd make 
> more sense for the printer to take the stream on construction) and the 
> function isn't passed the thing to print at all - that thing is accessed from 
> the printer. That seems fairly awkward to me - I'd expect a printing 
> operation to take a thing to be printed and a thing to print to.

Keep in mind that where those `printBlockName` / `printValue` methods are used, 
they will always be mixed in with printing of other things. So the alternative 
you're describing would result in code like:

  dbgs() << " currently at: ";  // explicit stream
  printer.printBlockName(block); // implicit stream
  dbgs() << '\n'; // explicit stream

I would argue that that ends up being more awkward because it mixes implicit 
streams with explicitly given streams.

We could perhaps add versions of the methods that return a `Printable` object, 
so that we can write:

  dbgs() << "currently at: " << printer.printableBlockName(block) << '\n';

By the way, the main motivation for making the CfgPrinter a separate object is 
that printing LLVM IR efficiently requires keeping a ModuleSlotTracker around. 
Splitting the CfgPrinter off from the CfgInterface allows the fast-path of code 
that doesn't want debug prints to not have to carry a reference to a 
ModuleSlotTracker around, even if it's just an empty std::unique_ptr. That's a 
comparatively small burden though, so I'd be fine with merging the CfgPrinter 
back into the CfgInterface.

> Perhaps setting aside the complexities of printing things - could you provide 
> an example of code, given a CfgGraph, that walks the graph - perhaps just 
> numbering the nodes/edges/etc to produce a dot graph? Showing what the code 
> would look like if it were passed a GraphTraits-implementing graph, a static 
> polymorphic CfgGraph, and a dynamically polymorphic GfgGraph - and also 
> showing what would be fandemantally possible with the CfgGraph that wouldn't 
> be possible with GraphTraits, if any such things exist (it's still unclear to 
>