[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo added a comment.

P.S: Please ignore the child revision ... accidentally committed it to the 
wrong branch. I've fixed the git branches locally but can't figure out how to 
tell phabricator ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80603



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

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 266410.
yaxunl added a comment.

Changed target id format to be like `gfx908:xnack+:sramecc-`.

I tried to introduce --offload-target-id but found that is not good because: 1. 
it will cause redundant code since I have to handle these options separately in 
CUDA and HIP action builder; 2. it causes unnecessary complexity since I have 
to handle interaction between `--offload-arch` and `--offload-target-id`, 
especially the special case of `all`; 3. `--offload-target-id` is really the 
same thing as `--offload-arch`. Therefore I kept using `--offload-arch`. For 
CUDA this is NFC, since it is not checked as target id.


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

https://reviews.llvm.org/D60620

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/OffloadArch.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OffloadArch.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/invalid-target-id.hip
  clang/test/Driver/target-id-macros.cl
  clang/test/Driver/target-id-macros.hip
  clang/test/Driver/target-id.cl
  clang/test/Driver/target-id.hip

Index: clang/test/Driver/target-id.hip
===
--- /dev/null
+++ clang/test/Driver/target-id.hip
@@ -0,0 +1,51 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --offload-arch=gfx908 \
+// RUN:   --offload-arch=gfx908:xnack+:sramecc+ \
+// RUN:   --offload-arch=gfx908:xnack+:sramecc- \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908" "-fcuda-is-device"
+
+// CHECK: [[OPT:".*opt"]] {{.*}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906_BC:".*-gfx908-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_906_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908"
+// CHECK-SAME: {{.*}} "-target-feature" "+xnack"
+// CHECK-SAME: {{.*}} "-target-feature" "+sram-ecc"
+
+// CHECK: [[OPT]] {{.*"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906XE_BC:".*-gfx908:xnack\+:sramecc\+.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_906XE_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908"
+// CHECK-SAME: {{.*}} "-target-feature" "+xnack"
+// CHECK-SAME: {{.*}} "-target-feature" "-sram-ecc"
+
+// CHECK: [[OPT]] {{.*}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906XN_BC:".*-gfx908:xnack\+:sramecc\-.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_906XN_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+
+
+// CHECK: {{".*clang-offload-bundler"}}
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa-gfx908,hip-amdgcn-amd-amdhsa-gfx908:xnack+:sramecc+,hip-amdgcn-amd-amdhsa-gfx908:xnack+:sramecc-"
Index: clang/test/Driver/target-id.cl
===
--- /dev/null
+++ clang/test/Driver/target-id.cl
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx908:xnack+:sramecc- \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+
+// RUN: %clang -### -target amdgcn-amd-amdpal \
+// RUN:   -mcpu=gfx908:xnack+:sramecc- \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+
+// RUN: %clang -### -target amdgcn--mesa3d \
+// RUN:   -mcpu=gfx908:xnack+:sramecc- \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+// CHECK: "-target-cpu" "gfx908"
+// CHECK-SAME: "-target-feature" "+xnack"
+// CHECK-SAME: "-target-feature" "-sram-ecc"
Index: clang/test/Driver/target-id-macros.hip
===
--- /dev/null
+++ clang/test/Driver/target-id-macros.hip
@@ -0,0 +1,12 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -E -dM -target x86_64-linux-gnu --cuda-device-only \
+// RUN:   --offload-arch=gfx908:xnack+:sramecc- -nogpulib -o - %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-DAG: #define __amdgcn_processor__ "gfx908"
+// CHECK-DAG: #define 

Re: [clang] c90e198 - Fix parsing of enum-base to follow C++11 rules.

[Akira Hatanaka via cfe-commits]

> On May 20, 2020, at 5:53 PM, Richard Smith  wrote:
> 
> On Wed, 20 May 2020 at 16:30, Akira Hatanaka via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Hi Richard,
> 
> It looks like this patch will reject the following code, which used to 
> compile fine:
> 
> $ cat test.cpp
> #include 
> 
> typedef CF_ENUM(unsigned, TestEnum) {
>   A = 2,
>   B = 3,
> };
> 
> $ clang++ -std=c++11 -c test.cpp
> 
> test.cpp:3:9: error: non-defining declaration of enumeration with a fixed 
> underlying type is only permitted as a standalone declaration; missing list 
> of enumerators? [-Welaborated-enum-base]
> typedef CF_ENUM(unsigned, TestEnum) {
> 
> The macro is defined in CFAvailability.h:
> 
> https://opensource.apple.com/source/CF/CF-855.17/CFAvailability.h.auto.html 
> 
> 
> What’s the best way to fix this?
> 
> Assuming this macro is always preceded by 'typedef', how about this:
> 
> -#define CF_ENUM(_type, _name) enum _name : _type _name; enum _name : _type
> +#define CF_ENUM(_type, _name) int _dummy_##_name; enum _name : _type _name; 
> typedef enum _name _name; enum _name : _type
> 
> Or this:
> 
> +#ifdef __cplusplus
>  #define CF_ENUM(_type, _name) int _dummy_##_name; enum _name : _type
> +#else
>  #define CF_ENUM(_type, _name) enum _name : _type _name; enum _name : _type
> +#endif
> 
> (One wonders why the 'typedef' is not part of the macro definition.)

Thanks! Is there a way to avoid the dummy typedef so that it doesn’t show up in 
completions and stuff or some way in C++ to undef the typedef? 

> 
>> On May 11, 2020, at 1:37 PM, Richard Smith via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> 
>> On Mon, 11 May 2020 at 06:37, Hans Wennborg via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> On Sat, May 9, 2020 at 4:32 AM Richard Smith via cfe-commits
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> >
>> >
>> > Author: Richard Smith
>> > Date: 2020-05-08T19:32:00-07:00
>> > New Revision: c90e198107431f64b73686bdce31c293e3380ac7
>> >
>> > URL: 
>> > https://github.com/llvm/llvm-project/commit/c90e198107431f64b73686bdce31c293e3380ac7
>> >  
>> > 
>> > DIFF: 
>> > https://github.com/llvm/llvm-project/commit/c90e198107431f64b73686bdce31c293e3380ac7.diff
>> >  
>> > 
>> >
>> > LOG: Fix parsing of enum-base to follow C++11 rules.
>> >
>> > Previously we implemented non-standard disambiguation rules to
>> > distinguish an enum-base from a bit-field but otherwise treated a :
>> > after an elaborated-enum-specifier as introducing an enum-base. That
>> > misparses various examples (anywhere an elaborated-type-specifier can
>> > appear followed by a colon, such as within a ternary operator or
>> > _Generic).
>> >
>> > We now implement the C++11 rules, with the old cases accepted as
>> > extensions where that seemed reasonable. These amount to:
>> >  * an enum-base must always be accompanied by an enum definition (except
>> >in a standalone declaration of the form 'enum E : T;')
>> >  * in a member-declaration, 'enum E :' always introduces an enum-base,
>> >never a bit-field
>> >  * in a type-specifier (or similar context), 'enum E :' is not
>> >permitted; the colon means whatever else it would mean in that
>> >context.
>> >
>> > Fixed underlying types for enums are also permitted in Objective-C and
>> > under MS extensions, plus as a language extension in all other modes.
>> > The behavior in ObjC and MS extensions modes is unchanged (but the
>> > bit-field disambiguation is a bit better); remaining language modes
>> > follow the C++11 rules.
>> >
>> > Fixes PR45726, PR39979, PR19810, PR44941, and most of PR24297, plus C++
>> > core issues 1514 and 1966.
>> 
>> Hello from Chromium :-)
>> 
>> We saw new errors from some code in a header that looked like this:
>> 
>>   // Adapted from NSPathUtilities.h and NSObjCRuntime.h.
>>   typedef enum NSSearchPathDirectory : unsigned long NSSearchPathDirectory;
>> 
>> For us we think the enum itself is enough, so we'll fix it by dropping
>> the typedef, but this raised the question of how your change affects
>> the Mac system headers. IIUC your change makes an exception for Obj-C,
>> but the headers can be used from regular C/C++ too. Do you think there
>> might be issues there?
>> 
>> The errors are DefaultError ExtWarns, so they will be suppressed by default 
>> in system headers. Even then:
>>  * In Objective-C (and Objective-C++), the prior rule is unchanged.
>>  * In (non-Objective) C++11 onwards, we now enforce the standard rules. 
>> (System headers should ideally be valid code, but if not, the system header 
>> exclusion will kick in. And the errors can be disabled by warning flag in 
>> user code written against old Clang.)
>>  * In any other language mode, 

[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: hokein, gribozavr.
Herald added a project: clang.

DO NOT PUSH. This patch includes two new tests that demo a bug introduced into
Transformer by https://reviews.llvm.org/D72534. This patch is intended only as a
demonstration of the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80606

Files:
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,54 @@
   testRule(Rule, Input, Expected);
 }
 
+// Demonstrates bug in applyFirst caused by
+// https://reviews.llvm.org/D72534. This passes before and fails after.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(castExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// BAD FIX: This attempt by the API client to fix the problem fails.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnoredBadFix) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(
+  traverse(clang::TK_AsIs, castExpr(hasSourceExpression(callExpr(,
+  changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,54 @@
   testRule(Rule, Input, Expected);
 }
 
+// Demonstrates bug in applyFirst caused by
+// https://reviews.llvm.org/D72534. This passes before and fails after.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(castExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// BAD FIX: This attempt by the API client to fix the problem fails.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnoredBadFix) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(
+  traverse(clang::TK_AsIs, castExpr(hasSourceExpression(callExpr(,
+  changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo created this revision.
Herald added subscribers: cfe-commits, danielkiss, kristof.beyls.
Herald added a project: clang.
oontvoo added reviewers: gribozavr, ymandel.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80603

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2643,6 +2643,13 @@
   parmVarDecl(hasDefaultArgument(;
 }
 
+TEST(IsAtPosition, Basic) {
+  EXPECT_TRUE(matches("void x(int a) {}", parmVarDecl(isAtPosition(0;
+  EXPECT_TRUE(matches("void x(int a, int b) {}", 
parmVarDecl(isAtPosition(1;
+  EXPECT_TRUE(matches("void x(int a, int b) {}", 
parmVarDecl(isAtPosition(1;
+  EXPECT_TRUE(notMatches("void x(int val) {}", parmVarDecl(isAtPosition(1;
+}
+
 TEST(IsArray, Basic) {
   EXPECT_TRUE(matches("struct MyClass {}; MyClass *p1 = new MyClass[10];",
   cxxNewExpr(isArray(;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7001,6 +7001,26 @@
   return InnerMatcher.matches(Node, Finder, Builder);
 }
 
+/// Matches the ParmVarDecl nodes that are at the N'th position in the 
parameter
+/// list.
+///
+/// Given
+///
+/// \code
+/// void f(int a, int b, int c) {
+/// }
+/// \endcode
+///
+/// ``parmVarDecl(isAtPosition(0))``` matches `int a`.
+///
+/// ``parmVarDecl(isAtPosition(1))`` matches `int b`.
+AST_MATCHER_P(clang::ParmVarDecl, isAtPosition, unsigned, N) {
+  return parmVarDecl(hasAncestor(functionDecl(
+ hasParameter(N, parmVarDecl().bind("this_decl",
+ equalsBoundNode("this_decl"))
+  .matches(Node, Finder, Builder);
+}
+
 
////
 // OpenMP handling.
 
////


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2643,6 +2643,13 @@
   parmVarDecl(hasDefaultArgument(;
 }
 
+TEST(IsAtPosition, Basic) {
+  EXPECT_TRUE(matches("void x(int a) {}", parmVarDecl(isAtPosition(0;
+  EXPECT_TRUE(matches("void x(int a, int b) {}", parmVarDecl(isAtPosition(1;
+  EXPECT_TRUE(matches("void x(int a, int b) {}", parmVarDecl(isAtPosition(1;
+  EXPECT_TRUE(notMatches("void x(int val) {}", parmVarDecl(isAtPosition(1;
+}
+
 TEST(IsArray, Basic) {
   EXPECT_TRUE(matches("struct MyClass {}; MyClass *p1 = new MyClass[10];",
   cxxNewExpr(isArray(;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -7001,6 +7001,26 @@
   return InnerMatcher.matches(Node, Finder, Builder);
 }
 
+/// Matches the ParmVarDecl nodes that are at the N'th position in the parameter
+/// list.
+///
+/// Given
+///
+/// \code
+/// void f(int a, int b, int c) {
+/// }
+/// \endcode
+///
+/// ``parmVarDecl(isAtPosition(0))``` matches `int a`.
+///
+/// ``parmVarDecl(isAtPosition(1))`` matches `int b`.
+AST_MATCHER_P(clang::ParmVarDecl, isAtPosition, unsigned, N) {
+  return parmVarDecl(hasAncestor(functionDecl(
+ hasParameter(N, parmVarDecl().bind("this_decl",
+ equalsBoundNode("this_decl"))
+  .matches(Node, Finder, Builder);
+}
+
 ////
 // OpenMP handling.
 ////
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D80369#2056094 , @aprantl wrote:

> > How does this data get used for Swift code and ObjC interoperability? At 
> > the moment I see no use of this IR metadata in LLVM. Does ObjC/Swift 
> > interop use the DI IR over in the Swift compiler? Got a link to the code 
> > there?
>
> Are you saying the retained types are not emitted in DWARF?


Sort of. I'm saying /these/ types (the ones that these tests are testing for) 
aren't retained types. The declaration subprograms are in the retained types 
list - but when retained types are emitted, only types in the retained types 
list are used, the subprograms (and the types they only indirectly reference) 
are ignored.

Ah, nope, I'm wrong - I /thought/ the test was correctly flagging these types 
as missing - that the only reason the types were there was because their 
subprogram was in the retained types list.

That's incorrect - the types are here, they're just reordered because. At least 
for the C++ test, this change makes it pass:

  diff --git clang/test/Modules/ModuleDebugInfo.cpp 
clang/test/Modules/ModuleDebugInfo.cpp
  index 26369c89605..b1ffe27ec22 100644
  --- clang/test/Modules/ModuleDebugInfo.cpp
  +++ clang/test/Modules/ModuleDebugInfo.cpp
  @@ -51,15 +51,6 @@
   // CHECK-SAME: )
   // CHECK: !DIEnumerator(name: "e5", value: 5, isUnsigned: true)
   
  -// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
  -// no mangled name here yet.
  -
  -// This type is anchored by a function parameter.
  -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
  -// CHECK-SAME: elements:
  -// CHECK-SAME: templateParams:
  -// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
  -
   // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
   // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
   
  @@ -85,6 +76,12 @@
   // CHECK-SAME: templateParams:
   // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIlNS_6traitsIl")
   
  +// This type is anchored by a function parameter.
  +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A"
  +// CHECK-SAME: elements:
  +// CHECK-SAME: templateParams:
  +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
  +
   // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstantiation"
   // no mangled name here yet.
   
  @@ -93,6 +90,9 @@
   // CHECK-SAME: flags: DIFlagFwdDecl
   // CHECK-SAME: identifier: 
"_ZTSN8DebugCXX8TemplateIfNS_6traitsIf")
   
  +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
  +// no mangled name here yet.
  +
   // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual"
   // CHECK-SAME: elements:
   // CHECK-SAME: identifier: "_ZTS7Virtual")




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

https://reviews.llvm.org/D80369



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

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D77184#2056306 , @vzakhari wrote:

> It does not work on Windows (msbuild) for me, because `${pathlist_escaped}` 
> contains paths like `%(build_mode)s/bin` (caused by `set_llvm_build_mode`).  
> This seems to break something so that `python` process does not produce any 
> output at all: I checked both `OUTPUT_VARIABLE` and `ERROR_VARIABLE` and they 
> were empty on any invocation of `execute_process`.  @thakis, do you have any 
> ideas what might be wrong?
>
> FWIW, the code does not seem to work as expected for multi-configuration 
> environments, since the `%(build_mode)` substitution is not done yet and the 
> paths are not real filesystem paths.


Please ignore this.  It turned out the issue was in a python wrapper stuck in 
the middle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a subscriber: echristo.
plotfi added a comment.

@pratlucas please reland once the bugzilla issue is resolved and the testsuite 
builds for aarch64. Thanks @echristo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D80412: Summary: [Lexer] Fix invalid suffix diagnostic for fixed-point literals

[Arthi via Phabricator via cfe-commits]

nagart added a comment.

I don't have commit access. Could any one please help to commit this patch. 
Thanks in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80412



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

[clang] 97a133f - Temporarily Revert "[Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts"

[Eric Christopher via cfe-commits]

Author: Eric Christopher
Date: 2020-05-26T18:51:00-07:00
New Revision: 97a133f15724aa7ddf5d9b62dc9c0657a4efd115

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

LOG: Temporarily Revert "[Clang][AArch64] Capturing proper pointer alignment 
for Neon vld1 intrinsicts"
as it's causing crashes on code generation and 
https://bugs.llvm.org/show_bug.cgi?id=46084

This reverts commit 98cad555e29187a03e2bc3db5780762981913902.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/aarch64-neon-intrinsics.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index bef0ad27145f..b5129249c016 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -10329,9 +10329,9 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
   }
   case NEON::BI__builtin_neon_vld1_v:
   case NEON::BI__builtin_neon_vld1q_v: {
-auto Alignment = CGM.getNaturalPointeeTypeAlignment(
-E->getArg(0)->IgnoreParenCasts()->getType());
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_v ? 8 : 16);
 return Builder.CreateAlignedLoad(VTy, Ops[0], Alignment);
   }
   case NEON::BI__builtin_neon_vst1_v:
@@ -10344,8 +10344,8 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
 Ops[1] = Builder.CreateBitCast(Ops[1], Ty);
 Ty = llvm::PointerType::getUnqual(VTy->getElementType());
 Ops[0] = Builder.CreateBitCast(Ops[0], Ty);
-auto Alignment = CGM.getNaturalPointeeTypeAlignment(
-E->getArg(0)->IgnoreParenCasts()->getType());
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_lane_v ? 8 : 16);
 Ops[0] =
 Builder.CreateAlignedLoad(VTy->getElementType(), Ops[0], Alignment);
 return Builder.CreateInsertElement(Ops[1], Ops[0], Ops[2], "vld1_lane");
@@ -10355,8 +10355,8 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned 
BuiltinID,
 Value *V = UndefValue::get(Ty);
 Ty = llvm::PointerType::getUnqual(VTy->getElementType());
 Ops[0] = Builder.CreateBitCast(Ops[0], Ty);
-auto Alignment = CGM.getNaturalPointeeTypeAlignment(
-E->getArg(0)->IgnoreParenCasts()->getType());
+auto Alignment = CharUnits::fromQuantity(
+BuiltinID == NEON::BI__builtin_neon_vld1_dup_v ? 8 : 16);
 Ops[0] =
 Builder.CreateAlignedLoad(VTy->getElementType(), Ops[0], Alignment);
 llvm::Constant *CI = ConstantInt::get(Int32Ty, 0);

diff  --git a/clang/test/CodeGen/aarch64-neon-intrinsics.c 
b/clang/test/CodeGen/aarch64-neon-intrinsics.c
index 1fb245f3d342..7744b4f4a159 100644
--- a/clang/test/CodeGen/aarch64-neon-intrinsics.c
+++ b/clang/test/CodeGen/aarch64-neon-intrinsics.c
@@ -8956,7 +8956,7 @@ float64_t test_vrsqrted_f64(float64_t a) {
 
 // CHECK-LABEL: @test_vld1q_u8(
 // CHECK:   [[TMP0:%.*]] = bitcast i8* %a to <16 x i8>*
-// CHECK:   [[TMP1:%.*]] = load <16 x i8>, <16 x i8>* [[TMP0]], align 1
+// CHECK:   [[TMP1:%.*]] = load <16 x i8>, <16 x i8>* [[TMP0]]
 // CHECK:   ret <16 x i8> [[TMP1]]
 uint8x16_t test_vld1q_u8(uint8_t const *a) {
   return vld1q_u8(a);
@@ -8965,7 +8965,7 @@ uint8x16_t test_vld1q_u8(uint8_t const *a) {
 // CHECK-LABEL: @test_vld1q_u16(
 // CHECK:   [[TMP0:%.*]] = bitcast i16* %a to i8*
 // CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to <8 x i16>*
-// CHECK:   [[TMP2:%.*]] = load <8 x i16>, <8 x i16>* [[TMP1]], align 2
+// CHECK:   [[TMP2:%.*]] = load <8 x i16>, <8 x i16>* [[TMP1]]
 // CHECK:   ret <8 x i16> [[TMP2]]
 uint16x8_t test_vld1q_u16(uint16_t const *a) {
   return vld1q_u16(a);
@@ -8974,7 +8974,7 @@ uint16x8_t test_vld1q_u16(uint16_t const *a) {
 // CHECK-LABEL: @test_vld1q_u32(
 // CHECK:   [[TMP0:%.*]] = bitcast i32* %a to i8*
 // CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to <4 x i32>*
-// CHECK:   [[TMP2:%.*]] = load <4 x i32>, <4 x i32>* [[TMP1]], align 4
+// CHECK:   [[TMP2:%.*]] = load <4 x i32>, <4 x i32>* [[TMP1]]
 // CHECK:   ret <4 x i32> [[TMP2]]
 uint32x4_t test_vld1q_u32(uint32_t const *a) {
   return vld1q_u32(a);
@@ -8983,7 +8983,7 @@ uint32x4_t test_vld1q_u32(uint32_t const *a) {
 // CHECK-LABEL: @test_vld1q_u64(
 // CHECK:   [[TMP0:%.*]] = bitcast i64* %a to i8*
 // CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to <2 x i64>*
-// CHECK:   [[TMP2:%.*]] = load <2 x i64>, <2 x i64>* [[TMP1]], align 8
+// CHECK:   [[TMP2:%.*]] = load <2 x i64>, <2 x i64>* [[TMP1]]
 // CHECK:   ret <2 x i64> [[TMP2]]
 uint64x2_t test_vld1q_u64(uint64_t const *a) {
   return vld1q_u64(a);
@@ -8991,7 +8991,7 @@ uint64x2_t test_vld1q_u64(uint64_t const *a) {
 
 // CHECK-LABEL: @test_vld1q_s8(
 // CHECK:   [[TMP0:%.*]] = bitcast 

[PATCH] D77184: Make it possible for lit.site.cfg to contain relative paths, and use it for llvm and clang

[Vyacheslav Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.
Herald added a project: LLVM.

It does not work on Windows (msbuild) for me, because `${pathlist_escaped}` 
contains paths like `%(build_mode)s/bin` (caused by `set_llvm_build_mode`).  
This seems to break something so that `python` process does not produce any 
output at all: I checked both `OUTPUT_VARIABLE` and `ERROR_VARIABLE` and they 
were empty on any invocation of `execute_process`.  @thakis, do you have any 
ideas what might be wrong?

FWIW, the code does not seem to work as expected for multi-configuration 
environments, since the `%(build_mode)` substitution is not done yet and the 
paths are not real filesystem paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77184



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

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

I think the code looks good, we should make the test changes clearer, see below.




Comment at: clang/test/CodeGen/align_value.cpp:7
  double & z __attribute__((align_value(128 { };
-// CHECK: define void @_Z3fooPdS_Rd(double* align 64 %x, double* align 32 %y, 
double* align 128 dereferenceable(8) %z)
 

Can you first make an NFC patch introducing autogenerated check lines for the 
tests, with `--function-signature` please. That way the difference becomes 
clear here.

I don't think such a patch is controversial but we should give others the 
chance to comment on it. 



Comment at: llvm/lib/IR/IRBuilder.cpp:1084
+ Value *OffsetValue) {
+  SmallVector Vals({PtrValue, AlignValue});
+  if (OffsetValue)

Nit: Make it size 4 to avoid a trivial dynamic allocation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71739



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

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123
+  auto Pos = SubArchName.find_first_of("+-");
+  if (Pos != SubArchName.npos)
+SubArchName = SubArchName.substr(0, Pos);

tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > Parsing should probably be extracted into a separate function to avoid 
> > > replicating it all over the place.
> > > 
> > > I'd also propose use a different syntax for the properties.
> > > * use explicit character to separate individual elements. This way 
> > > splitting the properties becomes independent of what those properties 
> > > are. If you decide to make properties with values or change their meaning 
> > > some other way, it would not affect how you compose them.
> > > * use `name=value` or `name[+-]` for individual properties. This makes it 
> > > easy to parse individual properties and normalize their names. This makes 
> > > property map creation independent of the property values.
> > > 
> > > Right now `[+-]` serves as both a separator and as the value, which would 
> > > present problems if you ever need more flexible parametrization of 
> > > properties. What if a property must be a number or a string. Granted, you 
> > > can always encode them as a set of bools, but that's rather unpractical. 
> > > 
> > > E.g. something like this would work a bit better: 
> > > `gfx111:foo+:bar=33:buz=string`.
> > > 
> > I discussed this with our team.
> > 
> > The target id features are not raw GPU properties. They are distilled to 
> > become a few target features to decide what the compiler should do.
> > 
> > Each target feature has 3 values: on, off, and default, which are encoded 
> > as +feature, -feature, and not present.
> > 
> > For runtime, it is simple and clear how to choose device binaries based on 
> > the target features: it will try exact match, otherwise choose the default.
> > 
> > For compiler, it is also simple and clear what to do for each target 
> > feature value, since they corresponding to backend target features.
> > 
> > Basically we expect the target id feature to be like flags, not key/value 
> > pairs. In case we do need key/value pairs, they can still use + as 
> > delimiter.
> > 
> > Another reason we use +/- format is that it is more in line with the format 
> > of existing clang-offload-bundler id and target triple, which uses - as 
> > delimiter.
> > 
> > Since the target id is an extension of offload arch and users will put it 
> > into command line, we want to make it short, concise and aesthetically 
> > appealing, we would avoid using non-alpha-numeric characters in the target 
> > id features. Target triple components have similar requirements. Using : as 
> > delimiter seems unnecessary, longer, and more difficult to read.
> > 
> > Consider the following example
> > 
> > 
> > ```
> > clang -offload-id gfx908+xnack-sramecc a.hip
> > 
> > clang -offload-id gfx908:xnack+:sramecc- a.hip
> > ```
> > 
> > We are more inclined to keep the original format. 
> You're thinking in terms what's needed by AMDGPU *now*. The scheme you're 
> proposing is sufficient for your use case and I'm fine with that. I'm 
> suggesting that you should consider what happens once this change lands.
> 
> The functionality you're implementing is exposed to end-users via top-level 
> clang driver argument. This is visible to users and will be relied on.
> This will make it hard to change in the future without breaking someone. It's 
> worth making sure we're not painting ourselves in the corner here.
> 
> Also, the functionality may be useful/applicable beyond the scope of amdgpu 
> and the binary flags will not be sufficient for everyone. The scheme you're 
> proposing would be somewhat restrictive if I need to pass an integer value or 
> string. We could use something like `gfx123+foo=456-bar=789` but it would 
> look rather odd, IMO. 
> 
> Granted, none of the above is a showstopper. I guess we could support 
> multiple formats if it comes to that, but I'd rather not multiply things 
> later because we didn't think of them earlier.
> 
> > Another reason we use +/- format is that it is more in line with the format 
> > of existing clang-offload-bundler id and target triple, which uses - as 
> > delimiter.
> 
> The point was that commingling field separator and the field value is not the 
> cleanest approach, IMO. I'd be fine fine with some other character.
> 
> > Since the target id is an extension of offload arch and users will put it 
> > into command line, we want to make it short, concise and aesthetically 
> > appealing, we would avoid using non-alpha-numeric characters in the target 
> > id features. Target triple components have similar requirements. Using : as 
> > delimiter seems unnecessary, longer, and more difficult to read.
> 
> The current use of `gfxXXX` seems to fit the 'short, concise & aesthetically 
> pleasing' part of your argument much 

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

@aaron.ballman  I think we agreed in Belfast in November (after the most recent 
comment) to get this in as it is and not be as draconian about `auto`. Is 
anything blocking this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54408



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

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D79675#2051682 , @fghanim wrote:

> > My goal is to save us time, during development, review, maintenance, and 
> > future extensions. I hope you know that.
>
> I am certain of that. However, I am starting to have doubts if my time is 
> accounted for as part of "save us time".


That is unfortunate because it is. Even if you don't believe me, it seems 
illogical for me to waste your time on purpose assuming I benefit from the 
work, wouldn't you agree?

Since I can neither change what I've said before, nor predict how future 
changes impact my past comments, I will not argue with you on me changing my 
mind. 
That happens even without outside interference during a code review.

In this particular situation I suggested something and someone came up with 
something better in a different patch, I will not defend something for the sake 
of having suggested
it in the first place. Instead, I made you aware of it and asked if there is a 
reason to not adopt the better solution we haven't considered before. This is 
the same as with code.
We replace code from anyone with something better as it becomes available. That 
is a good thing, not something to argue about.

I fail to see the point in committing for example your target type solution if 
we found a more generic version in the meantime.
We can for sure commit them and then replace them subsequently, but is that 
really helping anyone? It would not be a question if
they were in, since they are not it seems to me there is no benefit in blocking 
the other patch on them. I mean, the time you worked
on that part is not "less wasted" if we commit it. TBH, I don't thin it is 
wasted at all but that is a different conversation.

---

I'm sorry you feel I waste your time. I really would not do so on purpose. In 
order to avoid such situations we should have quick reviews.
I already try to provide feedback as soon as I can. While more reviewers would 
obviously help, it is known that smaller patches do too.
If you have ideas on other improvements of the process, I'm happy to try them 
out.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:62
+  /// return a copy of the current insertion point information
+  InsertPointTy SaveIP() { return Builder.saveIP(); }
+

fghanim wrote:
> jdoerfert wrote:
> > Unused?
> I'll happily drop them if you want. I needed them at one point, and assumed 
> we may need them later, and left them in to see what you think. So still LGTM 
> , or no LGTM?
Generally we should not include code we don't need (or that is not tested).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79675



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D69764#2056104 , @rsmith wrote:

> I'm uncomfortable about `clang-format` performing this transformation at all. 
> Generally, clang-format only makes changes that are guaranteed to preserve 
> the meaning of the source program, and does not make changes that are only 
> heuristically likely to be semantics-preserving. But this transformation is 
> not behavior-preserving in a variety of cases, such as:
>
>   #define INTPTR int *
>   const INTPTR a;
>   // INTPTR const a; means something else
>


At least in my case, our codebase doesn't have code like that. I wonder how 
common it is.

> I also don't think this is something that a user would *want* in 
> `clang-format`: changing the location of `const`s is something that would 
> likely be done rarely (probably only once) when migrating to a different 
> coding style, rather than being done on the fly after each edit to a source 
> file.

I don't think this is true.

All of the arguments in favor of `clang-format` existing apply here.

- Developers get it wrong. They put the `{` or the `*` in the "wrong" place 
according to the style guide, and they put the `const` in the "wrong" place 
according to the style guide.
- `clang-format` is much faster than `clang-tidy` and it is reasonable to have 
text editors run it on every "Save" and on all files in a repo on every git 
commit etc.
- The above means that the cost of developers getting it wrong is minimized or 
eliminated
- The above means that developers don't have to pay attention to `*` placement 
or `const` placement while writing code, but can (in theory at least) focus on 
what they're trying to convey.
- It seems that more tools understand `clang-format` than `clang-tidy` (eg text 
editors with support for it)

> Fundamentally, I don't think this transformation is simply reformatting, and 
> I don't think it can be done sufficiently-correctly with only a 
> largely-context-free, heuristic-driven C++ parser.

I agree that this is a less simple transformation than existing `clang-format` 
functionality.

I can't evaluate whether your macro example (or other examples you could come 
up with) mean that it can't be done sufficiently-correctly, but I doubt I would 
come to the same conclusion. I would probably base that on whether I could find 
real-world code which it breaks, and how common the breaking-patterns (the 
macro in your example) actually are in other code.

> As such, I think this belongs in `clang-tidy`, not in `clang-format`.

Given the speed difference and the developer convenience, I think this would be 
very unfortunate. I've already tried to write this conversion as a `clang-tidy` 
check. However, my implementation has far more bugs than this `clang-format` 
implementation, and it does not handle as many cases. You can see that many 
`clang-tidy` checks exclude types of code such as "all templates" from 
transformation to dampen the complexity of the check implementation.

Besides, a clang-tidy implementation would run into the same problem with your 
macro example, wouldn't it? Often the solution to that kind of problem in 
`clang-tidy` checks is to simply not transform code in macros. Would an option 
in clang-format to not transform around macro code be an more acceptable 
solution?


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

https://reviews.llvm.org/D69764



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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 266374.
tentzen added a comment.

update LangRef.rst for new intrinsics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCatchDotDotDot.cpp
  clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
  clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp

Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -4223,6 +4223,10 @@
   Assert(
   !F->isIntrinsic() || isa(I) ||
   F->getIntrinsicID() == Intrinsic::donothing ||
+  F->getIntrinsicID() == Intrinsic::seh_try_begin ||
+  F->getIntrinsicID() == Intrinsic::seh_try_end ||
+  F->getIntrinsicID() == Intrinsic::eha_scope_begin ||
+  F->getIntrinsicID() == Intrinsic::eha_scope_end ||
   F->getIntrinsicID() == Intrinsic::coro_resume ||
   F->getIntrinsicID() == Intrinsic::coro_destroy ||
   F->getIntrinsicID() == Intrinsic::experimental_patchpoint_void ||
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2789,6 +2789,10 @@
   llvm_unreachable("Cannot invoke this intrinsic");
 case Intrinsic::donothing:
   // Ignore invokes to @llvm.donothing: jump directly to the next BB.
+case Intrinsic::seh_try_begin:
+case Intrinsic::eha_scope_begin:
+case Intrinsic::seh_try_end:
+case Intrinsic::eha_scope_end:
   break;
 case Intrinsic::experimental_patchpoint_void:
 case Intrinsic::experimental_patchpoint_i64:
@@ -6583,6 +6587,10 @@
   lowerCallToExternalSymbol(I, FunctionName);
 return;
   case Intrinsic::donothing:
+  case Intrinsic::seh_try_begin:
+  case Intrinsic::eha_scope_begin:
+  case Intrinsic::seh_try_end:
+  case Intrinsic::eha_scope_end:
 // ignore
 return;
   case Intrinsic::experimental_stackmap:
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -456,6 +456,16 @@
  [llvm_ptr_ty, llvm_ptr_ty],
  [IntrNoMem]>;
 
+// To mark the beginning/end of a try-scope for Windows SEH -EHa
+//  calls/invokes to these intrinsics are placed to model control flows
+//caused by HW exceptions under option -EHa.
+//  calls/invokes to these intrinsics will be discarded during a codegen pass
+//   after EH tables are generated
+def int_seh_try_begin : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_seh_try_end : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_eha_scope_begin : Intrinsic<[], [], [IntrNoMem]>;
+def int_eha_scope_end : Intrinsic<[], [], [IntrNoMem]>;
+
 // Note: we treat stacksave/stackrestore as writemem because we don't otherwise
 // model their dependencies on allocas.
 def int_stacksave : Intrinsic<[llvm_ptr_ty]>,
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -11530,6 +11530,59 @@
 the escaped allocas are allocated, which would break attempts to use
 '``llvm.localrecover``'.
 
+'``llvm.seh.try.begin``' and '``llvm.seh.try.end``' Intrinsics
+
+
+Syntax:
+"""
+
+::
+
+  declare void @llvm.seh.try.begin()
+  declare void @llvm.seh.try.end()
+
+Overview:
+"
+
+The '``llvm.seh.try.begin``' and '``llvm.seh.try.end``' intrinsics mark
+the boundary of a _try region for Windows SEH Asynchrous Exception Handling.
+
+Semantics:
+""
+
+When a C-function is compiled with Windows SEH Asynchrous Exception option,
+-feh_asynch (aka MSVC -EHa), these two intrinsics are injected to mark _try
+boundary and to prevent from potential exceptions being moved across boundary.
+
+'``llvm.eha.scope.begin``' and '``llvm.eha.scope.end``' Intrinsics

[PATCH] D80404: [OPENMP50]Initial support for use_device_addr clause.

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM, one nit below




Comment at: clang/lib/Sema/SemaOpenMP.cpp:10179
 Diag(StartLoc, diag::err_omp_no_clause_for_directive)
-<< "'map' or 'use_device_ptr'"
-<< getOpenMPDirectiveName(OMPD_target_data);
+<< Expected << getOpenMPDirectiveName(OMPD_target_data);
 return StmtError();

The restriction text quoted above doesn't seem to match the code anymore (also 
before).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80404



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

[PATCH] D79910: [x86][seses] Add clang flag; Use lvi-cfi with seses

[Scott Constable via Phabricator via cfe-commits]

sconstab added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/X86.cpp:200
+if (!Args.hasArg(options::OPT_mno_lvi_cfi)) {
+  Features.push_back("+lvi-cfi");
+  LVIOpt = options::OPT_mlvi_cfi;

Would it be better to add `FeatureLVIControlFlowIntegrity` as a dependency for 
`FeatureSpeculativeExecutionSideEffectSuppression` in 
`llvm/lib/Target/X86/X86.td`?



Comment at: 
llvm/lib/Target/X86/X86SpeculativeExecutionSideEffectSuppression.cpp:90
+  const X86Subtarget  = MF.getSubtarget();
+  if (!Subtarget.useSpeculativeExecutionSideEffectSuppression() &&
+  !EnableSpeculativeExecutionSideEffectSuppression)

Is it really necessary to have the target feature and the CLI flag? If SESES is 
required for, say, a *.ll file, then `+seses` can always be added as a target 
feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79910



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

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 266370.
cchen added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/sema_alignas.cpp

Index: clang/test/OpenMP/sema_alignas.cpp
===
--- /dev/null
+++ clang/test/OpenMP/sema_alignas.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 %s
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 %s
+
+// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 %s
+// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -fopenmp-version=50 %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+struct FOO {
+  static const int vec_align_bytes = 32;
+
+  void foo() {
+alignas(vec_align_bytes) double a;
+#pragma omp parallel
+a++;
+
+#pragma omp master
+a++;
+
+#pragma omp parallel private(a)
+a++;
+  }
+};
+
+namespace {
+class FOO {
+public:
+  static const int vec_align_bytes = 32;
+
+  void foo() {
+alignas(vec_align_bytes) double a;
+#pragma omp parallel
+a++;
+
+#pragma omp master
+a++;
+
+#pragma omp parallel private(a)
+a++;
+  }
+};
+} // namespace
+
+template 
+struct FOO : public T {
+  static const T vec_align_bytes = 32;
+
+  void foo() {
+alignas(vec_align_bytes) double a;
+#pragma omp parallel
+a++;
+
+#pragma omp master
+a++;
+
+#pragma omp parallel private(a)
+a++;
+  }
+};
+
+namespace {
+namespace {
+template 
+class FOO : public T {
+public:
+  static const T vec_align_bytes = 32;
+
+  void foo() {
+alignas(vec_align_bytes) double a;
+#pragma omp parallel
+a++;
+
+#pragma omp master
+a++;
+
+#pragma omp parallel private(a)
+a++;
+  }
+};
+} // namespace
+} // namespace
+
+#endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2631,6 +2631,7 @@
   DSAStack->pop();
   DiscardCleanupsInEvaluationContext();
   PopExpressionEvaluationContext();
+  CleanupVarDeclMarking();
 }
 
 static bool FinishOpenMPLinearClause(OMPLinearClause , DeclRefExpr *IV,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75938: [DO NOT MERGE] X86 Mitigate for Load Value Injection (LVI)--All Code

[Scott Constable via Phabricator via cfe-commits]

sconstab abandoned this revision.
sconstab added a comment.

Changes have been merged.


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

https://reviews.llvm.org/D75938



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

[PATCH] D80450: [CUDA][HIP] Fix implicit HD function resolution

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 266366.
yaxunl added a comment.

Fix test.


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

https://reviews.llvm.org/D80450

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu


Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -541,3 +541,51 @@
   }
 };
 }
+
+// Implicit HD candidate competes with device candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is 
resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithD {
+  struct a {
+__device__ a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
+
+// Implicit HD candidate competes with host candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is 
resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithH {
+  struct a {
+a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
+
+// Implicit HD candidate comptes with HD candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is 
resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithHD {
+  struct a {
+__host__ __device__ a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9534,7 +9534,7 @@
   auto EmitThreshold =
   (S.getLangOpts().CUDAIsDevice && IsCallerImplicitHD &&
(IsCand1ImplicitHD || IsCand2ImplicitHD))
-  ? Sema::CFP_HostDevice
+  ? Sema::CFP_Never
   : Sema::CFP_WrongSide;
   auto Cand1Emittable = P1 > EmitThreshold;
   auto Cand2Emittable = P2 > EmitThreshold;


Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -541,3 +541,51 @@
   }
 };
 }
+
+// Implicit HD candidate competes with device candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithD {
+  struct a {
+__device__ a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
+
+// Implicit HD candidate competes with host candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithH {
+  struct a {
+a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
+
+// Implicit HD candidate comptes with HD candidate.
+// a and b have implicit HD copy ctor. In copy ctor of b, ctor of a is resolved.
+// copy ctor of a should win over a(short), otherwise there will be ambiguity
+// due to conversion operator.
+namespace TestImplicitHDWithHD {
+  struct a {
+__host__ __device__ a(short);
+__device__ operator unsigned() const;
+__device__ operator int() const;
+  };
+  struct b {
+a d;
+  };
+  void f(b g) { b e = g; }
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9534,7 +9534,7 @@
   auto EmitThreshold =
   (S.getLangOpts().CUDAIsDevice && IsCallerImplicitHD &&
(IsCand1ImplicitHD || IsCand2ImplicitHD))
-  ? Sema::CFP_HostDevice
+  ? Sema::CFP_Never
   : Sema::CFP_WrongSide;
   auto Cand1Emittable = P1 > EmitThreshold;
   auto Cand2Emittable = P2 > EmitThreshold;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

Reduced crash case even further:

  void f() {
signed char d[16];
__builtin_neon_vld1q_v(d, 32);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D80439: Replace separator in OpenMP variant name mangling.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Changes looks good to me.

We don't have tests that check the mangling, interesting (and my fault), should 
we add one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80439



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

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

There's a cute diagram of the interaction: 
https://github.com/apple/llvm-project/blob/1fda14a45e23c41ac661c20a248a7fa4b102230d/lldb/source/Symbol/SwiftASTContext.cpp#L3020


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

https://reviews.llvm.org/D80369



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

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D80590#2056009 , @ABataev wrote:

> In D80590#2055937 , @cchen wrote:
>
> > Haven't added test yet since I'm not sure in which file should I add the 
> > test.
>
>
> Where is the directive?


I just added it, sorry for my carelessness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

After reducing the test case it looks the same as the bugzilla filing, running 
`clang --target=aarch64-unknown-linux-gnu -c` on the following does it:

  typedef signed char int8_t;
  typedef __attribute__((neon_vector_type(16))) int8_t int8x16_t;
  typedef struct int8x16x2_t {
int8x16_t val[2];
  } int8x16x2_t;
  
  int8x16_t f() {
int8_t d[16];
return __extension__ ({ int8x16_t __ret; __ret = (int8x16_t) 
__builtin_neon_vld1q_v(d, 32); __ret; });
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D80369: [DebugInfo][CallSites] Remove decl subprograms from 'retainedTypes:'

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> How does this data get used for Swift code and ObjC interoperability? At the 
> moment I see no use of this IR metadata in LLVM. Does ObjC/Swift interop use 
> the DI IR over in the Swift compiler? Got a link to the code there?

Are you saying the retained types are not emitted in DWARF?
[Assuming the types do show up in the DWARF sections of the the fat .pcm files 
—] Swift has a component called ClangImporter that reads Clang ASTs and 
generates Swift ASTs to provide Swift <-> (Objective-)C interoperability. In 
the debugger, we don't want to have to depend on the availability of C headers 
(and the associated brittleness of include paths) just to realize mixed 
Swift-Objective-C types. Since LLDB already knows how to create Clang ASTs from 
DWARF, we read the types from DWARF build a Clang AST hand that over to the 
ClangImporter in LLDB's embedded Swift compiler and thus realize Clang-imported 
Swift types.


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

https://reviews.llvm.org/D80369



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I'm uncomfortable about `clang-format` performing this transformation at all. 
Generally, clang-format only makes changes that are guaranteed to preserve the 
meaning of the source program, and does not make changes that are only 
heuristically likely to be semantics-preserving. But this transformation is not 
behavior-preserving in a variety of cases, such as:

  #define INTPTR int *
  const INTPTR a;
  // INTPTR const a; means something else

I also don't think this is something that a user would *want* in 
`clang-format`: changing the location of `const`s is something that would 
likely be done rarely (probably only once) when migrating to a different coding 
style, rather than being done on the fly after each edit to a source file.

Fundamentally, I don't think this transformation is simply reformatting, and I 
don't think it can be done sufficiently-correctly with only a 
largely-context-free, heuristic-driven C++ parser. As such, I think this 
belongs in `clang-tidy`, not in `clang-format`.


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

https://reviews.llvm.org/D69764



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

[PATCH] D74387: [OpenMP][SYCL] Improve diagnosing of unsupported types usage

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74387#2053742 , @Fznamznon wrote:

> Re-implemented diagnostic itself, now only usages of declarations
>  with unsupported types are diagnosed.
>  Generalized approach between OpenMP and SYCL.


Great, thanks a lot!

In D74387#2054337 , @Fznamznon wrote:

> The tests are failing because calling function with unsupported type in 
> arguments/return value is diagnosed as well, i.e. :
>
>   double math(float f, double d, long double ld) { ... } // `ld` is not used 
> inside the `math` function
>   #pragma omp target map(r)
> { r += math(f, d, ld); } // error: 'math' requires 128 bit size 'long 
> double' type support, but device 'nvptx64-nvidia-cuda' does not support it
>
>
> Should we diagnose calls to such functions even if those arguments/return 
> value aren't used?


Yes, please! The test case (which I added) is broken and would result in a 
crash when you actually ask for PTX and not IR: https://godbolt.org/z/vL5Biw
This is exactly what we need to diagnose :)

---

I think the code looks good and this looks like a really nice way to fix this 
properly.

I inlined some questions. We might need to add some test coverage (if we 
haven't already), e.g., for the memcpy case. For example in OpenMP an object 
`X` with such types should be ok in a `map(tofrom:X)` clause.




Comment at: clang/lib/Sema/Sema.cpp:1727
+
+  QualType Ty = D->getType();
+  auto CheckType = [&](QualType Ty) {

Nit: Move below `CheckType` to avoid shadowing and confusion with the arg 
there. 



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:21
   char c;
   T() : a(12), f(15) {}
   T +(T ) { f += b.a; return *this;}

Why is this not diagnosed? I mean we cannot assign 15 on the device, can we? Or 
does it work because it is a constant (and we basically just memcpy something)?

If it's the latter, do we have a test in the negative version that makes sure 
`T(int i) : a(i), f(i) {}` is flagged?



Comment at: clang/test/OpenMP/nvptx_unsupported_type_codegen.cpp:81
-// CHECK: define weak void 
@__omp_offloading_{{.+}}foo{{.+}}_l75([[BIGTYPE:.+]]*
-// CHECK: store [[BIGTYPE]] 
{{0xL3FFF|0xM3FF0}}, 
[[BIGTYPE]]* %

Just checking, we verify in the other test this would result in an error, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 266360.
MyDeveloperDay added a comment.

rebase with master


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13628,6 +13628,13 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstPlacement = FormatStyle::CS_West;
+  CHECK_PARSE("ConstPlacement: Leave", ConstPlacement, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstPlacement: East", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: West", ConstPlacement, FormatStyle::CS_West);
+  CHECK_PARSE("ConstPlacement: Right", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: Left", ConstPlacement, FormatStyle::CS_West);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -16572,6 +16579,301 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  verifyFormat("const int a;", Style);
+  verifyFormat("const int *a;", Style);
+  verifyFormat("const int ", Style);
+  verifyFormat("const int &", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("const Foo a;", Style);
+  verifyFormat("const Foo *a;", Style);
+  verifyFormat("const Foo ", Style);
+  verifyFormat("const Foo &", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const *b const;", Style);
+
+  verifyFormat("LLVM_NODISCARD const int ();", Style);
+  verifyFormat("LLVM_NODISCARD int const ();", Style);
+
+  verifyFormat("volatile const int *restrict;", Style);
+  verifyFormat("const volatile int *restrict;", Style);
+  verifyFormat("const int volatile *restrict;", Style);
+}
+
+TEST_F(FormatTest, EastConst) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ConstPlacement = FormatStyle::CS_East;
+
+  verifyFormat("int const a;", Style);
+  verifyFormat("int const *a;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("Foo const a;", Style);
+  verifyFormat("Foo const *a;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo *const b;", Style);
+  verifyFormat("Foo const *const b;", Style);
+  verifyFormat("auto const v = get_value();", Style);
+  verifyFormat("long long const ", Style);
+  verifyFormat("unsigned char const *a;", Style);
+  verifyFormat("int main(int const argc, char const *const *const argv)",
+   Style);
+
+  verifyFormat("LLVM_NODISCARD int const ();", Style);
+  verifyFormat("SourceRange getSourceRange() const override LLVM_READONLY",
+   Style);
+  verifyFormat("void foo() const override;", Style);
+  verifyFormat("void foo() const override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const final;", Style);
+  verifyFormat("void foo() const final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const LLVM_READONLY;", Style);
+
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  "template  explicit Action(const Action& action);",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  "template \nexplicit Action(const Action& action);",
+  Style);
+
+  verifyFormat("int const a;", "const int a;", Style);
+  verifyFormat("int const *a;", "const int *a;", Style);
+  verifyFormat("int const ", "const int ", Style);
+  

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/HIP.cpp:121-123
+  auto Pos = SubArchName.find_first_of("+-");
+  if (Pos != SubArchName.npos)
+SubArchName = SubArchName.substr(0, Pos);

yaxunl wrote:
> tra wrote:
> > Parsing should probably be extracted into a separate function to avoid 
> > replicating it all over the place.
> > 
> > I'd also propose use a different syntax for the properties.
> > * use explicit character to separate individual elements. This way 
> > splitting the properties becomes independent of what those properties are. 
> > If you decide to make properties with values or change their meaning some 
> > other way, it would not affect how you compose them.
> > * use `name=value` or `name[+-]` for individual properties. This makes it 
> > easy to parse individual properties and normalize their names. This makes 
> > property map creation independent of the property values.
> > 
> > Right now `[+-]` serves as both a separator and as the value, which would 
> > present problems if you ever need more flexible parametrization of 
> > properties. What if a property must be a number or a string. Granted, you 
> > can always encode them as a set of bools, but that's rather unpractical. 
> > 
> > E.g. something like this would work a bit better: 
> > `gfx111:foo+:bar=33:buz=string`.
> > 
> I discussed this with our team.
> 
> The target id features are not raw GPU properties. They are distilled to 
> become a few target features to decide what the compiler should do.
> 
> Each target feature has 3 values: on, off, and default, which are encoded as 
> +feature, -feature, and not present.
> 
> For runtime, it is simple and clear how to choose device binaries based on 
> the target features: it will try exact match, otherwise choose the default.
> 
> For compiler, it is also simple and clear what to do for each target feature 
> value, since they corresponding to backend target features.
> 
> Basically we expect the target id feature to be like flags, not key/value 
> pairs. In case we do need key/value pairs, they can still use + as delimiter.
> 
> Another reason we use +/- format is that it is more in line with the format 
> of existing clang-offload-bundler id and target triple, which uses - as 
> delimiter.
> 
> Since the target id is an extension of offload arch and users will put it 
> into command line, we want to make it short, concise and aesthetically 
> appealing, we would avoid using non-alpha-numeric characters in the target id 
> features. Target triple components have similar requirements. Using : as 
> delimiter seems unnecessary, longer, and more difficult to read.
> 
> Consider the following example
> 
> 
> ```
> clang -offload-id gfx908+xnack-sramecc a.hip
> 
> clang -offload-id gfx908:xnack+:sramecc- a.hip
> ```
> 
> We are more inclined to keep the original format. 
You're thinking in terms what's needed by AMDGPU *now*. The scheme you're 
proposing is sufficient for your use case and I'm fine with that. I'm 
suggesting that you should consider what happens once this change lands.

The functionality you're implementing is exposed to end-users via top-level 
clang driver argument. This is visible to users and will be relied on.
This will make it hard to change in the future without breaking someone. It's 
worth making sure we're not painting ourselves in the corner here.

Also, the functionality may be useful/applicable beyond the scope of amdgpu and 
the binary flags will not be sufficient for everyone. The scheme you're 
proposing would be somewhat restrictive if I need to pass an integer value or 
string. We could use something like `gfx123+foo=456-bar=789` but it would look 
rather odd, IMO. 

Granted, none of the above is a showstopper. I guess we could support multiple 
formats if it comes to that, but I'd rather not multiply things later because 
we didn't think of them earlier.

> Another reason we use +/- format is that it is more in line with the format 
> of existing clang-offload-bundler id and target triple, which uses - as 
> delimiter.

The point was that commingling field separator and the field value is not the 
cleanest approach, IMO. I'd be fine fine with some other character.

> Since the target id is an extension of offload arch and users will put it 
> into command line, we want to make it short, concise and aesthetically 
> appealing, we would avoid using non-alpha-numeric characters in the target id 
> features. Target triple components have similar requirements. Using : as 
> delimiter seems unnecessary, longer, and more difficult to read.

The current use of `gfxXXX` seems to fit the 'short, concise & aesthetically 
pleasing' part of your argument much better than the proposed scheme.

Is the end user allowed to specify an arbitrary set of the features? Or is the 
offload-id set restricted to a smaller number of combinations (i.e. tied to 
particular hardware variants). I vaguely recall that in 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/EastWestConstFixer.cpp:139
+  return (Tok->isSimpleTypeSpecifier() ||
+  Tok->isOneOf(tok::kw_volatile, tok::kw_auto));
+}

aaron.ballman wrote:
> Do you need to look for `restrict` here as well as `volatile`?
I think restrict only occurs the other side of the * am I right?



Comment at: clang/lib/Format/EastWestConstFixer.cpp:157-158
+  IsCVQualifierOrType(Tok->Next->Next)) {
+// The unsigned/signed case  `const unsigned int` -> `unsigned int
+// const`
+swapFirstThreeTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next->Next,

aaron.ballman wrote:
> What about something like `const unsigned long long` or the 
> less-common-but-equally-valid `long const long unsigned`? Or multiple 
> qualifiers like `unsigned volatile long const long * restrict`
I would like to cover as many cases as possible, but a small part of me thinks 
that at least initially if we skip a case or two like this I'd be fine.

But I'll take a look and see what I think we could add easily in terms of 
multiple simple types in a row.


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

https://reviews.llvm.org/D69764



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 266358.
MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.

Fix issue when preprocessor #if/#else is present
Rename the config file name to `ConstPlacement`
change the command line option to be `--const-placement`
Add Left/Right into the documentation


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/EastWestConstFixer.cpp
  clang/lib/Format/EastWestConstFixer.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13628,6 +13628,13 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.ConstPlacement = FormatStyle::CS_West;
+  CHECK_PARSE("ConstPlacement: Leave", ConstPlacement, FormatStyle::CS_Leave);
+  CHECK_PARSE("ConstPlacement: East", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: West", ConstPlacement, FormatStyle::CS_West);
+  CHECK_PARSE("ConstPlacement: Right", ConstPlacement, FormatStyle::CS_East);
+  CHECK_PARSE("ConstPlacement: Left", ConstPlacement, FormatStyle::CS_West);
+
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   CHECK_PARSE("PointerAlignment: Left", PointerAlignment,
   FormatStyle::PAS_Left);
@@ -16567,6 +16574,301 @@
"}",
Style);
 }
+
+TEST_F(FormatTest, EastWestConst) {
+  FormatStyle Style = getLLVMStyle();
+
+  // keep the const style unaltered
+  verifyFormat("const int a;", Style);
+  verifyFormat("const int *a;", Style);
+  verifyFormat("const int ", Style);
+  verifyFormat("const int &", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("const Foo a;", Style);
+  verifyFormat("const Foo *a;", Style);
+  verifyFormat("const Foo ", Style);
+  verifyFormat("const Foo &", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const *b const;", Style);
+
+  verifyFormat("LLVM_NODISCARD const int ();", Style);
+  verifyFormat("LLVM_NODISCARD int const ();", Style);
+
+  verifyFormat("volatile const int *restrict;", Style);
+  verifyFormat("const volatile int *restrict;", Style);
+  verifyFormat("const int volatile *restrict;", Style);
+}
+
+TEST_F(FormatTest, EastConst) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ConstPlacement = FormatStyle::CS_East;
+
+  verifyFormat("int const a;", Style);
+  verifyFormat("int const *a;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const b;", Style);
+  verifyFormat("int const *b;", Style);
+  verifyFormat("int const ", Style);
+  verifyFormat("int const &", Style);
+  verifyFormat("int const *b const;", Style);
+  verifyFormat("int *const c;", Style);
+
+  verifyFormat("Foo const a;", Style);
+  verifyFormat("Foo const *a;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const b;", Style);
+  verifyFormat("Foo const *b;", Style);
+  verifyFormat("Foo const ", Style);
+  verifyFormat("Foo const &", Style);
+  verifyFormat("Foo const *b const;", Style);
+  verifyFormat("Foo *const b;", Style);
+  verifyFormat("Foo const *const b;", Style);
+  verifyFormat("auto const v = get_value();", Style);
+  verifyFormat("long long const ", Style);
+  verifyFormat("unsigned char const *a;", Style);
+  verifyFormat("int main(int const argc, char const *const *const argv)",
+   Style);
+
+  verifyFormat("LLVM_NODISCARD int const ();", Style);
+  verifyFormat("SourceRange getSourceRange() const override LLVM_READONLY",
+   Style);
+  verifyFormat("void foo() const override;", Style);
+  verifyFormat("void foo() const override LLVM_READONLY;", Style);
+  verifyFormat("void foo() const final;", Style);
+  verifyFormat("void foo() const final LLVM_READONLY;", Style);
+  verifyFormat("void foo() const LLVM_READONLY;", Style);
+
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  "template  explicit Action(const Action& action);",
+  Style);
+  verifyFormat(
+  "template  explicit Action(Action const );",
+  "template 

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D80590#2055937 , @cchen wrote:

> Haven't added test yet since I'm not sure in which file should I add the test.


Where is the directive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@efriedma I am also seeing a similar crash in the llvm-test-suite for 
llvm-test-suite/SingleSource/UnitTests/Vector/NEON/simple.c

I will try and reduce the case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

> We can avoid the assertion failure by either removing alignas or OpenMP 
> directive in the above code.

What OpenMP directive?

> Haven't added test yet since I'm not sure in which file should I add the test.

Let's go with something like `clang/test/OpenMP/sema_alignas.cpp` so we can 
start discussing the test at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590



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

[PATCH] D80020: [PowerPC] Add support for -mcpu=pwr10 in both clang and llvm

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision.
amyk added a comment.

This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80020



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

[PATCH] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGefd1a8e66eaa: [analyzer][MallocChecker] Make NewDeleteLeaks 
depend on DynamicMemoryModeling… (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77474

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/Malloc+NewDelete_intersections.cpp
  clang/test/Analysis/NewDelete-checker-test.cpp
  clang/test/Analysis/NewDelete-intersections.mm
  clang/test/Analysis/new.cpp

Index: clang/test/Analysis/new.cpp
===
--- clang/test/Analysis/new.cpp
+++ clang/test/Analysis/new.cpp
@@ -115,11 +115,6 @@
   delete c;
 }
 
-//
-// Check for intersection with other checkers from MallocChecker.cpp 
-// bounded with unix.Malloc
-//
-
 // new/delete oparators are subjects of cplusplus.NewDelete.
 void testNewDeleteNoWarn() {
   int i;
@@ -135,11 +130,11 @@
   int *p3 = new int; // no-warning
 }
 
-// unix.Malloc does not know about operators new/delete.
 void testDeleteMallocked() {
   int *x = (int *)malloc(sizeof(int));
-  delete x; // FIXME: Should detect pointer escape and keep silent after 'delete' is modeled properly.
-} // expected-warning{{Potential leak of memory pointed to by 'x'}}
+  // unix.MismatchedDeallocator would catch this, but we're not testing it here.
+  delete x;
+}
 
 void testDeleteOpAfterFree() {
   int *p = (int *)malloc(sizeof(int));
Index: clang/test/Analysis/NewDelete-intersections.mm
===
--- clang/test/Analysis/NewDelete-intersections.mm
+++ clang/test/Analysis/NewDelete-intersections.mm
@@ -1,7 +1,20 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
+// RUN:  -verify=newdelete \
+// RUN:  -analyzer-checker=core \
+// RUN:  -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
+// RUN:   -verify=leak \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
+
+// leak-no-diagnostics
+
+// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \
+// RUN:   -verify=mismatch \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.MismatchedDeallocator
+
 #include "Inputs/system-header-simulator-cxx.h"
 #include "Inputs/system-header-simulator-objc.h"
 
@@ -10,12 +23,6 @@
 extern "C" void *alloca(size_t);
 extern "C" void free(void *);
 
-//
-// Check for intersections with unix.Malloc and unix.MallocWithAnnotations 
-// checkers bounded with cplusplus.NewDelete.
-//
-
-//- malloc()/free() are subjects of unix.Malloc and unix.MallocWithAnnotations
 void testMallocFreeNoWarn() {
   int i;
   free(); // no warn
@@ -39,7 +46,8 @@
 
 void testDeleteMalloced() {
   int *p1 = (int *)malloc(sizeof(int));
-  delete p1; // no warn
+  delete p1;
+  // mismatch-warning@-1{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 
   int *p2 = (int *)__builtin_alloca(sizeof(int));
   delete p2; // no warn
@@ -54,35 +62,30 @@
 void testFreeOpNew() {
   void *p = operator new(0);
   free(p);
+  // mismatch-warning@-1{{Memory allocated by operator new should be deallocated by 'delete', not free()}}
 }
-#ifdef LEAKS
-// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
 
 void testFreeNewExpr() {
   int *p = new int;
   free(p);
+  // mismatch-warning@-1{{Memory allocated by 'new' should be deallocated by 'delete', not free()}}
+  free(p);
 }
-#ifdef LEAKS
-// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
 
 void testObjcFreeNewed() {
   int *p = new int;
   NSData *nsdata = [NSData dataWithBytesNoCopy:p length:sizeof(int) freeWhenDone:1];
-#ifdef LEAKS
-  // expected-warning@-2 {{Potential leak of memory pointed to by 'p'}}
-#endif
+  // mismatch-warning@-1{{+dataWithBytesNoCopy:length:freeWhenDone: cannot take ownership of memory allocated by 'new'}}
 }
 
 void testFreeAfterDelete() {
   int *p = new int;  
   delete p;
-  

[PATCH] D80532: [NFC] Fix formatting for the 'aix-ld.c' test case.

[Steven Wan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa924dac44f31: [NFC] Fix formatting for the 
aix-ld.c test case. (authored by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80532

Files:
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -2,177 +2,177 @@
 // sysroot to make these tests independent of the host system.
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target powerpc-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-target powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32 %s
 // CHECK-LD32-NOT: warning:
-// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
-// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD32: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-NOT: "-bnso"
-// CHECK-LD32: "-b32" 
-// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000" 
-// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
-// CHECK-LD32: "-L[[SYSROOT]]/usr/lib" 
-// CHECK-LD32: "-lc"
+// CHECK-LD32: "-b32"
+// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target powerpc64-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD64 %s
 // CHECK-LD64-NOT: warning:
-// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
-// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD64: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD64-NOT: "-bnso"
-// CHECK-LD64: "-b64" 
-// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000" 
-// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
-// CHECK-LD64: "-L[[SYSROOT]]/usr/lib" 
-// CHECK-LD64: "-lc"
+// CHECK-LD64: "-b64"
+// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -pthread \
-// RUN: -target powerpc-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-pthread \
+// RUN:-target powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32-PTHREAD %s
 // CHECK-LD32-PTHREAD-NOT: warning:
-// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
-// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-PTHREAD-NOT: "-bnso"
-// CHECK-LD32-PTHREAD: "-b32" 
-// CHECK-LD32-PTHREAD: "-bpT:0x1000" "-bpD:0x2000" 
-// CHECK-LD32-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
-// CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD32-PTHREAD: "-lpthreads"
-// CHECK-LD32-PTHREAD: "-lc"
+// CHECK-LD32-PTHREAD: "-b32"
+// CHECK-LD32-PTHREAD: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32-PTHREAD: "-lpthreads"
+// CHECK-LD32-PTHREAD: "-lc"
 
-// Check powerpc-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -pthreads \
-// RUN: -target powerpc64-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN: 

[PATCH] D80055: Diagnose union tail padding

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D80055#2055151 , @jfb wrote:

> I was wondering if any of the tests were surprising to you, or if the 
> behavior described was as expected?


I've highlighted one case where the test expectation doesn't match the standard 
rules.




Comment at: clang/test/CodeGen/union-tail-padding.c:42
+#elif __cplusplus < 201703
+Front front6 = Front(); // expected-warning {{Initializing union 'Front' field 
'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes 
undefined}}
+Front front7 = Front{}; // expected-warning {{Initializing union 'Front' field 
'i' only initializes the first 4 of 8 bytes, leaving the remaining 4 bytes 
undefined}}

This warning appears to be incorrect. Value-initialization of a union with a 
trivial default constructor performs zero-initialization, which does zero out 
padding bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80055



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

@rsmith Are you okay with this patch?


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

https://reviews.llvm.org/D79895



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10331
+auto Alignment = CGM.getNaturalPointeeTypeAlignment(
+E->getArg(0)->IgnoreParenCasts()->getType());
 Ops[0] = Builder.CreateBitCast(Ops[0], llvm::PointerType::getUnqual(VTy));

IgnoreParenCasts() here seems really dubious.  You could easily end up with a 
type that's not even a pointer.

Can we just set the alignment to 1 and leave it at that?  It's not like it 
really matters for NEON.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

This caused https://bugs.llvm.org/show_bug.cgi?id=46084


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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

[PATCH] D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 266339.
zequanwu added a comment.

Add `19.20` version. Only add mangled type for version 19. 20 or later to be 
abi-compatible with https://godbolt.org/z/Bhc__A


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

https://reviews.llvm.org/D80409

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility-version=19.20 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-windows-msvc | FileCheck %s
+
+template 
+class AutoParmTemplate {
+public:
+  AutoParmTemplate() {}
+};
+
+template 
+class AutoParmsTemplate {
+public:
+  AutoParmsTemplate() {}
+};
+
+void template_mangling() {
+  AutoParmTemplate<0> auto_int;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$MH0A@@@QEAA@XZ"
+  AutoParmTemplate auto_bool;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$M_N0A@@@QEAA@XZ"
+  AutoParmTemplate<'a'> auto_char;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$MD0GB@@@QEAA@XZ"
+  AutoParmTemplate<9223372036854775807LL> int64_max;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$M_J0HPPP@@@QEAA@XZ"
+  AutoParmTemplate<-9223372036854775807LL - 1LL> int64_min;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$M_J0?IAAA@@@QEAA@XZ"
+  AutoParmTemplate<(unsigned long long)-1> uint64_neg_1;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$M_K0?0@@QEAA@XZ"
+
+  AutoParmsTemplate<0, false, 'a'> c1;
+  // CHECK: call {{.*}} @"??0?$AutoParmsTemplate@$MH0A@$M_N0A@$MD0GB@@@QEAA@XZ"
+  AutoParmsTemplate<(unsigned long)1, 9223372036854775807LL> c2;
+  // CHECK: call {{.*}} @"??0?$AutoParmsTemplate@$MK00$M_J0HPPP@@@QEAA@XZ"
+}
Index: clang/lib/AST/MicrosoftMangle.cpp
===
--- clang/lib/AST/MicrosoftMangle.cpp
+++ clang/lib/AST/MicrosoftMangle.cpp
@@ -378,8 +378,10 @@
   void mangleFunctionClass(const FunctionDecl *FD);
   void mangleCallingConvention(CallingConv CC);
   void mangleCallingConvention(const FunctionType *T);
-  void mangleIntegerLiteral(const llvm::APSInt , bool IsBoolean);
-  void mangleExpression(const Expr *E);
+  void mangleIntegerLiteral(const llvm::APSInt ,
+const NonTypeTemplateParmDecl *PD = nullptr,
+QualType *TemplateArgType = nullptr);
+  void mangleExpression(const Expr *E, const NonTypeTemplateParmDecl *PD);
   void mangleThrowSpecification(const FunctionProtoType *T);
 
   void mangleTemplateArgs(const TemplateDecl *TD,
@@ -1357,24 +1359,35 @@
   mangleUnqualifiedName(TD);
 }
 
-void MicrosoftCXXNameMangler::mangleIntegerLiteral(const llvm::APSInt ,
-   bool IsBoolean) {
-  //  ::= $0 
-  Out << "$0";
-  // Make sure booleans are encoded as 0/1.
-  if (IsBoolean && Value.getBoolValue())
-mangleNumber(1);
-  else if (Value.isSigned())
+void MicrosoftCXXNameMangler::mangleIntegerLiteral(
+const llvm::APSInt , const NonTypeTemplateParmDecl *PD,
+QualType *TemplateArgType) {
+  //  ::= $ [] 0 
+  Out << "$";
+
+  // mangle argument type if parmeter is auto
+  if (getASTContext().getLangOpts().isCompatibleWithMSVC(
+  LangOptions::MSVC2019) &&
+  PD && PD->getType()->getTypeClass() == Type::Auto && TemplateArgType) {
+Out << "M";
+mangleType(*TemplateArgType, SourceRange(), QMM_Drop);
+  }
+
+  Out << "0";
+
+  if (Value.isSigned())
 mangleNumber(Value.getSExtValue());
   else
 mangleNumber(Value.getZExtValue());
 }
 
-void MicrosoftCXXNameMangler::mangleExpression(const Expr *E) {
+void MicrosoftCXXNameMangler::mangleExpression(
+const Expr *E, const NonTypeTemplateParmDecl *PD) {
   // See if this is a constant expression.
   llvm::APSInt Value;
   if (E->isIntegerConstantExpr(Value, Context.getASTContext())) {
-mangleIntegerLiteral(Value, E->getType()->isBooleanType());
+QualType T = E->getType();
+mangleIntegerLiteral(Value, PD, );
 return;
   }
 
@@ -1448,10 +1461,12 @@
 }
 break;
   }
-  case TemplateArgument::Integral:
+  case TemplateArgument::Integral: {
+QualType T = TA.getIntegralType();
 mangleIntegerLiteral(TA.getAsIntegral(),
- TA.getIntegralType()->isBooleanType());
+ cast(Parm), );
 break;
+  }
   case TemplateArgument::NullPtr: {
 QualType T = TA.getNullPtrType();
 if (const MemberPointerType *MPT = T->getAs()) {
@@ -1473,16 +1488,18 @@
 // However, we are free to use 0 *if* we would use multiple fields for
 // non-nullptr member pointers.
 if (!RD->nullFieldOffsetIsZero()) {
-  

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

Haven't added test yet since I'm not sure in which file should I add the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590



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

[PATCH] D80020: [PowerPC] Add support for -mcpu=pwr10 in both clang and llvm

[Stefan Pintilie via Phabricator via cfe-commits]

stefanp accepted this revision.
stefanp added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80020



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

[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
cchen added a comment.
cchen added a reviewer: ABataev.
cchen added subscribers: dreachem, sandoval.

Haven't added test yet since I'm not sure in which file should I add the test.


For the below case, Clang seems to regress (asserts) after 
https://github.com/llvm/llvm-project/commit/715f7a1bd058c64a39cc4773114dfb46ae8cc8a3

  struct FOO
  {
static const int vec_align_bytes = 32;
  
void foo()
{
  alignas(vec_align_bytes) double a;
  a++;
}
  
  };

We can avoid the assertion failure by either removing alignas or
OpenMP directive in the above code. It is apparent the usage of alignas
is resonable so the issue might be stem from OpenMP code.
In addition, replacing OpenMP directive with some arbitrary Clang directive
can avoid the assertion failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80590

Files:
  clang/lib/Sema/SemaOpenMP.cpp


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2631,6 +2631,7 @@
   DSAStack->pop();
   DiscardCleanupsInEvaluationContext();
   PopExpressionEvaluationContext();
+  CleanupVarDeclMarking();
 }
 
 static bool FinishOpenMPLinearClause(OMPLinearClause , DeclRefExpr *IV,


Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2631,6 +2631,7 @@
   DSAStack->pop();
   DiscardCleanupsInEvaluationContext();
   PopExpressionEvaluationContext();
+  CleanupVarDeclMarking();
 }
 
 static bool FinishOpenMPLinearClause(OMPLinearClause , DeclRefExpr *IV,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a924dac - [NFC] Fix formatting for the 'aix-ld.c' test case.

[Steven Wan via cfe-commits]

Author: stevewan
Date: 2020-05-26T18:11:49-04:00
New Revision: a924dac44f31ffa19508165fc61a9f10cd1d4836

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

LOG: [NFC] Fix formatting for the 'aix-ld.c' test case.

Summary:
Based on comments received in D80415 pertinent to test case format, the 
following fixes are provided to other tests in 'aix-ld.c' for the sake of 
consistency and readability,
  - Align flags in RUN directives vertically.
  - Align patterns in CHECK directives vertically.
  - Remove the ‘-o %t.o’ as it’s unnecessary for tests with ‘-###’.
  - Fix typos in comments.

Reviewers: ZarkoCA, hubert.reinterpretcast, daltenty

Reviewed By: hubert.reinterpretcast

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/test/Driver/aix-ld.c

Removed: 




diff  --git a/clang/test/Driver/aix-ld.c b/clang/test/Driver/aix-ld.c
index 218fbd2bb380..59e35248af30 100644
--- a/clang/test/Driver/aix-ld.c
+++ b/clang/test/Driver/aix-ld.c
@@ -2,177 +2,177 @@
 // sysroot to make these tests independent of the host system.
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target powerpc-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-target powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32 %s
 // CHECK-LD32-NOT: warning:
-// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
-// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD32: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-NOT: "-bnso"
-// CHECK-LD32: "-b32" 
-// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000" 
-// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
-// CHECK-LD32: "-L[[SYSROOT]]/usr/lib" 
-// CHECK-LD32: "-lc"
+// CHECK-LD32: "-b32"
+// CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target powerpc64-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-target powerpc64-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD64 %s
 // CHECK-LD64-NOT: warning:
-// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
-// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD64: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD64-NOT: "-bnso"
-// CHECK-LD64: "-b64" 
-// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000" 
-// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
-// CHECK-LD64: "-L[[SYSROOT]]/usr/lib" 
-// CHECK-LD64: "-lc"
+// CHECK-LD64: "-b64"
+// CHECK-LD64: "-bpT:0x1" "-bpD:0x11000"
+// CHECK-LD64: "[[SYSROOT]]/usr/lib{{/|}}crt0_64.o"
+// CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -pthread \
-// RUN: -target powerpc-ibm-aix7.1.0.0 \
-// RUN: --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-pthread \
+// RUN:-target powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32-PTHREAD %s
 // CHECK-LD32-PTHREAD-NOT: warning:
-// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
-// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
-// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}" 
+// CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" 
"powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-PTHREAD-NOT: "-bnso"
-// CHECK-LD32-PTHREAD: "-b32" 
-// CHECK-LD32-PTHREAD: "-bpT:0x1000" "-bpD:0x2000" 
-// CHECK-LD32-PTHREAD: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
-// CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
-// 

[clang] efd1a8e - [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

[Kirstóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-05-27T00:03:53+02:00
New Revision: efd1a8e66eaa13afff709ebf16ff6280caa82ead

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

LOG: [analyzer][MallocChecker] Make NewDeleteLeaks depend on 
DynamicMemoryModeling rather than NewDelete

If you remember the mail [1] I sent out about how I envision the future of the
already existing checkers to look dependencywise, one my main points was that no
checker that emits diagnostics should be a dependency. This is more problematic
for some checkers (ahem, RetainCount [2]) more than for others, like this one.

The MallocChecker family is a mostly big monolithic modeling class some small
reporting checkers that only come to action when we are constructing a warning
message, after the actual bug was detected. The implication of this is that
NewDeleteChecker doesn't really do anything to depend on, so this change was
relatively simple.

The only thing that complicates this change is that FreeMemAux (MallocCheckers
method that models general memory deallocation) returns after calling a bug
reporting method, regardless whether the report was ever emitted (which may not
always happen, for instance, if the checker responsible for the report isn't
enabled). This return unfortunately happens before cleaning up the maps in the
GDM keeping track of the state of symbols (whether they are released, whether
that release was successful, etc). What this means is that upon disabling some
checkers, we would never clean up the map and that could've lead to false
positives, e.g.:

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/NewDelete-intersections.mm Line 66: Potential leak 
of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 73: Potential leak 
of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 77: Potential leak 
of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 111: Undefined or 
garbage value returned to caller
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 200: Potential leak 
of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/new.cpp Line 137: Potential leak of memory pointed 
to by 'x'
There two possible approaches I had in mind:

Make bug reporting methods of MallocChecker returns whether they succeeded, and
proceed with the rest of FreeMemAux if not,
Halt execution with a sink node upon failure. I decided to go with this, as
described in the code.
As you can see from the removed/changed test files, before the big checker
dependency effort landed, there were tests to check for all the weird
configurations of enabled/disabled checkers and their messy interactions, I
largely repurposed these.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html
[2] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063205.html

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/NewDelete-intersections.mm
clang/test/Analysis/new.cpp

Removed: 
clang/test/Analysis/Malloc+NewDelete_intersections.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index bc4b7d00e2d4..2ba3881c6135 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -556,13 +556,13 @@ def NewDeleteChecker : Checker<"NewDelete">,
 
 def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
   HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
-  Dependencies<[NewDeleteChecker]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation;
 
 def PlacementNewChecker : Checker<"PlacementNew">,
   HelpText<"Check if default placement new is provided with pointers to "
"sufficient storage capacity">,
-  Dependencies<[NewDeleteChecker]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation;
 
 def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a7c62a7e8046..fa69bc253fbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -684,41 +684,42 @@ class MallocChecker
   static bool SummarizeValue(raw_ostream , SVal V);
   static bool SummarizeRegion(raw_ostream , const MemRegion *MR);
 
-  void 

[PATCH] D80436: [clang][docs] Document additional bits of libc that -ffreestanding envs must provide

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs closed this revision.
jroelofs added a comment.

1e06b169be3e59799b8dcaf16d1d03bd4c12da42 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80436



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

[PATCH] D80237: [hip] Ensure pointer in struct argument has proper `addrspacecast`.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D80237#2051933 , @rjmccall wrote:

> Okay.  Can you explain why we need to coerce in the first place, though?  
> Especially if the representation is the same, why is your target-lowering 
> requiring parameters to be coerced to involve pointers in a different address 
> space?


It's not a requirement, but an optimization. The pointer representation is the 
same, but there's a penalty to using the generic pointer. From the language 
context here, we know the flat pointer can never alias the non-global address 
spaces. InferAddressSpaces won't be able to infer this (maybe it could, but it 
might break a theoretical language that allows passing some valid 32-bit 
address in 64-bit flat pointers).

I think the real problem here is the IR is missing a no-op cast between 
pointers with different address spaces. With the current promotion, GVN sees 
the bits of the addrspace(0) ptr get reinterpreted as addrspace(1), and doesn't 
have a better option than a ptrtoint/inttoptr pair. When addrspacecast was 
added, pointer bitcasts between different address spaces were disallowed (I 
think mostly for reasons that no longer apply). If we reallowed pointer bitcast 
between equivalent sized address spaces as known no-op cast, we wouldn't need 
to change the argument promotion in the frontend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80237



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:250
+__OMP_SIZE_TYPE(SizeTy)
+#undef __OMP_SIZE_TYPE
+

jdoerfert wrote:
> Why the indirection via `__OMP_SIZE_TYPE`? Wouldn't `OMP_TYPE(SizeTy, 
> M.getDataLayout().getIntPtrType(Ctx))` suffice?
I think I did it just to make it more explicit while it was temporary, but now 
that we're defining it here there's no point, I'll change it.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124
+assert(FnTy == Fn->getFunctionType() &&
+   "Found OpenMP runtime function has mismatched types");
   }

jdoerfert wrote:
> I now think we either put the stuff in `#ifndef NDEBUG` or move the `FnTy =` 
> into the first switch that does the lookup.
Yeah, I was wondering about the cost of the extra lookup. But we could 
definitely just get the type when we look up the function and just do a 
comparison in this branch.



Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100
 th_counter[i] = 0;
-#pragma omp parallel num_threads(N)
+#pragma omp parallel // num_threads(N)
 {

jdoerfert wrote:
> jhuber6 wrote:
> > I am not entirely sure why, but commenting this out causes the problem to 
> > go away. I tried adding proper names to the forward-declared functions but 
> > since clang already knew I had something called ident_t, I couldn't declare 
> > a new struct with the same name.
> This is not good. The difference should only be that the `kmpc_fork_call` has 
> a different argument, right? Does the segfault happen at compile or runtime?
> 
> You can just use the ident_t clang created, right? Did you print the function 
> names requested by clang as we discussed?
I added an assertion and debug statements. If I try to declare a struct named 
"Ident_t" I get the following error message in the seg-fault. I think the 
seg-fault is compile-time.

Found OpenMP runtime function __kmpc_global_thread_num with type i32 
(%struct.ident_t.0*). Expected type is i32 (%struct.ident_t*)
clang: 
/home/jhuber/Documents/llvm-project/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124:
 static llvm::Function* 
llvm::OpenMPIRBuilder::getOrCreateRuntimeFunction(llvm::Module&, 
llvm::omp::RuntimeFunction): Assertion `FnTy == Fn->getFunctionType() && "Found 
OpenMP runtime function has mismatched types"' failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:250
+__OMP_SIZE_TYPE(SizeTy)
+#undef __OMP_SIZE_TYPE
+

Why the indirection via `__OMP_SIZE_TYPE`? Wouldn't `OMP_TYPE(SizeTy, 
M.getDataLayout().getIntPtrType(Ctx))` suffice?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:124
+assert(FnTy == Fn->getFunctionType() &&
+   "Found OpenMP runtime function has mismatched types");
   }

I now think we either put the stuff in `#ifndef NDEBUG` or move the `FnTy =` 
into the first switch that does the lookup.



Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100
 th_counter[i] = 0;
-#pragma omp parallel num_threads(N)
+#pragma omp parallel // num_threads(N)
 {

jhuber6 wrote:
> I am not entirely sure why, but commenting this out causes the problem to go 
> away. I tried adding proper names to the forward-declared functions but since 
> clang already knew I had something called ident_t, I couldn't declare a new 
> struct with the same name.
This is not good. The difference should only be that the `kmpc_fork_call` has a 
different argument, right? Does the segfault happen at compile or runtime?

You can just use the ident_t clang created, right? Did you print the function 
names requested by clang as we discussed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80472: [clangd] Add access specifier information to hover contents

[Daniel Martín via Phabricator via cfe-commits]

danielmartin added a comment.

Thanks for the review! I don't have commit access so I'd need someone to land 
this patch for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472



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

[PATCH] D80046: [StackSafety] Make full LTO to attach metadata if MTE is enabled

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka planned changes to this revision.
vitalybuka added a comment.

I'll probably abandon this. I've converted the pass into pure analysis and I 
need to upstream that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80046



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

[clang] 1e06b16 - [clang][docs] Document additional bits of libc that -ffreestanding envs must provide

[Jon Roelofs via cfe-commits]

Author: Jon Roelofs
Date: 2020-05-26T15:42:32-06:00
New Revision: 1e06b169be3e59799b8dcaf16d1d03bd4c12da42

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

LOG: [clang][docs] Document additional bits of libc that -ffreestanding envs 
must provide

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

Added: 


Modified: 
clang/docs/CommandGuide/clang.rst

Removed: 




diff  --git a/clang/docs/CommandGuide/clang.rst 
b/clang/docs/CommandGuide/clang.rst
index 6947450beb43..de0e0eda9097 100644
--- a/clang/docs/CommandGuide/clang.rst
+++ b/clang/docs/CommandGuide/clang.rst
@@ -246,7 +246,9 @@ Language Selection and Mode Options
 .. option:: -ffreestanding
 
  Indicate that the file should be compiled for a freestanding, not a hosted,
- environment.
+ environment. Note that it is assumed that a freestanding environment will
+ additionally provide `memcpy`, `memmove`, `memset` and `memcmp`
+ implementations, as these are needed for efficient codegen for many programs.
 
 .. option:: -fno-builtin
 



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/lib/Format/EastWestConstFixer.cpp:22
+
+#define DEBUG_TYPE "using-declarations-sorter"
+

Should this be removed?


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

https://reviews.llvm.org/D69764



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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2055716 , @MyDeveloperDay 
wrote:

> I really do appreciate the reviews and the comments especially regarding 
> east/west/left/right/wrong ;-), I know there won't be 100% agreement, but I 
> think most people who even considered cloning the LLVM repo know what we mean 
> by East/West. As such I'm going to leave the internal code that way for now. 
> (and the name of the TokenAnalyzer Files)


I think that's totally reasonable. People working on dev tools are more likely 
to be aware of newer terminology anyway.

> However, I have added support for Left/Right and East/West in the config, but 
> I'm going to refrain from adding Before/After otherwise it just gets silly 
> having too many options.

Sounds good to me!

> Whilst I've been developing this I've tried both ways, to be honest, I 
> instinctively know what is meant by East Const because its the style I don't 
> use myself (don't shoot me for that comment). But when talking in terms of 
> Left/Right I feel I have to think about what it means quite hard. Especially 
> as Right feels Wrong to me too!

Would it help if we named the option `ConstPlacement` (or something along those 
lines) instead of `ConstStyle` as a reminder that this is about the placement 
of the qualifier relative to the base type? Or we could keep `ConstStyle` and 
go with `OnRight`|`OnLeft` (in addition to `East`|`West`) if that reads more 
clearly.

> Let me reiterate my goal. For me the goal was to make east/west const 
> conversations disappear in the same way that tab and whitespace conversations 
> have disappeared (mostly) because I think those conversations are a waste of 
> good conference time.

I think it's a great goal and I'm really looking forward to having this option 
in clang-format -- thank you for working on this feature!


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

https://reviews.llvm.org/D69764



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:245
+__OMP_PTR_TYPE(Int8PtrPtr, Int8Ptr)
+__OMP_PTR_TYPE(Int8PtrPtrPtr, Int8PtrPtr)
+

I added these types as @fghanim suggested.



Comment at: openmp/runtime/test/tasking/kmp_taskloop.c:100
 th_counter[i] = 0;
-#pragma omp parallel num_threads(N)
+#pragma omp parallel // num_threads(N)
 {

I am not entirely sure why, but commenting this out causes the problem to go 
away. I tried adding proper names to the forward-declared functions but since 
clang already knew I had something called ident_t, I couldn't declare a new 
struct with the same name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral

[Zequan Wu via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp:17
+  AutoParmTemplate<0> auto_int;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$H0A@@@QAE@XZ"
+  AutoParmTemplate auto_bool;

thakis wrote:
> zequanwu wrote:
> > thakis wrote:
> > > Are you sure this is correct? MSVC produces a different mangling 
> > > (https://godbolt.org/z/VxRfJd) and neither `undname` nor `llvm-undname` / 
> > > `demumble` can demangle the symbol here (while they demange the msvc 
> > > output according to godbolt fine).
> > I use `x64 msvc v19.24` version, which gives 
> > `@"??0?$AutoParmTemplate@$MH0A@@@QAE@XZ"`. The extra `M` might come from 
> > qualifier mangling. 
> > 
> > For `x86 msvc v19.24(WINE)` version, it produces 
> > `??0?$AutoParmTemplate@$0A@@@QAE@XZ` for both `AutoParmTemplate<0> 
> > auto_int` and `AutoParmTemplate auto_int`. Isn't this a bug?
> The test at the top says -triple=i386. If you have x64 mangling results in 
> here, you should use a 64-bit triple (ie -tripe=x86_64-pc-windows). If the 
> mangling is structurally different for different bitnesses, we should 
> probably have tests for both.
> 
> For symbols that can be exported, we need to match msvc's mangling to be 
> abi-compatible, no matter if we consider the mangling a bug or not.
In `x64 msvc v19.14`, it still gives the same mangling results as `x86`. From 
godbolt, it looks like that the mangled type was only added since `msvc 
v19.20`. (https://godbolt.org/z/xGT3w-). But the `MSVCMajorVersion` is only up 
to 1914, which I guess means `v19.14`. 

I will add a new version for `v19.20` which will add `M[type]` for each auto 
template parameter.


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

https://reviews.llvm.org/D80409



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

[PATCH] D80020: [PowerPC] Add support for -mcpu=pwr10 in both clang and llvm

[Lei Huang via Phabricator via cfe-commits]

lei updated this revision to Diff 266321.
lei added a comment.

change how we generate p10 feature list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80020

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/init-ppc64.c
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
  llvm/test/CodeGen/PowerPC/check-cpu.ll

Index: llvm/test/CodeGen/PowerPC/check-cpu.ll
===
--- llvm/test/CodeGen/PowerPC/check-cpu.ll
+++ llvm/test/CodeGen/PowerPC/check-cpu.ll
@@ -2,9 +2,13 @@
 ; RUN: -mcpu=future < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN: -mcpu=future < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=power10 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr10 < %s | FileCheck %s
 
 
-; Test mcpu=future that should be recognized on PowerPC.
+; Test -mcpu=[pwr10|future] is recognized on PowerPC.
 
 ; CHECK-NOT: is not a recognized processor for this target
 ; CHECK: .text
Index: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
@@ -651,11 +651,12 @@
   if (CacheLineSize.getNumOccurrences() > 0)
 return CacheLineSize;
 
-  // On P7, P8 or P9 we have a cache line size of 128.
+  // Starting with P7 we have a cache line size of 128.
   unsigned Directive = ST->getCPUDirective();
   // Assume that Future CPU has the same cache line size as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 128;
 
   // On other processors return a default of 64 bytes.
@@ -687,9 +688,11 @@
   // For P7 and P8, floating-point instructions have a 6-cycle latency and
   // there are two execution units, so unroll by 12x for latency hiding.
   // FIXME: the same for P9 as previous gen until POWER9 scheduling is ready
+  // FIXME: the same for P10 as previous gen until POWER10 scheduling is ready
   // Assume that future is the same as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 12;
 
   // For most things, modern systems have two execution units (and
Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -34,32 +34,33 @@
 
 namespace PPC {
   // -m directive values.
-  enum {
-DIR_NONE,
-DIR_32,
-DIR_440,
-DIR_601,
-DIR_602,
-DIR_603,
-DIR_7400,
-DIR_750,
-DIR_970,
-DIR_A2,
-DIR_E500,
-DIR_E500mc,
-DIR_E5500,
-DIR_PWR3,
-DIR_PWR4,
-DIR_PWR5,
-DIR_PWR5X,
-DIR_PWR6,
-DIR_PWR6X,
-DIR_PWR7,
-DIR_PWR8,
-DIR_PWR9,
-DIR_PWR_FUTURE,
-DIR_64
-  };
+enum {
+  DIR_NONE,
+  DIR_32,
+  DIR_440,
+  DIR_601,
+  DIR_602,
+  DIR_603,
+  DIR_7400,
+  DIR_750,
+  DIR_970,
+  DIR_A2,
+  DIR_E500,
+  DIR_E500mc,
+  DIR_E5500,
+  DIR_PWR3,
+  DIR_PWR4,
+  DIR_PWR5,
+  DIR_PWR5X,
+  DIR_PWR6,
+  DIR_PWR6X,
+  DIR_PWR7,
+  DIR_PWR8,
+  DIR_PWR9,
+  DIR_PWR10,
+  DIR_PWR_FUTURE,
+  DIR_64
+};
 }
 
 class GlobalValue;
@@ -138,6 +139,7 @@
   bool HasAddiLoadFusion;
   bool HasAddisLoadFusion;
   bool IsISA3_0;
+  bool IsISA3_1;
   bool UseLongCalls;
   bool SecurePlt;
   bool VectorsUseTwoUnits;
@@ -308,6 +310,7 @@
   bool hasHTM() const { return HasHTM; }
   bool hasFloat128() const { return HasFloat128; }
   bool isISA3_0() const { return IsISA3_0; }
+  bool isISA3_1() const { return IsISA3_1; }
   bool useLongCalls() const { return UseLongCalls; }
   bool hasFusion() const { return HasFusion; }
   bool hasAddiLoadFusion() const { return HasAddiLoadFusion; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -115,6 +115,7 @@
   HasAddiLoadFusion = false;
   HasAddisLoadFusion = false;
   

[PATCH] D79628: [Clang][Driver] Add Bounds and Thread to SupportsCoverage list

[Vitaly Buka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG14de6e29b131: [Clang][Driver] Add Bounds and Thread to 
SupportsCoverage list (authored by melver, committed by vitalybuka).

Changed prior to commit:
  https://reviews.llvm.org/D79628?vs=266316=266324#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79628

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/sanitize-coverage.c
  clang/test/Driver/fsanitize-coverage.c
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
  compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
  compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp

Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
@@ -1,7 +1,6 @@
 // Tests trace pc guard coverage collection.
 //
 // REQUIRES: has_sancovcc,stable-runtime,x86_64-linux
-// XFAIL: tsan
 //
 // RUN: DIR=%t_workdir
 // RUN: CLANG_ARGS="-O0 -fsanitize-coverage=trace-pc-guard"
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
@@ -1,7 +1,5 @@
 // Tests -fsanitize-coverage=stack-depth
 //
-// XFAIL: tsan
-//
 // RUN: %clangxx -O0 -std=c++11 -fsanitize-coverage=stack-depth %s -o %t
 // RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not Assertion{{.*}}failed
 // RUN: %clangxx -O0 -std=c++11 -fsanitize-coverage=trace-pc-guard,stack-depth \
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
@@ -2,7 +2,7 @@
 
 // REQUIRES: has_sancovcc,stable-runtime
 // UNSUPPORTED: i386-darwin
-// XFAIL: ubsan,tsan
+// XFAIL: ubsan
 // XFAIL: android && asan
 
 // RUN: %clangxx -O0 %s -S -o - -emit-llvm -fsanitize-coverage=trace-pc,bb,no-prune 2>&1 | grep "call void @__sanitizer_cov_trace_pc" | count 3
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
@@ -5,7 +5,6 @@
 //
 // RUN: %clangxx -O0 %s -fsanitize-coverage=inline-bool-flag,pc-table -o %t
 // RUN: %run %t 2>&1 | FileCheck %s
-// XFAIL: tsan
 
 #include 
 #include 
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
@@ -5,7 +5,6 @@
 //
 // RUN: %clangxx -O0 %s -fsanitize-coverage=inline-8bit-counters,pc-table -o %t
 // RUN: %run %t 2>&1 | FileCheck %s
-// XFAIL: tsan
 
 #include 
 #include 
Index: clang/test/Driver/fsanitize-coverage.c
===
--- clang/test/Driver/fsanitize-coverage.c
+++ clang/test/Driver/fsanitize-coverage.c
@@ -12,8 +12,10 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-memory -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=leak -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=bounds -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=bool -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=dataflow -fsanitize-coverage=func,trace-pc %s -### 2>&1 | 

[PATCH] D80472: [clangd] Add access specifier information to hover contents

[Daniel Martín via Phabricator via cfe-commits]

danielmartin updated this revision to Diff 266322.
danielmartin marked 2 inline comments as done.
danielmartin added a comment.

Address feedback

Rename getAccess to getAccessSpelling.
Replace more parts of the codebase that were using their own version of 
getAccessSpelling.
Use StringRef's str() method instead of explicit std::string constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80472

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp

Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/LocInfoType.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 
 using namespace clang;
 
@@ -436,19 +437,10 @@
 }
 
 void TextNodeDumper::dumpAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none:
-break;
-  case AS_public:
-OS << "public";
-break;
-  case AS_protected:
-OS << "protected";
-break;
-  case AS_private:
-OS << "private";
-break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return;
+  OS << AccessSpelling;
 }
 
 void TextNodeDumper::dumpCleanupObject(
Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -1,5 +1,6 @@
 #include "clang/AST/JSONNodeDumper.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/StringSwitch.h"
 
@@ -465,13 +466,10 @@
 #undef FIELD2
 
 std::string JSONNodeDumper::createAccessSpecifier(AccessSpecifier AS) {
-  switch (AS) {
-  case AS_none: return "none";
-  case AS_private: return "private";
-  case AS_protected: return "protected";
-  case AS_public: return "public";
-  }
-  llvm_unreachable("Unknown access specifier");
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+return "none";
+  return AccessSpelling.str();
 }
 
 llvm::json::Object
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -289,12 +289,10 @@
 }
 
 void DeclPrinter::Print(AccessSpecifier AS) {
-  switch(AS) {
-  case AS_none:  llvm_unreachable("No access specifier!");
-  case AS_public:Out << "public"; break;
-  case AS_protected: Out << "protected"; break;
-  case AS_private:   Out << "private"; break;
-  }
+  const auto AccessSpelling = getAccessSpelling(AS);
+  if (AccessSpelling.empty())
+llvm_unreachable("No access specifier!");
+  Out << AccessSpelling;
 }
 
 void DeclPrinter::PrintConstructorInitializers(CXXConstructorDecl *CDecl,
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -366,7 +366,7 @@
 
   llvm::StringRef getParameterABISpelling(ParameterABI kind);
 
-  inline llvm::StringRef getAccess(AccessSpecifier AS) {
+  inline llvm::StringRef getAccessSpelling(AccessSpecifier AS) {
 switch (AS) {
 case AccessSpecifier::AS_public:
   return "public";
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -468,7 +468,7 @@
   HoverInfo HI;
   const ASTContext  = D->getASTContext();
 
-  HI.AccessSpecifier = std::string(getAccess(D->getAccess()));
+  HI.AccessSpecifier = getAccessSpelling(D->getAccess()).str();
   HI.NamespaceScope = getNamespaceScope(D);
   if (!HI.NamespaceScope->empty())
 HI.NamespaceScope->append("::");
Index: clang-tools-extra/clang-doc/MDGenerator.cpp
===
--- clang-tools-extra/clang-doc/MDGenerator.cpp
+++ clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -157,7 +157,7 @@
 First = false;
   }
   writeHeader(I.Name, 3, OS);
-  std::string Access = std::string(getAccess(I.Access));
+  std::string Access = getAccessSpelling(I.Access).str();
   if (Access != "")
 writeLine(genItalic(Access + " " + I.ReturnType.Type.Name + " " + I.Name +
 "(" + Stream.str() + ")"),
@@ -250,7 +250,7 @@
   if (!I.Members.empty()) {
 writeHeader("Members", 2, OS);
 for (const auto  : I.Members) {
-  std::string Access = std::string(getAccess(Member.Access));
+  std::string Access = 

[PATCH] D79628: [Clang][Driver] Add Bounds and Thread to SupportsCoverage list

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 266316.
vitalybuka added a comment.

simplify some tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79628

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/sanitize-coverage.c
  clang/test/Driver/fsanitize-coverage.c
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
  compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
  compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
  
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp

Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
@@ -1,7 +1,6 @@
 // Tests trace pc guard coverage collection.
 //
 // REQUIRES: has_sancovcc,stable-runtime,x86_64-linux
-// XFAIL: tsan
 //
 // RUN: DIR=%t_workdir
 // RUN: CLANG_ARGS="-O0 -fsanitize-coverage=trace-pc-guard"
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp
@@ -1,7 +1,5 @@
 // Tests -fsanitize-coverage=stack-depth
 //
-// XFAIL: tsan
-//
 // RUN: %clangxx -O0 -std=c++11 -fsanitize-coverage=stack-depth %s -o %t
 // RUN: %run %t 2>&1 | FileCheck %s --implicit-check-not Assertion{{.*}}failed
 // RUN: %clangxx -O0 -std=c++11 -fsanitize-coverage=trace-pc-guard,stack-depth \
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp
@@ -2,7 +2,7 @@
 
 // REQUIRES: has_sancovcc,stable-runtime
 // UNSUPPORTED: i386-darwin
-// XFAIL: ubsan,tsan
+// XFAIL: ubsan
 // XFAIL: android && asan
 
 // RUN: %clangxx -O0 %s -S -o - -emit-llvm -fsanitize-coverage=trace-pc,bb,no-prune 2>&1 | grep "call void @__sanitizer_cov_trace_pc" | count 3
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
@@ -5,7 +5,6 @@
 //
 // RUN: %clangxx -O0 %s -fsanitize-coverage=inline-bool-flag,pc-table -o %t
 // RUN: %run %t 2>&1 | FileCheck %s
-// XFAIL: tsan
 
 #include 
 #include 
Index: compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
===
--- compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
+++ compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp
@@ -5,7 +5,6 @@
 //
 // RUN: %clangxx -O0 %s -fsanitize-coverage=inline-8bit-counters,pc-table -o %t
 // RUN: %run %t 2>&1 | FileCheck %s
-// XFAIL: tsan
 
 #include 
 #include 
Index: clang/test/Driver/fsanitize-coverage.c
===
--- clang/test/Driver/fsanitize-coverage.c
+++ clang/test/Driver/fsanitize-coverage.c
@@ -12,8 +12,10 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-memory -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=leak -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=bounds -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=bool -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=dataflow -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

>   if I put any declarations inside the preprocess clauses they actually don't 
> get converted.

Sorry, I'm not certain what this means. Does it mean that if you have

  #if 0
  Foo> P;
  #else
  Foo> P;
  #endif

that neither of them get converted?

Can you point me to a git branch I can use to try this out? The last patch I 
tried to download from phab didn't apply cleanly. If you have a git branch I 
can rebase it with more confidence.


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

https://reviews.llvm.org/D69764



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

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 266315.
yaxunl marked an inline comment as done.
yaxunl added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely.

Fixed passing target id to clang -cc1. Added predefined macros for target id.


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

https://reviews.llvm.org/D60620

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/OffloadArch.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/OffloadArch.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/amdgpu-macros.cl
  clang/test/Driver/invalid-target-id.cl
  clang/test/Driver/invalid-target-id.hip
  clang/test/Driver/target-id-macros.cl
  clang/test/Driver/target-id-macros.hip
  clang/test/Driver/target-id.cl
  clang/test/Driver/target-id.hip

Index: clang/test/Driver/target-id.hip
===
--- /dev/null
+++ clang/test/Driver/target-id.hip
@@ -0,0 +1,54 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --offload-arch=gfx908 \
+// RUN:   --offload-arch=gfx908+xnack+sramecc \
+// RUN:   --offload-arch=gfx908+xnack-sramecc \
+// RUN:   --hip-device-lib-path=%S/Inputs/hip_dev_lib \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908" "-fcuda-is-device"
+
+// CHECK: [[OPT:".*opt"]] {{".*-gfx908-linked.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906_BC:".*-gfx908-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_906_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+// CHECK-SAME: "-o" {{".*-gfx908-.*o"}}
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908"
+// CHECK-SAME: {{.*}} "-target-feature" "+xnack"
+// CHECK-SAME: {{.*}} "-target-feature" "+sram-ecc"
+
+// CHECK: [[OPT]] {{".*-gfx908\+xnack\+sramecc.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906XE_BC:".*-gfx908\+xnack\+sramecc.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_906XE_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+// CHECK-SAME: "-o" {{".*-gfx908\+xnack\+sramecc.*o"}}
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx908"
+// CHECK-SAME: {{.*}} "-target-feature" "+xnack"
+// CHECK-SAME: {{.*}} "-target-feature" "-sram-ecc"
+
+// CHECK: [[OPT]] {{".*-gfx908\+xnack-sramecc.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx908"
+// CHECK-SAME: "-o" [[OPT_906XN_BC:".*-gfx908\+xnack-sramecc.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_906XN_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: {{.*}} "-mcpu=gfx908"
+// CHECK-SAME: "-o" {{".*-gfx908\+xnack-sramecc.*o"}}
+
+
+// CHECK: {{".*clang-offload-bundler"}}
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-amdgcn-amd-amdhsa-gfx908,hip-amdgcn-amd-amdhsa-gfx908+xnack+sramecc,hip-amdgcn-amd-amdhsa-gfx908+xnack-sramecc"
Index: clang/test/Driver/target-id.cl
===
--- /dev/null
+++ clang/test/Driver/target-id.cl
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx908+xnack-sramecc \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+
+// RUN: %clang -### -target amdgcn-amd-amdpal \
+// RUN:   -mcpu=gfx908+xnack-sramecc \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+
+// RUN: %clang -### -target amdgcn--mesa3d \
+// RUN:   -mcpu=gfx908+xnack-sramecc \
+// RUN:   -nostdlib %s 2>&1 | FileCheck %s
+
+// CHECK: "-target-cpu" "gfx908"
+// CHECK-SAME: "-target-feature" "+xnack"
+// CHECK-SAME: "-target-feature" "-sram-ecc"
Index: clang/test/Driver/target-id-macros.hip
===
--- /dev/null
+++ clang/test/Driver/target-id-macros.hip
@@ -0,0 +1,12 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -E -dM -target x86_64-linux-gnu --cuda-device-only \
+// RUN:   --offload-arch=gfx908+xnack-sramecc -nogpulib -o - %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// CHECK-DAG: #define __amdgcn_processor__ "gfx908"
+// CHECK-DAG: #define __amdgcn_xnack__ 1
+// CHECK-DAG: #define __amdgcn_sramecc__ 0
+// CHECK-DAG: #define __amdgcn_target_id__ "gfx908+xnack-sramecc"
Index: clang/test/Driver/target-id-macros.cl
===
--- /dev/null

[PATCH] D78442: Create a warning flag for 'warn_conv_*_not_used'

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

Ping.


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

https://reviews.llvm.org/D78442



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

[clang] 14de6e2 - [Clang][Driver] Add Bounds and Thread to SupportsCoverage list

[Vitaly Buka via cfe-commits]

Author: Marco Elver
Date: 2020-05-26T13:36:21-07:00
New Revision: 14de6e29b1315e9abe61d71e3e13f75bff80e1be

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

LOG: [Clang][Driver] Add Bounds and Thread to SupportsCoverage list

Summary:
This permits combining -fsanitize-coverage with -fsanitize=bounds or
-fsanitize=thread. Note that, GCC already supports combining these.

Tested:
- Add Clang end-to-end test checking IR is generated for both combinations
of sanitizers.
- Several previously failing TSAN tests now pass.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=45831

Reviewers: vitalybuka

Reviewed By: vitalybuka

Subscribers: #sanitizers, dvyukov, nickdesaulniers, cfe-commits

Tags: #clang, #sanitizers

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

Added: 
clang/test/CodeGen/sanitize-coverage.c

Modified: 
clang/lib/Driver/SanitizerArgs.cpp
clang/test/Driver/fsanitize-coverage.c

compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline8bit_counter.cpp

compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_inline_bool_flag.cpp
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_no_prune.cpp

compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_stack_depth.cpp

compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp

Removed: 




diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index bc186fa5a598..35e982a502ef 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -43,11 +43,12 @@ static const SanitizerMask SupportsCoverage =
 SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress |
 SanitizerKind::MemTag | SanitizerKind::Memory |
 SanitizerKind::KernelMemory | SanitizerKind::Leak |
-SanitizerKind::Undefined | SanitizerKind::Integer |
+SanitizerKind::Undefined | SanitizerKind::Integer | SanitizerKind::Bounds |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
 SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
 SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero |
-SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack;
+SanitizerKind::SafeStack | SanitizerKind::ShadowCallStack |
+SanitizerKind::Thread;
 static const SanitizerMask RecoverableByDefault =
 SanitizerKind::Undefined | SanitizerKind::Integer |
 SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |

diff  --git a/clang/test/CodeGen/sanitize-coverage.c 
b/clang/test/CodeGen/sanitize-coverage.c
new file mode 100644
index ..6fc8e39354d4
--- /dev/null
+++ b/clang/test/CodeGen/sanitize-coverage.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S   
-fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck %s 
--check-prefixes=CHECK
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=address-fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,ASAN
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=bounds -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,BOUNDS
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=memory -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,MSAN
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=thread -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,TSAN
+// RUN: %clang %s -target x86_64-unknown-linux-gnu -emit-llvm -S 
-fsanitize=undefined  -fsanitize-coverage=trace-pc,trace-cmp -o - | FileCheck 
%s --check-prefixes=CHECK,UBSAN
+
+int x[10];
+
+// CHECK-LABEL: define dso_local void @foo(
+void foo(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
+  // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
+  // ASAN-DAG: call void @__asan_report_store
+  // MSAN-DAG: call void @__msan_warning
+  // BOUNDS-DAG: call void @__ubsan_handle_out_of_bounds
+  // TSAN-DAG: call void @__tsan_func_entry
+  // UBSAN-DAG: call void @__ubsan_handle
+  if (n)
+x[n] = 42;
+}
+// CHECK-LABEL: declare void

diff  --git a/clang/test/Driver/fsanitize-coverage.c 
b/clang/test/Driver/fsanitize-coverage.c
index b10fc86bb391..02078d847512 100644
--- a/clang/test/Driver/fsanitize-coverage.c
+++ b/clang/test/Driver/fsanitize-coverage.c
@@ -12,8 +12,10 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-memory 
-fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=leak 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I really do appreciate the reviews and the comments especially regarding 
east/west/left/right/wrong ;-), I know there won't be 100% agreement, but I 
think most people who even consider clone the LLVM repo know what we mean by 
East/West. as such I'm going to leave the internal code that way for now. (and 
the name of the TokenAnalyzer Files)

However, I have added support for Left/Right and East/West in the config, but 
I'm going to refrain from adding Before/After otherwise it just gets silly 
having too many options.

Whilst I've been developing this I've tried both ways, to be honest I 
instinctively know what is meant by East Const because its the style I don't 
use personally (don't shoot me for that comment). But when taking in terms of 
Left/Right I feel I have to think about what it means quite hard. Especially as 
Right feels Wrong to me!

Let me reiterate my goal. For me the goal was to make east/west const 
conversations disappear in the same way that tab and whitespace conversations 
have disappeared (mostly) because I think those conversations are a waste of 
good conference time.

The answer should just be "use clang-format", and for me, this is part of my 
own feeling that we should "clang-format all the things". I feel the best 
compromise is to offer both, (I'll likely update the documentation to not have 
so much of a bias)

If we can agree to that, then I'll be happy in a year or so to do a poll of the 
.clang-format files in github.com and then change the internal code to match 
the majority, to me that would be the greatest measure of which should be the 
primary option.

Apart from that, I've still some work to do here, this case from @steveire is 
really stumping me. Every preprocessor branch causes a different iteration over 
the same original code and as such this is causing multiple replacements to be 
added as each pass reanalyses the original code and doesn't regenerate the code 
in between (super odd)

  verifyFormat("Foo> P;\n#if 0\n#else\n#endif", "Foo 
const> P;\n#if 0\n#else\n#endif", Style);

I tried to only perform the replacement on the first pass, but alas that means 
if I put any declarations inside the preprocess clauses they actually don't get 
converted. (I'm not sure if anyone has seen this before), but its likely the 
fact that I'm using clang-format to create replacements.

In the background I've been testing this on LLVM itself (which isn't easy 
because the code isn't all clang-formatted itself, another pet peeve of mine), 
apart from the #if issue (which hits lib/Analyzer/CFG.cpp and a few other 
files) it seems to work pretty well (although I've not done a detailed 
walkthrough to see if it's missing much)

I would like to land this at some point, but cannot whilst I still have the 
#if/#else issue

Thanks for the help and the feedback, just wanted to give everyone a progress 
report.


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

https://reviews.llvm.org/D69764



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

[PATCH] D80409: [MS ABI] Add mangled type for auto template parameter whose argument kind is Integeral

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/test/CodeGenCXX/mangle-ms-auto-templates.cpp:17
+  AutoParmTemplate<0> auto_int;
+  // CHECK: call {{.*}} @"??0?$AutoParmTemplate@$H0A@@@QAE@XZ"
+  AutoParmTemplate auto_bool;

zequanwu wrote:
> thakis wrote:
> > Are you sure this is correct? MSVC produces a different mangling 
> > (https://godbolt.org/z/VxRfJd) and neither `undname` nor `llvm-undname` / 
> > `demumble` can demangle the symbol here (while they demange the msvc output 
> > according to godbolt fine).
> I use `x64 msvc v19.24` version, which gives 
> `@"??0?$AutoParmTemplate@$MH0A@@@QAE@XZ"`. The extra `M` might come from 
> qualifier mangling. 
> 
> For `x86 msvc v19.24(WINE)` version, it produces 
> `??0?$AutoParmTemplate@$0A@@@QAE@XZ` for both `AutoParmTemplate<0> auto_int` 
> and `AutoParmTemplate auto_int`. Isn't this a bug?
The test at the top says -triple=i386. If you have x64 mangling results in 
here, you should use a 64-bit triple (ie -tripe=x86_64-pc-windows). If the 
mangling is structurally different for different bitnesses, we should probably 
have tests for both.

For symbols that can be exported, we need to match msvc's mangling to be 
abi-compatible, no matter if we consider the mangling a bug or not.


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

https://reviews.llvm.org/D80409



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

[PATCH] D76793: [Matrix] Implement + and - operators for MatrixType.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:11989
+  else
+return false;
+

rjmccall wrote:
> I would suggest checking some preconditions and then just calling 
> `PrepareScalarCast`.
> 
> You should allow implicit conversions from class types, which somewhat 
> surprisingly I'm not sure we have a convenient method for, but which you can 
> find workable code for in `ConvertForConditional` in SemaExprCXX.cpp.  Test 
> case is `struct DoubleWrapper { operator double(); };`, and you should test 
> using that even when the element type isn't a double.
> 
> In SemaOverload, you should add builtin candidates for these operators if one 
> operand or the other is a matrix type.  Basically:
> 
> 1. Collect matrix types in `AddTypesConvertedFrom`
> 2. For each matrix type `M` on the LHS, add candidates for `(M, M) -> M` and 
> `(M, E) -> M`, and then analogously on the RHS.  Although you might need to 
> avoid adding redundant candidates if the same type shows up on both sides.
Thank you very much John, I hope I managed to update the patch properly 
according to your comments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793



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

[PATCH] D76793: [Matrix] Implement + and - operators for MatrixType.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 266303.
fhahn marked an inline comment as done.
fhahn added a comment.

Add support for user-defined conversion function, use PrepareScalarCast and add 
overloads for matrix +/- operators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76793

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGenCXX/matrix-type-operators.cpp
  clang/test/Sema/matrix-type-operators.c
  clang/test/SemaCXX/matrix-type-operators.cpp
  llvm/include/llvm/IR/MatrixBuilder.h

Index: llvm/include/llvm/IR/MatrixBuilder.h
===
--- llvm/include/llvm/IR/MatrixBuilder.h
+++ llvm/include/llvm/IR/MatrixBuilder.h
@@ -127,6 +127,16 @@
   /// Add matrixes \p LHS and \p RHS. Support both integer and floating point
   /// matrixes.
   Value *CreateAdd(Value *LHS, Value *RHS) {
+assert(LHS->getType()->isVectorTy() || RHS->getType()->isVectorTy());
+if (LHS->getType()->isVectorTy() && !RHS->getType()->isVectorTy())
+  RHS = B.CreateVectorSplat(
+  cast(LHS->getType())->getNumElements(), RHS,
+  "scalar.splat");
+else if (!LHS->getType()->isVectorTy() && RHS->getType()->isVectorTy())
+  LHS = B.CreateVectorSplat(
+  cast(RHS->getType())->getNumElements(), LHS,
+  "scalar.splat");
+
 return cast(LHS->getType())
->getElementType()
->isFloatingPointTy()
@@ -137,6 +147,16 @@
   /// Subtract matrixes \p LHS and \p RHS. Support both integer and floating
   /// point matrixes.
   Value *CreateSub(Value *LHS, Value *RHS) {
+assert(LHS->getType()->isVectorTy() || RHS->getType()->isVectorTy());
+if (LHS->getType()->isVectorTy() && !RHS->getType()->isVectorTy())
+  RHS = B.CreateVectorSplat(
+  cast(LHS->getType())->getNumElements(), RHS,
+  "scalar.splat");
+else if (!LHS->getType()->isVectorTy() && RHS->getType()->isVectorTy())
+  LHS = B.CreateVectorSplat(
+  cast(RHS->getType())->getNumElements(), LHS,
+  "scalar.splat");
+
 return cast(LHS->getType())
->getElementType()
->isFloatingPointTy()
Index: clang/test/SemaCXX/matrix-type-operators.cpp
===
--- clang/test/SemaCXX/matrix-type-operators.cpp
+++ clang/test/SemaCXX/matrix-type-operators.cpp
@@ -84,3 +84,96 @@
   a[2] = f;
   // expected-error@-1 {{single subscript expressions are not allowed for matrix values}}
 }
+
+template 
+struct MyMatrix {
+  using matrix_t = EltTy __attribute__((matrix_type(Rows, Columns)));
+
+  matrix_t value;
+};
+
+template 
+typename MyMatrix::matrix_t add(MyMatrix , MyMatrix ) {
+  char *v1 = A.value + B.value;
+  // expected-error@-1 {{cannot initialize a variable of type 'char *' with an rvalue of type 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))')}}
+  // expected-error@-2 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))') and 'MyMatrix::matrix_t' (aka 'float __attribute__((matrix_type(2, 2)))'))}}
+  // expected-error@-3 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))') and 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))'))}}
+
+  return A.value + B.value;
+  // expected-error@-1 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))') and 'MyMatrix::matrix_t' (aka 'float __attribute__((matrix_type(2, 2)))'))}}
+  // expected-error@-2 {{invalid operands to binary expression ('MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))') and 'MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(3, 3)))'))}}
+}
+
+void test_add_template(unsigned *Ptr1, float *Ptr2) {
+  MyMatrix Mat1;
+  MyMatrix Mat2;
+  MyMatrix Mat3;
+  Mat1.value = *((decltype(Mat1)::matrix_t *)Ptr1);
+  unsigned v1 = add(Mat1, Mat1);
+  // expected-error@-1 {{cannot initialize a variable of type 'unsigned int' with an rvalue of type 'typename MyMatrix::matrix_t' (aka 'unsigned int __attribute__((matrix_type(2, 2)))')}}
+  // expected-note@-2 {{in instantiation of function template specialization 'add' requested here}}
+
+  Mat1.value = add(Mat1, Mat2);
+  // expected-note@-1 {{in instantiation of function template specialization 'add' requested here}}
+
+  Mat1.value = add(Mat2, Mat3);
+  // expected-note@-1 {{in instantiation of function template specialization 'add' requested here}}
+}
+
+template 
+typename MyMatrix::matrix_t subtract(MyMatrix , MyMatrix ) {
+  char *v1 = A.value - B.value;
+  // expected-error@-1 {{cannot initialize a 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/Format.cpp:2547
 
+  if (Style.isCpp() || Style.Language == FormatStyle::LK_ObjC) {
+if (Style.ConstStyle != FormatStyle::CS_Leave)

aaron.ballman wrote:
> MyDeveloperDay wrote:
> > aaron.ballman wrote:
> > > This prevents us from using this in C code despite C having qualifiers 
> > > that can go to the left or right of the base type but still allows you to 
> > > use if from Objective-C. That seems incorrect.
> > clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++)
> > 
> > but you did highlight that I don't need the extra LK_ObjC check
> > clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++)
> 
> Wow, that's a really poorly named function then! Thank you for the 
> clarification.
I've been trying to persuade people ;-) {D80079}


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

https://reviews.llvm.org/D69764



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

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In D50078#2055227 , @srj wrote:

> > And I also think it should be on by default instead of modifying many 
> > .clang-format files. So IMHO if we add an option, it should be opt-out.
>
> I can live with it being opt-out. My big concern is that (as it currently 
> stands) our project gets different 'canonical' clang-format results depending 
> on whether people are using LLVM10 vs LLVM11 (and both are common and 
> supported by our project), which is a headache. I just want a way to get 
> consistent, predictable formatting regardless of the clang-format version.


Adding an option would be an issue there, I think: since clang-format is quite 
strict regarding options, the option would need to exist in both LLVM10 and 
LLVM11 if you need to support such use-case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50078



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

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

[Balogh , Ádám via Phabricator via cfe-commits]

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

In D80522#2055040 , @NoQ wrote:

> Nice, looks like you managed to reuse most of the code. I still feel like we 
> should ditch `DeclRegion` entirely (forcing its ~5 current users consider its 
> specific variants separately) but i don't insist.


Thank you for you comments. I will check tomorrow whether we could remove 
`DeclRegion` without too much code duplication.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:539-540
   (void)VV;
-  assert(cast(VV.castAs().getRegion())
- ->getStackFrame()->getParent()
- ->getStackFrame() == LC->getStackFrame());
+  if (const auto *VR =
+  dyn_cast(VV.castAs().getRegion())) {
+assert(VR->getStackFrame()->getParent()

NoQ wrote:
> Why did you relax this `cast<>` to `dyn_cast<>`?
I started from the original approach and forgot to reset this file.


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

https://reviews.llvm.org/D80522



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

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 266298.
baloghadamsoftware added a comment.
Herald added a subscriber: wuzish.

Wrong diff uploaded previously, now fixed.


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

https://reviews.llvm.org/D80522

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/explain-svals.c
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/explain-svals.m
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/ParamRegionTest.cpp

Index: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
===
--- /dev/null
+++ clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
@@ -0,0 +1,109 @@
+//===- unittests/StaticAnalyzer/ParamRegionTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Reusables.h"
+
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class ParamRegionTestConsumer : public ExprEngineConsumer {
+  void performTest(const Decl *D) {
+StoreManager  = Eng.getStoreManager();
+MemRegionManager  = StMgr.getRegionManager();
+const StackFrameContext *SFC =
+Eng.getAnalysisDeclContextManager().getStackFrame(D);
+
+if (const auto *FD = dyn_cast(D)) {
+  for (const auto *P : FD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+} else if (const auto *CD = dyn_cast(D)) {
+  for (const auto *P : CD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+} else if (const auto *MD = dyn_cast(D)) {
+  for (const auto *P : MD->parameters()) {
+const TypedValueRegion *Reg = MRMgr.getVarRegion(P, SFC);
+if (SFC->inTopFrame())
+  assert(isa(Reg));
+else
+  assert(isa(Reg));
+  }
+}
+  }
+
+public:
+  ParamRegionTestConsumer(CompilerInstance ) : ExprEngineConsumer(C) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const auto *D : DG) {
+  performTest(D);
+}
+return true;
+  }
+};
+
+class ParamRegionTestAction : public ASTFrontendAction {
+public:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef File) override {
+return std::make_unique(Compiler);
+  }
+};
+
+TEST(ParamRegion, ParamRegionTest) {
+  EXPECT_TRUE(
+  tooling::runToolOnCode(std::make_unique(),
+ R"(void foo(int n) {
+ auto lambda = [n](int m) {
+   return n + m;
+ };
+
+ int k = lambda(2);
+   }
+
+   void bar(int l) {
+ foo(l);
+   }
+
+   struct S {
+ int n;
+ S(int nn): n(nn) {}
+   };
+
+   void baz(int p) {
+ S s(p);
+   })"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(std::make_unique(),
+ R"(@interface O
+   + alloc;
+   - initWithInt:(int)q;
+   @end
+
+   void qix(int r) {
+ O *o = [[O alloc] initWithInt:r];
+   })",
+ "input.m"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ 

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 266296.
baloghadamsoftware added a comment.
Herald added subscribers: llvm-commits, MaskRay, hiraditya, arichardson, 
nemanjai, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: MaskRay.
Herald added a project: LLVM.

Updated according to the comments.


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

https://reviews.llvm.org/D80522

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/explain-svals.c
  clang/test/Analysis/explain-svals.cpp
  clang/test/Analysis/explain-svals.m
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/ParamRegionTest.cpp
  compiler-rt/lib/fuzzer/afl/afl_driver.cpp
  lld/ELF/InputFiles.cpp
  lld/test/ELF/invalid/verneed-shared.test
  lld/test/ELF/invalid/verneed-shared.yaml
  lldb/include/lldb/Utility/Args.h
  lldb/test/API/python_api/hello_world/TestHelloWorld.py
  lldb/test/API/python_api/sbdata/TestSBData.py
  llvm/include/llvm/Analysis/ObjCARCAnalysisUtils.h
  llvm/include/llvm/CodeGen/ResourcePriorityQueue.h
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp
  llvm/lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp
  llvm/lib/Support/YAMLTraits.cpp
  llvm/test/CodeGen/PowerPC/pr45709.ll
  llvm/unittests/Support/YAMLIOTest.cpp
  llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
@@ -55,6 +55,7 @@
 "SimpleConstraintManager.cpp",
 "SimpleSValBuilder.cpp",
 "Store.cpp",
+"SubEngine.cpp",
 "SymbolManager.cpp",
 "TextDiagnostics.cpp",
 "WorkList.cpp",
Index: llvm/unittests/Support/YAMLIOTest.cpp
===
--- llvm/unittests/Support/YAMLIOTest.cpp
+++ llvm/unittests/Support/YAMLIOTest.cpp
@@ -333,6 +333,7 @@
   uint16_tu16;
   uint8_t u8;
   boolb;
+  charc;
   int64_t s64;
   int32_t s32;
   int16_t s16;
@@ -357,6 +358,7 @@
   io.mapRequired("u16",  bt.u16);
   io.mapRequired("u8",   bt.u8);
   io.mapRequired("b",bt.b);
+  io.mapRequired("c",bt.c);
   io.mapRequired("s64",  bt.s64);
   io.mapRequired("s32",  bt.s32);
   io.mapRequired("s16",  bt.s16);
@@ -386,6 +388,7 @@
 "u16:  65000\n"
 "u8:   255\n"
 "b:false\n"
+"c:'c'\n"
 "s64:  -50\n"
 "s32:  -20\n"
 "s16:  -32000\n"
@@ -396,7 +399,7 @@
 "h16:  0x8765\n"
 "h32:  0xFEDCBA98\n"
 "h64:  0xFEDCBA9876543210\n"
-   "...\n");
+"...\n");
   yin >> map;
 
   EXPECT_FALSE(yin.error());
@@ -407,6 +410,7 @@
   EXPECT_EQ(map.u16, 65000);
   EXPECT_EQ(map.u8,  255);
   EXPECT_EQ(map.b,   false);
+  EXPECT_EQ(map.c,   'c');
   EXPECT_EQ(map.s64, -50LL);
   EXPECT_EQ(map.s32, -20L);
   EXPECT_EQ(map.s16, -32000);
@@ -434,6 +438,7 @@
 map.u16 = 5;
 map.u8  = 254;
 map.b   = true;
+map.c   = 'd';
 map.s64 = -60LL;
 map.s32 = -20;
 map.s16 = -32000;
@@ -463,6 +468,7 @@
 EXPECT_EQ(map.u16,  5);
 EXPECT_EQ(map.u8,   254);
 EXPECT_EQ(map.b,true);
+EXPECT_EQ(map.c,'d');
 EXPECT_EQ(map.s64,  -60LL);
 EXPECT_EQ(map.s32,  -20L);
 EXPECT_EQ(map.s16,  -32000);
Index: llvm/test/CodeGen/PowerPC/pr45709.ll
===
--- llvm/test/CodeGen/PowerPC/pr45709.ll
+++ llvm/test/CodeGen/PowerPC/pr45709.ll
@@ -10,37 +10,30 @@
 define dso_local void @_ZN1a1bEv(<4 x float> %in) local_unnamed_addr #0 align 2 {
 ; CHECK-LABEL: _ZN1a1bEv:
 ; CHECK:   # %bb.0:
-; CHECK-NEXT:bc 12, 4*cr5+lt, .LBB0_6
-; CHECK-NEXT:b .LBB0_1
-; CHECK-NEXT:  .LBB0_1: # %.preheader
-; CHECK-NEXT:b .LBB0_2
-; CHECK-NEXT:  .LBB0_2:
-; CHECK-NEXT:b .LBB0_3
-; CHECK-NEXT:  .LBB0_3:

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

@MyDeveloperDay Thanks for the update. I pinged you on slack about this, but I 
guess you're not using it at the moment. I asked if you have a git branch 
somewhere with this change. Downloading patches from phab is such a pain I have 
no idea why we use it.

If you can link me to a branch somehow, I can re-test this.

Regarding

  #if 0
  #else
  #endif

blocks causing multiple re-parses, presumably this is because clang-format 
formats code in the "other" preprocessor branch? At least I think it reformats 
comments in that case. Maybe the problem can be worked around with that in mind.




Comment at: clang/docs/ClangFormatStyleOptions.rst:1378
 
+**ConstStyle** (``ConstAlignmentStyle``)
+  Different ways to arrange const.

klimek wrote:
> MyDeveloperDay wrote:
> > aaron.ballman wrote:
> > > MyDeveloperDay wrote:
> > > > klimek wrote:
> > > > > Personally, I'm somewhat against having 3 different aliases for the 
> > > > > options. I'd chose one, even though it doesn't make everybody happy, 
> > > > > and move on. I'm fine with East/West as long as the documentation 
> > > > > makes it clear what it is.
> > > > If I have to drop the other options, I think I'd want to go with 
> > > > East/West const as I feel it has more momentum, just letting people 
> > > > know before I change the code back (to my original patch ;-) )
> > > > 
> > > > https://www.youtube.com/watch?v=gRmI_gsNqcI
> > > > 
> > > > {F10954065}
> > > > 
> > > @klimek I requested that we do not go with East/West the options and I'm 
> > > still pretty insistent on it. East/West is a kitschy way to phrase it 
> > > that I think is somewhat US-centric (where we make a pretty big 
> > > distinction between the east and west coasts). I do not want to have to 
> > > mentally map left/right to the less-clear east/west in the config file. 
> > > Would you be fine if we only had Left/Right instead of East/West? I would 
> > > be fine with that option, but figured enough people like the cute 
> > > East/West designation that we might as well support it.
> > Just for a reference, I'm not from the US and I think east/west still 
> > translates pretty well. I was happy to support the others. 
> I'd be fine with only having left/right; my personal feeling is also that 
> west-const / east-const has kinda become a term of art, though, so I really 
> don't know which one is "right" :)
> 
> Generally, I think this is one of the cases where, given good docs, we're 
> quickly spending more engineering hours discussing the right solution than 
> it'll cost aggregated over all future users, under the assumption that people 
> do not write new configs very often, and the few who will, will quickly 
> remember.
> 
> I'd be fine with only having left/right; my personal feeling is also that 
> west-const / east-const has kinda become a term of art, though, so I really 
> don't know which one is "right" :)

This reminds me of the joke that Americans drive on the "Right" side of the 
road, and English drive on the "Correct" side. Sort of gives a different 
meaning to `ConstStyle : Right` and that the alternative is `Wrong` :). Maybe 
that language ambiguity is why `East`/`West` caught on.

> people do not write new configs very often

Agreed. It seems a small number of strong views might influence this enough to 
make `East`/`West` not be used. What a pity.


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

https://reviews.llvm.org/D69764



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

Re: [clang-tools-extra] 61559d0 - [clangd] Squash GCC error with StringRef + gtest MatchesRegex()

[David Blaikie via cfe-commits]

Might be handy to link to a buildbot and/or quote the specific error
message in the commit message for changes like this so it's clear what's
being addressed by the change.

On Tue, May 19, 2020 at 4:58 AM Sam McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Sam McCall
> Date: 2020-05-19T13:58:23+02:00
> New Revision: 61559d045868e3980deca6cf5d30ad8816951960
>
> URL:
> https://github.com/llvm/llvm-project/commit/61559d045868e3980deca6cf5d30ad8816951960
> DIFF:
> https://github.com/llvm/llvm-project/commit/61559d045868e3980deca6cf5d30ad8816951960.diff
>
> LOG: [clangd] Squash GCC error with StringRef + gtest MatchesRegex()
>
> Added:
>
>
> Modified:
> clang-tools-extra/clangd/unittests/support/TraceTests.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
> b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
> index cee43f73f211..953002b69a46 100644
> --- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
> +++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
> @@ -170,12 +170,16 @@ TEST_F(CSVMetricsTracerTest, RecordsValues) {
>Counter.record(1, "");
>Dist.record(2, "y");
>
> -  EXPECT_THAT(
> -  outputLines(),
> +  auto Lines = outputLines();
> +  ASSERT_THAT(
> +  Lines,
>ElementsAre("Kind,Metric,Label,Value,Timestamp",
> -
> MatchesRegex(R"(d,dist,x,1\.00e\+00,[0-9]+\.[0-9]{6})"),
> +  StartsWith("d,dist,x,1.00e+00,"),
>StartsWith("c,cnt,,1.00e+00,"),
>StartsWith("d,dist,y,2.00e+00,"), ""));
> +  // Also check timestamp format.
> +  EXPECT_THAT(Lines[1].str(),
> +
> MatchesRegex(R"(d,dist,x,1\.00e\+00,[0-9]+\.[0-9]{6})"));
>  }
>
>  TEST_F(CSVMetricsTracerTest, Escaping) {
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68049: Propeller: Clang options for basic block sections

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

Ping.


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

https://reviews.llvm.org/D68049



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

[PATCH] D80374: [Clang] Enable KF and KC mode for [_Complex] __float128

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3970
+  DestWidth = 128;
+  break;
 case 'T':

Are there interactions with the other mode specifiers?  For example, should 
this be allowed with integer modes?  If so, I think this needs more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80374



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

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:242
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }

amccarth wrote:
> So, in other words, we'll always set `PP.SplitTemplateClosers` to true, now.
Yes. I thought it was clearest to write it separately in each branch, as it's 
basically a coincidence and I can give a separate comment.

Happy to fold it together if you think it's confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



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

[PATCH] D80532: [NFC] Fix formatting for the 'aix-ld.c' test case.

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast 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/D80532/new/

https://reviews.llvm.org/D80532



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

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

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

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:857-868
+// FEof and other states are possible.
+// The path with FEof is the one that can continue.
+// For this reason a non-fatal error is generated to continue the analysis
+// with only FEof state set.
+ExplodedNode *N = C.generateNonFatalErrorNode(State);
+if (N) {
+  C.emitReport(std::make_unique(

balazske wrote:
> Szelethus wrote:
> > Szelethus wrote:
> > > Ugh, tough one. I think this just isn't really *the* error to highlight 
> > > here, but rather that its bad practice to not check on the stream after 
> > > an operation. But I'm willing to accept I might be wrong here.
> > Actually, I take this back. If we had `NoteTag`s to explain that "this 
> > stream operation failed and left the stream file position indication in an 
> > indeterminate state", this would be great.
> Adding of `NoteTag` and other bug path improvements are planned in the next 
> changes. (The message text could mention that error check was probably 
> forgotten. But the faulty function can be called despite the error check was 
> done.)
Cool! Proceed as its most convenient, no need to rush it while the checker is 
in alpha.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018



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

[PATCH] D80548: [Analyzer][NFC] Remove the SubEngine interface

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

Szelethus added a comment.

- claps in excitement *


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80548



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

[PATCH] D80374: [Clang] Enable KF and KC mode for [_Complex] __float128

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3970
+  DestWidth = 128;
+  break;
 case 'T':

rjmccall wrote:
> Are there interactions with the other mode specifiers?  For example, should 
> this be allowed with integer modes?  If so, I think this needs more tests.
I shouldn't have said "if so" — *either way*, this needs more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80374



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

1 more nit that I saw (don't use an 'else' after an 'if' branch that returns), 
but otherwise I think this is good from the CFE perspective.  Someone better 
understanding of LLVM needs to look at the rest though.




Comment at: clang/lib/Sema/SemaChecking.cpp:1816
+  return ExprError();
+} else {
+  llvm::APFloat Probability = Eval.Val.getFloat();

You don't need the else, since the above branch returns.


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

https://reviews.llvm.org/D79830



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6446
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<

ext_typecheck_compare_complete_incomplete_pointers should be `InGroup`



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6447
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : ExtWarn<
+  "ordered comparison of complete and incomplete pointers (%0 and %1)">,

warn_typecheck_compare_complete_incomplete_pointers should be a Warning, not an 
ExtWarn.  And should be DefaultIgnore.  And should indicate why we're warning.  
And should be in the group C99Compat.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[PATCH] D80533: [Clang] Enable _Complex __float

[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:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80533



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

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:249-250
+  // cannot apply the constraint. Actually, other checkers like
+  // CallAndMessage should catch this situation earlier, because we call a
+  // function with an uninitialized argument.
+  return nullptr;

Szelethus wrote:
> Would an unreachable be appropriate here then?
Yes, good point, just added that. CallAndMessage is already a dependency, so 
this must not fire.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:993
+RetType{IntTy}, EvalCallAsPure)
+.ArgConstraint(BufferSize(0, 1)));
   }

Szelethus wrote:
> In most places, where we refer to an argument number, we use `ArgNo`. Is 
> there a reason we don't do that here? Can we enforce greater type safety?
Yeah, good point, I am going with this:
```
BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1;
```

About type safety: I was thinking about a strong typedef, but we don't actually 
have a convenient template for that in LLVM. And most of the time here in LLVM 
people just apply the /*Arg=*/ pythonish form.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77066



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

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 266287.
martong marked 4 inline comments as done.
martong added a comment.

- Add assert
- Create the BufferSize arg constraints in a more readable way


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77066

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -122,3 +122,30 @@
   // bugpath-warning{{Function argument constraint is not satisfied}} \
   // bugpath-note{{Function argument constraint is not satisfied}}
 }
+
+int __buf_size_arg_constraint(const void *, size_t);
+void test_buf_size_concrete() {
+  char buf[3]; // bugpath-note{{'buf' initialized here}}
+  __buf_size_arg_constraint(buf, 4); // \
+  // report-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-warning{{Function argument constraint is not satisfied}} \
+  // bugpath-note{{Function argument constraint is not satisfied}}
+}
+void test_buf_size_symbolic(int s) {
+  char buf[3];
+  __buf_size_arg_constraint(buf, s);
+  clang_analyzer_eval(s <= 3); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{'s' is <= 3}}
+}
+void test_buf_size_symbolic_and_offset(int s) {
+  char buf[3];
+  __buf_size_arg_constraint(buf + 1, s);
+  clang_analyzer_eval(s <= 2); // \
+  // report-warning{{TRUE}} \
+  // bugpath-warning{{TRUE}} \
+  // bugpath-note{{TRUE}} \
+  // bugpath-note{{'s' is <= 2}}
+}
Index: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
===
--- clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicSize.cpp
@@ -44,5 +44,28 @@
   return DivisionV.castAs();
 }
 
+SVal getDynamicSizeWithOffset(ProgramStateRef State, const SVal ,
+  SValBuilder ) {
+  const MemRegion *MRegion = BufV.getAsRegion();
+  if (!MRegion)
+return UnknownVal();
+  RegionOffset Offset = MRegion->getAsOffset();
+  if (Offset.hasSymbolicOffset())
+return UnknownVal();
+  const MemRegion *BaseRegion = MRegion->getBaseRegion();
+  if (!BaseRegion)
+return UnknownVal();
+
+  NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
+  Offset.getOffset() /
+  MRegion->getMemRegionManager().getContext().getCharWidth());
+  DefinedOrUnknownSVal ExtentInBytes =
+  getDynamicSize(State, BaseRegion, SvalBuilder);
+
+  return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
+   ExtentInBytes, OffsetInBytes,
+   SvalBuilder.getArrayIndexType());
+}
+
 } // namespace ento
 } // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -56,6 +56,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 
 using namespace clang;
 using namespace clang::ento;
@@ -108,7 +109,8 @@
 /// Apply the effects of the constraint on the given program state. If null
 /// is returned then the constraint is not feasible.
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
-  const Summary ) const = 0;
+  const Summary ,
+  CheckerContext ) const = 0;
 virtual ValueConstraintPtr negate() const {
   llvm_unreachable("Not implemented");
 };
@@ -143,7 +145,8 @@
const Summary ) const;
   public:
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
-  const Summary ) const override {
+  const Summary ,
+  CheckerContext ) const override {
   switch (Kind) {
   case OutOfRange:
 return applyAsOutOfRange(State, Call, Summary);
@@ -178,7 +181,8 @@
 ArgNo getOtherArgNo() const { return OtherArgN; }
 BinaryOperator::Opcode getOpcode() const { return Opcode; }
 ProgramStateRef apply(ProgramStateRef State, const CallEvent ,
-   

[PATCH] D80020: [PowerPC] Add support for -mcpu=pwr10 in both clang and llvm

[Lei Huang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7eb666b1556b: [PowerPC] Add support for -mcpu=pwr10 in both 
clang and llvm (authored by lei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80020

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/init-ppc64.c
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
  llvm/test/CodeGen/PowerPC/check-cpu.ll

Index: llvm/test/CodeGen/PowerPC/check-cpu.ll
===
--- llvm/test/CodeGen/PowerPC/check-cpu.ll
+++ llvm/test/CodeGen/PowerPC/check-cpu.ll
@@ -2,9 +2,13 @@
 ; RUN: -mcpu=future < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN: -mcpu=future < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=power10 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr10 < %s | FileCheck %s
 
 
-; Test mcpu=future that should be recognized on PowerPC.
+; Test -mcpu=[pwr10|future] is recognized on PowerPC.
 
 ; CHECK-NOT: is not a recognized processor for this target
 ; CHECK: .text
Index: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
@@ -651,11 +651,12 @@
   if (CacheLineSize.getNumOccurrences() > 0)
 return CacheLineSize;
 
-  // On P7, P8 or P9 we have a cache line size of 128.
+  // Starting with P7 we have a cache line size of 128.
   unsigned Directive = ST->getCPUDirective();
   // Assume that Future CPU has the same cache line size as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 128;
 
   // On other processors return a default of 64 bytes.
@@ -687,9 +688,11 @@
   // For P7 and P8, floating-point instructions have a 6-cycle latency and
   // there are two execution units, so unroll by 12x for latency hiding.
   // FIXME: the same for P9 as previous gen until POWER9 scheduling is ready
+  // FIXME: the same for P10 as previous gen until POWER10 scheduling is ready
   // Assume that future is the same as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 12;
 
   // For most things, modern systems have two execution units (and
Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -34,32 +34,33 @@
 
 namespace PPC {
   // -m directive values.
-  enum {
-DIR_NONE,
-DIR_32,
-DIR_440,
-DIR_601,
-DIR_602,
-DIR_603,
-DIR_7400,
-DIR_750,
-DIR_970,
-DIR_A2,
-DIR_E500,
-DIR_E500mc,
-DIR_E5500,
-DIR_PWR3,
-DIR_PWR4,
-DIR_PWR5,
-DIR_PWR5X,
-DIR_PWR6,
-DIR_PWR6X,
-DIR_PWR7,
-DIR_PWR8,
-DIR_PWR9,
-DIR_PWR_FUTURE,
-DIR_64
-  };
+enum {
+  DIR_NONE,
+  DIR_32,
+  DIR_440,
+  DIR_601,
+  DIR_602,
+  DIR_603,
+  DIR_7400,
+  DIR_750,
+  DIR_970,
+  DIR_A2,
+  DIR_E500,
+  DIR_E500mc,
+  DIR_E5500,
+  DIR_PWR3,
+  DIR_PWR4,
+  DIR_PWR5,
+  DIR_PWR5X,
+  DIR_PWR6,
+  DIR_PWR6X,
+  DIR_PWR7,
+  DIR_PWR8,
+  DIR_PWR9,
+  DIR_PWR10,
+  DIR_PWR_FUTURE,
+  DIR_64
+};
 }
 
 class GlobalValue;
@@ -138,6 +139,7 @@
   bool HasAddiLoadFusion;
   bool HasAddisLoadFusion;
   bool IsISA3_0;
+  bool IsISA3_1;
   bool UseLongCalls;
   bool SecurePlt;
   bool VectorsUseTwoUnits;
@@ -308,6 +310,7 @@
   bool hasHTM() const { return HasHTM; }
   bool hasFloat128() const { return HasFloat128; }
   bool isISA3_0() const { return IsISA3_0; }
+  bool isISA3_1() const { return IsISA3_1; }
   bool useLongCalls() const { return UseLongCalls; }
   bool hasFusion() const { return HasFusion; }
   bool hasAddiLoadFusion() const { return HasAddiLoadFusion; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ 

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Zhi Zhuang via Phabricator via cfe-commits]

LukeZhuang updated this revision to Diff 266285.
LukeZhuang added a comment.

**updated: 05/26/2020**
(1) add Sema test
(2) improve assert
(3) format change


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

https://reviews.llvm.org/D79830

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-expect-with-probability-switch.c
  clang/test/CodeGen/builtin-expect-with-probability-template.cpp
  clang/test/CodeGen/builtin-expect-with-probability.c
  clang/test/Sema/builtin-expect-with-probability.cpp
  llvm/docs/BranchWeightMetadata.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp

Index: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===
--- llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -55,13 +55,32 @@
 "unlikely-branch-weight", cl::Hidden, cl::init(1),
 cl::desc("Weight of the branch unlikely to be taken (default = 1)"));
 
+std::pair setBranchWeight(Intrinsic::ID IntrinsicID,
+  CallInst *CI, int BranchCount) {
+  if (IntrinsicID == Intrinsic::expect) {
+// __builtin_expect
+return {LikelyBranchWeight, UnlikelyBranchWeight};
+  } else {
+// __builtin_expect_with_probability
+assert(CI->getNumOperands() >= 3 &&
+   "expect with probability must have 3 arguments");
+ConstantFP *Confidence = dyn_cast(CI->getArgOperand(2));
+double TrueProb = Confidence->getValueAPF().convertToDouble();
+double FalseProb = (1.0 - TrueProb) / (BranchCount - 1);
+uint32_t LikelyBW = ceil((TrueProb * (double)(INT32_MAX - 1)) + 1.0);
+uint32_t UnlikelyBW = ceil((FalseProb * (double)(INT32_MAX - 1)) + 1.0);
+return {LikelyBW, UnlikelyBW};
+  }
+}
+
 static bool handleSwitchExpect(SwitchInst ) {
   CallInst *CI = dyn_cast(SI.getCondition());
   if (!CI)
 return false;
 
   Function *Fn = CI->getCalledFunction();
-  if (!Fn || Fn->getIntrinsicID() != Intrinsic::expect)
+  if (!Fn || (Fn->getIntrinsicID() != Intrinsic::expect &&
+  Fn->getIntrinsicID() != Intrinsic::expect_with_probability))
 return false;
 
   Value *ArgValue = CI->getArgOperand(0);
@@ -71,15 +90,20 @@
 
   SwitchInst::CaseHandle Case = *SI.findCaseValue(ExpectedValue);
   unsigned n = SI.getNumCases(); // +1 for default case.
-  SmallVector Weights(n + 1, UnlikelyBranchWeight);
+  std::pair WeightNums =
+  setBranchWeight(Fn->getIntrinsicID(), CI, n + 1);
+  uint32_t LikelyBranchWeightVal = WeightNums.first;
+  uint32_t UnlikelyBranchWeightVal = WeightNums.second;
+
+  SmallVector Weights(n + 1, UnlikelyBranchWeightVal);
 
   uint64_t Index = (Case == *SI.case_default()) ? 0 : Case.getCaseIndex() + 1;
-  Weights[Index] = LikelyBranchWeight;
+  Weights[Index] = LikelyBranchWeightVal;
 
-  SI.setMetadata(
-  LLVMContext::MD_misexpect,
-  MDBuilder(CI->getContext())
-  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+  SI.setMetadata(LLVMContext::MD_misexpect,
+ MDBuilder(CI->getContext())
+ .createMisExpect(Index, LikelyBranchWeightVal,
+  UnlikelyBranchWeightVal));
 
   SI.setCondition(ArgValue);
   misexpect::checkFrontendInstrumentation(SI);
@@ -223,15 +247,19 @@
 return true;
   return false;
 };
+std::pair WeightNums = setBranchWeight(
+Expect->getCalledFunction()->getIntrinsicID(), Expect, 2);
+uint32_t LikelyBranchWeightVal = WeightNums.first;
+uint32_t UnlikelyBranchWeightVal = WeightNums.second;
 
 if (IsOpndComingFromSuccessor(BI->getSuccessor(1)))
-  BI->setMetadata(
-  LLVMContext::MD_prof,
-  MDB.createBranchWeights(LikelyBranchWeight, UnlikelyBranchWeight));
+  BI->setMetadata(LLVMContext::MD_prof,
+  MDB.createBranchWeights(LikelyBranchWeightVal,
+  UnlikelyBranchWeightVal));
 else if (IsOpndComingFromSuccessor(BI->getSuccessor(0)))
-  BI->setMetadata(
-  LLVMContext::MD_prof,
-  MDB.createBranchWeights(UnlikelyBranchWeight, LikelyBranchWeight));
+  BI->setMetadata(LLVMContext::MD_prof,
+  MDB.createBranchWeights(UnlikelyBranchWeightVal,
+  LikelyBranchWeightVal));
   }
 }
 
@@ -277,7 +305,8 @@
   }
 
   Function *Fn = CI->getCalledFunction();
-  if (!Fn || Fn->getIntrinsicID() != Intrinsic::expect)
+  if (!Fn || (Fn->getIntrinsicID() != Intrinsic::expect &&
+  Fn->getIntrinsicID() != Intrinsic::expect_with_probability))
 return false;
 
   Value *ArgValue = CI->getArgOperand(0);
@@ -289,13 +318,22 @@
   MDNode *Node;
   MDNode *ExpNode;
 
+  std::pair 

[PATCH] D80412: Summary: [Lexer] Fix invalid suffix diagnostic for fixed-point literals

[Leonard Chan via Phabricator via cfe-commits]

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

LGTM. Thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80412



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Zhi Zhuang via Phabricator via cfe-commits]

LukeZhuang marked 3 inline comments as done.
LukeZhuang added a comment.

Fixed in latest update as well as adding test. Thank you!


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

https://reviews.llvm.org/D79830



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

[PATCH] D80548: [Analyzer][NFC] Remove the SubEngine interface

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In D80548#2055446 , @MyDeveloperDay 
wrote:

> I think the buildbot might be failing 
> http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/3664


Fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80548



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

[PATCH] D80536: [clang-tidy][modernize-loop-convert] Make loop var type human readable

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm having trouble with the reproduction of this - https://godbolt.org/z/tsMfcj.
Aside from that this needs some test cases to demonstrate the patch is indeed 
working


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80536



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

[PATCH] D80554: [DebugInfo] Use SplitTemplateClosers (foo >) in DWARF too

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:242
+// SplitTemplateClosers yields better interop with GCC and GDB (PR46052).
+PP.SplitTemplateClosers = true;
   }

So, in other words, we'll always set `PP.SplitTemplateClosers` to true, now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80554



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

[PATCH] D80020: [PowerPC] Add support for -mcpu=pwr10 in both clang and llvm

[Lei Huang via Phabricator via cfe-commits]

lei updated this revision to Diff 266283.
lei added a comment.

update as per reviewers comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80020

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/init-ppc64.c
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h
  llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
  llvm/test/CodeGen/PowerPC/check-cpu.ll

Index: llvm/test/CodeGen/PowerPC/check-cpu.ll
===
--- llvm/test/CodeGen/PowerPC/check-cpu.ll
+++ llvm/test/CodeGen/PowerPC/check-cpu.ll
@@ -2,9 +2,13 @@
 ; RUN: -mcpu=future < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN: -mcpu=future < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN: -mcpu=power10 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN: -mcpu=pwr10 < %s | FileCheck %s
 
 
-; Test mcpu=future that should be recognized on PowerPC.
+; Test -mcpu=[pwr10|future] is recognized on PowerPC.
 
 ; CHECK-NOT: is not a recognized processor for this target
 ; CHECK: .text
Index: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp
@@ -651,11 +651,12 @@
   if (CacheLineSize.getNumOccurrences() > 0)
 return CacheLineSize;
 
-  // On P7, P8 or P9 we have a cache line size of 128.
+  // Starting with P7 we have a cache line size of 128.
   unsigned Directive = ST->getCPUDirective();
   // Assume that Future CPU has the same cache line size as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 128;
 
   // On other processors return a default of 64 bytes.
@@ -687,9 +688,11 @@
   // For P7 and P8, floating-point instructions have a 6-cycle latency and
   // there are two execution units, so unroll by 12x for latency hiding.
   // FIXME: the same for P9 as previous gen until POWER9 scheduling is ready
+  // FIXME: the same for P10 as previous gen until POWER10 scheduling is ready
   // Assume that future is the same as the others.
   if (Directive == PPC::DIR_PWR7 || Directive == PPC::DIR_PWR8 ||
-  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR_FUTURE)
+  Directive == PPC::DIR_PWR9 || Directive == PPC::DIR_PWR10 ||
+  Directive == PPC::DIR_PWR_FUTURE)
 return 12;
 
   // For most things, modern systems have two execution units (and
Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -34,32 +34,33 @@
 
 namespace PPC {
   // -m directive values.
-  enum {
-DIR_NONE,
-DIR_32,
-DIR_440,
-DIR_601,
-DIR_602,
-DIR_603,
-DIR_7400,
-DIR_750,
-DIR_970,
-DIR_A2,
-DIR_E500,
-DIR_E500mc,
-DIR_E5500,
-DIR_PWR3,
-DIR_PWR4,
-DIR_PWR5,
-DIR_PWR5X,
-DIR_PWR6,
-DIR_PWR6X,
-DIR_PWR7,
-DIR_PWR8,
-DIR_PWR9,
-DIR_PWR_FUTURE,
-DIR_64
-  };
+enum {
+  DIR_NONE,
+  DIR_32,
+  DIR_440,
+  DIR_601,
+  DIR_602,
+  DIR_603,
+  DIR_7400,
+  DIR_750,
+  DIR_970,
+  DIR_A2,
+  DIR_E500,
+  DIR_E500mc,
+  DIR_E5500,
+  DIR_PWR3,
+  DIR_PWR4,
+  DIR_PWR5,
+  DIR_PWR5X,
+  DIR_PWR6,
+  DIR_PWR6X,
+  DIR_PWR7,
+  DIR_PWR8,
+  DIR_PWR9,
+  DIR_PWR10,
+  DIR_PWR_FUTURE,
+  DIR_64
+};
 }
 
 class GlobalValue;
@@ -138,6 +139,7 @@
   bool HasAddiLoadFusion;
   bool HasAddisLoadFusion;
   bool IsISA3_0;
+  bool IsISA3_1;
   bool UseLongCalls;
   bool SecurePlt;
   bool VectorsUseTwoUnits;
@@ -308,6 +310,7 @@
   bool hasHTM() const { return HasHTM; }
   bool hasFloat128() const { return HasFloat128; }
   bool isISA3_0() const { return IsISA3_0; }
+  bool isISA3_1() const { return IsISA3_1; }
   bool useLongCalls() const { return UseLongCalls; }
   bool hasFusion() const { return HasFusion; }
   bool hasAddiLoadFusion() const { return HasAddiLoadFusion; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -115,6 +115,7 @@
   HasAddiLoadFusion = false;
   HasAddisLoadFusion = false;
   IsISA3_0 =