[PATCH] D63607: [clang][driver] Add basic --driver-mode=fortran support for flang

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 219666.
peterwaller-arm added a comment.

Fixed assertion message "Input output." => "Invalid output". The erroneous text 
came was copied from: 
https://github.com/llvm/llvm-project/blob/6b9df910d04fae62dacc22c1c84f66c0f126cde0/clang/lib/Driver/ToolChains/Clang.cpp#L3849


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

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,21 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/flang.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang.f90
@@ -0,0 +1,56 @@
+! Check that flang -fc1 is invoked when in --driver-mode=fortran.
+
+! See also: flang.F90 - "an input which also requires preprocessing".
+
+! Test various output types:
+! * The default (-emit-obj)
+! * -fsyntax-only
+! * -emit-llvm
+! * -emit-llvm -S
+! * -S
+
+! Most tests use ALL-LABEL to bracket the checks with the compiler invocation and the source, which appear at the beginning and end.
+! Use of "...-DAG" allows the order of options to change within that bracket.
+
+
+! All invocations should begin with flang -fc1, consume up to here.
+! ALL-LABEL: "{{[^"]*}}flang" "-fc1"
+
+! RUN: %clang --driver-mode=fortran -### %s 2>&1 | FileCheck --check-prefixes=ALL,CHECK-PLAIN %s
+! CHECK-PLAIN-DAG: "-triple"
+! CHECK-PLAIN-DAG: "-emit-obj"
+! CHECK-PLAIN-DAG: "-o" "{{[^"]*}}.o"
+
+! RUN: %clang --driver-mode=fortran -emit-ast-### %s 2>&1 | FileCheck --check-prefixes=ALL,CHECK-EMIT-AST %s
+! CHECK-EMIT-AST-DAG: "-triple"
+! CHECK-EMIT-AST-DAG: "-emit-ast"
+! CHECK-EMIT-AST-DAG: "-o" "{{[^"]*}}.ast"
+
+! RUN: %clang --driver-mode=fortran -fsyntax-only   -### %s 2>&1 | FileCheck --check-prefixes=ALL,CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-NOT: "-o"
+! CHECK-SYNTAX-ONLY-DAG: "-fsyntax-only"
+
+! RUN: %clang --driver-mode=fortran -emit-llvm  -### %s 2>&1 | FileCheck --check-prefixes=ALL,CHECK-EMIT-LLVM %s
+! CHECK-EMIT-LLVM-NOT: "-emit-llvm"
+! CHECK-EMIT-LLVM-DAG: "-emit-llvm-uselists"
+! CHECK-EMIT-LLVM-DAG: "-emit-llvm-bc"
+! CHECK-EMIT-LLVM-DAG: "-o" "{{[^"]*}}.bc"
+
+! RUN: %clang --driver-mode=fortran -emit-llvm -S   -### %s 2>&1 | FileCheck --check-prefixes=ALL,CHECK-EMIT-LLVM-S %s
+! CHECK-EMIT-LLVM-S-DAG: "-emit-llvm"
+! CHECK-EMIT-LLVM-S-DAG: "-o" "{{[^"]*}}.ll"
+
+! RUN: 

[PATCH] D67135: [clang-tidy] performance-inefficient-vector-operation: Support proto repeated field

[Haojian Wu via Phabricator via cfe-commits]

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

thanks, looks good.




Comment at: 
clang-tools-extra/test/clang-tidy/performance-inefficient-vector-operation.cpp:315
+foo.mutable_x();
+// CHECK-FIXES-NOT: foo.mutable_x->Reserve(5);
+for (int i = 0; i < 5; i++) {

looks like we miss `()`, the same below.


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

https://reviews.llvm.org/D67135



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

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

krisb updated this revision to Diff 219670.
krisb added a comment.

Rebased


Repository:
  rC Clang

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

https://reviews.llvm.org/D66018

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c

Index: test/Driver/arm-features.c
===
--- test/Driver/arm-features.c
+++ test/Driver/arm-features.c
@@ -37,7 +37,8 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
-// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
+// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto:
 //
@@ -45,14 +46,36 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // CHECK-NOCRYPTO2-NOT: "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
 //
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2-CPU %s
+// CHECK-CRYPTO2-CPU: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+norypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// CHECK-NOCRYPTO2-CPU-NOT: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
+//
 // Check +crypto -sha2 -aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nosha2+noaes -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
 // CHECK-CRYPTO3-NOT: "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto +sha2 +aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+nocrypto+sha2+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nocrypto+sha2+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+sha2+aes -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
 // CHECK-CRYPTO4: "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check +crypto for M and R profiles:
+//
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-r+crypto   -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.base+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8-m.main+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-m23+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO5 %s
+// CHECK-NOCRYPTO5: warning: ignoring extension 'crypto' because the {{.*}} architecture does not support it
+// CHECK-NOCRYPTO5-NOT: "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -477,23 +477,26 @@
   Features.push_back("-crc");
   }
 
-  // For Arch >= ARMv8.0:  crypto = sha2 + aes
+  // For Arch >= A

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio marked 4 inline comments as done.
dnsampaio added a comment.

Hi @jfb. In a example such as:

  struct { int a : 1; int b : 16; } S;
  extern int expensive_computaion(int v);
  void foo(volatile S* s){
s->b = expensive_computation(s->b);
  }

There is no guarantee that `s->a` is not modified during a expensive 
computation, so it must be obtained just before writing the `s->b` value, as 
`a` and `b` share the same memory position. This is already done by llvm. 
Indeed, the exact output would be

  define void @foo(%struct.S* %S) local_unnamed_addr #0 {
  entry:
%0 = bitcast %struct.S* %S to i32*
%bf.load = load volatile i32, i32* %0, align 4
%bf.shl = shl i32 %bf.load, 15
%bf.ashr = ashr i32 %bf.shl, 16
%call = tail call i32 @expensive_computation(i32 %bf.ashr) #2
%bf.load1 = load volatile i32, i32* %0, align 4 ; <<<== Here it obtains the 
value to s->a to restore it.
%bf.value = shl i32 %call, 1
%bf.shl2 = and i32 %bf.value, 131070
%bf.clear = and i32 %bf.load1, -131071
%bf.set = or i32 %bf.clear, %bf.shl2
store volatile i32 %bf.set, i32* %0, align 4
ret void
  }

These extra loads here are required to make uniform the number of times the 
volatile bitfield is read, independent if they share or not memory with other 
data.

We could have it under a flag, such as `-faacps-volatilebitfield`, disabled by 
default.

The other point not conformant to the AACPS is the width of the loads/stores 
used to obtain bitfields. They should be the width of the container, if it does 
that would not overlap with non-bitfields. Do you have any idea where that 
could be computed? I imagine that would be when computing the alignment of the 
elements of the structure, where I can check if the performing the entire 
container width load would overlap with other elements. Could you point me 
where that is done?




Comment at: clang/test/CodeGen/aapcs-bitfield.c:541
 // BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], 
%struct.st9* [[M:%.*]], i32 0, i32 0
+// BE-NEXT:[[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // BE-NEXT:store volatile i8 1, i8* [[TMP0]], align 4

jfb wrote:
> These are just extra loads? Why?
Yes, these are just extra loads. As the AACPS describes, every write requires 
to perform a load as well, even if all bits of the volatile bitfield is going 
to be replaced.



Comment at: clang/test/CodeGen/aapcs-bitfield.c:552
 // LE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], 
%struct.st9* [[M:%.*]], i32 0, i32 0
 // LE-NEXT:[[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4
 // LE-NEXT:[[INC:%.*]] = add i8 [[BF_LOAD]], 1

jfb wrote:
> Why isn't this load sufficient?
Technically speaking, that is the load for reading the bitfield, not the load 
required when writing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67399



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

r371597 - [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Diogo N. Sampaio via cfe-commits]

Author: dnsampaio
Date: Wed Sep 11 02:06:17 2019
New Revision: 371597

URL: http://llvm.org/viewvc/llvm-project?rev=371597&view=rev
Log:
[ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

Submittin in behalf of krisb (Kristina Bessonova) 

Summary:
'+crypto' means '+aes' and '+sha2' for arch >= ARMv8 when they were
not disabled explicitly. But this is correctly handled only in case of
'-march' option, though the feature may also be specified through
the '-mcpu' or '-mfpu' options. In the following example:

  $ clang -mcpu=cortex-a57 -mfpu=crypto-neon-fp-armv8

'aes' and 'sha2' are disabled that is quite unexpected:

  $ clang -cc1 -triple armv8--- -target-cpu cortex-a57
<...> -target-feature -sha2 -target-feature -aes -target-feature +crypto

This exposed by https://reviews.llvm.org/D63936 that makes
the 'aes' and 'sha2' features disabled by default.

So, while handling the 'crypto' feature we need to take into account:
  - a CPU name, as it provides the information about architecture
(if no '-march' option specified),
  - features, specified by the '-mcpu' and '-mfpu' options.

Reviewers: SjoerdMeijer, ostannard, labrinea, dnsampaio

Reviewed By: dnsampaio

Subscribers: ikudrin, javed.absar, kristof.beyls, cfe-commits

Tags: #clang

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

Author: krisb

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
cfe/trunk/test/Driver/arm-features.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=371597&r1=371596&r2=371597&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Wed Sep 11 02:06:17 
2019
@@ -384,6 +384,9 @@ def warn_target_unsupported_abs2008 : Wa
 def warn_target_unsupported_compact_branches : Warning<
   "ignoring '-mcompact-branches=' option because the '%0' architecture does 
not"
   " support it">, InGroup;
+def warn_target_unsupported_extension : Warning<
+  "ignoring extension '%0' because the '%1' architecture does not support it">,
+   InGroup;
 def warn_drv_unsupported_gpopt : Warning<
   "ignoring '-mgpopt' option as it cannot be used with %select{|the implicit"
   " usage of }0-mabicalls">,

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp?rev=371597&r1=371596&r2=371597&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp Wed Sep 11 02:06:17 2019
@@ -477,23 +477,26 @@ fp16_fml_fallthrough:
   Features.push_back("-crc");
   }
 
-  // For Arch >= ARMv8.0:  crypto = sha2 + aes
+  // For Arch >= ARMv8.0 && A profile:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
-  if (ArchName.find_lower("armv8a") != StringRef::npos ||
-  ArchName.find_lower("armv8.1a") != StringRef::npos ||
-  ArchName.find_lower("armv8.2a") != StringRef::npos ||
-  ArchName.find_lower("armv8.3a") != StringRef::npos ||
-  ArchName.find_lower("armv8.4a") != StringRef::npos) {
-if (ArchName.find_lower("+crypto") != StringRef::npos) {
-  if (ArchName.find_lower("+nosha2") == StringRef::npos)
+  auto CryptoIt =
+llvm::find_if(llvm::reverse(Features),
+  [](const StringRef F) { return F.contains("crypto"); });
+  if (CryptoIt != Features.rend() && CryptoIt->take_front() == "+") {
+StringRef ArchSuffix = arm::getLLVMArchSuffixForARM(
+arm::getARMTargetCPU(CPUName, ArchName, Triple), ArchName, Triple);
+if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8 &&
+llvm::ARM::parseArchProfile(ArchSuffix) == llvm::ARM::ProfileKind::A) {
+  if (ArchName.find_lower("+nosha2") == StringRef::npos &&
+  CPUName.find_lower("+nosha2") == StringRef::npos)
 Features.push_back("+sha2");
-  if (ArchName.find_lower("+noaes") == StringRef::npos)
+  if (ArchName.find_lower("+noaes") == StringRef::npos &&
+  CPUName.find_lower("+noaes") == StringRef::npos)
 Features.push_back("+aes");
-} else if (ArchName.find_lower("-crypto") != StringRef::npos) {
-  if (ArchName.find_lower("+sha2") == StringRef::npos)
-Features.push_back("-sha2");
-  if (ArchName.find_lower("+aes") == StringRef::npos)
-Features.push_back("-aes");
+} else {
+  D.Diag(clang::diag::warn_target_unsupported_extension)
+<< "crypto" << 
llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
+  Features.push_back("-crypto");
 }
   }
 
@@ -655,7 +658,7 @@ std::string arm::getARMTargetCPU(StringR
 llvm::ARM::ArchKind

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Diogo N. Sampaio via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371597: [ARM] Take into account -mcpu and -mfpu options 
while handling 'crypto' feature (authored by dnsampaio, committed by 
).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66018

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
  cfe/trunk/test/Driver/arm-features.c

Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -384,6 +384,9 @@
 def warn_target_unsupported_compact_branches : Warning<
   "ignoring '-mcompact-branches=' option because the '%0' architecture does not"
   " support it">, InGroup;
+def warn_target_unsupported_extension : Warning<
+  "ignoring extension '%0' because the '%1' architecture does not support it">,
+   InGroup;
 def warn_drv_unsupported_gpopt : Warning<
   "ignoring '-mgpopt' option as it cannot be used with %select{|the implicit"
   " usage of }0-mabicalls">,
Index: cfe/trunk/test/Driver/arm-features.c
===
--- cfe/trunk/test/Driver/arm-features.c
+++ cfe/trunk/test/Driver/arm-features.c
@@ -37,7 +37,8 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
-// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2 %s
+// CHECK-CRYPTO2: "-cc1"{{.*}} "-target-cpu" "generic"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto:
 //
@@ -45,14 +46,36 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.4a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.5a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2 %s
 // CHECK-NOCRYPTO2-NOT: "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
 //
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO2-CPU %s
+// CHECK-CRYPTO2-CPU: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+norypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57 -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO2-CPU %s
+// CHECK-NOCRYPTO2-CPU-NOT: "-cc1"{{.*}} "-target-cpu" "cortex-a57"{{.*}} "-target-feature" "+crypto"{{.*}} "-target-feature" "+sha2" "-target-feature" "+aes"
+//
 // Check +crypto -sha2 -aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nosha2+noaes -mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO3 %s
 // CHECK-CRYPTO3-NOT: "-target-feature" "+sha2" "-target-feature" "+aes"
 //
 // Check -crypto +sha2 +aes:
 //
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1a+nocrypto+sha2+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+nocrypto+sha2+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
+// RUN: %clang -target arm-arm-none-eabi -mcpu=cortex-a57+sha2+aes -mfpu=neon-fp-armv8 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO4 %s
 // CHECK-CRYPTO4: "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check +crypto for M and R profiles:
+//
+// RUN: %clang -target arm-arm-none-eabi -mar

[PATCH] D60748: Fix i386 struct and union parameter alignment

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 added a comment.

@RKSimon I'm busy with other stuff and my colleague: LiuChen3 will help finish 
the work.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60748



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added inline comments.



Comment at: compiler-rt/trunk/lib/profile/xxhash.h:41-42
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+

Sorry folks, but you can't do this.

You can't depend on ADT from compiler-rt currently.

There are at least two problems here:

First problem is that this pollutes the profile library with symbols from ADT. 
That really doesn't seem reasonable without *significant* and invasive changes 
to ADT. Otherwise building LLVM and linking it with the profile library will 
create an ODR violation (imagine different assert levels or different versions 
of LLVM buing built and the host toolchain).


Second, and much more critically, we haven't gotten to 100% relicensed on ADT, 
so it is critical that we not depend on it from runtime libraries.

Third, a lot of this code seems to use old license headers. Please do not add 
any code like this to LLVM, and instead use the new LLVM license for all new 
code.

For now, this patch (and any related patches) need to be reverted until these 
are addressed. Especially the license issues.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

r371598 - Revert "clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM"

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Wed Sep 11 02:16:17 2019
New Revision: 371598

URL: http://llvm.org/viewvc/llvm-project?rev=371598&view=rev
Log:
Revert "clang-misexpect: Profile Guided Validation of Performance Annotations 
in LLVM"

This reverts commit r371584. It introduced a dependency from compiler-rt
to llvm/include/ADT, which is problematic for multiple reasons.

One is that it is a novel dependency edge, which needs cross-compliation
machinery for llvm/include/ADT (yes, it is true that right now
compiler-rt included only header-only libraries, however, if we allow
compiler-rt to depend on anything from ADT, other libraries will
eventually get used).

Secondly, depending on ADT from compiler-rt exposes ADT symbols from
compiler-rt, which would cause ODR violations when Clang is built with
the profile library.

Removed:
cfe/trunk/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
cfe/trunk/test/Profile/Inputs/misexpect-branch.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default-only.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-nonconst.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch.proftext
cfe/trunk/test/Profile/misexpect-branch-cold.c
cfe/trunk/test/Profile/misexpect-branch-nonconst-expected-val.c
cfe/trunk/test/Profile/misexpect-branch-unpredictable.c
cfe/trunk/test/Profile/misexpect-branch.c
cfe/trunk/test/Profile/misexpect-switch-default.c
cfe/trunk/test/Profile/misexpect-switch-nonconst.c
cfe/trunk/test/Profile/misexpect-switch-only-default-case.c
cfe/trunk/test/Profile/misexpect-switch.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=371598&r1=371597&r2=371598&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Wed Sep 11 
02:16:17 2019
@@ -275,12 +275,7 @@ def warn_profile_data_missing : Warning<
 def warn_profile_data_unprofiled : Warning<
   "no profile data available for file \"%0\"">,
   InGroup;
-def warn_profile_data_misexpect : Warning<
-  "Potential performance regression from use of __builtin_expect(): "
-  "Annotation was correct on %0 of profiled executions.">,
-  BackendInfo,
-  InGroup,
-  DefaultIgnore;
+
 } // end of instrumentation issue category
 
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=371598&r1=371597&r2=371598&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Sep 11 02:16:17 2019
@@ -1042,7 +1042,6 @@ def BackendOptimizationFailure : DiagGro
 def ProfileInstrMissing : DiagGroup<"profile-instr-missing">;
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
-def MisExpect : DiagGroup<"misexpect">;
 
 // AddressSanitizer frontend instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=371598&r1=371597&r2=371598&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Wed Sep 11 02:16:17 2019
@@ -14,7 +14,6 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
-#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/LangStandard.h"
 #include "clang/Basic/SourceManager.h"
@@ -366,9 +365,6 @@ namespace clang {
 bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D);
 /// Specialized handler for unsupported backend feature diagnostic.
 void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D);
-/// Specialized handler for misexpect warnings.
-/// Note that misexpect remarks are emitted through ORE
-void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D);
 /// Specialized handlers for optimization remarks.
 /// Note that these handlers only accept remarks and they always handle
 /// them.
@@ -621,25 +617,6 @@ void BackendConsumer::UnsupportedDiagHan
 << Filename << Line << Column;
 }
 
-void Backend

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Reverted in r371598.

Another concern that I have is cross-compilation. LLVM's ADT is not set up to 
be cross-compiled like the rest of compiler-rt is.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar updated this revision to Diff 219673.
andwar marked 6 inline comments as done.
andwar added a comment.

- Removed `declare variant` from Attr.td
- Moved the 'vector-var-id' lookup from ParseOpenMP.cpp to SemaOpenMP.cpp
- Parsing 'vector-var-id' as an expression
- Removed the creation of the attribute in `ActOnOpenMPDeclareVariantDirective`


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Basic/OpenMPKinds.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/declare_variant_messages.cpp

Index: test/OpenMP/declare_variant_messages.cpp
===
--- /dev/null
+++ test/OpenMP/declare_variant_messages.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c++ -std=c++11 -fms-extensions -Wno-pragma-pack %s
+
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp-simd -x c++ -std=c++11 -fms-extensions -Wno-pragma-pack %s
+
+// The dummy vector variant
+void vector_foo();
+
+//=
+// Basic errors
+//=
+// This is (and should be) perfectly fine for the parser, although semantically
+// not valid OpenMP 5.
+#pragma omp declare variant(vector_foo)
+void foo();
+
+// expected-error@+1 {{expected an OpenMP directive}}
+#pragma omp declare
+
+// expected-error@+2 {{'#pragma omp declare variant' can only be applied to functions}}
+#pragma omp declare variant(vector_foo)
+int a;
+// expected-error@+2 {{'#pragma omp declare variant' can only be applied to functions}}
+#pragma omp declare variant (vector_foo)
+#pragma omp threadprivate(a)
+int var;
+#pragma omp threadprivate(var)
+
+// expected-error@+3 {{expected an OpenMP directive}}
+// expected-error@+1 {{function declaration is expected after 'declare variant' directive}}
+#pragma omp declare variant
+#pragma omp declare
+
+// expected-error@+1 {{single declaration is expected after 'declare variant' directive}}
+#pragma omp declare variant(vector_foo)
+int b, c;
+
+//=
+// Syntax/semantic error: everything excluding linear/aligned/uniform
+//=
+// expected-error@+1 {{expected an OpenMP directive}}
+#pragma omp variant
+// expected-error@+1 {{expected '(' after '#pragma omp declare variant'}}
+#pragma omp declare variant
+// expected-warning@+2 {{extra tokens at the end of '#pragma omp declare variant' are ignored}}
+// expected-error@+1 {{expected '(' after '#pragma omp declare variant'}}
+#pragma omp declare variant )
+// expected-error@+1 {{expected unqualified-id representing 'vector-variant-id'}}
+#pragma omp declare variant()
+// expected-error@+1 {{no declaration for 'undeclared_func', only declared functions can be used as }}
+#pragma omp declare variant(undeclared_func)
+// expected-error@+2 {{expected ')'}}
+// expected-note@+1 {{to match this '('}}
+#pragma omp declare variant(vector_foo
+void foo();
+
+//=
+// Templates
+//=
+// expected-error@+1 {{'#pragma omp declare variant' can only be used to decorate instantiated templates}}
+#pragma omp declare variant(vector_foo)
+template 
+void h(C *hp, C *hp2, C *hq, C *lin) {
+  b = 0;
+}
+
+#pragma omp declare variant(vector_foo)
+template <>
+void h(int *hp, int *hp2, int *hq, int *lin) {
+  h((float *)hp, (float *)hp2, (float *)hq, (float *)lin);
+}
+
+//=
+// 'declare variant' embedded in namespaces
+//=
+namespace N {
+  // expected-error@+1 {{function declaration is expected after 'declare variant' directive}}
+  #pragma omp declare variant
+}
+// expected-error@+1 {{function declaration is expected after 'declare variant' directive}}
+#pragma omp declare variant
+// Now verify that the parser recovered fine from the previous error and identifies another one correctly
+// expected-error@+1 {{function declaration is expected after 'declare variant' directive}}
+#pragma omp declare variant
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -3438,6 +3438,7 @@
   case OMPD_declare_reduction:
   case OMPD_declare_mapper:
   case OMPD_declare_simd:
+  case OMPD_declare_variant:
   case OM

[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Andrzej Warzynski via Phabricator via cfe-commits]

andwar added a comment.

I've addressed most of the comments except for those related to templates. I'd 
like to clarify as what is the expected behaviour there before proceeding with 
implementation.




Comment at: include/clang/Basic/Attr.td:3220
+private:
+  NamedDecl *VecVarNamedDecl;
+

ABataev wrote:
> This is definitely wrong, especially if we need to handle templates.
What else would you use? Also, we're not handling template-ids as 
'variant-func-id' just yet.



Comment at: lib/Parse/ParseOpenMP.cpp:779-783
+  auto Identinfo = PP.getIdentifierInfo(VectorVariantId.Ident->getName());
+  LookupResult Lookup(Actions, Identinfo, VectorVariantId.Loc,
+  Sema::LookupOrdinaryName);
+  CXXScopeSpec SS;
+  if (!Actions.LookupParsedName(Lookup, getCurScope(), &SS)) {

ABataev wrote:
> 1. All this lookup must be performed in Sema.
> 2. How do you want to handle templates?
> 3. Why just do not parse it as an expression? And rely on existing 
> parsing/sema analysis for expressions.
1. That's fine, let me move it there.

2. TL;DR We're not handling templates yet. 

3. How about switching to `ParseUnqualifiedId` instead? That will still require 
going through `LookupParsedName` though.

Re 2. What the interaction between templates and #pragma(s) should be? The C++ 
standard doesn't specify that, but IMHO mixing '#pragma omp declare variant' 
and templates requires that interaction to be clearly defined. 

I know that OpenMP 5 allows 'variant-func-id' to be a template-id, but I 
couldn't find an example of any attribute that would use C++ template 
parameters. Also, isn't parsing of attributes and templates _orthogonal_ in 
clang (i.e. happening at completely different points during parsing)?

In any case, we felt that support for 'template-id' could come later. How do 
you feel about it?



Comment at: lib/Sema/SemaOpenMP.cpp:4886
 
+Sema::DeclGroupPtrTy Sema::ActOnOpenMPDeclareVariantDirective(
+DeclGroupPtrTy DG, IdentifierLoc &VectorVariantId, SourceRange SR) {

ABataev wrote:
> In this patch need to implement just basic checks, no need to create 
> attributes, etc. I would suggest to look at the checks for Target 
> Multiversioning attribite (see D40819)
This makes sense, but do you reckon that that should be split into separate 
functions? I was inspired by the implementation of `declare simd`. From what I 
can tell, the only Sema checking for `declare simd` is happening in 
`ActOnOpenMPDeclareVariantDirective`. It feels that doing the same for `declare 
variant` would be good for consistency. To limit this method to Sema checks, I 
will remove the call to `CreateImplicit`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D66324#1665773 , @gribozavr wrote:

> Reverted in r371598.
>
> Another concern that I have is cross-compilation. LLVM's ADT is not set up to 
> be cross-compiled like the rest of compiler-rt is.


Uhm, i have a better question still - why xxhash is even there? it's not used 
in the diff, and was not reviewed, it ajust magically appeared in the last 
update of the patch:
https://reviews.llvm.org/D66324?vs=219617&id=219645


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I left some minor notes but overall I think we should put this in and improve 
on it in-tree. @ABataev, would that be OK with you?




Comment at: lib/Parse/ParseOpenMP.cpp:748
 }
 
+/// Parses clauses for 'declare variant' directive.

Remove the "vector" parts in the descriptions and variable names below please, 
it is not "vector" specific.

Nit: I'd remove the note justifying the choice for a member function.



Comment at: lib/Parse/ParseOpenMP.cpp:788
+}
+
 /// Parse clauses for '#pragma omp declare simd'.

I actually doubt the "vec-var-id" has to be an `tok::identifier` but let's put 
that discussion on hold for later patches and get it in for the most common 
case, identifiers.



Comment at: lib/Sema/SemaOpenMP.cpp:4930
+}
+
 StmtResult Sema::ActOnOpenMPParallelDirective(ArrayRef Clauses,

I'm a little unsure about the template instantiation check (have to actually 
open the standard on that) but I guess being to conservative now and making 
progress is an acceptable way to go anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294



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

[PATCH] D67418: [analyzer] NFC: Move PathDiagnostic::resetDiagnosticLocationToMainFile to bug reporter.

[Kristóf Umann via Phabricator via cfe-commits]

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

Sure!


Repository:
  rC Clang

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

https://reviews.llvm.org/D67418



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

[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/Basic/Attr.td:3220
+private:
+  NamedDecl *VecVarNamedDecl;
+

andwar wrote:
> ABataev wrote:
> > This is definitely wrong, especially if we need to handle templates.
> What else would you use? Also, we're not handling template-ids as 
> 'variant-func-id' just yet.
But we have to handle templates from the very beginning. This is the thing that 
would be very hard to support later and we have to think about it in the first 
patch.
Just parse function identifier as an expression.
Your solution also will break serialization/deserialization since attributes 
dies not support serilization of the custom fields.



Comment at: lib/Parse/ParseOpenMP.cpp:779-783
+  auto Identinfo = PP.getIdentifierInfo(VectorVariantId.Ident->getName());
+  LookupResult Lookup(Actions, Identinfo, VectorVariantId.Loc,
+  Sema::LookupOrdinaryName);
+  CXXScopeSpec SS;
+  if (!Actions.LookupParsedName(Lookup, getCurScope(), &SS)) {

andwar wrote:
> ABataev wrote:
> > 1. All this lookup must be performed in Sema.
> > 2. How do you want to handle templates?
> > 3. Why just do not parse it as an expression? And rely on existing 
> > parsing/sema analysis for expressions.
> 1. That's fine, let me move it there.
> 
> 2. TL;DR We're not handling templates yet. 
> 
> 3. How about switching to `ParseUnqualifiedId` instead? That will still 
> require going through `LookupParsedName` though.
> 
> Re 2. What the interaction between templates and #pragma(s) should be? The 
> C++ standard doesn't specify that, but IMHO mixing '#pragma omp declare 
> variant' and templates requires that interaction to be clearly defined. 
> 
> I know that OpenMP 5 allows 'variant-func-id' to be a template-id, but I 
> couldn't find an example of any attribute that would use C++ template 
> parameters. Also, isn't parsing of attributes and templates _orthogonal_ in 
> clang (i.e. happening at completely different points during parsing)?
> 
> In any case, we felt that support for 'template-id' could come later. How do 
> you feel about it?
No, don't try to parse function names directly. What about `::fun` format? Or 
`std::fun`? Your solution does not support it.
I have basic implementation that handles all these cases. Let me commit it, 
because we need it to support other context selectors. It is very similar to 
your solution but handles all the problems. We just don't have time to wait, 
need to support other context selectors.



Comment at: lib/Parse/ParseOpenMP.cpp:788
+}
+
 /// Parse clauses for '#pragma omp declare simd'.

jdoerfert wrote:
> I actually doubt the "vec-var-id" has to be an `tok::identifier` but let's 
> put that discussion on hold for later patches and get it in for the most 
> common case, identifiers.
This is definitely wrong.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294



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

r371605 - [Diagnostics] Add -Wsizeof-array-div

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Wed Sep 11 03:59:47 2019
New Revision: 371605

URL: http://llvm.org/viewvc/llvm-project?rev=371605&view=rev
Log:
[Diagnostics] Add -Wsizeof-array-div

Summary: Clang version of https://www.viva64.com/en/examples/v706/

Reviewers: rsmith

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

Added:
cfe/trunk/test/Sema/div-sizeof-array.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11 03:59:47 
2019
@@ -3406,6 +3406,10 @@ def note_pointer_declared_here : Note<
 def warn_division_sizeof_ptr : Warning<
   "'%0' will return the size of the pointer, not the array itself">,
   InGroup>;
+def warn_division_sizeof_array : Warning<
+  "expression does not compute the number of elements in this array; element "
+  "type is %0, not %1">,
+  InGroup>;
 
 def note_function_warning_silence : Note<
 "prefix with the address-of operator to silence this warning">;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019
@@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe
   else
 RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
 
-  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
-return;
-  if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() !=
-  RHSTy.getCanonicalType().getUnqualifiedType())
-return;
+  if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
+if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), RHSTy))
+  return;
 
-  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << LHS->getSourceRange();
-  if (const auto *DRE = dyn_cast(LHSArg)) {
-if (const ValueDecl *LHSArgDecl = DRE->getDecl())
-  S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
-  << LHSArgDecl;
+S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
LHS->getSourceRange();
+if (const auto *DRE = dyn_cast(LHSArg)) {
+  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
+S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
+<< LHSArgDecl;
+}
+  } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
+QualType ArrayElemTy = ArrayTy->getElementType();
+if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
+S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
+  return;
+S.Diag(Loc, diag::warn_division_sizeof_array)
+<< LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
+if (const auto *DRE = dyn_cast(LHSArg)) {
+  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
+S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here)
+<< LHSArgDecl;
+}
   }
 }
 

Added: cfe/trunk/test/Sema/div-sizeof-array.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto
==
--- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)
+++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
+
+template 
+int f(Ty (&Array)[N]) {
+  return sizeof(Array) / sizeof(Ty); // Should not warn
+}
+
+typedef int int32;
+
+void test(void) {
+  int arr[12];// expected-note 2 {{array 'arr' declared here}}
+  unsigned long long arr2[4];
+  int *p = &arr[0];
+  int a1 = sizeof(arr) / sizeof(*arr);
+  int a2 = sizeof arr / sizeof p; // expected-warning {{expression does not 
compute the number of elements in this array; element type is 'int', not 'int 
*'}}
+  int a4 = sizeof arr2 / sizeof p;
+  int a5 = sizeof(arr) / sizeof(short); // expected-warning {{expression does 
not compute the number of elements in this array; element type is 'int', not 
'short'}}
+  int a6 = sizeof(arr) / sizeof(int32);
+  int a7 = sizeof(arr) / sizeof(int);
+  int a9 = sizeof(arr) / sizeof(unsigned int);
+  const char arr3[2] = "A";
+  int a10 = sizeof(arr3) / sizeof(char);
+
+  int arr4[10][12]; // expected-note 3 {{array 'arr4' 
declared here}}
+  int b1 = sizeof(arr4) / sizeof(arr2[12]); // expected-warning {{expression 
does not compute the number of elements in this array; element type is 'int 
[12]', not '

[PATCH] D67436: CodeGen: set correct result for atomic compound expressions

[Tim Northover via Phabricator via cfe-commits]

t.p.northover created this revision.
Herald added subscribers: jfb, mcrosier.
Herald added a project: clang.

Atomic compound expressions try to use atomicrmw if possible, but this path 
doesn't set the `Result` variable, leaving it to crash in later code if 
anything ever tries to use the result of the expression. This fixes that issue 
by recalculating the new value based on the old one atomically loaded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67436

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/atomic_ops.c

Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -37,3 +37,56 @@
 // CHECK: {{store atomic|call void @__atomic_store}}
   x += y;
 }
+
+_Atomic(int) compound_add(_Atomic(int) in) {
+// CHECK-LABEL: @compound_add
+// CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = add i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in += 5);
+}
+
+_Atomic(int) compound_sub(_Atomic(int) in) {
+// CHECK-LABEL: @compound_sub
+// CHECK: [[OLD:%.*]] = atomicrmw sub i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = sub i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in -= 5);
+}
+
+_Atomic(int) compound_xor(_Atomic(int) in) {
+// CHECK-LABEL: @compound_xor
+// CHECK: [[OLD:%.*]] = atomicrmw xor i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = xor i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in ^= 5);
+}
+
+_Atomic(int) compound_or(_Atomic(int) in) {
+// CHECK-LABEL: @compound_or
+// CHECK: [[OLD:%.*]] = atomicrmw or i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = or i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in |= 5);
+}
+
+_Atomic(int) compound_and(_Atomic(int) in) {
+// CHECK-LABEL: @compound_and
+// CHECK: [[OLD:%.*]] = atomicrmw and i32* {{.*}}, i32 5 seq_cst
+// CHECK: [[NEW:%.*]] = and i32 [[OLD]], 5
+// CHECK: ret i32 [[NEW]]
+
+  return (in &= 5);
+}
+
+_Atomic(int) compound_mul(_Atomic(int) in) {
+// CHECK-LABEL: @compound_mul
+// CHECK: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
+// CHECK: ret i32 [[NEW]]
+
+  return (in *= 5);
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2840,7 +2840,8 @@
   CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) &&
 CGF.getLangOpts().getSignedOverflowBehavior() !=
 LangOptions::SOB_Trapping) {
-  llvm::AtomicRMWInst::BinOp aop = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::AtomicRMWInst::BinOp AtomicOp = llvm::AtomicRMWInst::BAD_BINOP;
+  llvm::Instruction::BinaryOps Op;
   switch (OpInfo.Opcode) {
 // We don't have atomicrmw operands for *, %, /, <<, >>
 case BO_MulAssign: case BO_DivAssign:
@@ -2849,30 +2850,40 @@
 case BO_ShrAssign:
   break;
 case BO_AddAssign:
-  aop = llvm::AtomicRMWInst::Add;
+  AtomicOp = llvm::AtomicRMWInst::Add;
+  Op = llvm::Instruction::Add;
   break;
 case BO_SubAssign:
-  aop = llvm::AtomicRMWInst::Sub;
+  AtomicOp = llvm::AtomicRMWInst::Sub;
+  Op = llvm::Instruction::Sub;
   break;
 case BO_AndAssign:
-  aop = llvm::AtomicRMWInst::And;
+  AtomicOp = llvm::AtomicRMWInst::And;
+  Op = llvm::Instruction::And;
   break;
 case BO_XorAssign:
-  aop = llvm::AtomicRMWInst::Xor;
+  AtomicOp = llvm::AtomicRMWInst::Xor;
+  Op = llvm::Instruction::Xor;
   break;
 case BO_OrAssign:
-  aop = llvm::AtomicRMWInst::Or;
+  AtomicOp = llvm::AtomicRMWInst::Or;
+  Op = llvm::Instruction::Or;
   break;
 default:
   llvm_unreachable("Invalid compound assignment type");
   }
-  if (aop != llvm::AtomicRMWInst::BAD_BINOP) {
-llvm::Value *amt = CGF.EmitToMemory(
+  if (AtomicOp != llvm::AtomicRMWInst::BAD_BINOP) {
+llvm::Value *Amt = CGF.EmitToMemory(
 EmitScalarConversion(OpInfo.RHS, E->getRHS()->getType(), LHSTy,
  E->getExprLoc()),
 LHSTy);
-Builder.CreateAtomicRMW(aop, LHSLV.getPointer(), amt,
+Value *OldVal = Builder.CreateAtomicRMW(
+AtomicOp, LHSLV.getPointer(), Amt,
 llvm::AtomicOrdering::SequentiallyConsistent);
+
+// Since operation is atomic, the result type is guaranteed to be the
+// same as the input in LLVM terms.
+Result = Builder.CreateBinOp(Op, OldVal, Amt);
 return LHSLV;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67381: [analyzer] NFC: Move stack hints to a side map.

[Kristóf Umann via Phabricator via cfe-commits]

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

> I'm open to discuss a better design here. Eg., i thought about making it part 
> of the visitor interface instead, but i don't immediately see how to do this 
> without breaking the logic of "only add the note at the call site in which 
> the event has happened, not every time allocated memory is returned from 
> anywhere".

When I stumbled upon this stack hint thingie during the refactoring of 
BugReporter.cpp, I found it very clunky, and while I still do, I'm none the 
wiser about how to do it any better.

Side note, now that you had to work with the freshly rewritten file, do you 
have any feedback on it?




Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:333-337
+  SmallString<200> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  os << Msg << " via " << ArgIndex << llvm::getOrdinalSuffix(ArgIndex)
+ << " parameter";

How about a simple twine?



Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:34
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/ADT/ArrayRef.h"

Nice, how did you catch this? Some compiler warning?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67381



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

r371497 - Revert Remove REQUIRES:shell from tests that pass for me on Windows

[James Henderson via cfe-commits]

Author: jhenderson
Date: Tue Sep 10 01:48:33 2019
New Revision: 371497

URL: http://llvm.org/viewvc/llvm-project?rev=371497&view=rev
Log:
Revert Remove REQUIRES:shell from tests that pass for me on Windows

This reverts r371478 (git commit a9980f60ce083fa6d5fd03c12c58ca0b293e3d60)

Modified:
cfe/trunk/test/Analysis/crash-trace.c
cfe/trunk/test/CodeGen/thinlto_backend.ll
cfe/trunk/test/Driver/check-time-trace-sections.cpp
cfe/trunk/test/Driver/check-time-trace.cpp
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/test/Driver/crash-report-crashfile.m
cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c
cfe/trunk/test/Format/style-on-command-line.cpp
cfe/trunk/test/Frontend/dependency-gen-has-include.c
cfe/trunk/test/Index/crash-recovery-modules.m
cfe/trunk/test/Modules/at-import-in-framework-header.m
cfe/trunk/test/Modules/builtins.m
cfe/trunk/test/Modules/dependency-dump-dependent-module.m
cfe/trunk/test/Modules/dependency-dump.m
cfe/trunk/test/Modules/implicit-invalidate-common.c
cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/task_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_private_codegen.cpp
cfe/trunk/test/PCH/modified-header-error.c
cfe/trunk/test/Parser/crash-report.c

Modified: cfe/trunk/test/Analysis/crash-trace.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/crash-trace.c?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/Analysis/crash-trace.c (original)
+++ cfe/trunk/test/Analysis/crash-trace.c Tue Sep 10 01:48:33 2019
@@ -1,8 +1,9 @@
 // RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
%s 2>&1 | FileCheck %s
 // REQUIRES: crash-recovery
 
-// Stack traces require back traces.
-// REQUIRES: backtrace
+// FIXME: CHECKs might be incompatible to win32.
+// Stack traces also require back traces.
+// REQUIRES: shell, backtrace
 
 void clang_analyzer_crash(void);
 
@@ -17,6 +18,6 @@ void test() {
 // CHECK: 0.   Program arguments: {{.*}}clang
 // CHECK-NEXT: 1.   parser at end of file
 // CHECK-NEXT: 2. While analyzing stack: 
-// CHECK-NEXT:  #0 Calling inlined at line 14
+// CHECK-NEXT:  #0 Calling inlined at line 15
 // CHECK-NEXT:  #1 Calling test
 // CHECK-NEXT: 3.  {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating 
statement

Modified: cfe/trunk/test/CodeGen/thinlto_backend.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto_backend.ll?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto_backend.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll Tue Sep 10 01:48:33 2019
@@ -1,4 +1,5 @@
-; REQUIRES: x86-registered-target
+; shell required since the windows bot does not like the "(cd ..."
+; REQUIRES: x86-registered-target, shell
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto_backend.ll
@@ -31,14 +32,10 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-dis %t1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT 
%s
 ; RUN: mkdir -p %T/dir1
-; RUN: cd %T/dir1
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=cwd
-; RUN: cd ../..
+; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
 ; RUN: llvm-dis %T/dir1/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir2
-; RUN: cd %T/dir2
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps
-; RUN: cd ../..
+; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
 ; RUN: llvm-dis %T/dir2/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

Modified: cfe/trunk/test/Driver/check-time-trace-sections.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/check-time-trace-sections.cpp?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/Driver/check-time-trace-sections.cpp (original)
+++ cfe/trunk/test/Driver/check-time-trace-sections.cpp Tue Sep 10 01:48:33 2019
@@ -1,3 +1,4 

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

[Alina Sbirlea via cfe-commits]

Hi David,

Does this still reproduce?
I'm seeing the load after the call to autoreleasePoolPop at ToT, but
perhaps I'm not using the proper flags?
I only checked out the github repo and did "clang
-fobjc-runtime=gnustep-2.0 -O3 -emit-llvm -S
libobjc2/Test/AssociatedObject.m" with a ToT clang.

Thanks,
Alina


On Sun, Feb 24, 2019 at 9:56 AM Pete Cooper via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

> Hey David
>
> Thanks for letting me know, and analysing it this far!
>
> I also can't see anything wrong with the intrinsic.  Its just defined as:
>
> def int_objc_autoreleasePoolPop : Intrinsic<[], [llvm_ptr_ty]>;
>
>
> which (I believe) means it has unmodelled side effects so it should have
> been fine for your example.
>
> I'll try build the same file you did and see if I can reproduce.
>
> Cheers,
> Pete
>
> On Feb 24, 2019, at 7:48 AM, David Chisnall via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
> theraven added a comment.
> Herald added a project: LLVM.
>
> After some bisection, it appears that this is the revision that introduced
> the regression in the GNUstep Objective-C runtime test suite that I
> reported on the list a few weeks ago.  In this is the test (compiled with
> `-fobjc-runtime=gnustep-2.0 -O3` and an ELF triple):
>
> https://github.com/gnustep/libobjc2/blob/master/Test/AssociatedObject.m
>
> After this change, Early CSE w/ MemorySSA is determining that the second
> load of `deallocCalled` is redundant.  The code goes from:
>
>%7 = load i1, i1* @deallocCalled, align 1
>br i1 %7, label %8, label %9
>
>  ; :8:  ; preds = %0
>call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]*
> @__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x
> i8]* @.str, i64 0, i64 0), i32 26, i8* getelementptr inbounds ([15 x i8],
> [15 x i8]* @.str.1, i64 0, i64 0)) #5
>unreachable
>
>  ; :9:  ; preds = %0
>call void @llvm.objc.autoreleasePoolPop(i8* %1)
>%10 = load i1, i1* @deallocCalled, align 1
>br i1 %10, label %12, label %11
>
>  ; :11: ; preds = %9
>call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]*
> @__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x
> i8]* @.str, i64 0, i64 0), i32 29, i8* getelementptr inbounds ([14 x i8],
> [14 x i8]* @.str.2, i64 0, i64 0)) #5
>unreachable
>
> to:
>
>%7 = load i1, i1* @deallocCalled, align 1
>br i1 %7, label %8, label %9
>
>  ; :8:  ; preds = %0
>call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]*
> @__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x
> i8]* @.str, i64 0, i64 0), i32 26, i8* getelementptr inbounds ([15 x i8],
> [15 x i8]* @.str.1, i64 0, i64 0)) #5
>unreachable
>
>  ; :9:  ; preds = %0
>call void @llvm.objc.autoreleasePoolPop(i8* %1)
>br i1 %7, label %11, label %10
>
>  ; :10: ; preds = %9
>call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]*
> @__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x
> i8]* @.str, i64 0, i64 0), i32 29, i8* getelementptr inbounds ([14 x i8],
> [14 x i8]* @.str.2, i64 0, i64 0)) #5
>unreachable
>
> Later optimisations then determine that, because the assert does not
> return, the only possible value for %7 is false and cause the second assert
> to fire unconditionally.
>
> It appears that we are not correctly modelling the side effects of the
> `llvm.objc.autoreleasePoolPop` intrinsic, but it's not entirely clear why
> not.  The same test compiled for the macos runtime does not appear to
> exhibit the same behaviour.  The previous revision, where we emitted a call
> to `objc_autoreleasePoolPop` and not the intrinsic worked correctly, but
> with this change the optimisers are assuming that no globals can be
> modified across an autorelease pool pop operation (at least, in some
> situations).
>
> Looking at the definition of the intrinsic, I don't see anything wrong, so
> I still suspect that there is a MemorySSA bug that this has uncovered,
> rather than anything wrong in this series of commits.  Any suggestions as
> to where to look would be appreciated.
>
>
> Repository:
>  rL LLVM
>
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D55802/new/
>
> https://reviews.llvm.org/D55802
>
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore marked an inline comment as done.
stephanemoore added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:30
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WhitelistedPrefixes", WhitelistedPrefixes);
+}

aaron.ballman wrote:
> dgatwood wrote:
> > aaron.ballman wrote:
> > > dgatwood wrote:
> > > > aaron.ballman wrote:
> > > > > `ExpectedPrefixes` here as well.
> > > > > 
> > > > > Should there be a default list of these?
> > > > Done.  And no, there should be no default, unless somehow Xcode's 
> > > > project prefix makes it down as far as LLVM, in which case //maybe// 
> > > > that could be the default.
> > > > 
> > > > The idea is that you can whitelist your own Xcode project's prefix, 
> > > > along with the prefixes of your own in-house libraries, so that each 
> > > > individual team/workgroup can add categories on their own classes, but 
> > > > will get warned when they try to add unprefixed category methods on 
> > > > classes that they don't own (e.g. classes in system frameworks, 
> > > > third-party frameworks, etc.).
> > > Still wondering whether we should have a default list of expected 
> > > prefixes or not.
> > This is weird.  I don't know why this comment system didn't submit my 
> > comment before.
> > 
> > No, there should be no default, unless somehow Xcode's project prefix makes 
> > it down as far as LLVM, in which case maybe that could be the default.
> > 
> > The idea is that you can whitelist your own Xcode project's prefix, along 
> > with the prefixes of your own in-house libraries, so that each individual 
> > team/workgroup can add categories on their own classes, but will get warned 
> > when they try to add unprefixed category methods on classes that they don't 
> > own (e.g. classes in system frameworks, third-party frameworks, etc.).
> Ah, thank you for the explanation!
Agreed that there should be no default.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:22-23
+
+  Finder->addMatcher(objcMethodDecl().bind(CustomCategoryMethodIdentifier),
+  this);
+}

What do you think of `objcMethodDecl(hasDeclContext(objcCategoryDecl()))`? I 
think that more narrowly targets the method declarations that we are interested 
in. There are other method declarations that we like to prefix but I consider 
those outside the domain of category method name prefixing practices.


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

https://reviews.llvm.org/D65917



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

[PATCH] D67159: [clang] New __attribute__((__clang_builtin)).

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham updated this revision to Diff 219691.
simon_tatham added a comment.

New version which renames the attribute to be MVE-specific, and locks it down 
as requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67159

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/arm-mve-intrinsics/alias-attr.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -17,6 +17,7 @@
 // CHECK-NEXT: Annotate ()
 // CHECK-NEXT: AnyX86NoCfCheck (SubjectMatchRule_hasType_functionType)
 // CHECK-NEXT: ArcWeakrefUnavailable (SubjectMatchRule_objc_interface)
+// CHECK-NEXT: ArmMveAlias (SubjectMatchRule_function)
 // CHECK-NEXT: AssumeAligned (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
Index: clang/test/CodeGen/arm-mve-intrinsics/alias-attr.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-mve-intrinsics/alias-attr.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple armv8.1m.main-arm-none-eabi -verify -fsyntax-only %s
+
+static __inline__ __attribute__((__clang_arm_mve_alias(__builtin_arm_nop)))
+void nop(void); // expected-error {{__clang_arm_mve_alias attribute can only be used for Arm MVE builtins}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5045,6 +5045,12 @@
   AL.getAttributeSpellingListIndex()));
 }
 
+static void handleArmMveAliasAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  D->addAttr(::new (S.Context) ArmMveAliasAttr(
+  AL.getRange(), S.Context, AL.getArgAsIdent(0)->Ident,
+  AL.getAttributeSpellingListIndex()));
+}
+
 //===--===//
 // Checker-specific attribute handlers.
 //===--===//
@@ -7434,6 +7440,10 @@
   case ParsedAttr::AT_MSAllocator:
 handleMSAllocatorAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_ArmMveAlias:
+handleArmMveAliasAttr(S, D, AL);
+break;
   }
 }
 
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -46,6 +46,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetCXXABI.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Visibility.h"
@@ -3077,6 +3078,11 @@
 
 FunctionDecl *FunctionDecl::getCanonicalDecl() { return getFirstDecl(); }
 
+static bool ArmMveAliasValid(unsigned BuiltinID, StringRef AliasName) {
+  // This will be filled in by Tablegen which isn't written yet
+  return false;
+}
+
 /// Returns a value indicating whether this function corresponds to a builtin
 /// function.
 ///
@@ -3091,10 +3097,24 @@
 /// functions as their wrapped builtins. This shouldn't be done in general, but
 /// it's useful in Sema to diagnose calls to wrappers based on their semantics.
 unsigned FunctionDecl::getBuiltinID(bool ConsiderWrapperFunctions) const {
-  if (!getIdentifier())
-return 0;
+  unsigned BuiltinID;
+
+  if (hasAttr()) {
+IdentifierInfo *II = getAttr()->getBuiltinName();
+BuiltinID = II->getBuiltinID();
+
+if (!ArmMveAliasValid(BuiltinID, getIdentifier()->getName())) {
+  getASTContext().getDiagnostics().Report(
+getLocation(), diag::err_attribute_arm_mve_alias);
+  return 0;
+}
+  } else {
+if (!getIdentifier())
+  return 0;
+
+BuiltinID = getIdentifier()->getBuiltinID();
+  }
 
-  unsigned BuiltinID = getIdentifier()->getBuiltinID();
   if (!BuiltinID)
 return 0;
 
@@ -3118,7 +3138,8 @@
 
   // If the function is marked "overloadable", it has a different mangled name
   // and is not the C library function.
-  if (!ConsiderWrapperFunctions && hasAttr())
+  if 

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham updated this revision to Diff 219692.
simon_tatham added a comment.

Added a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/lax-vector-conversions.c


Index: clang/test/Driver/lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Driver/lax-vector-conversions.c
@@ -0,0 +1,8 @@
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main -### -c %s 2>&1 
\
+// RUN:   | FileCheck --check-prefix=CHECK-STRICT %s
+
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8a -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LAX %s
+
+// CHECK-STRICT: "-fno-lax-vector-conversions"
+// CHECK-LAX-NOT: "-fno-lax-vector-conversions"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1363,6 +1363,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4678,10 +4689,13 @@
   if (TC.SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_mfentry);
 
-  // -flax-vector-conversions is default.
-  if (!Args.hasFlag(options::OPT_flax_vector_conversions,
-options::OPT_fno_lax_vector_conversions))
+  if (const Arg *A = Args.getLastArg(options::OPT_flax_vector_conversions,
+options::OPT_fno_lax_vector_conversions)) {
+if (A->getOption().matches(options::OPT_fno_lax_vector_conversions))
+  CmdArgs.push_back("-fno-lax-vector-conversions");
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
 CmdArgs.push_back("-fno-lax-vector-conversions");
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))


Index: clang/test/Driver/lax-vector-conversions.c
===
--- /dev/null
+++ clang/test/Driver/lax-vector-conversions.c
@@ -0,0 +1,8 @@
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8.1m.main -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STRICT %s
+
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8a -### -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-LAX %s
+
+// CHECK-STRICT: "-fno-lax-vector-conversions"
+// CHECK-LAX-NOT: "-fno-lax-vector-conversions"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1363,6 +1363,17 @@
   }
 }
 
+static bool isLaxVectorConversionsDefault(const llvm::Triple &Triple) {
+  switch (Triple.getArch()) {
+  default:
+return true;
+
+  case llvm::Triple::thumb:
+  case llvm::Triple::thumbeb:
+return Triple.getSubArch() != llvm::Triple::ARMSubArch_v8_1m_mainline;
+  }
+}
+
 static bool isNoCommonDefault(const llvm::Triple &Triple) {
   switch (Triple.getArch()) {
   default:
@@ -4678,10 +4689,13 @@
   if (TC.SupportsProfiling())
 Args.AddLastArg(CmdArgs, options::OPT_mfentry);
 
-  // -flax-vector-conversions is default.
-  if (!Args.hasFlag(options::OPT_flax_vector_conversions,
-options::OPT_fno_lax_vector_conversions))
+  if (const Arg *A = Args.getLastArg(options::OPT_flax_vector_conversions,
+options::OPT_fno_lax_vector_conversions)) {
+if (A->getOption().matches(options::OPT_fno_lax_vector_conversions))
+  CmdArgs.push_back("-fno-lax-vector-conversions");
+  } else if (!isLaxVectorConversionsDefault(Triple)) {
 CmdArgs.push_back("-fno-lax-vector-conversions");
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67382: [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Kristóf Umann via Phabricator via cfe-commits]

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

LGTM! I think code readability improved geatly.

> This creates a certain problem in `RetainCountChecker` (surprise!!~)

We constantly bully this checker, but still not enough :^)




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:284
+  getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC,
+ bool UseEnd = false);
+

Lets explain what `UseEnd` is: `Whether to use getEndLoc() rather then 
getBeginLoc() for \p S`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:276
+
+  /// Find the next statement that is going to be executed after this node.
+  /// Useful for explaining control flow that follows the current node.

Is this correct? Shouldn't we instead say that "Find the next statement that 
was symbolically executed on this path of execution"?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:282
+
+  /// Find the statement that was executed immediately before that node.
+  /// Useful when the node corresponds to a CFG block entrance.

before this node?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:288
+
+  /// Find the statement that was executed at or immediately before that node.
+  /// Useful when any nearby statement will do.

before this node?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2185
   } else {
-assert(ErrorNode);
-hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));

Why delete the assert?



Comment at: clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp:302-303
   // Find the node's current statement in the CFG.
-  if (const Stmt *S = PathDiagnosticLocation::getStmt(this))
+  // FIXME: getStmtForDiagnostics() does nasty things in order to provide
+  // a valid statement for body farms, do we need this behavior here?
+  if (const Stmt *S = getStmtForDiagnostics())

But still fails...



Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:565-566
 
   // FIXME: Ironically, this assert actually fails in some cases.
   //assert(L.isValid());
   return L;

I guess didn't change much :^)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67382



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

[PATCH] D67419: [analyzer] NFC: Move PathDiagnostic to libAnalysis.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Looks great! Are we sure that `PathDiagnostic.h` is a good header name?




Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:26
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Config/config.h"
 #include "clang/Format/Format.h"

NoQ wrote:
> This needed to go up because that's where `CLANG_ENABLE_STATIC_ANALYZER` is 
> defined.
Let's add some scary comments then, to warn everyone trying to blindly 
clang-format this file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67419



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

[PATCH] D67419: [analyzer] NFC: Move PathDiagnostic to libAnalysis.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:26
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Config/config.h"
 #include "clang/Format/Format.h"

Szelethus wrote:
> NoQ wrote:
> > This needed to go up because that's where `CLANG_ENABLE_STATIC_ANALYZER` is 
> > defined.
> Let's add some scary comments then, to warn everyone trying to blindly 
> clang-format this file.
I'd suggest to move all conditionally-included headers into a separate 
"section" below, separated by a newline from the rest of the headers. 
clang-format will respect that and won't reorder sections.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67419



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Are you planning to move users of these options -- like `PlistDiagnostics` -- 
to libAnalysis?




Comment at: clang/include/clang/Analysis/PathDiagnostic.h:81
+  /// because deterministic mode is always superior when done right, but
+  /// for some consumers is experimental and needs to be off by default.
+  bool ShouldWriteStableReportFilename;

"but for some consumers is experimental" -- parse error. What is experimental?



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:534
+  C.push_back(new PlistDiagnostics(std::move(DiagOpts), s, PP, CTU,
+   /*supportsMultipleFiles*/ false));
+}

I think we need an equal sign for ClangTidy to pick it up: `/*foo=*/`


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr requested changes to this revision.
gribozavr added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.h:37
 const cross_tu::CrossTranslationUnitContext &CTU);
-#include "clang/StaticAnalyzer/Core/Analyses.def"
+#include "clang/Analysis/PathDiagnosticConsumers.def"
 

Does this code declare e.g., createPlistDiagnosticConsumer? That function is 
defined in libStaticAnalyzer. Declaring it libAnalysis is not appropriate, and 
effectively introduces a layering violation. Users who link against libAnalysis 
will get link errors if they don't link against libStaticAnalyzer.



Repository:
  rC Clang

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

https://reviews.llvm.org/D67422



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

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

The "bugtype" in IssueHash is specific to Static Analyzer (or at least not 
documented sufficiently abstractly).


Repository:
  rC Clang

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

https://reviews.llvm.org/D67421



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

[PATCH] D67160: [clang, ARM] Default to -fno-lax-vector-conversions in ARM v8.1-M.

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67160



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

> @Szelethus: I could have stored `PathDiagnosticConsumerOptions` in 
> `AnalyzerOptions` by value and pass a const reference around, but it wasn't 
> pleasant to integrate with `AnalyzerOptions.def`. I.e., i'd have to implement 
> a new kind of option that doesn't allocate its own field but instead re-uses 
> a field within a sub-object. Do you want me to go for it or is this 
> implementation good enough?

I don't really like this direction :^) Imagine how tedious and non-obvious 
it'll be to add a new pathdiagnostic option. I don't like we're copying this 
around. There is also a discussion to be had about whether having to specify 
`-analyzer-config` to a no longer analyzer specific feature is any good.

> or do you have other approaches in mind?

I do! The level idea is to completely separate pathdiagnostic configs from the 
`AnalyzerOptions`, I believe this is more in line with your goal.

- Let's create the equivalent of `-analyzer-config`, `-pathdiagnostic-config`, 
and things like `-pathdiagnostic-config-help`.
- Avoid getting a parsing error for specifying `-analyzer-config 
macro-expansion` instead of  `-pathdiagnostic-config macro-expansion`.
  - Create a field similar to `AnalyzerOptions::AnalyzerConfigCmdFlags` called 
`AnalyzerOptions::AnalyzerConfigAliases` with the 4 configs that we're moving 
over. In compatibility mode, `AnalyzerOptions::isUnknownAnalyzerConfig()` shall 
search in this field as well. In non-compatibility mode, this will result in an 
error.
- Make `PathDiagnosticConsumerOptions` field of `CompilerInvocation`, similar 
to `AnalyzerOptions`.
- Your idea of implementing a `DIAGNOSTIC_OPTION` macro sounds nice.
  - Delete these entries from `AnalyzerOptions.def`.
  - Create `clang/include/Analysis/PathDiagnostics.def`, list them there.
  - Generate fields to `PathDiagnosticConsumerOptions` similar to how 
`AnalyzerOptions` does it.
- Parse the options `CompilerInvocation.cpp`
  - Most of the analyzer config parsing functions can be reused!
  - For these select 4 options that suffer compatibility issues, manually check 
`AnalyzerOptions::ConfigTable`.

WDYT? I am happy to help out, I realize this is plenty of code to write, if we 
go for it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

rebase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64146



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

r371627 - [OPENMP]Updated status page, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Sep 11 07:44:30 2019
New Revision: 371627

URL: http://llvm.org/viewvc/llvm-project?rev=371627&view=rev
Log:
[OPENMP]Updated status page, NFC.

Modified:
cfe/trunk/docs/OpenMPSupport.rst

Modified: cfe/trunk/docs/OpenMPSupport.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/OpenMPSupport.rst?rev=371627&r1=371626&r2=371627&view=diff
==
--- cfe/trunk/docs/OpenMPSupport.rst (original)
+++ cfe/trunk/docs/OpenMPSupport.rst Wed Sep 11 07:44:30 2019
@@ -149,7 +149,7 @@ implementation.
 
+--+--+--+---+
 | loop extension   | collapse imperfectly nested loop  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| loop extension   | collapse non-rectangular nested loop  
   | :part:`worked on`| 
  |
+| loop extension   | collapse non-rectangular nested loop  
   | :good:`done` | 
  |
 
+--+--+--+---+
 | loop extension   | C++ range-base for loop   
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
@@ -233,7 +233,7 @@ implementation.
 
+--+--+--+---+
 | device extension | map(replicate) or map(local) when requires 
unified_shared_me | :part:`worked on`| D55719,D55892
 |
 
+--+--+--+---+
-| device extension | teams construct on the host device
   | :part:`worked on`| 
  |
+| device extension | teams construct on the host device
   | :part:`worked on`| Clang part is done, r371553.
  |
 
+--+--+--+---+
 | atomic extension | hints for the atomic construct
   | :part:`worked on`| D51233  
  |
 
+--+--+--+---+
@@ -251,7 +251,7 @@ implementation.
 
+--+--+--+---+
 | misc extension   | conditional modifier for lastprivate clause   
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| misc extension   | user-defined function variants
   | :none:`unclaimed`| 
  |
+| misc extension   | user-defined function variants
   | :part:`worked on`| D67294, D64095  
  |
 
+--+--+--+-

[PATCH] D67294: Register and parse a simplified version of '#pragma omp declare variant'

[Andrzej Warzynski via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParseOpenMP.cpp:779-783
+  auto Identinfo = PP.getIdentifierInfo(VectorVariantId.Ident->getName());
+  LookupResult Lookup(Actions, Identinfo, VectorVariantId.Loc,
+  Sema::LookupOrdinaryName);
+  CXXScopeSpec SS;
+  if (!Actions.LookupParsedName(Lookup, getCurScope(), &SS)) {

ABataev wrote:
> andwar wrote:
> > ABataev wrote:
> > > 1. All this lookup must be performed in Sema.
> > > 2. How do you want to handle templates?
> > > 3. Why just do not parse it as an expression? And rely on existing 
> > > parsing/sema analysis for expressions.
> > 1. That's fine, let me move it there.
> > 
> > 2. TL;DR We're not handling templates yet. 
> > 
> > 3. How about switching to `ParseUnqualifiedId` instead? That will still 
> > require going through `LookupParsedName` though.
> > 
> > Re 2. What the interaction between templates and #pragma(s) should be? The 
> > C++ standard doesn't specify that, but IMHO mixing '#pragma omp declare 
> > variant' and templates requires that interaction to be clearly defined. 
> > 
> > I know that OpenMP 5 allows 'variant-func-id' to be a template-id, but I 
> > couldn't find an example of any attribute that would use C++ template 
> > parameters. Also, isn't parsing of attributes and templates _orthogonal_ in 
> > clang (i.e. happening at completely different points during parsing)?
> > 
> > In any case, we felt that support for 'template-id' could come later. How 
> > do you feel about it?
> No, don't try to parse function names directly. What about `::fun` format? Or 
> `std::fun`? Your solution does not support it.
> I have basic implementation that handles all these cases. Let me commit it, 
> because we need it to support other context selectors. It is very similar to 
> your solution but handles all the problems. We just don't have time to wait, 
> need to support other context selectors.
That's true - my approach doesn't support `::fun` or `std::fun`. The idea is to 
fill in the gaps as we progress later. 

@ABataev, If you have a more generic implementation  then it makes a lot of 
sense to go with that instead. Please, feel free to re-use this patch if that 
helps.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67294



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

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

@dnsampaio, many thanks for committing this!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66018



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

r371633 - [Clang][Bundler] Fix for a potential memory leak [NFC]

[Sergey Dmitriev via cfe-commits]

Author: sdmitriev
Date: Wed Sep 11 09:03:21 2019
New Revision: 371633

URL: http://llvm.org/viewvc/llvm-project?rev=371633&view=rev
Log:
[Clang][Bundler] Fix for a potential memory leak [NFC]

Bundler leaks memory if it is called with -type=o but given input isn't an 
object file (though it has to have a known binary type like IR, archive, 
etc...). Memory leak is happening when binary object returned by the 
createBinary(...) call cannot be casted to an ObjectFile type. In this case 
returned BinaryOrErr object releases ownership of the binary, but no one is 
taking it (see line 626).

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

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=371633&r1=371632&r2=371633&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Wed Sep 11 
09:03:21 2019
@@ -611,24 +611,15 @@ static FileHandler *CreateObjectFileHand
   // Check if the input file format is one that we know how to deal with.
   Expected> BinaryOrErr = createBinary(FirstInput);
 
-  // Failed to open the input as a known binary. Use the default binary 
handler.
-  if (!BinaryOrErr) {
-// We don't really care about the error (we just consume it), if we could
-// not get a valid device binary object we use the default binary handler.
-consumeError(BinaryOrErr.takeError());
+  // We only support regular object files. If failed to open the input as a
+  // known binary or this is not an object file use the default binary handler.
+  if (errorToBool(BinaryOrErr.takeError()) || !isa(*BinaryOrErr))
 return new BinaryFileHandler();
-  }
 
-  // We only support regular object files. If this is not an object file,
-  // default to the binary handler. The handler will be owned by the client of
-  // this function.
-  std::unique_ptr Obj(
-  dyn_cast(BinaryOrErr.get().release()));
-
-  if (!Obj)
-return new BinaryFileHandler();
-
-  return new ObjectFileHandler(std::move(Obj));
+  // Otherwise create an object file handler. The handler will be owned by the
+  // client of this function.
+  return new ObjectFileHandler(
+  std::unique_ptr(cast(BinaryOrErr->release(;
 }
 
 /// Return an appropriate handler given the input files and options.


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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

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



Comment at: compiler-rt/trunk/lib/profile/xxhash.h:41-42
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+

chandlerc wrote:
> Sorry folks, but you can't do this.
> 
> You can't depend on ADT from compiler-rt currently.
> 
> There are at least two problems here:
> 
> First problem is that this pollutes the profile library with symbols from 
> ADT. That really doesn't seem reasonable without *significant* and invasive 
> changes to ADT. Otherwise building LLVM and linking it with the profile 
> library will create an ODR violation (imagine different assert levels or 
> different versions of LLVM buing built and the host toolchain).
> 
> 
> Second, and much more critically, we haven't gotten to 100% relicensed on 
> ADT, so it is critical that we not depend on it from runtime libraries.
> 
> Third, a lot of this code seems to use old license headers. Please do not add 
> any code like this to LLVM, and instead use the new LLVM license for all new 
> code.
> 
> For now, this patch (and any related patches) need to be reverted until these 
> are addressed. Especially the license issues.
Sorry, this looks like a mismerge somehow. My patch should't have anything from 
compiler-rt. I think maybe a local change got rolled in when Petr landed my 
patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: compiler-rt/trunk/lib/profile/xxhash.h:41-42
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+

paulkirth wrote:
> chandlerc wrote:
> > Sorry folks, but you can't do this.
> > 
> > You can't depend on ADT from compiler-rt currently.
> > 
> > There are at least two problems here:
> > 
> > First problem is that this pollutes the profile library with symbols from 
> > ADT. That really doesn't seem reasonable without *significant* and invasive 
> > changes to ADT. Otherwise building LLVM and linking it with the profile 
> > library will create an ODR violation (imagine different assert levels or 
> > different versions of LLVM buing built and the host toolchain).
> > 
> > 
> > Second, and much more critically, we haven't gotten to 100% relicensed on 
> > ADT, so it is critical that we not depend on it from runtime libraries.
> > 
> > Third, a lot of this code seems to use old license headers. Please do not 
> > add any code like this to LLVM, and instead use the new LLVM license for 
> > all new code.
> > 
> > For now, this patch (and any related patches) need to be reverted until 
> > these are addressed. Especially the license issues.
> Sorry, this looks like a mismerge somehow. My patch should't have anything 
> from compiler-rt. I think maybe a local change got rolled in when Petr landed 
> my patch.
Sorry about that, this was my mistake, it seems two of my stales files got 
included in when I applied the patch locally. I'll revert the change and reland 
it correctly.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D66324#1665784 , @lebedev.ri wrote:

> In D66324#1665773 , @gribozavr wrote:
>
> > Reverted in r371598.
> >
> > Another concern that I have is cross-compilation. LLVM's ADT is not set up 
> > to be cross-compiled like the rest of compiler-rt is.
>
>
> Uhm, i have a better question still - why xxhash is even there? it's not used 
> in the diff, and was not reviewed, it just magically appeared in the last 
> update of the patch:
>  https://reviews.llvm.org/D66324?vs=219617&id=219645


It shouldn't have. My last change only modified a test file to not trigger a 
memory corruption bug. I'm filing a bug for that, but am waiting on a login for 
the bug tracker.

What I suspect happened is that when this landed some local changes got sucked 
in. I'm terribly sorry, it wasn't my intention to add anything significant to 
the patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 219728.
paulkirth added a comment.

Revert mismerge when landing.


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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch-unpredictable.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 0.00% (0 / 8112) of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:2

r371635 - Reland "clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM"

[Petr Hosek via cfe-commits]

Author: phosek
Date: Wed Sep 11 09:19:50 2019
New Revision: 371635

URL: http://llvm.org/viewvc/llvm-project?rev=371635&view=rev
Log:
Reland "clang-misexpect: Profile Guided Validation of Performance Annotations 
in LLVM"

This patch contains the basic functionality for reporting potentially
incorrect usage of __builtin_expect() by comparing the developer's
annotation against a collected PGO profile. A more detailed proposal and
discussion appears on the CFE-dev mailing list
(http://lists.llvm.org/pipermail/cfe-dev/2019-July/062971.html) and a
prototype of the initial frontend changes appear here in D65300

We revised the work in D65300 by moving the misexpect check into the
LLVM backend, and adding support for IR and sampling based profiles, in
addition to frontend instrumentation.

We add new misexpect metadata tags to those instructions directly
influenced by the llvm.expect intrinsic (branch, switch, and select)
when lowering the intrinsics. The misexpect metadata contains
information about the expected target of the intrinsic so that we can
check against the correct PGO counter when emitting diagnostics, and the
compiler's values for the LikelyBranchWeight and UnlikelyBranchWeight.
We use these branch weight values to determine when to emit the
diagnostic to the user.

A future patch should address the comment at the top of
LowerExpectIntrisic.cpp to hoist the LikelyBranchWeight and
UnlikelyBranchWeight values into a shared space that can be accessed
outside of the LowerExpectIntrinsic pass. Once that is done, the
misexpect metadata can be updated to be smaller.

In the long term, it is possible to reconstruct portions of the
misexpect metadata from the existing profile data. However, we have
avoided this to keep the code simple, and because some kind of metadata
tag will be required to identify which branch/switch/select instructions
are influenced by the use of llvm.expect

Patch By: paulkirth
Differential Revision: https://reviews.llvm.org/D66324

Added:
cfe/trunk/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
cfe/trunk/test/Profile/Inputs/misexpect-branch.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default-only.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-default.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch-nonconst.proftext
cfe/trunk/test/Profile/Inputs/misexpect-switch.proftext
cfe/trunk/test/Profile/misexpect-branch-cold.c
cfe/trunk/test/Profile/misexpect-branch-nonconst-expected-val.c
cfe/trunk/test/Profile/misexpect-branch-unpredictable.c
cfe/trunk/test/Profile/misexpect-branch.c
cfe/trunk/test/Profile/misexpect-switch-default.c
cfe/trunk/test/Profile/misexpect-switch-nonconst.c
cfe/trunk/test/Profile/misexpect-switch-only-default-case.c
cfe/trunk/test/Profile/misexpect-switch.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=371635&r1=371634&r2=371635&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Wed Sep 11 
09:19:50 2019
@@ -275,7 +275,12 @@ def warn_profile_data_missing : Warning<
 def warn_profile_data_unprofiled : Warning<
   "no profile data available for file \"%0\"">,
   InGroup;
-
+def warn_profile_data_misexpect : Warning<
+  "Potential performance regression from use of __builtin_expect(): "
+  "Annotation was correct on %0 of profiled executions.">,
+  BackendInfo,
+  InGroup,
+  DefaultIgnore;
 } // end of instrumentation issue category
 
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=371635&r1=371634&r2=371635&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Sep 11 09:19:50 2019
@@ -1042,6 +1042,7 @@ def BackendOptimizationFailure : DiagGro
 def ProfileInstrMissing : DiagGroup<"profile-instr-missing">;
 def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
 def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
+def MisExpect : DiagGroup<"misexpect">;
 
 // AddressSanitizer frontend instrumentation remarks.
 def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=371635&r1=371634&r2=371635&view=diff
=

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371635: Reland "clang-misexpect: Profile Guided 
Validation of Performance Annotations… (authored by phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D66324?vs=219728&id=219733#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

Files:
  cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-branch.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch-default-only.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch-default.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch-nonconst.proftext
  cfe/trunk/test/Profile/Inputs/misexpect-switch.proftext
  cfe/trunk/test/Profile/misexpect-branch-cold.c
  cfe/trunk/test/Profile/misexpect-branch-nonconst-expected-val.c
  cfe/trunk/test/Profile/misexpect-branch-unpredictable.c
  cfe/trunk/test/Profile/misexpect-branch.c
  cfe/trunk/test/Profile/misexpect-switch-default.c
  cfe/trunk/test/Profile/misexpect-switch-nonconst.c
  cfe/trunk/test/Profile/misexpect-switch-only-default-case.c
  cfe/trunk/test/Profile/misexpect-switch.c
  llvm/trunk/include/llvm/IR/DiagnosticInfo.h
  llvm/trunk/include/llvm/IR/FixedMetadataKinds.def
  llvm/trunk/include/llvm/IR/MDBuilder.h
  llvm/trunk/include/llvm/Transforms/Utils/MisExpect.h
  llvm/trunk/lib/IR/DiagnosticInfo.cpp
  llvm/trunk/lib/IR/MDBuilder.cpp
  llvm/trunk/lib/Transforms/IPO/SampleProfile.cpp
  llvm/trunk/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
  llvm/trunk/lib/Transforms/Utils/MisExpect.cpp
  llvm/trunk/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/trunk/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/trunk/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-stripped.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch-unpredictable.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/trunk/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/trunk/lib/IR/MDBuilder.cpp
===
--- llvm/trunk/lib/IR/MDBuilder.cpp
+++ llvm/trunk/lib/IR/MDBuilder.cpp
@@ -309,3 +309,15 @@
   };
   return MDNode::get(Context, Vals);
 }
+
+MDNode *MDBuilder::createMisExpect(uint64_t Index, uint64_t LikleyWeight,
+   uint64_t UnlikleyWeight) {
+  auto *IntType = Type::getInt64Ty(Context);
+  Metadata *Vals[] = {
+  createString("misexpect"),
+  createConstant(ConstantInt::get(IntType, Index)),
+  createConstant(ConstantInt::get(IntType, LikleyWeight)),
+  createConstant(ConstantInt::get(IntType, UnlikleyWeight)),
+  };
+  return MDNode::get(Context, Vals);
+}
Index: llvm/trunk/lib/IR/DiagnosticInfo.cpp
===
--- llvm/trunk/lib/IR/DiagnosticInfo.cpp
+++ llvm/trunk/lib/IR/DiagnosticInfo.cpp
@@ -370,5 +370,16 @@
   return OS.str();
 }
 
+DiagnosticInfoMisExpect::DiagnosticInfoMisExpect(const Instruction *Inst,
+ Twine &Msg)
+: DiagnosticInfoWithLocationBase(DK_MisExpect, DS_Warning,
+ *Inst->getParent()->getParent(),
+ Inst->getDebugLoc()),
+  Msg(Msg) {}
+
+void DiagnosticInfoMisExpect::print(DiagnosticPrinter &DP) const {
+  DP << getLocationStr() << ": " << getMsg();
+}
+
 void OptimizationRemarkAnalysisFPCommute::anchor() {}
 void OptimizationRemarkAnalysisAliasing::anchor() {}
Index: llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
===
--- llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
+++ llvm/trunk/lib/Transforms/Utils/CMakeLists.txt
@@ -40,6 +40,7 @@
   LowerSwitch.cpp
   Mem2Reg.cpp
   MetaRenamer.cpp
+  MisExpect.cpp
   ModuleUtils.cpp
   NameAnonGlobals.cpp
   PredicateInfo.cpp
Index: llvm/trunk/lib/Transforms/Utils/MisExpect.cpp
===
--- llvm/trunk/lib/Transforms/Utils/MisExpect.cpp
+++ llvm/trunk/lib/Transforms/Utils/MisExpect.cpp
@@ -0,0 +1,177 @@
+//===--- MisExpect.

r371637 - [Clang][Bundler] Replace std::vector by SmallVector [NFC]

[Sergey Dmitriev via cfe-commits]

Author: sdmitriev
Date: Wed Sep 11 09:28:47 2019
New Revision: 371637

URL: http://llvm.org/viewvc/llvm-project?rev=371637&view=rev
Log:
[Clang][Bundler] Replace std::vector by SmallVector [NFC]

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

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=371637&r1=371636&r2=371637&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Wed Sep 11 
09:28:47 2019
@@ -17,6 +17,7 @@
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -41,7 +42,6 @@
 #include 
 #include 
 #include 
-#include 
 
 using namespace llvm;
 using namespace llvm::object;
@@ -658,10 +658,8 @@ static bool BundleFiles() {
   }
 
   // Open input files.
-  std::vector> InputBuffers(
-  InputFileNames.size());
-
-  unsigned Idx = 0;
+  SmallVector, 8u> InputBuffers;
+  InputBuffers.reserve(InputFileNames.size());
   for (auto &I : InputFileNames) {
 ErrorOr> CodeOrErr =
 MemoryBuffer::getFileOrSTDIN(I);
@@ -669,7 +667,7 @@ static bool BundleFiles() {
   errs() << "error: Can't open file " << I << ": " << EC.message() << "\n";
   return true;
 }
-InputBuffers[Idx++] = std::move(CodeOrErr.get());
+InputBuffers.emplace_back(std::move(CodeOrErr.get()));
   }
 
   // Get the file handler. We use the host buffer as reference.


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

[PATCH] D67452: [clang] [unittest] Import LLVMTestingSupport if necessary

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: sammccall, gribozavr, compnerd.
Herald added subscribers: kadircet, ilya-biryukov.

Add LLVMTestingSupport directory from LLVM_MAIN_SRC_DIR when building
clang stand-alone and LLVMTestingSupport library is not present.  This
is needed to fix stand-alone builds without clang-tools-extra.

// The code is copied verbatim from clangd unittests


https://reviews.llvm.org/D67452

Files:
  clang/unittests/CMakeLists.txt


Index: clang/unittests/CMakeLists.txt
===
--- clang/unittests/CMakeLists.txt
+++ clang/unittests/CMakeLists.txt
@@ -1,6 +1,15 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if(CLANG_BUILT_STANDALONE)
+  # LLVMTestingSupport library is needed for some of the unittests.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang


Index: clang/unittests/CMakeLists.txt
===
--- clang/unittests/CMakeLists.txt
+++ clang/unittests/CMakeLists.txt
@@ -1,6 +1,15 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if(CLANG_BUILT_STANDALONE)
+  # LLVMTestingSupport library is needed for some of the unittests.
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  AND NOT TARGET LLVMTestingSupport)
+add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
+  lib/Testing/Support)
+  endif()
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Up :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D67031: [Clang][Bundler] Error reporting improvements

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev updated this revision to Diff 219742.
sdmitriev added a reviewer: Hahnfeld.
sdmitriev added a comment.

Rebase


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

https://reviews.llvm.org/D67031

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -26,15 +26,17 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/StringSaver.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
@@ -42,6 +44,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace llvm::object;
@@ -122,35 +125,36 @@
 
   /// Update the file handler with information from the header of the bundled
   /// file
-  virtual void ReadHeader(MemoryBuffer &Input) = 0;
+  virtual Error ReadHeader(MemoryBuffer &Input) = 0;
 
   /// Read the marker of the next bundled to be read in the file. The triple of
   /// the target associated with that bundle is returned. An empty string is
   /// returned if there are no more bundles to be read.
-  virtual StringRef ReadBundleStart(MemoryBuffer &Input) = 0;
+  virtual Expected>
+  ReadBundleStart(MemoryBuffer &Input) = 0;
 
   /// Read the marker that closes the current bundle.
-  virtual void ReadBundleEnd(MemoryBuffer &Input) = 0;
+  virtual Error ReadBundleEnd(MemoryBuffer &Input) = 0;
 
   /// Read the current bundle and write the result into the stream \a OS.
-  virtual void ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
+  virtual Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
 
   /// Write the header of the bundled file to \a OS based on the information
   /// gathered from \a Inputs.
-  virtual void WriteHeader(raw_fd_ostream &OS,
-   ArrayRef> Inputs) = 0;
+  virtual Error WriteHeader(raw_fd_ostream &OS,
+ArrayRef> Inputs) = 0;
 
   /// Write the marker that initiates a bundle for the triple \a TargetTriple to
   /// \a OS.
-  virtual void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
+  virtual Error WriteBundleStart(raw_fd_ostream &OS,
+ StringRef TargetTriple) = 0;
 
   /// Write the marker that closes a bundle for the triple \a TargetTriple to \a
-  /// OS. Return true if any error was found.
-
-  virtual bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
+  /// OS.
+  virtual Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
 
   /// Write the bundle from \a Input into \a OS.
-  virtual void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
+  virtual Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
 };
 
 /// Handler for binary files. The bundled file will have the following format
@@ -222,7 +226,7 @@
 
   ~BinaryFileHandler() final {}
 
-  void ReadHeader(MemoryBuffer &Input) final {
+  Error ReadHeader(MemoryBuffer &Input) final {
 StringRef FC = Input.getBuffer();
 
 // Initialize the current bundle with the end of the container.
@@ -231,16 +235,16 @@
 // Check if buffer is smaller than magic string.
 size_t ReadChars = sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1;
 if (ReadChars > FC.size())
-  return;
+  return Error::success();
 
 // Check if no magic was found.
 StringRef Magic(FC.data(), sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1);
 if (!Magic.equals(OFFLOAD_BUNDLER_MAGIC_STR))
-  return;
+  return Error::success();
 
 // Read number of bundles.
 if (ReadChars + 8 > FC.size())
-  return;
+  return Error::success();
 
 uint64_t NumberOfBundles = Read8byteIntegerFromBuffer(FC, ReadChars);
 ReadChars += 8;
@@ -250,35 +254,35 @@
 
   // Read offset.
   if (ReadChars + 8 > FC.size())
-return;
+return Error::success();
 
   uint64_t Offset = Read8byteIntegerFromBuffer(FC, ReadChars);
   ReadChars += 8;
 
   // Read size.
   if (ReadChars + 8 > FC.size())
-return;
+return Error::success();
 
   uint64_t Size = Read8byteIntegerFromBuffer(FC, ReadChars);
   ReadChars += 8;
 
   // Read triple size.
   if (ReadChars + 8 > FC.size())
-return;
+return Error::success();
 
   uint64_t TripleSize = Read8byteIntegerFromBuffer(FC, ReadChars)

[PATCH] D67381: [analyzer] NFC: Move stack hints to a side map.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:34
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "llvm/ADT/ArrayRef.h"

Szelethus wrote:
> Nice, how did you catch this? Some compiler warning?
No, it's just that removing these includes is the whole point of the patch. In 
order to be able to move this file out of static analyzer, it should not 
include headers from static analyzer. This patch removes one such include.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67381



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

[PATCH] D67246: clang-format: Add support for formatting lambdas with explicit template parameters.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Do you think this should land with the comment FIXME for now? It improves 
formatting of this language feature when that heuristic is not used, and 
changing the heuristic is independent of the rest of this patch.


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

https://reviews.llvm.org/D67246



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

[PATCH] D67382: [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2185
   } else {
-assert(ErrorNode);
-hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));

Szelethus wrote:
> Why delete the assert?
Because it's enforced by the type system these days. This is a method on 
`PathSensitiveBugReport` that always has an error node, as asserted in its 
constructor. I should have removed this assert in D66572.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67382



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

r371642 - [MS] Consder constexpr globals to be inline, as in C++17

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Sep 11 11:09:10 2019
New Revision: 371642

URL: http://llvm.org/viewvc/llvm-project?rev=371642&view=rev
Log:
[MS] Consder constexpr globals to be inline, as in C++17

Summary:
Microsoft seems to do this regardless of the language mode, so we must
also do it in order to be ABI compatible.

Fixes PR36125

Reviewers: thakis

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGenCXX/ms-constexpr-static-data-member.cpp
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
cfe/trunk/test/CXX/drs/dr7xx.cpp
cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members-exported.cpp
cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members.cpp
cfe/trunk/test/SemaCXX/cxx1y-variable-templates_in_class.cpp
cfe/trunk/test/SemaCXX/dllexport.cpp
cfe/trunk/test/SemaCXX/dllimport.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=371642&r1=371641&r2=371642&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Sep 11 11:09:10 2019
@@ -6846,7 +6846,9 @@ NamedDecl *Sema::ActOnVariableDeclarator
 // C++1z [dcl.spec.constexpr]p1:
 //   A static data member declared with the constexpr specifier is
 //   implicitly an inline variable.
-if (NewVD->isStaticDataMember() && getLangOpts().CPlusPlus17)
+if (NewVD->isStaticDataMember() &&
+(getLangOpts().CPlusPlus17 ||
+ Context.getTargetInfo().getCXXABI().isMicrosoft()))
   NewVD->setImplicitlyInline();
 break;
 
@@ -12003,7 +12005,8 @@ void Sema::ActOnUninitializedDecl(Decl *
   if (Var->isStaticDataMember()) {
 // C++1z removes the relevant rule; the in-class declaration is always
 // a definition there.
-if (!getLangOpts().CPlusPlus17) {
+if (!getLangOpts().CPlusPlus17 &&
+!Context.getTargetInfo().getCXXABI().isMicrosoft()) {
   Diag(Var->getLocation(),
diag::err_constexpr_static_mem_var_requires_init)
 << Var->getDeclName();

Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp?rev=371642&r1=371641&r2=371642&view=diff
==
--- cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp (original)
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp Wed Sep 11 
11:09:10 2019
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++1z %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify 
-std=c++11 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify 
-std=c++14 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fsyntax-only -verify 
-std=c++1z %s
+
+// MSVC always adopted the C++17 rule that implies that constexpr variables are
+// implicitly inline, so do the test again.
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -DMS_ABI -fsyntax-only -verify 
-std=c++11 %s
 
 struct notlit { // expected-note {{not literal because}}
   notlit() {}
@@ -29,7 +33,7 @@ void f2(constexpr int i) {} // expected-
 struct s2 {
   constexpr int mi1; // expected-error {{non-static data member cannot be 
constexpr; did you intend to make it const?}}
   static constexpr int mi2;
-#if __cplusplus <= 201402L
+#if __cplusplus <= 201402L && !defined(MS_ABI)
   // expected-error@-2 {{requires an initializer}}
 #else
   // expected-error@-4 {{default initialization of an object of const}}

Modified: cfe/trunk/test/CXX/drs/dr7xx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr7xx.cpp?rev=371642&r1=371641&r2=371642&view=diff
==
--- cfe/trunk/test/CXX/drs/dr7xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr7xx.cpp Wed Sep 11 11:09:10 2019
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors
-// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors
-// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors
-// RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++98 %s -verify 
-fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 %s -verify 
-fexceptions -fcxx-exceptions -pedantic-errors
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++14 %s -verify 
-fexce

[PATCH] D47956: [MS] Consder constexpr globals to be inline, as in C++17

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371642: [MS] Consder constexpr globals to be inline, as in 
C++17 (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47956?vs=219638&id=219746#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D47956

Files:
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
  cfe/trunk/test/CXX/drs/dr7xx.cpp
  cfe/trunk/test/CodeGenCXX/ms-constexpr-static-data-member.cpp
  cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members-exported.cpp
  cfe/trunk/test/CodeGenCXX/ms-integer-static-data-members.cpp
  cfe/trunk/test/SemaCXX/cxx1y-variable-templates_in_class.cpp
  cfe/trunk/test/SemaCXX/dllexport.cpp
  cfe/trunk/test/SemaCXX/dllimport.cpp

Index: cfe/trunk/test/SemaCXX/dllimport.cpp
===
--- cfe/trunk/test/SemaCXX/dllimport.cpp
+++ cfe/trunk/test/SemaCXX/dllimport.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple x86_64-win32   -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DMS %s
 // RUN: %clang_cc1 -triple i686-mingw32   -fsyntax-only -fms-extensions -verify -std=c++1y -Wunsupported-dll-base-class-template -DGNU %s
 // RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify -std=c++11 -Wunsupported-dll-base-class-template -DGNU %s
+// RUN: %clang_cc1 -triple x86_64-mingw32 -fsyntax-only -fms-extensions -verify -std=c++17 -Wunsupported-dll-base-class-template -DGNU %s
 
 // Helper structs to make templates more expressive.
 struct ImplicitInst_Imported {};
@@ -586,7 +587,10 @@
   __declspec(dllimport) static  const  int  StaticConstFieldEqualInit = 1;
   __declspec(dllimport) static  const  int  StaticConstFieldBraceInit{1};
   __declspec(dllimport) constexpr static int ConstexprField = 1;
-  __declspec(dllimport) constexpr static int ConstexprFieldDef = 1; // expected-note{{attribute is here}}
+#if __cplusplus < 201703L && !defined(MS)
+  // expected-note@+2{{attribute is here}}
+#endif
+  __declspec(dllimport) constexpr static int ConstexprFieldDef = 1;
 };
 
 #ifdef MS
@@ -631,7 +635,10 @@
 
int  ImportMembers::StaticFieldDef; // expected-error{{definition of dllimport static field not allowed}}
 const  int  ImportMembers::StaticConstFieldDef = 1; // expected-error{{definition of dllimport static field not allowed}}
-constexpr int ImportMembers::ConstexprFieldDef; // expected-error{{definition of dllimport static field not allowed}}
+#if __cplusplus < 201703L && !defined(MS)
+// expected-error@+2{{definition of dllimport static field not allowed}}
+#endif
+constexpr int ImportMembers::ConstexprFieldDef;
 
 
 // Import on member definitions.
@@ -667,7 +674,11 @@
 
 __declspec(dllimport)int  ImportMemberDefs::StaticField; // expected-error{{definition of dllimport static field not allowed}} expected-note{{attribute is here}}
 __declspec(dllimport) const  int  ImportMemberDefs::StaticConstField = 1; // expected-error{{definition of dllimport static field not allowed}} expected-note{{attribute is here}}
-__declspec(dllimport) constexpr int ImportMemberDefs::ConstexprField; // expected-error{{definition of dllimport static field not allowed}} expected-note{{attribute is here}}
+#if __cplusplus < 201703L && !defined(MS)
+// expected-error@+3{{definition of dllimport static field not allowed}}
+// expected-note@+2{{attribute is here}}
+#endif
+__declspec(dllimport) constexpr int ImportMemberDefs::ConstexprField;
 
 
 // Import special member functions.
@@ -800,7 +811,7 @@
 
   static int  StaticField; // expected-note{{previous declaration is here}}
   static  const  int  StaticConstField;// expected-note{{previous declaration is here}}
-  constexpr static int ConstexprField = 1; // expected-note{{previous declaration is here}}
+  constexpr static int ConstexprField = 1; // expected-note-re{{previous {{(declaration|definition)}} is here}}
 };
 
 __declspec(dllimport)void MemberRedecl::normalDef() {} // expected-error{{redeclaration of 'MemberRedecl::normalDef' cannot add 'dllimport' attribute}}
@@ -831,9 +842,15 @@
 __declspec(dllimport) const  int  MemberRedecl::StaticConstField = 1;  // expected-error{{redeclaration of 'MemberRedecl::StaticConstField' cannot add 'dllimport' attribute}}
// expected-error@-1{{definition of dllimport static field not allowed}}
// expected-note@-2{{attribute is here}}
-__declspec(dllimport) constexpr int MemberRedecl::ConstexprField;  // expected-error{{redeclaration of 'MemberRedecl::ConstexprField' cannot add 'dllimport' attribute}}
-

[PATCH] D67382: [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2185
   } else {
-assert(ErrorNode);
-hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));

NoQ wrote:
> Szelethus wrote:
> > Why delete the assert?
> Because it's enforced by the type system these days. This is a method on 
> `PathSensitiveBugReport` that always has an error node, as asserted in its 
> constructor. I should have removed this assert in D66572.
Previously this assert was on the generic `BugReport::Profile` and it was there 
to state that "either a report has an error node, or a manually set location". 
Now have a clear separation between path-sensitive and path-insensitive reports 
in our type system, so it doesn't make sense to enforce this alternative 
manually.

Also, no, it wasn't failing :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67382



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

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for doing all this work -- in general, this is looking great and 
really cleans things up!




Comment at: clang/include/clang/Basic/Attr.td:2266
   let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">,
-   C2x<"", "maybe_unused">];
+   C2x<"", "maybe_unused">, Pragma<"", "unused">];
   let Subjects = SubjectList<[Var, ObjCIvar, Type, Enum, EnumConstant, Label,

I'm surprised to see this as part of an NFC refactoring. Hopefully it will 
become more obvious later. :-D



Comment at: clang/include/clang/Basic/AttributeCommonInfo.h:66-69
+  unsigned AttrKind : 16;
+  /// Corresponds to the Syntax enum.
+  unsigned SyntaxUsed : 3;
+  unsigned SpellingIndex : 4;

Do you want to do `= 0` here to give them in-class initializers? (well, 
`SpellingIndex` should probably be `SpellingNotCalculated`).



Comment at: clang/include/clang/Lex/Preprocessor.h:373
 
-  /// The source location of the currently-active
+  /// The source location and identifier of the currently-active
   /// \#pragma clang arc_cf_code_audited begin.

Can you correct the order in the comment to be the same as the order in the 
pair?



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:298-300
+  AttributeCommonInfo({}, AttributeCommonInfo::AT_Aligned,
+  AttributeCommonInfo::AS_GNU,
+  AlignedAttr::GNU_aligned)));

I'm surprised we'd give a parse syntax to an implicit attribute. Wouldn't we 
rather note that these have no associated syntax or spelling?



Comment at: clang/lib/Sema/SemaAttr.cpp:892
 = (VisibilityAttr::VisibilityType) rawType;
+
   SourceLocation loc = Stack->back().second;

Spurious change?



Comment at: clang/lib/Sema/SemaDecl.cpp:8962-8965
+SourceLocation Loc = D.getDeclSpec().getNoreturnSpecLoc();
+AttributeCommonInfo Info(SourceRange{Loc}, 
AttributeCommonInfo::AT_NoReturn,
+ AttributeCommonInfo::AS_Keyword);
+NewFD->addAttr(::new (Context) C11NoReturnAttr(Context, Info));

It would be nice if we didn't have to repeat information, if possible. The 
previous version of this didn't need to specify `AT_NoReturn` because the 
`C11NoReturnAttr` was sufficient to glean that information. It would be awesome 
if we could find a way to make this more succinct. Perhaps 
`ConcreteAttr::Create(Context, Range, Syntax, Spelling = DefaultValue)` or 
something along those lines?



Comment at: clang/lib/Sema/SemaDecl.cpp:13880
   fmt = "NSString";
-FD->addAttr(FormatAttr::CreateImplicit(Context,
-   &Context.Idents.get(fmt),
-   FormatIdx+1,
-   HasVAListArg ? 0 : FormatIdx+2,
-   FD->getLocation()));
+FD->addAttr(FormatAttr::CreateImplicit(
+Context, &Context.Idents.get(fmt), FormatIdx + 1,

Spurious formatting change? Same for the next three changes.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:3349
+AttributeCommonInfo Info{SourceRange(VS.getOverrideLoc()),
+ AttributeCommonInfo::UnknownAttribute,
+ AttributeCommonInfo::AS_Keyword, 0};

Can we use something other than `UnknownAttribute` here? That implies we don't 
know the spelling of the attribute name, which seems wrong.



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4464-4465
 // merge the attribute into implementation.
-method->addAttr(
-  ObjCRequiresSuperAttr::CreateImplicit(S.Context,
-method->getLocation()));
+method->addAttr(ObjCRequiresSuperAttr::CreateImplicit(
+S.Context, method->getLocation()));
   }

Spurious formatting change.



Comment at: clang/lib/Sema/SemaObjCProperty.cpp:2414-2415
 if (property->hasAttr())
-  GetterMethod->addAttr(NSReturnsNotRetainedAttr::CreateImplicit(Context,
- Loc));
+  GetterMethod->addAttr(
+  NSReturnsNotRetainedAttr::CreateImplicit(Context, Loc));
 

I'll stop pointing out the formatting changes, but you should probably drop 
these from the patch.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:50
   FnScope->setHasFallthroughStmt();
-  return ::new (S.Context) auto(Attr);
+  return ::new (S.Context) FallThroughAttr(S.Context, A);
 }

Thank you for this change. I have no idea how the original code made it through 
a code review.



Comment at: clang/lib/Sema/Sema

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67421#1666058 , @gribozavr wrote:

> The "bugtype" in IssueHash is specific to Static Analyzer (or at least not 
> documented sufficiently abstractly).


Yeah, i'll need to document this.

That's basically the warning message; it doesn't have to have anything to do 
with the Analyzer's `BugType` structure. But if the warning message contains 
fragile stuff (eg., mentions a name of a variable or (why??) a line number), 
the user may want to wipe it out of the hash, and in this case Analyzer 
`BugType`s turn out to be fairly handy.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67421



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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/PathDiagnosticConsumers.h:37
 const cross_tu::CrossTranslationUnitContext &CTU);
-#include "clang/StaticAnalyzer/Core/Analyses.def"
+#include "clang/Analysis/PathDiagnosticConsumers.def"
 

gribozavr wrote:
> Does this code declare e.g., createPlistDiagnosticConsumer? That function is 
> defined in libStaticAnalyzer. Declaring it libAnalysis is not appropriate, 
> and effectively introduces a layering violation. Users who link against 
> libAnalysis will get link errors if they don't link against libStaticAnalyzer.
> 
Oh crap, i forgot to move them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67422



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67420#1666141 , @Szelethus wrote:

> I do!


Hmm, it sounds like you want to force both Clang frontend and Clang-Tidy to use 
the same set of flags to control these options (?) Much like 
`-analyzer-config`, these flags will have different "style" compared to other 
flags in the tool. Which is probably not too bad for hidden frontend flags that 
control the Analyzer because they're anyway set by a separate GUI checkbox, but 
the inconsistency with other flags would be super glaring in case of Clang-Tidy 
CLI. Do we really want to go in that direction? I'll be much more comfortable 
with letting each tool deal with its flags the way it prefers - this doesn't 
look like a good place for code reuse to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



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

Re: r371605 - [Diagnostics] Add -Wsizeof-array-div

[Yvan Roux via cfe-commits]

Hi David,

This commit broken ARMv7 bots, logs are available here:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/10203/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adiv-sizeof-array.cpp

Thanks,
Yvan



On Wed, 11 Sep 2019 at 12:58, David Bolvansky via cfe-commits
 wrote:
>
> Author: xbolva00
> Date: Wed Sep 11 03:59:47 2019
> New Revision: 371605
>
> URL: http://llvm.org/viewvc/llvm-project?rev=371605&view=rev
> Log:
> [Diagnostics] Add -Wsizeof-array-div
>
> Summary: Clang version of https://www.viva64.com/en/examples/v706/
>
> Reviewers: rsmith
>
> Differential Revision: https://reviews.llvm.org/D67287
>
> Added:
> cfe/trunk/test/Sema/div-sizeof-array.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11 03:59:47 
> 2019
> @@ -3406,6 +3406,10 @@ def note_pointer_declared_here : Note<
>  def warn_division_sizeof_ptr : Warning<
>"'%0' will return the size of the pointer, not the array itself">,
>InGroup>;
> +def warn_division_sizeof_array : Warning<
> +  "expression does not compute the number of elements in this array; element 
> "
> +  "type is %0, not %1">,
> +  InGroup>;
>
>  def note_function_warning_silence : Note<
>  "prefix with the address-of operator to silence this warning">;
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019
> @@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe
>else
>  RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>
> -  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
> -return;
> -  if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() !=
> -  RHSTy.getCanonicalType().getUnqualifiedType())
> -return;
> +  if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
> +if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), RHSTy))
> +  return;
>
> -  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
> LHS->getSourceRange();
> -  if (const auto *DRE = dyn_cast(LHSArg)) {
> -if (const ValueDecl *LHSArgDecl = DRE->getDecl())
> -  S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
> -  << LHSArgDecl;
> +S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
> LHS->getSourceRange();
> +if (const auto *DRE = dyn_cast(LHSArg)) {
> +  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
> +S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
> +<< LHSArgDecl;
> +}
> +  } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
> +QualType ArrayElemTy = ArrayTy->getElementType();
> +if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
> +S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
> +  return;
> +S.Diag(Loc, diag::warn_division_sizeof_array)
> +<< LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
> +if (const auto *DRE = dyn_cast(LHSArg)) {
> +  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
> +S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here)
> +<< LHSArgDecl;
> +}
>}
>  }
>
>
> Added: cfe/trunk/test/Sema/div-sizeof-array.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto
> ==
> --- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)
> +++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019
> @@ -0,0 +1,28 @@
> +// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
> +
> +template 
> +int f(Ty (&Array)[N]) {
> +  return sizeof(Array) / sizeof(Ty); // Should not warn
> +}
> +
> +typedef int int32;
> +
> +void test(void) {
> +  int arr[12];// expected-note 2 {{array 'arr' declared 
> here}}
> +  unsigned long long arr2[4];
> +  int *p = &arr[0];
> +  int a1 = sizeof(arr) / sizeof(*arr);
> +  int a2 = sizeof arr / sizeof p; // expected-warning {{expression does not 
> compute the number of elements in this array; element type is 'int', not 'int 
> *'}}
> +  int a4 = sizeof arr2 / sizeof p;
> +  int a5 = sizeof(arr) / sizeof(short); // expected-warning {{expression 
> does not compute the number of elements 

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67420#1666141 , @Szelethus wrote:

> - For these select 4 options that suffer compatibility issues, manually check 
> `AnalyzerOptions::ConfigTable`.


*3 options. `ToolInvocation` is not really an option that you're ever supposed 
to adjust manually. Which is one more reason for me to think of this structure 
as of an implementation detail - an interface through which diagnostic 
consumers' behavior can be tweaked, regardless of why the tool wants it to be 
tweaked (because the human told it to or because the tool itself knows better).


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



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

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D67420#1666050 , @gribozavr wrote:

> Are you planning to move users of these options -- like `PlistDiagnostics` -- 
> to libAnalysis?


Yup, i was supposed to do that in D67422  :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



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

[PATCH] D67454: Start porting ivfsoverlay tests to Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: amccarth.
Herald added a subscriber: arphaman.
Herald added a project: clang.

Part of PR43272, the changes are:

1. Use @ as the sed pattern delimiter instead of : so that the drive

letter in lit substitutions isn't an issue.

2. Use the %/t and %/S substitutions to get paths with forward slashes

to work around string quoting issues in the yaml file.

3. Replace REQUIRES:shell with XFAIL:windows. These tests should pass on

Windows, but do not for reasons that are not yet understood. We would
like to know if they pass unexpectedly.

I was able to remove the XFAILs from two tests, since they already pass
with my sed fix:

  clang/test/VFS/module_missing_vfs.m
  clang/test/VFS/test_nonmodular.c


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67454

Files:
  clang/test/Index/index-module-with-vfs.m
  clang/test/Modules/crash-vfs-ivfsoverlay.m
  clang/test/Modules/double-quotes.m
  clang/test/Modules/framework-public-includes-private.m
  clang/test/VFS/external-names.c
  clang/test/VFS/framework-import.m
  clang/test/VFS/implicit-include.c
  clang/test/VFS/include-mixed-real-and-virtual.c
  clang/test/VFS/include-real-from-virtual.c
  clang/test/VFS/include-virtual-from-real.c
  clang/test/VFS/include.c
  clang/test/VFS/incomplete-umbrella.m
  clang/test/VFS/module-import.m
  clang/test/VFS/module_missing_vfs.m
  clang/test/VFS/real-path-found-first.m
  clang/test/VFS/relative-path.c
  clang/test/VFS/subframework-symlink.m
  clang/test/VFS/test_nonmodular.c
  clang/test/VFS/umbrella-framework-import-skipnonexist.m
  clang/test/VFS/vfsroot-include.c
  clang/test/VFS/vfsroot-module.m
  clang/test/VFS/vfsroot-with-overlay.c

Index: clang/test/VFS/vfsroot-with-overlay.c
===
--- clang/test/VFS/vfsroot-with-overlay.c
+++ clang/test/VFS/vfsroot-with-overlay.c
@@ -1,8 +1,10 @@
-// REQUIRES: shell
+// FIXME: PR43272
+// XFAIL: windows
+
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
-// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml
-// RUN: sed -e "s:INPUT_DIR:/indirect-vfs-root-files:g" -e "s:OUT_DIR:/overlay-dir:g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
+// RUN: sed -e "s@TEST_DIR@%/S@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsroot.yaml > %t.yaml
+// RUN: sed -e "s@INPUT_DIR@/indirect-vfs-root-files@g" -e "s@OUT_DIR@/overlay-dir@g" %S/Inputs/vfsoverlay.yaml > %t/vfsoverlay.yaml
 // RUN: %clang_cc1 -Werror -ivfsoverlay %t.yaml -ivfsoverlay /direct-vfs-root-files/vfsoverlay.yaml -I /overlay-dir -fsyntax-only /tests/vfsroot-with-overlay.c
 
 #include "not_real.h"
Index: clang/test/VFS/vfsroot-module.m
===
--- clang/test/VFS/vfsroot-module.m
+++ clang/test/VFS/vfsroot-module.m
@@ -1,7 +1,9 @@
-// REQUIRES: shell
+// FIXME: PR43272
+// XFAIL: windows
+
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
-// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml
+// RUN: sed -e "s@TEST_DIR@%/S@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsroot.yaml > %t.yaml
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t/cache -ivfsoverlay %t.yaml -F %S/Inputs -fsyntax-only /tests/vfsroot-module.m
 
 // Test that a file missing from the VFS root is not found, even if it is
Index: clang/test/VFS/vfsroot-include.c
===
--- clang/test/VFS/vfsroot-include.c
+++ clang/test/VFS/vfsroot-include.c
@@ -1,7 +1,9 @@
-// REQUIRES: shell
+// FIXME: PR43272
+// XFAIL: windows
+
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
-// RUN: sed -e "s:TEST_DIR:%S:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsroot.yaml > %t.yaml
+// RUN: sed -e "s@TEST_DIR@%/S@g" -e "s@OUT_DIR@%/t@g" %S/Inputs/vfsroot.yaml > %t.yaml
 // RUN: not %clang_cc1 -Werror -ivfsoverlay %t.yaml -I %S/Inputs -I /direct-vfs-root-files -fsyntax-only /tests/vfsroot-include.c 2>&1 | FileCheck %s
 // The line above tests that the compiler input file is looked up through VFS.
 
Index: clang/test/VFS/umbrella-framework-import-skipnonexist.m
===
--- clang/test/VFS/umbrella-framework-import-skipnonexist.m
+++ clang/test/VFS/umbrella-framework-import-skipnonexist.m
@@ -1,13 +1,13 @@
-// REQUIRES: crash-recovery, shell
+// REQUIRES: crash-recovery
 
-// FIXME: This XFAIL is cargo-culted from crash-report.c. Do we need it?
-// XFAIL: windows-gnu
+// FIXME: PR43272
+// XFAIL: windows
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/vdir %t/outdir %t/cache
 // RUN: cp -R %S/Inputs/Bar.framework %t/outdir/
 //
-// RUN: sed -e "s:VDIR:%t/vdir:g" -e "s:OUT_DIR:%t/outdir:g" %S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
+// RUN: sed -e "s@VDIR@%/t/vdir@g" -e "s@OUT_DIR@%/t/outdir@g" %S/Inputs/bar-headers.yaml > %t/vdir/bar-headers.yaml
 // RUN: rm -f %t/outdir/Bar.framework/Headers/B.h
 // RUN: %clang_cc1 -fmodules 

[PATCH] D65130: [clang][OpenMP] Add clang-offload-wrapper tool

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev updated this revision to Diff 219750.
sdmitriev added a comment.

Rebase


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

https://reviews.llvm.org/D65130

Files:
  clang/test/Driver/clang-offload-wrapper.c
  clang/tools/CMakeLists.txt
  clang/tools/clang-offload-wrapper/CMakeLists.txt
  clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp

Index: clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
===
--- /dev/null
+++ clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp
@@ -0,0 +1,360 @@
+//===-- clang-offload-wrapper/ClangOffloadWrapper.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// Implementation of the offload wrapper tool. It takes offload target binaries
+/// as input and creates wrapper bitcode from them which registers given
+/// binaries in offload runtime.
+///
+//===--===//
+
+#include "clang/Basic/Version.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
+#include 
+#include 
+
+using namespace llvm;
+
+static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden);
+
+// Mark all our options with this category, everything else (except for -version
+// and -help) will be hidden.
+static cl::OptionCategory
+ClangOffloadWrapperCategory("clang-offload-wrapper options");
+
+static cl::opt Output("o", cl::Required,
+   cl::desc("Output filename"),
+   cl::value_desc("filename"),
+   cl::cat(ClangOffloadWrapperCategory));
+
+static cl::list Inputs(cl::Positional, cl::OneOrMore,
+cl::desc(""),
+cl::cat(ClangOffloadWrapperCategory));
+
+static cl::opt Target("target", cl::Required,
+   cl::desc("Target triple"),
+   cl::value_desc("triple"),
+   cl::cat(ClangOffloadWrapperCategory));
+
+namespace {
+
+class BinaryWrapper {
+  LLVMContext C;
+  Module M;
+
+  StructType *EntryTy = nullptr;
+  StructType *ImageTy = nullptr;
+  StructType *DescTy = nullptr;
+
+private:
+  IntegerType *getSizeTTy() {
+switch (M.getDataLayout().getPointerTypeSize(Type::getInt8PtrTy(C))) {
+case 4u:
+  return Type::getInt32Ty(C);
+case 8u:
+  return Type::getInt64Ty(C);
+}
+llvm_unreachable("unsupported pointer type size");
+  }
+
+  // struct __tgt_offload_entry {
+  //   void *addr;
+  //   char *name;
+  //   size_t size;
+  //   int32_t flags;
+  //   int32_t reserved;
+  // };
+  StructType *getEntryTy() {
+if (!EntryTy)
+  EntryTy = StructType::create("__tgt_offload_entry", Type::getInt8PtrTy(C),
+   Type::getInt8PtrTy(C), getSizeTTy(),
+   Type::getInt32Ty(C), Type::getInt32Ty(C));
+return EntryTy;
+  }
+
+  PointerType *getEntryPtrTy() { return PointerType::getUnqual(getEntryTy()); }
+
+  // struct __tgt_device_image {
+  //   void *ImageStart;
+  //   void *ImageEnd;
+  //   __tgt_offload_entry *EntriesBegin;
+  //   __tgt_offload_entry *EntriesEnd;
+  // };
+  StructType *getDeviceImageTy() {
+if (!ImageTy)
+  ImageTy = StructType::create("__tgt_device_image", Type::getInt8PtrTy(C),
+   Type::getInt8PtrTy(C), getEntryPtrTy(),
+   getEntryPtrTy());
+return ImageTy;
+  }
+
+  PointerType *getDeviceImagePtrTy() {
+return PointerType::getUnqual(getDeviceImageTy());
+  }
+
+  // struct __tgt_bin_desc {
+  //   int32_t NumDeviceImages;
+  //   __tgt_device_image *DeviceImages;
+  //   __tgt_offload_entry *HostEntriesBegin;
+  //   __tgt_offload_entry *HostEntriesEnd;
+  // };
+  StructType *getBinDescTy() {
+if (!DescTy)
+  DescTy = StructType::create("__tgt_bin_desc", Type::getInt32Ty(C),
+  getDeviceImagePtrTy(), getEntryPtrTy(),
+  getEntryPtrTy(

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: aaron.ballman, rsmith, erichkeane.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

r369705 did not consider the addition of gnu_inline on function
declarations of alias attributed functions. This resulted in a reported
regression in the clang-9-rc4 release from the Zig developers building
glibc, which was observable as a failed assertion:

llvm-project/clang/lib/AST/Decl.cpp:3336: bool
clang::FunctionDecl::isInlineDefinitionExternallyVisible() const:
Assertion `(doesThisDeclarationHaveABody() || willHaveBody()) && "Must
be a function definition"' failed.

Alias function declarations do not have bodies, so allow us to proceed
if we have the alias function attribute but no body/definition, and add
a test case.  The emitted symbols and their linkage matches GCC for the
added test case.

Link: https://bugs.llvm.org/show_bug.cgi?id=43268


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67455

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/alias.c


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3348,8 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
- "Must be a function definition");
+  assert((doesThisDeclarationHaveABody() || willHaveBody()) ||
+ hasAttr() && "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();
 


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3348,8 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
- "Must be a function definition");
+  assert((doesThisDeclarationHaveABody() || willHaveBody()) ||
+ hasAttr() && "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66856#1650803 , @aaron.ballman 
wrote:

> To me, actual problems at runtime belong in -Wformat and logical 
> inconsistencies that don't cause runtime problems belong in 
> -Wformat-pedantic. However, I think it's a defect that the C standard has no 
> clearly UB-free way to print a _Bool value. I will bring this up on the 
> reflectors to ask if we can clarify the standard here.


The reflector discussion is still happening and there are issues with 
ambiguities that we are pretty sure we want to correct. I've got a paper out 
that touches on some of this: 
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2420.pdf




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8123
+def warn_format_bool_as_character : Warning<
+  "format specifies a character but argument has boolean value">,
+  InGroup;

How about: `using '%0' format specifier, but argument has boolean value` and 
then pass in the character specifier used?



Comment at: clang/test/Sema/format-bool.c:26
+#ifdef PEDANTIC
+  // expected-warning@-2 {{format specifies type 'short' but the argument has 
type}}
+#endif

Just an FYI (not related to your patch): it seems that at least some people 
think this should be diagnosed as something other than by `-Wformat-pedantic`. 
Their thinking is that `-Wformat-pedantic` is for things that are required to 
have a diagnostic according to the standard but are not sufficiently 
interesting to warn about by default. This particular case is not required to 
be warned on by the standard, so it's not really a "pedantic" warning. It 
sounds like there may be interest in having `-Wformat-pedantic` for that 
understanding of pedantic, and introduce something like 
`-Wformat-type-mismatch` for these other cases where there is type confusion 
but not sufficiently dangerous to warrant warning by default?



Comment at: clang/test/Sema/format-bool.c:37
+  p("%lc", b); // expected-warning {{format specifies a character but argument 
has boolean value}}
+  p("%c", 1 == 1); // expected-warning {{format specifies a character but 
argument has boolean value}}
+  p("%f", b); // expected-warning{{format specifies type 'double' but the 
argument has type}}

The diagnostic is both correct and incorrect. Maybe that's fine. However, `==` 
returns an `int` in C and `int` is reasonable to pass in to `%c`, so the 
diagnostic seems a bit strange when it claims the argument has a boolean value. 
Then again, the results really are boolean despite the type being an int.

I think I've convinced myself this is fine as-is. :-)



Comment at: clang/test/Sema/format-bool.c:43
+#ifdef __OBJC__
+  [NSString stringWithFormat: @"%c", 0]; // probably fine?
+  [NSString stringWithFormat: @"%c", NO]; // expected-warning {{format 
specifies a character but argument has boolean value}}

Seems correct to me.


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

https://reviews.llvm.org/D66856



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

[PATCH] D52524: Add -Wpoison-system-directories warning

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

Ping @aaron.ballman , please verify the change.


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

https://reviews.llvm.org/D52524



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

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

[Andrew Kelley via Phabricator via cfe-commits]

andrewrk requested changes to this revision.
andrewrk added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/AST/Decl.cpp:3352
+  assert((doesThisDeclarationHaveABody() || willHaveBody()) ||
+ hasAttr() && "Must be a function definition");
   assert(isInlined() && "Function must be inline");

It looks like the rparen is in the wrong place. I'd expect:

assert((doesThisDeclarationHaveABody() || willHaveBody() ||
 hasAttr()) && "Must be a function definition");


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67455



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

r371646 - [NFC] Added triple to test file to avoid arm buildbots failures

[David Bolvansky via cfe-commits]

Author: xbolva00
Date: Wed Sep 11 11:55:56 2019
New Revision: 371646

URL: http://llvm.org/viewvc/llvm-project?rev=371646&view=rev
Log:
[NFC] Added triple to test file to avoid arm buildbots failures

Modified:
cfe/trunk/test/Sema/div-sizeof-array.cpp

Modified: cfe/trunk/test/Sema/div-sizeof-array.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371646&r1=371645&r2=371646&view=diff
==
--- cfe/trunk/test/Sema/div-sizeof-array.cpp (original)
+++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 11:55:56 2019
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
+// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only 
-triple=x86_64-linux-gnu
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=x86_64-linux-gnu
 
 template 
 int f(Ty (&Array)[N]) {


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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, does anybody want me to write an example tool that emits path diagnostics 
but doesn't link to `libStaticAnalyzer*`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67422



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

Re: r371605 - [Diagnostics] Add -Wsizeof-array-div

[Dávid Bolvanský via cfe-commits]

Thanks,

Reproduced with -triple armv7–. Added triple to run line, hopefully it will fix 
the bots. I will check this buildbot if all ok.

rL371646

Dňa 11. 9. 2019 o 20:35 užívateľ Yvan Roux  napísal:

> Hi David,
> 
> This commit broken ARMv7 bots, logs are available here:
> 
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/10203/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adiv-sizeof-array.cpp
> 
> Thanks,
> Yvan
> 
> 
> 
> On Wed, 11 Sep 2019 at 12:58, David Bolvansky via cfe-commits
>  wrote:
>> 
>> Author: xbolva00
>> Date: Wed Sep 11 03:59:47 2019
>> New Revision: 371605
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=371605&view=rev
>> Log:
>> [Diagnostics] Add -Wsizeof-array-div
>> 
>> Summary: Clang version of https://www.viva64.com/en/examples/v706/
>> 
>> Reviewers: rsmith
>> 
>> Differential Revision: https://reviews.llvm.org/D67287
>> 
>> Added:
>>cfe/trunk/test/Sema/div-sizeof-array.cpp
>> Modified:
>>cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>cfe/trunk/lib/Sema/SemaExpr.cpp
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=371605&r1=371604&r2=371605&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 11 03:59:47 
>> 2019
>> @@ -3406,6 +3406,10 @@ def note_pointer_declared_here : Note<
>> def warn_division_sizeof_ptr : Warning<
>>   "'%0' will return the size of the pointer, not the array itself">,
>>   InGroup>;
>> +def warn_division_sizeof_array : Warning<
>> +  "expression does not compute the number of elements in this array; 
>> element "
>> +  "type is %0, not %1">,
>> +  InGroup>;
>> 
>> def note_function_warning_silence : Note<
>> "prefix with the address-of operator to silence this warning">;
>> 
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=371605&r1=371604&r2=371605&view=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Sep 11 03:59:47 2019
>> @@ -9158,17 +9158,28 @@ static void DiagnoseDivisionSizeofPointe
>>   else
>> RHSTy = RUE->getArgumentExpr()->IgnoreParens()->getType();
>> 
>> -  if (!LHSTy->isPointerType() || RHSTy->isPointerType())
>> -return;
>> -  if (LHSTy->getPointeeType().getCanonicalType().getUnqualifiedType() !=
>> -  RHSTy.getCanonicalType().getUnqualifiedType())
>> -return;
>> +  if (LHSTy->isPointerType() && !RHSTy->isPointerType()) {
>> +if (!S.Context.hasSameUnqualifiedType(LHSTy->getPointeeType(), RHSTy))
>> +  return;
>> 
>> -  S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
>> LHS->getSourceRange();
>> -  if (const auto *DRE = dyn_cast(LHSArg)) {
>> -if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>> -  S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
>> -  << LHSArgDecl;
>> +S.Diag(Loc, diag::warn_division_sizeof_ptr) << LHS << 
>> LHS->getSourceRange();
>> +if (const auto *DRE = dyn_cast(LHSArg)) {
>> +  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>> +S.Diag(LHSArgDecl->getLocation(), diag::note_pointer_declared_here)
>> +<< LHSArgDecl;
>> +}
>> +  } else if (const auto *ArrayTy = S.Context.getAsArrayType(LHSTy)) {
>> +QualType ArrayElemTy = ArrayTy->getElementType();
>> +if (ArrayElemTy->isDependentType() || RHSTy->isDependentType() ||
>> +S.Context.getTypeSize(ArrayElemTy) == S.Context.getTypeSize(RHSTy))
>> +  return;
>> +S.Diag(Loc, diag::warn_division_sizeof_array)
>> +<< LHSArg->getSourceRange() << ArrayElemTy << RHSTy;
>> +if (const auto *DRE = dyn_cast(LHSArg)) {
>> +  if (const ValueDecl *LHSArgDecl = DRE->getDecl())
>> +S.Diag(LHSArgDecl->getLocation(), diag::note_array_declared_here)
>> +<< LHSArgDecl;
>> +}
>>   }
>> }
>> 
>> 
>> Added: cfe/trunk/test/Sema/div-sizeof-array.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/div-sizeof-array.cpp?rev=371605&view=auto
>> ==
>> --- cfe/trunk/test/Sema/div-sizeof-array.cpp (added)
>> +++ cfe/trunk/test/Sema/div-sizeof-array.cpp Wed Sep 11 03:59:47 2019
>> @@ -0,0 +1,28 @@
>> +// RUN: %clang_cc1 %s -verify -Wsizeof-array-div -fsyntax-only
>> +
>> +template 
>> +int f(Ty (&Array)[N]) {
>> +  return sizeof(Array) / sizeof(Ty); // Should not warn
>> +}
>> +
>> +typedef int int32;
>> +
>> +void test(void) {
>> +  int arr[12];// expected-note 2 {{array 'arr' declared 
>> here}}
>> +  unsigned long long arr2[4];
>> +  int *p = &arr[0];
>> +  int a1 = sizeof(arr) 

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

I'm not sure copying the crtbegin/crtend mechanism from the early days of C 
runtime is ideal. Since the data is stored in a common section anyway, please 
could we rename it to __omp_offloading_entries in which case the linker will 
provide start/end symbols automatically? That removes the two object files and 
the link order dependency which is a hazard to bitcode libraries.


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

https://reviews.llvm.org/D64943



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

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Right, that sounds reasonable.  You should be making them link up exactly like 
a normal method implementation, so if SemaObjC doesn't consider normal method 
implementations to be redeclarations, then I guess these aren't either.  And if 
we want to change that in the future, then we'll change it for these, too.


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

https://reviews.llvm.org/D66121



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

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 219756.
Quuxplusone added a comment.
Herald added a subscriber: dexonsmith.

Rebased on master.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,64 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main(int, char**)
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+
+  return 0;
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,53 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.h"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+std::ex

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 219758.
Quuxplusone added a comment.

Rebased on master.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47358

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsynchronized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/ctor_does_not_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/sync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/unsync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_options.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_upstream_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_options.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
  test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
  test/support/count_new.h

Index: test/support/count_new.h
===
--- test/support/count_new.h
+++ test/support/count_new.h
@@ -210,6 +210,11 @@
 return disable_checking || n != delete_called;
 }
 
+bool checkDeleteCalledGreaterThan(int n) const
+{
+return disable_checking || delete_called > n;
+}
+
 bool checkAlignedNewCalledEq(int n) const
 {
 return disable_checking || n == aligned_new_called;
Index: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
@@ -0,0 +1,28 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: apple-clang-7
+
+// 
+
+// struct pool_options
+
+#include 
+#include 
+
+int main(int, char**)
+{
+const std::experimental::pmr::pool_options p;
+assert(p.max_blocks_per_chunk == 0);
+assert(p.largest_required_pool_block == 0);
+
+return 0;
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
@@ -0,0 +1,28 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class unsynchronized_pool_resource
+
+#include 
+#include 
+
+#include "count_new.h"
+
+int main(int, char**)
+{
+std::experimental::pmr::unsynchronized_pool_resource unsync;
+LIBCPP_ASSERT_NOEXCEPT(unsync.upstream_resource());
+
+return 0;
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental

[PATCH] D67429: Improve code generation for thread_local variables:

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67429



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

> OpenMP linker script is known to cause problems for gold and lld linkers on 
> Linux and it will also cause problems for Windows enabling in future

What are the known problems with the linker script? I'm wondering if they can 
be resolved without the overhead of introducing a new tool.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D64943#158 , @JonChesterfield 
wrote:

> > OpenMP linker script is known to cause problems for gold and lld linkers on 
> > Linux and it will also cause problems for Windows enabling in future
>
> What are the known problems with the linker script? I'm wondering if they can 
> be resolved without the overhead of introducing a new tool.


They just do not support linker script. And, thus, cannot be used for 
offloading. Only `ld` supports it.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67382: [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp:302-303
   // Find the node's current statement in the CFG.
-  if (const Stmt *S = PathDiagnosticLocation::getStmt(this))
+  // FIXME: getStmtForDiagnostics() does nasty things in order to provide
+  // a valid statement for body farms, do we need this behavior here?
+  if (const Stmt *S = getStmtForDiagnostics())

Szelethus wrote:
> But still fails...
Well, not everything is a statement.



Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:565-566
 
   // FIXME: Ironically, this assert actually fails in some cases.
   //assert(L.isValid());
   return L;

Szelethus wrote:
> I guess didn't change much :^)
Indeed :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D67382



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

[PATCH] D52524: Add -Wpoison-system-directories warning

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from a minor nit, this LGTM. However, I'm not the most familiar with how 
cross-compiling works in the first place, so I may be the wrong one to approve 
this.




Comment at: clang/lib/Frontend/InitHeaderSearch.cpp:141-143
+  if (HasSysroot) {
+if (MappedPathStr.startswith("/usr/include") ||
+MappedPathStr.startswith("/usr/local/include")) {

These should be combined into a single if statement:
```
if (HasSysroot && (MappedPathStr.startswith(...) || 
MappedPathStr.startswith(...))) {
```


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

https://reviews.llvm.org/D52524



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D64943#173 , @ABataev wrote:

> In D64943#158 , @JonChesterfield 
> wrote:
>
> > > OpenMP linker script is known to cause problems for gold and lld linkers 
> > > on Linux and it will also cause problems for Windows enabling in future
> >
> > What are the known problems with the linker script? I'm wondering if they 
> > can be resolved without the overhead of introducing a new tool.
>
>
> They just do not support linker script. And, thus, cannot be used for 
> offloading. Only `ld` supports it.


In what respect? I've used linker scripts with both gold and lld, and both 
instances of --help text claim to support them. In the case of lld, a very 
complicated script hit a few internal errors, but I believe they've all been 
fixed since.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D64943#178 , @JonChesterfield 
wrote:

> In D64943#173 , @ABataev wrote:
>
> > In D64943#158 , 
> > @JonChesterfield wrote:
> >
> > > > OpenMP linker script is known to cause problems for gold and lld 
> > > > linkers on Linux and it will also cause problems for Windows enabling 
> > > > in future
> > >
> > > What are the known problems with the linker script? I'm wondering if they 
> > > can be resolved without the overhead of introducing a new tool.
> >
> >
> > They just do not support linker script. And, thus, cannot be used for 
> > offloading. Only `ld` supports it.
>
>
> In what respect? I've used linker scripts with both gold and lld, and both 
> instances of --help text claim to support them. In the case of lld, a very 
> complicated script hit a few internal errors, but I believe they've all been 
> fixed since.


Hmm, I tried it with gold some time ago and it just did not work for me. The 
linking failed with diagnostics that some of the commands in the script are 
unknown.


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

https://reviews.llvm.org/D64943



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

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D64943#179 , @ABataev wrote:

> In D64943#178 , @JonChesterfield 
> wrote:
>
> > In D64943#173 , @ABataev wrote:
> >
> > > In D64943#158 , 
> > > @JonChesterfield wrote:
> > >
> > > > > OpenMP linker script is known to cause problems for gold and lld 
> > > > > linkers on Linux and it will also cause problems for Windows enabling 
> > > > > in future
> > > >
> > > > What are the known problems with the linker script? I'm wondering if 
> > > > they can be resolved without the overhead of introducing a new tool.
> > >
> > >
> > > They just do not support linker script. And, thus, cannot be used for 
> > > offloading. Only `ld` supports it.
> >
> >
> > In what respect? I've used linker scripts with both gold and lld, and both 
> > instances of --help text claim to support them. In the case of lld, a very 
> > complicated script hit a few internal errors, but I believe they've all 
> > been fixed since.
>
>
> Hmm, I tried it with gold some time ago and it just did not work for me. The 
> linking failed with diagnostics that some of the commands in the script are 
> unknown.


The problem turns out to be the 'insert before' statement. ld and lld support 
it, gold does not. According to 
https://bugzilla.redhat.com/show_bug.cgi?id=927573, the recommended workaround 
is essentially that implemented in this differential.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: alexfh, alexfh_.
poelmanc added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

clang-tidy's modernize-use-using feature is great! But if it finds any commas 
that are not within parentheses, it won't create a fix. That means it won't 
change lines like:
`  typedef std::pair Point;`
to
`  using Point = std::pair;`
or even:
`  typedef std::map MyMap;`
`  typedef std::vector> MyVector;`

This patch allows the fix to apply to lines with commas if they are within 
parentheses //or// angle brackets.

One test is include


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D67460

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/modernize-use-using.cpp


Index: clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
@@ -182,4 +182,11 @@
 class E : public C {
   void f() override { super::f(); }
 };
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
 }
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -40,6 +40,7 @@
 
   Token Tok;
   int ParenLevel = 0;
+  int AngleBracketLevel = 0;
   bool FoundTypedef = false;
 
   while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) {
@@ -54,10 +55,16 @@
 case tok::r_paren:
   ParenLevel--;
   break;
+case tok::less: // '<', start template
+  AngleBracketLevel++;
+  break;
+case tok::greater: // '>', end template
+  AngleBracketLevel--;
+  break;
 case tok::comma:
-  if (ParenLevel == 0) {
-// If there is comma and we are not between open parenthesis then it is
-// two or more declarations in this chain.
+  if (ParenLevel == 0 && AngleBracketLevel == 0) {
+// If there is comma and we are not between open parenthesis or between
+// open angle brackets then it is two or more declarations in this 
chain.
 return false;
   }
   break;
@@ -88,8 +95,7 @@
   if (StartLoc.isMacroID() && IgnoreMacros)
 return;
 
-  auto Diag =
-  diag(StartLoc, "use 'using' instead of 'typedef'");
+  auto Diag = diag(StartLoc, "use 'using' instead of 'typedef'");
 
   // do not fix if there is macro or array
   if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID())


Index: clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/modernize-use-using.cpp
@@ -182,4 +182,11 @@
 class E : public C {
   void f() override { super::f(); }
 };
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
 }
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -40,6 +40,7 @@
 
   Token Tok;
   int ParenLevel = 0;
+  int AngleBracketLevel = 0;
   bool FoundTypedef = false;
 
   while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) {
@@ -54,10 +55,16 @@
 case tok::r_paren:
   ParenLevel--;
   break;
+case tok::less: // '<', start template
+  AngleBracketLevel++;
+  break;
+case tok::greater: // '>', end template
+  AngleBracketLevel--;
+  break;
 case tok::comma:
-  if (ParenLevel == 0) {
-// If there is comma and we are not between open parenthesis then it is
-// two or more declarations in this chain.
+  if (ParenLevel == 0 && AngleBracketLevel == 0) {
+// If there is comma and we are not between open parenthesis or between
+// open angle brackets then it is two or more declarations in this chain.
 return false;
   }
   break;
@@ -88,8 +95,7 @@
   if (StartLoc.isMacroID() && IgnoreMacros)
 return;
 
-  auto Diag =
-  diag(StartLoc, "use 'using' instead of 'typedef'");
+  auto Diag = diag(StartLoc, "use 'using' instead of 'typedef'");
 
   // do not fix if there is macro or array
   if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID())
___
cfe-commits mailing list
cfe-commits@lists.llvm.

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

In D64943#193 , @JonChesterfield 
wrote:

> In D64943#179 , @ABataev wrote:
>
> > In D64943#178 , 
> > @JonChesterfield wrote:
> >
> > > In D64943#173 , @ABataev 
> > > wrote:
> > >
> > > > In D64943#158 , 
> > > > @JonChesterfield wrote:
> > > >
> > > > > > OpenMP linker script is known to cause problems for gold and lld 
> > > > > > linkers on Linux and it will also cause problems for Windows 
> > > > > > enabling in future
> > > > >
> > > > > What are the known problems with the linker script? I'm wondering if 
> > > > > they can be resolved without the overhead of introducing a new tool.
> > > >
> > > >
> > > > They just do not support linker script. And, thus, cannot be used for 
> > > > offloading. Only `ld` supports it.
> > >
> > >
> > > In what respect? I've used linker scripts with both gold and lld, and 
> > > both instances of --help text claim to support them. In the case of lld, 
> > > a very complicated script hit a few internal errors, but I believe 
> > > they've all been fixed since.
> >
> >
> > Hmm, I tried it with gold some time ago and it just did not work for me. 
> > The linking failed with diagnostics that some of the commands in the script 
> > are unknown.
>
>
> The problem turns out to be the 'insert before' statement. ld and lld support 
> it, gold does not. According to 
> https://bugzilla.redhat.com/show_bug.cgi?id=927573, the recommended 
> workaround is essentially that implemented in this differential. See also 
> https://sourceware.org/bugzilla/show_bug.cgi?id=15373.


A small example that I presented on the OpenMP multi company meeting earlier:

  bash-4.2$ cat foo.c
  #include 
  
  int main() {
int X = 0;
  
  #pragma omp target map(tofrom: X)
X += 3;
  
printf("X = %d\n", X);
return 0;
  }
  
  bash-4.2$ clang -fopenmp -fopenmp-targets=x86_64-pc-linux-gnu -fuse-ld=gold 
foo.c
  /usr/bin/ld.gold: error: /tmp/a-c699cd.lk:25:8: syntax error, unexpected 
STRING
  /usr/bin/ld.gold: fatal error: unable to parse script file /tmp/a-c699cd.lk
  clang-10: error: linker command failed with exit code 1 (use -v to see 
invocation)
  bash-4.2$ clang -fopenmp -fopenmp-targets=x86_64-pc-linux-gnu -fuse-ld=lld 
foo.c
  ld.lld: error: unable to INSERT AFTER/BEFORE .data: section not defined
  clang-10: error: linker command failed with exit code 1 (use -v to see 
invocation)
  bash-4.2$ 

Also OpenMP linker script will obviously cause problems on Windows once we 
start enabling offload on Windows.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: thakis, hans.
Herald added a project: clang.

C++ does not allow shadowing template parameters, but previously we
allowed it under -fms-extensions. Now this behavior is controlled by
-fms-compatibility, and we emit a -Wmicrosoft-template warning when it
happens.

Fixes PR43265


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67463

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Parser/DelayedTemplateParsing.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp


Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- clang/test/SemaCXX/MicrosoftCompatibility.cpp
+++ clang/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -366,3 +366,21 @@
 int S::fn() { return 0; } // expected-warning {{is missing exception 
specification}}
 }
 
+namespace PR43265 {
+template  // expected-note {{template parameter is declared here}}
+struct Foo {
+  static const int N = 42; // expected-warning {{declaration of 'N' shadows 
template parameter}}
+};
+}
+
+namespace Inner_Outer_same_template_param_name {
+template  // expected-note {{template parameter is declared here}}
+struct Outmost {
+  template  // expected-warning {{declaration of 'T' shadows 
template parameter}}
+  struct Inner {
+void f() {
+  T *var;
+}
+  };
+};
+}
Index: clang/test/Parser/DelayedTemplateParsing.cpp
===
--- clang/test/Parser/DelayedTemplateParsing.cpp
+++ clang/test/Parser/DelayedTemplateParsing.cpp
@@ -48,22 +48,6 @@
 
   
 
-namespace Inner_Outer_same_template_param_name {  
-
-template 
-class Outmost {
-public:
-template 
-class Inner {
-public:
-void f() {
-T* var;
-}
-   };
-};
-
-}
-
 
 namespace PR11931 {
 
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -849,15 +849,14 @@
 void Sema::DiagnoseTemplateParameterShadow(SourceLocation Loc, Decl *PrevDecl) 
{
   assert(PrevDecl->isTemplateParameter() && "Not a template parameter");
 
-  // Microsoft Visual C++ permits template parameters to be shadowed.
-  if (getLangOpts().MicrosoftExt)
-return;
-
   // C++ [temp.local]p4:
   //   A template-parameter shall not be redeclared within its
   //   scope (including nested scopes).
-  Diag(Loc, diag::err_template_param_shadow)
-<< cast(PrevDecl)->getDeclName();
+  //
+  // Make this a warning when MSVC compatibility is requested.
+  unsigned DiagId = getLangOpts().MSVCCompat ? diag::ext_template_param_shadow
+ : diag::err_template_param_shadow;
+  Diag(Loc, DiagId) << cast(PrevDecl)->getDeclName();
   Diag(PrevDecl->getLocation(), diag::note_template_param_here);
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4000,6 +4000,8 @@
 // C++ Template Declarations
 def err_template_param_shadow : Error<
   "declaration of %0 shadows template parameter">;
+def ext_template_param_shadow : ExtWarn<
+  err_template_param_shadow.Text>, InGroup;
 def note_template_param_here : Note<"template parameter is declared here">;
 def warn_template_export_unsupported : Warning<
   "exported templates are unsupported">;


Index: clang/test/SemaCXX/MicrosoftCompatibility.cpp
===
--- clang/test/SemaCXX/MicrosoftCompatibility.cpp
+++ clang/test/SemaCXX/MicrosoftCompatibility.cpp
@@ -366,3 +366,21 @@
 int S::fn() { return 0; } // expected-warning {{is missing exception specification}}
 }
 
+namespace PR43265 {
+template  // expected-note {{template parameter is declared here}}
+struct Foo {
+  static const int N = 42; // expected-warning {{declaration of 'N' shadows template parameter}}
+};
+}
+
+namespace Inner_Outer_same_template_param_name {
+template  // expected-note {{template parameter is declared here}}
+struct Outmost {
+  template  // expected-warning {{declaration of 'T' shadows template parameter}}
+  struct Inner {
+void f() {
+  T *var;
+}
+  };
+};
+}
Index: clang/test/Parser/DelayedTemplateParsing.cpp
===
--- clang/test/Parser/DelayedTemplateParsing.cpp
+++ clang/test/Parser/DelayedTemplateParsing.cpp
@@ -48,22 +48,6 @@
 
   
 
-namespace Inner_Outer_same_template_param_name {  
-
-template 
-class Outmost {
-public:
-template 
-class Inner {
-public:
-void f() {
-T* var;
-}
-   };
-};
-
-}
-
 
 namespace PR11931 {
 
Index: clang/lib/Sema/SemaTemplate.cpp
=

[PATCH] D67454: Start porting ivfsoverlay tests to Windows

[Adrian McCarthy via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67454



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

RE: [PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Narayanaswamy, Ravi via cfe-commits]

Microsoft linker does not support linker script. 
We need to solution that works with all linkers.   The begin/end is supported 
by most linker of interest.

-Original Message-
From: Jon Chesterfield via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Wednesday, September 11, 2019 12:21 PM
To: Dmitriev, Serguei N ; hfin...@anl.gov; 
a.bat...@hotmail.com; Narayanaswamy, Ravi ; 
jdoerf...@anl.gov; gheorghe-teod.ber...@ibm.com; Rokos, Georgios 

Cc: lebedev...@gmail.com; jonathanchesterfi...@gmail.com; Zakharin, Vyacheslav 
P ; sando...@cray.com; 
openmp-comm...@lists.llvm.org; cfe-commits@lists.llvm.org; mgo...@gentoo.org; 
zhang.guans...@gmail.com; mgrang.1...@gmail.com; t...@google.com; 
ztur...@roblox.com; peter.wal...@arm.com
Subject: [PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker 
script

JonChesterfield added a comment.

In D64943#173 , @ABataev wrote:

> In D64943#158 , @JonChesterfield 
> wrote:
>
> > > OpenMP linker script is known to cause problems for gold and lld linkers 
> > > on Linux and it will also cause problems for Windows enabling in future
> >
> > What are the known problems with the linker script? I'm wondering if they 
> > can be resolved without the overhead of introducing a new tool.
>
>
> They just do not support linker script. And, thus, cannot be used for 
> offloading. Only `ld` supports it.


In what respect? I've used linker scripts with both gold and lld, and both 
instances of --help text claim to support them. In the case of lld, a very 
complicated script hit a few internal errors, but I believe they've all been 
fixed since.


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

https://reviews.llvm.org/D64943



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

[PATCH] D67425: [WebAssembly] Narrowing and widening SIMD ops

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 219784.
tlively added a comment.

- Make narrows binary ops


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67425

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -463,4 +463,40 @@
 # CHECK: f64x2.convert_i64x2_u # encoding: [0xfd,0xb2,0x01]
 f64x2.convert_i64x2_u
 
+# CHECK: i8x16.narrow_i16x8_s # encoding: [0xfd,0xc6,0x01]
+i8x16.narrow_i16x8_s
+
+# CHECK: i8x16.narrow_i16x8_u # encoding: [0xfd,0xc7,0x01]
+i8x16.narrow_i16x8_u
+
+# CHECK: i16x8.narrow_i32x4_s # encoding: [0xfd,0xc8,0x01]
+i16x8.narrow_i32x4_s
+
+# CHECK: i16x8.narrow_i32x4_u # encoding: [0xfd,0xc9,0x01]
+i16x8.narrow_i32x4_u
+
+# CHECK: i16x8.widen_low_i8x16_s # encoding: [0xfd,0xca,0x01]
+i16x8.widen_low_i8x16_s
+
+# CHECK: i16x8.widen_high_i8x16_s # encoding: [0xfd,0xcb,0x01]
+i16x8.widen_high_i8x16_s
+
+# CHECK: i16x8.widen_low_i8x16_u # encoding: [0xfd,0xcc,0x01]
+i16x8.widen_low_i8x16_u
+
+# CHECK: i16x8.widen_high_i8x16_u # encoding: [0xfd,0xcd,0x01]
+i16x8.widen_high_i8x16_u
+
+# CHECK: i32x4.widen_low_i16x8_s # encoding: [0xfd,0xce,0x01]
+i32x4.widen_low_i16x8_s
+
+# CHECK: i32x4.widen_high_i16x8_s # encoding: [0xfd,0xcf,0x01]
+i32x4.widen_high_i16x8_s
+
+# CHECK: i32x4.widen_low_i16x8_u # encoding: [0xfd,0xd0,0x01]
+i32x4.widen_low_i16x8_u
+
+# CHECK: i32x4.widen_high_i16x8_u # encoding: [0xfd,0xd1,0x01]
+i32x4.widen_high_i16x8_u
+
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -87,6 +87,30 @@
   ret <16 x i8> %a
 }
 
+; CHECK-LABEL: narrow_signed_v16i8:
+; SIMD128-NEXT: .functype narrow_signed_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.narrow_i16x8_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.narrow.signed.v16i8.v8i16(<8 x i16>, <8 x i16>)
+define <16 x i8> @narrow_signed_v16i8(<8 x i16> %low, <8 x i16> %high) {
+  %a = call <16 x i8> @llvm.wasm.narrow.signed.v16i8.v8i16(
+<8 x i16> %low, <8 x i16> %high
+  )
+  ret <16 x i8> %a
+}
+
+; CHECK-LABEL: narrow_unsigned_v16i8:
+; SIMD128-NEXT: .functype narrow_unsigned_v16i8 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i8x16.narrow_i16x8_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <16 x i8> @llvm.wasm.narrow.unsigned.v16i8.v8i16(<8 x i16>, <8 x i16>)
+define <16 x i8> @narrow_unsigned_v16i8(<8 x i16> %low, <8 x i16> %high) {
+  %a = call <16 x i8> @llvm.wasm.narrow.unsigned.v16i8.v8i16(
+<8 x i16> %low, <8 x i16> %high
+  )
+  ret <16 x i8> %a
+}
+
 ; ==
 ; 8 x i16
 ; ==
@@ -166,6 +190,70 @@
   ret <8 x i16> %a
 }
 
+; CHECK-LABEL: narrow_signed_v8i16:
+; SIMD128-NEXT: .functype narrow_signed_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.narrow_i32x4_s $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.narrow.signed.v8i16.v4i32(<4 x i32>, <4 x i32>)
+define <8 x i16> @narrow_signed_v8i16(<4 x i32> %low, <4 x i32> %high) {
+  %a = call <8 x i16> @llvm.wasm.narrow.signed.v8i16.v4i32(
+<4 x i32> %low, <4 x i32> %high
+  )
+  ret <8 x i16> %a
+}
+
+; CHECK-LABEL: narrow_unsigned_v8i16:
+; SIMD128-NEXT: .functype narrow_unsigned_v8i16 (v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.narrow_i32x4_u $push[[R:[0-9]+]]=, $0, $1{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.narrow.unsigned.v8i16.v4i32(<4 x i32>, <4 x i32>)
+define <8 x i16> @narrow_unsigned_v8i16(<4 x i32> %low, <4 x i32> %high) {
+  %a = call <8 x i16> @llvm.wasm.narrow.unsigned.v8i16.v4i32(
+<4 x i32> %low, <4 x i32> %high
+  )
+  ret <8 x i16> %a
+}
+
+; CHECK-LABEL: widen_low_signed_v8i16:
+; SIMD128-NEXT: .functype widen_low_signed_v8i16 (v128) -> (v128){{$}}
+; SIMD128-NEXT: i16x8.widen_low_i8x16_s $push[[R:[0-9]+]]=, $0{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <8 x i16> @llvm.wasm.widen.low.signed.v8i16.v16i8(<16 x i8>)
+define <8 x i16> @widen_low_signed_v8i16(<16 x i8> %v) {
+  %a = call <8 x i16> @llvm.wasm.widen.low.signed.v8i16.v16i8(<

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

In D64943#136 , @JonChesterfield 
wrote:

> I'm not sure copying the crtbegin/crtend mechanism from the early days of C 
> runtime is ideal. Since the data is stored in a common section anyway, please 
> could we rename it to __omp_offloading_entries in which case the linker will 
> provide start/end symbols automatically?


Well, I never said that it is an ideal solution, but it is a known mechanism 
that works well in many cases and can also be reused for the offloading entry 
table.
I do not fully understand your suggestion for renaming entries section, how it 
will help with providing start/end symbols for the entries. Can you please 
provide more details?


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

https://reviews.llvm.org/D64943



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

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai marked 2 inline comments as done.
vsapsai added a comment.

Thanks for the review.




Comment at: clang/lib/Lex/HeaderSearch.cpp:892-902
+IsInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(IsInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
 }
+if (IsMapped)

dexonsmith wrote:
> I wonder if this would be easier to follow if you refactored like this:
> 
> ```
> if (!MappedName.empty()) {
>   // other logic.
>   if (IsMapped)
> *IsMapped = true;
> } else if (IsInHeaderMap && File) {
>   if (IsMapped)
> *IsMapped = true;
> }
> ```
> 
> but maybe my aesthetics are being thrown off by all the intervening comments 
> in Phab.  I'll leave it up to you.
I've tried the suggested approach but don't find it better. It is a personal 
preference but I like how `*IsMapped |= ...` conveys the value is an 
"aggregate" of the previous file lookups.


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

https://reviews.llvm.org/D58094



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

r371655 - Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Wed Sep 11 13:39:04 2019
New Revision: 371655

URL: http://llvm.org/viewvc/llvm-project?rev=371655&view=rev
Log:
Fix -Wnonportable-include-path suppression for header maps with absolute paths.

In `DirectoryLookup::LookupFile` parameter `HasBeenMapped` doesn't cover
the case when clang finds a file through a header map but doesn't remap
the lookup filename because the target path is an absolute path. As a
result, -Wnonportable-include-path suppression for header maps
introduced in r301592 wasn't triggered.

Change parameter `HasBeenMapped` to `IsInHeaderMap` and use parameter
`MappedName` to track the filename remapping. This way we can handle
both relative and absolute paths in header maps, and account for their
specific properties, like filename remapping being a property preserved
across lookups in multiple directories.

rdar://problem/39516483

Reviewers: dexonsmith, bruno

Reviewed By: dexonsmith

Subscribers: jkorous, cfe-commits, ributzka

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

Added:
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
Modified:
cfe/trunk/include/clang/Lex/DirectoryLookup.h
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c

Modified: cfe/trunk/include/clang/Lex/DirectoryLookup.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DirectoryLookup.h?rev=371655&r1=371654&r2=371655&view=diff
==
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h (original)
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h Wed Sep 11 13:39:04 2019
@@ -183,7 +183,7 @@ public:
  SmallVectorImpl *RelativePath, Module *RequestingModule,
  ModuleMap::KnownHeader *SuggestedModule,
  bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
- bool &HasBeenMapped, SmallVectorImpl &MappedName) const;
+ bool &IsInHeaderMap, SmallVectorImpl &MappedName) const;
 
 private:
   Optional DoFrameworkLookup(

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=371655&r1=371654&r2=371655&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Wed Sep 11 13:39:04 2019
@@ -341,9 +341,10 @@ Optional DirectoryLookup::
 SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath,
 Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
 bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
-bool &HasBeenMapped, SmallVectorImpl &MappedName) const {
+bool &IsInHeaderMap, SmallVectorImpl &MappedName) const {
   InUserSpecifiedSystemFramework = false;
-  HasBeenMapped = false;
+  IsInHeaderMap = false;
+  MappedName.clear();
 
   SmallString<1024> TmpDir;
   if (isNormalDir()) {
@@ -377,6 +378,8 @@ Optional DirectoryLookup::
   if (Dest.empty())
 return None;
 
+  IsInHeaderMap = true;
+
   auto FixupSearchPath = [&]() {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
@@ -393,10 +396,8 @@ Optional DirectoryLookup::
   // ("Foo.h" -> "Foo/Foo.h"), in which case continue header lookup using the
   // framework include.
   if (llvm::sys::path::is_relative(Dest)) {
-MappedName.clear();
 MappedName.append(Dest.begin(), Dest.end());
 Filename = StringRef(MappedName.begin(), MappedName.size());
-HasBeenMapped = true;
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
@@ -883,18 +884,22 @@ Optional HeaderSearch::Loo
   // Check each directory in sequence to see if it contains this file.
   for (; i != SearchDirs.size(); ++i) {
 bool InUserSpecifiedSystemFramework = false;
-bool HasBeenMapped = false;
+bool IsInHeaderMap = false;
 bool IsFrameworkFoundInDir = false;
 Optional File = SearchDirs[i].LookupFile(
 Filename, *this, IncludeLoc, SearchPath, RelativePath, 
RequestingModule,
 SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
-HasBeenMapped, MappedName);
-if (HasBeenMapped) {
+IsInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(IsInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
 }
+if (IsMapped)
+  // A filename is mapped when a header map remapped it to a relative path
+  // used in subsequent header search or to an absolute path pointing to an
+  // existin

r371656 - [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Sep 11 13:40:31 2019
New Revision: 371656

URL: http://llvm.org/viewvc/llvm-project?rev=371656&view=rev
Log:
[clang-scan-deps] add skip excluded conditional preprocessor block 
preprocessing optimization

This commit adds an optimization to clang-scan-deps and clang's preprocessor 
that skips excluded preprocessor
blocks by bumping the lexer pointer, and not lexing the tokens until reaching 
appropriate #else/#endif directive.
The skip positions and lexer offsets are computed when the file is minimized, 
directly from the minimized tokens.

On an 18-core iMacPro with macOS Catalina Beta I got 10-15% speed-up from this 
optimization when running clang-scan-deps on
the compilation database for a recent LLVM and Clang (3511 files).

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

Added:

cfe/trunk/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
Modified:
cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
cfe/trunk/include/clang/Lex/Lexer.h
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/include/clang/Lex/PreprocessorOptions.h

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Modified: cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h?rev=371656&r1=371655&r2=371656&view=diff
==
--- cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h (original)
+++ cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h Wed Sep 
11 13:40:31 2019
@@ -66,6 +66,24 @@ struct Token {
   Token(TokenKind K, int Offset) : K(K), Offset(Offset) {}
 };
 
+/// Simplified token range to track the range of a potentially skippable PP
+/// directive.
+struct SkippedRange {
+  /// Offset into the output byte stream of where the skipped directive begins.
+  int Offset;
+
+  /// The number of bytes that can be skipped before the preprocessing must
+  /// resume.
+  int Length;
+};
+
+/// Computes the potential source ranges that can be skipped by the 
preprocessor
+/// when skipping a directive like #if, #ifdef or #elsif.
+///
+/// \returns false on success, true on error.
+bool computeSkippedRanges(ArrayRef Input,
+  llvm::SmallVectorImpl &Range);
+
 } // end namespace minimize_source_to_dependency_directives
 
 /// Minimize the input down to the preprocessor directives that might have

Modified: cfe/trunk/include/clang/Lex/Lexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=371656&r1=371655&r2=371656&view=diff
==
--- cfe/trunk/include/clang/Lex/Lexer.h (original)
+++ cfe/trunk/include/clang/Lex/Lexer.h Wed Sep 11 13:40:31 2019
@@ -265,6 +265,21 @@ public:
   /// Return the current location in the buffer.
   const char *getBufferLocation() const { return BufferPtr; }
 
+  /// Returns the current lexing offset.
+  unsigned getCurrentBufferOffset() {
+assert(BufferPtr >= BufferStart && "Invalid buffer state");
+return BufferPtr - BufferStart;
+  }
+
+  /// Skip over \p NumBytes bytes.
+  ///
+  /// If the skip is successful, the next token will be lexed from the new
+  /// offset. The lexer also assumes that we skipped to the start of the line.
+  ///
+  /// \returns true if the skip failed (new offset would have been past the
+  /// end of the buffer), false otherwise.
+  bool skipOver(unsigned NumBytes);
+
   /// Stringify - Convert the specified string into a C string by i) escaping
   /// '\\' and " characters and ii) replacing newline character(s) with "\\n".
   /// If Charify is true, this escapes the ' character instead of ".

Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=371656&r1=371655&r2=371656&view=diff
==
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Wed Sep 11 13:40:31 2019
@@ -28,6 

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371655: Fix -Wnonportable-include-path suppression for 
header maps with absolute paths. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58094?vs=219194&id=219788#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58094

Files:
  cfe/trunk/include/clang/Lex/DirectoryLookup.h
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
  cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
  cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Baz.h
  cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c

Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -341,9 +341,10 @@
 SmallVectorImpl *SearchPath, SmallVectorImpl *RelativePath,
 Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
 bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
-bool &HasBeenMapped, SmallVectorImpl &MappedName) const {
+bool &IsInHeaderMap, SmallVectorImpl &MappedName) const {
   InUserSpecifiedSystemFramework = false;
-  HasBeenMapped = false;
+  IsInHeaderMap = false;
+  MappedName.clear();
 
   SmallString<1024> TmpDir;
   if (isNormalDir()) {
@@ -377,6 +378,8 @@
   if (Dest.empty())
 return None;
 
+  IsInHeaderMap = true;
+
   auto FixupSearchPath = [&]() {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
@@ -393,10 +396,8 @@
   // ("Foo.h" -> "Foo/Foo.h"), in which case continue header lookup using the
   // framework include.
   if (llvm::sys::path::is_relative(Dest)) {
-MappedName.clear();
 MappedName.append(Dest.begin(), Dest.end());
 Filename = StringRef(MappedName.begin(), MappedName.size());
-HasBeenMapped = true;
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
@@ -883,18 +884,22 @@
   // Check each directory in sequence to see if it contains this file.
   for (; i != SearchDirs.size(); ++i) {
 bool InUserSpecifiedSystemFramework = false;
-bool HasBeenMapped = false;
+bool IsInHeaderMap = false;
 bool IsFrameworkFoundInDir = false;
 Optional File = SearchDirs[i].LookupFile(
 Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
 SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
-HasBeenMapped, MappedName);
-if (HasBeenMapped) {
+IsInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(IsInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
 }
+if (IsMapped)
+  // A filename is mapped when a header map remapped it to a relative path
+  // used in subsequent header search or to an absolute path pointing to an
+  // existing file.
+  *IsMapped |= (!MappedName.empty() || (IsInHeaderMap && File));
 if (IsFrameworkFound)
   // Because we keep a filename remapped for subsequent search directory
   // lookups, ignore IsFrameworkFoundInDir after the first remapping and not
Index: cfe/trunk/include/clang/Lex/DirectoryLookup.h
===
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h
@@ -183,7 +183,7 @@
  SmallVectorImpl *RelativePath, Module *RequestingModule,
  ModuleMap::KnownHeader *SuggestedModule,
  bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
- bool &HasBeenMapped, SmallVectorImpl &MappedName) const;
+ bool &IsInHeaderMap, SmallVectorImpl &MappedName) const;
 
 private:
   Optional DoFrameworkLookup(
Index: cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
===
--- cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
+++ cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
@@ -1,6 +1,9 @@
 {
   "mappings" :
 {
- "Foo/Foo.h" : "headers/foo/Foo.h"
+ "Foo/Foo.h" : "headers/foo/Foo.h",
+ "Bar.h" : "headers/foo/Bar.h",
+ "Foo/Bar.h" : "INPUTS_DIR/nonportable-hmaps/headers/foo/Bar.h",
+ "headers/Foo/Baz.h" : "/not/existing/absolute/path/Baz.h"
 }
 }
Index: cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
===
--- cfe/trunk/test/Preprocessor/nonporta

[PATCH] D67127: [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371656: [clang-scan-deps] add skip excluded conditional 
preprocessor block… (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67127?vs=218821&id=219789#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67127

Files:
  cfe/trunk/include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  cfe/trunk/include/clang/Lex/Lexer.h
  cfe/trunk/include/clang/Lex/Preprocessor.h
  
cfe/trunk/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h
  cfe/trunk/include/clang/Lex/PreprocessorOptions.h
  
cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  cfe/trunk/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/lib/Lex/PPDirectives.cpp
  cfe/trunk/lib/Lex/Preprocessor.cpp
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
  cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
  cfe/trunk/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: cfe/trunk/lib/Lex/Preprocessor.cpp
===
--- cfe/trunk/lib/Lex/Preprocessor.cpp
+++ cfe/trunk/lib/Lex/Preprocessor.cpp
@@ -158,6 +158,11 @@
 
   if (this->PPOpts->GeneratePreamble)
 PreambleConditionalStack.startRecording();
+
+  ExcludedConditionalDirectiveSkipMappings =
+  this->PPOpts->ExcludedConditionalDirectiveSkipMappings;
+  if (ExcludedConditionalDirectiveSkipMappings)
+ExcludedConditionalDirectiveSkipMappings->clear();
 }
 
 Preprocessor::~Preprocessor() {
Index: cfe/trunk/lib/Lex/Lexer.cpp
===
--- cfe/trunk/lib/Lex/Lexer.cpp
+++ cfe/trunk/lib/Lex/Lexer.cpp
@@ -218,6 +218,15 @@
   return L;
 }
 
+bool Lexer::skipOver(unsigned NumBytes) {
+  IsAtPhysicalStartOfLine = true;
+  IsAtStartOfLine = true;
+  if ((BufferPtr + NumBytes) > BufferEnd)
+return true;
+  BufferPtr += NumBytes;
+  return false;
+}
+
 template  static void StringifyImpl(T &Str, char Quote) {
   typename T::size_type i = 0, e = Str.size();
   while (i < e) {
Index: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -865,6 +865,54 @@
   return Error;
 }
 
+bool clang::minimize_source_to_dependency_directives::computeSkippedRanges(
+ArrayRef Input, llvm::SmallVectorImpl &Range) {
+  struct Directive {
+enum DirectiveKind {
+  If,  // if/ifdef/ifndef
+  Else // elif,else
+};
+int Offset;
+DirectiveKind Kind;
+  };
+  llvm::SmallVector Offsets;
+  for (const Token &T : Input) {
+switch (T.K) {
+case pp_if:
+case pp_ifdef:
+case pp_ifndef:
+  Offsets.push_back({T.Offset, Directive::If});
+  break;
+
+case pp_elif:
+case pp_else: {
+  if (Offsets.empty())
+return true;
+  int PreviousOffset = Offsets.back().Offset;
+  Range.push_back({PreviousOffset, T.Offset - PreviousOffset});
+  Offsets.push_back({T.Offset, Directive::Else});
+  break;
+}
+
+case pp_endif: {
+  if (Offsets.empty())
+return true;
+  int PreviousOffset = Offsets.back().Offset;
+  Range.push_back({PreviousOffset, T.Offset - PreviousOffset});
+  do {
+Directive::DirectiveKind Kind = Offsets.pop_back_val().Kind;
+if (Kind == Directive::If)
+  break;
+  } while (!Offsets.empty());
+  break;
+}
+default:
+  break;
+}
+  }
+  return false;
+}
+
 bool clang::minimizeSourceToDependencyDirectives(
 StringRef Input, SmallVectorImpl &Output,
 SmallVectorImpl &Tokens, DiagnosticsEngine *Diags,
Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -370,6 +370,37 @@
   return DiscardUntilEndOfDirective().getEnd();
 }
 
+Optional Preprocessor::getSkippedRangeForExcludedConditionalBlock(
+SourceLocation HashLoc) {
+  if (!ExcludedConditionalDirectiveSkipMappings)
+return None;
+  if (!HashLoc.isFileID())
+return None;
+
+  std::pair HashFileOffset =
+  SourceMgr.getDecomposedLoc(HashLoc);
+  const llvm::MemoryBuffer *Buf = SourceMgr.getBuffer(HashFileOffset.first);

[PATCH] D67455: [Clang][CodeGen] support alias attribute w/ gnu_inline

[Nick Desaulniers via Phabricator via cfe-commits]

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

- adjust parens


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67455

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGen/alias.c


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3348,7 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
+  assert((doesThisDeclarationHaveABody() || willHaveBody() ||
+  hasAttr()) &&
  "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();


Index: clang/test/CodeGen/alias.c
===
--- clang/test/CodeGen/alias.c
+++ clang/test/CodeGen/alias.c
@@ -99,3 +99,8 @@
 // CHECKGLOBALS-NOT: @test11_foo = dso_local
 void test11(void) {}
 static void test11_foo(void) __attribute__((alias("test11")));
+
+// Test that gnu_inline+alias work.
+// CHECKGLOBALS: @test12_alias = alias void (), void ()* @test12
+void test12(void) {}
+inline void test12_alias(void) __attribute__((gnu_inline, alias("test12")));
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3348,7 +3348,8 @@
 /// an externally visible symbol, but "extern inline" will not create an
 /// externally visible symbol.
 bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
-  assert((doesThisDeclarationHaveABody() || willHaveBody()) &&
+  assert((doesThisDeclarationHaveABody() || willHaveBody() ||
+  hasAttr()) &&
  "Must be a function definition");
   assert(isInlined() && "Function must be inline");
   ASTContext &Context = getASTContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371660 - [analyzer] NFC: Move resetDiagnosticLocationToMainFile() to BugReporter.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Sep 11 13:54:24 2019
New Revision: 371660

URL: http://llvm.org/viewvc/llvm-project?rev=371660&view=rev
Log:
[analyzer] NFC: Move resetDiagnosticLocationToMainFile() to BugReporter.

This method of PathDiagnostic is a part of Static Analyzer's particular
path diagnostic construction scheme. As such, it doesn't belong to
the PathDiagnostic class, but to the Analyzer.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=371660&r1=371659&r2=371660&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Wed Sep 11 13:54:24 2019
@@ -792,11 +792,6 @@ public:
 VerboseDesc += S;
   }
 
-  /// If the last piece of the report point to the header file, resets
-  /// the location of the report to be the last location in the main source
-  /// file.
-  void resetDiagnosticLocationToMainFile();
-
   StringRef getVerboseDescription() const { return VerboseDesc; }
 
   StringRef getShortDescription() const {
@@ -807,11 +802,6 @@ public:
   StringRef getBugType() const { return BugType; }
   StringRef getCategory() const { return Category; }
 
-  /// Return the semantic context where an issue occurred.  If the
-  /// issue occurs along a path, this represents the "central" area
-  /// where the bug manifests.
-  const Decl *getDeclWithIssue() const { return DeclWithIssue; }
-
   using meta_iterator = std::deque::const_iterator;
 
   meta_iterator meta_begin() const { return OtherDesc.begin(); }
@@ -826,10 +816,23 @@ public:
 return *ExecutedLines;
   }
 
+  /// Return the semantic context where an issue occurred.  If the
+  /// issue occurs along a path, this represents the "central" area
+  /// where the bug manifests.
+  const Decl *getDeclWithIssue() const { return DeclWithIssue; }
+
+  void setDeclWithIssue(const Decl *D) {
+DeclWithIssue = D;
+  }
+
   PathDiagnosticLocation getLocation() const {
 return Loc;
   }
 
+  void setLocation(PathDiagnosticLocation NewLoc) {
+Loc = NewLoc;
+  }
+
   /// Get the location on which the report should be uniqued.
   PathDiagnosticLocation getUniqueingLoc() const {
 return UniqueingLoc;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=371660&r1=371659&r2=371660&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Wed Sep 11 13:54:24 2019
@@ -3125,6 +3125,71 @@ BugReporter::generateDiagnosticForConsum
   return Out;
 }
 
+static PathDiagnosticCallPiece *
+getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
+const SourceManager &SMgr) {
+  SourceLocation CallLoc = CP->callEnter.asLocation();
+
+  // If the call is within a macro, don't do anything (for now).
+  if (CallLoc.isMacroID())
+return nullptr;
+
+  assert(AnalysisManager::isInCodeFile(CallLoc, SMgr) &&
+ "The call piece should not be in a header file.");
+
+  // Check if CP represents a path through a function outside of the main file.
+  if (!AnalysisManager::isInCodeFile(CP->callEnterWithin.asLocation(), SMgr))
+return CP;
+
+  const PathPieces &Path = CP->path;
+  if (Path.empty())
+return nullptr;
+
+  // Check if the last piece in the callee path is a call to a function outside
+  // of the main file.
+  if (auto *CPInner = dyn_cast(Path.back().get()))
+return getFirstStackedCallToHeaderFile(CPInner, SMgr);
+
+  // Otherwise, the last piece is in the main file.
+  return nullptr;
+}
+
+static void resetDiagnosticLocationToMainFile(PathDiagnostic &PD) {
+  if (PD.path.empty())
+return;
+
+  PathDiagnosticPiece *LastP = PD.path.back().get();
+  assert(LastP);
+  const SourceManager &SMgr = LastP->getLocation().getManager();
+
+  // We only need to check if the report ends inside headers, if the last piece
+  // is a call piece.
+  if (auto *CP = dyn_cast(LastP)) {
+CP = getFirstStackedCallToHeaderFile(CP, SMgr);
+if (CP) {
+  // Mark the piece.
+   CP->setAsLastInMainSourceFile();
+
+  // Update the path diagnostic message.
+  const auto *ND = dyn_cast(CP->getCallee());
+  if (ND) {
+SmallString<200> buf;
+llvm::raw_svector_ostream os(buf);
+os << " (within a call to '" << ND->getDeclName() << "')";

r371658 - [analyzer] NFC: Re-implement stack hints as a side map in BugReport.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Sep 11 13:54:17 2019
New Revision: 371658

URL: http://llvm.org/viewvc/llvm-project?rev=371658&view=rev
Log:
[analyzer] NFC: Re-implement stack hints as a side map in BugReport.

That's one of the few random entities in the PathDiagnostic interface that
are specific to the Static Analyzer. By moving them out we could let
everybody use path diagnostics without linking against Static Analyzer.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=371658&r1=371657&r2=371658&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Wed 
Sep 11 13:54:17 2019
@@ -70,6 +70,50 @@ class SValBuilder;
 using DiagnosticForConsumerMapTy =
 llvm::DenseMap>;
 
+/// Interface for classes constructing Stack hints.
+///
+/// If a PathDiagnosticEvent occurs in a different frame than the final
+/// diagnostic the hints can be used to summarize the effect of the call.
+class StackHintGenerator {
+public:
+  virtual ~StackHintGenerator() = 0;
+
+  /// Construct the Diagnostic message for the given ExplodedNode.
+  virtual std::string getMessage(const ExplodedNode *N) = 0;
+};
+
+/// Constructs a Stack hint for the given symbol.
+///
+/// The class knows how to construct the stack hint message based on
+/// traversing the CallExpr associated with the call and checking if the given
+/// symbol is returned or is one of the arguments.
+/// The hint can be customized by redefining 'getMessageForX()' methods.
+class StackHintGeneratorForSymbol : public StackHintGenerator {
+private:
+  SymbolRef Sym;
+  std::string Msg;
+
+public:
+  StackHintGeneratorForSymbol(SymbolRef S, StringRef M) : Sym(S), Msg(M) {}
+  ~StackHintGeneratorForSymbol() override = default;
+
+  /// Search the call expression for the symbol Sym and dispatch the
+  /// 'getMessageForX()' methods to construct a specific message.
+  std::string getMessage(const ExplodedNode *N) override;
+
+  /// Produces the message of the following form:
+  ///   'Msg via Nth parameter'
+  virtual std::string getMessageForArg(const Expr *ArgE, unsigned ArgIndex);
+
+  virtual std::string getMessageForReturn(const CallExpr *CallExpr) {
+return Msg;
+  }
+
+  virtual std::string getMessageForSymbolNotFound() {
+return Msg;
+  }
+};
+
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
 class BugReport {
@@ -313,6 +357,14 @@ protected:
 
   const Stmt *getStmt() const;
 
+  /// If an event occurs in a different frame than the final diagnostic,
+  /// supply a message that will be used to construct an extra hint on the
+  /// returns from all the calls on the stack from this event to the final
+  /// diagnostic.
+  // FIXME: Allow shared_ptr keys in DenseMap?
+  std::map>
+  StackHints;
+
 public:
   PathSensitiveBugReport(const BugType &bt, StringRef desc,
  const ExplodedNode *errorNode)
@@ -455,6 +507,26 @@ public:
   bool addTrackedCondition(const ExplodedNode *Cond) {
 return TrackedConditions.insert(Cond).second;
   }
+
+  void addCallStackHint(PathDiagnosticPieceRef Piece,
+std::unique_ptr StackHint) {
+StackHints[Piece] = std::move(StackHint);
+  }
+
+  bool hasCallStackHint(PathDiagnosticPieceRef Piece) const {
+return StackHints.count(Piece) > 0;
+  }
+
+  /// Produce the hint for the given node. The node contains
+  /// information about the call for which the diagnostic can be generated.
+  std::string
+  getCallStackMessage(PathDiagnosticPieceRef Piece,
+  const ExplodedNode *N) const {
+auto I = StackHints.find(Piece);
+if (I != StackHints.end())
+  return I->second->getMessage(N);
+return "";
+  }
 };
 
 
//===--===//

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAn

r371659 - [analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Sep 11 13:54:21 2019
New Revision: 371659

URL: http://llvm.org/viewvc/llvm-project?rev=371659&view=rev
Log:
[analyzer] NFC: Move getStmt() and createEndOfPath() out of PathDiagnostic.

These static functions deal with ExplodedNodes which is something we don't want
the PathDiagnostic interface to know anything about, as it's planned to be
moved out of libStaticAnalyzerCore.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
cfe/trunk/lib/StaticAnalyzer/Checkers/Taint.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=371659&r1=371658&r2=371659&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Wed Sep 11 13:54:21 2019
@@ -52,11 +52,6 @@ class SourceManager;
 
 namespace ento {
 
-class ExplodedNode;
-class SymExpr;
-
-using SymbolRef = const SymExpr *;
-
 
//===--===//
 // High-level interface for handlers of path-sensitive diagnostics.
 
//===--===//
@@ -276,18 +271,21 @@ public:
   static PathDiagnosticLocation createDeclEnd(const LocationContext *LC,
const SourceManager &SM);
 
-  /// Create a location corresponding to the given valid ExplodedNode.
+  /// Create a location corresponding to the given valid ProgramPoint.
   static PathDiagnosticLocation create(const ProgramPoint &P,
const SourceManager &SMng);
 
-  /// Create a location corresponding to the next valid ExplodedNode as end
-  /// of path location.
-  static PathDiagnosticLocation createEndOfPath(const ExplodedNode* N);
-
   /// Convert the given location into a single kind location.
   static PathDiagnosticLocation createSingleLocation(
  const PathDiagnosticLocation 
&PDL);
 
+  /// Construct a source location that corresponds to either the beginning
+  /// or the end of the given statement, or a nearby valid source location
+  /// if the statement does not have a valid source location of its own.
+  static SourceLocation
+  getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC,
+ bool UseEndOfStatement = false);
+
   bool operator==(const PathDiagnosticLocation &X) const {
 return K == X.K && Loc == X.Loc && Range == X.Range;
   }
@@ -332,13 +330,6 @@ public:
   void Profile(llvm::FoldingSetNodeID &ID) const;
 
   void dump() const;
-
-  /// Given an exploded node, retrieve the statement that should be used
-  /// for the diagnostic location.
-  static const Stmt *getStmt(const ExplodedNode *N);
-
-  /// Retrieve the statement corresponding to the successor node.
-  static const Stmt *getNextStmt(const ExplodedNode *N);
 };
 
 class PathDiagnosticLocationPair {

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=371659&r1=371658&r2=371659&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
Wed Sep 11 13:54:21 2019
@@ -267,6 +267,30 @@ public:
   /// Trivial nodes may be skipped while prin

[clang-tools-extra] r371661 - [analyzer] NFC: Move PathDiagnostic classes to libAnalysis.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Sep 11 13:54:27 2019
New Revision: 371661

URL: http://llvm.org/viewvc/llvm-project?rev=371661&view=rev
Log:
[analyzer] NFC: Move PathDiagnostic classes to libAnalysis.

At this point the PathDiagnostic, PathDiagnosticLocation, PathDiagnosticPiece
structures no longer rely on anything specific to Static Analyzer, so we can
move them out of it for everybody to use.

PathDiagnosticConsumers are still to be handed off.

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

Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=371661&r1=371660&r2=371661&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Wed Sep 11 13:54:27 2019
@@ -36,10 +36,6 @@
 #include "clang/Rewrite/Frontend/FixItRewriter.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/Tooling/Core/Diagnostic.h"
-#if CLANG_ENABLE_STATIC_ANALYZER
-#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
-#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
-#endif // CLANG_ENABLE_STATIC_ANALYZER
 #include "clang/Tooling/DiagnosticsYaml.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/ReplacementsYaml.h"
@@ -49,6 +45,11 @@
 #include 
 #include 
 
+#if CLANG_ENABLE_STATIC_ANALYZER
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#endif // CLANG_ENABLE_STATIC_ANALYZER
+
 using namespace clang::ast_matchers;
 using namespace clang::driver;
 using namespace clang::tooling;


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

[PATCH] D67381: [analyzer] NFC: Move stack hints to a side map.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 2 inline comments as done.
NoQ added a comment.

In D67381#1665930 , @Szelethus wrote:

> Side note, now that you had to work with the freshly rewritten file, do you 
> have any feedback on it?


Dunno, i'm a very functional / pure / stateless person. "I debugged, I edited, 
I forgot ". "I suffer from 
short-term memory loss 
".

But given that it was fairly easy for me to focus on the task at hand and this 
whole thing was definitely not traumatizing, i suspect that the file ended up 
in a fairly good shape and i can't complain.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67381



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

[PATCH] D67419: [analyzer] NFC: Move PathDiagnostic to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 2 inline comments as done.
NoQ added a comment.

In D67419#1665993 , @Szelethus wrote:

> Looks great! Are we sure that `PathDiagnostic.h` is a good header name?


Not really, but i haven't come up with a better name yet (see also the summary 
of D67422 ).

P.S. Sry, i seem to have had a spam streak today.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67419



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