date:20230823

[PATCH] D158461: [Driver] Remove unlikely-working Minix.cpp and Contiki.cpp

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

@brad :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158461

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

[PATCH] D158697: [clang-format][doc] Correct typos

2023-08-23 Thread Owen Pan via Phabricator via cfe-commits

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



Comment at: clang/docs/ClangFormatStyleOptions.rst:5129
 
   * ``bool AfterControlStatements`` If ``true``, put space between control 
statement keywords
 (for/if/while...) and opening parentheses.

11e2975810acd fixed the typo `betwee` here while it should have fixed it in 
Format.h instead.



Comment at: clang/include/clang/Format/Format.h:4046
   struct SpaceBeforeParensCustom {
-/// If ``true``, put space betwee control statement keywords
+/// If ``true``, put space between control statement keywords
 /// (for/if/while...) and opening parentheses.

Or to be more precise: `put a space between a control statement keyword 
(for/if/while) and the opening parenthesis that follows.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158697

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

[PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

2023-08-23 Thread Sandeep via Phabricator via cfe-commits

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



Comment at: clang/test/OpenMP/target_parallel_for_simd_tl_codegen.cpp:30
+// OMP51-NEXT:  entry:
+// OMP51-NEXT:[[DOTGLOBAL_TID__ADDR_I:%.*]] = alloca i32, align 4
+// OMP51-NEXT:[[DOTPART_ID__ADDR_I:%.*]] = alloca ptr, align 8

ABataev wrote:
> Why removed these checks?
I did not remove any check lines in this function.
But I removed checks in `omp_task_entry` function that were not related to my 
changes, to avoid failures. I only wanted to check whether 
`__kmpc_set_thread_limit()` is called.

Same for all the other test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152054

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

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a41af86041b: [X86] Support arch=x86-64{,-v2,-v3,-v4} for 
target_clones attribute (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D158329?vs=552078=552989#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158329

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-target-clones.c
  clang/test/CodeGen/builtin-cpu-supports.c
  clang/test/Sema/attr-target-clones.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/lib/TargetParser/X86TargetParser.cpp

Index: llvm/lib/TargetParser/X86TargetParser.cpp
===
--- llvm/lib/TargetParser/X86TargetParser.cpp
+++ llvm/lib/TargetParser/X86TargetParser.cpp
@@ -237,6 +237,7 @@
 // listed here before, which means it doesn't support -march, -mtune and so on.
 // FIXME: Remove OnlyForCPUDispatchSpecific after all CPUs here support both
 // cpu_dispatch/specific() feature and -march, -mtune, and so on.
+// clang-format off
 constexpr ProcInfo Processors[] = {
  // Empty processor. Include X87 and CMPXCHG8 for backwards compatibility.
   { {""}, CK_None, ~0U, FeatureX87 | FeatureCMPXCHG8B, '\0', false },
@@ -404,13 +405,14 @@
   { {"znver3"}, CK_ZNVER3, FEATURE_AVX2, FeaturesZNVER3, '\0', false },
   { {"znver4"}, CK_ZNVER4, FEATURE_AVX512VBMI2, FeaturesZNVER4, '\0', false },
   // Generic 64-bit processor.
-  { {"x86-64"}, CK_x86_64, ~0U, FeaturesX86_64, '\0', false },
-  { {"x86-64-v2"}, CK_x86_64_v2, ~0U, FeaturesX86_64_V2, '\0', false },
-  { {"x86-64-v3"}, CK_x86_64_v3, ~0U, FeaturesX86_64_V3, '\0', false },
-  { {"x86-64-v4"}, CK_x86_64_v4, ~0U, FeaturesX86_64_V4, '\0', false },
+  { {"x86-64"}, CK_x86_64, FEATURE_SSE2 , FeaturesX86_64, '\0', false },
+  { {"x86-64-v2"}, CK_x86_64_v2, FEATURE_SSE4_2 , FeaturesX86_64_V2, '\0', false },
+  { {"x86-64-v3"}, CK_x86_64_v3, FEATURE_AVX2, FeaturesX86_64_V3, '\0', false },
+  { {"x86-64-v4"}, CK_x86_64_v4, FEATURE_AVX512VL, FeaturesX86_64_V4, '\0', false },
   // Geode processors.
   { {"geode"}, CK_Geode, ~0U, FeaturesGeode, '\0', false },
 };
+// clang-format on
 
 constexpr const char *NoTuneList[] = {"x86-64-v2", "x86-64-v3", "x86-64-v4"};
 
Index: compiler-rt/lib/builtins/cpu_model.c
===
--- compiler-rt/lib/builtins/cpu_model.c
+++ compiler-rt/lib/builtins/cpu_model.c
@@ -158,6 +158,19 @@
   FEATURE_AVX512BITALG,
   FEATURE_AVX512BF16,
   FEATURE_AVX512VP2INTERSECT,
+
+  FEATURE_CMPXCHG16B = 46,
+  FEATURE_F16C = 49,
+  FEATURE_LAHF_LM = 54,
+  FEATURE_LM,
+  FEATURE_WP,
+  FEATURE_LZCNT,
+  FEATURE_MOVBE,
+
+  FEATURE_X86_64_BASELINE = 95,
+  FEATURE_X86_64_V2,
+  FEATURE_X86_64_V3,
+  FEATURE_X86_64_V4,
   CPU_FEATURE_MAX
 };
 
@@ -677,6 +690,7 @@
  unsigned *Features) {
   unsigned EAX = 0, EBX = 0;
 
+#define hasFeature(F) ((Features[F / 32] >> (F % 32)) & 1)
 #define setFeature(F)  \
   Features[F / 32] |= 1U << (F % 32)
 
@@ -697,14 +711,20 @@
 setFeature(FEATURE_SSSE3);
   if ((ECX >> 12) & 1)
 setFeature(FEATURE_FMA);
+  if ((ECX >> 13) & 1)
+setFeature(FEATURE_CMPXCHG16B);
   if ((ECX >> 19) & 1)
 setFeature(FEATURE_SSE4_1);
   if ((ECX >> 20) & 1)
 setFeature(FEATURE_SSE4_2);
+  if ((ECX >> 22) & 1)
+setFeature(FEATURE_MOVBE);
   if ((ECX >> 23) & 1)
 setFeature(FEATURE_POPCNT);
   if ((ECX >> 25) & 1)
 setFeature(FEATURE_AES);
+  if ((ECX >> 29) & 1)
+setFeature(FEATURE_F16C);
 
   // If CPUID indicates support for XSAVE, XRESTORE and AVX, and XGETBV
   // indicates that the AVX registers will be saved and restored on context
@@ -786,12 +806,39 @@
 
   bool HasExtLeaf1 = MaxExtLevel >= 0x8001 &&
  !getX86CpuIDAndInfo(0x8001, , , , );
-  if (HasExtLeaf1 && ((ECX >> 6) & 1))
-setFeature(FEATURE_SSE4_A);
-  if (HasExtLeaf1 && ((ECX >> 11) & 1))
-setFeature(FEATURE_XOP);
-  if (HasExtLeaf1 && ((ECX >> 16) & 1))
-setFeature(FEATURE_FMA4);
+  if (HasExtLeaf1) {
+if (ECX & 1)
+  setFeature(FEATURE_LAHF_LM);
+if ((ECX >> 5) & 1)
+  setFeature(FEATURE_LZCNT);
+if (((ECX >> 6) & 1))
+  setFeature(FEATURE_SSE4_A);
+if (((ECX >> 11) & 1))
+  setFeature(FEATURE_XOP);
+if (((ECX >> 16) & 1))
+  setFeature(FEATURE_FMA4);
+if (((EDX >> 29) & 1))
+  setFeature(FEATURE_LM);
+  }
+
+  if (hasFeature(FEATURE_LM) && hasFeature(FEATURE_SSE2)) {
+setFeature(FEATURE_X86_64_BASELINE);
+if (hasFeature(FEATURE_CMPXCHG16B) && hasFeature(FEATURE_POPCNT) &&
+hasFeature(FEATURE_LAHF_LM) && hasFeature(FEATURE_SSE4_2)) {
+

[PATCH] D158702: [clang][Interp] Check pointer inc/dec ops for initialization

2023-08-23 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158702

Files:
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -523,36 +523,72 @@
   }
   static_assert(incBool(), "");
 
-  template
+  /// FIXME: The diagnostics for pre-inc/dec of pointers doesn't match the
+  /// current interpreter. But they are stil OK.
+  template
   constexpr int uninit() {
 T a;
-if constexpr (Inc)
-  ++a; // ref-note 2{{increment of uninitialized}} \
-   // expected-note 2{{increment of uninitialized}}
-else
-  --a; // ref-note 2{{decrement of uninitialized}} \
-   // expected-note 2{{decrement of uninitialized}}
+if constexpr (Inc) {
+  if (Pre)
+++a; // ref-note 3{{increment of uninitialized}} \
+ // expected-note 2{{increment of uninitialized}} \
+ // expected-note {{read of uninitialized}}
+  else
+a++; // ref-note 2{{increment of uninitialized}} \
+ // expected-note 2{{increment of uninitialized}}
+} else {
+  if (Pre)
+--a; // ref-note 3{{decrement of uninitialized}} \
+ // expected-note 2{{decrement of uninitialized}} \
+ // expected-note {{read of uninitialized}}
+  else
+a--; // ref-note 2{{decrement of uninitialized}} \
+ // expected-note 2{{decrement of uninitialized}}
+}
 return 1;
   }
-  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
-  // ref-note {{in call to 'uninit()'}} \
-  // expected-error {{not an integral constant expression}} \
-  // expected-note {{in call to 'uninit()'}}
-
-  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
-   // ref-note {{in call to 'uninit()'}} \
-   // expected-error {{not an integral constant expression}} \
-   // expected-note {{in call to 'uninit()'}}
-
-  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
-// ref-note {{in call to 'uninit()'}} \
-// expected-error {{not an integral constant expression}} \
-// expected-note {{in call to 'uninit()'}}
-
-  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
- // ref-note {{in call to 'uninit()'}} \
- // expected-error {{not an integral constant expression}} \
- // expected-note {{in call to 'uninit()'}}
+  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{in call to 'uninit()'}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{in call to 'uninit()'}}
+  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{in call to 'uninit()'}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{in call to 'uninit()'}}
+
+  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to 'uninit()'}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to 'uninit()'}}
+  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'uninit()'}} \
+   // expected-error {{not an integral constant expression}} \
+   // expected-note {{in call to 'uninit()'}}
+  static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

2023-08-23 Thread Alex Cameron via Phabricator via cfe-commits

tetsuo-cpp added a comment.

Thanks for the review @nridge! I'd really appreciate if you could help me with 
the commit (I don't have commit access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157990

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

[PATCH] D158665: [clang-tidy] Improve cppcoreguidelines-avoid-reference-coroutine-parameters check

2023-08-23 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp:20
+  Finder->addMatcher(
+  functionDecl(unless(parameterCountIs(0)), hasBody(coroutineBodyStmt()))
+  .bind("fnt"),

ccotter wrote:
> Can we do this by matching if any parameter is of reference type, rather than 
> relying on the logic living in `check` (I wrote this originally, although 
> perhaps this would be an improvement). Happy to take this up as a followup...
Matching functionDecl first is cheaper...
In theory using `hasParent` instead of `hasAncestor` should be sufficient, but 
I never trusted that relation.

Something like this should do a trick also:

```
functionDecl(unless(parameterCountIs(0)), hasBody(coroutineBodyStmt()), 
forEach(parmVarDecl(hasType(qualType(hasCanonicalType(  
references(qualType()).bind("param"))
```

should also work, but I didn't test it.
Current code will do a same thing, not using matchers should even make it 
faster, and `check` method is called only for coroutines.

Personally I just prefer matcher to match nodes that we got less.
This also enable other thing, like providing 1 warning per coroutine and 
additional notes for every parameter with parameter name in it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158665

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

[PATCH] D158329: [X86] Support arch=x86-64{,-v2,-v3,-v4} for target_clones attribute

2023-08-23 Thread Phoebe Wang via Phabricator via cfe-commits

pengfei accepted this revision.
pengfei 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/D158329/new/

https://reviews.llvm.org/D158329

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

[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC

2023-08-23 Thread Min-Yih Hsu via Phabricator via cfe-commits

myhsu added a comment.

In D149867#4603853 , @aaron.ballman 
wrote:

> In D149867#4544489 , @myhsu wrote:
>
>> Sorry I was busy on my phd defense (which I passed!) in the past month. I'll 
>> get back to this next week.
>
> Congratulations!! 

Thank you!


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

https://reviews.llvm.org/D149867

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

[PATCH] D158698: [Clang][M68k] Use `DefineStd` for target-specific macros

2023-08-23 Thread Min-Yih Hsu via Phabricator via cfe-commits

myhsu created this revision.
myhsu added reviewers: jrtc27, 0x59616e.
Herald added a project: All.
myhsu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

(From the comments of D149867 )
Use `DefineStd` for target-specific macros such that GNU-style definitions can 
be correctly toggled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158698

Files:
  clang/lib/Basic/Targets/M68k.cpp
  clang/test/Driver/m68k-macros.cpp

Index: clang/test/Driver/m68k-macros.cpp
===
--- clang/test/Driver/m68k-macros.cpp
+++ clang/test/Driver/m68k-macros.cpp
@@ -4,55 +4,61 @@
 // CHECK-MX881: #define __HAVE_68881__ 1
 // CHECK-NOMX881-NOT: #define __HAVE_68881__ 1
 
-// RUN: %clang -target m68k-unknown-linux -m68000 -dM -E %s | FileCheck --check-prefixes=CHECK-MX,CHECK-NOMX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68000 -std=c++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX,CHECK-NOMX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68000 -std=gnu++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX,CHECK-MX-GNU,CHECK-NOMX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68000 -mhard-float -dM -E %s | FileCheck --check-prefix=CHECK-MX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68000 -m68881 -dM -E %s | FileCheck --check-prefix=CHECK-MX881 %s
 // CHECK-MX: #define __mc68000 1
 // CHECK-MX: #define __mc68000__ 1
-// CHECK-MX: #define mc68000 1
+// CHECK-MX-GNU: #define mc68000 1
 
-// RUN: %clang -target m68k-unknown-linux -m68010 -dM -E %s | FileCheck --check-prefixes=CHECK-MX10,CHECK-NOMX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68010 -std=c++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX10,CHECK-NOMX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68010 -std=gnu++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX10,CHECK-MX10-GNU,CHECK-NOMX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68010 -mhard-float -dM -E %s | FileCheck --check-prefix=CHECK-MX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68010 -m68881 -dM -E %s | FileCheck --check-prefix=CHECK-MX881 %s
 // CHECK-MX10: #define __mc68000 1
 // CHECK-MX10: #define __mc68000__ 1
 // CHECK-MX10: #define __mc68010 1
 // CHECK-MX10: #define __mc68010__ 1
-// CHECK-MX10: #define mc68000 1
-// CHECK-MX10: #define mc68010 1
+// CHECK-MX10-GNU: #define mc68000 1
+// CHECK-MX10-GNU: #define mc68010 1
 
-// RUN: %clang -target m68k-unknown-linux -m68020 -dM -E %s | FileCheck --check-prefixes=CHECK-MX20,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68020 -std=c++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX20,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68020 -std=gnu++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX20,CHECK-MX20-GNU,CHECK-MX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68020 -msoft-float -dM -E %s | FileCheck --check-prefix=CHECK-NOMX881 %s
 // CHECK-MX20: #define __mc68000 1
 // CHECK-MX20: #define __mc68000__ 1
 // CHECK-MX20: #define __mc68020 1
 // CHECK-MX20: #define __mc68020__ 1
-// CHECK-MX20: #define mc68000 1
-// CHECK-MX20: #define mc68020 1
+// CHECK-MX20-GNU: #define mc68000 1
+// CHECK-MX20-GNU: #define mc68020 1
 
-// RUN: %clang -target m68k-unknown-linux -m68030 -dM -E %s | FileCheck --check-prefixes=CHECK-MX30,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68030 -std=c++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX30,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68030 -std=gnu++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX30,CHECK-MX30-GNU,CHECK-MX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68030 -msoft-float -dM -E %s | FileCheck --check-prefix=CHECK-NOMX881 %s
 // CHECK-MX30: #define __mc68000 1
 // CHECK-MX30: #define __mc68000__ 1
 // CHECK-MX30: #define __mc68030 1
 // CHECK-MX30: #define __mc68030__ 1
-// CHECK-MX30: #define mc68000 1
-// CHECK-MX30: #define mc68030 1
+// CHECK-MX30-GNU: #define mc68000 1
+// CHECK-MX30-GNU: #define mc68030 1
 
-// RUN: %clang -target m68k-unknown-linux -m68040 -dM -E %s | FileCheck --check-prefixes=CHECK-MX40,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68040 -std=c++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX40,CHECK-MX881 %s
+// RUN: %clang -target m68k-unknown-linux -m68040 -std=gnu++11 -dM -E %s | FileCheck --check-prefixes=CHECK-MX40,CHECK-MX40-GNU,CHECK-MX881 %s
 // RUN: %clang -target m68k-unknown-linux -m68040 -msoft-float -dM -E %s | FileCheck --check-prefix=CHECK-NOMX881 %s
 // CHECK-MX40: #define __mc68000 1
 // CHECK-MX40: #define __mc68000__ 1
 // CHECK-MX40: #define __mc68040 1
 // CHECK-MX40: #define __mc68040__ 1
-// CHECK-MX40: #define mc68000 1
-// CHECK-MX40: #define mc68040 1
+// CHECK-MX40-GNU: #define mc68000 1
+// CHECK-MX40-GNU: #define mc68040 1
 
-// RUN: %clang -target m68k-unknown-linux -m68060 -dM -E %s | FileCheck

[PATCH] D158697: [clang-format][doc] Correct typos

2023-08-23 Thread sstwcw via Phabricator via cfe-commits

sstwcw created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
sstwcw requested review of this revision.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


I ran the script for dumping the format style options before committing
16ccba51072b 
 just in 
case.  It turned out that the 2 files didn't matchc
any more due to an attempt at fixing typos.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158697

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -971,13 +971,13 @@
   /// For example:
   /// \code
   ///   x = (char *__capability)
-  ///   int function(void) __ununsed;
+  ///   int function(void) __unused;
   ///   void only_writes_to_buffer(char *__output buffer);
   /// \endcode
   ///
   /// In the .clang-format configuration file, this can be configured like:
   /// \code{.yaml}
-  ///   AttributeMacros: ['__capability', '__output', '__ununsed']
+  ///   AttributeMacros: ['__capability', '__output', '__unused']
   /// \endcode
   ///
   /// \version 12
@@ -4043,7 +4043,7 @@
   /// AfterFunctionDefinitionName: true
   /// \endcode
   struct SpaceBeforeParensCustom {
-/// If ``true``, put space betwee control statement keywords
+/// If ``true``, put space between control statement keywords
 /// (for/if/while...) and opening parentheses.
 /// \code
 ///true:  false:
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1549,14 +1549,14 @@
   .. code-block:: c++
 
 x = (char *__capability)
-int function(void) __ununsed;
+int function(void) __unused;
 void only_writes_to_buffer(char *__output buffer);
 
   In the .clang-format configuration file, this can be configured like:
 
   .. code-block:: yaml
 
-AttributeMacros: ['__capability', '__output', '__ununsed']
+AttributeMacros: ['__capability', '__output', '__unused']
 
 .. _BinPackArguments:
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -971,13 +971,13 @@
   /// For example:
   /// \code
   ///   x = (char *__capability)
-  ///   int function(void) __ununsed;
+  ///   int function(void) __unused;
   ///   void only_writes_to_buffer(char *__output buffer);
   /// \endcode
   ///
   /// In the .clang-format configuration file, this can be configured like:
   /// \code{.yaml}
-  ///   AttributeMacros: ['__capability', '__output', '__ununsed']
+  ///   AttributeMacros: ['__capability', '__output', '__unused']
   /// \endcode
   ///
   /// \version 12
@@ -4043,7 +4043,7 @@
   /// AfterFunctionDefinitionName: true
   /// \endcode
   struct SpaceBeforeParensCustom {
-/// If ``true``, put space betwee control statement keywords
+/// If ``true``, put space between control statement keywords
 /// (for/if/while...) and opening parentheses.
 /// \code
 ///true:  false:
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1549,14 +1549,14 @@
   .. code-block:: c++
 
 x = (char *__capability)
-int function(void) __ununsed;
+int function(void) __unused;
 void only_writes_to_buffer(char *__output buffer);
 
   In the .clang-format configuration file, this can be configured like:
 
   .. code-block:: yaml
 
-AttributeMacros: ['__capability', '__output', '__ununsed']
+AttributeMacros: ['__capability', '__output', '__unused']
 
 .. _BinPackArguments:

[PATCH] D158695: [clang] Fix missing contract flag in sqrt intrinsic

2023-08-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: arsenm, rjmccall.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

Fix: https://github.com/llvm/llvm-project/issues/64653


https://reviews.llvm.org/D158695

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/fp-contract-fast-pragma.cpp


Index: clang/test/CodeGen/fp-contract-fast-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-fast-pragma.cpp
+++ clang/test/CodeGen/fp-contract-fast-pragma.cpp
@@ -5,8 +5,10 @@
 // CHECK: _Z13fp_contract_1fff
 // CHECK: %[[M:.+]] = fmul contract float %a, %b
 // CHECK-NEXT: fadd contract float %[[M]], %c
+// CHECK-NEXT: tail call contract float @llvm.sqrt.f32(float %a)
+
 #pragma clang fp contract(fast)
-  return a * b + c;
+  return a * b + c + __builtin_sqrtf(a);
 }
 
 // Is FP_CONTRACT state cleared on exiting compound statements?
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -495,6 +495,8 @@
 unsigned ConstrainedIntrinsicID) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(
+  CGF, E->getFPFeaturesInEffect(CGF.getLangOpts()));
   if (CGF.Builder.getIsFPConstrained()) {
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
 Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, 
Src0->getType());


Index: clang/test/CodeGen/fp-contract-fast-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-fast-pragma.cpp
+++ clang/test/CodeGen/fp-contract-fast-pragma.cpp
@@ -5,8 +5,10 @@
 // CHECK: _Z13fp_contract_1fff
 // CHECK: %[[M:.+]] = fmul contract float %a, %b
 // CHECK-NEXT: fadd contract float %[[M]], %c
+// CHECK-NEXT: tail call contract float @llvm.sqrt.f32(float %a)
+
 #pragma clang fp contract(fast)
-  return a * b + c;
+  return a * b + c + __builtin_sqrtf(a);
 }
 
 // Is FP_CONTRACT state cleared on exiting compound statements?
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -495,6 +495,8 @@
 unsigned ConstrainedIntrinsicID) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
 
+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(
+  CGF, E->getFPFeaturesInEffect(CGF.getLangOpts()));
   if (CGF.Builder.getIsFPConstrained()) {
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
 Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-23 Thread Ian Anderson via Phabricator via cfe-commits

iana updated this revision to Diff 552971.
iana added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

Files:
  clang/lib/Headers/stddef.h
  clang/test/Headers/stddef.c
  clang/test/Headers/stddefneeds.c

Index: clang/test/Headers/stddefneeds.c
===
--- /dev/null
+++ clang/test/Headers/stddefneeds.c
@@ -0,0 +1,169 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=c99 -std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c23 -std=c23 %s
+
+// Use C99 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+struct astruct { char member; };
+
+ptrdiff_t p0; // c99-error{{unknown type name 'ptrdiff_t'}} c23-error{{unknown type}}
+size_t s0; // c99-error{{unknown type name 'size_t'}} c23-error{{unknown type}}
+rsize_t r0; // c99-error{{unknown type name 'rsize_t'}} c23-error{{unknown type}}
+wchar_t wc0; // c99-error{{unknown type name 'wchar_t'}} c23-error{{unknown type}}
+void *v0 = NULL; // c99-error{{use of undeclared identifier 'NULL'}} c23-error{{undeclared identifier}}
+nullptr_t n0; // c99-error{{unknown type name 'nullptr_t'}} c23-error{{unknown type}}
+static void f0(void) { unreachable(); } // c99-error{{call to undeclared function 'unreachable'}} c23-error{{undeclared identifier 'unreachable'}}
+max_align_t m0; // c99-error{{unknown type name 'max_align_t'}} c23-error{{unknown type}}
+size_t o0 = offsetof(struct astruct, member); // c99-error{{unknown type name 'size_t'}} c99-error{{call to undeclared function 'offsetof'}} c99-error{{expected expression}} c99-error{{use of undeclared identifier 'member'}} \
+ c23-error{{unknown type name 'size_t'}} c23-error{{undeclared identifier 'offsetof'}} c23-error{{expected expression}} c23-error{{use of undeclared identifier 'member'}}
+wint_t wi0; // c99-error{{unknown type name 'wint_t'}} c23-error{{unknown type}}
+
+#define __need_ptrdiff_t
+#include 
+
+ptrdiff_t p1;
+size_t s1; // c99-error{{unknown type}} c23-error{{unknown type}}
+rsize_t r1; // c99-error{{unknown type}} c23-error{{unknown type}}
+wchar_t wc1; // c99-error{{unknown type}} c23-error{{unknown type}}
+void *v1 = NULL; // c99-error{{undeclared identifier}} c23-error{{undeclared identifier}}
+nullptr_t n1; // c99-error{{unknown type}} c23-error{{unknown type}}
+static void f1(void) { unreachable(); } // c99-error{{undeclared function}} c23-error{{undeclared identifier}}
+max_align_t m1; // c99-error{{unknown type}} c23-error{{unknown type}}
+size_t o1 = offsetof(struct astruct, member); // c99-error{{unknown type}} c99-error{{expected expression}} c99-error{{undeclared identifier}} \
+ c23-error{{unknown type}} c23-error{{undeclared identifier}} c23-error{{expected expression}} c23-error{{undeclared identifier}}
+wint_t wi1; // c99-error{{unknown type}} c23-error{{unknown type}}
+
+#define __need_size_t
+#include 
+
+ptrdiff_t p2;
+size_t s2;
+rsize_t r2; // c99-error{{unknown type}} c23-error{{unknown type}}
+// c99-note@stddef.h:*{{'size_t' declared here}} c23-note@stddef.h:*{{'size_t' declared here}}
+wchar_t wc2; // c99-error{{unknown type}} c23-error{{unknown type}}
+void *v2 = NULL; // c99-error{{undeclared identifier}} c23-error{{undeclared identifier}}
+nullptr_t n2; // c99-error{{unknown type}} c23-error{{unknown type}}
+static void f2(void) { unreachable(); } // c99-error{{undeclared function}} c23-error{{undeclared identifier}}
+max_align_t m2; // c99-error{{unknown type}} c23-error{{unknown type}}
+size_t o2 = offsetof(struct astruct, member); // c99-error{{expected expression}} c99-error{{undeclared identifier}} \
+ c23-error{{undeclared identifier}} c23-error{{expected expression}} c23-error{{undeclared identifier}}
+wint_t wi2; // c99-error{{unknown type}} c23-error{{unknown type}}
+
+#define __need_rsize_t
+#include 
+
+ptrdiff_t p3;
+size_t s3;
+rsize_t r3;
+wchar_t wc3; // c99-error{{unknown type}} c23-error{{unknown type}}
+void *v3 = NULL; // c99-error{{undeclared identifier}} c23-error{{undeclared identifier}}
+nullptr_t n3; // c99-error{{unknown type}} c23-error{{unknown type}}
+static void f3(void) { unreachable(); } // c99-error{{undeclared function}} c23-error{{undeclared identifier}}
+max_align_t m3; // c99-error{{unknown type}} c23-error{{unknown type}}
+size_t o3 = offsetof(struct astruct, member); // c99-error{{expected expression}} c99-error{{undeclared identifier}} \
+ c23-error{{undeclared identifier}} c23-error{{expected expression}} c23-error{{undeclared identifier}}
+wint_t wi3; // c99-error{{unknown type}} c23-error{{unknown type}}
+
+#define __need_wchar_t
+#include 
+
+ptrdiff_t p4;
+size_t s4;
+rsize_t r4;
+wchar_t wc4;
+void *v4 = NULL; //

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

2023-08-23 Thread sstwcw via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG16ccba51072b: [clang-format] Break long string literals in 
C#, etc. (authored by sstwcw).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTestCSharp.cpp
  clang/unittests/Format/FormatTestJS.cpp
  clang/unittests/Format/FormatTestJava.cpp
  clang/unittests/Format/FormatTestVerilog.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1815,6 +1815,30 @@
   EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[5], tok::comma, TT_Unknown);
   EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_Unknown);
+
+  // String literals in concatenation.
+  Tokens = Annotate("x = {\"\"};");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_StringInConcatenation);
+  Tokens = Annotate("x = {\"\", \"\"};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_StringInConcatenation);
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_StringInConcatenation);
+  Tokens = Annotate("x = '{{\"\"}};");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_StringInConcatenation);
+  // Cases where the string should not be annotated that type.  Fix the
+  // `TT_Unknown` if needed in the future.
+  Tokens = Annotate("x = {\"\" == \"\"};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::string_literal, TT_Unknown);
+  Tokens = Annotate("x = {(\"\")};");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::string_literal, TT_Unknown);
+  Tokens = Annotate("x = '{\"\"};");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::string_literal, TT_Unknown);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -1157,6 +1157,66 @@
   verifyFormat("{< Ranges(1, tooling::Range(Offset, Length));
-tooling::Replacements Replaces = reformat(Style, Code, Ranges);
-auto Result = applyAllReplacements(Code, Replaces);
-EXPECT_TRUE(static_cast(Result));
-LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
-return *Result;
-  }
-
-  static std::string
-  format(llvm::StringRef Code,
- const FormatStyle  = getGoogleStyle(FormatStyle::LK_Java)) {
-return format(Code, 0, Code.size(), Style);
+  FormatStyle getDefaultStyle() const override {
+return getGoogleStyle(FormatStyle::LK_Java);
   }
 
   static FormatStyle getStyleWithColumns(unsigned ColumnLimit) {
@@ -41,13 +26,6 @@
 Style.ColumnLimit = ColumnLimit;
 return Style;
   }
-
-  static void verifyFormat(
-  llvm::StringRef Code,
-  const FormatStyle  = getGoogleStyle(FormatStyle::LK_Java)) {
-EXPECT_EQ(Code.str(), format(Code, Style)) << "Expected code is not stable";
-EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
-  }
 };
 
 TEST_F(FormatTestJava, NoAlternativeOperatorNames) {
@@ -565,9 +543,9 @@
 }
 
 TEST_F(FormatTestJava, BreaksStringLiterals) {
-  // FIXME: String literal breaking is currently disabled for Java and JS, as it
-  // requires strings to be merged using "+" which we don't support.
-  verifyFormat("\"some text other\";", getStyleWithColumns(14));
+  verifyFormat("x = \"some text \"\n"
+   "+ \"other\";",
+   "x = \"some text other\";", getStyleWithColumns(18));
 }
 
 TEST_F(FormatTestJava, AlignsBlockComments) {
@@ -625,5 +603,7 @@
Style);
 }
 
+} // namespace
+} // namespace test
 } // namespace format
-} // end namespace clang
+} // namespace clang
Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -1505,6 +1505,97 @@
 TEST_F(FormatTestJS, StringLiteralConcatenation) {
   verifyFormat("var literal = 'hello ' +\n"
"'world';");
+
+  // Long strings should be broken.
+  verifyFormat("var literal =\n"
+

[PATCH] D154467: [clang-format] Add Verilog suffixes to the scripts

2023-08-23 Thread sstwcw via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG825cec2c0b1e: [clang-format] Add Verilog suffixes to the 
scripts (authored by sstwcw).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154467

Files:
  clang/tools/clang-format/clang-format-diff.py
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -98,6 +98,7 @@
   'ts',  # TypeScript
   'cs',  # C Sharp
   'json',  # Json
+  'sv', 'svh', 'v', 'vh', # Verilog
   ])
 
   p = argparse.ArgumentParser(
Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -61,8 +61,8 @@
 parser.add_argument(
 "-iregex",
 metavar="PATTERN",
-default=r".*\.(cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp|hxx"
-r"|m|mm|inc|js|ts|proto|protodevel|java|cs|json)",
+default=r".*\.(?:cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp"
+r"|hxx|m|mm|inc|js|ts|proto|protodevel|java|cs|json|s?vh?)",
 help="custom pattern selecting file paths to reformat "
 "(case insensitive, overridden by -regex)",
 )


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -98,6 +98,7 @@
   'ts',  # TypeScript
   'cs',  # C Sharp
   'json',  # Json
+  'sv', 'svh', 'v', 'vh', # Verilog
   ])
 
   p = argparse.ArgumentParser(
Index: clang/tools/clang-format/clang-format-diff.py
===
--- clang/tools/clang-format/clang-format-diff.py
+++ clang/tools/clang-format/clang-format-diff.py
@@ -61,8 +61,8 @@
 parser.add_argument(
 "-iregex",
 metavar="PATTERN",
-default=r".*\.(cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp|hxx"
-r"|m|mm|inc|js|ts|proto|protodevel|java|cs|json)",
+default=r".*\.(?:cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp"
+r"|hxx|m|mm|inc|js|ts|proto|protodevel|java|cs|json|s?vh?)",
 help="custom pattern selecting file paths to reformat "
 "(case insensitive, overridden by -regex)",
 )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 16ccba5 - [clang-format] Break long string literals in C#, etc.

2023-08-23 Thread via cfe-commits


Author: sstwcw
Date: 2023-08-24T03:16:31Z
New Revision: 16ccba51072bbc5ff4c66f91f939163dc91e5d96

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

LOG: [clang-format] Break long string literals in C#, etc.

Now strings that are too long for one line in C#, Java, JavaScript, and
Verilog get broken into several lines.  C# and JavaScript interpolated
strings are not broken.

A new subclass BreakableStringLiteralUsingOperators is used to handle
the logic for adding plus signs and commas.  The updateAfterBroken
method was added because now parentheses or braces may be required after
the parentheses or commas are added.  In order to decide whether the
added plus sign should be unindented in the BreakableToken object, the
logic for it is taken out into a separate function
shouldUnindentNextOperator.

The logic for finding the continuation indentation when the option
AlignAfterOpenBracket is set to DontAlign is not implemented yet.  So in
that case the new line may have the wrong indentation, and the parts may
have the wrong length if the string needs to be broken more than once
because finding where to break the string depends on where the string
starts.

The preambles for the C# and Java unit tests are changed to the newer
style in order to allow the 3-argument verifyFormat macro.  Some cases
are changed from verifyFormat to verifyImcompleteFormat because those
use incomplete code and the new verifyFormat function checks that the
code is complete.

The line in the doc was changed to being indented by 4 spaces, that is,
the default continuation indentation.  It has always been the case.  It
was probably a mistake that the doc showed 2 spaces previously.

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/BreakableToken.cpp
clang/lib/Format/BreakableToken.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTestCSharp.cpp
clang/unittests/Format/FormatTestJS.cpp
clang/unittests/Format/FormatTestJava.cpp
clang/unittests/Format/FormatTestVerilog.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 3b3f6f2860906a..871d7c63cdace9 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2722,6 +2722,8 @@ the configuration (without a prefix: ``Auto``).
 **BreakStringLiterals** (``Boolean``) :versionbadge:`clang-format 3.9` :ref:`¶ 
`
   Allow breaking string literals when formatting.
 
+  In C, C++, and Objective-C:
+
   .. code-block:: c++
 
  true:
@@ -2731,7 +2733,34 @@ the configuration (without a prefix: ``Auto``).
 
  false:
  const char* x =
-   "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
+ "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
+
+  In C#, Java, and JavaScript:
+
+  .. code-block:: c++
+
+ true:
+ var x = "veryVeryVeryVeryVeryVe" +
+ "ryVeryVeryVeryVeryVery" +
+ "VeryLongString";
+
+ false:
+ var x =
+ "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
+  C# and JavaScript interpolated strings are not broken.
+
+  In Verilog:
+
+  .. code-block:: c++
+
+ true:
+ string x = {"veryVeryVeryVeryVeryVe",
+ "ryVeryVeryVeryVeryVery",
+ "VeryLongString"};
+
+ false:
+ string x =
+ "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
 
 .. _ColumnLimit:
 

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 192cc68e51fad7..b7b25657927aa6 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2008,6 +2008,8 @@ struct FormatStyle {
   bool BreakAfterJavaFieldAnnotations;
 
   /// Allow breaking string literals when formatting.
+  ///
+  /// In C, C++, and Objective-C:
   /// \code
   ///true:
   ///const char* x = "veryVeryVeryVeryVeryVe"
@@ -2016,8 +2018,34 @@ struct FormatStyle {
   ///
   ///false:
   ///const char* x =
-  ///  "veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
+  ///"veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongString";
+  /// \endcode
+  ///
+  /// In C#, Java, and JavaScript:
+  /// \code
+  ///true:
+  ///var x = "veryVeryVeryVeryVeryVe" +
+  ///"ryVeryVeryVeryVeryVery" +
+  ///"VeryLongString";
+  ///
+  ///false:
+  ///var x =
+  ///

[clang] 825cec2 - [clang-format] Add Verilog suffixes to the scripts

2023-08-23 Thread via cfe-commits


Author: sstwcw
Date: 2023-08-24T03:06:45Z
New Revision: 825cec2c0b1ecec49995126674a8e58cd813476c

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

LOG: [clang-format] Add Verilog suffixes to the scripts

I decided not to wait for D149088 because it was taking a long time.

The capture group in the regexp was changed to non-capturing.  It
doesn't need to be captured.

Reviewed By: HazardyKnusperkeks, owenpan

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

Added: 


Modified: 
clang/tools/clang-format/clang-format-diff.py
clang/tools/clang-format/git-clang-format

Removed: 




diff  --git a/clang/tools/clang-format/clang-format-
diff .py b/clang/tools/clang-format/clang-format-
diff .py
index 6e707fc0fb6471a..324ef5b7f6b35f6 100755
--- a/clang/tools/clang-format/clang-format-
diff .py
+++ b/clang/tools/clang-format/clang-format-
diff .py
@@ -61,8 +61,8 @@ def main():
 parser.add_argument(
 "-iregex",
 metavar="PATTERN",
-default=r".*\.(cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp|hxx"
-r"|m|mm|inc|js|ts|proto|protodevel|java|cs|json)",
+default=r".*\.(?:cpp|cc|c\+\+|cxx|cppm|ccm|cxxm|c\+\+m|c|cl|h|hh|hpp"
+r"|hxx|m|mm|inc|js|ts|proto|protodevel|java|cs|json|s?vh?)",
 help="custom pattern selecting file paths to reformat "
 "(case insensitive, overridden by -regex)",
 )

diff  --git a/clang/tools/clang-format/git-clang-format 
b/clang/tools/clang-format/git-clang-format
index 695545d9e325522..c0b99b82203234c 100755
--- a/clang/tools/clang-format/git-clang-format
+++ b/clang/tools/clang-format/git-clang-format
@@ -98,6 +98,7 @@ def main():
   'ts',  # TypeScript
   'cs',  # C Sharp
   'json',  # Json
+  'sv', 'svh', 'v', 'vh', # Verilog
   ])
 
   p = argparse.ArgumentParser(



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

[PATCH] D158665: [clang-tidy] Improve cppcoreguidelines-avoid-reference-coroutine-parameters check

2023-08-23 Thread Chris Cotter via Phabricator via cfe-commits

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

Thanks!




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidReferenceCoroutineParametersCheck.cpp:20
+  Finder->addMatcher(
+  functionDecl(unless(parameterCountIs(0)), hasBody(coroutineBodyStmt()))
+  .bind("fnt"),

Can we do this by matching if any parameter is of reference type, rather than 
relying on the logic living in `check` (I wrote this originally, although 
perhaps this would be an improvement). Happy to take this up as a followup...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158665

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

2023-08-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/amdgpu-code-object-version-linking.cu:12
+// RUN: llvm-link %t_0 %t_5 -o -| llvm-dis -o - | FileCheck 
-check-prefix=LINKED5 %s
+
+#include "Inputs/cuda.h"

need to test using clang -cc1 with -O3 and -mlink-builtin-bitcode to link the 
device lib and verify the load of llvm.amdgcn.abi.version being eliminated 
after optimization.

I think currently it cannot do that since llvm.amdgcn.abi.version is not 
internalized by the internalization pass. This can cause some significant perf 
drops since loading is expensive. Need to tweak the function controlling what 
variables can be internalized for amdgpu so that this variable gets 
internalized, or having a generic way to tell that function which variables 
should be internalized, e.g. by adding a metadata amdgcn.internalize


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-08-23 Thread sstwcw via Phabricator via cfe-commits

sstwcw abandoned this revision.
sstwcw added a comment.

I am closing it since no one seems to like it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154091

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

[clang] 19ab266 - [NFC] [C++20] [Coroutines] Mention the side effect of a fix may bring regressions

2023-08-23 Thread Chuanqi Xu via cfe-commits


Author: Chuanqi Xu
Date: 2023-08-24T10:41:48+08:00
New Revision: 19ab2664ad3182ffa8fe3a95bb19765e4ae84653

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

LOG: [NFC] [C++20] [Coroutines] Mention the side effect of a fix may bring 
regressions

The fix we sent for https://github.com/llvm/llvm-project/issues/56301
may bring performance regressions. But we didn't mention it in the
ReleaseNotes so that users may get confused. e.g,
https://github.com/llvm/llvm-project/issues/64933. So this patch
mentions the possible side effect and the potential solutions in
https://github.com/llvm/llvm-project/issues/64945 to avoid
misunderstandings.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 157f57d73030ce..42b4afa455e38a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -163,6 +163,9 @@ Bug Fixes in This Version
   ``await_suspend`` could be misoptimized, including accesses to the awaiter
   object itself.
   (`#56301 `_)
+  The current solution may bring performance regressions if the awaiters have
+  non-static data members. See
+  `#64945 `_ for details.
 
 Bug Fixes to Compiler Builtins
 ^^



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

[PATCH] D155826: [HIP][Clang][Preprocessor][RFC] Add preprocessor support for C++ Parallel Algorithm Offload

2023-08-23 Thread Yaxun Liu via Phabricator via cfe-commits

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

LGTM. Thanks


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

https://reviews.llvm.org/D155826

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

[PATCH] D158690: [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI pipeline

2023-08-23 Thread Louis Dionne via Phabricator via cfe-commits

ldionne added a comment.

The follow-up to the temporary workaround is going to be done tomorrow in 
D158694 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158690

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

[PATCH] D158690: [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI pipeline

2023-08-23 Thread Louis Dionne via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbb6073c7e32b: [clang] Fix the pre-commit CI pipeline after 
changes to libc++s own CI pipeline (authored by ldionne).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158690

Files:
  clang/utils/ci/run-buildbot


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -60,6 +60,10 @@
 BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
 INSTALL_DIR="${BUILD_DIR}/install"
 
+function clean() {
+rm -rf "${BUILD_DIR}"
+}
+
 # Print the version of a few tools to aid diagnostics in some cases
 cmake --version
 ninja --version
@@ -95,7 +99,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx03
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++03" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++03"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-cxx26)
 buildkite-agent artifact download install.tar.xz .
@@ -103,7 +117,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx26
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++26" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++26"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-modules)
 buildkite-agent artifact download install.tar.xz .
@@ -111,7 +135,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-modules
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="enable_modules=clang" \
+  -DLIBCXXABI_TEST_PARAMS="enable_modules=clang"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 #
 # Insert vendor-specific internal configurations below.


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -60,6 +60,10 @@
 BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
 INSTALL_DIR="${BUILD_DIR}/install"
 
+function clean() {
+rm -rf "${BUILD_DIR}"
+}
+
 # Print the version of a few tools to aid diagnostics in some cases
 cmake --version
 ninja --version
@@ -95,7 +99,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx03
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++03" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++03"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-cxx26)
 buildkite-agent artifact download install.tar.xz .
@@ -103,7 +117,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx26
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++26" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++26"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-modules)
 buildkite-agent artifact download install.tar.xz .
@@ -111,7 +135,17 @@
 export

[clang] bb6073c - [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI pipeline

2023-08-23 Thread Louis Dionne via cfe-commits


Author: Louis Dionne
Date: 2023-08-23T22:21:53-04:00
New Revision: bb6073c7e32b5062a6969242c6bd4fe859e2e5e0

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

LOG: [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI 
pipeline

We made some changes to the libc++ CI pipeline that take for granted
that we're running on libc++'s own Docker images. This was necessary for
a temporary period until widely-used tools update to a version that can
handle C++20 modules.

However, this had the unintended consequence of breaking the Clang CI
pipeline, which used the libc++ CI scripts as an implementation detail.
Instead, decouple the Clang CI pipeline from the libc++ build scripts.

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

Added: 


Modified: 
clang/utils/ci/run-buildbot

Removed: 




diff  --git a/clang/utils/ci/run-buildbot b/clang/utils/ci/run-buildbot
index ef3909a5babc9d..91246f1d429736 100755
--- a/clang/utils/ci/run-buildbot
+++ b/clang/utils/ci/run-buildbot
@@ -60,6 +60,10 @@ MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse 
--show-toplevel)"}"
 BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
 INSTALL_DIR="${BUILD_DIR}/install"
 
+function clean() {
+rm -rf "${BUILD_DIR}"
+}
+
 # Print the version of a few tools to aid diagnostics in some cases
 cmake --version
 ninja --version
@@ -95,7 +99,17 @@ generic-cxx03)
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx03
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++03" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++03"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-cxx26)
 buildkite-agent artifact download install.tar.xz .
@@ -103,7 +117,17 @@ generic-cxx26)
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx26
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++26" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++26"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-modules)
 buildkite-agent artifact download install.tar.xz .
@@ -111,7 +135,17 @@ generic-modules)
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-modules
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="enable_modules=clang" \
+  -DLIBCXXABI_TEST_PARAMS="enable_modules=clang"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 #
 # Insert vendor-specific internal configurations below.



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

[PATCH] D158690: [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI pipeline

2023-08-23 Thread Louis Dionne via Phabricator via cfe-commits

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

Along with aa60b2687ce3b67e454ba7c8c58a8688247df0b8 
, this 
will make the Clang CI green again. We'll figure out what's the underlying 
cause for aa60b2687ce3b67e454ba7c8c58a8688247df0b8 
 tomorrow, 
it's too late for today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158690

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

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

2023-08-23 Thread Kai Luo via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ceaec3e8104: [PowerPC][altivec] Optimize codegen of 
vec_promote (authored by lkail).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c
  llvm/test/CodeGen/PowerPC/vec-promote.ll

Index: llvm/test/CodeGen/PowerPC/vec-promote.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/vec-promote.ll
@@ -0,0 +1,276 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -mtriple=powerpc64-unknown-unknown -verify-machineinstrs -mcpu=pwr8 \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-BE
+; RUN: llc -mtriple=powerpc64le-unknown-unknown -verify-machineinstrs -mcpu=pwr8 \
+; RUN:   < %s | FileCheck %s --check-prefix=CHECK-LE
+
+define noundef <2 x double> @vec_promote_double_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_double_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfd 0, 0(3)
+; CHECK-BE-NEXT:xxlxor 1, 1, 1
+; CHECK-BE-NEXT:xxmrghd 34, 0, 1
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_double_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfd 0, 0(3)
+; CHECK-LE-NEXT:xxlxor 1, 1, 1
+; CHECK-LE-NEXT:xxmrghd 34, 1, 0
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load double, ptr %p, align 8
+  %vecins.i = insertelement <2 x double> , double %0, i64 0
+  ret <2 x double> %vecins.i
+}
+
+define noundef <2 x double> @vec_promote_double(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_double:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lxvdsx 34, 0, 3
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_double:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lxvdsx 34, 0, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load double, ptr %p, align 8
+  %vecins.i = insertelement <2 x double> poison, double %0, i64 0
+  ret <2 x double> %vecins.i
+}
+
+define noundef <4 x float> @vec_promote_float_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_float_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfs 1, 0(3)
+; CHECK-BE-NEXT:xxlxor 0, 0, 0
+; CHECK-BE-NEXT:xxspltd 2, 0, 0
+; CHECK-BE-NEXT:xxmrghd 0, 1, 0
+; CHECK-BE-NEXT:xvcvdpsp 34, 2
+; CHECK-BE-NEXT:xvcvdpsp 35, 0
+; CHECK-BE-NEXT:vmrgew 2, 3, 2
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_float_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfs 1, 0(3)
+; CHECK-LE-NEXT:xxlxor 0, 0, 0
+; CHECK-LE-NEXT:xxspltd 2, 0, 0
+; CHECK-LE-NEXT:xxmrghd 0, 0, 1
+; CHECK-LE-NEXT:xvcvdpsp 34, 2
+; CHECK-LE-NEXT:xvcvdpsp 35, 0
+; CHECK-LE-NEXT:vmrgew 2, 2, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load float, ptr %p, align 8
+  %vecins.i = insertelement <4 x float> , float %0, i64 0
+  ret <4 x float> %vecins.i
+}
+
+define noundef <4 x float> @vec_promote_float(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_float:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lfiwzx 0, 0, 3
+; CHECK-BE-NEXT:xxspltw 34, 0, 1
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_float:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lfiwzx 0, 0, 3
+; CHECK-LE-NEXT:xxspltw 34, 0, 1
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load float, ptr %p, align 8
+  %vecins.i = insertelement <4 x float> poison, float %0, i64 0
+  ret <4 x float> %vecins.i
+}
+
+define noundef <2 x i64> @vec_promote_long_long_zeroed(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_long_long_zeroed:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:ld 3, 0(3)
+; CHECK-BE-NEXT:li 4, 0
+; CHECK-BE-NEXT:mtfprd 0, 4
+; CHECK-BE-NEXT:mtfprd 1, 3
+; CHECK-BE-NEXT:xxmrghd 34, 1, 0
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_long_long_zeroed:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:ld 3, 0(3)
+; CHECK-LE-NEXT:li 4, 0
+; CHECK-LE-NEXT:mtfprd 0, 4
+; CHECK-LE-NEXT:mtfprd 1, 3
+; CHECK-LE-NEXT:xxmrghd 34, 0, 1
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load i64, ptr %p, align 8
+  %vecins.i = insertelement <2 x i64> , i64 %0, i64 0
+  ret <2 x i64> %vecins.i
+}
+
+define noundef <2 x i64> @vec_promote_long_long(ptr nocapture noundef readonly %p) {
+; CHECK-BE-LABEL: vec_promote_long_long:
+; CHECK-BE:   # %bb.0: # %entry
+; CHECK-BE-NEXT:lxvdsx 34, 0, 3
+; CHECK-BE-NEXT:blr
+;
+; CHECK-LE-LABEL: vec_promote_long_long:
+; CHECK-LE:   # %bb.0: # %entry
+; CHECK-LE-NEXT:lxvdsx 34, 0, 3
+; CHECK-LE-NEXT:blr
+entry:
+  %0 = load i64, ptr %p, align 8
+  %vecins.i = insertelement

[clang] 1ceaec3 - [PowerPC][altivec] Optimize codegen of vec_promote

2023-08-23 Thread Kai Luo via cfe-commits


Author: Kai Luo
Date: 2023-08-24T02:10:13Z
New Revision: 1ceaec3e81044d8a671b28d1f556045cf7fe6ef0

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

LOG: [PowerPC][altivec] Optimize codegen of vec_promote

According to 
https://www.ibm.com/docs/en/xl-c-and-cpp-linux/16.1.1?topic=functions-vec-promote,
 elements not specified by the input index argument are undefined. So that we 
don't need to set these elements to be zeros.

Reviewed By: nemanjai, #powerpc

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

Added: 
llvm/test/CodeGen/PowerPC/vec-promote.ll

Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/PowerPC/builtins-ppc-vsx.c

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 44b5a24de89f1a..4971631c50f412 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -14647,67 +14647,86 @@ static __inline__ void __ATTRS_o_ai vec_stvrxl(vector 
float __a, int __b,
 
 static __inline__ vector signed char __ATTRS_o_ai vec_promote(signed char __a,
   int __b) {
-  vector signed char __res = (vector signed char)(0);
+  const vector signed char __zero = (vector signed char)0;
+  vector signed char __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1);
   __res[__b & 0xf] = __a;
   return __res;
 }
 
 static __inline__ vector unsigned char __ATTRS_o_ai
 vec_promote(unsigned char __a, int __b) {
-  vector unsigned char __res = (vector unsigned char)(0);
+  const vector unsigned char __zero = (vector unsigned char)(0);
+  vector unsigned char __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1,
+  -1, -1, -1, -1, -1, -1, -1, -1);
   __res[__b & 0xf] = __a;
   return __res;
 }
 
 static __inline__ vector short __ATTRS_o_ai vec_promote(short __a, int __b) {
-  vector short __res = (vector short)(0);
+  const vector short __zero = (vector short)(0);
+  vector short __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1);
   __res[__b & 0x7] = __a;
   return __res;
 }
 
 static __inline__ vector unsigned short __ATTRS_o_ai
 vec_promote(unsigned short __a, int __b) {
-  vector unsigned short __res = (vector unsigned short)(0);
+  const vector unsigned short __zero = (vector unsigned short)(0);
+  vector unsigned short __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1, -1, -1, -1, -1);
   __res[__b & 0x7] = __a;
   return __res;
 }
 
 static __inline__ vector int __ATTRS_o_ai vec_promote(int __a, int __b) {
-  vector int __res = (vector int)(0);
+  const vector int __zero = (vector int)(0);
+  vector int __res = __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1);
   __res[__b & 0x3] = __a;
   return __res;
 }
 
 static __inline__ vector unsigned int __ATTRS_o_ai vec_promote(unsigned int 
__a,
int __b) {
-  vector unsigned int __res = (vector unsigned int)(0);
+  const vector unsigned int __zero = (vector unsigned int)(0);
+  vector unsigned int __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1);
   __res[__b & 0x3] = __a;
   return __res;
 }
 
 static __inline__ vector float __ATTRS_o_ai vec_promote(float __a, int __b) {
-  vector float __res = (vector float)(0);
+  const vector float __zero = (vector float)(0);
+  vector float __res = __builtin_shufflevector(__zero, __zero, -1, -1, -1, -1);
   __res[__b & 0x3] = __a;
   return __res;
 }
 
 #ifdef __VSX__
 static __inline__ vector double __ATTRS_o_ai vec_promote(double __a, int __b) {
-  vector double __res = (vector double)(0);
+  const vector double __zero = (vector double)(0);
+  vector double __res = __builtin_shufflevector(__zero, __zero, -1, -1);
   __res[__b & 0x1] = __a;
   return __res;
 }
 
 static __inline__ vector signed long long __ATTRS_o_ai
 vec_promote(signed long long __a, int __b) {
-  vector signed long long __res = (vector signed long long)(0);
+  const vector signed long long __zero = (vector signed long long)(0);
+  vector signed long long __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1);
   __res[__b & 0x1] = __a;
   return __res;
 }
 
 static __inline__ vector unsigned long long __ATTRS_o_ai
 vec_promote(unsigned long long __a, int __b) {
-  vector unsigned long long __res = (vector unsigned long long)(0);
+  const vector unsigned long long __zero = (vector unsigned long long)(0);
+  vector unsigned long long __res =
+  __builtin_shufflevector(__zero, __zero, -1, -1);
   __res[__b & 0x1] = __a;
   return __res;
 }

diff  --git

[PATCH] D155826: [HIP][Clang][Preprocessor][RFC] Add preprocessor support for C++ Parallel Algorithm Offload

2023-08-23 Thread Alex Voicu via Phabricator via cfe-commits

AlexVlx updated this revision to Diff 552952.
AlexVlx added a comment.

Define the interpose macro iff in interpose mode.


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

https://reviews.llvm.org/D155826

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/predefined-macros.c


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -290,3 +290,20 @@
 // RUN:   -fcuda-is-device -fgpu-default-stream=per-thread \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-PTH
 // CHECK-PTH: #define HIP_API_PER_THREAD_DEFAULT_STREAM 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -triple 
x86_64-unknown-linux-gnu \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIPSTDPAR
+// CHECK-HIPSTDPAR: #define __HIPSTDPAR__ 1
+// CHECK-HIPSTDPAR-NOT: #define __HIPSTDPAR_INTERPOSE_ALLOC__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -hipstdpar-interpose-alloc 
\
+// RUN:  -triple x86_64-unknown-linux-gnu | FileCheck -match-full-lines %s \
+// RUN:  --check-prefix=CHECK-HIPSTDPAR-INTERPOSE
+// CHECK-HIPSTDPAR-INTERPOSE: #define __HIPSTDPAR_INTERPOSE_ALLOC__ 1
+// CHECK-HIPSTDPAR-INTERPOSE: #define __HIPSTDPAR__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -hipstdpar-interpose-alloc 
\
+// RUN:  -triple amdgcn-amd-amdhsa -fcuda-is-device | FileCheck 
-match-full-lines \
+// RUN:  %s --check-prefix=CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG
+// CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG: #define __HIPSTDPAR__ 1
+// CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG-NOT: #define 
__HIPSTDPAR_INTERPOSE_ALLOC__ 1
\ No newline at end of file
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -585,6 +585,11 @@
 Builder.defineMacro("__HIP_MEMORY_SCOPE_WORKGROUP", "3");
 Builder.defineMacro("__HIP_MEMORY_SCOPE_AGENT", "4");
 Builder.defineMacro("__HIP_MEMORY_SCOPE_SYSTEM", "5");
+if (LangOpts.HIPStdPar) {
+  Builder.defineMacro("__HIPSTDPAR__");
+  if (LangOpts.HIPStdParInterposeAlloc)
+Builder.defineMacro("__HIPSTDPAR_INTERPOSE_ALLOC__");
+}
 if (LangOpts.CUDAIsDevice) {
   Builder.defineMacro("__HIP_DEVICE_COMPILE__");
   if (!TI.hasHIPImageSupport()) {


Index: clang/test/Preprocessor/predefined-macros.c
===
--- clang/test/Preprocessor/predefined-macros.c
+++ clang/test/Preprocessor/predefined-macros.c
@@ -290,3 +290,20 @@
 // RUN:   -fcuda-is-device -fgpu-default-stream=per-thread \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-PTH
 // CHECK-PTH: #define HIP_API_PER_THREAD_DEFAULT_STREAM 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -triple x86_64-unknown-linux-gnu \
+// RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIPSTDPAR
+// CHECK-HIPSTDPAR: #define __HIPSTDPAR__ 1
+// CHECK-HIPSTDPAR-NOT: #define __HIPSTDPAR_INTERPOSE_ALLOC__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -hipstdpar-interpose-alloc \
+// RUN:  -triple x86_64-unknown-linux-gnu | FileCheck -match-full-lines %s \
+// RUN:  --check-prefix=CHECK-HIPSTDPAR-INTERPOSE
+// CHECK-HIPSTDPAR-INTERPOSE: #define __HIPSTDPAR_INTERPOSE_ALLOC__ 1
+// CHECK-HIPSTDPAR-INTERPOSE: #define __HIPSTDPAR__ 1
+
+// RUN: %clang_cc1 %s -E -dM -o - -x hip -hipstdpar -hipstdpar-interpose-alloc \
+// RUN:  -triple amdgcn-amd-amdhsa -fcuda-is-device | FileCheck -match-full-lines \
+// RUN:  %s --check-prefix=CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG
+// CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG: #define __HIPSTDPAR__ 1
+// CHECK-HIPSTDPAR-INTERPOSE-DEV-NEG-NOT: #define __HIPSTDPAR_INTERPOSE_ALLOC__ 1
\ No newline at end of file
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -585,6 +585,11 @@
 Builder.defineMacro("__HIP_MEMORY_SCOPE_WORKGROUP", "3");
 Builder.defineMacro("__HIP_MEMORY_SCOPE_AGENT", "4");
 Builder.defineMacro("__HIP_MEMORY_SCOPE_SYSTEM", "5");
+if (LangOpts.HIPStdPar) {
+  Builder.defineMacro("__HIPSTDPAR__");
+  if (LangOpts.HIPStdParInterposeAlloc)
+Builder.defineMacro("__HIPSTDPAR_INTERPOSE_ALLOC__");
+}
 if (LangOpts.CUDAIsDevice) {
   Builder.defineMacro("__HIP_DEVICE_COMPILE__");
   if (!TI.hasHIPImageSupport()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

2023-08-23 Thread Kai Luo via Phabricator via cfe-commits

lkail added a comment.

> but it might be a good idea to improve codegen for the insert which might be 
> more common.

Yes. Usually `insertelement` is transformed to `BUILD_VECTOR` is SDAG, 
currently we don't have much optimization for `BUILD_VECTOR` in special 
patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

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

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

2023-08-23 Thread Yeoul Na via Phabricator via cfe-commits

rapidsna added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17931
+  auto It = llvm::find_if(RD->fields(), [&](const FieldDecl *Field) {
+return Field->getName() == ECA->getCountedByField()->getName();
+  });

What happens in this corner case where the `Field` is actually the field that 
the attribute appertains to?
```
struct foo {
  int count;
  int counted[__attribute__((counted_by(counted)))];
};

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

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

[PATCH] D158691: [clang-tidy] Container-size-empty fixed c++ version in tests to support string_literals operator

2023-08-23 Thread Félix-Antoine Constantin via Phabricator via cfe-commits

felix642 created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a project: All.
felix642 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158691

Files:
  
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
readability-container-size-empty %t -- \
 // RUN: -config="{CheckOptions: 
{readability-container-size-empty.ExcludedComparisonTypes: 
'::std::array;::IgnoredDummyType'}}" \
 // RUN: -- -fno-delayed-template-parsing -isystem %clang_tidy_headers
 #include 
@@ -23,7 +23,7 @@
 }
 
 namespace string_literals{
-string operator""_s(const char *, size_t);
+string operator""s(const char *, size_t);
 }
 
 }
@@ -778,7 +778,7 @@
 bool testStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == ""_s;
+  return s == ""s;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be 
used
   // CHECK-FIXES: {{^  }}return s.empty()
 }
@@ -786,5 +786,5 @@
 bool testNotEmptyStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == "foo"_s;
+  return s == "foo"s;
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-container-size-empty %t -- \
+// RUN: %check_clang_tidy -std=c++14-or-later %s readability-container-size-empty %t -- \
 // RUN: -config="{CheckOptions: {readability-container-size-empty.ExcludedComparisonTypes: '::std::array;::IgnoredDummyType'}}" \
 // RUN: -- -fno-delayed-template-parsing -isystem %clang_tidy_headers
 #include 
@@ -23,7 +23,7 @@
 }
 
 namespace string_literals{
-string operator""_s(const char *, size_t);
+string operator""s(const char *, size_t);
 }
 
 }
@@ -778,7 +778,7 @@
 bool testStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == ""_s;
+  return s == ""s;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}return s.empty()
 }
@@ -786,5 +786,5 @@
 bool testNotEmptyStringLiterals(const std::string& s)
 {
   using namespace std::string_literals;
-  return s == "foo"_s;
+  return s == "foo"s;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158690: [clang] Fix the pre-commit CI pipeline after changes to libc++'s own CI pipeline

2023-08-23 Thread Louis Dionne via Phabricator via cfe-commits

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

We made some changes to the libc++ CI pipeline that take for granted
that we're running on libc++'s own Docker images. This was necessary for
a temporary period until widely-used tools update to a version that can
handle C++20 modules.

However, this had the unintended consequence of breaking the Clang CI
pipeline, which used the libc++ CI scripts as an implementation detail.
Instead, decouple the Clang CI pipeline from the libc++ build scripts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158690

Files:
  clang/utils/ci/run-buildbot


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -60,6 +60,10 @@
 BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
 INSTALL_DIR="${BUILD_DIR}/install"
 
+function clean() {
+rm -rf "${BUILD_DIR}"
+}
+
 # Print the version of a few tools to aid diagnostics in some cases
 cmake --version
 ninja --version
@@ -95,7 +99,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx03
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++03" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++03"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-cxx26)
 buildkite-agent artifact download install.tar.xz .
@@ -103,7 +117,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx26
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++26" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++26"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-modules)
 buildkite-agent artifact download install.tar.xz .
@@ -111,7 +135,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-modules
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="enable_modules=clang" \
+  -DLIBCXXABI_TEST_PARAMS="enable_modules=clang"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 #
 # Insert vendor-specific internal configurations below.


Index: clang/utils/ci/run-buildbot
===
--- clang/utils/ci/run-buildbot
+++ clang/utils/ci/run-buildbot
@@ -60,6 +60,10 @@
 BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
 INSTALL_DIR="${BUILD_DIR}/install"
 
+function clean() {
+rm -rf "${BUILD_DIR}"
+}
+
 # Print the version of a few tools to aid diagnostics in some cases
 cmake --version
 ninja --version
@@ -95,7 +99,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx03
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
+  -DLIBCXX_TEST_PARAMS="std=c++03" \
+  -DLIBCXXABI_TEST_PARAMS="std=c++03"
+
+ninja -vC "${BUILD_DIR}" check-runtimes
 ;;
 generic-cxx26)
 buildkite-agent artifact download install.tar.xz .
@@ -103,7 +117,17 @@
 export CC=$(pwd)/install/bin/clang
 export CXX=$(pwd)/install/bin/clang++
 chmod +x install/bin/clang install/bin/clang++
-libcxx/utils/ci/run-buildbot generic-cxx26
+
+clean
+cmake -S "${MONOREPO_ROOT}/runtimes" -B "${BUILD_DIR}" -GNinja \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
+  -DLIBCXX_CXX_ABI=libcxxabi \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

2023-08-23 Thread Yeoul Na via Phabricator via cfe-commits

rapidsna added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:7209
+/* ... */
+struct bar *fam[] __attribute__((counted_by(num_fam_elements)));
+  };

I don't think it's necessary for this patch but for [[ 
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/113?u=rapidsna
 | -fbounds-safety ]], we'd eventually turn this into a type attribute that we 
will use to create a sugared type or qualified type. And we'd make the 
attribute work for **both** this position and inside the array bracket (e.g., 
`fam[__counted_by(n)]`). Would it work for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

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

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

2023-08-23 Thread Chuanqi Xu via Phabricator via cfe-commits

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158601

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

[PATCH] D158562: [clang][Sema] Add truncation warning on fortified snprintf

2023-08-23 Thread Takuya Shimizu via Phabricator via cfe-commits

hazohelet added a comment.

Gcc can diagnose wider cases of overflow/truncation by specifying a higher 
level for it, like `-Wformat-overflow=2` (https://godbolt.org/z/n5facjW1c).
 The current clang counterpart only diagnoses when the format string is 
//always// larger than the buffer size. If we are going to implement more 
general warnings like gcc's, I think we should separate these counterparts into 
their own warning flags (`-Wformat-overflow/truncation`).

Comment at: clang/lib/Sema/SemaChecking.cpp:1134
   };
+  auto ProcessFormatStringLiteral =
+  [&](const Expr *FormatExpr, StringRef , size_t ) {

nickdesaulniers wrote:
> naive question, so will we create a lambda with capture even though a few 
> cases in the switch below will never use it?
I made it a static function.

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

https://reviews.llvm.org/D158562

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

[PATCH] D158688: [Driver,ARM,AArch64] Ignore -mbranch-protection= diagnostics for assembler input

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay created this revision.
MaskRay added reviewers: chill, peter.smith, simon_tatham, stuij.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some options are only claimed in AddARMTargetArgs/AddAArch64TargetArgs/etc 
(called by
Clang::RenderTargetOptions).
For assembler input, `Add*TargetArgs` is not called. If an option is
unclaimed, it either leads to a -Wunused-command-line-argument warning
or an error (if `TargetSpecific` is set)

  // clang '-###' --target=aarch64 -mbranch-protection=bti -c a.s
  clang: error: unsupported option '-mbranch-protection=' for target 'aarch64'

It seems that ignoring the diagnostics is most useful as it matches GCC
and users can do

  clang --target=aarch64 -mbranch-protection=bti -S a.c
  clang --target=aarch64 -mbranch-protection=bti -c a.s

without changing the options.

Planned for main and release/17.x


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158688

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/aarch64-security-options.c
  clang/test/Driver/arm-security-options.c


Index: clang/test/Driver/arm-security-options.c
===
--- clang/test/Driver/arm-security-options.c
+++ clang/test/Driver/arm-security-options.c
@@ -75,6 +75,9 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv7-r -c %s -### 
-mbranch-protection=bti 2>&1 | \
 // RUN: FileCheck %s --check-prefix=INCOMPATIBLE-ARCH
 
+/// Don't warn for assembler input.
+// RUN: %clang -### -Werror --target=arm-arm-none-eabi -march=armv7-m -x 
assembler -c %s -mbranch-protection=pac-ret
+
 // RA-OFF: "-msign-return-address=none"
 // RA-NON-LEAF: "-msign-return-address=non-leaf"
 // RA-ALL: "-msign-return-address=all"
Index: clang/test/Driver/aarch64-security-options.c
===
--- clang/test/Driver/aarch64-security-options.c
+++ clang/test/Driver/aarch64-security-options.c
@@ -27,8 +27,12 @@
 // RUN: not %clang --target=aarch64 -c %s -### -mbranch-protection=bar 
2>&1 | \
 // RUN: FileCheck %s --check-prefix=BAD-BP-PROTECTION --check-prefix=WARN
 
-// RUN: %clang --target=aarch64 -### -o /dev/null -mbranch-protection=standard 
/dev/null 2>&1 | \
-// RUN: FileCheck --allow-empty %s --check-prefix=LINKER-DRIVER
+/// Check that the linker driver doesn't warn about 
-mbranch-protection=standard
+/// as an unused option.
+// RUN: %clang --target=aarch64 -Werror -### -o /dev/null 
-mbranch-protection=standard /dev/null
+
+/// Don't warn for assembler input.
+// RUN: %clang -### --target=aarch64 -Werror -c -x assembler %s 
-mbranch-protection=standard -msign-return-address=all
 
 // WARN-NOT: warning: ignoring '-mbranch-protection=' option because the 
'aarch64' architecture does not support it [-Wbranch-protection]
 
@@ -49,7 +53,3 @@
 
 // BAD-B-KEY-COMBINATION: unsupported argument 'b-key' to option 
'-mbranch-protection='
 // BAD-LEAF-COMBINATION: unsupported argument 'leaf' to option 
'-mbranch-protection='
-
-// Check that the linker driver doesn't warn about -mbranch-protection=standard
-// as an unused option.
-// LINKER-DRIVER-NOT: warning: argument unused during compilation: 
'-mbranch-protection=standard'
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -539,6 +539,10 @@
   }
 }
 
+// Some target-specific options are only handled in AddARMTargetArgs, not
+// called for assembler input. Claim them.
+Args.claimAllArgs(options::OPT_mbranch_protection_EQ);
+
 // The integrated assembler doesn't implement e_flags setting behavior for
 // -meabi=gnu (gcc -mabi={apcs-gnu,atpcs} passes -meabi=gnu to gas). For
 // compatibility we accept but warn.
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -259,12 +259,18 @@
   // Enable NEON by default.
   Features.push_back("+neon");
   llvm::StringRef WaMArch;
-  if (ForAS)
+  if (ForAS) {
+// Some target-specific options are only handled in AddAArch64TargetArgs,
+// not called for assembler input. Claim them.
+Args.claimAllArgs(options::OPT_mbranch_protection_EQ,
+options::OPT_msign_return_address_EQ);
+
 for (const auto *A :
  Args.filtered(options::OPT_Wa_COMMA, options::OPT_Xassembler))
   for (StringRef Value : A->getValues())
 if (Value.startswith("-march="))
   WaMArch = Value.substr(7);
+  }
   // Call getAArch64ArchFeaturesFromMarch only if "-Wa,-march=" or
   // "-Xassembler -march"

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

2023-08-23 Thread Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1654634f6f8b: [clang][ASTImporter] Fix partially import of 
repeated friend templates (authored by dingfei fd...@feysh.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4054,6 +4054,25 @@
  ->lookup(ToRecordOfFriend->getDeclName())
  .empty());
   }
+
+  void testRepeatedFriendImport(const char *Code) {
+Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
+Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
+
+auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+auto *FromFriend1 =
+FirstDeclMatcher().match(FromTu, friendDecl());
+auto *FromFriend2 =
+LastDeclMatcher().match(FromTu, friendDecl());
+
+FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
+FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+
+EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+EXPECT_EQ(ToFriend1, ToImportedFriend1);
+EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  }
 };
 
 TEST_P(ImportFriendClasses, ImportOfFriendRecordDoesNotMergeDefinition) {
@@ -4395,21 +4414,7 @@
 friend class X;
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
-
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
-
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
@@ -4420,21 +4425,31 @@
 friend void f();
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+  testRepeatedFriendImport(Code);
+}
 
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend void m();
+  template  friend void m();
+};
+  )";
+  testRepeatedFriendImport(Code);
+}
 
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend class X;
+  template  friend class X;
+};
+  )";
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4079,22 +4079,34 @@
   unsigned int IndexOfDecl;
 };
 
-template 
-static FriendCountAndPosition getFriendCountAndPosition(
-const FriendDecl *FD,
-llvm::function_ref GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter , FriendDecl *FD1,
+   FriendDecl *FD2) {
+  if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+return false;
+
+  if (const TypeSourceInfo *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(
+TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+  ASTImporter::NonEquivalentDeclSet NonEquivalentDecls;
+  StructuralEquivalenceContext Ctx(
+  FD1->getASTContext(), FD2->getASTContext(), NonEquivalentDecls,
+  StructuralEquivalenceKind::Default,
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
+}
+

[clang] 1654634 - [clang][ASTImporter] Fix partially import of repeated friend templates

2023-08-23 Thread via cfe-commits


Author: dingfei
Date: 2023-08-24T09:10:14+08:00
New Revision: 1654634f6f8bc9a78c6c47f58e4f067fa9a7faef

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

LOG: [clang][ASTImporter] Fix partially import of repeated friend templates

Pointer comparison is not reliable in this case. Use ASTStructuralEquivalence
to compute FriendCountAndPosition.

Reviewed By: balazske

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 5ef8b6a5c50ee9..77be3b3098cabd 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -4079,22 +4079,34 @@ struct FriendCountAndPosition {
   unsigned int IndexOfDecl;
 };
 
-template 
-static FriendCountAndPosition getFriendCountAndPosition(
-const FriendDecl *FD,
-llvm::function_ref GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter , FriendDecl *FD1,
+   FriendDecl *FD2) {
+  if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+return false;
+
+  if (const TypeSourceInfo *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(
+TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+  ASTImporter::NonEquivalentDeclSet NonEquivalentDecls;
+  StructuralEquivalenceContext Ctx(
+  FD1->getASTContext(), FD2->getASTContext(), NonEquivalentDecls,
+  StructuralEquivalenceKind::Default,
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
+}
+
+static FriendCountAndPosition getFriendCountAndPosition(ASTImporter ,
+FriendDecl *FD) {
   unsigned int FriendCount = 0;
   std::optional FriendPosition;
   const auto *RD = cast(FD->getLexicalDeclContext());
 
-  T TypeOrDecl = GetCanTypeOrDecl(FD);
-
-  for (const FriendDecl *FoundFriend : RD->friends()) {
+  for (FriendDecl *FoundFriend : RD->friends()) {
 if (FoundFriend == FD) {
   FriendPosition = FriendCount;
   ++FriendCount;
-} else if (!FoundFriend->getFriendDecl() == !FD->getFriendDecl() &&
-   GetCanTypeOrDecl(FoundFriend) == TypeOrDecl) {
+} else if (IsEquivalentFriend(Importer, FD, FoundFriend)) {
   ++FriendCount;
 }
   }
@@ -4104,21 +4116,6 @@ static FriendCountAndPosition getFriendCountAndPosition(
   return {FriendCount, *FriendPosition};
 }
 
-static FriendCountAndPosition getFriendCountAndPosition(const FriendDecl *FD) {
-  if (FD->getFriendType())
-return getFriendCountAndPosition(FD, [](const FriendDecl *F) {
-  if (TypeSourceInfo *TSI = F->getFriendType())
-return TSI->getType().getCanonicalType();
-  llvm_unreachable("Wrong friend object type.");
-});
-  else
-return getFriendCountAndPosition(FD, [](const FriendDecl *F) {
-  if (Decl *D = F->getFriendDecl())
-return D->getCanonicalDecl();
-  llvm_unreachable("Wrong friend object type.");
-});
-}
-
 ExpectedDecl ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
   // Import the major distinguishing characteristics of a declaration.
   DeclContext *DC, *LexicalDC;
@@ -4129,26 +4126,13 @@ ExpectedDecl 
ASTNodeImporter::VisitFriendDecl(FriendDecl *D) {
   // FriendDecl is not a NamedDecl so we cannot use lookup.
   // We try to maintain order and count of redundant friend declarations.
   const auto *RD = cast(DC);
-  FriendDecl *ImportedFriend = RD->getFirstFriend();
   SmallVector ImportedEquivalentFriends;
-
-  while (ImportedFriend) {
-bool Match = false;
-if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-  Match =
-  IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
-/*Complain=*/false);
-} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-  Match = Importer.IsStructurallyEquivalent(
-  D->getFriendType()->getType(),
-  ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
-}
-if (Match)
+  for (FriendDecl *ImportedFriend : RD->friends())
+if (IsEquivalentFriend(Importer, D, ImportedFriend))
   ImportedEquivalentFriends.push_back(ImportedFriend);
 
-ImportedFriend = ImportedFriend->getNextFriend();
-  }
-  FriendCountAndPosition CountAndPosition = getFriendCountAndPosition(D);
+  FriendCountAndPosition CountAndPosition =
+  getFriendCountAndPosition(Importer, D);
 
   assert(ImportedEquivalentFriends.size() <= CountAndPosition.TotalCount &&
  "Class with non-matching friends is imported, ODR check wrong?");

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

2023-08-23 Thread Yeoul Na via Phabricator via cfe-commits

rapidsna added a comment.

> Future additions will include supporting, FAMs and counts in

sub-structures.

For sub-structures, are you referring to cases like this?

  struct outer {
struct {
  int count;
} sub;
int flex[__attribute__((counted_by(sub.count)))];
  };

For `-fbounds-safety` we would limit it to anonymous sub-structs only because 
otherwise the code can create a pointer to sub-struct and its count can easily 
be updated to an invalid count. Were you considering the same restrictions for 
this?

  void foo(struct outer *p1, struct sub *p2) {
p1 = (int*)malloc(sizeof(int) *10);
p1->count = 10;
  
p2->count = INT_MAX;
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

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

[PATCH] D157855: [clang][ExprConstant] Improve error message of compound assignment against uninitialized object

2023-08-23 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik accepted this revision.
shafik added a comment.

LGTM


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

https://reviews.llvm.org/D157855

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

[PATCH] D158562: [clang][Sema] Add truncation warning on fortified snprintf

2023-08-23 Thread Takuya Shimizu via Phabricator via cfe-commits

hazohelet updated this revision to Diff 552939.
hazohelet marked 4 inline comments as done.
hazohelet added a comment.

Address review comments


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

https://reviews.llvm.org/D158562

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Analysis/taint-generic.c
  clang/test/Sema/format-strings.c
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
 // RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -Wno-fortify-source %s -verify=nofortify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 -Wno-fortify-source %s -verify=nofortify
 
 typedef unsigned long size_t;
 
@@ -87,6 +89,10 @@
   char buf[10];
   __builtin_snprintf(buf, 10, "merp");
   __builtin_snprintf(buf, 11, "merp"); // expected-warning {{'snprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
+  __builtin_snprintf(buf, 0, "merp");
+  __builtin_snprintf(buf, 3, "merp"); // expected-warning {{'snprintf' will always be truncated; specified size is 3, but format string expands to at least 5}}
+  __builtin_snprintf(buf, 4, "merp"); // expected-warning {{'snprintf' will always be truncated; specified size is 4, but format string expands to at least 5}}
+  __builtin_snprintf(buf, 5, "merp");
 }
 
 void call_vsnprintf(void) {
@@ -94,14 +100,20 @@
   __builtin_va_list list;
   __builtin_vsnprintf(buf, 10, "merp", list);
   __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
+  __builtin_vsnprintf(buf, 0, "merp", list);
+  __builtin_vsnprintf(buf, 3, "merp", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 3, but format string expands to at least 5}}
+  __builtin_vsnprintf(buf, 4, "merp", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 4, but format string expands to at least 5}}
+  __builtin_vsnprintf(buf, 5, "merp", list);
 }
 
 void call_sprintf_chk(char *buf) {
   __builtin___sprintf_chk(buf, 1, 6, "hell\n");
   __builtin___sprintf_chk(buf, 1, 5, "hell\n"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
-  __builtin___sprintf_chk(buf, 1, 6, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
-  __builtin___sprintf_chk(buf, 1, 2, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
-  // expected-warning@-1 {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 5}}
+  __builtin___sprintf_chk(buf, 1, 6, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+// nofortify-warning {{format string contains '\0' within the string body}}
+  __builtin___sprintf_chk(buf, 1, 2, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+// nofortify-warning {{format string contains '\0' within the string body}}
+  // expected-warning@-2 {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 5}}
   __builtin___sprintf_chk(buf, 1, 6, "hello");
   __builtin___sprintf_chk(buf, 1, 5, "hello"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
   __builtin___sprintf_chk(buf, 1, 2, "%c", '9');
@@ -150,9 +162,11 @@
 
 void call_sprintf(void) {
   char buf[6];
-  sprintf(buf, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
-  sprintf(buf, "hello b\0y"); // expected-warning {{format string contains '\0' within the string body}}
-  // expected-warning@-1 {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 8}}
+  sprintf(buf, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}} \
+  // nofortify-warning {{format string contains '\0' within the string body}}
+  sprintf(buf, "hello b\0y"); // expected-warning {{format string contains '\0' within the string body}} \
+  // nofortify-warning {{format string contains '\0' within the string body}}
+  // expected-warning@-2 {{'sprintf' will

[clang] 61c8af6 - AMDGPU: InstCombine amdgcn.sqrt.f16 to sqrt.f16

2023-08-23 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2023-08-23T20:30:40-04:00
New Revision: 61c8af67924b02c8f2cf871439c24650a0207f29

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

LOG: AMDGPU: InstCombine amdgcn.sqrt.f16 to sqrt.f16

There's nothing special about f16 sqrt handling.

https://reviews.llvm.org/D158090

Added: 


Modified: 
clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
llvm/test/Transforms/InstCombine/AMDGPU/rcp-contract-rsq.ll

Removed: 




diff  --git a/clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl 
b/clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
index 4f7ac1673af37d..756f90b282a9a2 100644
--- a/clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
+++ b/clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
@@ -24,7 +24,7 @@ void test_rcp_f16(global half* out, half a)
 }
 
 // CHECK-LABEL: @test_sqrt_f16
-// CHECK: call half @llvm.amdgcn.sqrt.f16
+// CHECK: call half @llvm.sqrt.f16
 void test_sqrt_f16(global half* out, half a)
 {
   *out = __builtin_amdgcn_sqrth(a);

diff  --git a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp 
b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
index a0274aecfa3274..992f9964bf2e7d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
@@ -488,6 +488,14 @@ GCNTTIImpl::instCombineIntrinsic(InstCombiner , 
IntrinsicInst ) const {
   return IC.replaceInstUsesWith(II, QNaN);
 }
 
+// f16 amdgcn.sqrt is identical to regular sqrt.
+if (IID == Intrinsic::amdgcn_sqrt && Src->getType()->isHalfTy()) {
+  Function *NewDecl = Intrinsic::getDeclaration(
+  II.getModule(), Intrinsic::sqrt, {II.getType()});
+  II.setCalledFunction(NewDecl);
+  return 
+}
+
 break;
   }
   case Intrinsic::amdgcn_log:

diff  --git a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll 
b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
index 9413258d67b0ba..804283cc20cd6a 100644
--- a/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-intrinsics.ll
@@ -107,8 +107,7 @@ define double @test_constant_fold_sqrt_f64_undef() nounwind 
{
 
 define half @test_constant_fold_sqrt_f16_0() nounwind {
 ; CHECK-LABEL: @test_constant_fold_sqrt_f16_0(
-; CHECK-NEXT:[[VAL:%.*]] = call half @llvm.amdgcn.sqrt.f16(half 0xH) 
#[[ATTR15:[0-9]+]]
-; CHECK-NEXT:ret half [[VAL]]
+; CHECK-NEXT:ret half 0xH
 ;
   %val = call half @llvm.amdgcn.sqrt.f16(half 0.0) nounwind readnone
   ret half %val
@@ -116,7 +115,7 @@ define half @test_constant_fold_sqrt_f16_0() nounwind {
 
 define float @test_constant_fold_sqrt_f32_0() nounwind {
 ; CHECK-LABEL: @test_constant_fold_sqrt_f32_0(
-; CHECK-NEXT:[[VAL:%.*]] = call float @llvm.amdgcn.sqrt.f32(float 
0.00e+00) #[[ATTR15]]
+; CHECK-NEXT:[[VAL:%.*]] = call float @llvm.amdgcn.sqrt.f32(float 
0.00e+00) #[[ATTR15:[0-9]+]]
 ; CHECK-NEXT:ret float [[VAL]]
 ;
   %val = call float @llvm.amdgcn.sqrt.f32(float 0.0) nounwind readnone
@@ -134,8 +133,7 @@ define double @test_constant_fold_sqrt_f64_0() nounwind {
 
 define half @test_constant_fold_sqrt_f16_neg0() nounwind {
 ; CHECK-LABEL: @test_constant_fold_sqrt_f16_neg0(
-; CHECK-NEXT:[[VAL:%.*]] = call half @llvm.amdgcn.sqrt.f16(half 0xH8000) 
#[[ATTR15]]
-; CHECK-NEXT:ret half [[VAL]]
+; CHECK-NEXT:ret half 0xH8000
 ;
   %val = call half @llvm.amdgcn.sqrt.f16(half -0.0) nounwind readnone
   ret half %val
@@ -186,6 +184,42 @@ define double @test_constant_fold_sqrt_neg1() nounwind {
   ret double %val
 }
 
+define half @test_amdgcn_sqrt_f16(half %arg) {
+; CHECK-LABEL: @test_amdgcn_sqrt_f16(
+; CHECK-NEXT:[[VAL:%.*]] = call half @llvm.sqrt.f16(half [[ARG:%.*]])
+; CHECK-NEXT:ret half [[VAL]]
+;
+  %val = call half @llvm.amdgcn.sqrt.f16(half %arg)
+  ret half %val
+}
+
+define half @test_amdgcn_sqrt_f16_flags(half %arg) {
+; CHECK-LABEL: @test_amdgcn_sqrt_f16_flags(
+; CHECK-NEXT:[[VAL:%.*]] = call nnan half @llvm.sqrt.f16(half [[ARG:%.*]])
+; CHECK-NEXT:ret half [[VAL]]
+;
+  %val = call nnan half @llvm.amdgcn.sqrt.f16(half %arg)
+  ret half %val
+}
+
+define float @test_amdgcn_sqrt_f32(float %arg) {
+; CHECK-LABEL: @test_amdgcn_sqrt_f32(
+; CHECK-NEXT:[[VAL:%.*]] = call float @llvm.amdgcn.sqrt.f32(float 
[[ARG:%.*]])
+; CHECK-NEXT:ret float [[VAL]]
+;
+  %val = call float @llvm.amdgcn.sqrt.f32(float %arg)
+  ret float %val
+}
+
+define double @test_amdgcn_sqrt_f64(double %arg) {
+; CHECK-LABEL: @test_amdgcn_sqrt_f64(
+; CHECK-NEXT:[[VAL:%.*]] = call double @llvm.amdgcn.sqrt.f64(double 
[[ARG:%.*]])
+; CHECK-NEXT:ret double

[PATCH] D146773: [-Wunsafe-buffer-usage] Make raw (ungrouped) warnings a bit more verbose.

2023-08-23 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:256
-  foo(f(p, , a, a)[1]); // expected-warning{{unsafe buffer access}}
-  // FIXME: expected note@-1{{in instantiation of 
function template specialization 'f' requested here}}
 

I don't think this FIXME was correct. The code we're warning about isn't 
expanded from a template.


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

https://reviews.llvm.org/D146773

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

[clang] 8a62d6b - [ORC][clang-repl] Fix UnitTest after 122ebe3b500.

2023-08-23 Thread Lang Hames via cfe-commits


Author: Lang Hames
Date: 2023-08-23T17:19:07-07:00
New Revision: 8a62d6ba7edc3a7d397e52884a9ce63b4e579ae1

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

LOG: [ORC][clang-repl] Fix UnitTest after 122ebe3b500.

Commit 122ebe3b500 changed the way that we look up eh-frame registration
functions. This made LLJIT work "out of the box" in some Linux configs that
didn't work before, but caused InterpreterExceptionTest to start failing:
The test was skipped if a construction of a basic LLJIT config failed, but the
test actually depended on debugger support working too. When the basic config
started working we no longer skipped the test, then failed due to missing
debugger support. I've extended the skip-test check to include the debugger
support requirement, which should fix the issue.

Added: 


Modified: 
clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp

Removed: 




diff  --git 
a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp 
b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
index 70e10b1e53bd9d..8700f506d9b17d 100644
--- a/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ b/clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -52,7 +52,9 @@ TEST(InterpreterTest, CatchException) {
   llvm::InitializeNativeTargetAsmPrinter();
 
   {
-auto J = llvm::orc::LLJITBuilder().create();
+auto J = llvm::orc::LLJITBuilder()
+ .setEnableDebuggerSupport(true)
+ .create();
 if (!J) {
   // The platform does not support JITs.
   // Using llvm::consumeError will require typeinfo for ErrorInfoBase, we



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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

2023-08-23 Thread Christopher Di Bella via Phabricator via cfe-commits

cjdb added a comment.

In D157572#4606513 , @aaron.ballman 
wrote:

> In D157572#4604595 , @philnik wrote:
>
>> In D157572#4604482 , 
>> @aaron.ballman wrote:
>>
 This allows standard libraries to mark symbols as extensions, so the 
 compiler can generate extension warnings when they are used.
>>>
>>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>>> is used to silence extension warnings)?
>>
>> I guess, kind-of. I never really understood the semantics of `__extension__` 
>> though, so I'm not 100% certain.
>
> It's used in system headers to say "I'm using an extension here, don't warn 
> about it in -pedantic mode".
>
>>> I don't think we need to introduce a new attribute to do this, we already 
>>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that 
>>> suffice?
>>
>> Part of the idea here is that the diagnostics should be part of 
>> `-Wc++ab-extension`.
>
> Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty 
> because it's telling the user about all use of extensions, not extensions 
> being introduced by the library itself? (e.g., 
> https://godbolt.org/z/Gs3YGheMM is also not what you're after)
>
>> I guess we could allow warning flags instead of just `"warning"` and 
>> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
>> should be part of. Something like 
>> `__attribute__((__diagnose_if__(__cplusplus >= 201703L, "basic_string_view 
>> is a C++17 extension", "-Wc++17-extensions")))`. I'm not sure how one could 
>> implement that, but I guess there is some mechanism to translate 
>> "-Wwhatever" to a warning group, since you can push and pop warnings.  That 
>> would open people up to add a diagnostic to pretty much any warning group. I 
>> don't know if that's a good idea. I don't really see a problem with that 
>> other than people writing weird code, but people do that all the time 
>> anyways. Maybe I'm missing something really problematic though.
>
> That's actually a pretty interesting idea; `diagnose_if` could be given 
> another parameter to specify a diagnostic group to associate the diagnostic 
> with. This would let you do some really handy things like:
>
>   void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative 
> value to 'func' is deprecated", "warning", "-Wdeprecated")));
>
> But if we went this route, would we want to expose other diagnostic-related 
> knobs like "show in system header" and "default to an error"? Also, the 
> attribute currently can only be associated with a function; we can use this 
> for classes by sticking it on a constructor but there's not much help for 
> putting it on say a namespace or an enumeration. So we may need to extend the 
> attribute in other ways. CC @cjdb as this seems of interest to you as well.

I don't dislike it, but I am a bit concerned about misuse being noisy. As much 
as I hate suppressing diagnostics, I think there needs to be a way to suppress 
the `diagnose_if` forms of warning without suppressing something that the 
compiler would otherwise generate. Something like:

- `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
- `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
- `-Wno-deprecated=non-diagnose_if`: complement to #2.

(and similarly for `-Wno-error=`.)

I'm not sure about the system header knob though: `[[deprecated]]` and 
`[[nodiscard]]` still show up even when the declaration is in a system header?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D152279#4612099 , @craig.topper 
wrote:

> In D152279#4612087 , @MaskRay wrote:
>
>> I am still interested in moving this forward. What should be done here? If 
>> the decision is to keep the current odd default 8 for 
>> `toolchains::RISCVToolChain`, I guess I'll have to take the compromise as 
>> making a step forward is better than nothing.
>
> On 1 RV64 CPU I tried in our RTL simulator, changing from 8 to 0 reduced 
> dhrystone score by 2.7%. Using 16, or 32 gave the same score as 8. Reducing 8 
> to 4 improved the score by 0.5%.

Thank you for sharing the benchmarks!

My view is that global pointer relaxation is an expert option that the user 
needs to tune (like that ld.lld doesn't default to `--relax-gp`).
People can create more articles about global pointer relaxation usage, and make 
the default out of the business of the driver.
If anyone tells me sdata/srodata/sbss adoption for other languages' compiler 
drivers, I'll definitely tell them not to copy the 0/8 complex rules in clang 
driver:)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152279

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

2023-08-23 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added a comment.

Codegen parts LGTM, questions with the driver parts




Comment at: clang/lib/Driver/ToolChain.cpp:1368
 if (A->getOption().matches(options::OPT_m_Group)) {
-  if (SameTripleAsHost)
+  // Pass code objection version to device toolchain
+  // to correctly set meta-data in intermediate files.

Typos



Comment at: clang/lib/Driver/ToolChain.cpp:1369
+  // Pass code objection version to device toolchain
+  // to correctly set meta-data in intermediate files.
+  if (SameTripleAsHost ||





Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8649-8650
 
+  // code-object-version=X needs to be passed to clang-linker-wrapper to ensure
+  // that it is used by lld.
+  if (const Arg *A = Args.getLastArg(options::OPT_mcode_object_version_EQ)) {

so device rtl is linked once as a normal library?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8653-8654
+CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+CmdArgs.push_back(Args.MakeArgString(
+Twine("--amdhsa-code-object-version=") + A->getValue()));
+  }

Why do you need this? The code object version is supposed to come from a module 
flag. We should be getting rid of the command line argument for it



Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:406-410
+  // pass on -mllvm options to the clang
+  for (const opt::Arg *Arg : Args.filtered(OPT_mllvm)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Arg->getValue());
+  }

Shouldn't need this?



Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:417
 CmdArgs.push_back("-save-temps");
+// CmdArgs.push_back(Args.MakeArgString("--amdhsa-code-object-version=5"));
+  }

Commented out code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

rnk wrote:
> erichkeane wrote:
> > aeubanks wrote:
> > > erichkeane wrote:
> > > > aeubanks wrote:
> > > > > aaron.ballman wrote:
> > > > > > aeubanks wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > aeubanks wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > What happens for tentative definitions where the value 
> > > > > > > > > > isn't known? e.g.,
> > > > > > > > > > ```
> > > > > > > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > What happens if the types are similar but not the same?
> > > > > > > > > > ```
> > > > > > > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > > > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > Should we diagnose taking the address of such an attributed 
> > > > > > > > > > variable so users have some hope of spotting the 
> > > > > > > > > > non-conforming situations?
> > > > > > > > > > 
> > > > > > > > > > Does this attribute have impacts across translation unit 
> > > > > > > > > > boundaries (perhaps only when doing LTO) or only within a 
> > > > > > > > > > single TU?
> > > > > > > > > > 
> > > > > > > > > > What does this attribute do in C++ in the presence of 
> > > > > > > > > > constructors and destructors? e.g.,
> > > > > > > > > > ```
> > > > > > > > > > struct S {
> > > > > > > > > >   S();
> > > > > > > > > >   ~S();
> > > > > > > > > > };
> > > > > > > > > > 
> > > > > > > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and 
> > > > > > > > > > there's only one ctor/dtor call?
> > > > > > > > > > ```
> > > > > > > > > globals are only mergeable if they're known to be constant 
> > > > > > > > > and have the same value/size. this can be done at compile 
> > > > > > > > > time only if the optimizer can see the constant values, or at 
> > > > > > > > > link time
> > > > > > > > > 
> > > > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > > > 
> > > > > > > > > but yeah that does imply that we should warn when the 
> > > > > > > > > attribute is used on non const, non-POD globals. I'll update 
> > > > > > > > > this patch to do that
> > > > > > > > > 
> > > > > > > > > as mentioned in the description, we actually do want to take 
> > > > > > > > > the address of these globals for table-driven parsing, but we 
> > > > > > > > > don't care about identity equality
> > > > > > > > > globals are only mergeable if they're known to be constant 
> > > > > > > > > and have the same value/size. this can be done at compile 
> > > > > > > > > time only if the optimizer can see the constant values, or at 
> > > > > > > > > link time
> > > > > > > > >
> > > > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > > 
> > > > > > > > A that's good to know. So I assume we *will* merge these?
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > struct S {
> > > > > > > >   int i, j;
> > > > > > > >   float f;
> > > > > > > > };
> > > > > > > > 
> > > > > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > > but yeah that does imply that we should warn when the 
> > > > > > > > > attribute is used on non const, non-POD globals. I'll update 
> > > > > > > > > this patch to do that
> > > > > > > > 
> > > > > > > > Thank you, I think that will be more user-friendly
> > > > > > > > 
> > > > > > > > > as mentioned in the description, we actually do want to take 
> > > > > > > > > the address of these globals for table-driven parsing, but we 
> > > > > > > > > don't care about identity equality
> > > > > > > > 
> > > > > > > > Yeah, I still wonder if we want to diagnose just the same -- if 
> > > > > > > > the address is never taken, there's not really a way to notice 
> > > > > > > > the optimization, but if the address is taken, you basically 
> > > > > > > > get UB (and I think we should explicitly document it as such). 
> > > > > > > > Given how easy it is to accidentally take the address of 
> > > > > > > > something (like via a reference in C++), I think we should warn 
> > > > > > > > by default, but still have a warning group for folks who want 
> > > > > > > > to live life dangerously.
> > > > > > > > > globals are only mergeable if they're known to be constant 
> > > > > > > > > and have the same value/size. this can be done at compile 
> > > > > > > > > time only if the optimizer can see the constant values, or at 
> > > > > > > > > link time
> > > > > > > >

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

2023-08-23 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

In D152279#4612087 , @MaskRay wrote:

> I am still interested in moving this forward. What should be done here? If 
> the decision is to keep the current odd default 8 for 
> `toolchains::RISCVToolChain`, I guess I'll have to take the compromise as 
> making a step forward is better than nothing.

On 1 RV64 CPU I tried in our RTL simulator, changing from 8 to 0 reduced 
dhrystone score by 2.7%. Using 16, or 32 gave the same score as 8. Reducing 8 
to 4 improved the score by 0.5%.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152279

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

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added a comment.

In D158469#4612012 , @benlangmuir 
wrote:

>> I tried that approach, but found it way too easy to keep `ModuleDeps` 
>> around, which keep scanning instances alive, and use tons of memory.
>
> It seems like the problem (too easy to keep around MD) is the same either 
> way, it's just instead of wasting memory it's leaving dangling pointers that 
> could cause UAF if they're actually used.

You're right. I have a version of this patch that stores 
`std::weak_ptr MDC` in `ModuleDeps` and asserts when you're 
about to dereference dangling pointer. That might solve that part of the issue.

> Where were you seeing MD held too long? I wonder if there's another way to 
> fix that.

For example in `clang-scan-deps`, we accumulate all `ModuleDeps` into 
`FullDeps::Modules`. This means that at the end of the scan, that can be 
keeping up to `NumTUs` worth of `CompilerInvocations` alive.

>> Hmm, maybe we could avoid holding on to the whole CompilerInstance.
>
> This seems promising!

OK, I'll try to explore this a little further. I'm a bit concerned that 
`FileManager` (and its caches) might still be too heavy to keep around, though.

---

I guess what makes this harder to get right is the fact that `ModuleDeps` and 
`ModuleDepsGraph` live on different levels. I think that maybe restructuring 
the API a little bit could simplify things. What I have in mind is replacing 
the `DependencyConsumer` function `handleModuleDependency(ModuleDeps)` with 
`handleModuleDepsGraph(ModuleDepsGraph)`. We could then turn `ModuleDepsGraph` 
into an iterator over proxy objects representing what's currently called 
`ModuleDeps`. (Lifetime of these proxies would be clearly tied to that of 
`ModuleDepsGraph`.) I think we can assume that scanner clients are less tempted 
to "permanently" store the full `ModuleDepsGraph` (since that's wasteful due to 
cross-TU sharing), compared to `ModuleDeps` (which you probably want to store 
in some form). Clients would therefore be nudged into creating own 
storage-friendly version of `ModuleDeps` before disposing of `ModuleDepsGraph`, 
which would force them to call `getBuildArguments()` and `getFileDeps()` on our 
`ModuleDeps` proxy before disposing of the heavy-weight graph. WDYT about this 
direction?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158469

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

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I am still interested in moving this forward. What should be done here? If the 
decision is to keep the current odd default 8 for `toolchains::RISCVToolChain`, 
I guess I'll have to take the compromise as making a step forward is better 
than nothing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152279

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

[PATCH] D158668: RFC: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB added a comment.

> I'd also posit, that maybe since we're changing this we should reevaluate the 
> numbers we use as defaults.

Heh, same here. Internally we have a handful of functions that end up using 
`[[likely]]` loop conditions in a triple-nested loops leading to the estimated 
block frequencies going through the roof and unjustly dominating everything 
else in the program... Though admittedly I am not decided yet whether I want to 
blame the 2000:1 ratio or rather the programmers for using the annotation too 
freely.

There is also a disconnect with the GCC documentation saying 
`__attribute__((expected))` corresponding to a 99:1 ratio.

Anyway we probably need a separate/patch discussion to not derail this diff too 
much...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D154784: [clang] Fix crash caused by PseudoObjectExprBitfields::NumSubExprs overflow

2023-08-23 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: clang/include/clang/AST/Stmt.h:596-597
 
-// These don't need to be particularly wide, because they're
-// strictly limited by the forms of expressions we permit.
-unsigned NumSubExprs : 8;
-unsigned ResultIndex : 32 - 8 - NumExprBits;
+unsigned NumSubExprs : 16;
+unsigned ResultIndex : 16;
   };

aaron.ballman wrote:
> yronglin wrote:
> > yronglin wrote:
> > > dblaikie wrote:
> > > > yronglin wrote:
> > > > > dblaikie wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > yronglin wrote:
> > > > > > > > dblaikie wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > Could/should we add some error checking in the ctor to 
> > > > > > > > > > > > assert that we don't overflow these longer values/just 
> > > > > > > > > > > > hit the bug later on?
> > > > > > > > > > > > 
> > > > > > > > > > > > (& could we use `unsigned short` here rather than 
> > > > > > > > > > > > bitfields?)
> > > > > > > > > > > We've already got them packed in with other bit-fields 
> > > > > > > > > > > from the expression bits, so I think it's reasonable to 
> > > > > > > > > > > continue the pattern of using bit-fields (that way we 
> > > > > > > > > > > don't accidentally end up with padding between the 
> > > > > > > > > > > unnamed bits at the start and the named bits in this 
> > > > > > > > > > > object).
> > > > > > > > > > > 
> > > > > > > > > > > I think adding some assertions would not be a bad idea as 
> > > > > > > > > > > a follow-up.
> > > > > > > > > > Maybe some unconditional (rather than only in asserts 
> > > > > > > > > > builds) error handling? (report_fatal_error, if this is low 
> > > > > > > > > > priority enough to not have an elegant failure mode, but 
> > > > > > > > > > something where we don't just overflow and carry on would 
> > > > > > > > > > be good... )
> > > > > > > > > Ping on this? I worry this code has just punted the same bug 
> > > > > > > > > further down, but not plugged the hole/ensured we don't 
> > > > > > > > > overflow on novel/larger inputs.
> > > > > > > > Sorry for the late reply, I was looking through the emails and 
> > > > > > > > found this. I agree add some assertions to check the value is a 
> > > > > > > > good idea, It's easy to help people catch bugs, at least with 
> > > > > > > > when `-DLLVM_ENABLE_ASSERTIONS=ON`, and I'm glad to work on it, 
> > > > > > > > but one thing that worries me is that, in ASTReader, we access 
> > > > > > > > this field directly, not through the constructor or accessor, 
> > > > > > > > and we have to add assertions everywhere. 
> > > > > > > > https://github.com/llvm/llvm-project/blob/05b4310c8aec7a050574277ced08a0ab86b27681/clang/lib/Serialization/ASTReaderStmt.cpp#L1382
> > > > > > > I don't think we have to add too many assertions. As best I can 
> > > > > > > tell, we'll need one in each of the `PseudoObjectExpr` 
> > > > > > > constructors and one in `ASTStmtReader::VisitPseudoObjectExpr()`, 
> > > > > > > but those are the only places we assign a value into the 
> > > > > > > bit-field. Three assertions isn't a lot, but if we're worried, we 
> > > > > > > could add a setter method that does the assertion and use the 
> > > > > > > setter in all three places.
> > > > > > My concern wasn't (well, wasn't entirely) about adding more 
> > > > > > assertions - but about having a reliable error here. The patch only 
> > > > > > makes the sizes larger, but doesn't have a hard-stop in case those 
> > > > > > sizes are exceeded again (which, admittedly, is much harder to do - 
> > > > > > maybe it's totally unreachable now, for all practical purposes?) 
> > > > > > 
> > > > > > I suspect with more carefully constructed recursive inputs could 
> > > > > > still reach the higher limit & I think it'd be good to fail hard in 
> > > > > > that case in some way? (it's probably rare enough that a 
> > > > > > report_fatal_error would be not-the-worst-thing-ever)
> > > > > > 
> > > > > > But good assertions would be nice too (the old code only failed 
> > > > > > when you hit /exactly/ on just the overflow value, and any more 
> > > > > > than that the wraparound would not crash/fail, but misbehave) - I 
> > > > > > did add the necessary assertion to ArrayRef (begin <= end) which 
> > > > > > would've helped detect this more reliably, but some assert checking 
> > > > > > for overflow in the ctor would be good too (with all the usual 
> > > > > > nuance/care in checking for overflow) - unless we're going to make 
> > > > > > that into a fatal or other real error.
> > > > > Sorry for the very late reply. I have no preference between assertion 
> > > > > and `llvm_unreachable`, if error then fail fast is looks good. I have 
> > > > > a patch D158296 to add assertion.
> > > > Thanks for the assertions - though they still haven't met my main 
> > > > concern that this

[PATCH] D158668: RFC: Add getLikelyBranchWeight helper function

2023-08-23 Thread Paul Kirth via Phabricator via cfe-commits

paulkirth added a comment.

In D158668#4611842 , @MatzeB wrote:

>> My initial reaction to this was that we should keep the 
>> --unlikely-branch-weights flag available
>
> I don't feel strongly about it and can put it back. But can you give some 
> reasoning? I only see this flag having a real use to express small ratios 
> like 3:2 which doesn't really seem helpful to express "likely"/"unlikely"...

I think the rest of the comment agrees w/ you that it probably isn't that 
useful :)

My concern was testing, but given that you've barley had to update anything, 
I'd say that early feeling is just wrong, since we aren't using it except in a 
handful of cases. As such I think it can go away w/ only a short FYI about a 
planned change.

In D158668#4611845 , @MatzeB wrote:

>> On the other hand, I dislike exposing some internal detail of a pass.
>
> I think the question to ask is whether a concept like "likely branch" or 
> "unlikely branch" shouldn't be more universal and not be a pass internal 
> detail? Otherwise it feels like my patches and other places would need to 
> insert new `llvm.expect` usages into the IR and add more instances of 
> `LowerExpect` pass into the pipeline to not risk pass ordering problems for 
> the small gain of having a more abstract representation...

Again, I think the comments directly following agree w/ your position:

> That said, it seems like these have more value when exposed, so I think it 
> would be fine, as long as we can prevent frontends from using the APIs, like 
> you mentioned in https://reviews.llvm.org/D158642#4611025.

I think he fact that we're discussing this means it isn't functionally internal 
anymore, so that's maybe reason enough to do this.

So to clarify, I'm broadly in favor of this direction. I'd also posit, that 
maybe since we're changing this we should reevaluate the numbers we use as 
defaults.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D157547: Arm64EC entry/exit thunks, consolidated.

2023-08-23 Thread Eli Friedman via Phabricator via cfe-commits

efriedma updated this revision to Diff 552917.
efriedma added a comment.

Update to address issues found so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157547

Files:
  clang/lib/CodeGen/CGCXX.cpp
  llvm/include/llvm/IR/CallingConv.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/Target/AArch64/AArch64.h
  llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AArch64MCInstLower.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/CMakeLists.txt
  llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
  llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h
  llvm/test/CodeGen/AArch64/arm64ec-dllimport.ll
  llvm/test/CodeGen/AArch64/arm64ec-reservedregs.ll
  llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
  llvm/test/CodeGen/AArch64/stack-protector-target.ll
  llvm/test/CodeGen/AArch64/win-alloca.ll

Index: llvm/test/CodeGen/AArch64/win-alloca.ll
===
--- llvm/test/CodeGen/AArch64/win-alloca.ll
+++ llvm/test/CodeGen/AArch64/win-alloca.ll
@@ -21,4 +21,4 @@
 ; CHECK-OPT: sub [[REG3:x[0-9]+]], sp, x15, lsl #4
 ; CHECK-OPT: mov sp, [[REG3]]
 ; CHECK: bl func2
-; CHECK-ARM64EC: bl __chkstk_arm64ec
+; CHECK-ARM64EC: bl "#__chkstk_arm64ec"
Index: llvm/test/CodeGen/AArch64/stack-protector-target.ll
===
--- llvm/test/CodeGen/AArch64/stack-protector-target.ll
+++ llvm/test/CodeGen/AArch64/stack-protector-target.ll
@@ -39,6 +39,6 @@
 ; WINDOWS-ARM64EC: adrp x8, __security_cookie
 ; WINDOWS-ARM64EC: ldr x8, [x8, :lo12:__security_cookie]
 ; WINDOWS-ARM64EC: str x8, [sp, #8]
-; WINDOWS-ARM64EC: bl  _Z7CapturePi
+; WINDOWS-ARM64EC: bl "#_Z7CapturePi"
 ; WINDOWS-ARM64EC: ldr x0, [sp, #8]
-; WINDOWS-ARM64EC: bl  __security_check_cookie_arm64ec
+; WINDOWS-ARM64EC: bl "#__security_check_cookie_arm64ec"
Index: llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
===
--- llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
+++ llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=arm64ec-pc-windows-msvc < %s | FileCheck %s
-; RUN: llc -mtriple=arm64ec-pc-windows-msvc < %s -global-isel=1 -global-isel-abort=0 | FileCheck %s
+; RUN: llc -mtriple=arm64ec-pc-windows-msvc -arm64ec-generate-thunks=false < %s | FileCheck %s
+; RUN: llc -mtriple=arm64ec-pc-windows-msvc -arm64ec-generate-thunks=false < %s -global-isel=1 -global-isel-abort=0 | FileCheck %s
 
 define void @varargs_callee(double %x, ...) nounwind {
 ; CHECK-LABEL: varargs_callee:
@@ -35,16 +35,20 @@
 ; CHECK-NEXT:sub sp, sp, #48
 ; CHECK-NEXT:mov x4, sp
 ; CHECK-NEXT:add x8, sp, #16
-; CHECK-NEXT:mov x9, #4617315517961601024
-; CHECK-NEXT:mov x0, #4607182418800017408
-; CHECK-NEXT:mov w1, #2
-; CHECK-NEXT:mov x2, #4613937818241073152
-; CHECK-NEXT:mov w3, #4
-; CHECK-NEXT:mov w5, #16
+; CHECK-NEXT:mov x9, #4617315517961601024 // =0x4014
+; CHECK-NEXT:mov x0, #4607182418800017408 // =0x3ff0
+; CHECK-NEXT:mov w1, #2 // =0x2
+; CHECK-NEXT:mov x2, #4613937818241073152 // =0x4008
+; CHECK-NEXT:mov w3, #4 // =0x4
+; CHECK-NEXT:mov w5, #16 // =0x10
 ; CHECK-NEXT:stp xzr, x30, [sp, #24] // 8-byte Folded Spill
 ; CHECK-NEXT:stp x8, xzr, [sp, #8]
 ; CHECK-NEXT:str x9, [sp]
-; CHECK-NEXT:bl varargs_callee
+; CHECK-NEXT:.weak_anti_dep varargs_callee
+; CHECK-NEXT:  .set varargs_callee, "#varargs_callee"@WEAKREF
+; CHECK-NEXT:.weak_anti_dep "#varargs_callee"
+; CHECK-NEXT:  .set "#varargs_callee", varargs_callee@WEAKREF
+; CHECK-NEXT:bl "#varargs_callee"
 ; CHECK-NEXT:ldr x30, [sp, #32] // 8-byte Folded Reload
 ; CHECK-NEXT:add sp, sp, #48
 ; CHECK-NEXT:ret
@@ -71,17 +75,21 @@
 ; CHECK-NEXT:sub sp, sp, #64
 ; CHECK-NEXT:movi v0.2d, #
 ; CHECK-NEXT:mov x4, sp
-; CHECK-NEXT:mov x8, #4618441417868443648
+; CHECK-NEXT:mov x8, #4618441417868443648 // =0x4018
 ; CHECK-NEXT:add x9, sp, #16
 ; CHECK-NEXT:add x3, sp, #32
-; CHECK-NEXT:mov x0, #4607182418800017408
-; CHECK-NEXT:mov x1,

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

2023-08-23 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

> I tried that approach, but found it way too easy to keep `ModuleDeps` around, 
> which keep scanning instances alive, and use tons of memory.

It seems like the problem (too easy to keep around MD) is the same either way, 
it's just instead of wasting memory it's leaving dangling pointers that could 
cause UAF if they're actually used.  Where were you seeing MD held too long? I 
wonder if there's another way to fix that.

> Hmm, maybe we could avoid holding on to the whole CompilerInstance.

This seems promising!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158469

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

erichkeane wrote:
> aeubanks wrote:
> > erichkeane wrote:
> > > aeubanks wrote:
> > > > aaron.ballman wrote:
> > > > > aeubanks wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > aeubanks wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > What happens for tentative definitions where the value isn't 
> > > > > > > > > known? e.g.,
> > > > > > > > > ```
> > > > > > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > What happens if the types are similar but not the same?
> > > > > > > > > ```
> > > > > > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > Should we diagnose taking the address of such an attributed 
> > > > > > > > > variable so users have some hope of spotting the 
> > > > > > > > > non-conforming situations?
> > > > > > > > > 
> > > > > > > > > Does this attribute have impacts across translation unit 
> > > > > > > > > boundaries (perhaps only when doing LTO) or only within a 
> > > > > > > > > single TU?
> > > > > > > > > 
> > > > > > > > > What does this attribute do in C++ in the presence of 
> > > > > > > > > constructors and destructors? e.g.,
> > > > > > > > > ```
> > > > > > > > > struct S {
> > > > > > > > >   S();
> > > > > > > > >   ~S();
> > > > > > > > > };
> > > > > > > > > 
> > > > > > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and 
> > > > > > > > > there's only one ctor/dtor call?
> > > > > > > > > ```
> > > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > > > 
> > > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > > 
> > > > > > > > but yeah that does imply that we should warn when the attribute 
> > > > > > > > is used on non const, non-POD globals. I'll update this patch 
> > > > > > > > to do that
> > > > > > > > 
> > > > > > > > as mentioned in the description, we actually do want to take 
> > > > > > > > the address of these globals for table-driven parsing, but we 
> > > > > > > > don't care about identity equality
> > > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > > >
> > > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > 
> > > > > > > A that's good to know. So I assume we *will* merge these?
> > > > > > > 
> > > > > > > ```
> > > > > > > struct S {
> > > > > > >   int i, j;
> > > > > > >   float f;
> > > > > > > };
> > > > > > > 
> > > > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > > > > ```
> > > > > > > 
> > > > > > > > but yeah that does imply that we should warn when the attribute 
> > > > > > > > is used on non const, non-POD globals. I'll update this patch 
> > > > > > > > to do that
> > > > > > > 
> > > > > > > Thank you, I think that will be more user-friendly
> > > > > > > 
> > > > > > > > as mentioned in the description, we actually do want to take 
> > > > > > > > the address of these globals for table-driven parsing, but we 
> > > > > > > > don't care about identity equality
> > > > > > > 
> > > > > > > Yeah, I still wonder if we want to diagnose just the same -- if 
> > > > > > > the address is never taken, there's not really a way to notice 
> > > > > > > the optimization, but if the address is taken, you basically get 
> > > > > > > UB (and I think we should explicitly document it as such). Given 
> > > > > > > how easy it is to accidentally take the address of something 
> > > > > > > (like via a reference in C++), I think we should warn by default, 
> > > > > > > but still have a warning group for folks who want to live life 
> > > > > > > dangerously.
> > > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > > >
> > > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > 
> > > > > > > A that's good to know. So I assume we *will* merge these?
> > > > > > > 
> > > > > > > ```
> > > > > > > struct S {
> > > > > > >   int i, j;
> > > > >

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB added a comment.

And as another strawman / discussion-started I put up D158680 
 where I use `!{"branch_weights", i32 1, i32 
0}` to represent likely branches and the actual "LikelyWeight" mostly becomes 
an internal implementation detail of the BranchProbabilityInfo class... This 
seemed like a neat way to get an abstract "likely", "unlikely" notation, but 
not sure of the effects if we no longer would have "truly zero" weights because 
they would be interpreted differently now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added a comment.

Hmm, maybe we could avoid holding on to the whole `CompilerInstance`. For 
generating the command-line, we only need the `CompilerInvocation`. For 
collecting file dependencies, we could hold on to the `MemoryBuffer` (and maybe 
offset to the input files block), and deserialize that on-demand, without 
keeping the whole `CompilerInstance` around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158469

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

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added a comment.

In D158469#4611923 , @benlangmuir 
wrote:

> I find this a bit hard to understand as far as object lifetime is concerned: 
> you're storing the `ScanInstance` in `TranslationUnitDeps`, but that's a 
> layer above the actual consumer interface, which means every consumer needs 
> to understand how this lifetime is managed or it will have dangling 
> references by default.  You also seem to have ScanInstance stored twice -- 
> once explicitly and once in the graph.

Agreed, I'm not happy about the lifetime complexity here.

> What do you think of an alternate design where `ModuleDeps` has 
> `shared_ptr MDC` and `ModuleDepCollector` has 
> `shared_ptr`?  That way we don't need to explicitly expose 
> the scan instance to the consumer, it comes with the `ModuleDeps`, which then 
> keeps all the necessary memory alive?

I tried that approach, but found it way too easy to keep `ModuleDeps` around, 
which keep scanning instances alive, and use tons of memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158469

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

[PATCH] D158469: [clang][deps] Compute command lines and file deps on-demand

2023-08-23 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

I find this a bit hard to understand as far as object lifetime is concerned: 
you're storing the `ScanInstance` in `TranslationUnitDeps`, but that's a layer 
above the actual consumer interface, which means every consumer needs to 
understand how this lifetime is managed or it will have dangling references by 
default.  You also seem to have ScanInstance stored twice -- once explicitly 
and once in the graph.

What do you think of an alternate design where `ModuleDeps` has 
`shared_ptr MDC` and `ModuleDepCollector` has 
`shared_ptr`?  That way we don't need to explicitly expose 
the scan instance to the consumer, it comes with the `ModuleDeps`, which then 
keeps all the necessary memory alive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158469

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB added a comment.

> On the other hand, I dislike exposing some internal detail of a pass.

I think the question to ask is whether a concept like "likely branch" or 
"unlikely branch" shouldn't be more universal and not be a pass internal 
detail? Otherwise it feels like my patches and other places would need to 
insert new `llvm.expect` usages into the IR and litter the pass pipeline with 
more instances of the LowerExpect pass which seems overkill for little gain to 
me...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB added a comment.

> My initial reaction to this was that we should keep the 
> --unlikely-branch-weights flag available

I don't feel strongly about it and can put it back. But can you give some 
reasoning? I only see this flag having a real use to express small ratios like 
3:2 which doesn't really seem helpful to express "likely"/"unlikely"...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Paul Kirth via Phabricator via cfe-commits

paulkirth added a comment.

My initial reaction to this was that we should keep the 
`--unlikely-branch-weights` flag available, even if we decide to move forward 
with the other changes. but given that you haven't had to update many tests, I 
think its probably fine to go this way. Probably just an FYI on discourse is 
sufficient to notify potential downstream consumers.

As for exposing the Likely/unlikely weights ... I'm a bit conflicted. On one 
hand, I would love a simple way to query the current weights for MisExpect 
diagnostics, and some of the other things we've been prototyping like 
suggesting places where annotations would be profitable. On the other hand, I 
dislike exposing some internal detail of a pass.

That said, it seems like these have more value when exposed, so I think it 
woudl be fine, as long as we can prevent frontends from using the APIs, like 
you mentioned in https://reviews.llvm.org/D158642#4611025.

I'm not sure how to separate the APIs  the way we want, though. Most of the 
APIs in "ProfDataUtils.h" are desirable to be public, which means `include` is 
a good place for them, but `getLikelyBranchWeight` may not fit well in that 
regard... Maybe we need an internal header in `lib`? or perhaps I'm thinking 
bout this too much...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D157331: [clang] Implement C23

2023-08-23 Thread Zijun Zhao via Phabricator via cfe-commits

ZijunZhao updated this revision to Diff 552899.
ZijunZhao marked 2 inline comments as done.
ZijunZhao added a comment.

1. define __STDC_VERSION_STDCKDINT_H__
2. check short type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157331

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/stdckdint.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/C/C2x/n2359.c
  clang/test/C/C2x/n2683.c
  clang/test/C/C2x/n2683_2.c
  clang/test/Headers/stdckdint.c
  clang/test/Modules/Inputs/System/usr/include/module.map
  clang/test/SemaCXX/builtins-overflow.cpp
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -887,6 +887,11 @@
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2672.pdf;>N2672
   Yes
 
+
+  Towards Integer Safety
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2683.pdf;>N2683
+  Clang 18
+
 
   Adding Fundamental Type for N-bit Integers
 
Index: clang/test/SemaCXX/builtins-overflow.cpp
===
--- clang/test/SemaCXX/builtins-overflow.cpp
+++ clang/test/SemaCXX/builtins-overflow.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
-// expected-no-diagnostics
 
 #include 
 #include 
@@ -30,10 +29,10 @@
 template 
 constexpr Result add(LHS &, RHS &) {
   RET sum{};
-  return {__builtin_add_overflow(lhs, rhs, ), sum};
+  return {__builtin_add_overflow(lhs, rhs, ), sum}; /* expected-warning {{'short' may not be suitable to hold the result of operating two 'int's}} */
 }
 
-static_assert(add(static_cast(120), static_cast(10)) == Result{false, 130});
+static_assert(add(static_cast(120), static_cast(10)) == Result{false, 130}); /* expected-error {{static assertion expression is not an integral constant expression}} */ /* expected-note {{in instantiation of function template specialization 'add' requested here}} */
 static_assert(add(static_cast(120), static_cast(10)) == Result{true, -126});
 static_assert(add(INT_MAX, INT_MAX) == Result{false, static_cast(INT_MAX) * 2u});
 static_assert(add(static_cast(INT_MAX), 1u) == Result{true, INT_MIN});
@@ -45,12 +44,12 @@
 template 
 constexpr Result sub(LHS &, RHS &) {
   RET sum{};
-  return {__builtin_sub_overflow(lhs, rhs, ), sum};
+  return {__builtin_sub_overflow(lhs, rhs, ), sum}; /* expected-warning {{'short' may not be suitable to hold the result of operating two 'int's}} */
 }
 
 static_assert(sub(static_cast(0),static_cast(1)) == Result{true, UCHAR_MAX});
 static_assert(sub(static_cast(0),static_cast(1)) == Result{false, -1});
-static_assert(sub(static_cast(0),static_cast(1)) == Result{true, USHRT_MAX});
+static_assert(sub(static_cast(0),static_cast(1)) == Result{true, USHRT_MAX}); /* expected-error {{static assertion expression is not an integral constant expression}} */ /* expected-note {{in instantiation of function template specialization 'sub' requested here}} */
 static_assert(sub(static_cast(255),static_cast(100)) == Result{false, 155});
 
 static_assert(sub(17,22) == Result{false, -5});
Index: clang/test/Modules/Inputs/System/usr/include/module.map
===
--- clang/test/Modules/Inputs/System/usr/include/module.map
+++ clang/test/Modules/Inputs/System/usr/include/module.map
@@ -14,6 +14,11 @@
 header "stdbool.h"
   }
 
+  // In both directories (compiler support version wins, does not forward)
+  module stdckdint {
+header "stdckdint.h"
+  }
+
   // In both directories (compiler support version wins, forwards)
   module stdint {
 header "stdint.h"
Index: clang/test/Headers/stdckdint.c
===
--- /dev/null
+++ clang/test/Headers/stdckdint.c
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -emit-llvm -verify -std=c23 %s -o - | FileCheck %s
+// expected-no-diagnostics
+#include 
+
+_Static_assert(__STDC_VERSION_STDCKDINT_H__ == 202311L, "");
+
+// CHECK-LABEL: define dso_local zeroext i1 @test_ckd_add() #0 {
+// CHECK:  entry:
+// CHECK:%result = alloca i32, align 4
+// CHECK:%0 = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 -1073741826, i32 -1073741826)
+// CHECK:%1 = extractvalue { i32, i1 } %0, 1
+// CHECK:%2 = extractvalue { i32, i1 } %0, 0
+// CHECK:store i32 %2, ptr %result, align 4
+// CHECK:ret i1 %1
+// CHECK:}
+// CHECK:; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none)
+// CHECK:declare { i32, i1 } @llvm.sadd.with.overflow.i32(i32, i32) #1
+bool test_ckd_add() {
+  int result;
+  return ckd_add(, -1073741826, -1073741826);
+}
+
+// CHECK-LABEL: define

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:326
 def err_unsupported_abi_for_opt : Error<"'%0' can only be used with the '%1' 
ABI">;
+def err_unsupported_opt_for_execute_only_target
+: Error<"unsupported option '%0' for the execute only target '%1'">;

We don't need this diagnostic as a common kind (we only use it in driver).

I think we can reuse `err_drv_argument_not_allowed_with` . Though for PS5 you 
will get `... allowed with '-mexecute-only'` even if the user doesn't specify 
`-mexecute-only`, but I hope it is fine.



Comment at: clang/lib/Basic/Sanitizers.cpp:134-143
+  if ((A &&
+   A->getOption().matches(clang::driver::options::OPT_mexecute_only)) ||
+  (std::find(Features.begin(), Features.end(), "+execute-only") !=
+   Features.end())) {
+// The execute-only output is supported only on ARMv6T2 and ARMv7 and 
above.
+if (llvm::ARM::parseArchVersion(Triple.getArchName()) > 7 ||
+llvm::ARM::parseArch(Triple.getArchName()) ==

simon_tatham wrote:
> Why do we need to check //both// of `-mexecute-only` and the `+execute-only` 
> target feature? It looks to me as if the code in 
> `Driver/ToolChains/Arch/ARM.cpp` that handles `-mexecute-only` ends up 
> setting the same target feature anyway. And it checks the supported 
> architecture versions first.
> 
> Would it not be better to //just// check the target feature here, and avoid 
> having a duplicated copy of the rest of this logic which needs to be kept in 
> sync with the ARM driver?
> 
> Does something go wrong if you do it that way?
I think we only need to check the `+execute-only` target feature and remove 
driver option `-mexecute-only` check.

```
if (Triple.isPS5())
  return true;
if (!Triple.isARM() && !Triple.isThumb())
  return false;
return features contains "+execute-only" ;
```



Comment at: clang/lib/Driver/SanitizerArgs.cpp:406
+if (SanitizerMask KindsToDiagnose =
+Add & NotAllowedWithExecuteOnly & ~DiagnosedKinds) {
+  if (DiagnoseErrors)

I think it's clear not not to add the variable `NotAllowedWithExecuteOnly`.

Currently, I need to check the definition of `NotAllowedWithExecuteOnly` to 
understand that this comment does what it says. For now, just encoding 
`Function` here is clearer.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4405
+  // value of '-fsanitize=' must be `function` if function sanitizer is 
enabled.
+  if (isExecuteOnlyTarget(T, Args) &&
+  LangOpts.Sanitize.has(SanitizerKind::Function)) {

Remove.

We don't perform rigid error checking for cc1. If the user bypass the driver 
check with `-Xclang -fsanitize=function`, we don't provide more diagnostics.



Comment at: clang/test/CodeGen/ubsan-function.c:2
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s 
-o - | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -triple x86_64-sie-ps5 -fsanitize=function 
%s -o 2>&1 | FileCheck %s --check-prefix=UBSAN-FUNCTION-ERR
+// RUN: not %clang_cc1 -emit-llvm -triple armv6t2-unknown-unknown-eabi 
-target-feature +execute-only -fsanitize=function %s -o 2>&1 | FileCheck %s 
--check-prefix=UBSAN-FUNCTION-ERR

remove new tests. we only need test/Driver test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158614

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

[PATCH] D158570: [Darwin][StableABI][ASan] Remove version mismatch check from stable abi shim

2023-08-23 Thread Roy Sundahl via Phabricator via cfe-commits

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

LGTM since we ship the shim with the toolchain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158570

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

[PATCH] D61670: [clang] [MinGW] Add the option -fno-autoimport

2023-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added a comment.

In D61670#4607313 , @mstorsjo wrote:

>> I expected the answer would be "yes", so I said "lgtm" and then phrased my 
>> question very awkwardly.
>
> Ah, thanks for the clarification!
>
> Any opinion on the name, `-fno-autoimport` vs `-fno-auto-import`, given the 
> existing linker option `--disable-auto-import`?

Is there an official name, "auto-import"/"autoimport"/"auto import"? Since cc1 
flags are changeable and this hasn't landed yet, I think it'd be nice to 
standardize before we commit on a stable name.
If there isn't already a name, I like the dash in "auto-import".

In D61670#4604554 , @rnk wrote:

> In D61670#4604486 , @mstorsjo wrote:
>
>> In D61670#4604145 , @rnk wrote:
>>
>>> cc +@aeubanks @jyknight to consider using the code model for this purpose
>>
>> Hmm, I don't quite understand this comment; do you suggest that we after all 
>> should use the code model for controlling the use of `.refptr` stubs too - 
>> didn't we conclude earlier that it isn't a great fit for that?
>
> Sorry, let me elaborate. Since the first round of review, Arthur and James 
> have been looking at x86 code models, in particular the medium code model. I 
> guess what I really want to ask is, Arthur and James, do you agree with our 
> decision that the code model should not control the formation of these COFF 
> refptr stubs?
>
> I expected the answer would be "yes", so I said "lgtm" and then phrased my 
> question very awkwardly.

Yeah, at least with my understanding code models shouldn't change whether or 
not things are dso_local or not. They should only affect the instruction 
sequence we use to access globals/functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61670

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

aeubanks wrote:
> erichkeane wrote:
> > aeubanks wrote:
> > > aaron.ballman wrote:
> > > > aeubanks wrote:
> > > > > aaron.ballman wrote:
> > > > > > aeubanks wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > What happens for tentative definitions where the value isn't 
> > > > > > > > known? e.g.,
> > > > > > > > ```
> > > > > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > What happens if the types are similar but not the same?
> > > > > > > > ```
> > > > > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > Should we diagnose taking the address of such an attributed 
> > > > > > > > variable so users have some hope of spotting the non-conforming 
> > > > > > > > situations?
> > > > > > > > 
> > > > > > > > Does this attribute have impacts across translation unit 
> > > > > > > > boundaries (perhaps only when doing LTO) or only within a 
> > > > > > > > single TU?
> > > > > > > > 
> > > > > > > > What does this attribute do in C++ in the presence of 
> > > > > > > > constructors and destructors? e.g.,
> > > > > > > > ```
> > > > > > > > struct S {
> > > > > > > >   S();
> > > > > > > >   ~S();
> > > > > > > > };
> > > > > > > > 
> > > > > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and 
> > > > > > > > there's only one ctor/dtor call?
> > > > > > > > ```
> > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > > 
> > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > > 
> > > > > > > but yeah that does imply that we should warn when the attribute 
> > > > > > > is used on non const, non-POD globals. I'll update this patch to 
> > > > > > > do that
> > > > > > > 
> > > > > > > as mentioned in the description, we actually do want to take the 
> > > > > > > address of these globals for table-driven parsing, but we don't 
> > > > > > > care about identity equality
> > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > >
> > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > 
> > > > > > A that's good to know. So I assume we *will* merge these?
> > > > > > 
> > > > > > ```
> > > > > > struct S {
> > > > > >   int i, j;
> > > > > >   float f;
> > > > > > };
> > > > > > 
> > > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > > > ```
> > > > > > 
> > > > > > > but yeah that does imply that we should warn when the attribute 
> > > > > > > is used on non const, non-POD globals. I'll update this patch to 
> > > > > > > do that
> > > > > > 
> > > > > > Thank you, I think that will be more user-friendly
> > > > > > 
> > > > > > > as mentioned in the description, we actually do want to take the 
> > > > > > > address of these globals for table-driven parsing, but we don't 
> > > > > > > care about identity equality
> > > > > > 
> > > > > > Yeah, I still wonder if we want to diagnose just the same -- if the 
> > > > > > address is never taken, there's not really a way to notice the 
> > > > > > optimization, but if the address is taken, you basically get UB 
> > > > > > (and I think we should explicitly document it as such). Given how 
> > > > > > easy it is to accidentally take the address of something (like via 
> > > > > > a reference in C++), I think we should warn by default, but still 
> > > > > > have a warning group for folks who want to live life dangerously.
> > > > > > > globals are only mergeable if they're known to be constant and 
> > > > > > > have the same value/size. this can be done at compile time only 
> > > > > > > if the optimizer can see the constant values, or at link time
> > > > > > >
> > > > > > > so nothing would happen in any of the cases you've given.
> > > > > > 
> > > > > > A that's good to know. So I assume we *will* merge these?
> > > > > > 
> > > > > > ```
> > > > > > struct S {
> > > > > >   int i, j;
> > > > > >   float f;
> > > > > > };
> > > > > > 
> > > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > > [[clang::unnamed_addr]] const S s3 = s2;

[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 marked 5 inline comments as done.
jansvoboda11 added inline comments.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1169
+  writeSignature(Sig, Out);
+  std::copy_n(Out.begin(), Out.size(), Buffer.begin() + Offset);
+};

benlangmuir wrote:
> jansvoboda11 wrote:
> > I don't feel great about removing `const` from `Buffer` and writing into it 
> > directly, circumventing `Stream`. This currently works fine, because the 
> > `Stream` in `ASTWriter` is never backed by a file (and therefore never 
> > flushed). But if that ever changes, this code is problematic. Do you think 
> > this is worth spending more time on?
> > 
> > `Stream` already has the `BackpatchWord()` function, which makes sure the 
> > underlying file is updated as well in case we're backpatching 
> > already-flushed data.
> Interesting; what was the reason for not using BackpatchWord from the start? 
> IIUC our signatures should be a multiple of 4 bytes already.
Not sure how I arrived at this. This is fixed in the latest revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158573

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

[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 updated this revision to Diff 552897.
jansvoboda11 added a comment.

Early return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158573

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/test/Modules/ASTSignature.c

Index: clang/test/Modules/ASTSignature.c
===
--- clang/test/Modules/ASTSignature.c
+++ clang/test/Modules/ASTSignature.c
@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
-// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
 // RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
 // RUN: cp %t/MyHeader2.pcm %t1.pcm
 // RUN: rm -rf %t
@@ -8,17 +8,18 @@
 // RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
 // RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
 // RUN: cp %t/MyHeader2.pcm %t2.pcm
-// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm > %t1.dump
-// RUN: llvm-bcanalyzer --dump --disable-histogram %t2.pcm > %t2.dump
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t1.pcm > %t1.dump
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t2.pcm > %t2.dump
 // RUN: cat %t1.dump %t2.dump | FileCheck %s
 
 #include "my_header_2.h"
 
 my_int var = 42;
 
-// CHECK: [[AST_BLOCK_HASH:]]
-// CHECK: [[SIGNATURE:]]
-// CHECK: [[AST_BLOCK_HASH]]
-// CHECK-NOT: [[SIGNATURE]]
-// The modules built by this test are designed to yield the same AST. If this
-// test fails, it means that the AST block is has become non-relocatable.
+// CHECK:  blob data = '[[AST_BLOCK_HASH:.*]]'
+// CHECK:  blob data = '[[SIGNATURE:.*]]'
+// CHECK:  blob data = '[[AST_BLOCK_HASH]]'
+// CHECK-NOT:  blob data = '[[SIGNATURE]]'
+// The modules built by this test are designed to yield the same AST but distinct AST files.
+// If this test fails, it means that either the AST block has become non-relocatable,
+// or the file signature stopped hashing some parts of the AST file.
Index: clang/lib/Serialization/GlobalModuleIndex.cpp
===
--- clang/lib/Serialization/GlobalModuleIndex.cpp
+++ clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "llvm/ADT/DenseMap.h"
@@ -697,9 +698,12 @@
 }
 
 // Get Signature.
-if (State == DiagnosticOptionsBlock && Code == SIGNATURE)
-  getModuleFileInfo(File).Signature = ASTFileSignature::create(
-  Record.begin(), Record.begin() + ASTFileSignature::size);
+if (State == DiagnosticOptionsBlock && Code == SIGNATURE) {
+  auto Signature = ASTFileSignature::create(Blob.begin(), Blob.end());
+  assert(Signature != ASTFileSignature::createDummy() &&
+ "Dummy AST file signature not backpatched in ASTWriter.");
+  getModuleFileInfo(File).Signature = Signature;
+}
 
 // We don't care about this record.
   }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1120,50 +1120,96 @@
 }
 
 std::pair
-ASTWriter::createSignature(StringRef AllBytes, StringRef ASTBlockBytes) {
+ASTWriter::createSignature() const {
+  StringRef AllBytes(Buffer.data(), Buffer.size());
+
   llvm::SHA1 Hasher;
-  Hasher.update(ASTBlockBytes);
+  Hasher.update(AllBytes.slice(ASTBlockRange.first, ASTBlockRange.second));
   ASTFileSignature ASTBlockHash = ASTFileSignature::create(Hasher.result());
 
-  // Add the remaining bytes (i.e. bytes before the unhashed control block that
-  // are not part of the AST block).
-  Hasher.update(
-  AllBytes.take_front(ASTBlockBytes.bytes_end() - AllBytes.bytes_begin()));
+  // Add the remaining bytes:
+  //  1. Before the unhashed control block.
+  Hasher.update(AllBytes.slice(0, UnhashedControlBlockRange.first));
+  //  2. Between the unhashed control block and the AST block.
   Hasher.update(
-  AllBytes.take_back(AllBytes.bytes_end() - ASTBlockBytes.bytes_end()));
+  AllBytes.slice(UnhashedControlBlockRange.second, ASTBlockRange.first));
+  //  3. After the AST block.
+  Hasher.update(AllBytes.slice(ASTBlockRange.second, StringRef::npos));

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-23 Thread Ian Anderson via Phabricator via cfe-commits

iana added a comment.

In D157757#4611492 , @iana wrote:

> In D157757#4611425 , @ldionne wrote:
>
>> Also we should figure out why the Clang modules build is failing on top of 
>> this, it doesn't look like a false positive.
>
> Where do you see that failure?

Never mind, I see it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D158573: [clang][modules] Move `UNHASHED_CONTROL_BLOCK` up in the AST file

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 updated this revision to Diff 552896.
jansvoboda11 added a comment.

Use `Stream::BackpatchWord()` instead of manipulating `Buffer` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158573

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/test/Modules/ASTSignature.c

Index: clang/test/Modules/ASTSignature.c
===
--- clang/test/Modules/ASTSignature.c
+++ clang/test/Modules/ASTSignature.c
@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
-// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
 // RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
 // RUN: cp %t/MyHeader2.pcm %t1.pcm
 // RUN: rm -rf %t
@@ -8,17 +8,18 @@
 // RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
 // RUN:   -fmodules-cache-path=%t -fdisable-module-hash %s
 // RUN: cp %t/MyHeader2.pcm %t2.pcm
-// RUN: llvm-bcanalyzer --dump --disable-histogram %t1.pcm > %t1.dump
-// RUN: llvm-bcanalyzer --dump --disable-histogram %t2.pcm > %t2.dump
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t1.pcm > %t1.dump
+// RUN: llvm-bcanalyzer --dump --disable-histogram --show-binary-blobs %t2.pcm > %t2.dump
 // RUN: cat %t1.dump %t2.dump | FileCheck %s
 
 #include "my_header_2.h"
 
 my_int var = 42;
 
-// CHECK: [[AST_BLOCK_HASH:]]
-// CHECK: [[SIGNATURE:]]
-// CHECK: [[AST_BLOCK_HASH]]
-// CHECK-NOT: [[SIGNATURE]]
-// The modules built by this test are designed to yield the same AST. If this
-// test fails, it means that the AST block is has become non-relocatable.
+// CHECK:  blob data = '[[AST_BLOCK_HASH:.*]]'
+// CHECK:  blob data = '[[SIGNATURE:.*]]'
+// CHECK:  blob data = '[[AST_BLOCK_HASH]]'
+// CHECK-NOT:  blob data = '[[SIGNATURE]]'
+// The modules built by this test are designed to yield the same AST but distinct AST files.
+// If this test fails, it means that either the AST block has become non-relocatable,
+// or the file signature stopped hashing some parts of the AST file.
Index: clang/lib/Serialization/GlobalModuleIndex.cpp
===
--- clang/lib/Serialization/GlobalModuleIndex.cpp
+++ clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -15,6 +15,7 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "llvm/ADT/DenseMap.h"
@@ -697,9 +698,12 @@
 }
 
 // Get Signature.
-if (State == DiagnosticOptionsBlock && Code == SIGNATURE)
-  getModuleFileInfo(File).Signature = ASTFileSignature::create(
-  Record.begin(), Record.begin() + ASTFileSignature::size);
+if (State == DiagnosticOptionsBlock && Code == SIGNATURE) {
+  auto Signature = ASTFileSignature::create(Blob.begin(), Blob.end());
+  assert(Signature != ASTFileSignature::createDummy() &&
+ "Dummy AST file signature not backpatched in ASTWriter.");
+  getModuleFileInfo(File).Signature = Signature;
+}
 
 // We don't care about this record.
   }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1120,50 +1120,96 @@
 }
 
 std::pair
-ASTWriter::createSignature(StringRef AllBytes, StringRef ASTBlockBytes) {
+ASTWriter::createSignature() const {
+  StringRef AllBytes(Buffer.data(), Buffer.size());
+
   llvm::SHA1 Hasher;
-  Hasher.update(ASTBlockBytes);
+  Hasher.update(AllBytes.slice(ASTBlockRange.first, ASTBlockRange.second));
   ASTFileSignature ASTBlockHash = ASTFileSignature::create(Hasher.result());
 
-  // Add the remaining bytes (i.e. bytes before the unhashed control block that
-  // are not part of the AST block).
-  Hasher.update(
-  AllBytes.take_front(ASTBlockBytes.bytes_end() - AllBytes.bytes_begin()));
+  // Add the remaining bytes:
+  //  1. Before the unhashed control block.
+  Hasher.update(AllBytes.slice(0, UnhashedControlBlockRange.first));
+  //  2. Between the unhashed control block and the AST block.
   Hasher.update(
-  AllBytes.take_back(AllBytes.bytes_end() - ASTBlockBytes.bytes_end()));
+  AllBytes.slice(UnhashedControlBlockRange.second, ASTBlockRange.first));
+  //  3. After the AST block.
+

[clang] 3197357 - Revert "[clang] Match -isysroot behaviour with system compiler on Darwin"

2023-08-23 Thread Alex Lorenz via cfe-commits


Author: Alex Lorenz
Date: 2023-08-23T14:50:02-07:00
New Revision: 3197357b7e39a58bc7eb0600eb337ac2a1c8c225

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

LOG: Revert "[clang] Match -isysroot behaviour with system compiler on Darwin"

This reverts commit f24aa691aa4f25291db8f7c61c6e9007288859e7.

This change caused these two test failures on Darwin CI:
Clang.Tooling.clang-check-mac-libcxx-abspath.cpp
Clang.Tooling.clang-check-mac-libcxx-relpath.cpp

https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/37169/

More info in https://reviews.llvm.org/D157283

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-header-search-libcxx.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 5d8f006252fd9d..e45424a5f712a0 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2465,7 +2465,8 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
   //Also check whether this is used for setting library search paths.
   ToolChain::AddClangCXXStdlibIncludeArgs(DriverArgs, CC1Args);
 
-  if (DriverArgs.hasArg(options::OPT_nostdlibinc, options::OPT_nostdincxx))
+  if (DriverArgs.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
+options::OPT_nostdincxx))
 return;
 
   llvm::SmallString<128> Sysroot = GetEffectiveSysroot(DriverArgs);
@@ -2473,8 +2474,8 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
   switch (GetCXXStdlibType(DriverArgs)) {
   case ToolChain::CST_Libcxx: {
 // On Darwin, libc++ can be installed in one of the following two places:
-// 1. In a SDK (or a custom sysroot) in /usr/include/c++/v1
-// 2. Alongside the compiler in /include/c++/v1
+// 1. Alongside the compiler in /include/c++/v1
+// 2. In a SDK (or a custom sysroot) in /usr/include/c++/v1
 //
 // The precendence of paths is as listed above, i.e. we take the first path
 // that exists. Also note that we never include libc++ twice -- we take the
@@ -2482,17 +2483,6 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
 // include_next could break).
 
 // Check for (1)
-llvm::SmallString<128> SysrootUsr = Sysroot;
-llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1");
-if (getVFS().exists(SysrootUsr)) {
-  addSystemInclude(DriverArgs, CC1Args, SysrootUsr);
-  return;
-} else if (DriverArgs.hasArg(options::OPT_v)) {
-  llvm::errs() << "ignoring nonexistent directory \"" << SysrootUsr
-   << "\"\n";
-}
-
-// Otherwise, check for (2)
 // Get from '/bin' to '/include/c++/v1'.
 // Note that InstallBin can be relative, so we use '..' instead of
 // parent_path.
@@ -2507,6 +2497,17 @@ void DarwinClang::AddClangCXXStdlibIncludeArgs(
<< "\"\n";
 }
 
+// Otherwise, check for (2)
+llvm::SmallString<128> SysrootUsr = Sysroot;
+llvm::sys::path::append(SysrootUsr, "usr", "include", "c++", "v1");
+if (getVFS().exists(SysrootUsr)) {
+  addSystemInclude(DriverArgs, CC1Args, SysrootUsr);
+  return;
+} else if (DriverArgs.hasArg(options::OPT_v)) {
+  llvm::errs() << "ignoring nonexistent directory \"" << SysrootUsr
+   << "\"\n";
+}
+
 // Otherwise, don't add any path.
 break;
   }

diff  --git a/clang/test/Driver/darwin-header-search-libcxx.cpp 
b/clang/test/Driver/darwin-header-search-libcxx.cpp
index 4929511cd00af6..cc8ec9ceb89b3a 100644
--- a/clang/test/Driver/darwin-header-search-libcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libcxx.cpp
@@ -53,7 +53,7 @@
 // CHECK-LIBCXX-SYSROOT-1-NOT: "-internal-isystem" 
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
 
 // Check with both headers in the sysroot and headers alongside the 
installation
-// (the  headers should be preferred over the headers in the 
toolchain).
+// (the headers in the toolchain should be preferred over the  
headers).
 // Ensure that both -isysroot and --sysroot work, and that isysroot has 
precedence
 // over --sysroot.
 //
@@ -89,10 +89,10 @@
 // RUN:   --check-prefix=CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1 %s
 //
 // CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-cc1"
-// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/v1"
-// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" 
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1: "-internal-isystem" 
"[[TOOLCHAIN]]/usr/bin/../include/c++/v1"
+// CHECK-LIBCXX-SYSROOT_AND_TOOLCHAIN-1-NOT: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/v1"
 
-// Make sure that using -nostdinc++ or -nostdlib will drop both the

[PATCH] D158572: [clang][modules] Use relative offsets for input files

2023-08-23 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 updated this revision to Diff 552894.
jansvoboda11 added a comment.

Initialize absolute offsets in `ASTReader`/`ModuleFile`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158572

Files:
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - 
InputFilesOffsetBase);
 
 InputFileID = InputFileOffsets.size();
 
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2326,7 +2326,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2410,7 +2411,8 @@
   // Go find this input file.
   BitstreamCursor  = F.InputFilesCursor;
   SavedStreamPosition SavedPosition(Cursor);
-  if (llvm::Error Err = Cursor.JumpToBit(F.InputFileOffsets[ID - 1])) {
+  if (llvm::Error Err = Cursor.JumpToBit(F.InputFilesOffsetBase +
+ F.InputFileOffsets[ID - 1])) {
 // FIXME this drops errors on the floor.
 consumeError(std::move(Err));
   }
@@ -2788,6 +2790,7 @@
   Error("malformed block record in AST file");
   return Failure;
 }
+F.InputFilesOffsetBase = F.InputFilesCursor.GetCurrentBitNo();
 continue;
 
   case OPTIONS_BLOCK_ID:
@@ -5328,6 +5331,7 @@
   bool NeedsSystemInputFiles = Listener.needsSystemInputFileVisitation();
   bool NeedsImports = Listener.needsImportVisitation();
   BitstreamCursor InputFilesCursor;
+  uint64_t InputFilesOffsetBase = 0;
 
   RecordData Record;
   std::string ModuleDir;
@@ -5363,6 +5367,7 @@
 if (NeedsInputFiles &&
 ReadBlockAbbrevs(InputFilesCursor, INPUT_FILES_BLOCK_ID))
   return true;
+InputFilesOffsetBase = InputFilesCursor.GetCurrentBitNo();
 break;
 
   default:
@@ -5435,7 +5440,8 @@
 
 BitstreamCursor  = InputFilesCursor;
 SavedStreamPosition SavedPosition(Cursor);
-if (llvm::Error Err = Cursor.JumpToBit(InputFileOffs[I])) {
+if (llvm::Error Err =
+Cursor.JumpToBit(InputFilesOffsetBase + InputFileOffs[I])) {
   // FIXME this drops errors on the floor.
   consumeError(std::move(Err));
 }
Index: clang/include/clang/Serialization/ModuleFile.h
===
--- clang/include/clang/Serialization/ModuleFile.h
+++ clang/include/clang/Serialization/ModuleFile.h
@@ -245,7 +245,10 @@
   /// The cursor to the start of the input-files block.
   llvm::BitstreamCursor InputFilesCursor;
 
-  /// Offsets for all of the input file entries in the AST file.
+  /// Absolute offset of the start of the input-files block.
+  uint64_t InputFilesOffsetBase = 0;
+
+  /// Relative offsets for all of the input file entries in the AST file.
   const llvm::support::unaligned_uint64_t *InputFileOffsets = nullptr;
 
   /// The input files that have been loaded from this AST file.


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1570,6 +1570,8 @@
   IFHAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned IFHAbbrevCode = Stream.EmitAbbrev(std::move(IFHAbbrev));
 
+  uint64_t InputFilesOffsetBase = Stream.GetCurrentBitNo();
+
   // Get all ContentCache objects for files.
   std::vector UserFiles;
   std::vector SystemFiles;
@@ -1633,7 +1635,7 @@
   continue; // already recorded this file.
 
 // Record this entry's offset.
-InputFileOffsets.push_back(Stream.GetCurrentBitNo());
+InputFileOffsets.push_back(Stream.GetCurrentBitNo() - InputFilesOffsetBase);

[PATCH] D157283: [clang] Match -isysroot behaviour with system compiler on Darwin

2023-08-23 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman added a comment.

I will revert this for now, as this change broke Darwin CI:

https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/37169/

Clang.Tooling.clang-check-mac-libcxx-abspath.cpp
Clang.Tooling.clang-check-mac-libcxx-relpath.cpp

@ldionne has a patch for these tests I believe, please coordinate with him 
before recommitting this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157283

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

erichkeane wrote:
> aeubanks wrote:
> > aaron.ballman wrote:
> > > aeubanks wrote:
> > > > aaron.ballman wrote:
> > > > > aeubanks wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > What happens for tentative definitions where the value isn't 
> > > > > > > known? e.g.,
> > > > > > > ```
> > > > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > > > ```
> > > > > > > 
> > > > > > > What happens if the types are similar but not the same?
> > > > > > > ```
> > > > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > > > ```
> > > > > > > 
> > > > > > > Should we diagnose taking the address of such an attributed 
> > > > > > > variable so users have some hope of spotting the non-conforming 
> > > > > > > situations?
> > > > > > > 
> > > > > > > Does this attribute have impacts across translation unit 
> > > > > > > boundaries (perhaps only when doing LTO) or only within a single 
> > > > > > > TU?
> > > > > > > 
> > > > > > > What does this attribute do in C++ in the presence of 
> > > > > > > constructors and destructors? e.g.,
> > > > > > > ```
> > > > > > > struct S {
> > > > > > >   S();
> > > > > > >   ~S();
> > > > > > > };
> > > > > > > 
> > > > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's 
> > > > > > > only one ctor/dtor call?
> > > > > > > ```
> > > > > > globals are only mergeable if they're known to be constant and have 
> > > > > > the same value/size. this can be done at compile time only if the 
> > > > > > optimizer can see the constant values, or at link time
> > > > > > 
> > > > > > so nothing would happen in any of the cases you've given.
> > > > > > 
> > > > > > but yeah that does imply that we should warn when the attribute is 
> > > > > > used on non const, non-POD globals. I'll update this patch to do 
> > > > > > that
> > > > > > 
> > > > > > as mentioned in the description, we actually do want to take the 
> > > > > > address of these globals for table-driven parsing, but we don't 
> > > > > > care about identity equality
> > > > > > globals are only mergeable if they're known to be constant and have 
> > > > > > the same value/size. this can be done at compile time only if the 
> > > > > > optimizer can see the constant values, or at link time
> > > > > >
> > > > > > so nothing would happen in any of the cases you've given.
> > > > > 
> > > > > A that's good to know. So I assume we *will* merge these?
> > > > > 
> > > > > ```
> > > > > struct S {
> > > > >   int i, j;
> > > > >   float f;
> > > > > };
> > > > > 
> > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > > ```
> > > > > 
> > > > > > but yeah that does imply that we should warn when the attribute is 
> > > > > > used on non const, non-POD globals. I'll update this patch to do 
> > > > > > that
> > > > > 
> > > > > Thank you, I think that will be more user-friendly
> > > > > 
> > > > > > as mentioned in the description, we actually do want to take the 
> > > > > > address of these globals for table-driven parsing, but we don't 
> > > > > > care about identity equality
> > > > > 
> > > > > Yeah, I still wonder if we want to diagnose just the same -- if the 
> > > > > address is never taken, there's not really a way to notice the 
> > > > > optimization, but if the address is taken, you basically get UB (and 
> > > > > I think we should explicitly document it as such). Given how easy it 
> > > > > is to accidentally take the address of something (like via a 
> > > > > reference in C++), I think we should warn by default, but still have 
> > > > > a warning group for folks who want to live life dangerously.
> > > > > > globals are only mergeable if they're known to be constant and have 
> > > > > > the same value/size. this can be done at compile time only if the 
> > > > > > optimizer can see the constant values, or at link time
> > > > > >
> > > > > > so nothing would happen in any of the cases you've given.
> > > > > 
> > > > > A that's good to know. So I assume we *will* merge these?
> > > > > 
> > > > > ```
> > > > > struct S {
> > > > >   int i, j;
> > > > >   float f;
> > > > > };
> > > > > 
> > > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > > ```
> > > > yeah those are merged even just by clang
> > > > 
> > > > ```
> > > > struct S {
> > > >   int i, j;
> > > >   float f;
> > > > };
> > > > 
> > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> >

[PATCH] D151575: [clang][diagnostics] Always show include stacks on top-level diagnostics

2023-08-23 Thread Christopher Di Bella via Phabricator via cfe-commits

cjdb added a comment.

In D151575#4607047 , @aaron.ballman 
wrote:

> I am wrestling with this one because I think the status quo is unfortunate 
> (we silently drop relevant information in ways the user may not immediately 
> understand) but I think always printing the include stack could be verbose 
> and thus make the diagnostics harder to act on. If we had the ability to 
> hyperlink to other parts of the diagnostic output, then I'd ask if we could 
> perhaps print the full stack once and link to it from anywhere else it's 
> being used, but I can't think of an effective way to do that purely in text. 
> We could also add an explicit option to let the user control the behavior, 
> but I'd prefer to avoid that if possible as the combination of modes makes 
> testing a challenge.
>
> Maybe we should consider adding a flag to set diagnostic verbosity level? 
> e.g., `-fdiagnostic-verbosity=terse|default|verbose` where `terse` never 
> prints notes or include stacks, `default` is what we do today, and `verbose` 
> always prints include stacks?

I'm not fond of `terse` unless each note is printed once: I think that'll get 
misused and lead to a lot of frustration (most notes are genuinely helpful). 
There's a lot of design space here, but I don't think any of it can be achieved 
until we have nested diagnostics, but that requires a complete rewrite of the 
diagnostics engine.

`default` and `verbose` might be interesting though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151575

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

aeubanks wrote:
> aaron.ballman wrote:
> > aeubanks wrote:
> > > aaron.ballman wrote:
> > > > aeubanks wrote:
> > > > > aaron.ballman wrote:
> > > > > > What happens for tentative definitions where the value isn't known? 
> > > > > > e.g.,
> > > > > > ```
> > > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > > ```
> > > > > > 
> > > > > > What happens if the types are similar but not the same?
> > > > > > ```
> > > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > > ```
> > > > > > 
> > > > > > Should we diagnose taking the address of such an attributed 
> > > > > > variable so users have some hope of spotting the non-conforming 
> > > > > > situations?
> > > > > > 
> > > > > > Does this attribute have impacts across translation unit boundaries 
> > > > > > (perhaps only when doing LTO) or only within a single TU?
> > > > > > 
> > > > > > What does this attribute do in C++ in the presence of constructors 
> > > > > > and destructors? e.g.,
> > > > > > ```
> > > > > > struct S {
> > > > > >   S();
> > > > > >   ~S();
> > > > > > };
> > > > > > 
> > > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's 
> > > > > > only one ctor/dtor call?
> > > > > > ```
> > > > > globals are only mergeable if they're known to be constant and have 
> > > > > the same value/size. this can be done at compile time only if the 
> > > > > optimizer can see the constant values, or at link time
> > > > > 
> > > > > so nothing would happen in any of the cases you've given.
> > > > > 
> > > > > but yeah that does imply that we should warn when the attribute is 
> > > > > used on non const, non-POD globals. I'll update this patch to do that
> > > > > 
> > > > > as mentioned in the description, we actually do want to take the 
> > > > > address of these globals for table-driven parsing, but we don't care 
> > > > > about identity equality
> > > > > globals are only mergeable if they're known to be constant and have 
> > > > > the same value/size. this can be done at compile time only if the 
> > > > > optimizer can see the constant values, or at link time
> > > > >
> > > > > so nothing would happen in any of the cases you've given.
> > > > 
> > > > A that's good to know. So I assume we *will* merge these?
> > > > 
> > > > ```
> > > > struct S {
> > > >   int i, j;
> > > >   float f;
> > > > };
> > > > 
> > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > ```
> > > > 
> > > > > but yeah that does imply that we should warn when the attribute is 
> > > > > used on non const, non-POD globals. I'll update this patch to do that
> > > > 
> > > > Thank you, I think that will be more user-friendly
> > > > 
> > > > > as mentioned in the description, we actually do want to take the 
> > > > > address of these globals for table-driven parsing, but we don't care 
> > > > > about identity equality
> > > > 
> > > > Yeah, I still wonder if we want to diagnose just the same -- if the 
> > > > address is never taken, there's not really a way to notice the 
> > > > optimization, but if the address is taken, you basically get UB (and I 
> > > > think we should explicitly document it as such). Given how easy it is 
> > > > to accidentally take the address of something (like via a reference in 
> > > > C++), I think we should warn by default, but still have a warning group 
> > > > for folks who want to live life dangerously.
> > > > > globals are only mergeable if they're known to be constant and have 
> > > > > the same value/size. this can be done at compile time only if the 
> > > > > optimizer can see the constant values, or at link time
> > > > >
> > > > > so nothing would happen in any of the cases you've given.
> > > > 
> > > > A that's good to know. So I assume we *will* merge these?
> > > > 
> > > > ```
> > > > struct S {
> > > >   int i, j;
> > > >   float f;
> > > > };
> > > > 
> > > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > > [[clang::unnamed_addr]] const S s3 = s2;
> > > > ```
> > > yeah those are merged even just by clang
> > > 
> > > ```
> > > struct S {
> > >   int i, j;
> > >   float f;
> > > };
> > > 
> > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s3 = s2;
> > > 
> > > const void * f1() {
> > >   return 
> > > }
> > > 
> > > const void * f2() {
> > >   return 
> > > }
> > > 
> > > const void * f3() {
> > >   return 
> > > }
> >

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D158641#4611731 , @rprichard wrote:

> [...]
>
> I suspect Bionic ought to apply relocations to libraries in a bottom-up 
> fashion, so that libc.so is relocated before the executable or shared 
> objects, but I _think_ it's currently top-down. Deferring ifunc relocations 
> until after non-ifunc relocations are applied is a separate problem.

https://maskray.me/blog/2021-01-18-gnu-indirect-function#relocation-resolving-order
 glibc uses the bottom-up fashion. I agree that adopting the bottom-up fashion 
for Bionic will be great.

"l_initfini contains main, dep1.so, dep2.so, dep4.so, dep3.so, libc.so.6, 
ld.so. The relocation resolving order is ld.so (bootstrap), libc.so.6, dep3.so, 
dep4.so, dep2.so, dep1.so, main, ld.so."

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641

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

[libunwind] 5eb44df - Make _LIBUNWIND_SUPPORT_FRAME_APIS a build-time option

2023-08-23 Thread Sterling Augustine via cfe-commits


Author: Sterling Augustine
Date: 2023-08-23T14:34:40-07:00
New Revision: 5eb44df1b64dbd1a86b099128092a7fd2001c0ba

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

LOG: Make _LIBUNWIND_SUPPORT_FRAME_APIS a build-time option

Previously this was based on target architecture, but
that makes very little sense--frame API availability is generally
for libgcc compatibility and that is dependent on runtime
needs rather than target architecture.

Default this to on, so as not to remove the apis from
environments that already have them.

The functions this macro protects are stubs for libgcc-compatibility.
Today, libunwind as a drop-in replacement for libgcc_eh links on x86,
x86_64, and powerpc, but not aarch64, which doesn't really make
sense. As there is nothing architecture specific about these, they
should be provided everywhere or nowhere.

The target-specific protection goes all the way back to the original
code contribution in 312fcd0e1cf14482b2ae8eee8234541dcc3bc2c4 from
2013, so the original reason is lost to history, and probably not
relevant today.

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

Added: 


Modified: 
libunwind/CMakeLists.txt
libunwind/src/config.h

Removed: 




diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index bc2a820fe98ebb..84f8ce296a7410 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -51,6 +51,7 @@ option(LIBUNWIND_IS_BAREMETAL "Build libunwind for baremetal 
targets." OFF)
 option(LIBUNWIND_USE_FRAME_HEADER_CACHE "Cache frame headers for unwinding. 
Requires locking dl_iterate_phdr." OFF)
 option(LIBUNWIND_REMEMBER_HEAP_ALLOC "Use heap instead of the stack for 
.cfi_remember_state." OFF)
 option(LIBUNWIND_INSTALL_HEADERS "Install the libunwind headers." ON)
+option(LIBUNWIND_ENABLE_FRAME_APIS "Include libgcc-compatible frame apis." OFF)
 
 set(LIBUNWIND_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}" CACHE STRING
 "Define suffix of library directory name (32/64)")
@@ -250,6 +251,11 @@ if (NOT LIBUNWIND_ENABLE_CROSS_UNWINDING)
   add_compile_flags(-D_LIBUNWIND_IS_NATIVE_ONLY)
 endif()
 
+# Include stubs for __register_frame_info_bases and related
+if (LIBUNWIND_ENABLE_FRAME_APIS)
+  add_compile_flags(-D_LIBUNWIND_SUPPORT_FRAME_APIS)
+endif()
+
 # Threading-support
 if (NOT LIBUNWIND_ENABLE_THREADS)
   add_compile_flags(-D_LIBUNWIND_HAS_NO_THREADS)

diff  --git a/libunwind/src/config.h b/libunwind/src/config.h
index 6707d591361dfc..ef6b8d67181b4e 100644
--- a/libunwind/src/config.h
+++ b/libunwind/src/config.h
@@ -108,10 +108,6 @@
 #define _LIBUNWIND_BUILD_SJLJ_APIS
 #endif
 
-#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__)
-#define _LIBUNWIND_SUPPORT_FRAME_APIS
-#endif
-
 #if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) ||
\
 (!defined(__APPLE__) && defined(__arm__)) || defined(__aarch64__) ||   
\
 defined(__mips__) || defined(__riscv) || defined(__hexagon__) ||   
\



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

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

2023-08-23 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: clang/test/Driver/aarch64-features.c:10
 // Check Function Multi Versioning option and rtlib dependency.
-// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \
+// RUN: %clang --target=aarch64-linux-android23 -rtlib=compiler-rt \
 // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV %s

Deleting blank lines for the 3 sets of Android tests (i.e. grouping them 
together) seems to improve readability.



Comment at: compiler-rt/lib/builtins/cpu_model.c:1382
+return;
+#if defined(__ANDROID__)
+  // ifunc resolvers don't have hwcaps in arguments on Android API lower

I am unfamiliar with how Android ndk builds compiler-rt.

If `__ANDROID_API__ >= 30`, shall we use the regular Linux code path?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641

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

[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

2023-08-23 Thread Ryan Prichard via Phabricator via cfe-commits

rprichard added a comment.

> further confirmation if android_get_device_api_level should work from 
> ifunc_resolver

IIRC an ifunc resolver in Bionic can't generally call any functions in libc, 
including `android_get_device_api_level` or `__system_property_get`, because 
the ifunc resolver will typically be called before relocations in libc.so have 
been resolved. I believe an ifunc resolver also can't call 
`__system_property_get` because the libc.so system properties are initialized 
by a constructor function, which isn't called until after relocations are 
applied (`__libc_preinit` -> `__libc_preinit_impl` -> `__libc_init_common` -> 
`__system_properties_init`).

I suspect Bionic ought to apply relocations to libraries in a bottom-up 
fashion, so that libc.so is relocated before the executable or shared objects, 
but I _think_ it's currently top-down. Deferring ifunc relocations until after 
non-ifunc relocations are applied is a separate problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641

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

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

aaron.ballman wrote:
> aeubanks wrote:
> > aaron.ballman wrote:
> > > aeubanks wrote:
> > > > aaron.ballman wrote:
> > > > > What happens for tentative definitions where the value isn't known? 
> > > > > e.g.,
> > > > > ```
> > > > > [[clang::unnamed_addr]] int i1, i2;
> > > > > ```
> > > > > 
> > > > > What happens if the types are similar but not the same?
> > > > > ```
> > > > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > > > ```
> > > > > 
> > > > > Should we diagnose taking the address of such an attributed variable 
> > > > > so users have some hope of spotting the non-conforming situations?
> > > > > 
> > > > > Does this attribute have impacts across translation unit boundaries 
> > > > > (perhaps only when doing LTO) or only within a single TU?
> > > > > 
> > > > > What does this attribute do in C++ in the presence of constructors 
> > > > > and destructors? e.g.,
> > > > > ```
> > > > > struct S {
> > > > >   S();
> > > > >   ~S();
> > > > > };
> > > > > 
> > > > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's 
> > > > > only one ctor/dtor call?
> > > > > ```
> > > > globals are only mergeable if they're known to be constant and have the 
> > > > same value/size. this can be done at compile time only if the optimizer 
> > > > can see the constant values, or at link time
> > > > 
> > > > so nothing would happen in any of the cases you've given.
> > > > 
> > > > but yeah that does imply that we should warn when the attribute is used 
> > > > on non const, non-POD globals. I'll update this patch to do that
> > > > 
> > > > as mentioned in the description, we actually do want to take the 
> > > > address of these globals for table-driven parsing, but we don't care 
> > > > about identity equality
> > > > globals are only mergeable if they're known to be constant and have the 
> > > > same value/size. this can be done at compile time only if the optimizer 
> > > > can see the constant values, or at link time
> > > >
> > > > so nothing would happen in any of the cases you've given.
> > > 
> > > A that's good to know. So I assume we *will* merge these?
> > > 
> > > ```
> > > struct S {
> > >   int i, j;
> > >   float f;
> > > };
> > > 
> > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s3 = s2;
> > > ```
> > > 
> > > > but yeah that does imply that we should warn when the attribute is used 
> > > > on non const, non-POD globals. I'll update this patch to do that
> > > 
> > > Thank you, I think that will be more user-friendly
> > > 
> > > > as mentioned in the description, we actually do want to take the 
> > > > address of these globals for table-driven parsing, but we don't care 
> > > > about identity equality
> > > 
> > > Yeah, I still wonder if we want to diagnose just the same -- if the 
> > > address is never taken, there's not really a way to notice the 
> > > optimization, but if the address is taken, you basically get UB (and I 
> > > think we should explicitly document it as such). Given how easy it is to 
> > > accidentally take the address of something (like via a reference in C++), 
> > > I think we should warn by default, but still have a warning group for 
> > > folks who want to live life dangerously.
> > > > globals are only mergeable if they're known to be constant and have the 
> > > > same value/size. this can be done at compile time only if the optimizer 
> > > > can see the constant values, or at link time
> > > >
> > > > so nothing would happen in any of the cases you've given.
> > > 
> > > A that's good to know. So I assume we *will* merge these?
> > > 
> > > ```
> > > struct S {
> > >   int i, j;
> > >   float f;
> > > };
> > > 
> > > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > > [[clang::unnamed_addr]] const S s3 = s2;
> > > ```
> > yeah those are merged even just by clang
> > 
> > ```
> > struct S {
> >   int i, j;
> >   float f;
> > };
> > 
> > [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> > [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> > [[clang::unnamed_addr]] const S s3 = s2;
> > 
> > const void * f1() {
> >   return 
> > }
> > 
> > const void * f2() {
> >   return 
> > }
> > 
> > const void * f3() {
> >   return 
> > }
> > 
> > $ ./build/rel/bin/clang++ -S -emit-llvm -o - -O2 /tmp/a.cc
> > ```
> > > 
> > > > but yeah that does imply that we should warn when the attribute is used 
> > > > on non const, non-POD globals. I'll update this patch to do that
> > > 
> > > Thank you, I think that will

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks updated this revision to Diff 552889.
aeubanks edited the summary of this revision.
aeubanks added a comment.

add warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/unnamed-addr.c
  clang/test/SemaCXX/unnamed-addr.cpp

Index: clang/test/SemaCXX/unnamed-addr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/unnamed-addr.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo {
+  int a;
+};
+
+struct Bar {
+  int a;
+  Bar();
+};
+
+[[clang::unnamed_addr]] const int i = 8;
+
+[[clang::unnamed_addr]] int i2 = 8; // expected-warning{{unnamed_addr should only be used on const POD (plain old data) globals}}
+
+[[clang::unnamed_addr]] const Foo j = {2};
+
+[[clang::unnamed_addr]] Foo j2 = {2}; // expected-warning{{unnamed_addr should only be used on const POD (plain old data) globals}}
+
+[[clang::unnamed_addr]] const Bar k; // expected-warning{{unnamed_addr should only be used on const POD (plain old data) globals}}
Index: clang/test/CodeGen/unnamed-addr.c
===
--- /dev/null
+++ clang/test/CodeGen/unnamed-addr.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O1 -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s
+
+// CHECK: @i = unnamed_addr constant i32 8
+
+[[clang::unnamed_addr]] const int i = 8;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14519,6 +14519,13 @@
   PragmaClangRelroSection.PragmaLocation));
   }
 
+  if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() &&
+  VD->hasAttr() &&
+  (!VD->getType().isPODType(getASTContext()) ||
+   !VD->getType().isConstQualified())) {
+Diag(VD->getLocation(), diag::warn_attribute_unnamed_addr);
+  }
+
   if (auto *DD = dyn_cast(ThisDecl)) {
 for (auto *BD : DD->bindings()) {
   FinalizeDeclaration(BD);
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5255,6 +5255,9 @@
   GV->setConstant(true);
   }
 
+  if (D->hasAttr())
+GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
   CharUnits AlignVal = getContext().getDeclAlign(D);
   // Check for alignment specifed in an 'omp allocate' directive.
   if (std::optional AlignValFromAllocate =
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3587,6 +3587,9 @@
   "weakref declaration of %0 must be in a global context">;
 def err_attribute_weakref_without_alias : Error<
   "weakref declaration of %0 must also have an alias attribute">;
+def warn_attribute_unnamed_addr : Warning<
+  "unnamed_addr should only be used on const POD (plain old data) globals">,
+  InGroup>;
 def err_alias_not_supported_on_darwin : Error <
   "aliases are not supported on darwin">;
 def warn_attribute_wrong_decl_type_str : Warning<
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1408,6 +1408,24 @@
   }];
 }
 
+def UnnamedAddrDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``clang::unnamed_addr`` attribute specifies that the address of a global is
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+
+Example usage:
+
+.. code-block:: c
+
+  // i and j may have the same address.
+  [[clang::unnamed_addr]] const int i = 42;
+  [[clang::unnamed_addr]] const int j = 42;
+  }];
+}
+
 def ObjCRequiresSuperDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1794,6 +1794,13 @@
   let SimpleHandler = 1;
 }
 
+def UnnamedAddr : InheritableAttr {
+  let Spellings = [Clang<"unnamed_addr">];
+  let Subjects = SubjectList<[GlobalVar], ErrorDiag>;
+  let Documentation = [UnnamedAddrDocs];
+  let SimpleHandler = 1;
+}
+
 def ReturnsTwice : InheritableAttr {
   let Spellings = [GCC<"returns_twice">];
   let Subjects = SubjectList<[Function]>;

[PATCH] D158487: [PowerPC][altivec] Optimize codegen of vec_promote

2023-08-23 Thread Nemanja Ivanovic via Phabricator via cfe-commits

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

LGTM. This is a good idea and we should go ahead with this for anyone that uses 
`vec_promote`, but it might be a good idea to improve codegen for the insert 
which might be more common.




Comment at: llvm/test/CodeGen/PowerPC/vec-promote.ll:43
+
+define noundef <4 x float> @vec_promote_float_zeroed(ptr nocapture noundef 
readonly %p) {
+; CHECK-BE-LABEL: vec_promote_float_zeroed:

This code is absolutely terrible. Not only is the `lfs` super slow compared to 
`lfiwzx/lxsiwzx` that we actually want, but the two conversions and three 
permutes are super slow.

I think the change to `altivec.h` to produce better code for something like 
that is a good thing, but I wonder if something like this might come up in 
other contexts.

At least on Power9 and up, we can do much better than this. We don't do 
particularly well regardless of whether we're using a zero vector input or an 
arbitrary vector: https://godbolt.org/z/79fx8nsdP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158487

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

[PATCH] D158671: [NFC][Clang] Fix static analyzer concerns

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: aaron.ballman, tahonermann.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a reviewer: ributzka.
Herald added a project: All.
eandrews requested review of this revision.
Herald added a reviewer: dang.

Fix static analyzer concerns about null value reference


https://reviews.llvm.org/D158671

Files:
  clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5062,6 +5062,7 @@
 } IR{*this, PatternRec, NewRec};
 
 TypeSourceInfo *NewSI = IR.TransformType(Function->getTypeSourceInfo());
+assert(NewSI && "Type Transform failed?");
 Function->setType(NewSI->getType());
 Function->setTypeSourceInfo(NewSI);
 
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -609,7 +609,7 @@
   std::string Name;
   if (isa(Method)) {
 Name = Method->getNameAsString();
-if (dyn_cast(Method)->isExplicit())
+if (cast(Method)->isExplicit())
   Fragments.append("explicit", DeclarationFragments::FragmentKind::Keyword)
   .appendSpace();
   } else if (isa(Method))
Index: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
===
--- clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -192,7 +192,7 @@
 
   if (Decl->isStaticDataMember()) {
 SymbolReference Context;
-auto Record = dyn_cast(Decl->getDeclContext());
+auto Record = cast(Decl->getDeclContext());
 Context.Name = Record->getName();
 Context.USR = API.recordUSR(Record);
 auto Access = DeclarationFragmentsBuilder::getAccessControl(Decl);


Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5062,6 +5062,7 @@
 } IR{*this, PatternRec, NewRec};
 
 TypeSourceInfo *NewSI = IR.TransformType(Function->getTypeSourceInfo());
+assert(NewSI && "Type Transform failed?");
 Function->setType(NewSI->getType());
 Function->setTypeSourceInfo(NewSI);
 
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -609,7 +609,7 @@
   std::string Name;
   if (isa(Method)) {
 Name = Method->getNameAsString();
-if (dyn_cast(Method)->isExplicit())
+if (cast(Method)->isExplicit())
   Fragments.append("explicit", DeclarationFragments::FragmentKind::Keyword)
   .appendSpace();
   } else if (isa(Method))
Index: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
===
--- clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
+++ clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -192,7 +192,7 @@
 
   if (Decl->isStaticDataMember()) {
 SymbolReference Context;
-auto Record = dyn_cast(Decl->getDeclContext());
+auto Record = cast(Decl->getDeclContext());
 Context.Name = Record->getName();
 Context.USR = API.recordUSR(Record);
 auto Access = DeclarationFragmentsBuilder::getAccessControl(Decl);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ebe82d4 - [Driver] Remove unneeded TargetTriple entry in TripleAliases. NFC

2023-08-23 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2023-08-23T13:53:36-07:00
New Revision: ebe82d4aa6c5223425e27a0eb0157f49701d1057

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

LOG: [Driver] Remove unneeded TargetTriple entry in TripleAliases. NFC

This logic from 866faab4db1157e1edd2771024aee263367ec1fc (2012) is no
longer needed after D45233 added "Try to match the exact target triple first."

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index b7fd6208f54204..08bba7b4bd3625 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2683,10 +2683,6 @@ void 
Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
 break;
   }
 
-  // Always append the drivers target triple to the end, in case it doesn't
-  // match any of our aliases.
-  TripleAliases.push_back(TargetTriple.str());
-
   // Also include the multiarch variant if it's 
diff erent.
   if (TargetTriple.str() != BiarchTriple.str())
 BiarchTripleAliases.push_back(BiarchTriple.str());



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

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB added a comment.

Putting this out as a strawman/RFC. Should we add this function to allow LLVM 
passes to construct "likely"/"unlikely" branch_weights metadata? (I need this 
in D158642  and D157462 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

2023-08-23 Thread Matthias Braun via Phabricator via cfe-commits

MatzeB created this revision.
MatzeB added reviewers: lebedev.ri, spatel, davidxl, wenlei, hoy, paulkirth.
Herald added subscribers: StephenFan, modimo, hiraditya, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc.
Herald added projects: clang, LLVM.

- Add getLikelyBranchWeight() function returning the value of the 
`-likely-branch-weight` option defaulting to 2000.
- Remove `-unlikely-branch-weight` option and just use 1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158668

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
===
--- llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
+++ llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
@@ -2,7 +2,7 @@
 ; This can happen when the sum of all counters exceeds the max size of uint32_t
 
 ; RUN: llvm-profdata merge %S/Inputs/misexpect-branch-overflow.proftext -o %t.profdata
-; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -unlikely-branch-weight=2147483648 -likely-branch-weight=2147483648 -S 2>&1 | FileCheck %s --check-prefix=OVERFLOW
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=OVERFLOW
 
 ; OVERFLOW-NOT: warning: misexpect-branch.c:22:0: 50.00%
 ; OVERFLOW-NOT: remark: misexpect-branch.c:22:0: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 50.00% (2147483648 / 4294967296) of profiled executions.
@@ -32,9 +32,8 @@
   %lnot1 = xor i1 %lnot, true, !dbg !15
   %lnot.ext = zext i1 %lnot1 to i32, !dbg !15
   %conv = sext i32 %lnot.ext to i64, !dbg !15
-  %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1), !dbg !15
-  %tobool = icmp ne i64 %expval, 0, !dbg !15
-  br i1 %tobool, label %if.then, label %if.else, !dbg !15
+  %tobool = icmp ne i64 %conv, 0, !dbg !15
+  br i1 %tobool, label %if.then, label %if.else, !dbg !15, !prof !21
 
 if.then:  ; preds = %entry
   %1 = load i32, ptr %rando, align 4, !dbg !16, !tbaa !10
@@ -100,3 +99,4 @@
 !18 = !DILocation(line: 25, scope: !6)
 !19 = !DILocation(line: 27, scope: !6)
 !20 = !DILocation(line: 28, scope: !6)
+!21 = !{!"branch_weights", i32 2147483648, i32 2147483648}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -1,4 +1,4 @@
-; RUN: opt -S -passes='function(lower-expect),strip-dead-prototypes' -likely-branch-weight=2147483647 -unlikely-branch-weight=1 < %s | FileCheck %s
+; RUN: opt -S -passes='function(lower-expect),strip-dead-prototypes' -likely-branch-weight=2147483647 < %s | FileCheck %s
 
 ; The C case
 ; if (__builtin_expect_with_probability(((a0 == 1) || (a1 == 1) || (a2 == 1)), 1, 0))
Index: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===
--- llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -21,6 +21,7 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
@@ -36,31 +37,11 @@
 STATISTIC(ExpectIntrinsicsHandled,
   "Number of 'expect' intrinsic instructions handled");
 
-// These default values are chosen to represent an extremely skewed outcome for
-// a condition, but they leave some room for interpretation by later passes.
-//
-// If the documentation for __builtin_expect() was made explicit that it should
-// only be used in extreme cases, we could make this ratio higher. As it stands,
-// programmers may be using __builtin_expect() / llvm.expect to annotate that a
-// branch is likely or unlikely to be taken.
-
-// WARNING: these values are internal implementation detail of the pass.
-// They should not be exposed to the outside of the pass, front-end codegen
-// should emit @llvm.expect intrinsics instead of using these weights directly.
-// Transforms should use TargetTransformInfo's getPredictableBranchThreshold().
-static cl::opt LikelyBranchWeight(
-"likely-branch-weight", cl::Hidden, cl::init(2000),
-cl::desc("Weight of the

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

2023-08-23 Thread Nikolas Klauser via Phabricator via cfe-commits

philnik added a comment.

In D157572#4606513 , @aaron.ballman 
wrote:

> In D157572#4604595 , @philnik wrote:
>
>> In D157572#4604482 , 
>> @aaron.ballman wrote:
>>
 This allows standard libraries to mark symbols as extensions, so the 
 compiler can generate extension warnings when they are used.
>>>
>>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>>> is used to silence extension warnings)?
>>
>> I guess, kind-of. I never really understood the semantics of `__extension__` 
>> though, so I'm not 100% certain.
>
> It's used in system headers to say "I'm using an extension here, don't warn 
> about it in -pedantic mode".

Hm, OK. I thought I tried to use it that way at some point and it didn't work 
in the way I expected.

>>> I don't think we need to introduce a new attribute to do this, we already 
>>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that 
>>> suffice?
>>
>> Part of the idea here is that the diagnostics should be part of 
>> `-Wc++ab-extension`.
>
> Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty 
> because it's telling the user about all use of extensions, not extensions 
> being introduced by the library itself? (e.g., 
> https://godbolt.org/z/Gs3YGheMM is also not what you're after)

Yeah, for libc++ we don't support `-Wsystem-headers` in any meaningful way and 
this doesn't achieve the effect I want.

>> I guess we could allow warning flags instead of just `"warning"` and 
>> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
>> should be part of. Something like 
>> `__attribute__((__diagnose_if__(__cplusplus >= 201703L, "basic_string_view 
>> is a C++17 extension", "-Wc++17-extensions")))`. I'm not sure how one could 
>> implement that, but I guess there is some mechanism to translate 
>> "-Wwhatever" to a warning group, since you can push and pop warnings.  That 
>> would open people up to add a diagnostic to pretty much any warning group. I 
>> don't know if that's a good idea. I don't really see a problem with that 
>> other than people writing weird code, but people do that all the time 
>> anyways. Maybe I'm missing something really problematic though.
>
> That's actually a pretty interesting idea; `diagnose_if` could be given 
> another parameter to specify a diagnostic group to associate the diagnostic 
> with. This would let you do some really handy things like:
>
>   void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative 
> value to 'func' is deprecated", "warning", "-Wdeprecated")));
>
> But if we went this route, would we want to expose other diagnostic-related 
> knobs like "show in system header" and "default to an error"? Also, the 
> attribute currently can only be associated with a function; we can use this 
> for classes by sticking it on a constructor but there's not much help for 
> putting it on say a namespace or an enumeration. So we may need to extend the 
> attribute in other ways. CC @cjdb as this seems of interest to you as well.

I looked a bit around the code yesterday and it looks like it should be 
relatively easy to implement. I think we would just have to extend 
`CustomDiagInfo` to save some more information and pass the warning group in 
there from the `diagnose_if` sema handler. There are already utilities to 
translate `-Wwhatever` into the corresponding warning group IDs, so that part 
is trivial. Given that, I think this sounds like a very interesting way to go. 
This would also allow us to move the libc++ `atomic` checks to 
`-Watomic-memory-ordering` and I'm sure there will be more use-cases. Regarding 
the knobs, I think that would be interesting for some things too. We could add 
optional string arguments at the end like 
`"show_in_system_header={false,true}"`, `"default_to_error={false,true}"`, 
`"enable_by_default={false,true}"`, etc. If they aren't set we probably want to 
default to `show_in_system_header=false`, `default_to_error=false` and 
`enable_by_default=true`. Are there any other interesting knobs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews added a comment.

In D158666#4611497 , @erichkeane 
wrote:

> In D158666#4611494 , @eandrews 
> wrote:
>
>> In D158666#4611481 , @erichkeane 
>> wrote:
>>
>>> I think the .ifunc spelling was an oversight on my part when I implemented 
>>> this, I didn't spend enough time investigating GCC's behavior when 
>>> implementing this feature.  I think the alias is the right way about it, 
>>> but I think the .ifunc should be the alias (at least as far as I can think 
>>> it through right now). I think that works better because it supports a case 
>>> where the 'definition' of the target-clones function is generated with GCC, 
>>> but the 'caller' (also with target clones) comes from clang.  I THINK that 
>>> makes more sense? But perhaps try to chart out the behavior of the 
>>> GCC/Clang "Knows about TC"/"Doesn't know about TC" in each situation to see 
>>> which are troublesome?
>>>
>>> Additionally, this needs a release note.
>>
>> Thanks for taking a look! Can you explain why we need an alias? As in, if we 
>> just remove the .ifunc suffix in the 'ifunc' function here, it should work 
>> without an alias I think. I have to re-check but IIRC this is what GCC does
>
> At least short term, we have to have both .ifunc and  
> exposed, else we end up breaking SOMEONE.  At the moment, we're breaking the 
> example you have, however if we just switch the .ifunc to be , 
> we end up breaking cases where 1 TU is built with Clang 18 and the other with 
> Clang-trunk.  So we need to figure out what the 'compatibility story' is 
> between Clang 18, Clang-Trunk, and GCC with each of them in the position of:
>
> 1- Generated the function TU
> 2- Consumed the function, not knowing about it being TC
> 3- Consumed the function, knowing it was TC.

Makes sense. I'll experiment a bit and update review!


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

https://reviews.llvm.org/D158666

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

[PATCH] D157283: [clang] Match -isysroot behaviour with system compiler on Darwin

2023-08-23 Thread Jon Roelofs via Phabricator via cfe-commits

jroelofs added a comment.

In D157283#4611447 , @PiotrZSL wrote:

> I don't know how, but somehow this change breaks 
> clang-tidy/infrastructure/clang-tidy-mac-libcxx.cpp test.

I think we have a patch for that... @ldionne WDYT about upstreaming 
9edb9a711503d540cf3126c0fde11ce9a0d9a7aa 
 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157283

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D158666#4611494 , @eandrews wrote:

> In D158666#4611481 , @erichkeane 
> wrote:
>
>> I think the .ifunc spelling was an oversight on my part when I implemented 
>> this, I didn't spend enough time investigating GCC's behavior when 
>> implementing this feature.  I think the alias is the right way about it, but 
>> I think the .ifunc should be the alias (at least as far as I can think it 
>> through right now). I think that works better because it supports a case 
>> where the 'definition' of the target-clones function is generated with GCC, 
>> but the 'caller' (also with target clones) comes from clang.  I THINK that 
>> makes more sense? But perhaps try to chart out the behavior of the GCC/Clang 
>> "Knows about TC"/"Doesn't know about TC" in each situation to see which are 
>> troublesome?
>>
>> Additionally, this needs a release note.
>
> Thanks for taking a look! Can you explain why we need an alias? As in, if we 
> just remove the .ifunc suffix in the 'ifunc' function here, it should work 
> without an alias I think. I have to re-check but IIRC this is what GCC does

At least short term, we have to have both .ifunc and  
exposed, else we end up breaking SOMEONE.  At the moment, we're breaking the 
example you have, however if we just switch the .ifunc to be , 
we end up breaking cases where 1 TU is built with Clang 18 and the other with 
Clang-trunk.  So we need to figure out what the 'compatibility story' is 
between Clang 18, Clang-Trunk, and GCC with each of them in the position of:

1- Generated the function TU
2- Consumed the function, not knowing about it being TC
3- Consumed the function, knowing it was TC.

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

https://reviews.llvm.org/D158666

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews added a comment.

In D158666#4611481 , @erichkeane 
wrote:

> I think the .ifunc spelling was an oversight on my part when I implemented 
> this, I didn't spend enough time investigating GCC's behavior when 
> implementing this feature.  I think the alias is the right way about it, but 
> I think the .ifunc should be the alias (at least as far as I can think it 
> through right now). I think that works better because it supports a case 
> where the 'definition' of the target-clones function is generated with GCC, 
> but the 'caller' (also with target clones) comes from clang.  I THINK that 
> makes more sense? But perhaps try to chart out the behavior of the GCC/Clang 
> "Knows about TC"/"Doesn't know about TC" in each situation to see which are 
> troublesome?
>
> Additionally, this needs a release note.

Thanks for taking a look! Can you explain why we need an alias? As in, if we 
just remove the .ifunc suffix in the 'ifunc' function here, it should work 
without an alias I think. I have to re-check but IIRC this is what GCC does


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

https://reviews.llvm.org/D158666

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

2023-08-23 Thread Ian Anderson via Phabricator via cfe-commits

iana marked an inline comment as done.
iana added a comment.

In D157757#4611425 , @ldionne wrote:

> Also we should figure out why the Clang modules build is failing on top of 
> this, it doesn't look like a false positive.

Where do you see that failure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits

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

Accepted before I thought about the other order, so requesting changes so we 
can think this through... 1 thing we DO need to consider is how this works with 
OLDER versions of Clang/clang's behavior.


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

https://reviews.llvm.org/D158666

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Erich Keane via Phabricator via cfe-commits

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

I think the .ifunc spelling was an oversight on my part when I implemented 
this, I didn't spend enough time investigating GCC's behavior when implementing 
this feature.  I think the alias is the right way about it, but I think the 
.ifunc should be the alias (at least as far as I can think it through right 
now). I think that works better because it supports a case where the 
'definition' of the target-clones function is generated with GCC, but the 
'caller' (also with target clones) comes from clang.  I THINK that makes more 
sense? But perhaps try to chart out the behavior of the GCC/Clang "Knows about 
TC"/"Doesn't know about TC" in each situation to see which are troublesome?

Additionally, this needs a release note.


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

https://reviews.llvm.org/D158666

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

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

2023-08-23 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: erichkeane, tahonermann, aaron.ballman, zsrkmyn.
Herald added a project: All.
eandrews requested review of this revision.

Currently target_clones attribute results in a linker error when there are no 
multi-versioned function declarations in the calling TU.

  foo.cpp
  int foo1();
  __attribute__((target_clones("default", "arch=core2")))
  int foo1() { return 0; }
  
  main.cpp
  int foo1();
  int main()
  {   return foo1(); }
  
  $ clang++ main.cpp foo.cpp
  /usr/bin/ld: /tmp/main-981c32.o: in function `main':
  main.cpp:(.text+0x10): undefined reference to `foo1()'
  clang++: error: linker command failed with exit code 1 (use -v to see 
invocation)

In the calling TU, the call is generated with ‘normal’ assembly name.  This 
does not match any of the versions since their mangling includes a 
.versionstring. The linker error is not seen with GCC since the mangling for 
the ifunc symbol in GCC is the ‘normal’ assembly name for function.

  Clang – 
  
  $nm foo.o
  U   __cpu_indicator_init
  U   __cpu_model
  T_Z4foo1v.arch_core2.0
  T_Z4foo1v.default.1
  i _Z4foo1v.ifunc   <--
  W  _Z4foo1v.resolver
  
  $ nm main.o
  T main
   U _Z4foo1v
  
  GCC
  
  $ nm foo.o
  U   __cpu_indicator_init
  U  __cpu_model
  i_Z4foo1v <---
  t_Z4foo1v.arch_core2.0
  t_Z4foo1v.default.1
  W  _Z4foo1v.resolver
  
  $ nm main.o
  T main
  U _Z4foo1v

I was initially inclined to match GCC behavior here and remove the ifunc suffix 
but I decided against it because I am not sure why ifunc mangling was used in 
the first place. Maybe to maintain consistency between various multiversion 
attributes? I see target attribute also uses ifunc mangling (while GCC has some 
other mangling scheme for this attribute). As a less disruptive solution I just 
added an alias to the ifunc function, similar to what was done for CPU dispatch 
here - https://reviews.llvm.org/D67058. If the correct solution here is to 
remove the ifunc suffix, I can make that change instead.


https://reviews.llvm.org/D158666

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-target-clones.c
  clang/test/CodeGenCXX/attr-target-clones.cpp


Index: clang/test/CodeGenCXX/attr-target-clones.cpp
===
--- clang/test/CodeGenCXX/attr-target-clones.cpp
+++ clang/test/CodeGenCXX/attr-target-clones.cpp
@@ -1,6 +1,13 @@
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -emit-llvm %s -o - | 
FileCheck %s --check-prefix=LINUX
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows-pc -emit-llvm %s -o - | 
FileCheck %s --check-prefix=WINDOWS
 
+// Alias for ifunc
+// LINUX: @_Z10overloadedi = weak_odr alias i32 (i32), ptr 
@_Z10overloadedi.ifunc
+// LINUX: @_Z10overloadedPKc = weak_odr alias i32 (ptr), ptr 
@_Z10overloadedPKc.ifunc
+// LINUX: @_ZN1CIssE3fooEv = weak_odr alias i32 (ptr), ptr 
@_ZN1CIssE3fooEv.ifunc
+// LINUX: @_ZN1CIisE3fooEv = weak_odr alias i32 (ptr), ptr 
@_ZN1CIisE3fooEv.ifunc
+// LINUX: @_ZN1CIdfE3fooEv = weak_odr alias i32 (ptr), ptr 
@_ZN1CIdfE3fooEv.ifunc
+
 // Overloaded ifuncs
 // LINUX: @_Z10overloadedi.ifunc = weak_odr ifunc i32 (i32), ptr 
@_Z10overloadedi.resolver
 // LINUX: @_Z10overloadedPKc.ifunc = weak_odr ifunc i32 (ptr), ptr 
@_Z10overloadedPKc.resolver
Index: clang/test/CodeGen/attr-target-clones.c
===
--- clang/test/CodeGen/attr-target-clones.c
+++ clang/test/CodeGen/attr-target-clones.c
@@ -13,6 +13,13 @@
 // WINDOWS: $foo_inline = comdat any
 // WINDOWS: $foo_inline2 = comdat any
 
+// LINUX: @foo = weak_odr alias i32 (), ptr @foo.ifunc
+// LINUX: @foo_dupes = weak_odr alias void (), ptr @foo_dupes.ifunc
+// LINUX: @unused = weak_odr alias void (), ptr @unused.ifunc
+// LINUX: @foo_inline = weak_odr alias i32 (), ptr @foo_inline.ifunc
+// LINUX: @foo_inline2 = weak_odr alias i32 (), ptr @foo_inline2.ifunc
+// LINUX: @foo_used_no_defn = weak_odr alias i32 (), ptr 
@foo_used_no_defn.ifunc
+
 // LINUX: @foo.ifunc = weak_odr ifunc i32 (), ptr @foo.resolver
 // LINUX: @foo_dupes.ifunc = weak_odr ifunc void (), ptr @foo_dupes.resolver
 // LINUX: @unused.ifunc = weak_odr ifunc void (), ptr @unused.resolver
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4053,8 +4053,20 @@
 }
 
 llvm::Constant *ResolverConstant = GetOrCreateMultiVersionResolver(GD);
-if (auto *IFunc = dyn_cast(ResolverConstant))
+if (auto *IFunc = dyn_cast(ResolverConstant)) {
   ResolverConstant = IFunc->getResolver();
+  if (FD->isTargetClonesMultiVersion()) {
+const CGFunctionInfo  = getTypes().arrangeGlobalDeclaration(GD);
+llvm::FunctionType *DeclTy = getTypes().GetFunctionType(FI);
+std::string MangledName = getMangledNameImpl(
+

1 2 3 >

1 - 100 of 289 matches

Mail list logo