date:20230307

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

2023-03-07 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

In D144285#4176850 , @rupprecht wrote:

> In D144285#4163004 , @cor3ntin 
> wrote:
>
>> If however we find this change to disruptive, we should inform WG21.
>
> Thanks for the explanation, I think I understand the issue now. I got totally 
> thrown off by the title -- it's not about literally writing 
> `static_assert(false)`, it's about deferring `static_assert` evaluation to 
> template instantiations. Being able to write `static_assert(false)` (or any 
> falsy constant expression) is just the common use case for this.
>
> So possibly the most trivial example of something that used to break, but now 
> builds:
>
>   template 
>   void Fail() {
>   static_assert(false, "my assertion failed");
>   }
>
> ... but will still fail as soon as you invoke `Fail()` somewhere.
>
> It doesn't look like there's a lot of impact from this, and the breakages are 
> corner cases like this. It might be worth mentioning this one case to WG21 
> but I'm not sure what I would change about the wording.

Will do !




Comment at: clang/www/cxx_dr_status.html:14915
   
 https://wg21.link/cwg2518;>2518
 review

rupprecht wrote:
> Is it possible to make this link point to 
> https://cplusplus.github.io/CWG/issues/2518.html? This link is inaccessible 
> to anyone not on ISO.
It will get updated whenever a new core issue list gets published, which will 
hopefully be soon-ish.
This file is generated automatically so changing the link might actually not be 
trivial


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144285

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

[PATCH] D145238: [NVPTX] Expose LDU builtins

2023-03-07 Thread Jakub Chlanda via Phabricator via cfe-commits

jchlanda added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18267-18271
+auto HalfSupport = HasHalfSupport(BuiltinID);
+if (!HalfSupport.first) {
+  CGM.Error(E->getExprLoc(),
+HalfSupport.second.append(" requires native half type 
support.")
+.c_str());

tra wrote:
> I think we can simplify it all further.
> 
> ```
> auto HasHalfSupport = [&](unsigned BuiltinID) {
> auto  = getContext();
> return Context.getLangOpts().NativeHalfType || 
> !Context.getTargetInfo().useFP16ConversionIntrinsics();
> }
> ...
> 
> if (!HasHalfSupport(BuiltinID)) {
>   CGM.Error(E->getExprLoc(),   
> getContext().BuiltinInfo.getName(BuiltinID) + " requires native half type 
> support.");
> 
> ```
Done, although we need a string append there, StringRef and Twine would not 
work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

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

[PATCH] D145238: [NVPTX] Expose LDU builtins

2023-03-07 Thread Jakub Chlanda via Phabricator via cfe-commits

jchlanda updated this revision to Diff 503242.
jchlanda added a comment.

Simplify the check for half tys support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-native-half-type-err.c
  clang/test/CodeGen/builtins-nvptx-native-half-type.c
  clang/test/CodeGen/builtins-nvptx.c
  llvm/test/CodeGen/NVPTX/ldu-ldg.ll

Index: llvm/test/CodeGen/NVPTX/ldu-ldg.ll
===
--- llvm/test/CodeGen/NVPTX/ldu-ldg.ll
+++ llvm/test/CodeGen/NVPTX/ldu-ldg.ll
@@ -3,7 +3,13 @@
 
 
 declare i8 @llvm.nvvm.ldu.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i32 @llvm.nvvm.ldu.global.i.i32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 %align)
+declare double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 %align)
+declare half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 %align)
+declare <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 %align)
 
 declare i8 @llvm.nvvm.ldg.global.i.i8.p1(ptr addrspace(1) %ptr, i32 %align)
 declare i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 %align)
@@ -16,70 +22,112 @@
 
 ; CHECK: test_ldu_i8
 define i8 @test_ldu_i8(ptr addrspace(1) %ptr) {
-; ldu.global.u8
+  ; CHECK: ldu.global.u8
   %val = tail call i8 @llvm.nvvm.ldu.global.i.i8.p1(ptr addrspace(1) %ptr, i32 4)
   ret i8 %val
 }
 
+; CHECK: test_ldu_i16
+define i16 @test_ldu_i16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.u16
+  %val = tail call i16 @llvm.nvvm.ldu.global.i.i16.p1(ptr addrspace(1) %ptr, i32 2)
+  ret i16 %val
+}
+
 ; CHECK: test_ldu_i32
 define i32 @test_ldu_i32(ptr addrspace(1) %ptr) {
-; ldu.global.u32
+  ; CHECK: ldu.global.u32
   %val = tail call i32 @llvm.nvvm.ldu.global.i.i32.p1(ptr addrspace(1) %ptr, i32 4)
   ret i32 %val
 }
 
+; CHECK: test_ldu_i64
+define i64 @test_ldu_i64(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.u64
+  %val = tail call i64 @llvm.nvvm.ldu.global.i.i64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret i64 %val
+}
+
+; CHECK: test_ldu_f32
+define float @test_ldu_f32(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.f32
+  %val = tail call float @llvm.nvvm.ldu.global.f.f32.p1(ptr addrspace(1) %ptr, i32 4)
+  ret float %val
+}
+
+; CHECK: test_ldu_f64
+define double @test_ldu_f64(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.f64
+  %val = tail call double @llvm.nvvm.ldu.global.f.f64.p1(ptr addrspace(1) %ptr, i32 8)
+  ret double %val
+}
+
+; CHECK: test_ldu_f16
+define half @test_ldu_f16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.b16
+  %val = tail call half @llvm.nvvm.ldu.global.f.f16.p1(ptr addrspace(1) %ptr, i32 2)
+  ret half %val
+}
+
+; CHECK: test_ldu_v2f16
+define <2 x half> @test_ldu_v2f16(ptr addrspace(1) %ptr) {
+  ; CHECK: ldu.global.b32
+  %val = tail call <2 x half> @llvm.nvvm.ldu.global.f.v2f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ret <2 x half> %val
+}
+
 ; CHECK: test_ldg_i8
 define i8 @test_ldg_i8(ptr addrspace(1) %ptr) {
-; ld.global.nc.u8
+  ; CHECK: ld.global.nc.u8
   %val = tail call i8 @llvm.nvvm.ldg.global.i.i8.p1(ptr addrspace(1) %ptr, i32 4)
   ret i8 %val
 }
 
 ; CHECK: test_ldg_i16
 define i16 @test_ldg_i16(ptr addrspace(1) %ptr) {
-; ld.global.nc.u16
-  %val = tail call i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 4)
+  ; CHECK: ld.global.nc.u16
+  %val = tail call i16 @llvm.nvvm.ldg.global.i.i16.p1(ptr addrspace(1) %ptr, i32 2)
   ret i16 %val
 }
 
 ; CHECK: test_ldg_i32
 define i32 @test_ldg_i32(ptr addrspace(1) %ptr) {
-; ld.global.nc.u32
+  ; CHECK: ld.global.nc.u32
   %val = tail call i32 @llvm.nvvm.ldg.global.i.i32.p1(ptr addrspace(1) %ptr, i32 4)
   ret i32 %val
 }
 
 ; CHECK: test_ldg_i64
 define i64 @test_ldg_i64(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.u64
   %val = tail call i64 @llvm.nvvm.ldg.global.i.i64.p1(ptr addrspace(1) %ptr, i32 8)
   ret i64 %val
 }
 
 ; CHECK: test_ldg_f32
 define float @test_ldg_f32(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.f32
   %val = tail call float @llvm.nvvm.ldg.global.f.f32.p1(ptr addrspace(1) %ptr, i32 4)
   ret float %val
 }
 
 ; CHECK: test_ldg_f64
 define double @test_ldg_f64(ptr addrspace(1) %ptr) {
-; ld.global.nc.u64
+  ; CHECK: ld.global.nc.f64
   %val = tail call double @llvm.nvvm.ldg.global.f.f64.p1(ptr addrspace(1) %ptr, i32 8)
   ret double %val
 }
 
 ; CHECK: test_ldg_f16
 define half @test_ldg_f16(ptr addrspace(1) %ptr) {
-; ld.global.nc.b16
-  %val = tail call half @llvm.nvvm.ldg.global.f.f16.p1(ptr addrspace(1) %ptr, i32 4)
+  ; CHECK: ld.global.nc.b16
+  %val = tail call half

[PATCH] D141672: [RISCV] Support vector crypto extension ISA string and assembly

2023-03-07 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added a comment.

@reames and I recently talked and we agreed that vector crypto is far enough 
along that we'd like to get something committed to LLVM in experimental state. 
We can do iterative refinement if the spec continues to change. I guess the 
question is whether we should update to the most recent now or land this first.




Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:495
+   "'Zvknhb' (Vector SHA-2. (SHA-256 and SHA-512))",
+  [FeatureStdExtZvknha]>;
+def HasStdExtZvknhaOrZvknhb  : Predicate<"Subtarget->hasStdExtZvknha() || 
Subtarget->hasStdExtZvknhb()">,

Indent this to line up with `"'Zvknhb'` on the previous line



Comment at: llvm/lib/Target/RISCV/RISCVFeatures.td:524
+def HasStdExtZvksh : Predicate<"Subtarget->hasStdExtZvksh()">,
+AssemblerPredicate<(all_of FeatureStdExtZvksh),
+"'Zvksh' (SM3 Hash Function Instructions.)">;

This is indented 1 space too far.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:10
+// This file describes the RISC-V instructions from the standard 'Zvk',
+// Vector Cryptography Instructions extension, version 0.1.
+//

Is this version out of date?



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:15
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in {
+multiclass VALU_IV_V_X_VCLMUL funct6> {
+  def V  : VALUVV,

Can we name this `VCLMUL_MV_V_X`. That make it consistent with the similar 
VMUL_MV_V_X, VALU_MV_V_X, etc.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:45
+
+multiclass VALU_IV_V_X_I_VROR funct6,
+  Operand optype = uimm6, string vw = "v">

VROR_IV_V_X_I seems like a more consistent name.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:74
+
+multiclass PALUVvVs2NoVm funct6_vv, bits<6> funct6_vs, bits<5> vs1,
+ RISCVVFormat opv, string opcodestr> {

I think this should be something like `VAES_MV_V_S` based on how the other 
multiclasses are named.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td:82
+// to customize one for them.
+class PALUVI_CUSTOM funct6, string opcodestr, Operand optype>
+: VALUVINoVm {

Maybe this should be something like VAESKF_MV_I?



Comment at: llvm/test/MC/RISCV/rvv/zvkb.s:14
+# CHECK-ENCODING: [0x57,0x05,0x94,0x04]
+# CHECK-ERROR: instruction requires the following: 'Zvkb'
+# CHECK-UNKNOWN: 57 05 94 04   

Add `{{$}}` to the end of the line



Comment at: llvm/test/MC/RISCV/rvv/zvknh.s:22
+# CHECK-UNKNOWN: 77 25 94 b6   
+# CHECK-ERROR: instruction requires the following: 'Zvknha' (Vector SHA-2. 
(SHA-256 only)) or 'Zvknhb' (Vector SHA-2. (SHA-256 and SHA-512))
+

Add `{{$}}` to the end of the line to make sure it checks the whole line. Same 
for all similar lines in all tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141672

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

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

2023-03-07 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a subscriber: w2yehia.
hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGen/PowerPC/aix-roptr.c:9-10
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=DATA_SECTION_ERR
+// RUN: not %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -mroptr \
+// RUN: -S <%s 2>&1 | FileCheck %s --check-prefix=TARGET_ROPTR_ERR
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -mno-roptr \

Note that a driver test is needed for this sort of case as well.



Comment at: clang/test/CodeGen/PowerPC/aix-roptr.c:11-16
+// RUN: not %clang -target powerpc64le-unknown-linux-gnu -mno-roptr \
+// RUN: -S %s 2>&1 | FileCheck %s --check-prefix=TARGET_NOROPTR_ERR
+// RUN: not %clang -target powerpc-ibm-aix-xcoff -mroptr -shared \
+// RUN: -S %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR
+// RUN: not %clang -target powerpc64-ibm-aix-xcoff -mroptr -shared \
+// RUN: -S %s 2>&1 | FileCheck %s --check-prefix=SHARED_ERR

These are driver tests. Please move (and use `-###` to avoid accidental 
reliance on front-end processing).



Comment at: clang/test/Driver/ppc-roptr.c:1
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr %s 2>&1 | FileCheck 
%s
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 | 
\

Should test for `-c` and `-S` as well.
Should test for pure link (without compile) as well.



Comment at: clang/test/Driver/ppc-roptr.c:3
+// RUN: %clang -### -target powerpc-ibm-aix-xcoff -mroptr -mno-roptr %s 2>&1 | 
\
+// RUN: FileCheck %s --check-prefix=ROPTR
+// RUN: %clang -### -target powerpc64-ibm-aix-xcoff -mroptr %s 2>&1 | 
FileCheck %s

I think the prefix for this should be named `NO_ROPTR`.



Comment at: clang/test/Driver/ppc-roptr.c:10
+// ROPTR-NOT: "-mroptr"
+// ROPTR-NOT: "-bforceimprw"
+

Do we pass the option through to the LTO codegen when `-fprofile-generate` is 
used?
We may need a driver diagnostic for `-mroptr` without data sections when 
linking LTO objects (because the front-end won't be involved).

@w2yehia, does LLVM trunk have a gap in LTO driver enablement for AIX?



Comment at: clang/test/Driver/ppc-roptr.c:12-18
+char c1 = 10;
+char c2 = 20;
+char* const c1_ptr = 
+
+int main() {
+*(char**)_ptr = 
+}

Why have any code here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D145545: [clang][Interp] Fix local variable (destructor) management in loop bodies

2023-03-07 Thread Timm Bäder via Phabricator via cfe-commits

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

Ok, let me explain.

PREVIOUSLY:

When generating bytecode for a loop, we `visitStmt(Body)`, which will create 
its own local scope (that makes sense) and at the end of said scope, it will 
emit `Destroy` ops for all local variables, as well as emit code for the 
destructors of those local variables. However, the `Destroy` op calls the 
destructor for all data in the `Block`, and converts the blocks to 
`DeadBlock`s. That means the contents aren't usable anymore after it, and when 
we jump to the beginning of the loop and try to assign something to the block, 
we might end up trying to use the unusable data. This happens in the copy 
constructor of our `Pointer` class for example.

In pseudo code:

  loop {
  create locals...
  
 local destructors
 Destroy
 jump to beginning
  }

NOW:

I've split emitting the destructors for locals and emitting the `Destroy` op 
for a scope up into two separate parts. They are mostly still the same, 
i.e.when using a `AutoScope` or any of its subclasses, which emits the 
destructors and the `Destroy` op in its own destructor.

When visiting a loop, we create our own `LocalScope` to manage the lifetimes of 
locals created within the body and then call `emitDestructors` of that scope 
right before the `jmp` to the beginning of the loop. The `Destroy` op for the 
scope is emitted in the `LocalScope` destructor, so will only be emitted once.

In pseudo code:

  loop {
  create locals...
  
 local destructors
 jump to beginning
  }
  Destroy

The changes in this patch are on top of https://reviews.llvm.org/D137070, but 
the patch needs to go in before that //or// be merged with it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145545

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h

Index: clang/lib/AST/Interp/ByteCodeStmtGen.h
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.h
+++ clang/lib/AST/Interp/ByteCodeStmtGen.h
@@ -54,6 +54,7 @@
   // Statement visitors.
   bool visitStmt(const Stmt *S);
   bool visitCompoundStmt(const CompoundStmt *S);
+  bool visitUnscopedCompoundStmt(const Stmt *S);
   bool visitDeclStmt(const DeclStmt *DS);
   bool visitReturnStmt(const ReturnStmt *RS);
   bool visitIfStmt(const IfStmt *IS);
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -194,6 +194,17 @@
   }
 }
 
+template 
+bool ByteCodeStmtGen::visitUnscopedCompoundStmt(const Stmt *S) {
+  if (isa(S))
+return true;
+
+  for (auto *InnerStmt : cast(S)->body())
+if (!visitStmt(InnerStmt))
+  return false;
+  return true;
+}
+
 template 
 bool ByteCodeStmtGen::visitCompoundStmt(
 const CompoundStmt *CompoundStmt) {
@@ -306,11 +317,13 @@
   if (!this->jumpFalse(EndLabel))
 return false;
 
-  if (!this->visitStmt(Body))
+  LocalScope Scope(this);
+  if (!this->visitUnscopedCompoundStmt(Body))
 return false;
+
+  Scope.emitDestructors();
   if (!this->jump(CondLabel))
 return false;
-
   this->emitLabel(EndLabel);
 
   return true;
@@ -325,13 +338,16 @@
   LabelTy EndLabel = this->getLabel();
   LabelTy CondLabel = this->getLabel();
   LoopScope LS(this, EndLabel, CondLabel);
+  LocalScope Scope(this);
 
   this->emitLabel(StartLabel);
-  if (!this->visitStmt(Body))
+  if (!this->visitUnscopedCompoundStmt(Body))
 return false;
   this->emitLabel(CondLabel);
   if (!this->visitBool(Cond))
 return false;
+
+  Scope.emitDestructors();
   if (!this->jumpTrue(StartLabel))
 return false;
   this->emitLabel(EndLabel);
@@ -350,6 +366,7 @@
   LabelTy CondLabel = this->getLabel();
   LabelTy IncLabel = this->getLabel();
   LoopScope LS(this, EndLabel, IncLabel);
+  LocalScope Scope(this);
 
   if (Init && !this->visitStmt(Init))
 return false;
@@ -360,11 +377,13 @@
 if (!this->jumpFalse(EndLabel))
   return false;
   }
-  if (Body && !this->visitStmt(Body))
+  if (Body && !this->visitUnscopedCompoundStmt(Body))
 return false;
   this->emitLabel(IncLabel);
   if (Inc && !this->discard(Inc))
 return false;
+
+  Scope.emitDestructors();
   if (!this->jump(CondLabel))
 return false;
   this->emitLabel(EndLabel);
@@ -386,38 +405,38 @@
   LabelTy CondLabel = this->getLabel();
   LabelTy IncLabel = this->getLabel();
   LoopScope LS(this, EndLabel, IncLabel);
-  {
-ExprScope ES(this);
 
-// Emit declarations needed in the loop.
-if (Init && !this->visitStmt(Init))
-  return false;
-if (!this->visitStmt(RangeStmt))
-  return false;
-if

[PATCH] D145040: Add test for Flags.data_flow_trace

2023-03-07 Thread Vitaly Buka 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 rG67f5b05cdcdc: Add test for Flags.data_flow_trace (authored 
by yingcong-wu, committed by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145040

Files:
  compiler-rt/lib/fuzzer/FuzzerDriver.cpp
  compiler-rt/test/fuzzer/dataflow.test


Index: compiler-rt/test/fuzzer/dataflow.test
===
--- compiler-rt/test/fuzzer/dataflow.test
+++ compiler-rt/test/fuzzer/dataflow.test
@@ -97,6 +97,11 @@
 L20_FUZZM-NEXT: F2 0001{{$}}
 L20_FUZZM-NOT: F
 
+# Don't crash with missing data_flow args.
+RUN: rm -rf %t-DFT
+RUN: %t-ThreeFunctionsTest -collect_data_flow=%t-ThreeFunctionsTestDF
+RUN: %t-ThreeFunctionsTest -data_flow_trace=%t-DFT %t/IN/FUZZMU
+
 # Test libFuzzer's built in DFT collection.
 RUN: rm -rf %t-DFT
 RUN: %t-ThreeFunctionsTest  -collect_data_flow=%t-ThreeFunctionsTestDF 
-data_flow_trace=%t-DFT %t/IN/FUZZMU
Index: compiler-rt/lib/fuzzer/FuzzerDriver.cpp
===
--- compiler-rt/lib/fuzzer/FuzzerDriver.cpp
+++ compiler-rt/lib/fuzzer/FuzzerDriver.cpp
@@ -797,7 +797,7 @@
   if (Flags.verbosity)
 Printf("INFO: Seed: %u\n", Seed);
 
-  if (Flags.collect_data_flow && !Flags.fork &&
+  if (Flags.collect_data_flow && Flags.data_flow_trace && !Flags.fork &&
   !(Flags.merge || Flags.set_cover_merge)) {
 if (RunIndividualFiles)
   return CollectDataFlow(Flags.collect_data_flow, Flags.data_flow_trace,


Index: compiler-rt/test/fuzzer/dataflow.test
===
--- compiler-rt/test/fuzzer/dataflow.test
+++ compiler-rt/test/fuzzer/dataflow.test
@@ -97,6 +97,11 @@
 L20_FUZZM-NEXT: F2 0001{{$}}
 L20_FUZZM-NOT: F
 
+# Don't crash with missing data_flow args.
+RUN: rm -rf %t-DFT
+RUN: %t-ThreeFunctionsTest -collect_data_flow=%t-ThreeFunctionsTestDF
+RUN: %t-ThreeFunctionsTest -data_flow_trace=%t-DFT %t/IN/FUZZMU
+
 # Test libFuzzer's built in DFT collection.
 RUN: rm -rf %t-DFT
 RUN: %t-ThreeFunctionsTest  -collect_data_flow=%t-ThreeFunctionsTestDF -data_flow_trace=%t-DFT %t/IN/FUZZMU
Index: compiler-rt/lib/fuzzer/FuzzerDriver.cpp
===
--- compiler-rt/lib/fuzzer/FuzzerDriver.cpp
+++ compiler-rt/lib/fuzzer/FuzzerDriver.cpp
@@ -797,7 +797,7 @@
   if (Flags.verbosity)
 Printf("INFO: Seed: %u\n", Seed);
 
-  if (Flags.collect_data_flow && !Flags.fork &&
+  if (Flags.collect_data_flow && Flags.data_flow_trace && !Flags.fork &&
   !(Flags.merge || Flags.set_cover_merge)) {
 if (RunIndividualFiles)
   return CollectDataFlow(Flags.collect_data_flow, Flags.data_flow_trace,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145040: Add test for Flags.data_flow_trace

2023-03-07 Thread Vitaly Buka via Phabricator via cfe-commits

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

test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145040

Files:
  compiler-rt/lib/fuzzer/FuzzerDriver.cpp
  compiler-rt/test/fuzzer/dataflow.test


Index: compiler-rt/test/fuzzer/dataflow.test
===
--- compiler-rt/test/fuzzer/dataflow.test
+++ compiler-rt/test/fuzzer/dataflow.test
@@ -97,6 +97,11 @@
 L20_FUZZM-NEXT: F2 0001{{$}}
 L20_FUZZM-NOT: F
 
+# Don't crash with missing data_flow args.
+RUN: rm -rf %t-DFT
+RUN: %t-ThreeFunctionsTest -collect_data_flow=%t-ThreeFunctionsTestDF
+RUN: %t-ThreeFunctionsTest -data_flow_trace=%t-DFT %t/IN/FUZZMU
+
 # Test libFuzzer's built in DFT collection.
 RUN: rm -rf %t-DFT
 RUN: %t-ThreeFunctionsTest  -collect_data_flow=%t-ThreeFunctionsTestDF 
-data_flow_trace=%t-DFT %t/IN/FUZZMU
Index: compiler-rt/lib/fuzzer/FuzzerDriver.cpp
===
--- compiler-rt/lib/fuzzer/FuzzerDriver.cpp
+++ compiler-rt/lib/fuzzer/FuzzerDriver.cpp
@@ -797,7 +797,7 @@
   if (Flags.verbosity)
 Printf("INFO: Seed: %u\n", Seed);
 
-  if (Flags.collect_data_flow && !Flags.fork &&
+  if (Flags.collect_data_flow && Flags.data_flow_trace && !Flags.fork &&
   !(Flags.merge || Flags.set_cover_merge)) {
 if (RunIndividualFiles)
   return CollectDataFlow(Flags.collect_data_flow, Flags.data_flow_trace,


Index: compiler-rt/test/fuzzer/dataflow.test
===
--- compiler-rt/test/fuzzer/dataflow.test
+++ compiler-rt/test/fuzzer/dataflow.test
@@ -97,6 +97,11 @@
 L20_FUZZM-NEXT: F2 0001{{$}}
 L20_FUZZM-NOT: F
 
+# Don't crash with missing data_flow args.
+RUN: rm -rf %t-DFT
+RUN: %t-ThreeFunctionsTest -collect_data_flow=%t-ThreeFunctionsTestDF
+RUN: %t-ThreeFunctionsTest -data_flow_trace=%t-DFT %t/IN/FUZZMU
+
 # Test libFuzzer's built in DFT collection.
 RUN: rm -rf %t-DFT
 RUN: %t-ThreeFunctionsTest  -collect_data_flow=%t-ThreeFunctionsTestDF -data_flow_trace=%t-DFT %t/IN/FUZZMU
Index: compiler-rt/lib/fuzzer/FuzzerDriver.cpp
===
--- compiler-rt/lib/fuzzer/FuzzerDriver.cpp
+++ compiler-rt/lib/fuzzer/FuzzerDriver.cpp
@@ -797,7 +797,7 @@
   if (Flags.verbosity)
 Printf("INFO: Seed: %u\n", Seed);
 
-  if (Flags.collect_data_flow && !Flags.fork &&
+  if (Flags.collect_data_flow && Flags.data_flow_trace && !Flags.fork &&
   !(Flags.merge || Flags.set_cover_merge)) {
 if (RunIndividualFiles)
   return CollectDataFlow(Flags.collect_data_flow, Flags.data_flow_trace,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144190: [AIX][clang] Storage Locations for Constant Pointers

2023-03-07 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:229-231
+- Introduced the ``-mroptr`` option to place constant objects with relocatable
+  address values in the ready-only data section. This option is intended to
+  be used with the ``-fdata-sections`` option.

Should also mention the link-time behaviour that causes the read-only data 
sections with relocatable address values that resolve to imported symbols to be 
made writable.



Comment at: clang/docs/ReleaseNotes.rst:230
+- Introduced the ``-mroptr`` option to place constant objects with relocatable
+  address values in the ready-only data section. This option is intended to
+  be used with the ``-fdata-sections`` option.

Typo fix.



Comment at: clang/include/clang/Driver/Options.td:3896
+def mroptr : Flag<["-"], "mroptr">, Group, Flags<[CC1Option]>,
+  HelpText<"Place constant objects with relocatable address values in the RO 
data section">;
+def mno_roptr : Flag<["-"], "mno-roptr">, Group;

Also: "Implies -bforceimprw when specified at link time."



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:127
+  // The `-mroptr` option places constants in RO sections as much as possible.
+  // Then `-bforceimprw` changes such sections to RW if they contain import
+  // symbols that needs to be resolved.

Minor nit: grammar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144190

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

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

2023-03-07 Thread Jordan Rupprecht via Phabricator via cfe-commits

rupprecht added a comment.

In D144285#4163004 , @cor3ntin wrote:

> If however we find this change to disruptive, we should inform WG21.

Thanks for the explanation, I think I understand the issue now. I got totally 
thrown off by the title -- it's not about literally writing 
`static_assert(false)`, it's about deferring `static_assert` evaluation to 
template instantiations. Being able to write `static_assert(false)` (or any 
falsy constant expression) is just the common use case for this.

So possibly the most trivial example of something that used to break, but now 
builds:

  template 
  void Fail() {
  static_assert(false, "my assertion failed");
  }

... but will still fail as soon as you invoke `Fail()` somewhere.

It doesn't look like there's a lot of impact from this, and the breakages are 
corner cases like this. It might be worth mentioning this one case to WG21 but 
I'm not sure what I would change about the wording.




Comment at: clang/www/cxx_dr_status.html:14915
   
 https://wg21.link/cwg2518;>2518
 review

Is it possible to make this link point to 
https://cplusplus.github.io/CWG/issues/2518.html? This link is inaccessible to 
anyone not on ISO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144285

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

[PATCH] D145473: [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit

2023-03-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3283bf192c6: [test/ARCMT/verify.m] Add lit test for 
`5e035651fd3acbb2645abbe80cae332d90eac78… (authored by akyrtzi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145473

Files:
  clang/test/ARCMT/verify.m


Index: clang/test/ARCMT/verify.m
===
--- clang/test/ARCMT/verify.m
+++ clang/test/ARCMT/verify.m
@@ -8,6 +8,9 @@
 #error should not be ignored
 // expected-error@-1 {{should not be ignored}}
 
+#error
+// expected-error@-1 {{}}
+
 //  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
 // CHECK-NEXT: error: 'error' diagnostics seen but not expected:
 // CHECK-NEXT:   (frontend): error reading '{{.*}}verify.m.tmp.invalid'


Index: clang/test/ARCMT/verify.m
===
--- clang/test/ARCMT/verify.m
+++ clang/test/ARCMT/verify.m
@@ -8,6 +8,9 @@
 #error should not be ignored
 // expected-error@-1 {{should not be ignored}}
 
+#error
+// expected-error@-1 {{}}
+
 //  CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics'
 // CHECK-NEXT: error: 'error' diagnostics seen but not expected:
 // CHECK-NEXT:   (frontend): error reading '{{.*}}verify.m.tmp.invalid'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b3283bf - [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit

2023-03-07 Thread Argyrios Kyrtzidis via cfe-commits


Author: Argyrios Kyrtzidis
Date: 2023-03-07T20:23:07-08:00
New Revision: b3283bf192c6dbd6416b58e42f90ca443e8f005d

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

LOG: [test/ARCMT/verify.m] Add lit test for 
`5e035651fd3acbb2645abbe80cae332d90eac78a` commit

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

Added: 


Modified: 
clang/test/ARCMT/verify.m

Removed: 




diff  --git a/clang/test/ARCMT/verify.m b/clang/test/ARCMT/verify.m
index 9b46c1a729ac2..7480e0dc9677c 100644
--- a/clang/test/ARCMT/verify.m
+++ b/clang/test/ARCMT/verify.m
@@ -8,6 +8,9 @@
 #error should not be ignored
 // expected-error@-1 {{should not be ignored}}
 
+#error
+// expected-error@-1 {{}}
+
 //  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
 // CHECK-NEXT: error: 'error' diagnostics seen but not expected:
 // CHECK-NEXT:   (frontend): error reading '{{.*}}verify.m.tmp.invalid'



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

[PATCH] D145509: [HIP] Fix temporary files

2023-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 503218.
yaxunl edited the summary of this revision.
yaxunl added a reviewer: keith.

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

https://reviews.llvm.org/D145509

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/hip-link-bc-to-bc.hip
  clang/test/Driver/hip-temps-linux.hip
  clang/test/Driver/hip-temps-windows.hip

Index: clang/test/Driver/hip-temps-windows.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-windows.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-windows
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMP=%t/mytmp %clang --target=x86_64-pc-windows-msvc -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o "{{.*}}mytmp\\hip-temps-windows-gfx1030-{{.*}}.o"
+
+int main() {}
Index: clang/test/Driver/hip-temps-linux.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-linux.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-linux
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMPDIR=%t/mytmp %clang --target=x86_64-linux-gnu -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o {{.*}}/mytmp/hip-temps-linux-gfx1030-{{.*}}.o
+
+int main() {}
Index: clang/test/Driver/hip-link-bc-to-bc.hip
===
--- clang/test/Driver/hip-link-bc-to-bc.hip
+++ clang/test/Driver/hip-link-bc-to-bc.hip
@@ -11,10 +11,10 @@
 // RUN:   2>&1 | FileCheck -check-prefix=BITCODE %s
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle1.bc" "-output=[[B1HOST:.*\.bc]]" "-output=[[B1DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906.bc]]" "-x" "ir" "[[B1DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906-.*\.bc]]" "-x" "ir" "[[B1DEV1]]"
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle2.bc" "-output=[[B2HOST:.*\.bc]]" "-output=[[B2DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906.bc]]" "-x" "ir" "[[B2DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906-.*\.bc]]" "-x" "ir" "[[B2DEV1]]"
 
 // BITCODE: "{{.*}}llvm-link" "-o" "bundle1-hip-amdgcn-amd-amdhsa-gfx906.bc" "[[B1DEV2]]" "[[B2DEV2]]"
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5545,7 +5545,8 @@
 
 const char *Driver::CreateTempFile(Compilation , StringRef Prefix,
StringRef Suffix, bool MultipleArchs,
-   StringRef BoundArch) const {
+   StringRef BoundArch,
+   Action::OffloadKind OFK) const {
   SmallString<128> TmpName;
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
   std::optional CrashDirectory =
@@ -5565,9 +5566,19 @@
 }
   } else {
 if (MultipleArchs && !BoundArch.empty()) {
-  TmpName = GetTemporaryDirectory(Prefix);
-  llvm::sys::path::append(TmpName,
-  Twine(Prefix) + "-" + BoundArch + "." + Suffix);
+  llvm::Triple Triple(C.getDriver().getTargetTriple());
+  if ((OFK != Action::OFK_None && OFK != Action::OFK_Host) ||
+  !Triple.isOSDarwin()) {
+TmpName =
+GetTemporaryPath((Twine(Prefix) + "-" + BoundArch).str(), Suffix);
+  } else {
+// The non-offloading toolchain on Darwin requires deterministic input
+// file name for binaries to be deterministic.
+TmpName = GetTemporaryDirectory(Prefix);
+llvm::sys::path::append(TmpName,
+Twine(Prefix) + "-" + BoundArch + "." + Suffix);
+  }
+
 } else {
   TmpName = GetTemporaryPath(Prefix, Suffix);
 }
@@ -5683,7 +5694,8 @@
 StringRef Name = llvm::sys::path::filename(BaseInput);

[PATCH] D143496: [clangd] Add support for missing includes analysis.

2023-03-07 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

Oh, that was reported a while ago already. Reverted in 
2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[clang-tools-extra] 2eb5ac9 - Revert "[clangd] Add support for missing includes analysis."

2023-03-07 Thread Nico Weber via cfe-commits


Author: Nico Weber
Date: 2023-03-07T22:14:11-05:00
New Revision: 2eb5ac99a76dbbf8ac68c538211fabeaa5ac0bfd

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

LOG: Revert "[clangd] Add support for missing includes analysis."

This reverts commit 38b9fb5a129db3e086610d53b534833273c5b4d0.
Breaks tests on Windows, see comments on https://reviews.llvm.org/D143496

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index ab346aab0e8a..dffd54b01c45 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -88,12 +88,11 @@ struct Config {
 bool StandardLibrary = true;
   } Index;
 
-  enum class IncludesPolicy {
-/// Diagnose missing and unused includes.
+  enum class UnusedIncludesPolicy {
+/// Diagnose unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library for
-/// unused includes.
+/// The same as Strict, but using the include-cleaner library.
 Experiment,
   };
   /// Controls warnings and errors when parsing code.
@@ -108,12 +107,11 @@ struct Config {
   llvm::StringMap CheckOptions;
 } ClangTidy;
 
+UnusedIncludesPolicy UnusedIncludes = UnusedIncludesPolicy::None;
+
 /// Enable emitting diagnostics using stale preambles.
 bool AllowStalePreamble = false;
 
-IncludesPolicy UnusedIncludes = IncludesPolicy::None;
-IncludesPolicy MissingIncludes = IncludesPolicy::None;
-
 /// IncludeCleaner will not diagnose usages of these headers matched by
 /// these regexes.
 struct {

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 2f0ef892131c..18de6e4d5c3b 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -431,16 +431,16 @@ struct FragmentCompiler {
   });
 
 if (F.UnusedIncludes)
-  if (auto Val = compileEnum("UnusedIncludes",
- **F.UnusedIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
- .map("None", Config::IncludesPolicy::None)
- .value())
+  if (auto Val =
+  compileEnum("UnusedIncludes",
+**F.UnusedIncludes)
+  .map("Strict", Config::UnusedIncludesPolicy::Strict)
+  .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
+  .map("None", Config::UnusedIncludesPolicy::None)
+  .value())
 Out.Apply.push_back([Val](const Params &, Config ) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
-
 if (F.AllowStalePreamble) {
   if (auto Val = F.AllowStalePreamble)
 Out.Apply.push_back([Val](const Params &, Config ) {
@@ -448,16 +448,6 @@ struct FragmentCompiler {
 });
 }
 
-if (F.MissingIncludes)
-  if (auto Val = compileEnum("MissingIncludes",
- **F.MissingIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("None", Config::IncludesPolicy::None)
- .value())
-Out.Apply.push_back([Val](const Params &, Config ) {
-  C.Diagnostics.MissingIncludes = *Val;
-});
-
 compile(std::move(F.Includes));
 compile(std::move(F.ClangTidy));
   }

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 956e8a859944..a5597596196f 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -221,7 +221,7 @@ struct Fragment {
 /// This often has other advantages, such as skipping some analysis.
 std::vector> Suppress;
 
-/// Controls how clangd will correct "unnecessary"

[PATCH] D145538: [NFC][AArch64] Document and improve FMV code.

2023-03-07 Thread Pavel Iliin via Phabricator via cfe-commits

ilinpv created this revision.
ilinpv added reviewers: tmatheson, danielkiss.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
ilinpv requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145538

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/include/llvm/TargetParser/AArch64TargetParser.h

Index: llvm/include/llvm/TargetParser/AArch64TargetParser.h
===
--- llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -25,6 +25,9 @@
 class Triple;
 
 namespace AArch64 {
+// Function Multi Versioning CPU features. They must be kept in sync with
+// compiler-rt enum CPUFeatures in lib/builtins/cpu_model.c with FEAT_MAX as
+// sentinel.
 enum CPUFeatures {
   FEAT_RNG,
   FEAT_FLAGM,
@@ -155,17 +158,18 @@
 // SubtargetFeature which may represent either an actual extension or some
 // internal LLVM property.
 struct ExtensionInfo {
-  StringRef Name;   // Human readable name, e.g. "profile".
-  ArchExtKind ID;   // Corresponding to the ArchExtKind, this extensions
-// representation in the bitfield.
-  StringRef Feature;// -mattr enable string, e.g. "+spe"
-  StringRef NegFeature; // -mattr disable string, e.g. "-spe"
-
-  // FIXME These were added by D127812 FMV support and need documenting:
-  CPUFeatures CPUFeature; // Bitfield value set in __aarch64_cpu_features
-  StringRef DependentFeatures;
-  unsigned FmvPriority;
-  static constexpr unsigned MaxFMVPriority = 1000;
+  StringRef Name;  // Human readable name, e.g. "profile".
+  ArchExtKind ID;  // Corresponding to the ArchExtKind, this
+   // extensions representation in the bitfield.
+  StringRef Feature;   // -mattr enable string, e.g. "+spe"
+  StringRef NegFeature;// -mattr disable string, e.g. "-spe"
+  CPUFeatures CPUFeature;  // Function Multi Versioning (FMV) bitfield value
+   // set in __aarch64_cpu_features
+  StringRef DependentFeatures; // FMV enabled features string,
+   // e.g. "+dotprod,+fp-armv8,+neon"
+  unsigned FmvPriority;// FMV feature priority
+  static constexpr unsigned MaxFMVPriority =
+  1000; // Maximum priority for FMV feature
 };
 
 // clang-format off
@@ -559,6 +563,9 @@
 void fillValidCPUArchList(SmallVectorImpl );
 
 bool isX18ReservedByDefault(const Triple );
+
+// For given features returns a mask to check if CPU support them. The mask is
+// used in Function Multi Versioning resolver conditions code generation.
 uint64_t getCpuSupportsMask(ArrayRef FeatureStrs);
 
 } // namespace AArch64
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3508,6 +3508,7 @@
   enum SecondParam { None, CPU, Tune };
   enum ThirdParam { Target, TargetClones };
   HasCommas = HasCommas || Str.contains(',');
+  const TargetInfo  = Context.getTargetInfo();
   // Warn on empty at the beginning of a string.
   if (Str.size() == 0)
 return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
@@ -3517,9 +3518,9 @@
   while (!Parts.second.empty()) {
 Parts = Parts.second.split(',');
 StringRef Cur = Parts.first.trim();
-SourceLocation CurLoc = Literal->getLocationOfByte(
-Cur.data() - Literal->getString().data(), getSourceManager(),
-getLangOpts(), Context.getTargetInfo());
+SourceLocation CurLoc =
+Literal->getLocationOfByte(Cur.data() - Literal->getString().data(),
+   getSourceManager(), getLangOpts(), TInfo);
 
 bool DefaultIsDupe = false;
 bool HasCodeGenImpact = false;
@@ -3527,7 +3528,7 @@
   return Diag(CurLoc, diag::warn_unsupported_target_attribute)
  << Unsupported << None << "" << TargetClones;
 
-if (Context.getTargetInfo().getTriple().isAArch64()) {
+if (TInfo.getTriple().isAArch64()) {
   // AArch64 target clones specific
   if (Cur == "default") {
 DefaultIsDupe = HasDefault;
@@ -3542,13 +3543,12 @@
 while (!CurParts.second.empty()) {
   CurParts = CurParts.second.split('+');
   StringRef CurFeature = CurParts.first.trim();
-  if (!Context.getTargetInfo().validateCpuSupports(CurFeature)) {
+  if (!TInfo.validateCpuSupports(CurFeature)) {
 Diag(CurLoc, diag::warn_unsupported_target_attribute)
 << Unsupported << None << CurFeature << TargetClones;
 continue;
   }
-

[PATCH] D145509: [HIP] Fix temporary files

2023-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In D145509#4175773 , @tra wrote:

> LGTM, but we should probably get someone familiar with macos to chime in, 
> just in case there may be some reason behind macos using temp directories 
> here.
>
>> This change is OK for MacOS as lipo does not requires specific
>
> I'm curious why lipo has been singled out. Is that the only use case that 
> ends up using this path?

AFAIK clang/llvm tools and lld does not depend on input file names, but I am 
not so sure about lipo, that's why I checked lipo man page.

However, it seems the previous code using GetTemporaryDirectory was intentional 
(https://reviews.llvm.org/D111269). My guess is that lipo using some hash of 
input file name in the generated binary, GetTemporaryPath will make the 
generated binary non-deterministic.

It seems I need to limit this change to HIP as HIP uses clang-offload-bundler 
which generates binary not depending on input file names.

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

https://reviews.llvm.org/D145509

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

2023-03-07 Thread Nico Weber via Phabricator via cfe-commits

thakis added a comment.

This breaks tests on windows: http://45.33.8.238/win/75486/step_9.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D145034: [Clang][Sema] Preparations to fix handling of out-of-line definitions of constrained templates

2023-03-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:113-114
+  if (TemplateParamLists.size() == 1) {
+// FIXME: pick the correct template parameter list based on NNS, SS
+// and getCurScope().
+TemplateParameterList *L = TemplateParamLists[0];

I think this can be done fairly straightforwardly: we should pick out the 
template parameter list whose depth equals the depth of 
`ClassTemplate->getTemplateParameters()`.



Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:141
+  ClassTemplate->getInjectedClassNameSpecialization();
+  if (Context.hasSameType(Injected, ContextType))
+return ClassTemplate->getTemplatedDecl();

This should also be guarded by a check that `TemplateParameterListsAreEqual` 
between `ClassTemplate->getTemplateParameters()` and the template parameter 
list we picked out of the given array of template parameter lists.

(With that check in place, we can move this back before the search for partial 
specializations.)



Comment at: 
clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp:2-4
+// XFAIL: *
+// NOTE: This test is marked XFAIL until the diagnostics of
+// too many template parameters is fixed.

alexander-shaposhnikov wrote:
> rsmith wrote:
> > What is the new diagnostic output? (Presumably we no longer match the 
> > partial specialization when trying to match the declaration on line 25, 
> > because the template parameter list doesn't match.)
> the new diagnostic output:
>   error: nested name specifier 'A::' for declaration does not refer 
> into a class, class template or class template partial specialization 
> 
> ```
> template
> void A::f0() { }
> ```
> 
> 
Hm, it'd definitely be nice to produce some notes pointing out the non-matching 
candidates, and ideally, why they don't match. But I don't think this is so 
terrible that we need to block the patch on it. I think for now you should 
update the test expectations and update the FIXME below to say what we want 
here, and remove the XFAIL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145034

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

[PATCH] D144304: [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment

2023-03-07 Thread Ziqing Luo via Phabricator via cfe-commits

ziqingluo-90 added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:663
+return stmt(isInUnspecifiedPointerContext(expr(
+
ignoringImpCasts(unaryOperator(isPreInc()).bind(UPCPreIncrementTag);
+  }

NoQ wrote:
> You're not checking whether the operand is a variable. This isn't incorrect, 
> given that the fixable will be thrown away if it doesn't claim any variables. 
> However, for performance it makes sense to never match things that don't act 
> on variables, so that to never construct the fixable object in the first 
> place. The check for the variable being an actual `VarDecl` is also useful 
> and can be made here instead of the `getFixit` method.
> 
> I think this is something we should do consistently: //if it's not the right 
> code, stop doing things with it as early as possible//. Premature 
> optimizations are evil, but in these cases it's not much of an optimization 
> really, it's just easier to write code this way from the start.
> 
> It also makes the code easier to read: you can easily see which patterns the 
> gadget covers, by just reading the `matcher()` method.
aha, I agree with you and I like your last sentence the most!   Treating the 
`matcher()` methods as documents, i.e., we are guaranteed that a matched node 
conforms to the "description" of the `matcher()`.  So that we can get rid of 
defensive checks.  For example, the 
`if (auto DRE = dyn_cast(Node->getSubExpr()))` statement at line 
671 could be a defensive check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144304

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

[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `[any]` to `(DRE.data() + any)`

2023-03-07 Thread Ziqing Luo via Phabricator via cfe-commits

ziqingluo-90 updated this revision to Diff 503193.
ziqingluo-90 added a comment.

Thanks for the valuable discussion about the philosophy on the ideal forms of 
fix-its.  In this case, I think `()[any]` and `(DRE.data() + any)` are 
both straightforward enough for the user to tell what has been changed and why 
we change that.  And I believe both forms are idiomatic so that the user are 
likely happy with the form.   I will keep this discussion in mind as we have 
other cases whose fix-its may be less idiomatic, such as D144304 
.

Since we have to choose one form for this case, I buy @NoQ 's argument to use 
`()[any]`.

In addition, thanks to @t-rasmud for reminding me of that `[0]` can be 
converted to `DRE.data()` optimally.   In such a specific case, I think we do 
want it to be the most concise form without confusing the user.


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

https://reviews.llvm.org/D143128

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+int f(unsigned long, void *);
+
+[[clang::unsafe_buffer_usage]]
+int unsafe_f(unsigned long, void *);
+
+void address_to_integer(int x) {
+  int * p = new int[10];
+  unsigned long n = (unsigned long) [5];
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:42}:"()[5]"
+  unsigned long m = (unsigned long) [x];
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:42}:"()[x]"
+}
+
+void call_argument(int x) {
+  int * p = new int[10];
+
+  f((unsigned long) [5], [x]);
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:21-[[@LINE-1]]:26}:"()[5]"
+  // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:28-[[@LINE-2]]:33}:"()[x]"
+}
+
+void ignore_unsafe_calls(int x) {
+  // Cannot fix `[x]` for now as it is an argument of an unsafe
+  // call. So no fix for variable `p`.
+  int * p = new int[10];
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+  unsafe_f((unsigned long) [5],
+	   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+	   [x]);
+
+  int * q = new int[10];
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span q"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+  unsafe_f((unsigned long) [5],
+	   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:28-[[@LINE-1]]:33}:"()[5]"
+	   (void*)0);
+}
+
+void odd_subscript_form() {
+  int * p = new int[10];
+  unsigned long n = (unsigned long) &5[p];
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:42}:"()[5]"
+}
+
+void index_is_zero() {
+  int * p = new int[10];
+  int n = p[5];
+
+  f((unsigned long)[0],
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:20-[[@LINE-1]]:25}:"p.data()"
+[0]);
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:10}:"p.data()"
+}
+
+// CHECK-NOT: fix-it
+// To test multiple function declarations, each of which carries
+// different incomplete informations:
+[[clang::unsafe_buffer_usage]]
+void unsafe_g(void*);
+
+void unsafe_g(void*);
+
+void multiple_unsafe_fundecls() {
+  int * p = new int[10];
+
+  unsafe_g([5]);
+}
+
+void unsafe_h(void*);
+
+[[clang::unsafe_buffer_usage]]
+void unsafe_h(void*);
+
+void unsafe_h(void* p) { ((char*)p)[10]; }
+
+void multiple_unsafe_fundecls2() {
+  int * p = new int[10];
+
+  unsafe_h([5]);
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include 
 #include 
+#include 
 
 using namespace llvm;
 using namespace clang;
@@ -117,6 +118,15 @@
   bool Matches;
 };
 
+// Because we're dealing with raw pointers, let's define what we mean by that.
+static auto hasPointerType() {
+return hasType(hasCanonicalType(pointerType()));
+}
+
+static auto hasArrayType() {
+return hasType(hasCanonicalType(arrayType()));
+}
+
 AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher, innerMatcher) {
   const DynTypedMatcher  = static_cast(innerMatcher);
   
@@ -149,6 +159,29 @@
 ));
 // clang-format off
 }
+
+// Returns a matcher that matches any expression `e` such that `InnerMatcher`
+// matches `e` and `e` is in an Unspecified Pointer Context (UPC).
+static internal::Matcher
+isInUnspecifiedPointerContext(internal::Matcher InnerMatcher) {
+  // A UPC can be
+  // 1. an argument of a function call (except the

[PATCH] D145173: Make section attribute and -ffunction-sections play nicely

2023-03-07 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

The inconsistency stem from the fact that with the default `.text`, the 
compiler may generate `.text.$suffix` if `-ffunction-sections` is specified or 
if a COMDAT is needed.
While with an explicit section, there is no such suffix appending mechanism 
used together with `-ffunction-sections`.

Therefore, there is is a missing feature and perhaps GCC wants to add it as 
well, but just changing the semantics silently may upset some users.


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

https://reviews.llvm.org/D145173

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

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

2023-03-07 Thread Johannes Doerfert via Phabricator via cfe-commits

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

This seems to pass our other tests, LG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145514

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

[PATCH] D144304: [-Wunsafe-buffer-usage] Add a Fixable for pointer pre-increment

2023-03-07 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:149-152
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+  return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}

Oh interesting, so `hasOperatorName` wasn't sufficient, because operators `++` 
and `++` have the same "name"?

Yeah, sounds like a universally useful matcher, we should commit it to 
`ASTMatchers.h` for everybody to use.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:663
+return stmt(isInUnspecifiedPointerContext(expr(
+
ignoringImpCasts(unaryOperator(isPreInc()).bind(UPCPreIncrementTag);
+  }

You're not checking whether the operand is a variable. This isn't incorrect, 
given that the fixable will be thrown away if it doesn't claim any variables. 
However, for performance it makes sense to never match things that don't act on 
variables, so that to never construct the fixable object in the first place. 
The check for the variable being an actual `VarDecl` is also useful and can be 
made here instead of the `getFixit` method.

I think this is something we should do consistently: //if it's not the right 
code, stop doing things with it as early as possible//. Premature optimizations 
are evil, but in these cases it's not much of an optimization really, it's just 
easier to write code this way from the start.

It also makes the code easier to read: you can easily see which patterns the 
gadget covers, by just reading the `matcher()` method.



Comment at: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp:25
+
+  // Don't know how to fix the following cases:
+  // CHECK-NOT: fix-it:"{{.*}}":{

It's probably a good idea to explicitly say `FIXME:` if we think these cases 
should eventually be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144304

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

[clang] f6e7a5c - [clang-format][NFC] Remove isCpp11AttributeSpecifier()

2023-03-07 Thread Owen Pan via cfe-commits


Author: Owen Pan
Date: 2023-03-07T15:15:37-08:00
New Revision: f6e7a5c29221f445e4cbddc32667a1e12a1446db

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

LOG: [clang-format][NFC] Remove isCpp11AttributeSpecifier()

See https://reviews.llvm.org/D137486#3910570.

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index fa80588cd3b2..f8ef22736abb 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -591,10 +591,6 @@ class AnnotatingParser {
 return false;
   }
 
-  bool isCpp11AttributeSpecifier(const FormatToken ) {
-return isCppAttribute(Style.isCpp(), Tok);
-  }
-
   bool parseSquare() {
 if (!CurrentToken)
   return false;
@@ -617,7 +613,7 @@ class AnnotatingParser {
 
 const bool IsInnerSquare = Contexts.back().InCpp11AttributeSpecifier;
 const bool IsCpp11AttributeSpecifier =
-isCpp11AttributeSpecifier(*Left) || IsInnerSquare;
+isCppAttribute(Style.isCpp(), *Left) || IsInnerSquare;
 
 // Treat C# Attributes [STAThread] much like C++ attributes [[...]].
 bool IsCSharpAttributeSpecifier =
@@ -2244,7 +2240,7 @@ class AnnotatingParser {
 if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const,
   tok::kw_requires, tok::kw_throw, tok::arrow,
   Keywords.kw_override, Keywords.kw_final) ||
-isCpp11AttributeSpecifier(*Tok.Next)) {
+isCppAttribute(Style.isCpp(), *Tok.Next)) {
   return false;
 }
 



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

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

2023-03-07 Thread Michael Spencer 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 rG57c7750f703d: [clang][DependencyScanner] Cache modulemap 
stat failures (authored by Bigcheese).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145526

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,11 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  StringRef FName = llvm::sys::path::filename(Filename);
+  if (FName == "module.modulemap" || FName == "module.map")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,11 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  StringRef FName = llvm::sys::path::filename(Filename);
+  if (FName == "module.modulemap" || FName == "module.map")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 57c7750 - [clang][DependencyScanner] Cache modulemap stat failures

2023-03-07 Thread Michael Spencer via cfe-commits


Author: Michael Spencer
Date: 2023-03-07T15:13:55-08:00
New Revision: 57c7750f703ddee9025f819cdd01c9e97666e7ab

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

LOG: [clang][DependencyScanner] Cache modulemap stat failures

Add `module.modulemap` as a file we cache stat failures for as there
are a lot of stats for this file.

Clang currently uses the files it should minimize as a proxy for files
it should cache stat failures for, but really we should cache stat
failures for all paths we don't expect to change during the build.
Unfortunately the VFS API does not know _why_ clang is trying to stat
a path, so we use the filename as a proxy.

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

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp

Removed: 




diff  --git 
a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
index 97b41fc689176..0ddb5c24c5e6c 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,11 @@ static bool shouldCacheStatFailures(StringRef Filename) {
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  StringRef FName = llvm::sys::path::filename(Filename);
+  if (FName == "module.modulemap" || FName == "module.map")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 



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

[PATCH] D145449: [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

In D145449#4175864 , @phosek wrote:

>> LLVM_ENABLE_PROJECTS is automatically forwarded from stage 1 builds to
>> stage 2 builds, so setting FUCHSIA_ENABLE_LLDB has no effect on
>> two-stage builds.
>
> That shouldn't be the case, do you know where it's being passed through?

Looks like it's being passed through here 
https://github.com/llvm/llvm-project/blob/c0e9c55db3b6a2a1287ba96ef3d378b97b72714a/clang/CMakeLists.txt#L669
 which was introduced by D40258 . I'll see if 
we can remove that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145449

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

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

2023-03-07 Thread Michael Spencer via Phabricator via cfe-commits

Bigcheese updated this revision to Diff 503164.
Bigcheese added a comment.

Also handle `module.map`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145526

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,11 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  StringRef FName = llvm::sys::path::filename(Filename);
+  if (FName == "module.modulemap" || FName == "module.map")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,11 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  StringRef FName = llvm::sys::path::filename(Filename);
+  if (FName == "module.modulemap" || FName == "module.map")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `[any]` to `(DRE.data() + any)`

2023-03-07 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

In D143128#4167626 , @jkorous wrote:

> This is an interesting topic. In the abstract I see the question as: "Should 
> the Fix-Its prioritize how the code will fit the desired end state 
> (presumably modern idiomatic C++) or carefully respect the state of the code 
> as is now?"
>
> The only thing I feel pretty strongly about is that no matter what philosophy 
> we decide to use here we should apply it consistently to all our Fix-Its 
> (which might or might not already be the case).
>
> And FWIW I can also imagine at some point in the future we might either have 
> two dialects of the Fix-Its or that a separate modernizer tool (completely 
> independent of Safe Buffers) could suggest transformations like:
> "Would you like to change `()[any]` to `(DRE.data() + any)`?"

In D143128#4167655 , @ymandel wrote:

> Fantastic topic! :)  In our experience at Google, we've generally followed 
> the philosophy of leaving the code at least as good as it was before we 
> touched it. So, if the idiom is archaic, but we're just adjusting it, that's 
> fine (as in this example), but our tools shouldn't generate non-idiomatic (or 
> anti-idiomic) code.  We've also often taken the path of "leave cleanups to a 
> separate pass", especially when we already have say, a clang tidy check, that 
> does that kind of clean up running regularly.  But, this one is more a 
> judgment call. I'd probably lean towards "make the code better once you're at 
> it", but certainly see the conservative argument.

That's a fantastic topic in general, but in *this* case I'm really not sure 
which one's ultimately "better", I'd totally write code both ways and at 
different times prefer one over the other.

I honestly actually prefer code like `[any]`/`()[any]` to the one 
with pointer arithmetic, specifically because it avoids the subject of pointer 
arithmetic, but instead talks about "Let's take this object, for which we 
already have a name, and take its address". The readers don't have to typecheck 
the expression in their heads in order to figure out whether correct offset 
multipliers are applied during pointer arithmetic. Another advantage of 
`()[any]` is that it's really easy to transform it to idiomatic 
`[any]` once the user confirms that the bounds checks don't actually get in 
the way in their case. This is much harder to achieve with `DRE.data() + any` 
given that `DRE + any` isn't valid code anymore.

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

https://reviews.llvm.org/D143128

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

[PATCH] D145449: [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Daniel Thornburgh via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0e9c55db3b6: [Fuchsia] Add LLDB options to stage 1 cmake. 
(authored by mysterymath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145449

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly" CACHE 
STRING "")
+set(_FUCHSIA_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly")
 
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
@@ -17,6 +17,8 @@
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 # Passthrough stage1 flags to stage1.
 set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH
@@ -30,6 +32,9 @@
   LLVM_ENABLE_CURL
   CURL_ROOT
   OpenSSL_ROOT
+  FUCHSIA_ENABLE_LLDB
+  LLDB_ENABLE_CURSES
+  LLDB_ENABLE_LIBEDIT
   CMAKE_FIND_PACKAGE_PREFER_CONFIG
   CMAKE_SYSROOT
   CMAKE_MODULE_LINKER_FLAGS
@@ -157,6 +162,12 @@
   install-distribution-toolchain
   clang CACHE STRING "")
 
+set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
+if(FUCHSIA_ENABLE_LLDB)
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+endif()
+set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")
+
 get_cmake_property(variableNames VARIABLES)
 foreach(variableName ${variableNames})
   if(variableName MATCHES "^STAGE2_")
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,8 @@
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_STATIC_LINK_CXX_STDLIB ON CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 if(WIN32)
   set(LLVM_USE_CRT_RELEASE "MT" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,7 +4,7 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly" CACHE STRING "")
+set(_FUCHSIA_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly")
 
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
@@ -17,6 +17,8 @@
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 # Passthrough stage1 flags to stage1.
 set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH
@@ -30,6 +32,9 @@
   LLVM_ENABLE_CURL
   CURL_ROOT
   OpenSSL_ROOT
+  FUCHSIA_ENABLE_LLDB
+  LLDB_ENABLE_CURSES
+  LLDB_ENABLE_LIBEDIT
   CMAKE_FIND_PACKAGE_PREFER_CONFIG
   CMAKE_SYSROOT
   CMAKE_MODULE_LINKER_FLAGS
@@ -157,6 +162,12 @@
   install-distribution-toolchain
   clang CACHE STRING "")
 
+set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
+if(FUCHSIA_ENABLE_LLDB)
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+endif()
+set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")
+
 get_cmake_property(variableNames VARIABLES)
 foreach(variableName ${variableNames})
   if(variableName MATCHES "^STAGE2_")
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,8 @@
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_STATIC_LINK_CXX_STDLIB ON CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 if(WIN32)
   set(LLVM_USE_CRT_RELEASE "MT" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c0e9c55 - [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Daniel Thornburgh via cfe-commits


Author: Daniel Thornburgh
Date: 2023-03-07T15:04:31-08:00
New Revision: c0e9c55db3b6a2a1287ba96ef3d378b97b72714a

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

LOG: [Fuchsia] Add LLDB options to stage 1 cmake.

LLVM_ENABLE_PROJECTS is automatically forwarded from stage 1 builds to
stage 2 builds, so setting FUCHSIA_ENABLE_LLDB has no effect on
two-stage builds.

Instead, add FUCHSIA_ENABLE_LLDB to the stage one build as well.

This also disables curses and libedit by default for now in both stage1
and stage 2 builds; these should be opt-in.

Reviewed By: haowei

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
clang/cmake/caches/Fuchsia.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 016fa9eb5f9d2..c874d8cacd197 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,8 @@ set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_STATIC_LINK_CXX_STDLIB ON CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 if(WIN32)
   set(LLVM_USE_CRT_RELEASE "MT" CACHE STRING "")

diff  --git a/clang/cmake/caches/Fuchsia.cmake 
b/clang/cmake/caches/Fuchsia.cmake
index d9b34dd757e9a..cf63153da89c0 100644
--- a/clang/cmake/caches/Fuchsia.cmake
+++ b/clang/cmake/caches/Fuchsia.cmake
@@ -4,7 +4,7 @@ set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64;RISCV CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly" CACHE 
STRING "")
+set(_FUCHSIA_ENABLE_PROJECTS "bolt;clang;clang-tools-extra;lld;llvm;polly")
 
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
 set(LLVM_ENABLE_LIBEDIT OFF CACHE BOOL "")
@@ -17,6 +17,8 @@ set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
+set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
+set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
 
 # Passthrough stage1 flags to stage1.
 set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH
@@ -30,6 +32,9 @@ set(_FUCHSIA_BOOTSTRAP_PASSTHROUGH
   LLVM_ENABLE_CURL
   CURL_ROOT
   OpenSSL_ROOT
+  FUCHSIA_ENABLE_LLDB
+  LLDB_ENABLE_CURSES
+  LLDB_ENABLE_LIBEDIT
   CMAKE_FIND_PACKAGE_PREFER_CONFIG
   CMAKE_SYSROOT
   CMAKE_MODULE_LINKER_FLAGS
@@ -157,6 +162,12 @@ set(CLANG_BOOTSTRAP_TARGETS
   install-distribution-toolchain
   clang CACHE STRING "")
 
+set(FUCHSIA_ENABLE_LLDB OFF CACHE BOOL "Enable LLDB")
+if(FUCHSIA_ENABLE_LLDB)
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+endif()
+set(LLVM_ENABLE_PROJECTS ${_FUCHSIA_ENABLE_PROJECTS} CACHE STRING "")
+
 get_cmake_property(variableNames VARIABLES)
 foreach(variableName ${variableNames})
   if(variableName MATCHES "^STAGE2_")



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

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

2023-03-07 Thread Michael Spencer via Phabricator via cfe-commits

Bigcheese updated this revision to Diff 503160.
Bigcheese added a comment.

Forgot that `Filename` is actually a path, so call `llvm::sys::path::filename`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145526

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,10 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  if (llvm::sys::path::filename(Filename) == "module.modulemap")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,10 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  if (llvm::sys::path::filename(Filename) == "module.modulemap")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143680: [-Wunsafe-buffer-usage] Improve fix-its for local variable declarations with null pointer initializers

2023-03-07 Thread Artem Dergachev via Phabricator via cfe-commits

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

Ok I think this is good to go!


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

https://reviews.llvm.org/D143680

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

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

2023-03-07 Thread Michael Spencer via Phabricator via cfe-commits

Bigcheese created this revision.
Bigcheese added reviewers: ributzka, benlangmuir, jansvoboda11.
Bigcheese added a project: clang.
Herald added a project: All.
Bigcheese requested review of this revision.
Herald added a subscriber: cfe-commits.

Add `module.modulemap` as a file we cache stat failures for as there
are a lot of stats for this file.

Clang currently uses the files it should minimize as a proxy for files
it should cache stat failures for, but really we should cache stat
failures for all paths we don't expect to change during the build.
Unfortunately the VFS API does not know _why_ clang is trying to stat
a path, so we use the filename as a proxy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145526

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,10 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  if (Filename == "module.modulemap")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -181,7 +181,10 @@
   StringRef Ext = llvm::sys::path::extension(Filename);
   if (Ext.empty())
 return false; // This may be the module cache directory.
-  // Only cache stat failures on source files.
+  // Only cache stat failures on files that are not expected to change during
+  // the build.
+  if (Filename == "module.modulemap")
+return true;
   return shouldScanForDirectivesBasedOnExtension(Filename);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-07 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits

dexonsmith added a comment.

In D130303#4175664 , @collinbaker 
wrote:

> @dexonsmith can you weigh in?

Introducing `clang_isBitFieldDecl` sounds like a clean/straightforward solution 
to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130303

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

[clang] bb70dac - [clang-format][NFC] Remove an obsolete case in parsing concepts

2023-03-07 Thread Owen Pan via cfe-commits


Author: Owen Pan
Date: 2023-03-07T14:42:22-08:00
New Revision: bb70dacd60b54d39ca144cf91d47d6c04404b816

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

LOG: [clang-format][NFC] Remove an obsolete case in parsing concepts

See https://reviews.llvm.org/D142412#4078127.

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index bf613db2b2aef..8576de7b72396 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -3441,17 +3441,6 @@ void UnwrappedLineParser::parseConstraintExpression() {
   /*ClosingBraceKind=*/tok::greater);
   break;
 
-case tok::kw_bool:
-  // bool is only allowed if it is directly followed by a paren for a cast:
-  // concept C = bool(...);
-  // and bool is the only type, all other types as cast must be inside a
-  // cast to bool an thus are handled by the other cases.
-  if (Tokens->peekNextToken()->isNot(tok::l_paren))
-return;
-  nextToken();
-  parseParens();
-  break;
-
 default:
   if (!FormatTok->Tok.getIdentifierInfo()) {
 // Identifiers are part of the default case, we check for more then



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

[PATCH] D145435: Choose style (file) from within code for use in IDEs

2023-03-07 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

In D145435#4176109 , 
@HazardyKnusperkeks wrote:

> I think this would be a great feature.

I don't understand why anyone wants to do this. If I'm accustomed to a 
different style, I'd temporarily change the config file in the current 
directory because chances are I would need to look at code written by other 
people. If I lock down the style for the files I created as this requested 
feature allows, what would happen if others need to maintain these files down 
the road?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145435

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

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

2023-03-07 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak added inline comments.



Comment at: clang/test/Driver/save-stats.c:26
 // CHECK-LTO: "-plugin-opt=stats-file=save-stats.stats"
+// CHECK-LTO: "-plugin-opt=-stats-file-append"
 

ahatanak wrote:
> Doesn't this require `stats-file-append` to be supported by the plugin just 
> like `stats-file` is supported?
> 
> https://github.com/llvm/llvm-project/blob/main/llvm/tools/gold/gold-plugin.cpp#L309
Do we want to pass `stats-file-append` in `tools::addLTOOptions`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144981

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

[PATCH] D145344: [clang-format] Don't annotate left brace of class as FunctionLBrace

2023-03-07 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

In D145344#4174634 , @MyDeveloperDay 
wrote:

> I would hazard a guess that this actually solves more issues than we realize.

That would be icing on the cake. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145344

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

[PATCH] D144981: [Driver] Allow to collect `-save-stats` data to a file specified in the environment variable.

2023-03-07 Thread Akira Hatanaka via Phabricator via cfe-commits

ahatanak added inline comments.



Comment at: clang/test/Driver/save-stats.c:26
 // CHECK-LTO: "-plugin-opt=stats-file=save-stats.stats"
+// CHECK-LTO: "-plugin-opt=-stats-file-append"
 

Doesn't this require `stats-file-append` to be supported by the plugin just 
like `stats-file` is supported?

https://github.com/llvm/llvm-project/blob/main/llvm/tools/gold/gold-plugin.cpp#L309


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144981

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

[PATCH] D143496: [clangd] Add support for missing includes analysis.

2023-03-07 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

This change broke regression, non-one read: "This revision was landed with 
ongoing or failed builds."
./ClangdTests.exe/IncludeCleaner/GenerateMissingHeaderDiags tests fails on 
windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

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

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

2023-03-07 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL updated this revision to Diff 503144.
PiotrZSL added a comment.

Rebase due to failed clangd tests on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-e.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-i.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-n.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-o.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.first-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.fourth-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.second-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.self-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.third-s.hpp
  clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
@@ -0,0 +1,58 @@
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+// RUN: mkdir %T/misc-header-include-cycle-headers
+// RUN: cp -r %S/Inputs/header-include-cycle* %T/misc-header-include-cycle-headers/
+// RUN: mkdir %T/misc-header-include-cycle-headers/system
+// RUN: cp -r %S/Inputs/system/header-include-cycle* %T/misc-header-include-cycle-headers/system
+// RUN: clang-tidy %s -checks='-*,misc-header-include-cycle' -header-filter=.* \
+// RUN: -config="{CheckOptions: [{key: misc-header-include-cycle.IgnoredFilesList, value: 'header-include-cycle.self-e.hpp'}]}" \
+// RUN: -- -I%T/misc-header-include-cycle-headers -isystem %T/misc-header-include-cycle-headers/system \
+// RUN: --include %T/misc-header-include-cycle-headers/header-include-cycle.self-i.hpp | FileCheck %s \
+// RUN: -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error|note}}:"
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+
+#ifndef MAIN_GUARD
+#define MAIN_GUARD
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth-d.hpp:3:10: warning: circular header file dependency detected while including 'header-include-cycle.first-d.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third-d.hpp:3:10: note: 'header-include-cycle.fourth-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second-d.hpp:3:10: note: 'header-include-cycle.third-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first-d.hpp:3:10: note: 'header-include-cycle.second-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.cpp:[[@LINE-5]]:10: note: 'header-include-cycle.first-d.hpp' included from here
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth.hpp:2:10: warning: circular header file dependency detected while including 'header-include-cycle.first.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third.hpp:2:10: note: 'header-include-cycle.fourth.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second.hpp:2:10: note: 'header-include-cycle.third.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first.hpp:2:10: note: 'header-include-cycle.second.hpp' included from here
+//

[clang] ae05d2b - Update the C status page for WG14 N2844

2023-03-07 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-03-07T16:13:06-05:00
New Revision: ae05d2b17dca399b3190e0ae1dba814aa3946002

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

LOG: Update the C status page for WG14 N2844

The proposal is about the behavior of the _Float16,
_Float32, and _Float64 types and whether they
undergo default argument promotions (they don't).

Clang doesn't yet support TS 18661 or the parts
that made it into C2x, so we don't implement this
paper.

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index 20eec37fce3de..27296777f0740 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -1048,7 +1048,7 @@ C2x implementation status
 
   Remove default argument promotions for _FloatN types
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2844.pdf;>N2844
-  Unknown
+  No
 
 
   Revised Suggestions of Change for Numerically Equal/Equivalent



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

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

2023-03-07 Thread Akash Banerjee via Phabricator via cfe-commits

TIFitis added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1561
+  /// should be placed.
+  /// \param HasRegion True if the op has a region associated with it, false
+  /// otherwise

kiranchandramohan wrote:
> TIFitis wrote:
> > kiranchandramohan wrote:
> > > Can't the presence/absence of the BodyGenCB indicate the presence/absence 
> > > of a region?
> > The `BodyGenCB` is always present as an argument right? Passing a `nullptr` 
> > when its not required doesn't seem possible at least when using the 
> > `BodyGenCallbackTy`. Is there a way to selectively pass the `BodyGenCB`?
> The only optional CallBack seems to be the `FinalizeCallbackTy`. It is 
> defined as an `std::function` whereas `BodyGenCallBackTy` is defined as a 
> `function_ref`. But the definition of `function_ref` looks like it can be 
> used to check whether it is a `nullptr` or not. Did you face any issues in 
> trying to make it optional with a default parameter value of `nullptr`?
> 
> https://llvm.org/doxygen/STLFunctionalExtras_8h_source.html#l00036
> 
> ```
>   void emitCancelationCheckImpl(Value *CancelFlag,
> omp::Directive CanceledDirective,
> FinalizeCallbackTy ExitCB = {});
> ```
Sorry, I was being stupid and trying to pass `nullptr` as an argument instead 
of making the actual parameter optional. `BodyGenCallBackTy` works.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4052-4060
+  // LLVM utilities like blocks with terminators.
+  auto *UI = Builder.CreateUnreachable();
+  Instruction *ThenTI = UI, *ElseTI = nullptr;
+  if (IfCond) {
+SplitBlockAndInsertIfThenElse(IfCond, UI, , );
+ThenTI->getParent()->setName("omp_if.then");
+ElseTI->getParent()->setName("omp_if.else");

kiranchandramohan wrote:
> TIFitis wrote:
> > kiranchandramohan wrote:
> > > TIFitis wrote:
> > > > kiranchandramohan wrote:
> > > > > There is some recent recommendation to not insert artificial 
> > > > > instructions and remove them. The preferred approach is to use 
> > > > > `splitBB` function(s) inside the OpenMPIRBuilder. This function works 
> > > > > on blocks without terminators. You can consult the `IfCondition` code 
> > > > > in `createParallel`.
> > > > Thanks a lot for taking the time to review this lengthy patch.
> > > > 
> > > > This one seems a bit tricky to do. At first glance `createParallel` 
> > > > seems to be doing something different where its calling different 
> > > > runtime functions based on the `IfCondition` instead of much in the way 
> > > > of Control Flow changes.
> > > > 
> > > > The `unreachable` inst helps out a lot here as it makes it really easy 
> > > > to keep trace of the resume point for adding instructions after the 
> > > > `BodyGen` codes are generated.
> > > > 
> > > > I am still looking into finding a way to do this elegantly without 
> > > > having to use the `unreachable` instruction, but would it be a major 
> > > > blocker if we had to keep it?
> > > > 
> > > > I have addressed all the other changes you requested.
> > > Thanks for explaining the need for the `unreachable`.  I will leave it 
> > > with you on whether to make the change.
> > > 
> > > You can see an instance of a past request for not using temporary 
> > > instruction. That patch (if for createTask) addressed the issue one way.
> > > https://reviews.llvm.org/D130615#inline-1257711
> > > 
> > > I gave a quick try and came up with the following code. I don't think it 
> > > is very elegant, but it is one way to do it. 
> > > ```
> > > -  // LLVM utilities like blocks with terminators.
> > > -  auto *UI = Builder.CreateUnreachable();
> > > +  BasicBlock *ContBB = nullptr;
> > >if (IfCond) {
> > > -auto *ThenTI =
> > > -SplitBlockAndInsertIfThen(IfCond, UI, /* Unreachable */ false);
> > > -ThenTI->getParent()->setName("omp_if.then");
> > > -Builder.SetInsertPoint(ThenTI);
> > > -  } else {
> > > -Builder.SetInsertPoint(UI);
> > > +BasicBlock *EntryBB = Builder.GetInsertBlock();
> > > +ContBB = splitBB(Builder, /*CreateBranch*/ false);
> > > +BasicBlock *ThenBB =
> > > +BasicBlock::Create(Builder.getContext(), EntryBB->getName() + ".if",
> > > +   ContBB->getParent(), ContBB);
> > > +ContBB->setName(EntryBB->getName() + ".if.end");
> > > +Builder.CreateCondBr(IfCond, ThenBB, ContBB);
> > > +Builder.SetInsertPoint(ThenBB);
> > > +Builder.CreateBr(ContBB);
> > > +Builder.SetInsertPoint(ThenBB->getTerminator());
> > >}
> > >  
> > >ProcessMapOpCB(Builder.saveIP(), Builder.saveIP());
> > > @@ -4087,9 +4090,10 @@ OpenMPIRBuilder::InsertPointTy 
> > > OpenMPIRBuilder::createTargetData(
> > >  emitMapperCall(Builder.saveIP(), beginMapperFunc, srcLocInfo, 
> > > MapTypesArg,
> > > MapNamesArg, MapperAllocas, DeviceID, 
> > > MapTypeFlags.size());
> > >  
>

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

2023-03-07 Thread Akash Banerjee via Phabricator via cfe-commits

TIFitis updated this revision to Diff 503137.
TIFitis marked 3 inline comments as done.
TIFitis added a comment.

Addressed comments. Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/include/mlir/Target/LLVMIR/Dialect/OpenMPCommon.h
  mlir/lib/Target/LLVMIR/CMakeLists.txt
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMPCommon.cpp
  mlir/test/Target/LLVMIR/omptarget-llvm.mlir

Index: mlir/test/Target/LLVMIR/omptarget-llvm.mlir
===
--- /dev/null
+++ mlir/test/Target/LLVMIR/omptarget-llvm.mlir
@@ -0,0 +1,176 @@
+// RUN: mlir-translate -mlir-to-llvmir -split-input-file %s | FileCheck %s
+
+llvm.func @_QPopenmp_target_data() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, operand_segment_sizes = array, uniq_name = "_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
+  omp.target_data   map((tofrom -> %1 : !llvm.ptr)) {
+%2 = llvm.mlir.constant(99 : i32) : i32
+llvm.store %2, %1 : !llvm.ptr
+omp.terminator
+  }
+  llvm.return
+}
+
+// CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
+// CHECK-LABEL: define void @_QPopenmp_target_data() {
+// CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
+// CHECK: %[[VAL_3:.*]] = alloca i32, i64 1, align 4
+// CHECK: br label %[[VAL_4:.*]]
+// CHECK:   entry:; preds = %[[VAL_5:.*]]
+// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_6]], align 8
+// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_7]], align 8
+// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %[[VAL_8]], align 4
+// CHECK: %[[VAL_9:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_10:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: %[[VAL_11:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr %[[VAL_11]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: br label %[[VAL_12:.*]]
+// CHECK:   omp.data.region:  ; preds = %[[VAL_4]]
+// CHECK: store i32 99, ptr %[[VAL_3]], align 4
+// CHECK: br label %[[VAL_13:.*]]
+// CHECK:   omp.region.cont:  ; preds = %[[VAL_12]]
+// CHECK: %[[VAL_14:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_15:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: %[[VAL_16:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_14]], ptr %[[VAL_15]], ptr %[[VAL_16]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: ret void
+
+// -
+
+llvm.func @_QPopenmp_target_data_region(%1 : !llvm.ptr>) {
+  omp.target_data   map((from -> %1 : !llvm.ptr>)) {
+%2 = llvm.mlir.constant(99 : i32) : i32
+%3 = llvm.mlir.constant(1 : i64) : i64
+%4 = llvm.mlir.constant(1 : i64) : i64
+%5 = llvm.mlir.constant(0 : i64) : i64
+%6 = llvm.getelementptr %1[0, %5] : (!llvm.ptr>, i64) -> !llvm.ptr
+llvm.store %2, %6 : !llvm.ptr
+omp.terminator
+  }
+  llvm.return
+}
+
+// CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 2]
+// CHECK-LABEL: define void @_QPopenmp_target_data_region
+// CHECK: (ptr %[[ARG_0:.*]]) {
+// CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
+// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
+// CHECK: br label %[[VAL_3:.*]]
+// CHECK:   entry:; preds = %[[VAL_4:.*]]
+// CHECK: %[[VAL_5:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0

[PATCH] D144823: [Driver][FreeBSD] Simplify ARM handling

2023-03-07 Thread Brad Smith via Phabricator via cfe-commits

brad added a comment.

@emaste Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144823

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

[PATCH] D145435: Choose style (file) from within code for use in IDEs

2023-03-07 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/Format.cpp:3542
+
+// Prefix found, and at start of file or preceded by white space?
+if (PrefixPos != StringRef::npos &&

I understand the reasoning, but the check down below I think does not match 
this. You only look if there is exactly one whitespace character before your 
`Prefix`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145435

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

[PATCH] D145435: Choose style (file) from within code for use in IDEs

2023-03-07 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

I think this would be a great feature. But I also think that it needs more 
documentation.

You also need to make sure, that we don't change that comment, that is 
`SpacesInLineCommentPrefix` and `ColumnLimit` should have no effect on the 
comment.




Comment at: clang/lib/Format/Format.cpp:3544
+if (PrefixPos != StringRef::npos &&
+((PrefixPos == 0) || (Code.substr(PrefixPos - 1, 1).ltrim() == ""))) {
+

Drop the extra parens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145435

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

2023-03-07 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle added a comment.

In D145441#4175428 , @krzysz00 wrote:

> @foad I was trying to avoid sending in one mega-patch that has the entire 
> prototype.

The best way to do this is to work on a branch that you rebase regularly. You 
can push the branch to a personal clone of llvm-project on GitHub and point 
folks there from e.g. Discourse, and for the purpose of Phabricator you can set 
up a "Stack" via the "Edit Related Revisions" option.

If you're using `arc`, then `git rebase -i -x "arc diff @^"` is one way to 
update a bunch of patches at once. There are probably other tools that people 
have written.

> These [addrspace(8) pointers] are, however, integral, since inttoptr and 
> ptrtoint behave deterministically and reasonably.

That doesn't make sense to me. I though "integral" means that

  inttoptr(ptrtoint(p) + offset)

is the same (in terms of address, and potentially modulo provenance questions) 
as

  gep i8, p, offset

But that is rather untrue for addrspace(8), isn't it?

Comment at: llvm/docs/AMDGPUUsage.rst:794
+**Buffer Resource**
+  The buffer resource is an experemntal address space that is currently 
unsupported
+  in the backend. It exposes a non-integral pointer that will represent a 
128-bit

Typo: experimental

Comment at: llvm/docs/AMDGPUUsage.rst:804-805
+
+  Casting a buffer resource to a bufer fat pointer is permitted and adds an 
offset
+  of 0.
+

Presumably it is only permitted if the underlying descriptor satisfies all the 
rules mentioned for addrspace(7).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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

[PATCH] D144828: [clang-tidy] Add misc-header-include-cycle check

2023-03-07 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL updated this revision to Diff 503122.
PiotrZSL added a comment.

Ready for review, added new check option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144828

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
  clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-e.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-i.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-n.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self-o.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third-d.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.first-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.fourth-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.second-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.self-s.hpp
  
clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/system/header-include-cycle.third-s.hpp
  clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp
@@ -0,0 +1,58 @@
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+// RUN: mkdir %T/misc-header-include-cycle-headers
+// RUN: cp -r %S/Inputs/header-include-cycle* %T/misc-header-include-cycle-headers/
+// RUN: mkdir %T/misc-header-include-cycle-headers/system
+// RUN: cp -r %S/Inputs/system/header-include-cycle* %T/misc-header-include-cycle-headers/system
+// RUN: clang-tidy %s -checks='-*,misc-header-include-cycle' -header-filter=.* \
+// RUN: -config="{CheckOptions: [{key: misc-header-include-cycle.IgnoredFilesList, value: 'header-include-cycle.self-e.hpp'}]}" \
+// RUN: -- -I%T/misc-header-include-cycle-headers -isystem %T/misc-header-include-cycle-headers/system \
+// RUN: --include %T/misc-header-include-cycle-headers/header-include-cycle.self-i.hpp | FileCheck %s \
+// RUN: -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error|note}}:"
+// RUN: rm -rf %T/misc-header-include-cycle-headers
+
+#ifndef MAIN_GUARD
+#define MAIN_GUARD
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth-d.hpp:3:10: warning: circular header file dependency detected while including 'header-include-cycle.first-d.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third-d.hpp:3:10: note: 'header-include-cycle.fourth-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second-d.hpp:3:10: note: 'header-include-cycle.third-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first-d.hpp:3:10: note: 'header-include-cycle.second-d.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.cpp:[[@LINE-5]]:10: note: 'header-include-cycle.first-d.hpp' included from here
+
+#include 
+// CHECK-MESSAGES: header-include-cycle.fourth.hpp:2:10: warning: circular header file dependency detected while including 'header-include-cycle.first.hpp', please check the include path [misc-header-include-cycle]
+// CHECK-MESSAGES: header-include-cycle.third.hpp:2:10: note: 'header-include-cycle.fourth.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.second.hpp:2:10: note: 'header-include-cycle.third.hpp' included from here
+// CHECK-MESSAGES: header-include-cycle.first.hpp:2:10: note: 'header-include-cycle.second.hpp' included from here
+//

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

2023-03-07 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev updated this revision to Diff 503114.
ABataev added a comment.

Address comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145514

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_non_rectangular_codegen.c
  openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c

Index: openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
===
--- /dev/null
+++ openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
@@ -0,0 +1,37 @@
+// RUN: %libomp-compile-and-run
+
+#define N 10
+int arr[N][N][N];
+
+int collapsed(int mp) {
+  int Cnt = 0;
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+arr[j][i][i0] = 1;
+++Cnt;
+  }
+}
+  }
+  return Cnt;
+}
+
+int test(int mp) {
+  int Failed = 0;
+  for (int j = 0; j < mp; ++j) {
+for (int i = 0; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+if ((i < j && arr[j][i][i0] == 0) || (i >= j && arr[j][i][i0] == 1))
+  continue;
+++Failed;
+  }
+}
+  }
+  return Failed;
+}
+
+int main() {
+  int Res = collapsed(N);
+  return Res == N * (N + 1) && test(N) == 0;
+}
Index: clang/test/OpenMP/for_non_rectangular_codegen.c
===
--- /dev/null
+++ clang/test/OpenMP/for_non_rectangular_codegen.c
@@ -0,0 +1,259 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+//
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+void collapsed(int mp) {
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < 10; ++i0) {
+;
+  }
+}
+  }
+}
+
+#endif // HEADER
+// CHECK-LABEL: define {{[^@]+}}@collapsed
+// CHECK-SAME: (i32 noundef [[MP:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[MP_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP2:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MIN:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTMIN_LESS_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTUPPER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLOWER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_3:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I0:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP15:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J19:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I20:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I021:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP0:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB2:[0-9]+]])
+// CHECK-NEXT:store i32 [[MP]], ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:store i32 [[TMP1]], ptr [[DOTCAPTURE_EXPR_]], align 4
+// CHECK-NEXT:store i32 0, ptr [[TMP]], align 4
+// CHECK-NEXT:[[TMP2:%.*]] = load i32, ptr [[TMP]], align 4
+// CHECK-NEXT:store i32 [[TMP2]], ptr [[DOTLB_MIN]], align 4
+// CHECK-NEXT:[[TMP3:%.*]] = load i32, ptr [[DOTCAPTURE_EXPR_]], align 4
+// CHECK-NEXT:[[SUB:%.*]] = sub nsw i32 [[TMP3]], 1
+// CHECK-NEXT:[[DIV:%.*]] = sdiv i32 [[SUB]], 1
+// CHECK-NEXT:

[PATCH] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

2023-03-07 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

LGTM for NVPTX.




Comment at: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp:134
 assert(TargetTriple.isOSBinFormatELF() && "ELF only");
+assert((!TargetTriple.isNVPTX() && !TargetTriple.isAMDGPU()) &&
+   "Device targets are not supported");

Nit: `assert(!(TargetTriple.isNVPTX() || TargetTriple.isAMDGPU()))` would be a 
bit easier to read. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145519

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

[PATCH] D141714: Fix ast print of variables with attributes

2023-03-07 Thread Giuliano Belinassi via Phabricator via cfe-commits

giulianobelinassi added a comment.

Hi, Aron.

Just to make myself clear: What I need to do is that the clang dumps for C 
files are also accepted by GCC as input.

Here is why I wanted to output the attribute on middle:

https://godbolt.org/z/6aPc6aWcz

As you can see, GCC complains of `__attributes__` output on the right side of 
functions with bodies.

But overall, perhaps what should be output in the dumps are the following 
(please check if you agree with me):

1- Attributes in variables should be output to the right side, as it was done 
before. That is:

  int var __attribute__((unused)) = 0;

2- Variables or functions declared with __declspec attributes should go to the 
left side, as does MSVC says is recommended. That is:

  __declspec(thread) int var = 0;
  __declspec(noinline) int f(void);
  __declspec(noinline) int f(void) { return 0; }

3- Functions __prototypes__ should have its attributes output to the right 
side, that means:

  int f(void) __attribute__((unused));

4- But attributes specified in function declaration __with body__ should go to 
the __left__ side __because GCC rejects outputing them on the right side__, as:

  __attribute__((unused)) int f(void) { return 0; }

-

The result of this choice would be that the following K function __as input__ 
(notice how it is not clear where the __attribute__ is being applied to:

  int f(i)
   __attribute__((unused)) int i;
  { return 0; }

would be dumped as:

  __attribute__((unused)) int f(i)
  int i;
  { return 0; }

But in practical terms, GCC would accept it without problems and it is clear 
where the __attribute__ is being applied to. Outputting to the right side has 
some ambiguity here, which is what I want to avoid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

2023-03-07 Thread Michael Francis via Phabricator via cfe-commits

francii updated this revision to Diff 503109.
francii added a comment.

Test case nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145021

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ibm-profiling.c
  clang/test/Driver/zos-profiling-error.c

Index: clang/test/Driver/zos-profiling-error.c
===
--- clang/test/Driver/zos-profiling-error.c
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: %clang 2>&1 -### --target=s390x-none-zos -pg -S %s | FileCheck -check-prefix=FAIL-PG-NAME %s
-// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
Index: clang/test/Driver/ibm-profiling.c
===
--- /dev/null
+++ clang/test/Driver/ibm-profiling.c
@@ -0,0 +1,27 @@
+// Check that -pg throws an error on z/OS.
+// RUN: %clang -### 2>&1 --target=s390x-none-zos -S -pg %s | FileCheck -check-prefix=FAIL-PG-NAME %s
+// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
+
+// Check that -p is still used when not linking on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 -S -p -S %s \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: argument unused during compilation: '-p'
+
+// Check precedence: -pg is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -pg -p %s \
+// RUN:| FileCheck --check-prefix=CHECK2 %s
+// CHECK2-NOT: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK2: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK2: "[[SYSROOT]]/usr/lib{{/|}}mcrt0.o"
+// CHECK2: "-L[[SYSROOT]]/lib/profiled"
+// CHECK2: "-L[[SYSROOT]]/usr/lib/profiled"
+
+// Check precedence: -p is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -p -pg %s \
+// RUN:| FileCheck --check-prefix=CHECK3 %s
+// CHECK3: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK3: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK3: "[[SYSROOT]]/usr/lib{{/|}}gcrt0.o"
+// CHECK3: "-L[[SYSROOT]]/lib/profiled"
+// CHECK3: "-L[[SYSROOT]]/usr/lib/profiled"
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6322,20 +6322,26 @@
 << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
-if (TC.getTriple().isOSAIX()) {
-  CmdArgs.push_back("-pg");
-} else if (!TC.getTriple().isOSOpenBSD()) {
+
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
+if (TC.getTriple().isOSzOS()) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
-if (TC.getTriple().isOSzOS()) {
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
+if (!(TC.getTriple().isOSAIX() || TC.getTriple().isOSOpenBSD())) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg)) {
+if (A->getOption().matches(options::OPT_p)) {
+  A->claim();
+  if (TC.getTriple().isOSAIX() && !Args.hasArgNoClaim(options::OPT_pg))
+CmdArgs.push_back("-pg");
+}
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -164,11 +164,12 @@
   }
 
   auto getCrt0Basename = [, IsArch32Bit] {
+Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg);
 // Enable gprofiling when "-pg" is specified.
-if (Args.hasArg(options::OPT_pg))
+if (A->getOption().matches(options::OPT_pg))
   return IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o";
 // Enable profiling when "-p" is specified.
-else if (Args.hasArg(options::OPT_p))
+else if (A->getOption().matches(options::OPT_p))
   return IsArch32Bit ? "mcrt0.o" : "mcrt0_64.o";
 else
   return IsArch32Bit ? "crt0.o" : "crt0_64.o";
@@ -271,7 +272,7 @@
 
 CmdArgs.push_back("-lc");
 
-if (Args.hasArg(options::OPT_p, options::OPT_pg)) {
+if (Args.hasArgNoClaim(options::OPT_p, options::OPT_pg)) {
   CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
"/lib/profiled"));

[PATCH] D145021: [Clang][AIX][p] Claim -p in front end

2023-03-07 Thread Michael Francis via Phabricator via cfe-commits

francii updated this revision to Diff 503107.
francii added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145021

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/ibm-profiling.c
  clang/test/Driver/zos-profiling-error.c

Index: clang/test/Driver/zos-profiling-error.c
===
--- clang/test/Driver/zos-profiling-error.c
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: %clang 2>&1 -### --target=s390x-none-zos -pg -S %s | FileCheck -check-prefix=FAIL-PG-NAME %s
-// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
Index: clang/test/Driver/ibm-profiling.c
===
--- /dev/null
+++ clang/test/Driver/ibm-profiling.c
@@ -0,0 +1,27 @@
+// Check that -pg throws an error on z/OS.
+// RUN: %clang -### 2>&1 --target=s390x-none-zos -S -pg %s | FileCheck -check-prefix=FAIL-PG-NAME %s
+// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
+
+// Check that -p is still used when not linking on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 -S -p -S %s \
+// RUN:   | FileCheck --check-prefix=CHECK3 %s
+// CHECK3-NOT: warning: argument unused during compilation: '-p'
+
+// Check precedence: -pg is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -pg -p %s \
+// RUN:| FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK: "[[SYSROOT]]/usr/lib{{/|}}mcrt0.o"
+// CHECK: "-L[[SYSROOT]]/lib/profiled"
+// CHECK: "-L[[SYSROOT]]/usr/lib/profiled"
+
+// Check precedence: -p is unused when passed first on AIX.
+// RUN: %clang -### 2>&1 --target=powerpc-ibm-aix7.1.0.0 --sysroot %S/Inputs/aix_ppc_tree -p -pg %s \
+// RUN:| FileCheck --check-prefix=CHECK2 %s
+// CHECK2: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK2: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK2: "[[SYSROOT]]/usr/lib{{/|}}gcrt0.o"
+// CHECK2: "-L[[SYSROOT]]/lib/profiled"
+// CHECK2: "-L[[SYSROOT]]/usr/lib/profiled"
+
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6322,20 +6322,26 @@
 << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
-if (TC.getTriple().isOSAIX()) {
-  CmdArgs.push_back("-pg");
-} else if (!TC.getTriple().isOSOpenBSD()) {
+
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
+if (TC.getTriple().isOSzOS()) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
-if (TC.getTriple().isOSzOS()) {
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
+if (!(TC.getTriple().isOSAIX() || TC.getTriple().isOSOpenBSD())) {
   D.Diag(diag::err_drv_unsupported_opt_for_target)
   << A->getAsString(Args) << TripleStr;
 }
   }
+  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg)) {
+if (A->getOption().matches(options::OPT_p)) {
+  A->claim();
+  if (TC.getTriple().isOSAIX() && !Args.hasArgNoClaim(options::OPT_pg))
+CmdArgs.push_back("-pg");
+}
+  }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -164,11 +164,12 @@
   }
 
   auto getCrt0Basename = [, IsArch32Bit] {
+Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg);
 // Enable gprofiling when "-pg" is specified.
-if (Args.hasArg(options::OPT_pg))
+if (A->getOption().matches(options::OPT_pg))
   return IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o";
 // Enable profiling when "-p" is specified.
-else if (Args.hasArg(options::OPT_p))
+else if (A->getOption().matches(options::OPT_p))
   return IsArch32Bit ? "mcrt0.o" : "mcrt0_64.o";
 else
   return IsArch32Bit ? "crt0.o" : "crt0_64.o";
@@ -271,7 +272,7 @@
 
 CmdArgs.push_back("-lc");
 
-if (Args.hasArg(options::OPT_p, options::OPT_pg)) {
+if (Args.hasArgNoClaim(options::OPT_p, options::OPT_pg)) {
   CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
"/lib/profiled"));

[PATCH] D145519: [SanitizerBinaryMetadata] Do not add to GPU code

2023-03-07 Thread Marco Elver via Phabricator via cfe-commits

melver created this revision.
melver added a reviewer: dvyukov.
Herald added subscribers: Enna1, hiraditya.
Herald added a project: All.
melver requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

SanitizerBinaryMetadata should only apply to to host code, and not GPU
code. Recently AMD GPU target code has experimental sanitizer support.

If we're compiling a mixed host/device source file, only add sanitizer
metadata to host code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145519

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp


Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -131,6 +131,8 @@
 IRB(M.getContext()) {
 // FIXME: Make it work with other formats.
 assert(TargetTriple.isOSBinFormatELF() && "ELF only");
+assert((!TargetTriple.isNVPTX() && !TargetTriple.isAMDGPU()) &&
+   "Device targets are not supported");
   }
 
   bool run();
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -1100,13 +1100,16 @@
   // NVPTX doesn't currently support sanitizers.  Bailing out here means
   // that e.g. -fsanitize=address applies only to host code, which is what we
   // want for now.
-  //
-  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
-  if (TC.getTriple().isNVPTX() ||
-  (TC.getTriple().isAMDGPU() &&
-   !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize,
- true)))
+  if (TC.getTriple().isNVPTX())
 return;
+  // AMDGPU sanitizer support is experimental and controlled by -fgpu-sanitize.
+  bool GPUSanitize = false;
+  if (TC.getTriple().isAMDGPU()) {
+if (!Args.hasFlag(options::OPT_fgpu_sanitize, 
options::OPT_fno_gpu_sanitize,
+  true))
+  return;
+GPUSanitize = true;
+  }
 
   // Translate available CoverageFeatures to corresponding clang-cc1 flags.
   // Do it even if Sanitizers.empty() since some forms of coverage don't 
require
@@ -1143,20 +1146,22 @@
   addSpecialCaseListOpt(Args, CmdArgs, "-fsanitize-coverage-ignorelist=",
 CoverageIgnorelistFiles);
 
-  // Translate available BinaryMetadataFeatures to corresponding clang-cc1
-  // flags. Does not depend on any other sanitizers.
-  const std::pair BinaryMetadataFlags[] = {
-  std::make_pair(BinaryMetadataCovered, "covered"),
-  std::make_pair(BinaryMetadataAtomics, "atomics"),
-  std::make_pair(BinaryMetadataUAR, "uar")};
-  for (const auto  : BinaryMetadataFlags) {
-if (BinaryMetadataFeatures & F.first)
-  CmdArgs.push_back(
-  Args.MakeArgString("-fexperimental-sanitize-metadata=" + F.second));
+  if (!GPUSanitize) {
+// Translate available BinaryMetadataFeatures to corresponding clang-cc1
+// flags. Does not depend on any other sanitizers. Unsupported on GPUs.
+const std::pair BinaryMetadataFlags[] = {
+std::make_pair(BinaryMetadataCovered, "covered"),
+std::make_pair(BinaryMetadataAtomics, "atomics"),
+std::make_pair(BinaryMetadataUAR, "uar")};
+for (const auto  : BinaryMetadataFlags) {
+  if (BinaryMetadataFeatures & F.first)
+CmdArgs.push_back(
+Args.MakeArgString("-fexperimental-sanitize-metadata=" + 
F.second));
+}
+addSpecialCaseListOpt(Args, CmdArgs,
+  "-fexperimental-sanitize-metadata-ignorelist=",
+  BinaryMetadataIgnorelistFiles);
   }
-  addSpecialCaseListOpt(Args, CmdArgs,
-"-fexperimental-sanitize-metadata-ignorelist=",
-BinaryMetadataIgnorelistFiles);
 
   if (TC.getTriple().isOSWindows() && needsUbsanRt()) {
 // Instruct the code generator to embed linker directives in the object 
file


Index: llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -131,6 +131,8 @@
 IRB(M.getContext()) {
 // FIXME: Make it work with other formats.
 assert(TargetTriple.isOSBinFormatELF() && "ELF only");
+assert((!TargetTriple.isNVPTX() && !TargetTriple.isAMDGPU()) &&
+   "Device targets are not supported");
   }
 
   bool run();
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@

[PATCH] D143418: [libclang] Add API to override preamble storage path

2023-03-07 Thread Igor Kushnir via Phabricator via cfe-commits

vedgy added a comment.

In D143418#4175628 , @aaron.ballman 
wrote:

> Hmmm, don't relaxed loads and stores still have the potential to be racey? I 
> thought you needed a release on the store and an acquire on the load (or 
> sequential consistency), but this is definitely not my area of expertise.

The acquire/release semantics is needed if the atomic variable guards access to 
another non-atomic variable or the atomic pointer guards access to its 
non-atomic pointed-to value. The relaxed order means no guarantees about this 
variable's interaction with other atomic or non-atomic variables. But even the 
relaxed order prevents data races on the variable itself, which is sufficient 
here.

>> The option can also be added to `CXIndexOptions` in order to allow setting 
>> its initial value reliably (not worrying whether it could be used before the 
>> setter gets called after index construction).
>>
>> Is adding both the setter and the `CXIndexOptions` member OK or would you 
>> prefer only one of these two?
>
> It's a bit odd to me that we're deprecating the global option setters to turn 
> around and add a new global option setter. The old interfaces are not thread 
> safe and the new one is thread safe, so I get why the desire exists and is 
> reasonable, but (personally) I'd like to get rid of the option state setters 
> entirely someday.

I also thought about the deprecated old interfaces today. 
`clang_CXIndex_setGlobalOptions()` could be undeprecated by similarly making 
`CIndexer::Options` atomic. Safely setting `std::string` members would require 
a mutex.

> What I was envisioning was that the index creation would get global options 
> and if we wanted per-TU options, we'd add an interface akin to 
> `clang_parseTranslationUnit()` which takes another options struct for per-TU 
> options. (Perhaps the default values for those options come from the global 
> options -- e.g., `DisplayDiagnostics` is set at the global level but could be 
> overridden for a single TU). Do you have thoughts about that approach?

This would allow to specify whether to store each individual preamble in 
memory, which is more specific than necessary for my use case. This would shift 
the burden of passing around the `StorePreamblesInMemory` value to libclang 
users. Certainly more difficult to implement both in libclang and in KDevelop.

Advantages of getting rid of the option state setters entirely and passing 
options to per-TU APIs:

1. More flexibility (per-TU option specification).
2. Possibly more efficient (no need for atomics and mutexes).
3. Clearer to API users which TUs the modified options apply to and which TUs 
(created earlier) keep the original options.
4. Less risk of inconsistencies and bugs in libclang. Such as somehow passing a 
new option value to an old TU with unpredictable consequences.

Do the listed advantages explain your preference?

I am not sure what I would prefer from a hypothetical standpoint of a libclang 
maintainer. And you definitely have more experience in this area. So I won't 
argue for the index option state setters.

I'll probably implement the uncontroversial `CXIndexOptions` member first. And 
then, if I think implementing it is worth the effort, continue discussing 
`clang_parseTranslationUnitWithOptions()`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

2023-03-07 Thread Andrew Gozillon 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 rGe002a38b20e3: [Flang][OpenMP][MLIR][Driver][bbc] Add 
-fopenmp-is-device flag to Flang -fc1 … (authored by agozillon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,14 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE
+!RUN: bbc -fopenmp -emit-fir -o - %s | FileCheck %s --check-prefix=HOST
+!RUN: bbc -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device

[clang] e002a38 - [Flang][OpenMP][MLIR][Driver][bbc] Add -fopenmp-is-device flag to Flang -fc1 & the bbc tool, and omp.is_device attribute

2023-03-07 Thread Andrew Gozillon via cfe-commits


Author: Andrew Gozillon
Date: 2023-03-07T12:57:58-06:00
New Revision: e002a38b20e3ac40aecbbfa0774f8ba7b9690b0c

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

LOG: [Flang][OpenMP][MLIR][Driver][bbc] Add -fopenmp-is-device flag to Flang 
-fc1 & the bbc tool, and omp.is_device attribute

Adds the -fopenmp-is-device flag to bbc and Flang's -fc1 (but not flang-new) 
and in addition adds an omp.is_device attribute onto the module when fopenmp is 
passed, this is a boolean attribute that is set to false or true dependent on 
if fopenmp-is-device is specified alongside the fopenmp flag on the commandline 
when invoking flang or bbc.

Reviewers:
awarzynski
kiranchandramohan

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

Added: 
flang/test/Lower/OpenMP/omp-is-device.f90

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/LangOptions.def
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/test/Driver/driver-help.f90
flang/tools/bbc/bbc.cpp
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d61083f8b538a..b8a12660b32b7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6530,7 +6530,7 @@ def fno_cuda_host_device_constexpr : Flag<["-"], 
"fno-cuda-host-device-constexpr
 
 def fopenmp_is_device : Flag<["-"], "fopenmp-is-device">,
   HelpText<"Generate code only for an OpenMP target device.">,
-  Flags<[CC1Option, NoDriverOption]>;
+  Flags<[CC1Option, FC1Option, NoDriverOption]>;
 def fopenmp_host_ir_file_path : Separate<["-"], "fopenmp-host-ir-file-path">,
   HelpText<"Path to the IR file produced by the frontend for the host.">,
   Flags<[CC1Option, NoDriverOption]>;

diff  --git a/flang/include/flang/Frontend/LangOptions.def 
b/flang/include/flang/Frontend/LangOptions.def
index 9471beda8e05a..3648e09154520 100644
--- a/flang/include/flang/Frontend/LangOptions.def
+++ b/flang/include/flang/Frontend/LangOptions.def
@@ -34,6 +34,8 @@ LANGOPT(NoSignedZeros, 1, false)
 LANGOPT(AssociativeMath, 1, false)
 /// Allow division operations to be reassociated
 LANGOPT(ReciprocalMath, 1, false)
+/// Generate code only for OpenMP target device
+LANGOPT(OpenMPIsDevice, 1, false)
 
 #undef LANGOPT
 #undef ENUM_LANGOPT

diff  --git a/flang/lib/Frontend/CompilerInvocation.cpp 
b/flang/lib/Frontend/CompilerInvocation.cpp
index 4fb1eb8ef902c..d043df3d7 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -670,6 +670,10 @@ static bool parseDialectArgs(CompilerInvocation , 
llvm::opt::ArgList ,
   if (args.hasArg(clang::driver::options::OPT_fopenmp)) {
 res.getFrontendOpts().features.Enable(
 Fortran::common::LanguageFeature::OpenMP);
+
+if (args.hasArg(clang::driver::options::OPT_fopenmp_is_device)) {
+  res.getLangOpts().OpenMPIsDevice = 1;
+}
   }
 
   // -pedantic

diff  --git a/flang/lib/Frontend/FrontendActions.cpp 
b/flang/lib/Frontend/FrontendActions.cpp
index d2d3ab4cf1e35..d6c06e4c370d0 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -177,6 +177,13 @@ bool CodeGenAction::beginSourceFileAction() {
 
   // Fetch module from lb, so we can set
   mlirModule = std::make_unique(lb.getModule());
+
+  if (ci.getInvocation().getFrontendOpts().features.IsEnabled(
+  Fortran::common::LanguageFeature::OpenMP)) {
+mlir::omp::OpenMPDialect::setIsDevice(
+*mlirModule, ci.getInvocation().getLangOpts().OpenMPIsDevice);
+  }
+
   setUpTargetMachine();
   const llvm::DataLayout  = tm->createDataLayout();
   setMLIRDataLayout(*mlirModule, dl);

diff  --git a/flang/test/Driver/driver-help.f90 
b/flang/test/Driver/driver-help.f90
index 7fdce71190f61..ca5d7cf42a3c3 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -138,6 +138,7 @@
 ! HELP-FC1-NEXT: -fno-signed-zeros  Allow optimizations that ignore the 
sign of floating point zeros
 ! HELP-FC1-NEXT: -fno-stack-arrays  Allocate array temporaries on the heap 
(default)
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
+! HELP-FC1-NEXT: -fopenmp-is-device Generate code only for an OpenMP 
target device.
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate 
parallel code.
 ! HELP-FC1-NEXT: -fpass-plugin= Load pass plugin from a dynamic 
shared object file (only with new pass manager).
 ! HELP-FC1-NEXT: -freciprocal-math  Allow division operations to be 
reassociated

diff  --git a/flang/test/Lower/OpenMP/omp-is-device.f90

[PATCH] D145449: [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

> LLVM_ENABLE_PROJECTS is automatically forwarded from stage 1 builds to
> stage 2 builds, so setting FUCHSIA_ENABLE_LLDB has no effect on
> two-stage builds.

That shouldn't be the case, do you know where it's being passed through?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145449

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

[PATCH] D145449: [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Haowei Wu via Phabricator via cfe-commits

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

LGTM, but please wait @phosek to approve it.




Comment at: clang/cmake/caches/Fuchsia.cmake:167
+if(FUCHSIA_ENABLE_LLDB)
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+endif()

mysterymath wrote:
> haowei wrote:
> > You probably need a `string(REPLACE ";" "|" value 
> > "${_FUCHSIA_ENABLE_PROJECTS}")` after append a value. As by default cmake 
> > use `|` to separate items.
> > But do you actually need a `_FUCHSIA_ENABLE_PROJECTS` variable, couldn't 
> > you just append `lldb` at the end of `LLVM_ENABLE_PROJECTS` here?
> > You probably need a `string(REPLACE ";" "|" value 
> > "${_FUCHSIA_ENABLE_PROJECTS}")` after append a value. As by default cmake 
> > use `|` to separate items.
> Hm? CMake's list separator is the semicolon: 
> https://cmake.org/cmake/help/latest/command/list.html
> 
> > But do you actually need a `_FUCHSIA_ENABLE_PROJECTS` variable, couldn't 
> > you just append `lldb` at the end of `LLVM_ENABLE_PROJECTS` here?
> `(list APPEND ...)` doesn't work on cache variables. If I were to use the 
> (set CACHE FORCE) idiom instead, it would append lldb each time a configure 
> occurs. The idea here is to have FUCHSIA_ENABLE_LLDB affect the defaults, but 
> to respect whatever's already in the CMake cache otherwise.
> Hm? CMake's list separator is the semicolon: 
> https://cmake.org/cmake/help/latest/command/list.html
> 

I see, I am wrong. I thought it is similar to passthrough cmake variables to 
the next stage. 

> (list APPEND ...) doesn't work on cache variables. If I were to use the (set 
> CACHE FORCE) idiom instead, it would append lldb each time a configure 
> occurs. The idea here is to have FUCHSIA_ENABLE_LLDB affect the defaults, but 
> to respect whatever's already in the CMake cache otherwise.

I see. I didn't know that. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145449

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

[PATCH] D145238: [NVPTX] Expose LDU builtins

2023-03-07 Thread Artem Belevich via Phabricator via cfe-commits

tra added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:18267-18271
+auto HalfSupport = HasHalfSupport(BuiltinID);
+if (!HalfSupport.first) {
+  CGM.Error(E->getExprLoc(),
+HalfSupport.second.append(" requires native half type 
support.")
+.c_str());

I think we can simplify it all further.

```
auto HasHalfSupport = [&](unsigned BuiltinID) {
auto  = getContext();
return Context.getLangOpts().NativeHalfType || 
!Context.getTargetInfo().useFP16ConversionIntrinsics();
}
...

if (!HasHalfSupport(BuiltinID)) {
  CGM.Error(E->getExprLoc(),   getContext().BuiltinInfo.getName(BuiltinID) 
+ " requires native half type support.");

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145238

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

[clang-tools-extra] 284fcbb - [clang-tidy][NFC] Fix link in RelaseNotes

2023-03-07 Thread Piotr Zegar via cfe-commits


Author: Piotr Zegar
Date: 2023-03-07T18:39:45Z
New Revision: 284fcbb95a899228b99728e9d8960fc7a6c5c6ac

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

LOG: [clang-tidy][NFC] Fix link in RelaseNotes

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index bd38cf9d6a62e..426b92ab950cc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,7 +114,7 @@ New checks
 
   Warns when lambda specify a capture default and capture ``this``.
 
-- New :doc: `llvmlibc-inline-function-decl
+- New :doc:`llvmlibc-inline-function-decl
   ` check.
 
   Checks that all implicit and explicit inline functions in header files are



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

[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

2023-03-07 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL updated this revision to Diff 503092.
PiotrZSL added a comment.

Ping, Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144347

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
  clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp
@@ -0,0 +1,257 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-forward-usage %t -- -- -fno-delayed-template-parsing
+
+namespace std {
+
+struct false_type {
+  static constexpr bool value = false;
+};
+
+struct true_type {
+  static constexpr bool value = true;
+};
+
+template 
+struct is_lvalue_reference : false_type {};
+
+template 
+struct is_lvalue_reference : true_type {};
+
+template 
+struct remove_reference {
+  using type = T;
+};
+
+template 
+struct remove_reference {
+  using type = T;
+};
+
+template 
+struct remove_reference {
+  using type = T;
+};
+
+template 
+using remove_reference_t = typename remove_reference::type;
+
+template 
+constexpr T&& forward(typename std::remove_reference::type& t) noexcept {
+  return static_cast(t);
+}
+
+template 
+constexpr T&& forward(typename std::remove_reference::type&& t) noexcept {
+  static_assert(!std::is_lvalue_reference::value, "Can't forward an rvalue as an lvalue.");
+  return static_cast(t);
+}
+
+template 
+constexpr typename std::remove_reference::type&& move(T&& t) noexcept {
+  return static_cast::type&&>(t);
+}
+
+}
+
+struct TestType {
+int value = 0U;
+};
+
+struct TestTypeEx : TestType {
+};
+
+template
+void test(Args&&...) {}
+
+
+template
+void testCorrectForward(T&& value) {
+test(std::forward(value));
+}
+
+template
+void testForwardVariadicTemplate(Args&& ...args) {
+test(std::forward(args)...);
+}
+
+void callAll() {
+TestType type1;
+const TestType type2;
+testCorrectForward(type1);
+testCorrectForward(type2);
+testCorrectForward(std::move(type1));
+testCorrectForward(std::move(type2));
+testForwardVariadicTemplate(type1, TestType(), std::move(type2));
+}
+
+void testExplicit(TestType& value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(std::move(value));{{$}}
+}
+
+void testExplicit2(const TestType& value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(std::move(value));{{$}}
+}
+
+void testExplicit3(TestType value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(std::move(value));{{$}}
+}
+
+void testExplicit4(TestType&& value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(std::move(value));{{$}}
+}
+
+void testExplicit5(const TestType&& value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(std::move(value));{{$}}
+}
+
+void testExplicit6(TestType&& value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(value);{{$}}
+}
+
+void testExplicit7(TestType value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: remove redundant use of 'std::forward' as it is unnecessary in this context [readability-forward-usage]
+// CHECK-FIXES: {{^}}test(value);{{$}}
+}
+
+void testExplicit8(TestType value) {
+test(std::forward(value));
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::move' instead of 'std::forward' here, as it more accurately reflects the intended purpose

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

2023-03-07 Thread Johannes Doerfert via Phabricator via cfe-commits

jdoerfert added inline comments.



Comment at: openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c:33
+  collapsed(N);
+  return test(N);
+}

should we test the count?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145514

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

[PATCH] D145449: [Fuchsia] Add LLDB options to stage 1 cmake.

2023-03-07 Thread Daniel Thornburgh via Phabricator via cfe-commits

mysterymath added inline comments.



Comment at: clang/cmake/caches/Fuchsia.cmake:167
+if(FUCHSIA_ENABLE_LLDB)
+  list(APPEND _FUCHSIA_ENABLE_PROJECTS lldb)
+endif()

haowei wrote:
> You probably need a `string(REPLACE ";" "|" value 
> "${_FUCHSIA_ENABLE_PROJECTS}")` after append a value. As by default cmake use 
> `|` to separate items.
> But do you actually need a `_FUCHSIA_ENABLE_PROJECTS` variable, couldn't you 
> just append `lldb` at the end of `LLVM_ENABLE_PROJECTS` here?
> You probably need a `string(REPLACE ";" "|" value 
> "${_FUCHSIA_ENABLE_PROJECTS}")` after append a value. As by default cmake use 
> `|` to separate items.
Hm? CMake's list separator is the semicolon: 
https://cmake.org/cmake/help/latest/command/list.html

> But do you actually need a `_FUCHSIA_ENABLE_PROJECTS` variable, couldn't you 
> just append `lldb` at the end of `LLVM_ENABLE_PROJECTS` here?
`(list APPEND ...)` doesn't work on cache variables. If I were to use the (set 
CACHE FORCE) idiom instead, it would append lldb each time a configure occurs. 
The idea here is to have FUCHSIA_ENABLE_LLDB affect the defaults, but to 
respect whatever's already in the CMake cache otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145449

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

[PATCH] D145509: [HIP] Fix temporary files

2023-03-07 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

LGTM, but we should probably get someone familiar with macos to chime in, just 
in case there may be some reason behind macos using temp directories here.

> This change is OK for MacOS as lipo does not requires specific

I'm curious why lipo has been singled out. Is that the only use case that ends 
up using this path?




Comment at: clang/lib/Driver/Driver.cpp:5567-5572
 if (MultipleArchs && !BoundArch.empty()) {
-  TmpName = GetTemporaryDirectory(Prefix);
-  llvm::sys::path::append(TmpName,
-  Twine(Prefix) + "-" + BoundArch + "." + Suffix);
+  TmpName =
+  GetTemporaryPath((Twine(Prefix) + "-" + BoundArch).str(), Suffix);
 } else {
   TmpName = GetTemporaryPath(Prefix, Suffix);
 }

This could now be shortened to:
```
  TmpName = (MultipleArchs && !BoundArch.empty()) 
  ? GetTemporaryPath((Twine(Prefix) + "-" + BoundArch).str(), Suffix)
  : GetTemporaryPath(Prefix, Suffix);
```



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

https://reviews.llvm.org/D145509

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

[PATCH] D145517: MSVC: support version preference with search

2023-03-07 Thread Saleem Abdulrasool via Phabricator via cfe-commits

compnerd created this revision.
compnerd added reviewers: mstorsjo, rnk.
Herald added a subscriber: hiraditya.
Herald added a project: All.
compnerd requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added projects: clang, LLVM.

Extend the logic for the WinSDK and UCRT handling to prefer a user
specified version of the VisualC++ tools and Windows SDK.  This allows
us to now perform the regular search for the installation but select the
exact version of the SDK or VC++ tools to override the latest version.
Similar to the other flags controlling this behaviour, if the user
specifies a value, we will not perform validation on the input and will
attempt to prefer that, particularly in the case of VisualC++ tools
where no fallback occurs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145517

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  lld/COFF/Driver.cpp
  llvm/include/llvm/WindowsDriver/MSVCPaths.h
  llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
  llvm/lib/WindowsDriver/MSVCPaths.cpp

Index: llvm/lib/WindowsDriver/MSVCPaths.cpp
===
--- llvm/lib/WindowsDriver/MSVCPaths.cpp
+++ llvm/lib/WindowsDriver/MSVCPaths.cpp
@@ -609,8 +609,9 @@
   return false;
 }
 
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout ) {
+bool findVCToolChainViaSetupConfig(vfs::FileSystem ,
+   std::optional VCToolsVersion,
+   std::string , ToolsetLayout ) {
 #if !defined(USE_MSVC_SETUP_API)
   return false;
 #else
@@ -677,17 +678,24 @@
   std::string VCRootPath;
   convertWideToUTF8(std::wstring(VCPathWide), VCRootPath);
 
-  SmallString<256> ToolsVersionFilePath(VCRootPath);
-  sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
-"Microsoft.VCToolsVersion.default.txt");
+  std::string ToolsVersion;
+  if (VCToolsVersion.has_value()) {
+ToolsVersion = *VCToolsVersion;
+  } else {
+SmallString<256> ToolsVersionFilePath(VCRootPath);
+sys::path::append(ToolsVersionFilePath, "Auxiliary", "Build",
+  "Microsoft.VCToolsVersion.default.txt");
+
+auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
+if (!ToolsVersionFile)
+  return false;
+
+ToolsVersion = ToolsVersionFile->get()->getBuffer().rtrim();
+  }
 
-  auto ToolsVersionFile = MemoryBuffer::getFile(ToolsVersionFilePath);
-  if (!ToolsVersionFile)
-return false;
 
   SmallString<256> ToolchainPath(VCRootPath);
-  sys::path::append(ToolchainPath, "Tools", "MSVC",
-ToolsVersionFile->get()->getBuffer().rtrim());
+  sys::path::append(ToolchainPath, "Tools", "MSVC", ToolsVersion);
   auto Status = VFS.status(ToolchainPath);
   if (!Status || !Status->isDirectory())
 return false;
Index: llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
===
--- llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
+++ llvm/lib/ExecutionEngine/Orc/COFFVCRuntimeSupport.cpp
@@ -160,7 +160,7 @@
   if (!findVCToolChainViaCommandLine(*VFS, std::nullopt, std::nullopt,
  std::nullopt, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaEnvironment(*VFS, VCToolChainPath, VSLayout) &&
-  !findVCToolChainViaSetupConfig(*VFS, VCToolChainPath, VSLayout) &&
+  !findVCToolChainViaSetupConfig(*VFS, {}, VCToolChainPath, VSLayout) &&
   !findVCToolChainViaRegistry(VCToolChainPath, VSLayout))
 return make_error("Couldn't find msvc toolchain.",
inconvertibleErrorCode());
Index: llvm/include/llvm/WindowsDriver/MSVCPaths.h
===
--- llvm/include/llvm/WindowsDriver/MSVCPaths.h
+++ llvm/include/llvm/WindowsDriver/MSVCPaths.h
@@ -90,11 +90,15 @@
ToolsetLayout );
 
 // Query the Setup Config server for installs, then pick the newest version
-// and find its default VC toolchain.
+// and find its default VC toolchain. If `VCToolsVersion` is specified, that
+// version is preferred over the latest version.
+//
 // This is the preferred way to discover new Visual Studios, as they're no
 // longer listed in the registry.
-bool findVCToolChainViaSetupConfig(vfs::FileSystem , std::string ,
-   ToolsetLayout );
+bool
+findVCToolChainViaSetupConfig(vfs::FileSystem ,
+  std::optional VCToolsVersion,
+  std::string , ToolsetLayout );
 
 // Look in the registry for Visual Studio installs, and use that to get
 // a toolchain path. VS2017 and newer don't get added to the registry.
Index: lld/COFF/Driver.cpp
===
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

2023-03-07 Thread Kiran Chandramohan via Phabricator via cfe-commits

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.

LG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[clang] 9b28954 - Use explicit target in clang/test/Preprocessor/directives_asm.S

2023-03-07 Thread John Brawn via cfe-commits


Author: John Brawn
Date: 2023-03-07T18:06:41Z
New Revision: 9b2895469badad470dca186801fe025c52f5364c

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

LOG: Use explicit target in clang/test/Preprocessor/directives_asm.S

This prevents the test from failing when the default target doesn't
support the .warning directive.

Added: 


Modified: 
clang/test/Preprocessor/directives_asm.S

Removed: 




diff  --git a/clang/test/Preprocessor/directives_asm.S 
b/clang/test/Preprocessor/directives_asm.S
index 55e71d621e34..a384b911005f 100644
--- a/clang/test/Preprocessor/directives_asm.S
+++ b/clang/test/Preprocessor/directives_asm.S
@@ -1,4 +1,5 @@
-// RUN: %clang -c %s -o /dev/null 2>&1 | FileCheck %s
+// REQUIRES: x86-registered-target
+// RUN: %clang --target=i386-pc-linux -c %s -o /dev/null 2>&1 | FileCheck %s
 
 // Check that preprocessor directives are recognised as such, but lines 
starting
 // with a # that aren't directives are instead treated as comments.
@@ -6,7 +7,7 @@
 #define MACRO .warning "This is a macro"
 MACRO
 
-// CHECK: directives_asm.S:7:9: warning: This is a macro
+// CHECK: directives_asm.S:8:9: warning: This is a macro
 
 #not a preprocessing directive
 
@@ -16,7 +17,7 @@
 
 .warning "line number should not change"
 
-// CHECK: directives_asm.S:17:9: warning: line number should not change
+// CHECK: directives_asm.S:18:9: warning: line number should not change
 
 #line 100
 



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

[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing

2023-03-07 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL updated this revision to Diff 503082.
PiotrZSL added a comment.

Ping, Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144206

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
   a = h;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 
'DerivedThatOverridesH' to 'Base' discards override 'h'
 }
+
+struct BaseA {
+virtual ~BaseA() {}
+virtual void foo() {}
+
+int i;
+};
+
+struct BaseB : virtual BaseA {
+virtual void foo() {}
+};
+
+struct ClassWithVirtualBases : BaseB {
+  ClassWithVirtualBases(const BaseB& other) : BaseA(other), BaseB(other) {}
+  ClassWithVirtualBases(const ClassWithVirtualBases& other) : BaseA(other), 
BaseB(other) {}
+};
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,10 @@
   ` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were 
added.
 
+- Fixed a false positive in :doc:`cppcoreguidelines-slicing
+  ` check when warning would be
+  emitted in constructor for virtual base class initialization.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
   const auto HasTypeDerivedFromBaseDecl =
   anyOf(hasType(IsDerivedFromBaseDecl),
 hasType(references(IsDerivedFromBaseDecl)));
-  const auto IsWithinDerivedCtor =
-  hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl";
+  const auto IsCallToBaseClass = hasParent(cxxConstructorDecl(
+  ofClass(isSameOrDerivedFrom(equalsBoundNode("DerivedDecl"))),
+  hasAnyConstructorInitializer(allOf(
+  isBaseInitializer(), withInitializer(equalsBoundNode("Call"));
 
   // Assignment slicing: "a = b;" and "a = std::move(b);" variants.
   const auto SlicesObjectInAssignment =
-  callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
+  callExpr(expr().bind("Call"),
+   callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
   isMoveAssignmentOperator()),
 OfBaseClass)),
hasArgument(1, HasTypeDerivedFromBaseDecl));
@@ -53,17 +56,17 @@
   // Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of
   // slicing the letter will create a temporary and therefore call a ctor.
   const auto SlicesObjectInCtor = cxxConstructExpr(
+  expr().bind("Call"),
   hasDeclaration(cxxConstructorDecl(
   anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),
   hasArgument(0, HasTypeDerivedFromBaseDecl),
   // We need to disable matching on the call to the base copy/move
   // constructor in DerivedDecl's constructors.
-  unless(IsWithinDerivedCtor));
+  unless(IsCallToBaseClass));
 
-  Finder->addMatcher(traverse(TK_AsIs, expr(anyOf(SlicesObjectInAssignment,
-  SlicesObjectInCtor))
-   .bind("Call")),
- this);
+  Finder->addMatcher(
+  traverse(TK_AsIs, expr(SlicesObjectInAssignment).bind("Call")), this);
+  Finder->addMatcher(traverse(TK_AsIs, SlicesObjectInCtor), this);
 }
 
 /// Warns on methods overridden in DerivedDecl with respect to BaseDecl.


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
   a = h;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
 }
+
+struct BaseA {
+virtual ~BaseA() {}
+virtual void foo() {}
+
+int i;
+};
+
+struct BaseB : virtual BaseA {
+virtual void foo() {}
+};
+
+struct ClassWithVirtualBases : BaseB {
+  ClassWithVirtualBases(const BaseB& other) : BaseA(other), BaseB(other) {}
+  ClassWithVirtualBases(const ClassWithVirtualBases& other) : BaseA(other), BaseB(other) {}
+};
+
Index:

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-03-07 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

We're seeing new clang crashes that bisect to this commit, with modules only.

I have it mostly-reduced and will post shortly, trying to see if I can simplify 
any further (since modules reproducers are a pain).

Meanwhile, the assert/stack in case it's already useful:

  assertion failed at 
third_party/llvm/llvm-project/clang/lib/Sema/SemaTemplateInstantiate.cpp:4065 
in llvm::PointerUnion 
*clang::LocalInstantiationScope::findInstantiationOf(const Decl *): 
isa(D) && "declaration not instantiated in this scope"
  *** Check failure stack trace: ***
  @ 0x5609e4f16f44  __assert_fail
  @ 0x5609e1894234  
clang::LocalInstantiationScope::findInstantiationOf()
  @ 0x5609e18d073c  clang::Sema::FindInstantiatedDecl()
  @ 0x5609e18a99d0  clang::TreeTransform<>::TransformLambdaExpr()
  @ 0x5609e189dbee  (anonymous 
namespace)::TemplateInstantiator::TransformLambdaExpr()
  @ 0x5609e1892442  clang::TreeTransform<>::TransformExprs()
  @ 0x5609e189a71a  clang::TreeTransform<>::TransformCallExpr()
  @ 0x5609e189097a  clang::TreeTransform<>::TransformStmt()
  @ 0x5609e18afa54  clang::TreeTransform<>::TransformCompoundStmt()
  @ 0x5609e1890902  clang::Sema::SubstStmt()
  @ 0x5609e18e31df  clang::Sema::InstantiateFunctionDefinition()
  @ 0x5609e18e5ed2  clang::Sema::PerformPendingInstantiations()
  @ 0x5609e0fcad44  clang::Sema::ActOnEndOfTranslationUnitFragment()
  @ 0x5609e0fcbb66  clang::Sema::ActOnEndOfTranslationUnit()
  @ 0x5609e0d298e6  clang::Parser::ParseTopLevelDecl()
  @ 0x5609e0d2388e  clang::ParseAST()
  @ 0x5609e0a647c3  clang::FrontendAction::Execute()
  @ 0x5609e09d81ad  clang::CompilerInstance::ExecuteAction()
  @ 0x5609dfa05b08  clang::ExecuteCompilerInvocation()
  @ 0x5609df9f99f1  cc1_main()
  @ 0x5609df9f5d28  ExecuteCC1Tool()
  @ 0x5609e0b868be  llvm::function_ref<>::callback_fn<>()
  @ 0x5609e4d9ec35  llvm::CrashRecoveryContext::RunSafely()
  @ 0x5609e0b86103  clang::driver::CC1Command::Execute()
  @ 0x5609e0b44166  clang::driver::Compilation::ExecuteCommand()
  @ 0x5609e0b4448f  clang::driver::Compilation::ExecuteJobs()
  @ 0x5609e0b63e70  clang::driver::Driver::ExecuteCompilation()
  @ 0x5609df9f4ee7  clang_main()
  @ 0x5609df9f1bc4  main
  @ 0x7fad2cda4633  __libc_start_main
  @ 0x5609df9f1b2a  _start


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D141714: Fix ast print of variables with attributes

2023-03-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D141714#4175263 , 
@giulianobelinassi wrote:

> Hi, Alan. Thanks for your review again!
>
> With regard to middle, the patch sent in D145269 
>  may answer your questions. Basically 
> functions like:
>
>   int* f(void) __attribute__((unused));
>
> would be output as
>
>   int* __attribute__((unused)) f(void);
>
> if SIDE_MIDDLE is specified.

That would be a pretty unique location for those attributes to go and there are 
some attributes that can't go there. For example: 
https://godbolt.org/z/fo5cnEGTY

> The case I want to avoid is:
>
>   int f(void) __attribute__((unused))
> int i;
>   { return 0; }

Did you mean the example to be a K C function instead? If so, then this would 
apply the attribute to the function in Clang.

> Which is not clear where should the attribute be applied to. A left-to-right 
> parser that accepts attributes on the right side of a function declaration 
> may apply this to f, but if it don't then it will be applied to i. This is 
> why GCC rejects this kind of attribute output on the right side of functions.
> Hence, I thought it would be better to always output variable as
>
>   int __attribute__((unused)) var;

That will subtly change program semantics in Clang though, as now the attribute 
pretty prints to the parameter declaration (I'm still assuming your example was 
intended to be a K C function definition).

> when possible. I also found that __declspec attributes is also accepted this 
> way. But if that changes the meaning of things (I looked into generated 
> assembly and could not find such case) then I think dumping it on the right 
> side may be a better option, and we only have to be careful with functions 
> declarations, those being dumped as.
>
>   __attribute__((unused)) f(void) {}
>
> -
>
> As for changing the `__declspec` locations, I thought it was fine as it is 
> also accepted by clang. But looking at MSVC documentation it says that 
> outputing it at the begining is recommended so I think it may be better to 
> always output __declspecs on the left side?

That's where I'm most used to seeing it.

> -
>
> Can you explain a bit more about what warning you're seeing and under what 
> condition?
>
> Basically if you change this test in order for it to intentionally fail 
> because of output mismatch, this warning is generated.

Hmmm, I'm still not certain I understand what's going on. What surprises me is 
that the test code is ` __block StructWithCopyConstructor s(5);` but it pretty 
prints as `StructWithCopyConstructor __attribute__((blocks("byref"))) s(5);` 
which doesn't compile at all. That doesn't seem to be new behavior in your 
patch (and this particular attribute has no documentation or existing test 
coverage), but I think the fact that the keyword spelling is dropped and the 
attribute is moved helps point out an issue -- keyword attributes are special 
and their syntactic position is important, so maybe those should not shift at 
all.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D130303: Handle template parameter-dependent bit field widths in libclang

2023-03-07 Thread Collin Baker via Phabricator via cfe-commits

collinbaker added a comment.

@dexonsmith can you weigh in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130303

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

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

2023-03-07 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:642
  T __builtin_elementwise_log10(T x)  return the base 10 logarithm of x 
   floating point types
+ T __builtin_elementwise_exp(T x)returns the base-e exponential, 
or e^x, of the specified value   floating point types
+ T __builtin_elementwise_exp2(T x)   returns the base-2 exponential, 
or 2^x, of the specified value   floating point types

beanz wrote:
> erichkeane wrote:
> > The naming difference between these is a little clunky, but I dont use 
> > these enough to know if this is a common pattern.  Its weird to me that 
> > _exp means "e^x", but _exp2 means "2^x".  
> Agreed that it seems odd, but in C and C++ math libraries `exp` is base-e, 
> and `exp2` is base-2:
> 
> https://cplusplus.com/reference/cmath/exp/
> https://cplusplus.com/reference/cmath/exp2/
> 
> It is probably best to be consistent with C here even if it is unintuitive at 
> first glance.
Agreed, thanks for the data point!  I'm ok with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145270

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

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

2023-03-07 Thread Chris Bieneman via Phabricator via cfe-commits

beanz added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:642
  T __builtin_elementwise_log10(T x)  return the base 10 logarithm of x 
   floating point types
+ T __builtin_elementwise_exp(T x)returns the base-e exponential, 
or e^x, of the specified value   floating point types
+ T __builtin_elementwise_exp2(T x)   returns the base-2 exponential, 
or 2^x, of the specified value   floating point types

erichkeane wrote:
> The naming difference between these is a little clunky, but I dont use these 
> enough to know if this is a common pattern.  Its weird to me that _exp means 
> "e^x", but _exp2 means "2^x".  
Agreed that it seems odd, but in C and C++ math libraries `exp` is base-e, and 
`exp2` is base-2:

https://cplusplus.com/reference/cmath/exp/
https://cplusplus.com/reference/cmath/exp2/

It is probably best to be consistent with C here even if it is unintuitive at 
first glance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145270

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

[PATCH] D144780: Explicit cast on customized offsetof should not be ignored when evaluating as const

2023-03-07 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

As this resolves a crash, I'm backporting it: 
https://github.com/llvm/llvm-project/issues/61245


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144780

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

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

2023-03-07 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

Patch itself seems fine enough, but I want to give others a chance to poke at 
it.  The name makes me grumbly, but if there is sufficient 'prior art' here, 
I'm ok with it.




Comment at: clang/docs/LanguageExtensions.rst:642
  T __builtin_elementwise_log10(T x)  return the base 10 logarithm of x 
   floating point types
+ T __builtin_elementwise_exp(T x)returns the base-e exponential, 
or e^x, of the specified value   floating point types
+ T __builtin_elementwise_exp2(T x)   returns the base-2 exponential, 
or 2^x, of the specified value   floating point types

The naming difference between these is a little clunky, but I dont use these 
enough to know if this is a common pattern.  Its weird to me that _exp means 
"e^x", but _exp2 means "2^x".  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145270

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

2023-03-07 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D143418#4175381 , @vedgy wrote:

> I am implementing the `StorePreamblesInMemory` bool option discussed earlier. 
>  It would be nice to be able to modify it at any time, because it can be an 
> option in an IDE's UI and requiring to restart an IDE for the option change 
> to take effect is undesirable. In order to make the setter thread-safe, the 
> option can be stored as `std::atomic StorePreamblesInMemory` in `class 
> CIndexer` and stored/loaded with `memory_order_relaxed`. Setting this option 
> would apply only to subsequent `clang_parseTranslationUnit_Impl()` calls.

Hmmm, don't relaxed loads and stores still have the potential to be racey? I 
thought you needed a release on the store and an acquire on the load (or 
sequential consistency), but this is definitely not my area of expertise.

> The option can also be added to `CXIndexOptions` in order to allow setting 
> its initial value reliably (not worrying whether it could be used before the 
> setter gets called after index construction).
>
> Is adding both the setter and the `CXIndexOptions` member OK or would you 
> prefer only one of these two?

It's a bit odd to me that we're deprecating the global option setters to turn 
around and add a new global option setter. The old interfaces are not thread 
safe and the new one is thread safe, so I get why the desire exists and is 
reasonable, but (personally) I'd like to get rid of the option state setters 
entirely someday. What I was envisioning was that the index creation would get 
global options and if we wanted per-TU options, we'd add an interface akin to 
`clang_parseTranslationUnit()` which takes another options struct for per-TU 
options. (Perhaps the default values for those options come from the global 
options -- e.g., `DisplayDiagnostics` is set at the global level but could be 
overridden for a single TU). Do you have thoughts about that approach?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D145151: clang: Handle MatrixType in hasFloatingRepresentation

2023-03-07 Thread Florian Hahn via Phabricator via cfe-commits

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

LGTM, thanks!


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

https://reviews.llvm.org/D145151

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

[PATCH] D145251: [clang] Treat function parameter scope as an immediate function context

2023-03-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2739f111d97: [clang] Treat function parameter scope as an 
immediate function context (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145251

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/expr/expr.const/p6-2a.cpp


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared 
here}} \
- expected-note {{pointer 
to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+static_assert(r == 42);
 } // namespace P1073R3
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5917,8 +5917,15 @@
 assert(!InitWithCleanup->getNumObjects() &&
"default argument expression has capturing blocks?");
   }
+  // C++ [expr.const]p15.1:
+  //   An expression or conversion is in an immediate function context if it is
+  //   potentially evaluated and [...] its innermost enclosing non-block scope
+  //   is a function parameter scope of an immediate function.
   EnterExpressionEvaluationContext EvalContext(
-  *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+  *this,
+  FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+  Param);
   ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
   SkipImmediateInvocations;
   MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables*/ true);
@@ -6005,8 +6012,16 @@
 // Mark that we are replacing a default argument first.
 // If we are instantiating a template we won't have to
 // retransform immediate calls.
+// C++ [expr.const]p15.1:
+//   An expression or conversion is in an immediate function context if it
+//   is potentially evaluated and [...] its innermost enclosing non-block
+//   scope is a function parameter scope of an immediate function.
 EnterExpressionEvaluationContext EvalContext(
-*this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+*this,
+FD->isConsteval()
+? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+Param);
 
 if (Param->hasUninstantiatedDefaultArg()) {
   if (InstantiateDefaultArgument(CallLoc, FD, Param))


Index: clang/test/CXX/expr/expr.const/p6-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@
 constexpr Temporary t = {3}; // expected-error {{must have constant destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared here}} \
- expected-note {{pointer to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default

[clang] a2739f1 - [clang] Treat function parameter scope as an immediate function context

2023-03-07 Thread Mariya Podchishchaeva via cfe-commits


Author: Mariya Podchishchaeva
Date: 2023-03-07T11:46:26-05:00
New Revision: a2739f111d9795fe49109c26c2d816436f2143c3

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

LOG: [clang] Treat function parameter scope as an immediate function context

This results in expressions that appear in default function argument not
being checked for being actual constant expressions.
This aligns clang's behavior with the standard and fixes one of the
examples from https://wg21.link/P1073R3.

Reviewed By: shafik, cor3ntin

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

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/expr/expr.const/p6-2a.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 56abde04eaf0d..f01329a9dfb37 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5917,8 +5917,15 @@ bool Sema::CheckCXXDefaultArgExpr(SourceLocation 
CallLoc, FunctionDecl *FD,
 assert(!InitWithCleanup->getNumObjects() &&
"default argument expression has capturing blocks?");
   }
+  // C++ [expr.const]p15.1:
+  //   An expression or conversion is in an immediate function context if it is
+  //   potentially evaluated and [...] its innermost enclosing non-block scope
+  //   is a function parameter scope of an immediate function.
   EnterExpressionEvaluationContext EvalContext(
-  *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+  *this,
+  FD->isConsteval() ? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+  Param);
   ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer =
   SkipImmediateInvocations;
   MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables*/ true);
@@ -6005,8 +6012,16 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation 
CallLoc,
 // Mark that we are replacing a default argument first.
 // If we are instantiating a template we won't have to
 // retransform immediate calls.
+// C++ [expr.const]p15.1:
+//   An expression or conversion is in an immediate function context if it
+//   is potentially evaluated and [...] its innermost enclosing non-block
+//   scope is a function parameter scope of an immediate function.
 EnterExpressionEvaluationContext EvalContext(
-*this, ExpressionEvaluationContext::PotentiallyEvaluated, Param);
+*this,
+FD->isConsteval()
+? ExpressionEvaluationContext::ImmediateFunctionContext
+: ExpressionEvaluationContext::PotentiallyEvaluated,
+Param);
 
 if (Param->hasUninstantiatedDefaultArg()) {
   if (InstantiateDefaultArgument(CallLoc, FD, Param))

diff  --git a/clang/test/CXX/expr/expr.const/p6-2a.cpp 
b/clang/test/CXX/expr/expr.const/p6-2a.cpp
index 2ef067c549003..a937474d53b22 100644
--- a/clang/test/CXX/expr/expr.const/p6-2a.cpp
+++ b/clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,14 +43,12 @@ struct Temporary {
 constexpr Temporary t = {3}; // expected-error {{must have constant 
destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note 3 {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
-// FIXME: there should be no diagnostics associated with either h() or r.
-consteval int h(int (*p)() = g()) { return p(); } // expected-error {{call to 
consteval function 'P1073R3::g' is not a constant expression}} \
- expected-note {{declared 
here}} \
- expected-note {{pointer 
to a consteval declaration is not a constant expression}}
-constexpr int r = h();   // expected-note {{in the default initalizer of 'p'}}
+consteval int h(int (*p)() = g()) { return p(); }
+constexpr int r = h();
 constexpr auto e = g();  // expected-error {{call to consteval function 
'P1073R3::g' is not a constant expression}} \
 expected-error {{constexpr variable 'e' must be 
initialized by a constant expression}} \
 expected-note 2 {{pointer to a consteval 
declaration is not a constant expression}}
+static_assert(r == 42);
 } // namespace P1073R3



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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

2023-03-07 Thread Andrew Gozillon via Phabricator via cfe-commits

agozillon added a comment.

Cleaned up the test, thank you both for teaching me more about the compiler and 
test infrastructure! If you're happy with the test changes @kiranchandramohan 
I'll commit the changes upstream to close out the review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

2023-03-07 Thread Andrew Gozillon via Phabricator via cfe-commits

agozillon updated this revision to Diff 503070.
agozillon added a comment.

- [Flang][Driver] Tidy up omp-is-device.f90 test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/LangOptions.def
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Lower/OpenMP/omp-is-device.f90
  flang/tools/bbc/bbc.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Index: mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
===
--- mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1417,6 +1417,26 @@
   return success();
 }
 
+//===--===//
+// OpenMPDialect helper functions
+//===--===//
+
+// Set the omp.is_device attribute on the module with the specified boolean
+void OpenMPDialect::setIsDevice(Operation* module, bool isDevice) {
+  module->setAttr(
+  mlir::StringAttr::get(module->getContext(), llvm::Twine{"omp.is_device"}),
+  mlir::BoolAttr::get(module->getContext(), isDevice));
+}
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+bool OpenMPDialect::getIsDevice(Operation* module) {
+  if (Attribute isDevice = module->getAttr("omp.is_device"))
+if (isDevice.isa())
+  return isDevice.dyn_cast().getValue();
+  return false;
+}
+
 #define GET_ATTRDEF_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
 
Index: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
===
--- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -28,6 +28,15 @@
   let cppNamespace = "::mlir::omp";
   let dependentDialects = ["::mlir::LLVM::LLVMDialect"];
   let useDefaultAttributePrinterParser = 1;
+
+  let extraClassDeclaration = [{
+// Set the omp.is_device attribute on the module with the specified boolean
+static void setIsDevice(Operation* module, bool isDevice);
+
+// Return the value of the omp.is_device attribute stored in the module if it
+// exists, otherwise return false by default
+static bool getIsDevice(Operation* module);
+  }];
 }
 
 // OmpCommon requires definition of OpenACC_Dialect.
Index: flang/tools/bbc/bbc.cpp
===
--- flang/tools/bbc/bbc.cpp
+++ flang/tools/bbc/bbc.cpp
@@ -38,6 +38,7 @@
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/unparse-with-symbols.h"
 #include "flang/Version.inc"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/BuiltinOps.h"
 #include "mlir/IR/MLIRContext.h"
@@ -122,6 +123,11 @@
 llvm::cl::desc("enable openmp"),
 llvm::cl::init(false));
 
+static llvm::cl::opt
+enableOpenMPDevice("fopenmp-is-device",
+   llvm::cl::desc("enable openmp device compilation"),
+   llvm::cl::init(false));
+
 static llvm::cl::opt enableOpenACC("fopenacc",
  llvm::cl::desc("enable openacc"),
  llvm::cl::init(false));
@@ -237,6 +243,8 @@
   kindMap, loweringOptions, {});
   burnside.lower(parseTree, semanticsContext);
   mlir::ModuleOp mlirModule = burnside.getModule();
+  if (enableOpenMP)
+mlir::omp::OpenMPDialect::setIsDevice(mlirModule, enableOpenMPDevice);
   std::error_code ec;
   std::string outputName = outputFilename;
   if (!outputName.size())
Index: flang/test/Lower/OpenMP/omp-is-device.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/omp-is-device.f90
@@ -0,0 +1,14 @@
+!RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix=HOST
+!RUN: %flang_fc1 -emit-fir -fopenmp-is-device %s -o - | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+!RUN: bbc -fopenmp -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE
+!RUN: bbc -fopenmp -emit-fir -o - %s | FileCheck %s --check-prefix=HOST
+!RUN: bbc -fopenmp-is-device -emit-fir -o - %s | FileCheck %s --check-prefix=DEVICE-FLAG-ONLY
+
+!DEVICE: module attributes {{{.*}}, omp.is_device = true{{.*}}}
+!HOST: module attributes {{{.*}}, omp.is_device = false{{.*}}}
+!DEVICE-FLAG-ONLY: module attributes {{{.*}}"
+!DEVICE-FLAG-ONLY-NOT: , omp.is_device = {{.*}}
+!DEVICE-FLAG-ONLY-SAME: }

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

2023-03-07 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev created this revision.
ABataev added reviewers: jdoerfert, Meinersbur, mikerice.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
ABataev requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.
Herald added projects: clang, OpenMP.

The indeces of the dependent loops are properly ordered, just start from
1, so need just subtract 1 to get correct loop index.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145514

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_non_rectangular_codegen.c
  openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c

Index: openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
===
--- /dev/null
+++ openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c
@@ -0,0 +1,34 @@
+// RUN: %libomp-compile-and-run
+
+#define N 10
+int arr[N][N][N];
+
+void collapsed(int mp) {
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+arr[j][i][i0] = 1;
+  }
+}
+  }
+}
+
+int test(int mp) {
+  int Cnt = 0;
+  for (int j = 0; j < mp; ++j) {
+for (int i = 0; i < mp; ++i) {
+  for (int i0 = 0; i0 < N; ++i0) {
+if ((i < j && arr[j][i][i0] == 0) || (i >= j && arr[j][i][i0] == 1))
+  continue;
+++Cnt;
+  }
+}
+  }
+  return Cnt;
+}
+
+int main() {
+  collapsed(N);
+  return test(N);
+}
Index: clang/test/OpenMP/for_non_rectangular_codegen.c
===
--- /dev/null
+++ clang/test/OpenMP/for_non_rectangular_codegen.c
@@ -0,0 +1,259 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -x c -triple x86_64-unknown-unknown -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// SIMD-ONLY0-NOT: {{__kmpc|__tgt}}
+//
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+void collapsed(int mp) {
+#pragma omp for collapse(3)
+  for (int j = 0; j < mp; ++j) {
+for (int i = j; i < mp; ++i) {
+  for (int i0 = 0; i0 < 10; ++i0) {
+;
+  }
+}
+  }
+}
+
+#endif // HEADER
+// CHECK-LABEL: define {{[^@]+}}@collapsed
+// CHECK-SAME: (i32 noundef [[MP:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[MP_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_IV:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[TMP:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP1:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP2:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MIN:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLB_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTMIN_LESS_MAX:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTUPPER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTLOWER:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTCAPTURE_EXPR_3:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I0:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[_TMP15:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[DOTOMP_LB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_UB:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_STRIDE:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[DOTOMP_IS_LAST:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J19:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I20:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[I021:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[TMP0:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB2:[0-9]+]])
+// CHECK-NEXT:store i32 [[MP]], ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[MP_ADDR]], align 4
+// CHECK-NEXT:store i32 [[TMP1]], ptr [[DOTCAPTURE_EXPR_]], align 4
+// CHECK-NEXT:store i32 0, ptr [[TMP]], align 4
+// CHECK-NEXT:[[TMP2:%.*]] = load i32, ptr [[TMP]], align 4
+// CHECK-NEXT:store i32 [[TMP2]], ptr [[DOTLB_MIN]], align 4
+// CHECK-NEXT:[[TMP3:%.*]] =

[PATCH] D145007: Driver: introduce GNU spellings to control MSVC paths

2023-03-07 Thread Saleem Abdulrasool 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 rGf1440bf6fd22: Driver: introduce GNU spellings to control 
MSVC paths (authored by compnerd).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145007

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


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, 
Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f1440bf - Driver: introduce GNU spellings to control MSVC paths

2023-03-07 Thread Saleem Abdulrasool via cfe-commits


Author: Saleem Abdulrasool
Date: 2023-03-07T08:41:35-08:00
New Revision: f1440bf6fd22ca0a5fc3594000e966201989fd48

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

LOG: Driver: introduce GNU spellings to control MSVC paths

Add a set of `-Xmicrosoft` flags to control the Windows SDK and VisualC
tools directories.  This allows control over the selection of the SDK
and tools when using the GNU driver.

Differential Revision: https://reviews.llvm.org/D145007
Reviewed By: mstorjo

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index b914b1b8f12eb..d61083f8b538a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -7044,6 +7044,16 @@ def _SLASH_Gv : CLFlag<"Gv">,
 def _SLASH_Gregcall : CLFlag<"Gregcall">,
   HelpText<"Set __regcall as a default calling convention">;
 
+// GNU Driver aliases
+
+def : Separate<["-"], "Xmicrosoft-visualc-tools-root">, 
Alias<_SLASH_vctoolsdir>;
+def : Separate<["-"], "Xmicrosoft-visualc-tools-version">,
+Alias<_SLASH_vctoolsversion>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-root">,
+Alias<_SLASH_winsdkdir>;
+def : Separate<["-"], "Xmicrosoft-windows-sdk-version">,
+Alias<_SLASH_winsdkversion>;
+
 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;



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

[PATCH] D145362: [clang] Update test according to P1937

2023-03-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0db54e0dd32: [clang] Update test according to P1937 
(authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145362

Files:
  clang/test/CXX/expr/expr.const/p8-2a.cpp


Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 


Index: clang/test/CXX/expr/expr.const/p8-2a.cpp
===
--- clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d0db54e - [clang] Update test according to P1937

2023-03-07 Thread Mariya Podchishchaeva via cfe-commits


Author: Mariya Podchishchaeva
Date: 2023-03-07T11:30:25-05:00
New Revision: d0db54e0dd3272a044407a6394cae47c84cd3a70

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

LOG: [clang] Update test according to P1937

https://wg21.link/p1937 proposes that in unevaluated contexts, consteval
functions should not be immediately evaluated.
Clang implemented p1937 a while ago, its behavior is correct and the
test needs an update.

Reviewed By: aaron.ballman, shafik

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

Added: 


Modified: 
clang/test/CXX/expr/expr.const/p8-2a.cpp

Removed: 




diff  --git a/clang/test/CXX/expr/expr.const/p8-2a.cpp 
b/clang/test/CXX/expr/expr.const/p8-2a.cpp
index b8bee64b0aae2..c13b32a165dbd 100644
--- a/clang/test/CXX/expr/expr.const/p8-2a.cpp
+++ b/clang/test/CXX/expr/expr.const/p8-2a.cpp
@@ -2,7 +2,7 @@
 
 // expected-no-diagnostics
 
-namespace P1073R3 {
+namespace P1937R2 {
 struct N {
   constexpr N() {}
   N(N const&) = delete;
@@ -11,12 +11,22 @@ struct N {
 template constexpr void bad_assert_copyable() { T t; T t2 = t; }
 using ineffective = decltype(bad_assert_copyable());
 
-// bad_assert_copyable is not needed for constant evaluation
-// (and thus not instantiated)
 template consteval void assert_copyable() { T t; T t2 = t; }
+// Prior to P1937R2 consteval functions were evaluated even in otherwise
+// unevaluated context, now this is well-formed.
 using check = decltype(assert_copyable());
-// FIXME: this should give an error because assert_copyable is instantiated
-// (because it is needed for constant evaluation), but the attempt to copy t is
-// ill-formed.
-} // namespace P1073R3
+
+template
+__add_rvalue_reference(T) declval();
+
+constexpr auto add1(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add1(declval(), declval()));
+
+consteval auto add2(auto lhs, auto rhs) {
+return lhs + rhs;
+}
+using T = decltype(add2(declval(), declval()));
+} // namespace P1937R2
 



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

[clang] 128f7da - [Lex] Use line markers in preprocessed assembly predefines file

2023-03-07 Thread John Brawn via cfe-commits


Author: John Brawn
Date: 2023-03-07T16:20:43Z
New Revision: 128f7dac82ea4b996ddfd0db86edfdac4013b1da

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

LOG: [Lex] Use line markers in preprocessed assembly predefines file

GNU line marker directives are not recognised when preprocessing
assembly files, meaning they can't be used in the predefines file
meaning macros defined on the command line are reported as being
built-in.

Change this to permit line markers but only in the predefines file,
so we can correctly report command line macros as coming from the
command line.

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

Added: 
clang/test/Preprocessor/directives_asm.S
clang/test/Preprocessor/macro_redefined.S

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Frontend/InitPreprocessor.cpp
clang/lib/Lex/PPDirectives.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4bc2e54a85998..59c8c5c799f1a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,9 @@ Improvements to Clang's diagnostics
   class if that class was first introduced with a forward declaration.
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias`` 
attributes
   which point to functions whose names are mangled.
+- Diagnostics relating to macros on the command line of a preprocessed assembly
+  file are now reported as coming from the file  instead of
+  .
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 208c6a8db1598..ab00724af2fa9 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -1317,11 +1317,10 @@ void clang::InitializePreprocessor(
   llvm::raw_string_ostream Predefines(PredefineBuffer);
   MacroBuilder Builder(Predefines);
 
-  // Emit line markers for various builtin sections of the file.  We don't do
-  // this in asm preprocessor mode, because "# 4" is not a line marker 
directive
-  // in this mode.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 3");
+  // Emit line markers for various builtin sections of the file. The 3 here
+  // marks  as being a system header, which suppresses warnings when
+  // the same macro is defined multiple times.
+  Builder.append("# 1 \"\" 3");
 
   // Install things like __POWERPC__, __GNUC__, etc into the macro table.
   if (InitOpts.UsePredefines) {
@@ -1359,8 +1358,7 @@ void clang::InitializePreprocessor(
 
   // Add on the predefines from the driver.  Wrap in a #line directive to 
report
   // that they come from the command line.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 1");
+  Builder.append("# 1 \"\" 1");
 
   // Process #define's and #undef's in the order they are given.
   for (unsigned i = 0, e = InitOpts.Macros.size(); i != e; ++i) {
@@ -1372,8 +1370,7 @@ void clang::InitializePreprocessor(
   }
 
   // Exit the command line and go back to  (2 is LC_LEAVE).
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 2");
+  Builder.append("# 1 \"\" 2");
 
   // If -imacros are specified, include them now.  These are processed before
   // any -include directives.

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 9e2392529ff53..bda9d0877612e 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1185,8 +1185,12 @@ void Preprocessor::HandleDirective(Token ) {
 CurPPLexer->getConditionalStackDepth() > 
0);
 return;
   case tok::numeric_constant:  // # 7  GNU line marker directive.
-if (getLangOpts().AsmPreprocessor)
-  break;  // # 4 is not a preprocessor directive in .S files.
+// In a .S file "# 4" may be a comment so don't treat it as a preprocessor
+// directive. However do permit it in the predefines file, as we use line
+// markers to mark the builtin macros as being in a system header.
+if (getLangOpts().AsmPreprocessor &&
+SourceMgr.getFileID(SavedHash.getLocation()) != getPredefinesFileID())
+  break;
 return HandleDigitDirective(Result);
   default:
 IdentifierInfo *II = Result.getIdentifierInfo();

diff  --git a/clang/test/Preprocessor/directives_asm.S 
b/clang/test/Preprocessor/directives_asm.S
new file mode 100644
index 0..55e71d621e341
--- /dev/null
+++ b/clang/test/Preprocessor/directives_asm.S
@@ -0,0 +1,25 @@
+// RUN: %clang -c %s -o /dev/null 2>&1 | FileCheck %s
+
+// Check that preprocessor directives are recognised as such, but lines 
starting
+// with a # that aren't directives are instead treated as comments.
+
+#define MACRO .warning

[PATCH] D145397: [Lex] Use line markers in preprocessed assembly predefines file

2023-03-07 Thread John Brawn via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG128f7dac82ea: [Lex] Use line markers in preprocessed 
assembly predefines file (authored by john.brawn).

Changed prior to commit:
  https://reviews.llvm.org/D145397?vs=502680=503058#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145397

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Preprocessor/directives_asm.S
  clang/test/Preprocessor/macro_redefined.S

Index: clang/test/Preprocessor/macro_redefined.S
===
--- /dev/null
+++ clang/test/Preprocessor/macro_redefined.S
@@ -0,0 +1,10 @@
+// RUN: %clang %s -E -DCLI_MACRO=1 2>&1 | FileCheck %s
+
+#define CLI_MACRO
+// CHECK: macro_redefined.S{{.+}}: warning: 'CLI_MACRO' macro redefined
+// CHECK: {{.+}}: note: previous definition is here
+
+#define REGULAR_MACRO
+#define REGULAR_MACRO 1
+// CHECK: macro_redefined.S{{.+}}: warning: 'REGULAR_MACRO' macro redefined
+// CHECK: macro_redefined.S{{.+}}: note: previous definition is here
Index: clang/test/Preprocessor/directives_asm.S
===
--- /dev/null
+++ clang/test/Preprocessor/directives_asm.S
@@ -0,0 +1,25 @@
+// RUN: %clang -c %s -o /dev/null 2>&1 | FileCheck %s
+
+// Check that preprocessor directives are recognised as such, but lines starting
+// with a # that aren't directives are instead treated as comments.
+
+#define MACRO .warning "This is a macro"
+MACRO
+
+// CHECK: directives_asm.S:7:9: warning: This is a macro
+
+#not a preprocessing directive
+
+// CHECK-NOT: error: invalid preprocessing directive
+
+# 100
+
+.warning "line number should not change"
+
+// CHECK: directives_asm.S:17:9: warning: line number should not change
+
+#line 100
+
+.warning "line number should change"
+
+// CHECK: directives_asm.S:101:9: warning: line number should change
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1185,8 +1185,12 @@
 CurPPLexer->getConditionalStackDepth() > 0);
 return;
   case tok::numeric_constant:  // # 7  GNU line marker directive.
-if (getLangOpts().AsmPreprocessor)
-  break;  // # 4 is not a preprocessor directive in .S files.
+// In a .S file "# 4" may be a comment so don't treat it as a preprocessor
+// directive. However do permit it in the predefines file, as we use line
+// markers to mark the builtin macros as being in a system header.
+if (getLangOpts().AsmPreprocessor &&
+SourceMgr.getFileID(SavedHash.getLocation()) != getPredefinesFileID())
+  break;
 return HandleDigitDirective(Result);
   default:
 IdentifierInfo *II = Result.getIdentifierInfo();
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -1317,11 +1317,10 @@
   llvm::raw_string_ostream Predefines(PredefineBuffer);
   MacroBuilder Builder(Predefines);
 
-  // Emit line markers for various builtin sections of the file.  We don't do
-  // this in asm preprocessor mode, because "# 4" is not a line marker directive
-  // in this mode.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 3");
+  // Emit line markers for various builtin sections of the file. The 3 here
+  // marks  as being a system header, which suppresses warnings when
+  // the same macro is defined multiple times.
+  Builder.append("# 1 \"\" 3");
 
   // Install things like __POWERPC__, __GNUC__, etc into the macro table.
   if (InitOpts.UsePredefines) {
@@ -1359,8 +1358,7 @@
 
   // Add on the predefines from the driver.  Wrap in a #line directive to report
   // that they come from the command line.
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 1");
+  Builder.append("# 1 \"\" 1");
 
   // Process #define's and #undef's in the order they are given.
   for (unsigned i = 0, e = InitOpts.Macros.size(); i != e; ++i) {
@@ -1372,8 +1370,7 @@
   }
 
   // Exit the command line and go back to  (2 is LC_LEAVE).
-  if (!PP.getLangOpts().AsmPreprocessor)
-Builder.append("# 1 \"\" 2");
+  Builder.append("# 1 \"\" 2");
 
   // If -imacros are specified, include them now.  These are processed before
   // any -include directives.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -159,6 +159,9 @@
   class if that class was first introduced with a forward declaration.
 - Diagnostic notes and fix-its are now generated for ``ifunc``/``alias``

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

2023-03-07 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski accepted this revision.
awarzynski added a comment.

In D144864#4175001 , @agozillon wrote:

> And then provided @awarzynski is happy with the current state of the patch

LGTM, thanks for the updates and for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144864

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

[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

2023-03-07 Thread Krzysztof Drewniak via Phabricator via cfe-commits

krzysz00 added a comment.

@foad I was trying to avoid sending in one mega-patch that has the entire 
prototype.

As to your comments:

- Why 256-bit alignment? It's the next power of 2, and using an alignment that 
isn't a power of 2 causes an assertion failure
- Re not being allowed to use address space 8 pointers in the usual LLVM 
operations, I'd call that a target-specific rule about what address space 8 
means. I figure this is something that we can check in some sort of 
AMDGPU-specific validation
- I changed the fallback address space because the plan is that those 
intrinsics will be taking address space 8 arguments in the future


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

2023-03-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits

Fznamznon added a comment.

Filed https://github.com/llvm/llvm-project/issues/61243 to backport.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

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

[PATCH] D145509: [HIP] Fix temporary files

2023-03-07 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.
yaxunl added reviewers: tra, MaskRay.
Herald added a reviewer: JDevlieghere.
Herald added a project: All.
yaxunl requested review of this revision.

Currently HIP toolchain uses Driver::GetTemporaryDirectory to
create a temporary directory for some temporary files during
compilation. The temporary directories are not automatically
deleted after compilation. This slows down compilation
on Windows.

Switch to use GetTemporaryPath which only creates temporay
files which will be deleted automatically.

This change is OK for MacOS as lipo does not requires specific
file names (https://ss64.com/osx/lipo.html)


https://reviews.llvm.org/D145509

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/darwin-dsymutil.c
  clang/test/Driver/hip-link-bc-to-bc.hip
  clang/test/Driver/hip-temps-linux.hip
  clang/test/Driver/hip-temps-windows.hip

Index: clang/test/Driver/hip-temps-windows.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-windows.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-windows
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMP=%t/mytmp %clang --target=x86_64-pc-windows-msvc -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o "{{.*}}mytmp\\hip-temps-windows-gfx1030-{{.*}}.o"
+
+int main() {}
Index: clang/test/Driver/hip-temps-linux.hip
===
--- /dev/null
+++ clang/test/Driver/hip-temps-linux.hip
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: system-linux
+
+// Check no temporary files or directores are left after compilation.
+// RUN: rm -rf %t/mytmp
+// RUN: mkdir -p %t/mytmp
+// RUN: env TMPDIR=%t/mytmp %clang --target=x86_64-linux-gnu -nogpulib -nogpuinc \
+// RUN:   --rocm-path=%S/Inputs/rocm -no-hip-rt --offload-arch=gfx1030 -v %s 2>&1 | \
+// RUN:   FileCheck -check-prefixes=CHECK %s
+// RUN: ls %t/mytmp >%t/mytmp.txt 2>&1
+// RUN: touch %t/empty.txt
+// RUN: diff %t/mytmp.txt %t/empty.txt
+
+// CHECK: -o {{.*}}/mytmp/hip-temps-linux-gfx1030-{{.*}}.o
+
+int main() {}
Index: clang/test/Driver/hip-link-bc-to-bc.hip
===
--- clang/test/Driver/hip-link-bc-to-bc.hip
+++ clang/test/Driver/hip-link-bc-to-bc.hip
@@ -11,10 +11,10 @@
 // RUN:   2>&1 | FileCheck -check-prefix=BITCODE %s
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle1.bc" "-output=[[B1HOST:.*\.bc]]" "-output=[[B1DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906.bc]]" "-x" "ir" "[[B1DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B1DEV2:.*bundle1-gfx906-.*\.bc]]" "-x" "ir" "[[B1DEV1]]"
 
 // BITCODE: "{{.*}}clang-offload-bundler" "-type=bc" "-targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa-gfx906" "-input={{.*}}bundle2.bc" "-output=[[B2HOST:.*\.bc]]" "-output=[[B2DEV1:.*\.bc]]" "-unbundle" "-allow-missing-bundles"
-// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906.bc]]" "-x" "ir" "[[B2DEV1]]"
+// BITCODE: "{{.*}}clang{{.*}}" "-o" "[[B2DEV2:.*bundle2-gfx906-.*\.bc]]" "-x" "ir" "[[B2DEV1]]"
 
 // BITCODE: "{{.*}}llvm-link" "-o" "bundle1-hip-amdgcn-amd-amdhsa-gfx906.bc" "[[B1DEV2]]" "[[B2DEV2]]"
 
Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -48,17 +48,17 @@
 // RUN:   -arch x86_64 -arch arm64 -ccc-print-bindings %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-MULTIARCH-OUTPUT-NAME < %t %s
 //
-// CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
-// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
-// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
+// CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin11" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64{{.*}}.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64{{.*}}.out"
+// CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin11" - "darwin::Linker", inputs:

[clang-tools-extra] 38b9fb5 - [clangd] Add support for missing includes analysis.

2023-03-07 Thread Viktoriia Bakalova via cfe-commits


Author: Viktoriia Bakalova
Date: 2023-03-07T16:07:19Z
New Revision: 38b9fb5a129db3e086610d53b534833273c5b4d0

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

LOG: [clangd] Add support for missing includes analysis.

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index dffd54b01c459..ab346aab0e8ac 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -88,11 +88,12 @@ struct Config {
 bool StandardLibrary = true;
   } Index;
 
-  enum class UnusedIncludesPolicy {
-/// Diagnose unused includes.
+  enum class IncludesPolicy {
+/// Diagnose missing and unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library.
+/// The same as Strict, but using the include-cleaner library for
+/// unused includes.
 Experiment,
   };
   /// Controls warnings and errors when parsing code.
@@ -107,11 +108,12 @@ struct Config {
   llvm::StringMap CheckOptions;
 } ClangTidy;
 
-UnusedIncludesPolicy UnusedIncludes = UnusedIncludesPolicy::None;
-
 /// Enable emitting diagnostics using stale preambles.
 bool AllowStalePreamble = false;
 
+IncludesPolicy UnusedIncludes = IncludesPolicy::None;
+IncludesPolicy MissingIncludes = IncludesPolicy::None;
+
 /// IncludeCleaner will not diagnose usages of these headers matched by
 /// these regexes.
 struct {

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 18de6e4d5c3b6..2f0ef892131ca 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -431,16 +431,16 @@ struct FragmentCompiler {
   });
 
 if (F.UnusedIncludes)
-  if (auto Val =
-  compileEnum("UnusedIncludes",
-**F.UnusedIncludes)
-  .map("Strict", Config::UnusedIncludesPolicy::Strict)
-  .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
-  .map("None", Config::UnusedIncludesPolicy::None)
-  .value())
+  if (auto Val = compileEnum("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("Experiment", Config::IncludesPolicy::Experiment)
+ .map("None", Config::IncludesPolicy::None)
+ .value())
 Out.Apply.push_back([Val](const Params &, Config ) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
+
 if (F.AllowStalePreamble) {
   if (auto Val = F.AllowStalePreamble)
 Out.Apply.push_back([Val](const Params &, Config ) {
@@ -448,6 +448,16 @@ struct FragmentCompiler {
 });
 }
 
+if (F.MissingIncludes)
+  if (auto Val = compileEnum("MissingIncludes",
+ **F.MissingIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::None)
+ .value())
+Out.Apply.push_back([Val](const Params &, Config ) {
+  C.Diagnostics.MissingIncludes = *Val;
+});
+
 compile(std::move(F.Includes));
 compile(std::move(F.ClangTidy));
   }

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index a5597596196fa..956e8a8599446 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -221,7 +221,7 @@ struct Fragment {
 /// This often has other advantages, such as skipping some analysis.
 std::vector> Suppress;
 
-/// Controls how clangd will correct "unnecessary #include directives.
+/// Controls how clangd will correct "unnecessary" #include

[PATCH] D143496: [clangd] Add support for missing includes analysis.

2023-03-07 Thread Viktoriia Bakalova 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 rG38b9fb5a129d: [clangd] Add support for missing includes 
analysis. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143496

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
   }
 }
 
-static std::string spellHeader(const Header , HeaderSearch ,
-   const FileEntry *Main) {
+std::string spellHeader(const Header , HeaderSearch ,
+const FileEntry *Main) {
   switch (H.kind()) {
   case Header::Physical: {
 bool IsSystem = false;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
 std::string fixIncludes(const AnalysisResults , llvm::StringRef Code,
 const format::FormatStyle );
 
+std::string spellHeader(const Header , HeaderSearch ,
+const FileEntry *Main);
 } // namespace include_cleaner
 } // namespace clang
 
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -665,7 +665,7 @@
 TEST(PreamblePatch, DiagnosticsToPreamble) {
   Config Cfg;
   Cfg.Diagnostics.AllowStalePreamble = true;
-  Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue WithCfg(Config::Key, std::move(Cfg));
 
   llvm::StringMap AdditionalFiles;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -8,26 +8,58 @@
 
 #include "Annotations.h"
 #include "Config.h"
+#include "Diagnostics.h"
 #include "IncludeCleaner.h"
+#include "ParsedAST.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
 #include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
+using ::testing::Matcher;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
 
+Matcher withFix(::testing::Matcher FixMatcher) {
+  return Field(::Fixes, ElementsAre(FixMatcher));
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Range == Range && arg.Message == Message;
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   ::testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Message == Message && arg.Edits.size() == 1 &&
+ arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
+}
+
 std::string guard(llvm::StringRef Code) {
   return "#pragma once\n" + Code.str();
 }
@@ -342,7 +374,8 @@
   auto AST = TU.build();
   EXPECT_THAT(computeUnusedIncludes(AST),

[PATCH] D145486: [clang] Fix single-element array initialization in constexpr

2023-03-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf682f0df83f: [clang] Fix single-element array 
initialization in constexpr (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145486

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constexpr-single-element-array.cpp


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue  = Value->getArrayInitializedElt(0);


Index: clang/test/SemaCXX/constexpr-single-element-array.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue  = Value->getArrayInitializedElt(0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] af682f0 - [clang] Fix single-element array initialization in constexpr

2023-03-07 Thread Mariya Podchishchaeva via cfe-commits


Author: Mariya Podchishchaeva
Date: 2023-03-07T10:57:35-05:00
New Revision: af682f0df83f3364dac7ab92f39c7209dfbce28a

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

LOG: [clang] Fix single-element array initialization in constexpr

https://reviews.llvm.org/D130791 added an improvement that in case array
element has a trivial constructor, it is evaluated once and the result is
re-used for remaining elements. Make sure the constructor is evaluated
for single-elements arrays too.

Fixes https://github.com/llvm/llvm-project/issues/60803

Reviewed By: aaron.ballman

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

Added: 
clang/test/SemaCXX/constexpr-single-element-array.cpp

Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6b0beb1973893..26c23423b858f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10917,7 +10917,7 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const 
CXXConstructExpr *E,
 for (unsigned I = OldElts; I < N; ++I)
   Value->getArrayInitializedElt(I) = Filler;
 
-  if (HasTrivialConstructor && N == FinalSize) {
+  if (HasTrivialConstructor && N == FinalSize && FinalSize != 1) {
 // If we have a trivial constructor, only evaluate it once and copy
 // the result into all the array elements.
 APValue  = Value->getArrayInitializedElt(0);

diff  --git a/clang/test/SemaCXX/constexpr-single-element-array.cpp 
b/clang/test/SemaCXX/constexpr-single-element-array.cpp
new file mode 100644
index 0..a01b1a1c8f136
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-single-element-array.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// This test makes sure that a single element array doesn't produce
+// spurious errors during constexpr evaluation.
+
+// expected-no-diagnostics
+struct Sub { int x; };
+
+struct S {
+  constexpr S() { Arr[0] = Sub{}; }
+  Sub Arr[1];
+};
+
+constexpr bool test() {
+  S s;
+  return true;
+}
+
+static_assert(test());



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

1 2 >

1 - 100 of 180 matches

Mail list logo