[PATCH] D35383: [coroutines] Add serialization/deserialization of coroutines

[Yifeng Dong via Phabricator via cfe-commits]

dongAxis1944 added inline comments.
Herald added a subscriber: lxfind.



Comment at: cfe/trunk/lib/AST/StmtCXX.cpp:107-108
+  Result->NumParams = NumParams;
+  auto *ParamBegin = Result->getStoredStmts() + SubStmt::FirstParamMove;
+  std::uninitialized_fill(ParamBegin, ParamBegin + NumParams,
+  static_cast(nullptr));

hello, i have a question about that why clang commit info of parameters again 
in coroutine body?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D35383

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

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

HazardyKnusperkeks added a comment.

In D93844#2472352 , @njames93 wrote:

> In D93844#2472346 , @MyDeveloperDay 
> wrote:
>
>> I'm a little confused why this is needed as clang-format always read up the 
>> directory tree until it see a .clang-format file, perhaps I don't quite 
>> understand the use case. Can't you just have a different .clang-format in 
>> the subdirectory?
>
> It's my understanding this is akin to the `InheritParentConfig` option found 
> in `.clang-tidy`.
> The idea is you can put a `.clang-format` file in your project root, then for 
> specific directories you can add another `.clang-format` file that modifies 
> that root `.clang-format`
> The example here is a test folder that needs extended lines but want the rest 
> of the formatting to match the root folder without having to copy the 
> contents of the root and make the small edit.
>
> We have it quite lucky here at LLVM where for tests we can just use:
>
>   BasedOnStyle: LLVM
>   ColumnLimit: 0
>
> However for other projects with their own style they could then use:
>
>   # Copy the root directory clang.format config
>   BasedOnStyle: file
>   ColumnLimit: 0
>
> This would also make modifying the formatting style of a project much easier 
> as you would only need to touch the root directory `.clang-format` file in 
> most cases.

Exactly.

> With this implementation I'm not too sure on the specifics though. 
> `.clang-format` files are more complex to parse than `.clang-tidy` files so 
> there's more edge cases to consider.
> Also it appears this wouldn't handle the case of invoking clang-format with 
> the style set inline on the command line.
> `clang-format  --style='{BasedOnStyle: File, ColumnLimit: 80}'`
> Should this then check the directory for `.clang-format` files?

Initially I thought it shouldn't. But now I think maybe it should.




Comment at: clang/lib/Format/Format.cpp:2962
+llvm::sys::path::filename(FileName));
+if (auto OuterStyle = getStyle(DefaultFormatStyle, FileForLanguage,
+   FallbackStyleName, Code, FS,

njames93 wrote:
> This seems a very inefficient way of doing this, getStyle does a lot of 
> things we simple don't need to worry about. Maybe the file loading part 
> should be extracted out into its own function, that can then recursively call 
> itself. Or use the clang-tidy approach of storing configurations in a stack 
> (will have to be unparsed cause how clang-format works) and then parse them 
> on top of each other.
Yeah I know that this is double work, but I thought it only affects the case 
`BasedOnStyle: File` and keeps the "normal" way untouched.

If I do build a stack I would actually parse all `.clang-format` files up to 
the root directory and in the not `BasedOnStyle: File` case throw away the 
results, wouldn't I? The question is how often someone has a `.clang-fomrat` in 
one of his parent directories which is not used because it got overwritten.

But I can do that.

Or is there an easy way to just check the `BasedOnStyle` value? As far as I 
understood the yaml code it parses the whole file into an internal structure 
and we only query this structure, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93844

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

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

In D93179#2473342 , @pengfei wrote:

> Hi Simon, I found we have the same problem for fadd/fmul. See 
> https://godbolt.org/z/3YKaGx
> X86 Intrinsics imply `reassoc` flag, but `llvm.vector.reduce.*` doesn't.

I'm not surprised - my current plan (after the holidays) is to add doxygen 
descriptions for all the reduction intrinsics and then update them making it 
clear what fast-math flags are assumed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat use poison as inselt's placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune updated this revision to Diff 313945.
aqjune added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely.

Update IRBuilder to fill poison at shufflevector's empty operand


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

Files:
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2.c
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/arm-mve-intrinsics/compare.c
  clang/test/CodeGen/arm-mve-intrinsics/cplusplus.cpp
  clang/test/CodeGen/arm-mve-intrinsics/dup.c
  clang/test/CodeGen/arm-mve-intrinsics/ternary.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vmulq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmullbq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulltq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vsubq.c
  clang/test/CodeGen/builtins-ppc-p10vector.c
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGen/vecshift.c
  clang/test/CodeGenCXX/matrix-type-operators.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/CodeGenCXX/vector-splat-conversion.cpp
  clang/test/CodeGenOpenCL/bool_cast.cl
  clang/test/CodeGenOpenCL/shifts.cl
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/IRBuilder.cpp
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts.ll
  llvm/test/Transforms/InstCombine/gep-inbounds-null.ll
  llvm/test/Transforms/InstCombine/getelementptr.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow-inseltpoison.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow.ll
  llvm/test/Transforms/InstCombine/type_pun-inseltpoison.ll
  llvm/test/Transforms/InstCombine/type_pun.ll
  llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_shuffle.ll
  llvm/test/Transforms/InstCombine/vscale_cmp.ll
  llvm/test/Transforms/LoopVectorize/AArch64/arbitrary-induction-step.ll
  
llvm/test/Transforms/LoopVectorize/AArch64/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-gather-scatter-tailpred.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-reduction-types.ll
  llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
  llvm/test/Transforms/LoopVectorize/ARM/tail-folding-not-allowed.ll
  llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/X86/constant-fold.ll
  llvm/test/Transforms/LoopVectorize/X86/cost-model-assert.ll
  llvm/test/Transforms/LoopVectorize/X86/interleaving.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-load-gather.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
  llvm/test/Transforms/LoopVectorize/X86/masked_load_store.ll
  llvm/test/Transforms/LoopVectorize/X86/metadata-enable.ll
  llvm/test/Transforms/LoopVectorize/X86/optsize.ll
  
llvm/test/Transforms/LoopVectorize/X86/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/X86/pr34438.ll
  llvm/test/Transforms/LoopVectorize/X86/small-size.ll
  llvm/test/Transforms/LoopVectorize/X86/tail_loop_folding.ll
  llvm/test/Transforms/LoopVectorize/X86/uniform_mem_op.ll
  llvm/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll
  
llvm/test/Transforms/LoopVectorize/X86/x86-interleaved-accesses-masked-group.ll
  llvm/test/Transforms/LoopVectorize/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-const-TC.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-divisible-TC.ll
  llvm/test/Transforms/LoopVectorize/first-order-recurrence-complex.ll
  llvm/test/Transforms/LoopVectorize/float-induction.ll
  llvm/test/Transforms/LoopVectorize/float-minmax-instruction-flag.ll
  llvm/test/Transforms/LoopVectorize/if-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/induction-step.ll
  llvm/test/Transforms/LoopVectorize/induction.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-1.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses.ll
  llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/loop-form.ll
  llvm/test/Transforms/LoopVectorize/minmax_reduction.ll
  llvm/test/Transforms/LoopVectorize/multiple-strides

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c:84
   vd = vec_splat(vd, 1);
   // CHECK: shufflevector <2 x double> %{{.*}}, <2 x double> undef, <2 x i32> 

   // CHECK-ASM: vrepg

These SystemZ zvector tests are still creating shufflevector with undef, and I 
couldn't find where they stem from. :/ 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[clang] 5abfecc - [ARM][AArch64] Add Cortex-A78C Support for Clang and LLVM

[Mark Murray via cfe-commits]

Author: Mark Murray
Date: 2020-12-29T10:18:59Z
New Revision: 5abfeccf10bcbc0d673ece21ddd8d4ac4a0e7594

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

LOG: [ARM][AArch64] Add Cortex-A78C Support for Clang and LLVM

This patch upstreams support for the Armv8-a Cortex-A78C
processor for AArch64 and ARM.

In detail:

Adding cortex-a78c as cpu option for aarch64 and arm targets in clang
Adding Cortex-A78C CPU name and ProcessorModel in llvm
Details of the CPU can be found here:
https://www.arm.com/products/silicon-ip-cpu/cortex-a/cortex-a78c

Added: 


Modified: 
clang/test/Driver/aarch64-cpus.c
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/ARMTargetParser.def
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/lib/Target/ARM/ARM.td
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/ARMSubtarget.h
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 283660b321b3..7ac2473915e8 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -183,6 +183,8 @@
 // CORTEXX1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-x1"
 // RUN: %clang -target aarch64 -mcpu=cortex-a78  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEXA78 %s
 // CORTEXA78: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a78"
+// RUN: %clang -target aarch64 -mcpu=cortex-a78c  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEX-A78C %s
+// CORTEX-A78C: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"cortex-a78c"
 // RUN: %clang -target aarch64 -mcpu=neoverse-v1  -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-V1 %s
 // NEOVERSE-V1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-v1"
 
@@ -475,6 +477,12 @@
 // MCPU-MTUNE-THUNDERX2T99: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-cpu" "thunderx2t99"
 // MCPU-MTUNE-THUNDERX3T110: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-cpu" "thunderx3t110"
 
+// RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-a78c -### -c %s 2>&1 
| FileCheck -check-prefix=CHECK-CORTEX-A78C %s
+// RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-a78c 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-A78C-MFPU %s
+// CHECK-CORTEX-A78C: "-cc1"{{.*}} "-triple" "armv8.2a-{{.*}} "-target-cpu" 
"cortex-a78c"
+// CHECK-CORTEX-A78C-MFPU: "-cc1"{{.*}} "-target-feature" "+fp-armv8"
+// CHECK-CORTEX-A78C-MFPU: "-target-feature" "+crypto"
+
 // RUN: %clang -target aarch64 -march=armv8.1a -### -c %s 2>&1 | FileCheck 
-check-prefix=GENERICV81A %s
 // RUN: %clang -target aarch64 -march=armv8.1-a -### -c %s 2>&1 | FileCheck 
-check-prefix=GENERICV81A %s
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv8.1a -### -c %s 2>&1 
| FileCheck -check-prefix=GENERICV81A %s

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.def 
b/llvm/include/llvm/Support/AArch64TargetParser.def
index 97172730e364..f1a5bf734a13 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.def
+++ b/llvm/include/llvm/Support/AArch64TargetParser.def
@@ -147,6 +147,9 @@ AARCH64_CPU_NAME("cortex-a77", ARMV8_2A, 
FK_CRYPTO_NEON_FP_ARMV8, false,
 AARCH64_CPU_NAME("cortex-a78", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC 
|
   AArch64::AEK_SSBS))
+AARCH64_CPU_NAME("cortex-a78c", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
+ (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC 
|
+  AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-r82", ARMV8R, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_LSE))
 AARCH64_CPU_NAME("cortex-x1", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,

diff  --git a/llvm/include/llvm/Support/ARMTargetParser.def 
b/llvm/include/llvm/Support/ARMTargetParser.def
index 76341a051dbf..37cf0a93bb04 100644
--- a/llvm/include/llvm/Support/ARMTargetParser.def
+++ b/llvm/include/llvm/Support/ARMTargetParser.def
@@ -300,8 +300,10 @@ ARM_CPU_NAME("cortex-a76ae", ARMV8_2A, 
FK_CRYPTO_NEON_FP_ARMV8, false,
 (ARM::AEK_FP16 | ARM::AEK_DOTPROD))
 ARM_CPU_NAME("cortex-a77", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
 (ARM::AEK_FP16 | ARM::AEK_DOTPROD))
-ARM_CPU_NAME("cortex-a78",ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
+ARM_CPU_NAME("cortex-a78", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (ARM::AEK_FP16 | ARM::AEK_DOTPROD))
+ARM_CPU_NAME("cortex-a78c", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
+ ARM::AEK_FP16 | ARM::AEK_DOTPROD)
 ARM_CPU_NAME("cortex-x1", ARMV8_2A, FK_CRYPTO_NE

[PATCH] D93883: [WebAssembly] Prototype prefetch instructions

[Heejin Ahn via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:318
+  Intrinsic<[], [llvm_ptr_ty],
+[IntrInaccessibleMemOrArgMemOnly, IntrWillReturn,
+ ReadOnly>, NoCapture>],

It [[ 
https://github.com/llvm/llvm-project/blob/5abfeccf10bcbc0d673ece21ddd8d4ac4a0e7594/llvm/include/llvm/IR/Intrinsics.td#L130
 | looks ]] `IntrWillReturn` is applied by default?



Comment at: llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll:14
+;===
+; v128.load32_zero
+;===





Comment at: llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll:126
+;===
+; v128.load64_zero
+;===




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93883

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

[PATCH] D93883: [WebAssembly] Prototype prefetch instructions

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll:126
+;===
+; v128.load64_zero
+;===

aheejin wrote:
> 
Sorry the previous suggestion had an extra `;` and there seems no way to delete 
it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93883

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I like what you are suggesting, do you think `BasedOnStyle: File` is the best 
terminology as the term `File` is used elsewhere to mean read from the 
.clang_format file, how about

`BasedOnStyle: Inherit`
`BasedOnStyle: InheritFromParent`
`BasedOnStyle: InheritFromFile`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93844

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:8544-8556
+  EXPECT_EQ("struct foo {\n"
+"  int i;\n"
+"\n"
+"private:\n"
+"  int j;\n"
+"}\n",
+format("struct foo {\n"

thezbyg wrote:
> MyDeveloperDay wrote:
> > if you use verifyFormat it will check what happens when it messes the code 
> > up to ensure its stable
> After switching to verifyFormat all tests pass when checking C++ formatting, 
> but some of the same tests fail in Objective-C++ check:
> C style comment is attached to previous line:
> ```
>   Expected: Expected.str()
>   Which is: "struct foo {\n  /* comment */\nprivate:\n  int i;\n  int 
> j;\n}\n"
> To be equal to: format(test::messUp(Code), ObjCStyle)
>   Which is: "struct foo { /* comment */\nprivate:\n  int i;\n  int 
> j;\n}\n"
> With diff:
> @@ -1,4 +1,3 @@
> -struct foo {
> -  /* comment */
> +struct foo { /* comment */
>  private:
>int i;
> 
> ```
> Empty line before access modifier is removed:
> ```
>   Expected: Expected.str()
>   Which is: "struct foo {\n  int i;\n\nprivate:\n  int j;\n}\n"
> To be equal to: format(test::messUp(Code), ObjCStyle)
>   Which is: "struct foo {\n  int i;\nprivate:\n  int j;\n}\n"
> With diff:
> @@ -1,5 @@
>  struct foo {
>int i;
> -
>  private:
>int j;
> ```
> Looks like empty lines before modifiers are removed for Objective-C++ 
> language. What should I do?
please update the patch at least to user verifyFomat where you can.

Its likely means we are missing something


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

https://reviews.llvm.org/D93846

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

[PATCH] D93179: [X86] Convert fmin/fmax _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D93179#2473620 , @RKSimon wrote:

> In D93179#2473342 , @pengfei wrote:
>
>> Hi Simon, I found we have the same problem for fadd/fmul. See 
>> https://godbolt.org/z/3YKaGx
>> X86 Intrinsics imply `reassoc` flag, but `llvm.vector.reduce.*` doesn't.
>
> I'm not surprised - my current plan (after the holidays) is to add doxygen 
> descriptions for all the reduction intrinsics and then update them making it 
> clear what fast-math flags are assumed.

That's great. We are also updating intrinsic guide for such information. 
Anyway, have a good holiday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93179

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

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Nikita Popov via Phabricator via cfe-commits]

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

There are some polly tests that also need to be updated, but otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Sanjay Patel via Phabricator via cfe-commits]

spatel added inline comments.



Comment at: llvm/include/llvm/IR/IRBuilder.h:2527
   /// Create a unary shuffle. The second vector operand of the IR instruction
   /// is undefined.
   Value *CreateShuffleVector(Value *V, ArrayRef Mask,

update code comment: undefined -> poison



Comment at: llvm/lib/IR/IRBuilder.cpp:1020
   Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC));
-  return CreateShuffleVector(V, Undef, Zeros, Name + ".splat");
+  return CreateShuffleVector(V, Poison, Zeros, Name + ".splat");
 }

I'm still catching up on reviews/mails, so ignore if this was already discussed:
Can we change this usage of CreateShuffleVector to the unary operand version, 
so we're reducing the number of explicit places where we need to specify poison?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[PATCH] D93901: [NFC] Renaming PackStack to AlignPackStack

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast, efriedma, 
jyknight, aaron.ballman.
Xiangling_L requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch renames `PackStack` and related variable name to also contain 
`align` across Clang. As it is right now, clang already uses one stack to 
record those information from both `#pragma align` and `#pragma pack`. Leave it 
as PackStack is confusing, and people could actually ignore the `#pragma align` 
when developing with `PackStack`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93901

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  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
@@ -4151,24 +4151,24 @@
   Stream.EmitRecord(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS, Record);
 }
 
-/// Write the state of 'pragma pack' at the end of the module.
+/// Write the state of 'pragma align/pack' at the end of the module.
 void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) {
-  // Don't serialize pragma pack state for modules, since it should only take
-  // effect on a per-submodule basis.
+  // Don't serialize pragma align/pack state for modules, since it should only
+  // take effect on a per-submodule basis.
   if (WritingModule)
 return;
 
   RecordData Record;
-  Record.push_back(SemaRef.PackStack.CurrentValue);
-  AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record);
-  Record.push_back(SemaRef.PackStack.Stack.size());
-  for (const auto &StackEntry : SemaRef.PackStack.Stack) {
+  Record.push_back(SemaRef.AlignPackStack.CurrentValue);
+  AddSourceLocation(SemaRef.AlignPackStack.CurrentPragmaLocation, Record);
+  Record.push_back(SemaRef.AlignPackStack.Stack.size());
+  for (const auto &StackEntry : SemaRef.AlignPackStack.Stack) {
 Record.push_back(StackEntry.Value);
 AddSourceLocation(StackEntry.PragmaLocation, Record);
 AddSourceLocation(StackEntry.PragmaPushLocation, Record);
 AddString(StackEntry.StackSlotLabel, Record);
   }
-  Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record);
+  Stream.EmitRecord(ALIGN_PACK_PRAGMA_OPTIONS, Record);
 }
 
 /// Write the state of 'pragma float_control' at the end of the module.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3742,25 +3742,25 @@
   ForceCUDAHostDeviceDepth = Record[0];
   break;
 
-case PACK_PRAGMA_OPTIONS: {
+case ALIGN_PACK_PRAGMA_OPTIONS: {
   if (Record.size() < 3) {
 Error("invalid pragma pack record");
 return Failure;
   }
-  PragmaPackCurrentValue = Record[0];
-  PragmaPackCurrentLocation = ReadSourceLocation(F, Record[1]);
+  PragmaAlignPackCurrentValue = Record[0];
+  PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]);
   unsigned NumStackEntries = Record[2];
   unsigned Idx = 3;
   // Reset the stack when importing a new module.
-  PragmaPackStack.clear();
+  PragmaAlignPackStack.clear();
   for (unsigned I = 0; I < NumStackEntries; ++I) {
-PragmaPackStackEntry Entry;
+PragmaAlignPackStackEntry Entry;
 Entry.Value = Record[Idx++];
 Entry.Location = ReadSourceLocation(F, Record[Idx++]);
 Entry.PushLocation = ReadSourceLocation(F, Record[Idx++]);
-PragmaPackStrings.push_back(ReadString(Record, Idx));
-Entry.SlotLabel = PragmaPackStrings.back();
-PragmaPackStack.push_back(Entry);
+PragmaAlignPackStrings.push_back(ReadString(Record, Idx));
+Entry.SlotLabel = PragmaAlignPackStrings.back();
+PragmaAlignPackStack.push_back(Entry);
   }
   break;
 }
@@ -7875,32 +7875,36 @@
   }
   SemaObj->ForceCUDAHostDeviceDepth = ForceCUDAHostDeviceDepth;
 
-  if (PragmaPackCurrentValue) {
+  if (PragmaAlignPackCurrentValue) {
 // The bottom of the stack might have a default value. It must be adjusted
 // to the current value to ensure that the packing state is preserved after
 // popping entries that were included/imported from a PCH/module.
 bool DropFirst = false;
-if (!PragmaPackStack.empty() &&
-PragmaPackStack.front().Location.isInvalid()) {
-  assert(PragmaPackStack.front().Value == SemaObj->PackStack.DefaultValue &&
+if (!PragmaAlignPackStack.empty() &&
+PragmaAlignPackStack.front().Location.isInvalid()) {
+  assert(PragmaAlignPackStack.front().Value ==
+ SemaObj->AlignPackStack.Defa

[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect

[Xiangling Liao via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Sema/Sema.h:637
   PragmaStack VtorDispStack;
-  // #pragma pack.
-  // Sentinel to represent when the stack is set to mac68k alignment.
-  static const unsigned kMac68kAlignmentSentinel = ~0U;
-  PragmaStack PackStack;
+  PragmaStack PackStack;
   // The current #pragma pack values and locations at each #include.

jasonliu wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > We should consider renaming PackStack to AlignPackStack across Clang. 
> > > Maybe even as a NFC first. As it is right now, clang already uses one 
> > > stack to record those two informations from `Pragma align` and `Pragma 
> > > pack`. Leave it as PackStack is too confusing, and people could actually 
> > > ignore the pragma Align when developing with PackStack. 
> > That's a good point. I will create a NFC accordingly.
> Please post this NFC when you have time.
Thanks for reminding me of this. A NFC patch is posted here: 
https://reviews.llvm.org/D93901


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

https://reviews.llvm.org/D87702

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

[Albertas Vyšniauskas via Phabricator via cfe-commits]

thezbyg updated this revision to Diff 313973.
thezbyg added a comment.

Switched to verifyFormat in most of the tests.
Rerun clang-format.


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

https://reviews.llvm.org/D93846

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/test/Format/access-modifiers.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8540,6 +8540,186 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, FormatsAccessModifiers) {
+  EXPECT_EQ("struct foo {\n"
+"  int i;\n"
+"\n"
+"private:\n"
+"  int j;\n"
+"}\n",
+format("struct foo {\n"
+   "  int i;\n"
+   "\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   getLLVMStyle()));
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "}\n",
+   "struct foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "}\n");
+  EXPECT_EQ("struct foo {\n"
+"  int i;\n"
+"\n"
+"private:\n"
+"  int j;\n"
+"}\n",
+format("struct foo {\n"
+   "  int i;\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   getLLVMStyle()));
+  verifyFormat("struct foo {\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "  // comment\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n");
+  EXPECT_EQ("struct foo {\n"
+"  /* comment */\n"
+"private:\n"
+"  int j;\n"
+"}\n",
+format("struct foo {\n"
+   "  /* comment */\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   getLLVMStyle()));
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "#ifdef FOO\n"
+   "#endif\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n");
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n");
+  FormatStyle Style = getLLVMStyle();
+  Style.EmptyLineBeforeAccessModifier = false;
+  verifyFormat("struct foo {\n"
+   "  int i;\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "  int i;\n"
+   "\n"
+   "private:\n"
+   "  int j;\n"
+   "}\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   Style);
+  verifyFormat("struct foo {\n"
+   "  // comment\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   "struct foo {\n"
+   "  // comment\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",
+   Style);
+  EXPECT_EQ("struct foo {\n"
+"  /* comment */\n"
+"private:\n"
+"  int i;\n"
+"  int j;\n"
+"}\n",
+format("struct foo {\n"
+   "  /* comment */\n"
+   "\n"
+   "private:\n"
+   "  int i;\n"
+   "  int j;\n"

[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 313986.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Rebase on renaming NFC patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87702

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Driver/aix-pragma-pack.c
  clang/test/Layout/aix-double-struct-member.cpp
  clang/test/Layout/aix-power-natural-interaction.cpp
  clang/test/PCH/aix-pragma-pack.c
  clang/test/Sema/aix-pragma-pack-and-align.c

Index: clang/test/Sema/aix-pragma-pack-and-align.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-pack-and-align.c
@@ -0,0 +1,231 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -faix-pragma-pack -verify -fsyntax-only -x c++ %s | \
+// RUN:   FileCheck %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -faix-pragma-pack -verify -fsyntax-only -x c++ %s | \
+// RUN:   FileCheck %s
+
+namespace test1 {
+#pragma align(natural)
+#pragma pack(4)
+#pragma pack(2)
+struct A {
+  int i;
+  double d;
+};
+
+int a = sizeof(A);
+#pragma pack()
+#pragma pack(show) // expected-warning {{value of #pragma pack(show) == 4}}
+#pragma pack(pop)
+#pragma pack(show) // expected-warning {{value of #pragma pack(show) == 8}}
+struct B {
+  int i;
+  double d;
+};
+#pragma align(reset)
+
+int b = sizeof(B);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test1::A
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  4 |   double d
+// CHECK-NEXT:| [sizeof=12, dsize=12, align=2, preferredalign=2,
+// CHECK-NEXT:|  nvsize=12, nvalign=2, preferrednvalign=2]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test1::B
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  8 |   double d
+// CHECK-NEXT:| [sizeof=16, dsize=16, align=4, preferredalign=8,
+// CHECK-NEXT:|  nvsize=16, nvalign=4, preferrednvalign=8]
+
+} // namespace test1
+
+namespace test2 {
+#pragma align(natural)
+#pragma pack(2)
+struct A {
+  int i;
+  double d;
+};
+
+int a = sizeof(A);
+#pragma align(reset)
+
+struct B {
+  int i;
+  double d;
+};
+
+int b = sizeof(B);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test2::A
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  4 |   double d
+// CHECK-NEXT:| [sizeof=12, dsize=12, align=2, preferredalign=2,
+// CHECK-NEXT:|  nvsize=12, nvalign=2, preferrednvalign=2]
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test2::B
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  4 |   double d
+// CHECK-NEXT:| [sizeof=12, dsize=12, align=4, preferredalign=4,
+// CHECK-NEXT:|  nvsize=12, nvalign=4, preferrednvalign=4]
+
+} // namespace test2
+
+namespace test3 {
+#pragma pack(2)
+#pragma align(natural)
+struct A {
+  double d;
+};
+#pragma align(reset)
+#pragma pack(pop)
+
+int a = sizeof(A);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test3::A
+// CHECK-NEXT:  0 |   double d
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=4, preferredalign=8,
+// CHECK-NEXT:|  nvsize=8, nvalign=4, preferrednvalign=8]
+
+} // namespace test3
+
+namespace test4 {
+#pragma pack(2)
+#pragma align(natural)
+#pragma pack(pop)
+
+struct A {
+  int i;
+  double d;
+} a;
+#pragma align(reset)
+#pragma pack(pop)
+
+int i = sizeof(A);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test4::A
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  8 |   double d
+// CHECK-NEXT:| [sizeof=16, dsize=16, align=4, preferredalign=8,
+// CHECK-NEXT:|  nvsize=16, nvalign=4, preferrednvalign=8]
+
+} // namespace test4
+
+namespace test5 {
+#pragma align(power)
+#pragma align(natural)
+#pragma pack(2)
+#pragma align(reset)
+struct A {
+  int i;
+  double d;
+};
+#pragma align(reset)
+
+int a = sizeof(A);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct test5::A
+// CHECK-NEXT:  0 |   int i
+// CHECK-NEXT:  4 |   double d
+// CHECK-NEXT:| [sizeof=12, dsize=12,

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune updated this revision to Diff 313989.
aqjune added a comment.
Herald added a reviewer: bollu.

Update comments
Call unary CreateShuffleVector
Update polly tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

Files:
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2.c
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/arm-mve-intrinsics/compare.c
  clang/test/CodeGen/arm-mve-intrinsics/cplusplus.cpp
  clang/test/CodeGen/arm-mve-intrinsics/dup.c
  clang/test/CodeGen/arm-mve-intrinsics/ternary.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vmulq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmullbq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulltq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vsubq.c
  clang/test/CodeGen/builtins-ppc-p10vector.c
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGen/vecshift.c
  clang/test/CodeGenCXX/matrix-type-operators.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/CodeGenCXX/vector-splat-conversion.cpp
  clang/test/CodeGenOpenCL/bool_cast.cl
  clang/test/CodeGenOpenCL/shifts.cl
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/IRBuilder.cpp
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts.ll
  llvm/test/Transforms/InstCombine/gep-inbounds-null.ll
  llvm/test/Transforms/InstCombine/getelementptr.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow-inseltpoison.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow.ll
  llvm/test/Transforms/InstCombine/type_pun-inseltpoison.ll
  llvm/test/Transforms/InstCombine/type_pun.ll
  llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_shuffle.ll
  llvm/test/Transforms/InstCombine/vscale_cmp.ll
  llvm/test/Transforms/LoopVectorize/AArch64/arbitrary-induction-step.ll
  
llvm/test/Transforms/LoopVectorize/AArch64/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-gather-scatter-tailpred.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-reduction-types.ll
  llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
  llvm/test/Transforms/LoopVectorize/ARM/tail-folding-not-allowed.ll
  llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/X86/constant-fold.ll
  llvm/test/Transforms/LoopVectorize/X86/cost-model-assert.ll
  llvm/test/Transforms/LoopVectorize/X86/interleaving.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-load-gather.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
  llvm/test/Transforms/LoopVectorize/X86/masked_load_store.ll
  llvm/test/Transforms/LoopVectorize/X86/metadata-enable.ll
  llvm/test/Transforms/LoopVectorize/X86/optsize.ll
  
llvm/test/Transforms/LoopVectorize/X86/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/X86/pr34438.ll
  llvm/test/Transforms/LoopVectorize/X86/small-size.ll
  llvm/test/Transforms/LoopVectorize/X86/tail_loop_folding.ll
  llvm/test/Transforms/LoopVectorize/X86/uniform_mem_op.ll
  llvm/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll
  
llvm/test/Transforms/LoopVectorize/X86/x86-interleaved-accesses-masked-group.ll
  llvm/test/Transforms/LoopVectorize/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-const-TC.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-divisible-TC.ll
  llvm/test/Transforms/LoopVectorize/first-order-recurrence-complex.ll
  llvm/test/Transforms/LoopVectorize/float-induction.ll
  llvm/test/Transforms/LoopVectorize/float-minmax-instruction-flag.ll
  llvm/test/Transforms/LoopVectorize/if-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/induction-step.ll
  llvm/test/Transforms/LoopVectorize/induction.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-1.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses.ll
  llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/loop-form.ll
  llvm/test/Transforms/LoopVectorize/minmax_reduction.ll
  llvm/test/Transforms/LoopVectorize/multiple-strides-vectorization.ll
  l

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

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



Comment at: llvm/lib/IR/IRBuilder.cpp:1020
   Value *Zeros = ConstantAggregateZero::get(VectorType::get(I32Ty, EC));
-  return CreateShuffleVector(V, Undef, Zeros, Name + ".splat");
+  return CreateShuffleVector(V, Poison, Zeros, Name + ".splat");
 }

spatel wrote:
> I'm still catching up on reviews/mails, so ignore if this was already 
> discussed:
> Can we change this usage of CreateShuffleVector to the unary operand version, 
> so we're reducing the number of explicit places where we need to specify 
> poison?
Yes, it makes sense.
I'll update D93817 to use the unary version as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

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

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan updated this revision to Diff 313990.
abhina.sreeskantharajan added a comment.

Thanks for the review! I've addressed most of the comments but I still need to 
work on the translation issues in CharLiteralParser that was kindly pointed out 
by Tom and Richard. Here are the summary of changes in this patch:

- Removed TranslationState as a member of StringLiteralParser and pass it as an 
argument instead
- Added an assertion for wide character translation instead of translating them 
to the system charset
- Invalid char escapes are changed to '?' and then translated
- Updated testcases as requested


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/include/clang/Lex/LiteralTranslator.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/CMakeLists.txt
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/LiteralTranslator.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CodeGen/systemz-charset.c
  clang/test/CodeGen/systemz-charset.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang_f_opts.c
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Support/Triple.cpp

Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -1023,6 +1023,13 @@
   return Tmp.split('-').second;  // Strip second component
 }
 
+// System charset on z/OS is IBM-1047 and UTF-8 otherwise
+StringRef Triple::getSystemCharset() const {
+  if (getOS() == llvm::Triple::ZOS)
+return "IBM-1047";
+  return "UTF-8";
+}
+
 static unsigned EatNumber(StringRef &Str) {
   assert(!Str.empty() && Str[0] >= '0' && Str[0] <= '9' && "Not a number");
   unsigned Result = 0;
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -390,6 +390,9 @@
   /// if the environment component is present).
   StringRef getOSAndEnvironmentName() const;
 
+  /// getSystemCharset - Get the system charset of the triple.
+  StringRef getSystemCharset() const;
+
   /// @}
   /// @name Convenience Predicates
   /// @{
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -209,8 +209,14 @@
 // RUN: %clang -### -S -finput-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-CHARSET %s
 // CHECK-INVALID-CHARSET: error: invalid value 'iso-8859-1' in '-finput-charset=iso-8859-1'
 
-// RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
-// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
+// RUN: %clang -### -S -fexec-charset=invalid-charset -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
+// CHECK-INVALID-INPUT-CHARSET: error: invalid value 'invalid-charset' in '-fexec-charset'
+
+// Test that we support the following exec charsets.
+// RUN: %clang -### -S -fexec-charset=UTF-8 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: %clang -### -S -fexec-charset=ISO8859-1 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// RUN: %clang -### -S -fexec-charset=IBM-1047 -o /dev/null %s 2>&1 | FileCheck --check-prefix=INVALID %s
+// INVALID-NOT: error: invalid value
 
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
@@ -224,7 +230,7 @@
 // RUN: -fident -fno-ident\
 // RUN: -fimplicit-templates -fno-implicit-templates  \
 // RUN: -finput-charset=UTF-8 \
-// RUN: -fexec-charset=UTF-8 \
+// RUN: -fexec-charset=UTF-8  \
 // RUN: -fivopts -fno-ivopts  \
 // RUN: -fnon-call-exceptions -fno-non-call-exceptions\
 // RUN: -fpermissive -fno-permissive  \
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -209,10 +209,11 @@
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
 // source-charset-utf-16: invalid value 'utf-16'

[PATCH] D93793: [IR] Let IRBuilder's CreateVectorSplat/CreateShuffleVector use poison as placeholder

[Juneyoung Lee via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
aqjune marked an inline comment as done.
Closed by commit rG278aa65cc495: [IR] Let IRBuilder's 
CreateVectorSplat/CreateShuffleVector use poison as… (authored by aqjune).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93793

Files:
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-constrained.c
  clang/test/CodeGen/SystemZ/builtins-systemz-zvector2.c
  clang/test/CodeGen/SystemZ/zvector.c
  clang/test/CodeGen/arm-mve-intrinsics/compare.c
  clang/test/CodeGen/arm-mve-intrinsics/cplusplus.cpp
  clang/test/CodeGen/arm-mve-intrinsics/dup.c
  clang/test/CodeGen/arm-mve-intrinsics/ternary.c
  clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vhsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vmulq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqaddq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmullbq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqdmulltq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqrdmulhq.c
  clang/test/CodeGen/arm-mve-intrinsics/vqsubq.c
  clang/test/CodeGen/arm-mve-intrinsics/vsubq.c
  clang/test/CodeGen/builtins-ppc-p10vector.c
  clang/test/CodeGen/matrix-type-operators.c
  clang/test/CodeGen/vecshift.c
  clang/test/CodeGenCXX/matrix-type-operators.cpp
  clang/test/CodeGenCXX/vector-conditional.cpp
  clang/test/CodeGenCXX/vector-splat-conversion.cpp
  clang/test/CodeGenOpenCL/bool_cast.cl
  clang/test/CodeGenOpenCL/shifts.cl
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/IRBuilder.cpp
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/X86/x86-vector-shifts.ll
  llvm/test/Transforms/InstCombine/gep-inbounds-null.ll
  llvm/test/Transforms/InstCombine/getelementptr.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow-inseltpoison.ll
  llvm/test/Transforms/InstCombine/shuffle-select-narrow.ll
  llvm/test/Transforms/InstCombine/type_pun-inseltpoison.ll
  llvm/test/Transforms/InstCombine/type_pun.ll
  llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
  llvm/test/Transforms/InstCombine/vec_shuffle.ll
  llvm/test/Transforms/InstCombine/vscale_cmp.ll
  llvm/test/Transforms/LoopVectorize/AArch64/arbitrary-induction-step.ll
  
llvm/test/Transforms/LoopVectorize/AArch64/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-gather-scatter-tailpred.ll
  llvm/test/Transforms/LoopVectorize/ARM/mve-reduction-types.ll
  llvm/test/Transforms/LoopVectorize/ARM/pointer_iv.ll
  llvm/test/Transforms/LoopVectorize/ARM/tail-folding-not-allowed.ll
  llvm/test/Transforms/LoopVectorize/PowerPC/optimal-epilog-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/X86/constant-fold.ll
  llvm/test/Transforms/LoopVectorize/X86/cost-model-assert.ll
  llvm/test/Transforms/LoopVectorize/X86/interleaving.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-load-gather.ll
  llvm/test/Transforms/LoopVectorize/X86/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
  llvm/test/Transforms/LoopVectorize/X86/masked_load_store.ll
  llvm/test/Transforms/LoopVectorize/X86/metadata-enable.ll
  llvm/test/Transforms/LoopVectorize/X86/optsize.ll
  
llvm/test/Transforms/LoopVectorize/X86/outer_loop_test1_no_explicit_vect_width.ll
  llvm/test/Transforms/LoopVectorize/X86/pr34438.ll
  llvm/test/Transforms/LoopVectorize/X86/small-size.ll
  llvm/test/Transforms/LoopVectorize/X86/tail_loop_folding.ll
  llvm/test/Transforms/LoopVectorize/X86/uniform_mem_op.ll
  llvm/test/Transforms/LoopVectorize/X86/vect.omp.force.small-tc.ll
  
llvm/test/Transforms/LoopVectorize/X86/x86-interleaved-accesses-masked-group.ll
  llvm/test/Transforms/LoopVectorize/consecutive-ptr-uniforms.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-const-TC.ll
  llvm/test/Transforms/LoopVectorize/dont-fold-tail-for-divisible-TC.ll
  llvm/test/Transforms/LoopVectorize/first-order-recurrence-complex.ll
  llvm/test/Transforms/LoopVectorize/float-induction.ll
  llvm/test/Transforms/LoopVectorize/float-minmax-instruction-flag.ll
  llvm/test/Transforms/LoopVectorize/if-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/induction-step.ll
  llvm/test/Transforms/LoopVectorize/induction.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-1.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses-pred-stores.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses.ll
  llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/loop-form.ll

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan marked 8 inline comments as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/include/clang/Lex/LiteralSupport.h:244
   bool Pascal;
+  ConversionState TranslationState;
+

tahonermann wrote:
> abhina.sreeskantharajan wrote:
> > tahonermann wrote:
> > > Same concern here with respect to persisting the conversion state as a 
> > > data member.
> > If this member is removed in StringLiteralParser, we will need to pass the 
> > State to multiple functions in StringLiteralParser like init(). Would this 
> > solution be preferable to keeping a data member?
> I think so, yes.  Data members should be used to reflect the state of the 
> object, not as a convenient mechanism to avoid passing arguments.
Thanks, I've removed this member.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1322-1323
+  TranslationState = translationState;
+  if (Kind == tok::wide_string_literal)
+TranslationState = TranslateToSystemCharset;
+  else if (isUTFLiteral(Kind))

tahonermann wrote:
> abhina.sreeskantharajan wrote:
> > tahonermann wrote:
> > > Converting wide character literals to the system encoding doesn't seem 
> > > right to me.  For z/OS, this should presumably convert to the wide EBCDIC 
> > > encoding, but for all other supported platforms, the wide execution 
> > > character set is either UTF-16 or UTF-32 depending on the size of 
> > > `wchar_t` (which may be influenced by the `-fshort-wchar` option).
> > Since we don't implement -fwide-exec-charset yet, what do you think should 
> > be the default behaviour for the interim?
> Perhaps an Internal compiler error to indicate that appropriate support is 
> not yet in place?
Thanks for the suggestion. I've added assertions for wide character translation 
before we do any translation.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1594-1595
+  ConversionState State = TranslationState;
+  if (Kind == tok::wide_string_literal)
+State = TranslateToSystemCharset;
+  else if (isUTFLiteral(Kind))

tahonermann wrote:
> Converting wide string literals to the system encoding doesn't seem right to 
> me.  For z/OS, this should presumably convert to the wide EBCDIC encoding, 
> but for all other supported platforms, the wide execution character set is 
> either UTF-16 or UTF-32 depending on the size of `wchar_t` (which may be 
> influenced by the `-fshort-wchar` option).
I've now added an assertion when translating wide characters.



Comment at: clang/test/CodeGen/systemz-charset.cpp:1-25
+// RUN: %clang %s -std=c++17 -emit-llvm -S -target s390x-ibm-zos -o - | 
FileCheck %s
+
+const char *RawString = R"(Hello\n)";
+//CHECK: c"\C8\85\93\93\96\E0\95\00"
+
+char UnicodeChar8 = u8'1';
+//CHECK: i8 49

tahonermann wrote:
> This is good.  I suggest adding escape sequences and UCNs to validate that 
> they are not converted to IBM-1047.
Good idea, I added those testcases as per your suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

This change looks good to me but I would like @rnk to sign off.  My opinion is 
that the change is simple enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93747

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

[PATCH] D93031: Enable fexec-charset option

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan marked an inline comment as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Lex/LiteralSupport.cpp:234
+SmallString<8> ResultCharConv;
+Converter->convert(std::string(1, ByteChar), ResultCharConv);
+memcpy((void *)&ResultChar, ResultCharConv.data(), sizeof(unsigned));

tahonermann wrote:
> Conversion can fail here, particularly in the scenario corresponding to the 
> default switch case above; `ResultChar` could contain, for example, a lead 
> byte of a UTF-8 sequence.  Something sensible should be done here; either 
> rejecting the code with an error or substituting `?` (in the execution 
> encoding) seems appropriate to me.
Thanks, I added the substitution with the '?' character for invalid escapes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93031

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

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

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

HazardyKnusperkeks added a comment.

In D93844#2473725 , @MyDeveloperDay 
wrote:

> I like what you are suggesting, do you think `BasedOnStyle: File` is the best 
> terminology as the term `File` is used elsewhere to mean read from the 
> .clang_format file, how about
>
> `BasedOnStyle: Inherit`
> `BasedOnStyle: InheritFromParent`
> `BasedOnStyle: InheritFromFile`
>
> or why not just
>
> `BasedOnStyle: InheritParentConfig`
>
> to match clang-tidy?

I will update it to `InheritParentConfig`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93844

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

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Haven't reviewed the code in detail, but the idea seems sound. Is there any 
prior art in other compilers you can draw data/experiences from? & having some 
data from some reasonably sized codebases on false positive (how often the 
correct thing is to suppress the warning, versus fixing the warning) would 
probably be useful. I would guess it doesn't meet the on-by-default bar we 
would prefer for Clang, but might still be OK.




Comment at: clang/docs/ReleaseNotes.rst:64-74
+  - ``-Wimplicit-widening-of-pointer-offset``:
+
+.. code-block:: c++
+
+  char* ptr_add(char *base, int a, int b) {
+return base + a * b; // expected-warning {{result of multiplication in 
type 'int' is used as a pointer offset after an implicit widening conversion to 
type 'ssize_t'}}
+  }

Does this only trigger when the `sizeof(char*) > sizeof(int)`? (judging by the 
test coverage I think that's the case)

(ultimately, might be worth committing the two diagnostics separately - usual 
sort of reasons, separation of concerns, etc)



Comment at: 
clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24
+void t1(char *base, int a, int b) {
+  // FIXME: test `[a * b]base` pattern?
+}

Question is unclear - is there uncertainty about whether this should be tested? 
Or is this a false negative case? In which case I'd probably include the test 
showing no diagnostic and mention it could be fixed/improved?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

[PATCH] D93919: [PowerPC] powerpcle target 3/5

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 created this revision.
Herald added subscribers: shchenz, kbarton, nemanjai, emaste.
Bdragon28 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Splitting D92445 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93919

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/altivec.c
  clang/test/CodeGen/builtins-ppc-altivec.c
  clang/test/CodeGen/ppc32-and-aix-struct-return.c
  clang/test/CodeGen/target-data.c
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/ppc-endian.c

Index: clang/test/Driver/ppc-endian.c
===
--- clang/test/Driver/ppc-endian.c
+++ clang/test/Driver/ppc-endian.c
@@ -1,9 +1,19 @@
-// RUN: %clang -target powerpc64le -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64le -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// CHECK-LE: "-cc1"{{.*}} "-triple" "powerpc64le{{.*}}"
+// RUN: %clang -target powerpc-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpc-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpcle-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// CHECK-BE32: "-cc1"{{.*}} "-triple" "powerpc-{{.*}}"
 
-// RUN: %clang -target powerpc64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// RUN: %clang -target powerpc64 -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// RUN: %clang -target powerpc64le -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// CHECK-BE: "-cc1"{{.*}} "-triple" "powerpc64{{.*}}"
+// RUN: %clang -target powerpcle-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// RUN: %clang -target powerpcle-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// RUN: %clang -target powerpc-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// CHECK-LE32: "-cc1"{{.*}} "-triple" "powerpcle-{{.*}}"
+
+// RUN: %clang -target powerpc64-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// RUN: %clang -target powerpc64-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// RUN: %clang -target powerpc64le-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// CHECK-BE64: "-cc1"{{.*}} "-triple" "powerpc64-{{.*}}"
+
+// RUN: %clang -target powerpc64le-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// RUN: %clang -target powerpc64le-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// RUN: %clang -target powerpc64-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// CHECK-LE64: "-cc1"{{.*}} "-triple" "powerpc64le-{{.*}}"
Index: clang/test/Driver/linux-header-search.cpp
===
--- clang/test/Driver/linux-header-search.cpp
+++ clang/test/Driver/linux-header-search.cpp
@@ -178,7 +178,7 @@
 //
 // Test Ubuntu/Debian's Ubuntu 14.04 for powerpc64le
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
-// RUN: -target powerpc64le-unknown-linux-gnu -m32 -stdlib=libstdc++ \
+// RUN: -target powerpc64le-unknown-linux-gnu -stdlib=libstdc++ \
 // RUN: --sysroot=%S/Inputs/ubuntu_14.04_multiarch_tree \
 // RUN: --gcc-toolchain="" \
 // RUN:   | FileCheck --check-prefix=CHECK-UBUNTU-14-04-PPC64LE %s
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -126,6 +126,10 @@
 // RUN: FileCheck %s -check-prefix=PPC
 // PPC: target datalayout = "E-m:e-p:32:32-i64:64-n32"
 
+// RUN: %clang_cc1 -triple powerpcle-unknown -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=PPCLE
+// PPCLE: target datalayout = "e-m:e-p:32:32-i64:64-n32"
+
 // RUN: %clang_cc1 -triple powerpc64-freebsd -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC64-FREEBSD
 // PPC64-FREEBSD: target datalayout = "E-m:e-i64:64-n32:64"
Index: clang/test/CodeGen/ppc32-and-aix-struct-return.c

[PATCH] D92445: [PowerPC] Add powerpcle target. (5/5)

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 updated this revision to Diff 314012.
Bdragon28 retitled this revision from "[PowerPC] Add powerpcle target." to 
"[PowerPC] Add powerpcle target. (5/5)".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92445

Files:
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -40,6 +40,7 @@
   case Triple::mips64:
   case Triple::mips64el:
   case Triple::ppc:
+  case Triple::ppcle:
   case Triple::ppc64:
   case Triple::ppc64le:
   case Triple::x86:
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1033,6 +1033,7 @@
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_be)
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_32)
 __OMP_TRAIT_PROPERTY(device, arch, ppc)
+__OMP_TRAIT_PROPERTY(device, arch, ppcle)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64le)
 __OMP_TRAIT_PROPERTY(device, arch, x86)


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -40,6 +40,7 @@
   case Triple::mips64:
   case Triple::mips64el:
   case Triple::ppc:
+  case Triple::ppcle:
   case Triple::ppc64:
   case Triple::ppc64le:
   case Triple::x86:
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1033,6 +1033,7 @@
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_be)
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_32)
 __OMP_TRAIT_PROPERTY(device, arch, ppc)
+__OMP_TRAIT_PROPERTY(device, arch, ppcle)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64le)
 __OMP_TRAIT_PROPERTY(device, arch, x86)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for taking a look!

In D93822#2474236 , @dblaikie wrote:

> Haven't reviewed the code in detail, but the idea seems sound.

That's reassuring!

> Is there any prior art in other compilers you can draw data/experiences from?

As far as i can tell, no other compiler diagnoses these cases,
however e.g. GitHub's CodeQL static analysis tool complains about this by 
default:
https://github.com/darktable-org/rawspeed/security/code-scanning/7?query=342efe49c0bef4bf2450051586d899f9d303ae62

> & having some data from some reasonably sized codebases on false positive
> (how often the correct thing is to suppress the warning, versus fixing the 
> warning)
> would probably be useful.

I don't have those numbers presently (other than the long long diffs of fixes 
for true-positives i have already linked),
but as a rule of thumb, iff a particular project in question
never allocates more than `UINT_MAX` bytes / `UINT_MAX / sizeof(element)` 
elements,
*all* issued diagnostics for said project *will* be false-positives by 
definition.
So YMMV.

> I would guess it doesn't meet the on-by-default bar we would prefer for Clang,

Yeah, based on my initial observations, it's pretty chatty.

> but might still be OK.

I'm open to input here, but it would be good to have this at least in `-Wextra` 
(`-Weverything` always being a last resort option).




Comment at: clang/docs/ReleaseNotes.rst:64-74
+  - ``-Wimplicit-widening-of-pointer-offset``:
+
+.. code-block:: c++
+
+  char* ptr_add(char *base, int a, int b) {
+return base + a * b; // expected-warning {{result of multiplication in 
type 'int' is used as a pointer offset after an implicit widening conversion to 
type 'ssize_t'}}
+  }

dblaikie wrote:
> Does this only trigger when the `sizeof(char*) > sizeof(int)`? (judging by 
> the test coverage I think that's the case)
> 
> (ultimately, might be worth committing the two diagnostics separately - usual 
> sort of reasons, separation of concerns, etc)
> Does this only trigger when the sizeof(char*) > sizeof(int)? (judging by the 
> test coverage I think that's the case)

That should be the case, since `sizeof(size_t) == sizeof(char*)` and
`// RUN: %clang_cc1 -triple i686-linux-gnu   -fsyntax-only 
-Wimplicit-widening-of-pointer-offset -verify=silent %s`

> (ultimately, might be worth committing the two diagnostics separately - usual 
> sort of reasons, separation of concerns, etc)

Should i split this into two reviews to begin with?



Comment at: 
clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24
+void t1(char *base, int a, int b) {
+  // FIXME: test `[a * b]base` pattern?
+}

dblaikie wrote:
> Question is unclear - is there uncertainty about whether this should be 
> tested? Or is this a false negative case? In which case I'd probably include 
> the test showing no diagnostic and mention it could be fixed/improved?
I may be misremembering things, but IIRC `a[b]` and `b[a]` is the same thing,
but i'm not sure how to exercise the second spelling.
I.e. i'm just not sure how to write a test for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

[PATCH] D92445: [PowerPC] Add powerpcle target. (5/5)

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 updated this revision to Diff 314019.
Bdragon28 added a comment.

Re-uploading patch for part 5 now that I have the dependency tree fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92445

Files:
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -40,6 +40,7 @@
   case Triple::mips64:
   case Triple::mips64el:
   case Triple::ppc:
+  case Triple::ppcle:
   case Triple::ppc64:
   case Triple::ppc64le:
   case Triple::x86:
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1033,6 +1033,7 @@
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_be)
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_32)
 __OMP_TRAIT_PROPERTY(device, arch, ppc)
+__OMP_TRAIT_PROPERTY(device, arch, ppcle)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64le)
 __OMP_TRAIT_PROPERTY(device, arch, x86)


Index: llvm/lib/Frontend/OpenMP/OMPContext.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -40,6 +40,7 @@
   case Triple::mips64:
   case Triple::mips64el:
   case Triple::ppc:
+  case Triple::ppcle:
   case Triple::ppc64:
   case Triple::ppc64le:
   case Triple::x86:
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -1033,6 +1033,7 @@
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_be)
 __OMP_TRAIT_PROPERTY(device, arch, aarch64_32)
 __OMP_TRAIT_PROPERTY(device, arch, ppc)
+__OMP_TRAIT_PROPERTY(device, arch, ppcle)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64)
 __OMP_TRAIT_PROPERTY(device, arch, ppc64le)
 __OMP_TRAIT_PROPERTY(device, arch, x86)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49
 
+// LPIPELINE: Unique Internal Linkage Names
+// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass
 // PLAIN: @_ZL4glob = internal global

hoy wrote:
> dblaikie wrote:
> > aeubanks wrote:
> > > dblaikie wrote:
> > > > hoy wrote:
> > > > > aeubanks wrote:
> > > > > > hoy wrote:
> > > > > > > aeubanks wrote:
> > > > > > > > dblaikie wrote:
> > > > > > > > > hoy wrote:
> > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > Does this test validate the new behavior? (ie: does this 
> > > > > > > > > > > test fail without the LLVM changes and pass with it) Not 
> > > > > > > > > > > that it necessarily has to - since Clang isn't here to 
> > > > > > > > > > > test the LLVM behavior - perhaps this test is sufficient 
> > > > > > > > > > > in Clang to test that the code in BackendUtil works to 
> > > > > > > > > > > enable this pass.
> > > > > > > > > > > 
> > > > > > > > > > > This could possibly be staged as independent commits - 
> > > > > > > > > > > adding the LLVM functionality in one commit, which would 
> > > > > > > > > > > be a no-op for Clang because it wouldn't be setting 
> > > > > > > > > > > PTO.UniqueLinkageNames - then committing the Clang change 
> > > > > > > > > > > that would remove the custom pass addition and set 
> > > > > > > > > > > PTO.UniqueLinkageNames - and then it'd probably be 
> > > > > > > > > > > reasonable to have this test be made a bit more explicit 
> > > > > > > > > > > (testing the pass manager structure/order) to show that 
> > > > > > > > > > > that Clang change had an effect: Moving the pass to the 
> > > > > > > > > > > desired location in the pass pipeline.
> > > > > > > > > > This is a good question. No, this test does not validate 
> > > > > > > > > > the pipeline change on the LLVM side, since Clang shouldn't 
> > > > > > > > > > have knowledge about how the pipelines are arranged in 
> > > > > > > > > > LLVM. As you pointed out, the test here is to test if the 
> > > > > > > > > > specific pass is run and gives expected results.
> > > > > > > > > > 
> > > > > > > > > > Thanks for the suggestion to break the Clang changes and 
> > > > > > > > > > LLVM changes apart which would make the testing more 
> > > > > > > > > > specific. The pipeline ordering could be tested with a LLVM 
> > > > > > > > > > test but that would require a LLVM switch setup for 
> > > > > > > > > > UniqueLinkageNames and I'm not sure there's a need for that 
> > > > > > > > > > switch except for testing.
> > > > > > > > > > No, this test does not validate the pipeline change on the 
> > > > > > > > > > LLVM side, since Clang shouldn't have knowledge about how 
> > > > > > > > > > the pipelines are arranged in LLVM. 
> > > > > > > > > 
> > > > > > > > > "ish" - but Clang should have tests for changes to Clang, 
> > > > > > > > > ideally. Usually they can simply be testing LLVM's IR output 
> > > > > > > > > before it goes to LLVM for optimization/codegen - but for 
> > > > > > > > > features that don't have this serialization boundary that 
> > > > > > > > > makes testing and isolation clear/simple, it becomes a bit 
> > > > > > > > > fuzzier.
> > > > > > > > > 
> > > > > > > > > In this case, there is a clang change - from adding the pass 
> > > > > > > > > explicitly in Clang, to setting a parameter about how LLVM 
> > > > > > > > > will add the pass, and it has an observable effect. One way 
> > > > > > > > > to test this change while isolating the Clang test from 
> > > > > > > > > further changes to the pipeline in LLVM, would be to test 
> > > > > > > > > that the pass ends up somewhere in the LLVM-created part of 
> > > > > > > > > the pass pipeline - the parts that you can't get to from the 
> > > > > > > > > way the original pass addition was written in Clang. At least 
> > > > > > > > > I assume that's the case/what motivated the change from 
> > > > > > > > > adding it in Clang to adding it in LLVM?
> > > > > > > > > 
> > > > > > > > > eg: if LLVM always forms passes {x, y, z} and Clang is able 
> > > > > > > > > to add passes before/after, say it always adds 'a' before and 
> > > > > > > > > 'b' after, to make {a, x, y, z, b} - and this new pass u was 
> > > > > > > > > previously added at the start to make {u, a, x, y, z, b} but 
> > > > > > > > > now needs to go in {a, x, y, u, z, b} you could test that 'u' 
> > > > > > > > > is after 'a' and before 'b', or between 'x' and 'z', etc. If 
> > > > > > > > > there's some other more clear/simple/reliable marker of where 
> > > > > > > > > the LLVM-created passes start/end in the structured dump, 
> > > > > > > > > that'd be good to use as a landmark to make such a test more 
> > > > > > > > > robust. If there's some meaningful pass that this pass always 
> > > > > > > > > needs to go after - testing that might be OK, even if it's 
> > > > > > > > > somewhat an implementation detail of LLVM 

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49
 
+// LPIPELINE: Unique Internal Linkage Names
+// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass
 // PLAIN: @_ZL4glob = internal global

dblaikie wrote:
> hoy wrote:
> > dblaikie wrote:
> > > aeubanks wrote:
> > > > dblaikie wrote:
> > > > > hoy wrote:
> > > > > > aeubanks wrote:
> > > > > > > hoy wrote:
> > > > > > > > aeubanks wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > hoy wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > Does this test validate the new behavior? (ie: does 
> > > > > > > > > > > > this test fail without the LLVM changes and pass with 
> > > > > > > > > > > > it) Not that it necessarily has to - since Clang isn't 
> > > > > > > > > > > > here to test the LLVM behavior - perhaps this test is 
> > > > > > > > > > > > sufficient in Clang to test that the code in 
> > > > > > > > > > > > BackendUtil works to enable this pass.
> > > > > > > > > > > > 
> > > > > > > > > > > > This could possibly be staged as independent commits - 
> > > > > > > > > > > > adding the LLVM functionality in one commit, which 
> > > > > > > > > > > > would be a no-op for Clang because it wouldn't be 
> > > > > > > > > > > > setting PTO.UniqueLinkageNames - then committing the 
> > > > > > > > > > > > Clang change that would remove the custom pass addition 
> > > > > > > > > > > > and set PTO.UniqueLinkageNames - and then it'd probably 
> > > > > > > > > > > > be reasonable to have this test be made a bit more 
> > > > > > > > > > > > explicit (testing the pass manager structure/order) to 
> > > > > > > > > > > > show that that Clang change had an effect: Moving the 
> > > > > > > > > > > > pass to the desired location in the pass pipeline.
> > > > > > > > > > > This is a good question. No, this test does not validate 
> > > > > > > > > > > the pipeline change on the LLVM side, since Clang 
> > > > > > > > > > > shouldn't have knowledge about how the pipelines are 
> > > > > > > > > > > arranged in LLVM. As you pointed out, the test here is to 
> > > > > > > > > > > test if the specific pass is run and gives expected 
> > > > > > > > > > > results.
> > > > > > > > > > > 
> > > > > > > > > > > Thanks for the suggestion to break the Clang changes and 
> > > > > > > > > > > LLVM changes apart which would make the testing more 
> > > > > > > > > > > specific. The pipeline ordering could be tested with a 
> > > > > > > > > > > LLVM test but that would require a LLVM switch setup for 
> > > > > > > > > > > UniqueLinkageNames and I'm not sure there's a need for 
> > > > > > > > > > > that switch except for testing.
> > > > > > > > > > > No, this test does not validate the pipeline change on 
> > > > > > > > > > > the LLVM side, since Clang shouldn't have knowledge about 
> > > > > > > > > > > how the pipelines are arranged in LLVM. 
> > > > > > > > > > 
> > > > > > > > > > "ish" - but Clang should have tests for changes to Clang, 
> > > > > > > > > > ideally. Usually they can simply be testing LLVM's IR 
> > > > > > > > > > output before it goes to LLVM for optimization/codegen - 
> > > > > > > > > > but for features that don't have this serialization 
> > > > > > > > > > boundary that makes testing and isolation clear/simple, it 
> > > > > > > > > > becomes a bit fuzzier.
> > > > > > > > > > 
> > > > > > > > > > In this case, there is a clang change - from adding the 
> > > > > > > > > > pass explicitly in Clang, to setting a parameter about how 
> > > > > > > > > > LLVM will add the pass, and it has an observable effect. 
> > > > > > > > > > One way to test this change while isolating the Clang test 
> > > > > > > > > > from further changes to the pipeline in LLVM, would be to 
> > > > > > > > > > test that the pass ends up somewhere in the LLVM-created 
> > > > > > > > > > part of the pass pipeline - the parts that you can't get to 
> > > > > > > > > > from the way the original pass addition was written in 
> > > > > > > > > > Clang. At least I assume that's the case/what motivated the 
> > > > > > > > > > change from adding it in Clang to adding it in LLVM?
> > > > > > > > > > 
> > > > > > > > > > eg: if LLVM always forms passes {x, y, z} and Clang is able 
> > > > > > > > > > to add passes before/after, say it always adds 'a' before 
> > > > > > > > > > and 'b' after, to make {a, x, y, z, b} - and this new pass 
> > > > > > > > > > u was previously added at the start to make {u, a, x, y, z, 
> > > > > > > > > > b} but now needs to go in {a, x, y, u, z, b} you could test 
> > > > > > > > > > that 'u' is after 'a' and before 'b', or between 'x' and 
> > > > > > > > > > 'z', etc. If there's some other more clear/simple/reliable 
> > > > > > > > > > marker of where the LLVM-created passes start/end in the 
> > > > > > > > > > structured dump, that'd be good to use as a landmark to 
> > > > > > > > > > make such a test more robust. If there's some

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, rjmccall.
jyknight requested review of this revision.
Herald added projects: clang, libc++abi, LLVM.
Herald added subscribers: llvm-commits, libcxx-commits, cfe-commits.
Herald added a reviewer: libc++abi.

The two operations have acted differently since Clang 8, but were
unfortunately mangled the same. The new mangling uses new "vendor
extended expression" syntax proposed in
https://github.com/itanium-cxx-abi/cxx-abi/issues/112

GCC had the same mangling problem, https://gcc.gnu.org/PR88115, and
will hopefully be switching to the same mangling as implemented here.

Additionally, fix the mangling of `__uuidof` to use the new extension
syntax, instead of its previous nonstandard special-case.

Adjusts the demangler accordingly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93922

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-alignof.cpp
  clang/test/CodeGenCXX/microsoft-uuidof-mangling.cpp
  libcxxabi/src/demangle/ItaniumDemangle.h
  libcxxabi/test/test_demangle.pass.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h

Index: llvm/include/llvm/Demangle/ItaniumDemangle.h
===
--- llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -96,7 +96,6 @@
 X(InitListExpr) \
 X(FoldExpr) \
 X(ThrowExpr) \
-X(UUIDOfExpr) \
 X(BoolExpr) \
 X(StringLiteral) \
 X(LambdaExpr) \
@@ -2035,21 +2034,6 @@
   }
 };
 
-// MSVC __uuidof extension, generated by clang in -fms-extensions mode.
-class UUIDOfExpr : public Node {
-  Node *Operand;
-public:
-  UUIDOfExpr(Node *Operand_) : Node(KUUIDOfExpr), Operand(Operand_) {}
-
-  template void match(Fn F) const { F(Operand); }
-
-  void printLeft(OutputStream &S) const override {
-S << "__uuidof(";
-Operand->print(S);
-S << ")";
-  }
-};
-
 class BoolExpr : public Node {
   bool Value;
 
@@ -5013,6 +4997,43 @@
 }
 }
 return nullptr;
+  case 'u': {
+++First;
+Node *Name = getDerived().parseSourceName(/*NameState=*/nullptr);
+if (!Name)
+  return nullptr;
+// Special case legacy __uuidof mangling. The 't' and 'z' appear where the
+// standard encoding expects a , and would be otherwise be
+// interpreted as  node 'short' or 'ellipsis'. However, neither
+// __uuidof(short) nor __uuidof(...) can actually appear, so there is no
+// actual conflict here.
+if (Name->getBaseName() == "__uuidof") {
+  if (numLeft() < 2)
+return nullptr;
+  if (*First == 't') {
+++First;
+Node *Ty = getDerived().parseType();
+if (!Ty)
+  return nullptr;
+return make(Name, makeNodeArray(&Ty, &Ty + 1));
+  }
+  if (*First == 'z') {
+++First;
+Node *Ex = getDerived().parseExpr();
+if (!Ex)
+  return nullptr;
+return make(Name, makeNodeArray(&Ex, &Ex + 1));
+  }
+}
+size_t ExprsBegin = Names.size();
+while (!consumeIf('E')) {
+  Node *E = getDerived().parseTemplateArg();
+  if (E == nullptr)
+return E;
+  Names.push_back(E);
+}
+return make(Name, popTrailingNodeArray(ExprsBegin));
+  }
   case '1':
   case '2':
   case '3':
@@ -5024,21 +5045,6 @@
   case '9':
 return getDerived().parseUnresolvedName();
   }
-
-  if (consumeIf("u8__uuidoft")) {
-Node *Ty = getDerived().parseType();
-if (!Ty)
-  return nullptr;
-return make(Ty);
-  }
-
-  if (consumeIf("u8__uuidofz")) {
-Node *Ex = getDerived().parseExpr();
-if (!Ex)
-  return nullptr;
-return make(Ex);
-  }
-
   return nullptr;
 }
 
Index: libcxxabi/src/demangle/ItaniumDemangle.h
===
--- libcxxabi/src/demangle/ItaniumDemangle.h
+++ libcxxabi/src/demangle/ItaniumDemangle.h
@@ -96,7 +96,6 @@
 X(InitListExpr) \
 X(FoldExpr) \
 X(ThrowExpr) \
-X(UUIDOfExpr) \
 X(BoolExpr) \
 X(StringLiteral) \
 X(LambdaExpr) \
@@ -2035,21 +2034,6 @@
   }
 };
 
-// MSVC __uuidof extension, generated by clang in -fms-extensions mode.
-class UUIDOfExpr : public Node {
-  Node *Operand;
-public:
-  UUIDOfExpr(Node *Operand_) : Node(KUUIDOfExpr), Operand(Operand_) {}
-
-  template void match(Fn F) const { F(Operand); }
-
-  void printLeft(OutputStream &S) const override {
-S << "__uuidof(";
-Operand->print(S);
-S << ")";
-  }
-};
-
 class BoolExpr : public Node {
   bool Value;
 
@@ -5013,6 +4997,43 @@
 }
 }
 return nullptr;
+  case 'u': {
+++First;
+Node *Name = getDerived().parseSourceName(/*NameState=*/nullptr);
+if (!Name)
+  return nullptr;
+// Special case legacy __uuidof mangling. The 't' and 'z' appear where the
+// standard encoding expects a , and would be otherwise be
+// interpreted as  node 'short' or 'ellipsis'. However, neither
+// __uuidof(short

[PATCH] D93923: Use unary CreateShuffleVector if possible

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune created this revision.
aqjune added reviewers: spatel, nikic, lebedev.ri, craig.topper, RKSimon, 
efriedma.
Herald added subscribers: dexonsmith, kerbowa, pengfei, dmgreen, hiraditya, 
nhaehnle, jvesely, nemanjai, arsenm.
aqjune requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

As mentioned in D93793 , there are quite a few 
places where unary `IRBuilder::CreateShuffleVector(X, Mask)` can be used
instead of `IRBuilder::CreateShuffleVector(X, Undef, Mask)`.
Let's update them.

Actually, it would have been more natural if the patches were made in this 
order:
(1) let them use unary CreateShuffleVector first
(2) update IRBuilder::CreateShuffleVector to use poison as a placeholder value 
(D93793 )

The order is swapped, but in terms of correctness it is still fine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93923

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/X86/avx-builtins.c
  clang/test/CodeGen/X86/avx2-builtins.c
  clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
  clang/test/CodeGen/X86/avx512bw-builtins.c
  clang/test/CodeGen/X86/avx512dq-builtins.c
  clang/test/CodeGen/X86/avx512f-builtins.c
  clang/test/CodeGen/X86/avx512vl-builtins-constrained.c
  clang/test/CodeGen/X86/avx512vl-builtins.c
  clang/test/CodeGen/X86/avx512vlbw-builtins.c
  clang/test/CodeGen/X86/avx512vldq-builtins.c
  clang/test/CodeGen/X86/f16c-builtins-constrained.c
  clang/test/CodeGen/X86/f16c-builtins.c
  clang/test/CodeGen/X86/sse2-builtins.c
  clang/test/CodeGen/arm-mve-intrinsics/vmovl.c
  clang/test/CodeGen/arm-mve-intrinsics/vmovn.c
  clang/test/CodeGen/arm-mve-intrinsics/vrev.c
  clang/test/CodeGen/arm64-abi-vector.c
  clang/test/CodeGenOpenCL/as_type.cl
  clang/test/CodeGenOpenCL/partial_initializer.cl
  clang/test/CodeGenOpenCL/preserve_vec3.cl
  clang/test/CodeGenOpenCL/vectorLoadStore.cl
  clang/test/CodeGenOpenCL/vector_literals_valid.cl
  llvm/lib/Analysis/VectorUtils.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/InterleavedLoadCombinePass.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
  llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
  llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp
  llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
  llvm/lib/Target/X86/X86InterleavedAccess.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/CodeGen/AArch64/aarch64-interleaved-ld-combine.ll
  llvm/test/CodeGen/AMDGPU/lower-kernargs.ll
  llvm/test/CodeGen/AMDGPU/rewrite-out-arguments-address-space.ll
  llvm/test/CodeGen/AMDGPU/rewrite-out-arguments.ll
  llvm/test/CodeGen/Generic/expand-experimental-reductions.ll
  llvm/test/Instrumentation/MemorySanitizer/clmul.ll
  
llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts-inseltpoison.ll
  llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts.ll
  llvm/test/Transforms/InstCombine/canonicalize-vector-insert.ll
  
llvm/test/Transforms/InterleavedAccess/AArch64/interleaved-accesses-inseltpoison.ll
  llvm/test/Transforms/InterleavedAccess/AArch64/interleaved-accesses.ll
  
llvm/test/Transforms/InterleavedAccess/ARM/interleaved-accesses-inseltpoison.ll
  llvm/test/Transforms/InterleavedAccess/ARM/interleaved-accesses.ll
  llvm/test/Transforms/InterleavedAccess/X86/interleavedStore-inseltpoison.ll
  llvm/test/Transforms/InterleavedAccess/X86/interleavedStore.ll
  llvm/test/Transforms/LoopVectorize/AMDGPU/packed-math.ll
  llvm/test/Transforms/LoopVectorize/ARM/sphinx.ll
  llvm/test/Transforms/LoopVectorize/PowerPC/widened-massv-call.ll
  llvm/test/Transforms/LoopVectorize/PowerPC/widened-massv-vfabi-attr.ll
  llvm/test/Transforms/LoopVectorize/float-minmax-instruction-flag.ll
  llvm/test/Transforms/LoopVectorize/interleaved-accesses.ll
  llvm/test/Transforms/LoopVectorize/invariant-store-vectorization.ll
  llvm/test/Transforms/LoopVectorize/reduction.ll
  llvm/test/Transforms/LoopVectorize/select-reduction.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/bigger-expressions-double.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/const-gep.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/load-align-volatile.ll
  
llvm/test/Transforms/LowerMatrixIntrinsics/multiply-add-sub-double-row-major.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/multiply-double-contraction-fmf.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/multiply-double-contraction.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/multiply-double-row-major.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/multiply-double.ll
  llvm/test/Transforms/LowerMatrixIntrinsics/multiply-float-co

[PATCH] D93923: Use unary CreateShuffleVector if possible

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: llvm/lib/Transforms/Scalar/SROA.cpp:2260
-  V = IRB.CreateSelect(ConstantVector::get(Mask), V, Old, Name + "blend");
+  V = IRB.CreateSelect(ConstantVector::get(Mask2), V, Old, Name + "blend");
 
   LLVM_DEBUG(dbgs() << "blend: " << *V << "\n");

I think it is the only part where a non-trivial change has made


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93923

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

[PATCH] D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates

[Marek Kurdej via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:364
+
+if (TheLine->Last->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->Last && Style.BraceWrapping.SplitEmptyRecord) {

Could you please add a comment about this block of code?
Something's like "Don't merge blocks when SplitEmptyRecords is enabled." or 
whatever you judge more useful/correct.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:370
+  Previous = Previous->getPreviousNonComment();
+if (Previous && Previous->is(tok::greater))
+  return 0;

You might want to factor out the check of Previous from this and the next if.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:375
+  Previous->getPreviousNonComment();
+  if (PreviousPrevious && Previous->is(tok::identifier) &&
+  PreviousPrevious->isOneOf(tok::kw_class, tok::kw_struct))

You should check the type of Previous before computing PreviousPrevious to 
avoid the latter when unnecessary.



Comment at: clang/unittests/Format/FormatTest.cpp:9952
+   "{\n"
+   "  int x;\n"
+   "};",

What about testing specializations with empty body?


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

https://reviews.llvm.org/D93839

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Nit.




Comment at: clang/docs/ClangFormatStyleOptions.rst:1545
+**EmptyLineBeforeAccessModifier** (``bool``)
+  If true, the empty line is inserted before access modifiers
+

Full stop at the end of the phrase.


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

https://reviews.llvm.org/D93846

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

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: rtrieu.
dblaikie added a comment.

In D93822#2474355 , @lebedev.ri wrote:

> Thank you for taking a look!
>
> In D93822#2474236 , @dblaikie wrote:
>
>> Haven't reviewed the code in detail, but the idea seems sound.
>
> That's reassuring!
>
>> Is there any prior art in other compilers you can draw data/experiences from?
>
> As far as i can tell, no other compiler diagnoses these cases,
> however e.g. GitHub's CodeQL static analysis tool complains about this by 
> default:
> https://github.com/darktable-org/rawspeed/security/code-scanning/7?query=342efe49c0bef4bf2450051586d899f9d303ae62
>
>> & having some data from some reasonably sized codebases on false positive
>> (how often the correct thing is to suppress the warning, versus fixing the 
>> warning)
>> would probably be useful.
>
> I don't have those numbers presently (other than the long long diffs of fixes 
> for true-positives i have already linked),
> but as a rule of thumb, iff a particular project in question
> never allocates more than `UINT_MAX` bytes / `UINT_MAX / sizeof(element)` 
> elements,
> *all* issued diagnostics for said project *will* be false-positives by 
> definition.
> So YMMV.
>
>> I would guess it doesn't meet the on-by-default bar we would prefer for 
>> Clang,
>
> Yeah, based on my initial observations, it's pretty chatty.
>
>> but might still be OK.
>
> I'm open to input here, but it would be good to have this at least in 
> `-Wextra` (`-Weverything` always being a last resort option).

Yeah, not sure how @rsmith, @rtrieu, or @aaron.ballman feel about what the bar 
for warnings is these days - used to be a bit more hardcore about the "if it 
can't be on by default it shouldn't be in clang" than we are these days, I 
think. I'd guess -Wextra might be suitable.




Comment at: clang/docs/ReleaseNotes.rst:64-74
+  - ``-Wimplicit-widening-of-pointer-offset``:
+
+.. code-block:: c++
+
+  char* ptr_add(char *base, int a, int b) {
+return base + a * b; // expected-warning {{result of multiplication in 
type 'int' is used as a pointer offset after an implicit widening conversion to 
type 'ssize_t'}}
+  }

lebedev.ri wrote:
> dblaikie wrote:
> > Does this only trigger when the `sizeof(char*) > sizeof(int)`? (judging by 
> > the test coverage I think that's the case)
> > 
> > (ultimately, might be worth committing the two diagnostics separately - 
> > usual sort of reasons, separation of concerns, etc)
> > Does this only trigger when the sizeof(char*) > sizeof(int)? (judging by 
> > the test coverage I think that's the case)
> 
> That should be the case, since `sizeof(size_t) == sizeof(char*)` and
> `// RUN: %clang_cc1 -triple i686-linux-gnu   -fsyntax-only 
> -Wimplicit-widening-of-pointer-offset -verify=silent %s`
> 
> > (ultimately, might be worth committing the two diagnostics separately - 
> > usual sort of reasons, separation of concerns, etc)
> 
> Should i split this into two reviews to begin with?
> Should i split this into two reviews to begin with?
Nah, doubt it's worth splitting up right now - see how a few folks feel about 
the idea in general, and then if there's enough mechanical details to discuss 
might be worth splitting out one on top of the other.



Comment at: 
clang/test/Sema/implicit-widening-of-pointer-offset-in-array-subscript-expression.c:24
+void t1(char *base, int a, int b) {
+  // FIXME: test `[a * b]base` pattern?
+}

lebedev.ri wrote:
> dblaikie wrote:
> > Question is unclear - is there uncertainty about whether this should be 
> > tested? Or is this a false negative case? In which case I'd probably 
> > include the test showing no diagnostic and mention it could be 
> > fixed/improved?
> I may be misremembering things, but IIRC `a[b]` and `b[a]` is the same thing,
> but i'm not sure how to exercise the second spelling.
> I.e. i'm just not sure how to write a test for it.
> I may be misremembering things, but IIRC a[b] and b[a] is the same thing,
Yep, that's the case - `a[b]` where one of them is a pointer and teh other is 
an integer, is equivalent to `*(a + b)`, so that means it's the same as `b[a]`.

I guess you'd write it the same as t0, but with the expressions reversed? 
`return *(a * b)[base];`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/unique-internal-linkage-names.cpp:48-49
 
+// LPIPELINE: Unique Internal Linkage Names
+// NPIPELINE: Running pass: UniqueInternalLinkageNamesPass
 // PLAIN: @_ZL4glob = internal global

hoy wrote:
> dblaikie wrote:
> > hoy wrote:
> > > dblaikie wrote:
> > > > aeubanks wrote:
> > > > > dblaikie wrote:
> > > > > > hoy wrote:
> > > > > > > aeubanks wrote:
> > > > > > > > hoy wrote:
> > > > > > > > > aeubanks wrote:
> > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > hoy wrote:
> > > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > > Does this test validate the new behavior? (ie: does 
> > > > > > > > > > > > > this test fail without the LLVM changes and pass with 
> > > > > > > > > > > > > it) Not that it necessarily has to - since Clang 
> > > > > > > > > > > > > isn't here to test the LLVM behavior - perhaps this 
> > > > > > > > > > > > > test is sufficient in Clang to test that the code in 
> > > > > > > > > > > > > BackendUtil works to enable this pass.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > This could possibly be staged as independent commits 
> > > > > > > > > > > > > - adding the LLVM functionality in one commit, which 
> > > > > > > > > > > > > would be a no-op for Clang because it wouldn't be 
> > > > > > > > > > > > > setting PTO.UniqueLinkageNames - then committing the 
> > > > > > > > > > > > > Clang change that would remove the custom pass 
> > > > > > > > > > > > > addition and set PTO.UniqueLinkageNames - and then 
> > > > > > > > > > > > > it'd probably be reasonable to have this test be made 
> > > > > > > > > > > > > a bit more explicit (testing the pass manager 
> > > > > > > > > > > > > structure/order) to show that that Clang change had 
> > > > > > > > > > > > > an effect: Moving the pass to the desired location in 
> > > > > > > > > > > > > the pass pipeline.
> > > > > > > > > > > > This is a good question. No, this test does not 
> > > > > > > > > > > > validate the pipeline change on the LLVM side, since 
> > > > > > > > > > > > Clang shouldn't have knowledge about how the pipelines 
> > > > > > > > > > > > are arranged in LLVM. As you pointed out, the test here 
> > > > > > > > > > > > is to test if the specific pass is run and gives 
> > > > > > > > > > > > expected results.
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks for the suggestion to break the Clang changes 
> > > > > > > > > > > > and LLVM changes apart which would make the testing 
> > > > > > > > > > > > more specific. The pipeline ordering could be tested 
> > > > > > > > > > > > with a LLVM test but that would require a LLVM switch 
> > > > > > > > > > > > setup for UniqueLinkageNames and I'm not sure there's a 
> > > > > > > > > > > > need for that switch except for testing.
> > > > > > > > > > > > No, this test does not validate the pipeline change on 
> > > > > > > > > > > > the LLVM side, since Clang shouldn't have knowledge 
> > > > > > > > > > > > about how the pipelines are arranged in LLVM. 
> > > > > > > > > > > 
> > > > > > > > > > > "ish" - but Clang should have tests for changes to Clang, 
> > > > > > > > > > > ideally. Usually they can simply be testing LLVM's IR 
> > > > > > > > > > > output before it goes to LLVM for optimization/codegen - 
> > > > > > > > > > > but for features that don't have this serialization 
> > > > > > > > > > > boundary that makes testing and isolation clear/simple, 
> > > > > > > > > > > it becomes a bit fuzzier.
> > > > > > > > > > > 
> > > > > > > > > > > In this case, there is a clang change - from adding the 
> > > > > > > > > > > pass explicitly in Clang, to setting a parameter about 
> > > > > > > > > > > how LLVM will add the pass, and it has an observable 
> > > > > > > > > > > effect. One way to test this change while isolating the 
> > > > > > > > > > > Clang test from further changes to the pipeline in LLVM, 
> > > > > > > > > > > would be to test that the pass ends up somewhere in the 
> > > > > > > > > > > LLVM-created part of the pass pipeline - the parts that 
> > > > > > > > > > > you can't get to from the way the original pass addition 
> > > > > > > > > > > was written in Clang. At least I assume that's the 
> > > > > > > > > > > case/what motivated the change from adding it in Clang to 
> > > > > > > > > > > adding it in LLVM?
> > > > > > > > > > > 
> > > > > > > > > > > eg: if LLVM always forms passes {x, y, z} and Clang is 
> > > > > > > > > > > able to add passes before/after, say it always adds 'a' 
> > > > > > > > > > > before and 'b' after, to make {a, x, y, z, b} - and this 
> > > > > > > > > > > new pass u was previously added at the start to make {u, 
> > > > > > > > > > > a, x, y, z, b} but now needs to go in {a, x, y, u, z, b} 
> > > > > > > > > > > you could test that 'u' is after 'a' and before 'b', or 
> > > > > > > > > > > between 'x' and 'z', etc. If there's some other more 
> > > > > > > > > > > clear/simple

[PATCH] D93919: [PowerPC] powerpcle target 3/5

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:260
+  case llvm::Triple::ppcle:
+return "elf32lppc";
   case llvm::Triple::ppc64:

This needs a linker test in clang/test/Driver/ppc-features.cpp



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3297
   !(TT.getArch() == llvm::Triple::aarch64 ||
-TT.getArch() == llvm::Triple::ppc ||
-TT.getArch() == llvm::Triple::ppc64 ||
-TT.getArch() == llvm::Triple::ppc64le ||
+TT.isPPC() ||
 TT.getArch() == llvm::Triple::nvptx ||

Please format



Comment at: clang/test/Driver/ppc-endian.c:1
-// RUN: %clang -target powerpc64le -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64le -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64 -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// CHECK-LE: "-cc1"{{.*}} "-triple" "powerpc64le{{.*}}"
+// RUN: %clang -target powerpc-unknown -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpc-unknown -mbig-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-BE32 %s

"-unknown" is correct, though I usually omit it as it is not useful...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93919

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: llvm/tools/opt/NewPMDriver.cpp:136-138
+static cl::opt PseudoProbeForProfiling(
+"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden,
+cl::desc("Emit pseudo probes to enable PGO profile generation."));

dblaikie wrote:
> I guess this should probably have some separate testing, if it's a separate 
> flag/feature? (& flag+tests could be in a separate commit)
I'm not sure there's a separate need for this switch except for being tested in 
`unique-internal-linkage-names.ll`. The point of this whole patch is to place 
the unique name pass before the pseudo probe pass and test it works. Hence it 
sounds appropriate to me to include all changes in one patch. What do you think?



Comment at: llvm/tools/opt/NewPMDriver.cpp:253-258
 if (DebugInfoForProfiling)
   P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction,
  true);
+else if (PseudoProbeForProfiling)
+  P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction,
+ false, true);

dblaikie wrote:
> Both of these might be more readably written as something like:
> ```
> P.emplace();
> P.PseudoProbeForProfiling = true;
> ```
> & similar. (you can commit the change to DebugInfoForProfiling separately 
> before/after this change to make it consistent with the new one)
> 
> But no big deal either way - while it makes these tidier it makes them a bit 
> less consistent with the other three
That looks cleaner, but there are assertions in the constructor of `PGOOptions` 
which I would not like to bypass by setting the `PseudoProbeForProfiling` field 
separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

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

[PATCH] D93656: Moving UniqueInternalLinkageNamesPass to the start of IR pipelines.

[Hongtao Yu via Phabricator via cfe-commits]

hoy updated this revision to Diff 314043.
hoy added a comment.

Removing the checks of VerifierAnalysis in test code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93656

Files:
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Transforms/UniqueLinkageNames/unique-internal-linkage-names.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -133,6 +133,13 @@
 static cl::opt DebugInfoForProfiling(
 "new-pm-debug-info-for-profiling", cl::init(false), cl::Hidden,
 cl::desc("Emit special debug info to enable PGO profile generation."));
+static cl::opt PseudoProbeForProfiling(
+"new-pm-pseudo-probe-for-profiling", cl::init(false), cl::Hidden,
+cl::desc("Emit pseudo probes to enable PGO profile generation."));
+static cl::opt UniqueInternalLinkageNames(
+"new-pm-unique-internal-linkage-names", cl::init(false), cl::Hidden,
+cl::desc("Uniqueify Internal Linkage Symbol Names by appending the MD5 "
+ "hash of the module path."));
 /// @}}
 
 template 
@@ -246,6 +253,9 @@
 if (DebugInfoForProfiling)
   P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction,
  true);
+else if (PseudoProbeForProfiling)
+  P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction,
+ false, true);
 else
   P = None;
   }
@@ -281,6 +291,7 @@
   // option has been enabled.
   PTO.LoopUnrolling = !DisableLoopUnrolling;
   PTO.Coroutines = Coroutines;
+  PTO.UniqueLinkageNames = UniqueInternalLinkageNames;
   PassBuilder PB(DebugPM, TM, PTO, P, &PIC);
   registerEPCallbacks(PB);
 
Index: llvm/test/Transforms/UniqueLinkageNames/unique-internal-linkage-names.ll
===
--- /dev/null
+++ llvm/test/Transforms/UniqueLinkageNames/unique-internal-linkage-names.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S -passes='default' -new-pm-pseudo-probe-for-profiling -new-pm-unique-internal-linkage-names -debug-pass-manager < %s 2>&1 | FileCheck %s --check-prefix=O0 --check-prefix=UNIQUE
+; RUN: opt -S -passes='default' -new-pm-pseudo-probe-for-profiling -new-pm-unique-internal-linkage-names -debug-pass-manager < %s 2>&1 | FileCheck %s --check-prefix=O2 --check-prefix=UNIQUE
+; RUN: opt -S -passes='default' -new-pm-pseudo-probe-for-profiling -new-pm-unique-internal-linkage-names -debug-pass-manager < %s 2>&1 | FileCheck %s --check-prefix=O2 --check-prefix=UNIQUE
+; RUN: opt -S -passes='thinlto-pre-link' -new-pm-pseudo-probe-for-profiling -new-pm-unique-internal-linkage-names -debug-pass-manager < %s 2>&1 | FileCheck %s --check-prefix=O2 --check-prefix=UNIQUE
+; RUN: opt -S -passes='thinlto-pre-link' -new-pm-pseudo-probe-for-profiling -new-pm-unique-internal-linkage-names -debug-pass-manager < %s 2>&1 | FileCheck %s --check-prefix=O2 --check-prefix=UNIQUE
+
+define internal i32 @foo() {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 (...)* @bar() {
+entry:
+  ret i32 (...)* bitcast (i32 ()* @foo to i32 (...)*)
+}
+
+; O0: Running pass: UniqueInternalLinkageNamesPass
+
+; O2: Running pass: UniqueInternalLinkageNamesPass
+; O2: Running pass: SampleProfileProbePass
+
+; UNIQUE: define internal i32 @foo.__uniq.{{[0-9a-f]+}}()
+; UNIQUE: ret {{.*}} @foo.__uniq.{{[0-9a-f]+}} {{.*}}
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -284,6 +284,7 @@
   LicmMssaNoAccForPromotionCap = SetLicmMssaNoAccForPromotionCap;
   CallGraphProfile = true;
   MergeFunctions = false;
+  UniqueLinkageNames = false;
 }
 
 extern cl::opt EnableConstraintElimination;
@@ -1001,6 +1002,11 @@
ThinLTOPhase Phase) {
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (PTO.UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   // Place pseudo probe instrumentation as the first pass of the pipeline to
   // minimize the impact of optimization changes.
   if (PGOOpt && PGOOpt->PseudoProbeForProfiling &&
@@ -1764,6 +1770,11 @@
 
   ModulePassManager MPM(DebugLogging);
 
+  // Add UniqueInternalLinkageNames Pass which renames internal linkage
+  // symbols with unique names.
+  if (PTO.UniqueLinkageNames)
+MPM.addPass(UniqueInternalLinkageNamesPass());
+
   if (PGOOpt && (PGOOpt->Action == PGOOptions::IRInstr ||
  PGOOpt->Action == PGOOptions::IRUse))
 addPGOInstrPassesForO0(
Index: llvm/include/llvm/Passes/PassBuilder.h
===
--- llvm/include/llvm/Passes/

[PATCH] D93919: [PowerPC] Support powerpcle target in Clang [3/5]

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 updated this revision to Diff 314049.
Bdragon28 added a comment.

- Adjust the GNU LD settings for 32 bit powerpc to more closely match reality.
- Add linker emulation test for powerpcle, as well as adding testing for the 
FreeBSD emulations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93919

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/altivec.c
  clang/test/CodeGen/builtins-ppc-altivec.c
  clang/test/CodeGen/ppc32-and-aix-struct-return.c
  clang/test/CodeGen/target-data.c
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/ppc-endian.c
  clang/test/Driver/ppc-features.cpp

Index: clang/test/Driver/ppc-features.cpp
===
--- clang/test/Driver/ppc-features.cpp
+++ clang/test/Driver/ppc-features.cpp
@@ -1,9 +1,15 @@
 /// Check default CC1 and linker options for ppc32.
-// RUN: %clang -### -target powerpc-unknown-linux-gnu %s 2>&1 | FileCheck --check-prefix=PPC32 %s
+// RUN: %clang -### -target powerpcle-unknown-linux-gnu %s 2>&1 | FileCheck --check-prefixes=PPC32,PPC32LELNX %s
+// RUN: %clang -### -target powerpc-unknown-linux-gnu %s 2>&1 | FileCheck --check-prefixes=PPC32,PPC32BELNX %s
+// RUN: %clang -### -target powerpcle-unknown-freebsd13.0 %s 2>&1 | FileCheck --check-prefixes=PPC32,PPC32LEFBSD %s
+// RUN: %clang -### -target powerpc-unknown-freebsd13.0 %s 2>&1 | FileCheck --check-prefixes=PPC32,PPC32BEFBSD %s
 // PPC32:  "-munwind-tables"
 // PPC32-SAME: "-mfloat-abi" "hard"
 
-// PPC32: "-m" "elf32ppclinux"
+// PPC32LELNX-NEXT: "-m" "elf32lppclinux"
+// PPC32BELNX-NEXT: "-m" "elf32ppclinux"
+// PPC32LEFBSD-NEXT: "-m" "elf32lppc"
+// PPC32BEFBSD-NEXT: "-m" "elf32ppc_fbsd"
 
 // check -msoft-float option for ppc32
 // RUN: %clang -target powerpc-unknown-linux-gnu %s -msoft-float -### -o %t.o 2>&1 | FileCheck --check-prefix=CHECK-SOFTFLOAT %s
Index: clang/test/Driver/ppc-endian.c
===
--- clang/test/Driver/ppc-endian.c
+++ clang/test/Driver/ppc-endian.c
@@ -1,9 +1,19 @@
-// RUN: %clang -target powerpc64le -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64le -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE %s
-// CHECK-LE: "-cc1"{{.*}} "-triple" "powerpc64le{{.*}}"
+// RUN: %clang -target powerpc-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpc-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpcle-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE32 %s
+// CHECK-BE32: "-cc1"{{.*}} "-triple" "powerpc-{{.*}}"
 
-// RUN: %clang -target powerpc64 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// RUN: %clang -target powerpc64 -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// RUN: %clang -target powerpc64le -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE %s
-// CHECK-BE: "-cc1"{{.*}} "-triple" "powerpc64{{.*}}"
+// RUN: %clang -target powerpcle-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// RUN: %clang -target powerpcle-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// RUN: %clang -target powerpc-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE32 %s
+// CHECK-LE32: "-cc1"{{.*}} "-triple" "powerpcle-{{.*}}"
+
+// RUN: %clang -target powerpc64-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// RUN: %clang -target powerpc64-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// RUN: %clang -target powerpc64le-unknown -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE64 %s
+// CHECK-BE64: "-cc1"{{.*}} "-triple" "powerpc64-{{.*}}"
+
+// RUN: %clang -target powerpc64le-unknown -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// RUN: %clang -target powerpc64le-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// RUN: %clang -target powerpc64-unknown -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-LE64 %s
+// CHECK-LE64: "-cc1"{{.*}} "-triple" "powerpc64le-{{.*}}"
Index: clang/test/Driver/linux-header-search.cpp
===

[PATCH] D93919: [PowerPC] Support powerpcle target in Clang [3/5]

[Brandon Bergren via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/ppc-endian.c:1
-// RUN: %clang -target powerpc64le -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64le -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64 -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// CHECK-LE: "-cc1"{{.*}} "-triple" "powerpc64le{{.*}}"
+// RUN: %clang -target powerpc-unknown -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpc-unknown -mbig-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-BE32 %s

MaskRay wrote:
> "-unknown" is correct, though I usually omit it as it is not useful...
This is actually intentional!

I needed to force it to emit the hyphen so I could tell powerpc apart from 
powerpcle. The same for powerpc64, the BE test was actually not working 
properly as is because it would also match on LE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93919

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

[PATCH] D93919: [PowerPC] Support powerpcle target in Clang [3/5]

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/ppc-endian.c:1
-// RUN: %clang -target powerpc64le -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64le -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// RUN: %clang -target powerpc64 -mlittle-endian -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-LE %s
-// CHECK-LE: "-cc1"{{.*}} "-triple" "powerpc64le{{.*}}"
+// RUN: %clang -target powerpc-unknown -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-BE32 %s
+// RUN: %clang -target powerpc-unknown -mbig-endian -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-BE32 %s

Bdragon28 wrote:
> MaskRay wrote:
> > "-unknown" is correct, though I usually omit it as it is not useful...
> This is actually intentional!
> 
> I needed to force it to emit the hyphen so I could tell powerpc apart from 
> powerpcle. The same for powerpc64, the BE test was actually not working 
> properly as is because it would also match on LE.
You can write `"-triple" "powerpcle"` ..

I think the driver side works even without TargetInfo change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93919

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

[clang] 981a0bd - [X86] Add x86_amx type for intel AMX.

[via cfe-commits]

Author: Luo, Yuanke
Date: 2020-12-30T13:52:13+08:00
New Revision: 981a0bd85811fe49379fdbef35528e2c2f3511a3

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

LOG: [X86] Add x86_amx type for intel AMX.

The x86_amx is used for AMX intrisics. <256 x i32> is bitcast to x86_amx when
it is used by AMX intrinsics, and x86_amx is bitcast to <256 x i32> when it
is used by load/store instruction. So amx intrinsics only operate on type 
x86_amx.
It can help to separate amx intrinsics from llvm IR instructions (+-*/).
Thank Craig for the idea. This patch depend on https://reviews.llvm.org/D87981.

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

Added: 


Modified: 
clang/test/CodeGen/X86/amx_api.c
llvm/include/llvm-c/Core.h
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/CodeGen/ValueTypes.td
llvm/include/llvm/IR/DataLayout.h
llvm/include/llvm/IR/Intrinsics.h
llvm/include/llvm/IR/Intrinsics.td
llvm/include/llvm/IR/IntrinsicsX86.td
llvm/include/llvm/IR/Type.h
llvm/include/llvm/Support/MachineValueType.h
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/AsmParser/LLLexer.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/ValueTypes.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/lib/IR/ConstantFold.cpp
llvm/lib/IR/Core.cpp
llvm/lib/IR/DataLayout.cpp
llvm/lib/IR/Function.cpp
llvm/lib/IR/LLVMContextImpl.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/lib/IR/Type.cpp
llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86LowerAMXType.cpp
llvm/lib/Target/X86/X86RegisterInfo.td
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/test/CodeGen/X86/AMX/amx-across-func.ll
llvm/test/CodeGen/X86/AMX/amx-config.ll
llvm/test/CodeGen/X86/AMX/amx-intrinsic-chain.ll
llvm/test/CodeGen/X86/AMX/amx-spill.ll
llvm/test/CodeGen/X86/AMX/amx-type.ll
llvm/utils/TableGen/CodeGenTarget.cpp
llvm/utils/TableGen/IntrinsicEmitter.cpp

Removed: 




diff  --git a/clang/test/CodeGen/X86/amx_api.c 
b/clang/test/CodeGen/X86/amx_api.c
index c7fab8c6ed41..52eb9542228d 100644
--- a/clang/test/CodeGen/X86/amx_api.c
+++ b/clang/test/CodeGen/X86/amx_api.c
@@ -11,8 +11,8 @@ char buf2[1024];
 // This is an example code and integration test.
 void test_api(int cond, short row, short col) {
   //CHECK-LABEL: @test_api
-  //CHECK: call <256 x i32> @llvm.x86.tileloadd64.internal
-  //CHECK: call <256 x i32> @llvm.x86.tdpbssd.internal
+  //CHECK: call x86_amx @llvm.x86.tileloadd64.internal
+  //CHECK: call x86_amx @llvm.x86.tdpbssd.internal
   //CHECK: call void @llvm.x86.tilestored64.internal
   __tile1024i a = {row, 8};
   __tile1024i b = {8, col};
@@ -33,19 +33,22 @@ void test_api(int cond, short row, short col) {
 
 void test_tile_loadd(short row, short col) {
   //CHECK-LABEL: @test_tile_loadd
-  //CHECK: call <256 x i32> @llvm.x86.tileloadd64.internal
+  //CHECK: call x86_amx @llvm.x86.tileloadd64.internal
+  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
   __tile1024i a = {row, col};
   __tile_loadd(&a, buf, STRIDE);
 }
 
 void test_tile_dpbsud(__tile1024i a, __tile1024i b, __tile1024i c) {
   //CHECK-LABEL: @test_tile_dpbsud
-  //CHECK: call <256 x i32> @llvm.x86.tdpbssd.internal
+  //CHECK: call x86_amx @llvm.x86.tdpbssd.internal
+  //CHECK-NEXT: {{%.*}} = bitcast x86_amx {{%.*}} to <256 x i32>
   __tile_dpbsud(&c, a, b);
 }
 
 void test_tile_stored(__tile1024i c) {
   //CHECK-LABEL: @test_tile_stored
-  //CHECK: call void @llvm.x86.tilestored64.internal
+  //CHECK: {{%.*}} = bitcast <256 x i32> {{%.*}} to x86_amx
+  //CHECK-NEXT: call void @llvm.x86.tilestored64.internal
   __tile_stored(buf, STRIDE, c);
 }

diff  --git a/llvm/include/llvm-c/Core.h b/llvm/include/llvm-c/Core.h
index 86de259ea53e..8274213aa839 100644
--- a/llvm/include/llvm-c/Core.h
+++ b/llvm/include/llvm-c/Core.h
@@ -160,6 +160,7 @@ typedef enum {
   LLVMVectorTypeKind,/**< Fixed width SIMD vector type */
   LLVMMetadataTypeKind,  /**< Metadata */
   LLVMX86_MMXTypeKind,   /**< X86 MMX */
+  LLVMX86_AMXTypeKind,   /**< X86 AMX */
   LLVMTokenTypeKind, /**< Tokens */
   LLVMScalableVectorTypeKind, /**< Scalable SIMD vector type */
   LLVMBFloatTypeKind /**< 16 bit brain floating point type */
@@ -1493,6 +1494,11 @@ LLVMTypeRef LLVMLabelTypeInContext(LLVMContextRef C);
  */
 LLVMTypeRef LLVMX86MMXTypeInContext(LLVMContextRef C);
 
+/**
+ * Create a X86 AMX type in a context.
+ */
+LLVMTypeRef LLVMX86AMXTypeInContext(LLVMContextRef C);
+
 /**
  * Create a token type in a context.
  */
@@ -1510,6 +1516,7 @@ LLVMTypeRef LLVMMetadataTypeInContext(LLVMContextRef C);
 LLVMTypeRef LLVMVoidType(void

[PATCH] D91927: [X86] Add x86_amx type for intel AMX.

[LuoYuanke 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 rG981a0bd85811: [X86] Add x86_amx type for intel AMX. 
(authored by LuoYuanke).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91927

Files:
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/CodeGen/ValueTypes.td
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/Intrinsics.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/IR/Type.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantFold.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/LLVMContextImpl.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-intrinsic-chain.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -248,7 +248,8 @@
   IIT_V128 = 47,
   IIT_BF16 = 48,
   IIT_STRUCT9 = 49,
-  IIT_V256 = 50
+  IIT_V256 = 50,
+  IIT_AMX  = 51
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -276,6 +277,7 @@
   case MVT::token: return Sig.push_back(IIT_TOKEN);
   case MVT::Metadata: return Sig.push_back(IIT_METADATA);
   case MVT::x86mmx: return Sig.push_back(IIT_MMX);
+  case MVT::x86amx: return Sig.push_back(IIT_AMX);
   // MVT::OtherVT is used to mean the empty struct type here.
   case MVT::Other: return Sig.push_back(IIT_EMPTYSTRUCT);
   // MVT::isVoid is used to represent varargs here.
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -76,6 +76,7 @@
   case MVT::f128: return "MVT::f128";
   case MVT::ppcf128:  return "MVT::ppcf128";
   case MVT::x86mmx:   return "MVT::x86mmx";
+  case MVT::x86amx:   return "MVT::x86amx";
   case MVT::Glue: return "MVT::Glue";
   case MVT::isVoid:   return "MVT::isVoid";
   case MVT::v1i1: return "MVT::v1i1";
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-type.ll
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -8,18 +8,104 @@
 @buf = dso_local global [1024 x i8] zeroinitializer, align 16
 @buf2 = dso_local global [1024 x i8] zeroinitializer, align 16
 
+; test bitcast x86_amx to <256 x i32>
+define dso_local void @test_user_empty(i16 %m, i16 %n, i8 *%buf, i64 %s) #2 {
+; CHECK-LABEL: @test_user_empty(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[T1:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[M:%.*]], i16 [[N:%.*]], i8* [[BUF:%.*]], i64 [[S:%.*]]) [[ATTR3:#.*]]
+; CHECK-NEXT:ret void
+;
+entry:
+  %t1 = call x86_amx @llvm.x86.tileloadd64.internal(i16 %m, i16 %n, i8* %buf, i64 %s) #3
+  %t2 = bitcast x86_amx %t1 to <256 x i32>
+  ret void
+}
+
+; test bitcast <256 x i32> to x86_amx
+define dso_local void @test_user_empty2(<256 x i32> %in) #2 {
+; CHECK-LABEL: @test_user_empty2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:ret void
+;
+entry:
+  %t = bitcast <256 x i32> %in to x86_amx
+  ret void
+}
+
+define dso_local <256 x i32> @test_amx_load_bitcast(<256 x i32>* %in, i16 %m, i16 %n, i8 *%buf, i64 %s) #2 {
+; CHECK-LABEL: @test_amx_load_bitcast(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[T1:%.*]] = load <256 x i32>, <256 x i32>* [[IN:%.*]], align 64
+; CHECK-NEXT:[[TMP0:%.*]] = bitcast <256 x i32>* [[IN]] to i8*
+; CHECK-NEXT:[[TMP1:%.*]] = call x86_amx @llvm.x86.tileloadd64.internal(i16 [[M:%.*]], i16 [[N:%.*]], i8* [[TMP0]], i64 64)
+; CHECK-NEXT:call void @llvm.x86.tilestored64.internal(i16 [[M]], i16 [[N]], i8* [[BUF:%.*]], i64 [[S:%.*]], x86_amx [[TMP1]]) [[ATTR3]]
+; CHECK-NEXT:ret <256 x i32> [[T1]]
+;
+entry:
+  %t1 = load <256 x i32>, <256 x i32>* %in, align 64
+  %t2 = bitcast <256 x i32> %t1 to x86_amx
+  call void @llvm.x86.tilestored64.internal(i16 %m, i16 %n, i8* %buf, i64 %s, x86_amx %t2) #

[clang] 2820a2c - Move -fno-semantic-interposition dso_local logic from TargetMachine to Clang CodeGenModule

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-12-29T23:37:55-08:00
New Revision: 2820a2ca3a0e69c3f301845420e00672251b

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

LOG: Move -fno-semantic-interposition dso_local logic from TargetMachine to 
Clang CodeGenModule

This simplifies TargetMachine::shouldAssumeDSOLocal and and gives frontend the
decision to use dso_local. For LLVM synthesized functions/globals, they may lose
inferred dso_local but such optimizations are probably not very useful.

Note: the hasComdat() condition in canBenefitFromLocalAlias (D77429) may be 
dead now.
(llvm/CodeGen/X86/semantic-interposition-comdat.ll)
(Investigate whether we need test coverage when Fuchsia C++ ABI is clearer)

Added: 
clang/test/CodeGen/semantic-interposition-no.c

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
llvm/lib/Target/TargetMachine.cpp

Removed: 
llvm/test/CodeGen/X86/semantic-interposition-comdat.ll
llvm/test/CodeGen/X86/semantic-interposition-infer-dsolocal.ll



diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 93916e85a461..6d14298d9f5f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -954,8 +954,17 @@ static bool shouldAssumeDSOLocal(const CodeGenModule &CGM,
   const auto &CGOpts = CGM.getCodeGenOpts();
   llvm::Reloc::Model RM = CGOpts.RelocationModel;
   const auto &LOpts = CGM.getLangOpts();
-  if (RM != llvm::Reloc::Static && !LOpts.PIE)
-return false;
+  if (RM != llvm::Reloc::Static && !LOpts.PIE) {
+// On ELF, if -fno-semantic-interposition is specified, we can set 
dso_local
+// if using a local alias is preferable (can avoid GOT indirection).
+// Currently only x86 supports local alias.
+if (!TT.isOSBinFormatELF() ||
+!CGM.getLangOpts().ExplicitNoSemanticInterposition ||
+!GV->canBenefitFromLocalAlias())
+  return false;
+// The allowed targets need to match AsmPrinter::getSymbolPreferLocal.
+return TT.isX86();
+  }
 
   // A definition cannot be preempted from an executable.
   if (!GV->isDeclarationForLinker())

diff  --git a/clang/test/CodeGen/semantic-interposition-no.c 
b/clang/test/CodeGen/semantic-interposition-no.c
new file mode 100644
index ..cc53d1799f9d
--- /dev/null
+++ b/clang/test/CodeGen/semantic-interposition-no.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -mrelocation-model pic 
-pic-level 1 -fno-semantic-interposition %s -o - | FileCheck %s
+
+/// For ELF -fpic/-fPIC, if -fno-semantic-interposition is specified, mark
+/// defined variables and functions dso_local. ifunc isn't marked.
+
+// CHECK: @var = dso_local global i32 0, align 4
+// CHECK: @ext_var = external global i32, align 4
+int var;
+extern int ext_var;
+
+// CHECK: @ifunc = ifunc i32 (), bitcast (i8* ()* @ifunc_resolver to i32 ()*)
+int ifunc(void) __attribute__((ifunc("ifunc_resolver")));
+
+// CHECK: define dso_local i32 @func()
+// CHECK: declare i32 @ext()
+int func(void) { return 0; }
+int ext(void);
+
+static void *ifunc_resolver() { return func; }
+
+int foo() {
+  return var + ext_var + ifunc() + func() + ext();
+}

diff  --git a/llvm/lib/Target/TargetMachine.cpp 
b/llvm/lib/Target/TargetMachine.cpp
index ad0e90125258..b45b8e354b3a 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -163,14 +163,6 @@ bool TargetMachine::shouldAssumeDSOLocal(const Module &M,
 // If the symbol is defined, it cannot be preempted.
 if (!GV->isDeclarationForLinker())
   return true;
-  } else if (TT.isOSBinFormatELF()) {
-// If dso_local allows AsmPrinter::getSymbolPreferLocal to use a local
-// alias, set the flag. We cannot set dso_local for other global values,
-// because otherwise direct accesses to a probably interposable symbol 
(even
-// if the codegen assumes not) will be rejected by the linker.
-if (!GV->canBenefitFromLocalAlias())
-  return false;
-return TT.isX86() && M.noSemanticInterposition();
   }
 
   // ELF & wasm support preemption of other symbols.

diff  --git a/llvm/test/CodeGen/X86/semantic-interposition-comdat.ll 
b/llvm/test/CodeGen/X86/semantic-interposition-comdat.ll
deleted file mode 100644
index d11be2d6bd0c..
--- a/llvm/test/CodeGen/X86/semantic-interposition-comdat.ll
+++ /dev/null
@@ -1,28 +0,0 @@
-; RUN: llc -mtriple=x86_64 -relocation-model=pic < %s | FileCheck %s
-
-$comdat_func = comdat any
-
-; CHECK-LABEL: func2:
-; CHECK-NOT: .Lfunc2$local
-
-declare void @func()
-
-define hidden void @func2() {
-entry:
-  call void @func()
-  ret void
-}
-
-; CHECK: comdat_func:
-; CHECK-NOT: .Lcomdat_func$local
-
-define hidden void @comdat_func() comdat {
-entry:
-  call void @func()
-  ret void
-}
-
-!ll