[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: clang/lib/CodeGen/Address.h:30
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
 

dblaikie wrote:
> aeubanks wrote:
> > nikic wrote:
> > > Are we guaranteed 3 bits even on 32-bit architectures?
> > Apparently not, a static assert fires for a 32-bit build of clang. I was 
> > assuming 3 was fine based on the PointerIntPair comments.
> > ```
> > /// PointerIntPair - This class implements a pair of a pointer and small
> > /// integer.  It is designed to represent this in the space required by one
> > /// pointer by bitmangling the integer into the low part of the pointer.  
> > This
> > /// can only be done for small integers: typically up to 3 bits, but it 
> > depends
> > /// on the number of bits available according to PointerLikeTypeTraits for 
> > the
> > /// type.
> > ```
> > 
> > I suppose we could have a separate 32-bit vs 64-bit implementation of 
> > `Address`, although that's not very nice.
> You can get more spare bits by overaligning the type that you're pointing to, 
> if that's suitable/workable. We do that in a few places in LLVM for this sort 
> of reason.
As we're storing pointers to core LLVM types here (Value and Type), I don't 
think it's possible to over-align them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D117468: [RISCV] Add intrinsic for Zbt extension

[Chenbing.Zheng via Phabricator via cfe-commits]

Chenbing.Zheng updated this revision to Diff 401124.
Chenbing.Zheng added a comment.

rebase main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv32-zbt.c
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbt.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
  llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll

Index: llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll
@@ -0,0 +1,65 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zbt -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBT
+
+declare i32 @llvm.riscv.fsl.i32(i32, i32, i32)
+
+define i32 @fsl_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fslw a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsl.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.fsr.i32(i32, i32, i32)
+
+define i32 @fsr_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsrw a0, a1, a0, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+define i32 @fsri_i32(i32 %a, i32 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsriw a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 5)
+  ret i32 %1
+}
+
+declare i64 @llvm.riscv.fsl.i64(i64, i64, i64)
+
+define i64 @fsl_i64(i64 %a, i64 %b, i64 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsl a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsl.i64(i64 %a, i64 %b, i64 %c)
+  ret i64 %1
+}
+
+declare i64 @llvm.riscv.fsr.i64(i64, i64, i64)
+
+define i64 @fsr_i64(i64 %a, i64 %b, i64 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsr a0, a1, a0, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsr.i64(i64 %a, i64 %b, i64 %c)
+  ret i64 %1
+}
+
+define i64 @fsri_i64(i64 %a, i64 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i64:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsri a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i64 @llvm.riscv.fsr.i64(i64 %a, i64 %b, i64 5)
+  ret i64 %1
+}
Index: llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-zbt -verify-machineinstrs < %s \
+; RUN:   | FileCheck %s -check-prefix=RV32ZBT
+
+declare i32 @llvm.riscv.fsl.i32(i32, i32, i32)
+
+define i32 @fsl_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsl_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsl a0, a0, a1, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsl.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+declare i32 @llvm.riscv.fsr.i32(i32, i32, i32)
+
+define i32 @fsr_i32(i32 %a, i32 %b, i32 %c) nounwind {
+; RV32ZBT-LABEL: fsr_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsr a0, a1, a0, a2
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 %c)
+  ret i32 %1
+}
+
+define i32 @fsri_i32(i32 %a, i32 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i32:
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsri a0, a1, a0, 5
+; RV32ZBT-NEXT:ret
+  %1 = call i32 @llvm.riscv.fsr.i32(i32 %a, i32 %b, i32 5)
+  ret i32 %1
+}
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -4256,6 +4256,12 @@
   case Intrinsic::riscv_bfp:
 return DAG.getNode(RISCVISD::BFP, DL, XLenVT, Op.getOperand(1),
Op.getOperand(2));
+  case Intrinsic::riscv_fsl:
+return DAG.getNode(RISCVISD::FSL, DL, XLenVT, Op.getOperand(1),
+   Op.getOperand(2), Op.getOperand(3));
+  case Intrinsic::riscv_fsr:
+return DAG.getNode(RISCVISD::FSR, DL, XLenVT, Op.getOperand(1),
+   Op.getOperand(2), Op.getOperand(3));
   case Intrinsic::riscv_vmv_x_s:
 assert(Op.getValueType() == XLenVT && "Unexpected VT!");
 return DAG.getNode(RISCVISD::VMV_X_S, DL, Op.getValueType(),
@@ -5843,6 +5849,10 @@
 return RISCVISD::BDECOMPRESSW;
   case Intrinsic::riscv_bfp:
 return RISCVISD::BFPW;
+  case Intrinsic::ri

[clang] 480a1fa - [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej via cfe-commits]

Author: Elliott Maguire
Date: 2022-01-19T10:18:47+01:00
New Revision: 480a1fab72f4e367a5986d572914d252c318431d

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

LOG: [clang-format] Fix incorrect alignment of operator= overloads.

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

Added a check for operator keyword tokens.

Reviewed By: MyDeveloperDay, curdeius, owenpan, HazardyKnusperkeks

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

Added: 


Modified: 
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 9f3f18aa3eb0d..0d2e507ac5879 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@ void WhitespaceManager::alignConsecutiveAssignments() {
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index c16b6c20d5966..b8645b8896d2a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -16266,6 +16266,34 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed



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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Marek Kurdej 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 rG480a1fab72f4: [clang-format] Fix incorrect alignment of 
operator= overloads. (authored by glotchimo, committed by curdeius).

Changed prior to commit:
  https://reviews.llvm.org/D117421?vs=400696&id=401129#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16266,6 +16266,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16266,6 +16266,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int f() = default;\n"
+   "int &operator() = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAs

[PATCH] D117301: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

what about COMPOSITE_TYPE_SWITCH and INT_TYPE_SWITCH - should they be updated 
as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117301

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

[PATCH] D117468: [RISCV] Add intrinsic for Zbt extension

[Chenbing.Zheng via Phabricator via cfe-commits]

Chenbing.Zheng added a comment.

In D117468#3253493 , @craig.topper 
wrote:

> Out of curiosity, what is your interest in Zbt? Do you work for a company 
> that is implementing this extension in hardware?

My company has not implementing this extension，but may be in the future. I'm 
just doing a pre-research.




Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4246
+  case Intrinsic::riscv_fsl:
+return DAG.getNode(RISCVISD::FSL, DL, XLenVT, Op.getOperand(1),
+   Op.getOperand(2), Op.getOperand(3));

craig.topper wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > The operand order for RISCVISD::FSL/FSR match llvm.fshl and llvm.fshr 
> > > rather than rs1, rs2, rs3 order. I think the operand order you want for 
> > > riscv.fsl and riscv.fsr should be rs1, rs2, rs3.
> > > 
> > > And the assembly printing for fsl/fsr prints $rd, $rs1, $rs3, $rs2 makes 
> > > this even more confusing.
> > > 
> > > I'll put up a patch to change RISCVISD::FSL/FSR and RISCVISD::FSLW/FSRW 
> > > order to be in rs1, rs2, rs3 order.
> > i just noticed the proposed intrinsic order here 
> > https://github.com/riscv/riscv-bitmanip/blob/main-history/cproofs/rvintrin.h
> >   uses rs1, rs3, rs2/shamt ordering. Which is different than the order used 
> > in the spec pseudo code. But rs1, rs3, rs2/shamt matches how the 
> > instructions are printed.
> > 
> > So I guess we should use rs1, rs3, rs2/shamt order. I'll update the 
> > RISCVISD opcodes accordingly.
> I pushed a patch to change the operand order of RISCVISD::FSL/FSR
I think change swap op2 and op3 in Pat, so shall i keep the order op1, op2, op3 
here?

def : Pat<(riscv_fsl GPR:$rs1, GPR:$rs3, GPR:$rs2),
  (FSL GPR:$rs1, GPR:$rs2, GPR:$rs3)>;
def : Pat<(riscv_fsr GPR:$rs1, GPR:$rs3, GPR:$rs2),
  (FSR GPR:$rs1, GPR:$rs2, GPR:$rs3)>;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@HazardyKnusperkeks  (its not without merits to add a "C" Language the more i 
think about it, but I'm not sure quite in the form presented here)

If we DID add a "C" language then we need to add something like you suggested  
isCpp() already means more than we think.

  bool isCStyle() {
  return isCpp() || isC();
  }

Personally I really don't want to see the below change or it will change the 
world, so it would be better if the change was just from `!Style.isCpp()` to 
`!Style.isCStyle()`

  !Style.isCpp() && !Style.isC()

The ClangFormat.cpp changes are really unrelated and need to be removed (or 
resubmitted/discussed separately), and we really must have extensive unit tests 
if we are adding a new language (which this review doesn't have any) - (that 
means a new FormatCTests.cpp)

I'd personally want to see really limited use of `isCStyle()`  and ONLY what's 
needed to fix the exact example/tests we'd add. (its all to easy to 
find/replace thinking you are doing the right thing when actually we may not, 
it needs to be targetted changes)

But bottom line, I just don't see how we fix the .h problem? other than if the 
.clang-format section doesn't have a `Language: Cpp` then we don't assume its 
C++ (problem is it already does)

(actually this is a request that users can limit the guessing of languages to 
only those languages present in the .clang-format file)

This would be good for "C" projects but not much use for C++ or C++/C projects.

It wouldn't solve the `delete(foo);` issue this can still be a function or a 
delete of a bracketed variable, or a macro... so I don't see it helps

And bottom line the Lexer will give it to you as tok::kw_private and not as 
tok::identifier regardless of your language and so you'll need to perhaps do 
something in FormatTokenLexer to transform it back to an identifier (which is 
possible) checkout tryMergeLessLess() etc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D117647: [RISCV] Add destination operand for RVV nomask load intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, rogfer01, evandro, arcbbb, monkchiang.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
jrtc27, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
hiraditya.
khchen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, eopXD, MaskRay.
Herald added projects: clang, LLVM.

The goal is support tail and mask policy in RVV builtins.
We focus on IR part first.
If the destination is undef, we use tail agnostic, otherwise
use tail undisturbed.

Co-Authored-by: Hsiangkai Wang 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117647

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vloxei.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxei.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vleff.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vloxei.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vlse.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vluxei.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/rv32-vsetvli-intrinsics.ll
  llvm/test/CodeGen/RISCV/rvv/rv64-vsetvli-intrinsics.ll
  llvm/test/CodeGen/RISCV/rvv/rvv-out-arguments.ll
  llvm/test/CodeGen/RISCV/rvv/unmasked-tu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/unmasked-tu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vle-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vle-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vleff-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vleff-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vloxei-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vloxei-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vlse-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vlse-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vluxei-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vluxei-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
  llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
  llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.ll
  llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir

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

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I like it. Terse but complete.
I think you could polish the example code to look less synthetic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117568

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

ok, this is better but not quite right still.. I though I mentioned this before 
(please add as a unit test)

  HRESULT
  Foo::bar() {}
  
  HRESULT
  Foo::bar() {}

formats as

  HRESULT
  
  Foo::bar() {}
  
  HRESULT
  
  Foo::bar() {}

If you are windows developer using clang-format  
(HRESULT,LRESULT,LPSTR,LPCSTR,DWORD_PTR etc.. ) its an issue.

Sorry I so want this to be right, its singularly one of the things I've wanted 
for some time (you just got to it before me ;-) ) so I'm super excited by it.

From the other aspects we are getting there.


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

https://reviews.llvm.org/D117520

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

[PATCH] D117301: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D117301#3253874 , @RKSimon wrote:

> what about COMPOSITE_TYPE_SWITCH and INT_TYPE_SWITCH - should they be updated 
> as well?

I left them alone because `COMPOSITE_TYPE_SWITCH` didn't cause a dangling 
`else` problem and `INT_TYPE_SWITCH` was not used at all. I can fix them both 
or fix the former and remvoe the latter from the header file. Also, the current 
`do { ... } while (0)` statements in `TYPE_SWITCH_CASE` and 
`COMPOSITE_TYPE_SWITCH` are superfluous. I can remove them as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117301

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

[PATCH] D117566: [clang][lex] Introduce `DirectoryLookupIterator`

[Jan Svoboda via Phabricator via cfe-commits]

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

Fix comment typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117566

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/PPMacroExpansion.cpp

Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1157,9 +1157,9 @@
 /// EvaluateHasIncludeCommon - Process a '__has_include("path")'
 /// or '__has_include_next("path")' expression.
 /// Returns true if successful.
-static bool EvaluateHasIncludeCommon(Token &Tok,
- IdentifierInfo *II, Preprocessor &PP,
- const DirectoryLookup *LookupFrom,
+static bool EvaluateHasIncludeCommon(Token &Tok, IdentifierInfo *II,
+ Preprocessor &PP,
+ DirectoryLookupIterator LookupFrom,
  const FileEntry *LookupFromFile) {
   // Save the location of the current token.  If a '(' is later found, use
   // that location.  If not, use the end of this location instead.
@@ -1260,7 +1260,7 @@
   // issue a diagnostic.
   // FIXME: Factor out duplication with
   // Preprocessor::HandleIncludeNextDirective.
-  const DirectoryLookup *Lookup = PP.GetCurDirLookup();
+  DirectoryLookupIterator Lookup = PP.GetCurDirLookup();
   const FileEntry *LookupFromFile = nullptr;
   if (PP.isInPrimaryFile() && PP.getLangOpts().IsHeaderFile) {
 // If the main file is a header, then it's either for PCH/AST generation,
Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -66,7 +66,7 @@
 
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
-bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
+bool Preprocessor::EnterSourceFile(FileID FID, DirectoryLookupIterator CurDir,
SourceLocation Loc,
bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
@@ -100,7 +100,7 @@
 /// EnterSourceFileWithLexer - Add a source file to the top of the include stack
 ///  and start lexing tokens from it instead of the current buffer.
 void Preprocessor::EnterSourceFileWithLexer(Lexer *TheLexer,
-const DirectoryLookup *CurDir) {
+DirectoryLookupIterator CurDir) {
 
   // Add the current lexer to the include stack.
   if (CurPPLexer || CurTokenLexer)
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -817,13 +817,13 @@
 
 Optional Preprocessor::LookupFile(
 SourceLocation FilenameLoc, StringRef Filename, bool isAngled,
-const DirectoryLookup *FromDir, const FileEntry *FromFile,
-const DirectoryLookup **CurDirArg, SmallVectorImpl *SearchPath,
+DirectoryLookupIterator FromDir, const FileEntry *FromFile,
+DirectoryLookupIterator *CurDirArg, SmallVectorImpl *SearchPath,
 SmallVectorImpl *RelativePath,
 ModuleMap::KnownHeader *SuggestedModule, bool *IsMapped,
 bool *IsFrameworkFound, bool SkipCache) {
-  const DirectoryLookup *CurDirLocal = nullptr;
-  const DirectoryLookup *&CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
+  DirectoryLookupIterator CurDirLocal = nullptr;
+  DirectoryLookupIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
 
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
   bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
@@ -877,8 +877,8 @@
   if (FromFile) {
 // We're supposed to start looking from after a particular file. Search
 // the include path until we find that file or run out of files.
-const DirectoryLookup *TmpCurDir = CurDir;
-const DirectoryLookup *TmpFromDir = nullptr;
+DirectoryLookupIterator TmpCurDir = CurDir;
+DirectoryLookupIterator TmpFromDir = nullptr;
 while (Optional FE = HeaderInfo.LookupFile(
Filename, FilenameLoc, isAngled, TmpFromDir, &TmpCurDir,
Includers, SearchPath, RelativePath, RequestingModule,
@@ -1785,7 +1785,7 @@
 /// specifies the file to start searching from.
 void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
   Token &IncludeTok,
-  const Directo

[PATCH] D117567: [clang][dataflow] Add a test to justify skipping past references in UO_Deref

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added a comment.

@gribozavr2 suggested that this is probably because we use delayed template 
parsing on Windows for MSVC compatibility. I reproduced the error using the 
`--target=x86_64-windows-msvc` arg and verified that adding 
`-fno-delayed-template-parsing` arg fixes it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117567

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

[clang] acd4b03 - Revert "Revert "[clang][dataflow] Add a test to justify skipping past references in UO_Deref""

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-19T10:00:01Z
New Revision: acd4b0359097dd8ad166d569a4566879e82a2793

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

LOG: Revert "Revert "[clang][dataflow] Add a test to justify skipping past 
references in UO_Deref""

This reverts commit a0262043bb87fdef68c817722de320a5dd9eb9c9.

Add the -fno-delayed-template-parsing arg to fix the failing test on Windows.

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 0979f55c5a5c..865191916e50 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -182,6 +182,7 @@ class TransferVisitor : public 
ConstStmtVisitor {
 
 switch (S->getOpcode()) {
 case UO_Deref: {
+  // Skip past a reference to handle dereference of a dependent pointer.
   const auto *SubExprVal = cast_or_null(
   Env.getValue(*SubExpr, SkipPast::Reference));
   if (SubExprVal == nullptr)

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index ea035ba013da..eef1240f0da2 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -50,7 +50,7 @@ class TransferTest : public ::testing::Test {
 std::pair>>
 Results,
 ASTContext &ASTCtx) { Match(Results, ASTCtx); },
-{"-fsyntax-only",
+{"-fsyntax-only", "-fno-delayed-template-parsing",
  "-std=" +
  std::string(
  LangStandard::getLangStandardForKind(Std).getName())}),
@@ -1758,4 +1758,34 @@ TEST_F(TransferTest, AddrOfReference) {
   });
 }
 
+TEST_F(TransferTest, DerefDependentPtr) {
+  std::string Code = R"(
+template 
+void target(T *Foo) {
+  T &Bar = *Foo;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+  });
+}
+
 } // namespace



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

[PATCH] D93164: [AST] Add generator for source location introspection

[Andrew Anderson via Phabricator via cfe-commits]

andrew-wja added a comment.

In D93164#3252509 , @lancethepants 
wrote:

> In D93164#3250945 , @andrew-wja 
> wrote:
>
>> In D93164#3048130 , @lancethepants 
>> wrote:
>>
>>> In D93164#2653067 , @mgorny wrote:
>>>
 This change breaks cross-compilation now, as it tries running an 
 executable built for the target system:

   FAILED: tools/clang/lib/Tooling/ASTNodeAPI.json 
   cd /home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling && 
 /home/mgorny/llvm-project/build.arm64/bin/clang-ast-dump 
 --skip-processing=0 
 --astheader=/home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling/ASTTU.cpp
  -I /home/mgorny/llvm-project/build.arm64/lib/clang/13.0.0/include -I 
 /home/mgorny/llvm-project/llvm/../clang/include -I 
 /home/mgorny/llvm-project/build.arm64/tools/clang/include -I 
 /home/mgorny/llvm-project/build.arm64/include -I 
 /home/mgorny/llvm-project/llvm/include -I 
 /sysroot/arm64/usr/include/c++/v1 -I /usr/lib/clang/11.0.1/include -I 
 /sysroot/arm64/usr/include --json-output-path 
 /home/mgorny/llvm-project/build.arm64/tools/clang/lib/Tooling/ASTNodeAPI.json
   /bin/sh: /home/mgorny/llvm-project/build.arm64/bin/clang-ast-dump: Exec 
 format error

 I guess you can look at TableGens how to correctly generate and use host 
 executables.
>>>
>>> I am now running into this error now that clang13 has released.  I cannot 
>>> figure a way around this. I do not see any cmake options similar to the 
>>> tablegens that allow you to specify the binary for the build system.  Am I 
>>> missing something or is clang13 just broken for cross compilation?
>>
>> Another confirmation that this change has broken cross compilation. I have 
>> tried setting both `-DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF` and 
>> `-DCMAKE_CROSSCOMPILING=ON` separately and in combination. This means that 
>> LLVM cannot be cross compiled for AArch64, which is a pretty serious problem!
>
> So I did find a solution. Maybe somewhere else in the forums or on irc. Can't 
> remember for sure.
>
> -DCMAKE_SYSTEM_NAME="Linux"
> Add this is well to your cmake invocation. For some reason this triggers in 
> cmake that we are cross-compiling. Why -DCMAKE_CROSSCOMPILING=ON is 
> insufficient I have no idea why. I'm compiling on linux for linux, didn't 
> think I'd need it.  Kind of unintuitive, but that should hopefully get you 
> going.

@lancethepants @smeenai thanks for the pointers! Unfortunately, it still 
doesn't work for me after passing -DCMAKE_SYSTEM_NAME="Linux". Passing that 
option did change the CMake output, so it's definitely recognized at least (the 
messages about which sanitizer tests will run now call the platform "Linux" as 
opposed to referring to the platform as default). I am also compiling on Linux 
for Linux, I'm just trying to build an AArch64 toolchain which I can copy to an 
SD card for testing on an AArch64 board. @lancethepants could you check your 
CMake version where you got this to work? I'm using CMake 3.22.1 and trying to 
build llvmorg-13.0.0

The full invocation I'm using is the following:

  cmake -G Ninja \
-DLLVM_TABLEGEN=${TOP}/${CLANG_TBLGEN_DIR}/bin/llvm-tblgen \
-DCLANG_TABLEGEN=${TOP}/${CLANG_TBLGEN_DIR}/bin/clang-tblgen \
-DCMAKE_SYSTEM_NAME="Linux" \
-DCMAKE_CROSSCOMPILING=ON \
-DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF \
-DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \
-DCMAKE_CXX_COMPILER=aarch64-linux-gnu-g++ \
-DCMAKE_BUILD_TYPE=MinSizeRel \
-DCMAKE_INSTALL_PREFIX=${TOP}/${ROOT_DIR} \
-DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-linux-gnu \
-DLLVM_NATIVE_ARCH="X86" \
-DLLVM_TARGET_ARCH="AArch64" \
-DLLVM_TARGETS_TO_BUILD="AArch64" \
-DLLVM_ENABLE_PROJECTS="clang;compiler-rt;lld;" \
-DLLVM_ENABLE_TERMINFO=OFF \
-DLLVM_ENABLE_BINDINGS=OFF \
-DLLVM_BUILD_EXAMPLES=OFF \
-DLLVM_BUILD_TESTS=OFF \
-DLLVM_BUILD_BENCHMARKS=OFF \
${TOP}/${SRC_DIR}/llvm/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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

[PATCH] D115982: [clang][AVR] Implement '__flashN' for variables on different flash banks

[Ben Shi via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/AVR.cpp:27
   const char *DefineName;
+  const int MaxFlashBank; // -1 means the device does not support LPM/ELPM.
 };

aykevl wrote:
> This works and is fine, but I think you could also name it `NumFlashBanks` so 
> that it is the number of flash banks supported on the device (0 if LPM/ELPM 
> is not supported). With 0 for the attiny11 (which has no accessible flash 
> banks) and 1 for the attiny22 (because it has only one flash bank that can be 
> accessed: flash bank 0).
> 
> Just a suggestion, the current system looks good to me.
I will change it to NumFlashBanks when committing.


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

https://reviews.llvm.org/D115982

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

[PATCH] D115982: [clang][AVR] Implement '__flashN' for variables on different flash banks

[Ben Shi via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/avr-flash.c:10
+  static __flash5 const int a5[] = {4, 6}; // expected-error {{unknown type 
name '__flash5'}}
+  // TODO: It would be better to report "'__flash5' is not supported on 
at908515".
+  return a0[n] + a1[n];

aykevl wrote:
> I think this can be implemented by always setting the `__flash*` defines and 
> checking whether they are valid for the current device in 
> `getGlobalVarAddressSpace`.
Maybe it is better to do in `getGlobalVarAddressSpace` for reporting the exact 
source position.

```
a.c:1:20: error: variable ‘arr’ located in address space ‘__flash5’ beyond 
flash of 64 KiB
 __flash5 int const arr[8];
```

I will try it in the next patch.


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

https://reviews.llvm.org/D115982

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

[clang] 0731f6b - [Doc] Add documentation for the clang-offload-wrapper tool (NFC)

[Saiyedul Islam via cfe-commits]

Author: Saiyedul Islam
Date: 2022-01-19T10:45:13Z
New Revision: 0731f6ba4f5773071a7b91248caf79a287703a28

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

LOG: [Doc] Add documentation for the clang-offload-wrapper tool (NFC)

Add the missing documentation for this tool.

Reviewed By: sdmitriev

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

Added: 
clang/docs/ClangOffloadWrapper.rst

Modified: 
clang/docs/index.rst

Removed: 




diff  --git a/clang/docs/ClangOffloadWrapper.rst 
b/clang/docs/ClangOffloadWrapper.rst
new file mode 100644
index 0..2a1e9f3626080
--- /dev/null
+++ b/clang/docs/ClangOffloadWrapper.rst
@@ -0,0 +1,220 @@
+=
+Clang Offload Wrapper
+=
+
+.. contents::
+   :local:
+
+.. _clang-offload-wrapper:
+
+Introduction
+
+
+This tool is used in OpenMP offloading toolchain to embed device code objects
+(usually ELF) into a wrapper host llvm IR (bitcode) file. The wrapper host IR
+is then assembled and linked with host code objects to generate the executable
+binary. See :ref:`image-binary-embedding-execution` for more details.
+
+Usage
+=
+
+This tool can be used as follows:
+
+.. code-block:: console
+
+  $ clang-offload-wrapper -help
+  OVERVIEW: A tool to create a wrapper bitcode for offload target binaries.
+  Takes offload target binaries as input and produces bitcode file containing
+  target binaries packaged as data and initialization code which registers
+  target binaries in offload runtime.
+  USAGE: clang-offload-wrapper [options] 
+  OPTIONS:
+  Generic Options:
+--help - Display available options 
(--help-hidden for more)
+--help-list- Display list of available options 
(--help-list-hidden for more)
+--version  - Display the version of this program
+  clang-offload-wrapper options:
+-o=  - Output filename
+--target=  - Target triple for the output module
+
+Example
+===
+
+.. code-block:: console
+
+  clang-offload-wrapper -target host-triple -o host-wrapper.bc 
gfx90a-binary.out
+
+.. _openmp-device-binary_embedding:
+
+OpenMP Device Binary Embedding
+==
+
+Various structures and functions used in the wrapper host IR form the interface
+between the executable binary and the OpenMP runtime.
+
+Enum Types
+--
+
+:ref:`table-offloading-declare-target-flags` lists 
diff erent flag for
+offloading entries.
+
+  .. table:: Offloading Declare Target Flags Enum
+:name: table-offloading-declare-target-flags
+
+
+-+---+--+
+|  Name   | Value | Description
  |
+
+=+===+==+
+| OMP_DECLARE_TARGET_LINK | 0x01  | Mark the entry as having a 'link' 
attribute (w.r.t. link clause) |
+
+-+---+--+
+| OMP_DECLARE_TARGET_CTOR | 0x02  | Mark the entry as being a global 
constructor |
+
+-+---+--+
+| OMP_DECLARE_TARGET_DTOR | 0x04  | Mark the entry as being a global 
destructor  |
+
+-+---+--+
+
+Structure Types
+---
+
+:ref:`table-tgt_offload_entry`, :ref:`table-tgt_device_image`, and
+:ref:`table-tgt_bin_desc` are the structures used in the wrapper host IR.
+
+  .. table:: __tgt_offload_entry structure
+:name: table-tgt_offload_entry
+
+
+-+++
+|   Type  | Identifier | Description   
 |
+
+=+++
+|  void*  |addr| Address of global symbol within device image 
(function or global)  |
+
+-+++
+|  char*  |name| Name of the symbol
 |
+
+-+++
+|  size_t |size| Size of the entry info (0 if it is a function)

[PATCH] D117120: [Doc] Add documentation for the clang-offload-wrapper tool (NFC)

[Saiyedul Islam via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0731f6ba4f57: [Doc] Add documentation for the 
clang-offload-wrapper tool (NFC) (authored by saiislam).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117120

Files:
  clang/docs/ClangOffloadWrapper.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -84,6 +84,7 @@
ClangFormattedStatus
ClangNvlinkWrapper
ClangOffloadBundler
+   ClangOffloadWrapper
 
 Design Documents
 
Index: clang/docs/ClangOffloadWrapper.rst
===
--- /dev/null
+++ clang/docs/ClangOffloadWrapper.rst
@@ -0,0 +1,220 @@
+=
+Clang Offload Wrapper
+=
+
+.. contents::
+   :local:
+
+.. _clang-offload-wrapper:
+
+Introduction
+
+
+This tool is used in OpenMP offloading toolchain to embed device code objects
+(usually ELF) into a wrapper host llvm IR (bitcode) file. The wrapper host IR
+is then assembled and linked with host code objects to generate the executable
+binary. See :ref:`image-binary-embedding-execution` for more details.
+
+Usage
+=
+
+This tool can be used as follows:
+
+.. code-block:: console
+
+  $ clang-offload-wrapper -help
+  OVERVIEW: A tool to create a wrapper bitcode for offload target binaries.
+  Takes offload target binaries as input and produces bitcode file containing
+  target binaries packaged as data and initialization code which registers
+  target binaries in offload runtime.
+  USAGE: clang-offload-wrapper [options] 
+  OPTIONS:
+  Generic Options:
+--help - Display available options (--help-hidden for more)
+--help-list- Display list of available options (--help-list-hidden for more)
+--version  - Display the version of this program
+  clang-offload-wrapper options:
+-o=  - Output filename
+--target=  - Target triple for the output module
+
+Example
+===
+
+.. code-block:: console
+
+  clang-offload-wrapper -target host-triple -o host-wrapper.bc gfx90a-binary.out
+
+.. _openmp-device-binary_embedding:
+
+OpenMP Device Binary Embedding
+==
+
+Various structures and functions used in the wrapper host IR form the interface
+between the executable binary and the OpenMP runtime.
+
+Enum Types
+--
+
+:ref:`table-offloading-declare-target-flags` lists different flag for
+offloading entries.
+
+  .. table:: Offloading Declare Target Flags Enum
+:name: table-offloading-declare-target-flags
+
++-+---+--+
+|  Name   | Value | Description  |
++=+===+==+
+| OMP_DECLARE_TARGET_LINK | 0x01  | Mark the entry as having a 'link' attribute (w.r.t. link clause) |
++-+---+--+
+| OMP_DECLARE_TARGET_CTOR | 0x02  | Mark the entry as being a global constructor |
++-+---+--+
+| OMP_DECLARE_TARGET_DTOR | 0x04  | Mark the entry as being a global destructor  |
++-+---+--+
+
+Structure Types
+---
+
+:ref:`table-tgt_offload_entry`, :ref:`table-tgt_device_image`, and
+:ref:`table-tgt_bin_desc` are the structures used in the wrapper host IR.
+
+  .. table:: __tgt_offload_entry structure
+:name: table-tgt_offload_entry
+
++-+++
+|   Type  | Identifier | Description|
++=+++
+|  void*  |addr| Address of global symbol within device image (function or global)  |
++-+++
+|  char*  |name| Name of the symbol |
++-+++
+|  size_t |size| Size of the entry info (0 if it is a function) |

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst:22
+  std::shared_ptr x(new Foo[10]); // -> std::shared_ptr x(new 
Foo[10]);
+  // ^ warning: shared pointer to non-array is initialized 
with array [nugprone-shared-ptr-array-mismatch]
+  std::shared_ptr x1(new Foo), x2(new Foo[10]); // no replacement 

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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

[clang] 6ee9654 - [Doc] Fix wrong indentation

[Saiyedul Islam via cfe-commits]

Author: Saiyedul Islam
Date: 2022-01-19T11:15:31Z
New Revision: 6ee965471363972fafbed60ad8d27d0f666f8671

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

LOG: [Doc] Fix wrong indentation

Handle Sphinx's warning at line 218.

Added: 


Modified: 
clang/docs/ClangOffloadWrapper.rst

Removed: 




diff  --git a/clang/docs/ClangOffloadWrapper.rst 
b/clang/docs/ClangOffloadWrapper.rst
index 2a1e9f362608..efd042509547 100644
--- a/clang/docs/ClangOffloadWrapper.rst
+++ b/clang/docs/ClangOffloadWrapper.rst
@@ -214,7 +214,10 @@ For each offloading target, device ELF code objects are 
generated by ``clang``,
 
   * At compile time, the ``clang-offload-wrapper`` tool takes the following
 actions:
+
 * It embeds the ELF code objects for the device into the host code (see
   :ref:`openmp-device-binary_embedding`).
+
   * At execution time:
+
 * The global constructor gets run and it registers the device image.



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

[clang] a2f488c - [clang][AVR] Implement '__flashN' for variables on different flash banks

[Ben Shi via cfe-commits]

Author: Ben Shi
Date: 2022-01-19T11:24:01Z
New Revision: a2f488c6a5ce251426e4108fe3276f554f5bafac

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

LOG: [clang][AVR] Implement '__flashN' for variables on different flash banks

Reviewed By: aykevl

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

Added: 
clang/test/Sema/avr-flash.c

Modified: 
clang/lib/Basic/Targets/AVR.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/address-space-avr.c
clang/test/CodeGen/avr-flash.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index 50b0fc07b3118..6266ed72cd5c0 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -24,281 +24,282 @@ namespace targets {
 struct LLVM_LIBRARY_VISIBILITY MCUInfo {
   const char *Name;
   const char *DefineName;
+  const int NumFlashBanks; // -1 means the device does not support LPM/ELPM.
 };
 
 // This list should be kept up-to-date with AVRDevices.td in LLVM.
 static MCUInfo AVRMcus[] = {
-{"at90s1200", "__AVR_AT90S1200__"},
-{"attiny11", "__AVR_ATtiny11__"},
-{"attiny12", "__AVR_ATtiny12__"},
-{"attiny15", "__AVR_ATtiny15__"},
-{"attiny28", "__AVR_ATtiny28__"},
-{"at90s2313", "__AVR_AT90S2313__"},
-{"at90s2323", "__AVR_AT90S2323__"},
-{"at90s2333", "__AVR_AT90S2333__"},
-{"at90s2343", "__AVR_AT90S2343__"},
-{"attiny22", "__AVR_ATtiny22__"},
-{"attiny26", "__AVR_ATtiny26__"},
-{"at86rf401", "__AVR_AT86RF401__"},
-{"at90s4414", "__AVR_AT90S4414__"},
-{"at90s4433", "__AVR_AT90S4433__"},
-{"at90s4434", "__AVR_AT90S4434__"},
-{"at90s8515", "__AVR_AT90S8515__"},
-{"at90c8534", "__AVR_AT90c8534__"},
-{"at90s8535", "__AVR_AT90S8535__"},
-{"ata5272", "__AVR_ATA5272__"},
-{"attiny13", "__AVR_ATtiny13__"},
-{"attiny13a", "__AVR_ATtiny13A__"},
-{"attiny2313", "__AVR_ATtiny2313__"},
-{"attiny2313a", "__AVR_ATtiny2313A__"},
-{"attiny24", "__AVR_ATtiny24__"},
-{"attiny24a", "__AVR_ATtiny24A__"},
-{"attiny4313", "__AVR_ATtiny4313__"},
-{"attiny44", "__AVR_ATtiny44__"},
-{"attiny44a", "__AVR_ATtiny44A__"},
-{"attiny84", "__AVR_ATtiny84__"},
-{"attiny84a", "__AVR_ATtiny84A__"},
-{"attiny25", "__AVR_ATtiny25__"},
-{"attiny45", "__AVR_ATtiny45__"},
-{"attiny85", "__AVR_ATtiny85__"},
-{"attiny261", "__AVR_ATtiny261__"},
-{"attiny261a", "__AVR_ATtiny261A__"},
-{"attiny441", "__AVR_ATtiny441__"},
-{"attiny461", "__AVR_ATtiny461__"},
-{"attiny461a", "__AVR_ATtiny461A__"},
-{"attiny841", "__AVR_ATtiny841__"},
-{"attiny861", "__AVR_ATtiny861__"},
-{"attiny861a", "__AVR_ATtiny861A__"},
-{"attiny87", "__AVR_ATtiny87__"},
-{"attiny43u", "__AVR_ATtiny43U__"},
-{"attiny48", "__AVR_ATtiny48__"},
-{"attiny88", "__AVR_ATtiny88__"},
-{"attiny828", "__AVR_ATtiny828__"},
-{"at43usb355", "__AVR_AT43USB355__"},
-{"at76c711", "__AVR_AT76C711__"},
-{"atmega103", "__AVR_ATmega103__"},
-{"at43usb320", "__AVR_AT43USB320__"},
-{"attiny167", "__AVR_ATtiny167__"},
-{"at90usb82", "__AVR_AT90USB82__"},
-{"at90usb162", "__AVR_AT90USB162__"},
-{"ata5505", "__AVR_ATA5505__"},
-{"atmega8u2", "__AVR_ATmega8U2__"},
-{"atmega16u2", "__AVR_ATmega16U2__"},
-{"atmega32u2", "__AVR_ATmega32U2__"},
-{"attiny1634", "__AVR_ATtiny1634__"},
-{"atmega8", "__AVR_ATmega8__"},
-{"ata6289", "__AVR_ATA6289__"},
-{"atmega8a", "__AVR_ATmega8A__"},
-{"ata6285", "__AVR_ATA6285__"},
-{"ata6286", "__AVR_ATA6286__"},
-{"atmega48", "__AVR_ATmega48__"},
-{"atmega48a", "__AVR_ATmega48A__"},
-{"atmega48pa", "__AVR_ATmega48PA__"},
-{"atmega48pb", "__AVR_ATmega48PB__"},
-{"atmega48p", "__AVR_ATmega48P__"},
-{"atmega88", "__AVR_ATmega88__"},
-{"atmega88a", "__AVR_ATmega88A__"},
-{"atmega88p", "__AVR_ATmega88P__"},
-{"atmega88pa", "__AVR_ATmega88PA__"},
-{"atmega88pb", "__AVR_ATmega88PB__"},
-{"atmega8515", "__AVR_ATmega8515__"},
-{"atmega8535", "__AVR_ATmega8535__"},
-{"atmega8hva", "__AVR_ATmega8HVA__"},
-{"at90pwm1", "__AVR_AT90PWM1__"},
-{"at90pwm2", "__AVR_AT90PWM2__"},
-{"at90pwm2b", "__AVR_AT90PWM2B__"},
-{"at90pwm3", "__AVR_AT90PWM3__"},
-{"at90pwm3b", "__AVR_AT90PWM3B__"},
-{"at90pwm81", "__AVR_AT90PWM81__"},
-{"ata5790", "__AVR_ATA5790__"},
-{"ata5795", "__AVR_ATA5795__"},
-{"atmega16", "__AVR_ATmega16__"},
-{"atmega16a", "__AVR_ATmega16A__"},
-{"atmega161", "__AVR_ATmega161__"},
-{"atmega162", "__AVR_ATmega162__"},
-{"atmega163", "__AVR_ATmega163__"},
-{"atmega164a", "__AVR_ATmega164A__"},
-{"atmega164p", "__AVR_ATmega164P__"},
-{"atmega164pa", "__AVR_ATmega164PA__"},

[PATCH] D115982: [clang][AVR] Implement '__flashN' for variables on different flash banks

[Ben Shi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2f488c6a5ce: [clang][AVR] Implement '__flashN' 
for variables on different flash banks (authored by benshi001).

Changed prior to commit:
  https://reviews.llvm.org/D115982?vs=398846&id=401159#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115982

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/address-space-avr.c
  clang/test/CodeGen/avr-flash.c
  clang/test/Sema/avr-flash.c

Index: clang/test/Sema/avr-flash.c
===
--- /dev/null
+++ clang/test/Sema/avr-flash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -triple avr -target-cpu at90s8515 -fsyntax-only -verify
+
+int foo(int n) {
+  static __flash  const int a0[] = {4, 6}; // OK
+  static __flash1 const int a1[] = {4, 6}; // expected-error {{unknown type name '__flash1'}}
+  static __flash2 const int a2[] = {4, 6}; // expected-error {{unknown type name '__flash2'}}
+  static __flash3 const int a3[] = {4, 6}; // expected-error {{unknown type name '__flash3'}}
+  static __flash4 const int a4[] = {4, 6}; // expected-error {{unknown type name '__flash4'}}
+  static __flash5 const int a5[] = {4, 6}; // expected-error {{unknown type name '__flash5'}}
+  // TODO: It would be better to report "'__flash5' is not supported on at908515".
+  return a0[n] + a1[n];
+}
Index: clang/test/CodeGen/avr-flash.c
===
--- clang/test/CodeGen/avr-flash.c
+++ clang/test/CodeGen/avr-flash.c
@@ -1,6 +1,11 @@
-// RUN: %clang_cc1 -triple avr -emit-llvm-only -verify %s
+// RUN: %clang_cc1 -triple avr -target-cpu atxmega384c3 -emit-llvm-only -verify %s
 
 int foo(void) {
-  static __flash int b[] = {4, 6}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash'}}
-  return b[0];
+  static __flash  int b[] = {4, 6}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  static __flash1 int c[] = {8, 1}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  static __flash2 int d[] = {8, 1}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  static __flash3 int e[] = {8, 1}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  static __flash4 int f[] = {8, 1}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  static __flash5 int g[] = {8, 1}; // expected-error {{qualifier 'const' is needed for variables in address space '__flash*'}}
+  return b[0] + c[1];
 }
Index: clang/test/CodeGen/address-space-avr.c
===
--- clang/test/CodeGen/address-space-avr.c
+++ clang/test/CodeGen/address-space-avr.c
@@ -1,8 +1,11 @@
-// RUN: %clang_cc1 -triple avr -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -triple avr -target-cpu atmega2560 -emit-llvm < %s | FileCheck %s
 
 // CHECK: @var0 {{.*}} addrspace(1) constant [3 x i16]
+// CHECK: @f3var0 {{.*}} addrspace(4) constant [3 x i16]
 // CHECK: @bar.var2 {{.*}} addrspace(1) constant [3 x i16]
+// CHECK: @bar.f3var2 {{.*}} addrspace(4) constant [3 x i16]
 // CHECK: @var1 {{.*}} addrspace(1) constant [3 x i16]
+// CHECK: @f3var1 {{.*}} addrspace(4) constant [3 x i16]
 
 // CHECK: define{{.*}} void @bar() addrspace(1)
 // CHECK: call addrspace(1) void bitcast (void (...) addrspace(1)* @foo to void (i16) addrspace(1)*)
@@ -11,11 +14,16 @@
 __flash const int var0[] = {999, 888, 777};
 __flash static const int var1[] = {111, 222, 333};
 
+__flash3 const int f3var0[] = {12, 34, 56};
+__flash3 static const int f3var1[] = {52, 64, 96};
+
 int i;
 
 void foo();
 
 void bar() {
   static __flash const int var2[] = {555, 666, 777};
+  static __flash3 const int f3var2[] = {, , 7787};
   foo(var1[i]);
+  foo(f3var1[i]);
 }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8282,14 +8282,15 @@
 
   LangAS getGlobalVarAddressSpace(CodeGenModule &CGM,
   const VarDecl *D) const override {
-// Check if a global/static variable is defined within address space 1
+// Check if global/static variable is defined in address space
+// 1~6 (__flash, __flash1, __flash2, __flash3, __flash4, __flash5)
 // but not constant.
 LangAS AS = D->getType().getAddressSpace();
-if (isTargetAddressSpace(AS) && toTargetAddressSpace(AS) == 1 &&
-!D->getType().isConstQualified())
+if (isTargetAddressSpace(AS) && 1 <= toTargetAddressSpace(AS) &&
+toTargetAddressSpace(AS) <= 6 && !D->getType().isConstQualified())
   CGM.getDiags().

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

[Martin Storsjö via Phabricator via cfe-commits]

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

I think this looks reasonable. I haven't tried it, but I guess we should 
proceed with it instead of holding it back, as there's nobody else reviewing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116088

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

[PATCH] D111081: [clang] [MinGW] Fix paths on Gentoo

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D111081#3212890 , @mgorny wrote:

> Well, I dunno how MinGW works, so can't judge that part. Furthermore, I'm 
> thoroughly confused why this works on top of existing (non-Gentoo?) test 
> tree. Could you explain it a bit more?

It looks like the clang driver just blindly adds these directories to the list 
of include paths, regardless of whether they exist or not. So with the current 
test, they don't need to exist (but maybe it would be good to create such empty 
directories to make the test a bit more correct, in case the driver is modified 
to not add nonexistent directories blindly).

FYI @maharmstone the test needs to be rebased to fit on top of current git, 
after rG897c86dec5af2780d443edd852aa5911e2650ec6 
.


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

https://reviews.llvm.org/D111081

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

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:2371
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+

I think it would be useful for the user to see one example per constraint type 
that this checker supports.
RangeConstraint (was covered), ComparisonConstraint, ValueConstraint, Not null  
Constraint, BufferSize constraint etc.


It would be also nice to add a section "Limitations".

Describe there well known false positive cases or limitations in the bug 
diagnostics that limits understandability.
Essentially the most important well known cases why this checker is alpha.

This section would be useful for users to understand and help identifying cases 
that are known false positives and for the developers to know how to improve 
this checker. I remember many cases when we had to test multiple times "why a 
checker is in alpha", because we forgot about it. I think it is best to 
document it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117568

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

[PATCH] D114966: [clang][deps] Split stat and file content caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 401171.
jansvoboda11 marked 7 inline comments as done.
jansvoboda11 added a comment.

Update comments, remove `std::atomic<>`, merge `getOrEmplaceContentsForUID` 
into `getOrEmplaceEntryForUID`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114966

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- clang/unittests/Tooling/DependencyScannerTest.cpp
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -224,6 +224,8 @@
   EXPECT_TRUE(StatusFull1);
   EXPECT_EQ(StatusMinimized0->getSize(), 17u);
   EXPECT_EQ(StatusFull1->getSize(), 30u);
+  EXPECT_EQ(StatusMinimized0->getName(), StringRef("/mod.h"));
+  EXPECT_EQ(StatusFull1->getName(), StringRef("/mod.h"));
 }
 
 TEST(DependencyScanningFilesystem, IgnoredFilesAreCachedSeparately2) {
@@ -245,6 +247,8 @@
   EXPECT_TRUE(StatusMinimized1);
   EXPECT_EQ(StatusFull0->getSize(), 30u);
   EXPECT_EQ(StatusMinimized1->getSize(), 17u);
+  EXPECT_EQ(StatusFull0->getName(), StringRef("/mod.h"));
+  EXPECT_EQ(StatusMinimized1->getName(), StringRef("/mod.h"));
 }
 
 } // end namespace dependencies
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
@@ -16,10 +16,10 @@
 using namespace tooling;
 using namespace dependencies;
 
-llvm::ErrorOr
-CachedFileSystemEntry::initFile(StringRef Filename, llvm::vfs::FileSystem &FS) {
+llvm::ErrorOr
+DependencyScanningWorkerFilesystem::readFile(StringRef Filename) {
   // Load the file and its content from the file system.
-  auto MaybeFile = FS.openFileForRead(Filename);
+  auto MaybeFile = getUnderlyingFS().openFileForRead(Filename);
   if (!MaybeFile)
 return MaybeFile.getError();
   auto File = std::move(*MaybeFile);
@@ -34,24 +34,42 @@
 return MaybeBuffer.getError();
   auto Buffer = std::move(*MaybeBuffer);
 
-  OriginalContents = std::move(Buffer);
-  return Stat;
+  // If the file size changed between read and stat, pretend it didn't.
+  if (Stat.getSize() != Buffer->getBufferSize())
+Stat = llvm::vfs::Status::copyWithNewSize(Stat, Buffer->getBufferSize());
+
+  return TentativeEntry(Stat, std::move(Buffer));
 }
 
-void CachedFileSystemEntry::minimizeFile() {
-  assert(OriginalContents && "minimizing missing contents");
+EntryRef DependencyScanningWorkerFilesystem::minimizeIfNecessary(
+const CachedFileSystemEntry &Entry, StringRef Filename) {
+  if (Entry.isError() || Entry.isDirectory() || !shouldMinimize(Filename))
+return EntryRef(/*Minimized=*/false, Filename, Entry);
+
+  CachedFileContents *Contents = Entry.getContents();
+  assert(Contents && "contents not initialized");
+
+  // Double-checked locking.
+  if (Contents->MinimizedAccess.load())
+return EntryRef(/*Minimized=*/true, Filename, Entry);
+
+  std::lock_guard GuardLock(Contents->ValueLock);
+
+  // Double-checked locking.
+  if (Contents->MinimizedAccess.load())
+return EntryRef(/*Minimized=*/true, Filename, Entry);
 
   llvm::SmallString<1024> MinimizedFileContents;
   // Minimize the file down to directives that might affect the dependencies.
   SmallVector Tokens;
-  if (minimizeSourceToDependencyDirectives(OriginalContents->getBuffer(),
+  if (minimizeSourceToDependencyDirectives(Contents->Original->getBuffer(),
MinimizedFileContents, Tokens)) {
 // FIXME: Propagate the diagnostic if desired by the client.
 // Use the original file if the minimization failed.
-MinimizedContentsStorage =
-llvm::MemoryBuffer::getMemBuffer(*OriginalContents);
-MinimizedContentsAccess.store(MinimizedContentsStorage.get());
-return;
+Contents->MinimizedStorage =
+llvm::MemoryBuffer::getMemBuffer(*Contents->Original);
+Contents->MinimizedAccess.store(Contents->MinimizedStorage.get());
+return EntryRef(/*Minimized=*/true, Filename, Entry);
   }
 
   // The contents produced by the minimizer must be null terminated.
@@ -74,16 +92,17 @@
 }
 Mapping[Range.Offset] = Range.Length;
   }
-  PPSkippedRangeMapping = std::move(Mapping);
+  Contents->PPSkippedRangeMapping = std::move(Mapping);
 
-  MinimizedContentsStorage = std::make_unique(
+  Contents->MinimizedStorage = std::make_unique(
   std::move(MinimizedFileContents));
-  // The algorithm in `getOrCreateFileSystemEntry` uses the presence of
-  // minimized contents to decide whether an entry is up-to-date or not.
-  // If it is up-to-date, the skipped 

[PATCH] D114966: [clang][deps] Split stat and file content caches

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:132-134
+  /// Non-owning pointer to the file contents. This can be shared between
+  /// multiple entries (e.g. between a symlink and its target).
+  std::atomic ContentsAccess;

dexonsmith wrote:
> This comment is out of date, since there's a 1:1 correspondence between 
> CachedFileSystemEntry and CachedFileContents since they're both looked up by 
> UID.
> 
> Also, `ContentsAccess` is never modified after construction so it can just be 
> a raw pointer.
> 
> Outlining the allocation of `CachedFileContents` might still make sense, 
> since it's big and stat failures (with no content) are probably the common 
> case due to header search patterns... only if we actually create these for 
> stat failures though. Might be worth a comment saying why it's outlined. 
> Maybe it should even be delayed to a follow-up commit to simplify this patch, 
> since now that CachedFileSystemEntry is per-UID it doesn't seem to be 
> required here... but the fields probably need to be made `mutable` regardless 
> somehow so it doesn't seem like a ton of churn.
I've updated the comment and changed `ContentsAccess` to a raw pointer.

I agree outlining `CachedFileContents` in a follow-up patch would be cleaner, 
but I don't have much time to spare on prettifying git history at this moment 
unfortunately.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:160-161
+
+/// Map from unique IDs to cached contents.
+llvm::DenseMap 
ContentsCache;
+

dexonsmith wrote:
> I think this map can be deleted, since it's not actually used to 
> deduplicate/share anything that `EntriesByUID` doesn't handle.
Yeah, that's right. Removed this in the latest revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114966

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

[PATCH] D113676: [clang][lex] Fix search path usage remark with modules

[Jan Svoboda via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:264
+  if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) {
+noteModuleLookupUsage(Module, ImportLoc);
 return Module;

jansvoboda11 wrote:
> ahoppen wrote:
> > Just a thought: Could we move `noteModuleLookupUsage` into `findModule`? 
> > IIUC that would guarantee that we’re catching all cases and we wouldn’t 
> > need to call `noteModuleLookupUsage ` from both overloads of `lookupModule`.
> That would clean up `HeaderSearch` nicely. I'll look into creating new 
> `HeaderSearch::findModule` function that would wrap `ModuleMap::findModule` 
> and note usage.
Looking more into this, `ModuleMap::findModule` is used in a lot of different 
places. Since `noteModuleLookupUsage` takes `SourceLocation` of the `#include`, 
putting it into `findModule` that doesn't require `SourceLocation` ATM would be 
somewhat intrusive change.

I have cleaned up this patch so that `noteModuleLookupUsage` is called only in 
single `HeaderSearch::lookupModule`. I think this is good enough. WDYT?



Comment at: clang/lib/Lex/ModuleMap.cpp:851
   new Module("", Loc, Parent, /*IsFramework*/ false,
  /*IsExplicit*/ true, NumCreatedModules++);
   Result->Kind = Module::PrivateModuleFragment;

jansvoboda11 wrote:
> ahoppen wrote:
> > Maybe a stupid question but why don’t we need to call the callback e.g. 
> > here (or on the other `new Module`) calls in this file?
> This is related to C++20 modules, so it wasn't necessary for the test case I 
> had in mind.
> 
> But I agree we should handle this as well. I think more robust solution would 
> be to add factory function to `ModuleMap` through which all `Module` 
> instances get created, and invoke the callback there.
This is fixed in D116751.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113676

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

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 401175.
hokein marked 8 inline comments as done.
hokein added a comment.

refine the patch:

- address comments
- remove unneeded code
- move the BNF-grammar parsing code to a dedicate class, rather in Grammar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/lib/Tooling/Syntax/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/unittests/Tooling/Syntax/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/GrammarTests.cpp
@@ -0,0 +1,103 @@
+//===--- GrammarTests.cpp - grammar tests  *-
+//C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Pseudo/Grammar.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+namespace {
+
+using testing::AllOf;
+using testing::ElementsAre;
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+MATCHER_P(TargetID, SID, "") { return arg.target() == SID; }
+template  testing::Matcher Sequence(T... IDs) {
+  return testing::Property(&Rule::seq, ElementsAre(IDs...));
+}
+
+class GrammarTest : public ::testing::Test {
+public:
+  void build(llvm::StringRef BNF) {
+Diags.clear();
+G = Grammar::parseBNF(BNF, Diags);
+  }
+
+  SymbolID lookup(llvm::StringRef Name) const {
+for (unsigned I = 0; I < NumTerminals; ++I)
+  if (G->table().Terminals[I] == Name)
+return tokenSymbol(static_cast(I));
+for (SymbolID ID = 0; ID < G->table().Nonterminals.size(); ++ID)
+  if (G->table().Nonterminals[ID].Name == Name)
+return ID;
+assert(false);
+  }
+
+protected:
+  std::unique_ptr G;
+  std::vector Diags;
+};
+
+TEST_F(GrammarTest, Basic) {
+  build("expression := IDENTIFIER + expression # comment");
+  EXPECT_THAT(Diags, IsEmpty());
+
+  auto ExpectedRule =
+  AllOf(TargetID(lookup("expression")),
+Sequence(lookup("IDENTIFIER"), lookup("+"), lookup("expression")));
+  auto ExpressionID = lookup("expression");
+  // auto RuleID = 0;
+  EXPECT_EQ(G->symbolName(ExpressionID), "expression");
+  EXPECT_THAT(G->lookupRules(ExpressionID), UnorderedElementsAre(ExpectedRule));
+  const auto &Rule = G->lookupRule(/*RID=*/0);
+  EXPECT_THAT(Rule, ExpectedRule);
+  EXPECT_THAT(G->symbolName(Rule.seq()[0]), "IDENTIFIER");
+  EXPECT_THAT(G->symbolName(Rule.seq()[1]), "+");
+  EXPECT_THAT(G->symbolName(Rule.seq()[2]), "expression");
+}
+
+TEST_F(GrammarTest, EliminatedOptional) {
+  build("_ := CONST_opt INT ;_opt");
+  EXPECT_THAT(Diags, IsEmpty());
+  EXPECT_THAT(G->table().Rules,
+  UnorderedElementsAre(
+  Sequence(lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT")),
+  Sequence(lookup("CONST"), lookup("INT"), lookup(";")),
+  Sequence(lookup("INT"), lookup(";";
+}
+
+TEST_F(GrammarTest, Diagnostics) {
+  build(R"cpp(
+_ := ,_opt
+_ := undefined-sym
+null :=
+_ := IDENFIFIE # a typo of the terminal IDENFITIER
+
+invalid
+  )cpp");
+
+  EXPECT_THAT(Diags, UnorderedElementsAre(
+ "Rule '_ := ,_opt' emits a nullable symbol",
+ "Rule 'null := ' emits a nullable symbol",
+ "No rules for nonterminal: undefined-sym",
+ "Failed to parse 'invalid': no separator :=",
+ "Token-like name IDENFIFIE is used as a nonterminal",
+ "No rules for nonterminal: IDENFIFIE"));
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace syntax
+} // namespace clang
Index: clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
@@ -0,0 +1,21 @@
+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+add_custom_target(ClangPseudoUnitTests)
+add_unittest(ClangPseudoUnitTests ClangPseudoTests
+  GrammarTests.cpp
+  )
+
+clang_target_link_libraries(ClangPseudoTests
+  PRIVATE
+  clangBasic
+  clangLex
+  clangSyntaxPseudo
+  clangTesting
+  )
+
+target_link_libraries(ClangPseudoTests
+  PRIVATE
+  LLVMTestingSupport
+  )
Index: clang/unittests/Tooling

[PATCH] D114790: [syntax][pseudo] Add grammar facilities for the pseudo-parser

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:52
+// Terminal IDs correspond to the clang TokenKind enum.
+using SymbolID = uint16_t;
+// SymbolID is only 12 bits wide.

sammccall wrote:
> If we want strong types, we could use `enum class SymbolID : uint16_t {};`
we could do that, but I'd leave it as-is at the moment (until we figure out 
more details about static Grammar construction). 



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:110
+// Grammar that describes a programming language, C++ etc.
+// It parses BNF grammar rules, and is a building brick for constructing a
+// grammar-based parser.

sammccall wrote:
> sammccall wrote:
> > nit: building block is more idiomatic
> Nit: it's rather the RuleSpec factories and Grammar::build (which are static) 
> that parses BNF grammar.
> The Grammar object itself doesn't have this responsibility.
Yeah, you're right. Move the BNF-parsing bit to a dedicated place.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:144
+
+  // Lookup non-terminal by name.
+  SymbolID lookupSymbol(llvm::StringRef Name) const;

sammccall wrote:
> The implementation also (inefficiently) supports looking up terminals. 
> Intended?
this is a method function only being used for testing, I don't think we need 
it. Moved to the test.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:171
+
+// Underlying storage of the Grammar.
+struct Table {

sammccall wrote:
> Are compiled transition tables etc expected to live here, or is this a purely 
> descriptive version of teh grammar?
The later, the Grammar class purely handles grammar symbols/rules. Transition 
table will be lived somewhere else (table parser), and it will be generated 
from the TransitionTableBuilder which depends on Grammar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114790

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

[PATCH] D117549: [clangd] Sort targets before printing for tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 401178.
kadircet marked an inline comment as done.
kadircet added a comment.

- Store in vector and sort


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117549

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1283,11 +1283,7 @@
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
+   // Handle UnresolvedLookupExpr.
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,13 +33,16 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -1169,14 +1172,13 @@
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
-  bool First = true;
+  llvm::SmallVector Targets;
   for (const NamedDecl *T : R.Targets) {
-if (!First)
-  OS << ", ";
-else
-  First = false;
-OS << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
+llvm::raw_string_ostream Target(Targets.emplace_back());
+Target << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
   }
+  llvm::sort(Targets);
+  OS << llvm::join(Targets, ", ");
   OS << "}";
   if (R.Qualifier) {
 OS << ", qualifier = '";


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1283,11 +1283,7 @@
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
+   // Handle UnresolvedLookupExpr.
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,13 +33,16 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -1169,14 +1172,13 @@
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
-  bool First = true;
+  llvm::SmallVector Targets;
   for (const NamedDecl *T : R.Targets) {
-if (!First)
-  OS << ", ";
-else
-  First = false;
-OS << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
+llvm::raw_string_ostream Target(Targets.emplace_back());
+Target << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
   }
+  llvm::sort(Targets);
+  OS << llvm::join(Targets, ", ");
   OS << "}";
   if (R.Qualifier) {
 OS << ", qualifier = '";
___

[clang-tools-extra] cae932b - [clangd] Sort targets before printing for tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-01-19T14:06:53+01:00
New Revision: cae932b6c6d3caedca4ad39d6e1345bcdbc0c863

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

LOG: [clangd] Sort targets before printing for tests

Targets are not necessarily inserted in the order they appear in source
code. For example we could traverse overload sets, or selectively insert
template patterns after all other decls.
So order the targets before printing to make sure tests are not dependent on
such implementation details. We can also do it in production, but that might be
wasteful as we haven't seen any complaints in the wild around these orderings
yet.

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index 8b363efc99196..e96aa25fd780c 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -33,13 +33,16 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -1169,14 +1172,13 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, 
DeclRelationSet RS) {
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
-  bool First = true;
+  llvm::SmallVector Targets;
   for (const NamedDecl *T : R.Targets) {
-if (!First)
-  OS << ", ";
-else
-  First = false;
-OS << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
+llvm::raw_string_ostream Target(Targets.emplace_back());
+Target << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
   }
+  llvm::sort(Targets);
+  OS << llvm::join(Targets, ", ");
   OS << "}";
   if (R.Qualifier) {
 OS << ", qualifier = '";

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 9620db9838ae2..4887ee5b5deb3 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1283,11 +1283,7 @@ TEST_F(FindExplicitReferencesTest, All) {
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
+   // Handle UnresolvedLookupExpr.
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@ TEST_F(FindExplicitReferencesTest, All) {
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {



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

[PATCH] D117549: [clangd] Sort targets before printing for tests

[Kadir Cetinkaya 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 rGcae932b6c6d3: [clangd] Sort targets before printing for 
tests (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117549

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1283,11 +1283,7 @@
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
+   // Handle UnresolvedLookupExpr.
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,13 +33,16 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -1169,14 +1172,13 @@
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
-  bool First = true;
+  llvm::SmallVector Targets;
   for (const NamedDecl *T : R.Targets) {
-if (!First)
-  OS << ", ";
-else
-  First = false;
-OS << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
+llvm::raw_string_ostream Target(Targets.emplace_back());
+Target << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
   }
+  llvm::sort(Targets);
+  OS << llvm::join(Targets, ", ");
   OS << "}";
   if (R.Qualifier) {
 OS << ", qualifier = '";


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1283,11 +1283,7 @@
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
+   // Handle UnresolvedLookupExpr.
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,13 +33,16 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -1169,14 +1172,13 @@
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceLoc R) {
   // note we cannot print R.NameLoc without a source manager.
   OS << "targets = {";
-  bool First = true;
+  llvm::SmallVector Targets;
   for (const NamedDecl *T : R.Targets) {
-if (!First)
-  OS << ", ";
-else
-  First = false;
-OS << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
+llvm::raw_string_ostream Target(Targets.emplace_back());
+Target << printQualifiedName(*T) << printTemplateSpecializationArgs(*T);
   }
+  llvm::sort(Targets);
+  OS << llvm::join(Target

[clang-tools-extra] 6c78703 - [AST] Fix the incorrect auto-keyword loc for constrained auto type loc.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-19T14:18:38+01:00
New Revision: 6c78703e3abcd9c76aefbf066869fe6a32d59516

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

LOG: [AST] Fix the incorrect auto-keyword loc for constrained auto type loc.

E.g.  `Concept auto Func();`

The nameLoc for the constained auto type loc pointed to the concept name
loc, it should be the auto token loc. This patch fixes it, and remove
a relevant hack in clang-tidy check.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
clang/lib/Parse/ParseDecl.cpp
clang/unittests/AST/SourceLocationTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
index 2b9907d162664..60b80af35c7d8 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
@@ -285,31 +285,6 @@ SourceRange 
UseTrailingReturnTypeCheck::findReturnTypeAndCVSourceRange(
 return {};
   }
 
-  // If the return type is a constrained 'auto', we need to include the token
-  // after the concept. Unfortunately, the source range of an AutoTypeLoc, if 
it
-  // is constrained, does not include the 'auto'.
-  // FIXME: fix the AutoTypeLoc location in clang.
-  auto ATL = ReturnLoc.getAs();
-  if (ATL && ATL.isConstrained() && !ATL.isDecltypeAuto()) {
-SourceLocation End =
-expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
-SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
-
-// Extend the ReturnTypeRange until the last token before the function
-// name.
-std::pair Loc = SM.getDecomposedLoc(End);
-StringRef File = SM.getBufferData(Loc.first);
-const char *TokenBegin = File.data() + Loc.second;
-Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(),
-TokenBegin, File.end());
-Token T;
-SourceLocation LastTLoc = End;
-while (!Lexer.LexFromRawLexer(T) &&
-   SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) {
-  LastTLoc = T.getLocation();
-}
-ReturnTypeRange.setEnd(LastTLoc);
-  }
 
   // If the return type has no local qualifiers, it's source range is accurate.
   if (!hasAnyNestedLocalQualifiers(F.getReturnType()))

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index cff8c76183f6a..f21938c81689b 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3582,7 +3582,7 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
   isInvalid = DS.SetTypeSpecType(TST_decltype_auto, Loc, PrevSpec,
  DiagID, TemplateId, Policy);
 } else {
-  isInvalid = DS.SetTypeSpecType(TST_auto, Loc, PrevSpec, DiagID,
+  isInvalid = DS.SetTypeSpecType(TST_auto, AutoLoc, PrevSpec, DiagID,
  TemplateId, Policy);
 }
 break;

diff  --git a/clang/unittests/AST/SourceLocationTest.cpp 
b/clang/unittests/AST/SourceLocationTest.cpp
index a798f6b2225ba..28205c212f61d 100644
--- a/clang/unittests/AST/SourceLocationTest.cpp
+++ b/clang/unittests/AST/SourceLocationTest.cpp
@@ -247,6 +247,14 @@ TEST(TypeLoc, AutoTypeLocRange) {
   Verifier.expectRange(1, 1, 1, 14);
   EXPECT_TRUE(Verifier.match("decltype(auto) a = 1;", typeLoc(loc(autoType())),
  Lang_CXX14));
+
+  const char *Code =
+  R"cpp(template  concept C = true;
+C auto abc();
+)cpp";
+  // Should include "C auto" tokens.
+  Verifier.expectRange(2, 1, 2, 3); // token range.
+  EXPECT_TRUE(Verifier.match(Code, typeLoc(loc(autoType())), Lang_CXX20));
 }
 
 TEST(TypeLoc, LongDoubleRange) {



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

[PATCH] D117009: [AST] Fix the incorrect auto-keyword loc for constrained auto type loc.

[Haojian Wu 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 rG6c78703e3abc: [AST] Fix the incorrect auto-keyword loc for 
constrained auto type loc. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117009

Files:
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/unittests/AST/SourceLocationTest.cpp


Index: clang/unittests/AST/SourceLocationTest.cpp
===
--- clang/unittests/AST/SourceLocationTest.cpp
+++ clang/unittests/AST/SourceLocationTest.cpp
@@ -247,6 +247,14 @@
   Verifier.expectRange(1, 1, 1, 14);
   EXPECT_TRUE(Verifier.match("decltype(auto) a = 1;", typeLoc(loc(autoType())),
  Lang_CXX14));
+
+  const char *Code =
+  R"cpp(template  concept C = true;
+C auto abc();
+)cpp";
+  // Should include "C auto" tokens.
+  Verifier.expectRange(2, 1, 2, 3); // token range.
+  EXPECT_TRUE(Verifier.match(Code, typeLoc(loc(autoType())), Lang_CXX20));
 }
 
 TEST(TypeLoc, LongDoubleRange) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3582,7 +3582,7 @@
   isInvalid = DS.SetTypeSpecType(TST_decltype_auto, Loc, PrevSpec,
  DiagID, TemplateId, Policy);
 } else {
-  isInvalid = DS.SetTypeSpecType(TST_auto, Loc, PrevSpec, DiagID,
+  isInvalid = DS.SetTypeSpecType(TST_auto, AutoLoc, PrevSpec, DiagID,
  TemplateId, Policy);
 }
 break;
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
@@ -285,31 +285,6 @@
 return {};
   }
 
-  // If the return type is a constrained 'auto', we need to include the token
-  // after the concept. Unfortunately, the source range of an AutoTypeLoc, if 
it
-  // is constrained, does not include the 'auto'.
-  // FIXME: fix the AutoTypeLoc location in clang.
-  auto ATL = ReturnLoc.getAs();
-  if (ATL && ATL.isConstrained() && !ATL.isDecltypeAuto()) {
-SourceLocation End =
-expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
-SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
-
-// Extend the ReturnTypeRange until the last token before the function
-// name.
-std::pair Loc = SM.getDecomposedLoc(End);
-StringRef File = SM.getBufferData(Loc.first);
-const char *TokenBegin = File.data() + Loc.second;
-Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(),
-TokenBegin, File.end());
-Token T;
-SourceLocation LastTLoc = End;
-while (!Lexer.LexFromRawLexer(T) &&
-   SM.isBeforeInTranslationUnit(T.getLocation(), BeginNameF)) {
-  LastTLoc = T.getLocation();
-}
-ReturnTypeRange.setEnd(LastTLoc);
-  }
 
   // If the return type has no local qualifiers, it's source range is accurate.
   if (!hasAnyNestedLocalQualifiers(F.getReturnType()))


Index: clang/unittests/AST/SourceLocationTest.cpp
===
--- clang/unittests/AST/SourceLocationTest.cpp
+++ clang/unittests/AST/SourceLocationTest.cpp
@@ -247,6 +247,14 @@
   Verifier.expectRange(1, 1, 1, 14);
   EXPECT_TRUE(Verifier.match("decltype(auto) a = 1;", typeLoc(loc(autoType())),
  Lang_CXX14));
+
+  const char *Code =
+  R"cpp(template  concept C = true;
+C auto abc();
+)cpp";
+  // Should include "C auto" tokens.
+  Verifier.expectRange(2, 1, 2, 3); // token range.
+  EXPECT_TRUE(Verifier.match(Code, typeLoc(loc(autoType())), Lang_CXX20));
 }
 
 TEST(TypeLoc, LongDoubleRange) {
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3582,7 +3582,7 @@
   isInvalid = DS.SetTypeSpecType(TST_decltype_auto, Loc, PrevSpec,
  DiagID, TemplateId, Policy);
 } else {
-  isInvalid = DS.SetTypeSpecType(TST_auto, Loc, PrevSpec, DiagID,
+  isInvalid = DS.SetTypeSpecType(TST_auto, AutoLoc, PrevSpec, DiagID,
  TemplateId, Policy);
 }
 break;
Index: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
+++ clang-tools-ex

[clang] 5ea9898 - [clang-format] [docs] Fix link to avoid redirection. NFC.

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-01-19T14:23:04+01:00
New Revision: 5ea98988c65bc50f23cedaf11ead9470f1141235

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

LOG: [clang-format] [docs] Fix link to avoid redirection. NFC.

Added: 


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

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 8e2a7924063cd..76318e91cb203 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -2870,7 +2870,7 @@ the configuration (without a prefix: ``Auto``).
   readability to have the signature indented two levels and to use
   ``OuterScope``. The KJ style guide requires ``OuterScope``.
   `KJ style guide
-  `_
+  `_
 
   Possible values:
 

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 0b5f74503d807..326e85305c8e7 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2712,7 +2712,7 @@ struct FormatStyle {
   /// readability to have the signature indented two levels and to use
   /// ``OuterScope``. The KJ style guide requires ``OuterScope``.
   /// `KJ style guide
-  /// 
`_
+  /// `_
   /// \version 13
   LambdaBodyIndentationKind LambdaBodyIndentation;
 



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

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Correct me if I'm wrong, but I don't think this stubs are async signal safe nor 
will they work for preemptive multitasking systems?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116088

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

[PATCH] D117529: [clangd][NFC] Cache ClangTidy check globs to speed up createChecks

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 401197.
njames93 added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117529

Files:
  clang-tools-extra/clang-tidy/ClangTidyModule.h
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -282,6 +283,42 @@
   }
 }
 
+std::vector>
+buildChecks(tidy::ClangTidyContext *Context) {
+  using FactoryFunc = std::function(
+  llvm::StringRef, tidy::ClangTidyContext *)>;
+  const llvm::Optional &CheckGlob = Context->getOptions().Checks;
+  if (!CheckGlob || CheckGlob->empty())
+return {};
+  thread_local std::pair>>
+  Cached;
+  static std::atomic CacheHit, CacheMiss;
+  if (*CheckGlob == Cached.first) {
+++CacheHit;
+  } else {
+++CacheMiss;
+vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n"
+ "Rebuilding ClangTidyChecks factory with new glob '{1}'",
+ static_cast(CacheHit) / CacheMiss, *CheckGlob);
+Cached.first.assign(CheckGlob->data(), CheckGlob->size());
+Cached.second.clear();
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+tidy::GlobList Glob(*CheckGlob);
+for (const auto &Check : CTFactories) {
+  if (Glob.contains(Check.getKey()))
+Cached.second.emplace_back(Check.getKey(), Check.getValue());
+}
+  }
+  std::vector> Result;
+  Result.reserve(Cached.second.size());
+  for (auto &Factory : Cached.second) {
+Result.push_back(Factory.second(Factory.first, Context));
+  }
+  return Result;
+}
 } // namespace
 
 llvm::Optional
@@ -410,15 +447,12 @@
   // diagnostics.
   if (PreserveDiags) {
 trace::Span Tracer("ClangTidyInit");
-tidy::ClangTidyCheckFactories CTFactories;
-for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
-  E.instantiate()->addCheckFactories(CTFactories);
 CTContext.emplace(std::make_unique(
 tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
 CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
 CTContext->setASTContext(&Clang->getASTContext());
 CTContext->setCurrentFile(Filename);
-CTChecks = CTFactories.createChecks(CTContext.getPointer());
+CTChecks = buildChecks(CTContext.getPointer());
 llvm::erase_if(CTChecks, [&](const auto &Check) {
   return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
 });
Index: clang-tools-extra/clang-tidy/ClangTidyModule.h
===
--- clang-tools-extra/clang-tidy/ClangTidyModule.h
+++ clang-tools-extra/clang-tidy/ClangTidyModule.h
@@ -71,6 +71,7 @@
   FactoryMap::const_iterator begin() const { return Factories.begin(); }
   FactoryMap::const_iterator end() const { return Factories.end(); }
   bool empty() const { return Factories.empty(); }
+  size_t size() const { return Factories.size(); }
 
 private:
   FactoryMap Factories;


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -282,6 +283,42 @@
   }
 }
 
+std::vector>
+buildChecks(tidy::ClangTidyContext *Context) {
+  using FactoryFunc = std::function(
+  llvm::StringRef, tidy::ClangTidyContext *)>;
+  const llvm::Optional &CheckGlob = Context->getOptions().Checks;
+  if (!CheckGlob || CheckGlob->empty())
+return {};
+  thread_local std::pair>>
+  Cached;
+  static std::atomic CacheHit, CacheMiss;
+  if (*CheckGlob == Cached.first) {
+++CacheHit;
+  } else {
+++CacheMiss;
+vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n"
+ "Rebuilding ClangTidyChecks factory with new glob '{1}'",
+ static_cast(CacheHit) / CacheMiss, *CheckGlob);
+Cached.first.assign(CheckGlob->data(), CheckGlob->size());
+Cached.second.clear();
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+tidy::GlobList Glob(*CheckGlob);
+for (const auto &Check : CTFactories) {
+  if (Glob.contains(Check.ge

[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+  R"cpp(template  concept C = true;
+^C a^uto abc();
+)cpp");

sammccall wrote:
> This shouldn't be unavailable because it's constrained, it should be 
> unavailable because it's not deduced.
> 
> Does this case pass or fail today?
> 
> If the issue with constrained auto is syntactic, can you add a deductible 
> test case?
> it should be unavailable because it's not deduced.

This is ideal, but not the behavior today -- the tweak is available but being 
failed to apply as we don't check the auto is deduced during prepare stage (it 
is expensive, requiring an AST traversal). 

For deducible cases, we replace the `C auto` with the actual type, e.g.
```
template  concept C = true;
C auto var = 123;  // => int var = 123;
```
I don't think this is an expected behavior. Given the deductible & 
nondeductible cases, it seems like an improvement to disable the tweak for 
constrained auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117463

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

[PATCH] D117667: [clang][dataflow] Add a transfer function for conditional operator

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, xazax.hun, gribozavr2.
Herald added subscribers: steakhal, rnkovacs.
sgatev requested review of this revision.
Herald added a project: clang.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117667

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1788,4 +1788,44 @@
   });
 }
 
+TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
+  std::string Code = R"(
+struct A {};
+
+void target(A Foo, A Bar, bool Cond) {
+  A Baz = Cond ?  Foo : Bar;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+
+const auto *BazVal =
+dyn_cast(Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_NE(BazVal, FooVal);
+EXPECT_NE(BazVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -404,6 +404,16 @@
 }
   }
 
+  void VisitConditionalOperator(const ConditionalOperator *S) {
+// FIXME: Revisit this once flow conditions are added to the framework. For
+// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
+// condition.
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
   // FIXME: Add support for:
   // - CXXBoolLiteralExpr
 


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1788,4 +1788,44 @@
   });
 }
 
+TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
+  std::string Code = R"(
+struct A {};
+
+void target(A Foo, A Bar, bool Cond) {
+  A Baz = Cond ?  Foo : Bar;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+
+const auto *BazVal =
+dyn_cast(Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_NE(BazVal, FooVal);
+EXPECT_NE(BazVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -404,6 +404,16 @@
 }
   }
 
+  void VisitConditionalOperator(const ConditionalOperator *S) {
+// FIXME: Revisit this once flow conditions are added to the framework. For
+// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
+// condition.
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
   // FIX

[clang] 881b6a0 - [analyzer][NFC] Re-enable skipped SValTests by relaxing expectations

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-01-19T15:16:18+01:00
New Revision: 881b6a009fb6e2dd5fb924524cd6eacd14148a08

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

LOG: [analyzer][NFC] Re-enable skipped SValTests by relaxing expectations

Some tests were skipped in D114454 to resolve test failures on some
platforms, where the pointers have different bitwidth than expected.
This patch re-enables these tests, by relaxing the requirements on the
types of the SVal.

The issue:
There is no way to reconstruct the type of the `SVal` perfectly
accurately, since there could be multiple types having the required
bitwidth and signedness.
Consider platforms where `int` and `long` have the same bitwidth.
Additionally, we need to be careful about casting a pointer to an
integral representation, because we don't know what smallest integral
type can represent that.

To workaround these issues, I propose enforcing a type that has the
same signedness and bitwidth as the expected type, instead of perfect
equality.

In the `GetLocAsIntType` test, in case of pointer-to-integral casts
I'm using the widest standard integral type (long long) to make sure
that the pointer can be represented by the type without losing
precision. This won't affect the test in any meaningful way, since the
type of the `lvalue` remained the same.

In one case, I had to replace `getUIntPtrType()` with `UnsignedLongTy`
because on some platforms `getUIntPtrType()` is different then `long
int`.

In this patch, I also enforce that the tests must compile without
errors, to prevent narrowing conversions in the future.

Reviewed By: stevewan

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

Added: 


Modified: 
clang/unittests/StaticAnalyzer/SValTest.cpp

Removed: 




diff  --git a/clang/unittests/StaticAnalyzer/SValTest.cpp 
b/clang/unittests/StaticAnalyzer/SValTest.cpp
index 08853a7bb83f2..1a41faf098996 100644
--- a/clang/unittests/StaticAnalyzer/SValTest.cpp
+++ b/clang/unittests/StaticAnalyzer/SValTest.cpp
@@ -92,20 +92,15 @@ class SValCollector : public Checker {
   mutable SVals CollectedSVals;
 };
 
+static void expectSameSignAndBitWidth(QualType ExpectedTy, QualType ActualTy,
+  const ASTContext &Context) {
+  EXPECT_EQ(ExpectedTy->isUnsignedIntegerType(),
+ActualTy->isUnsignedIntegerType());
+  EXPECT_EQ(Context.getTypeSize(ExpectedTy), Context.getTypeSize(ActualTy));
+}
+
 // Fixture class for parameterized SValTest
-class SValTest : public testing::TestWithParam {
-protected:
-  // FIXME: The tests "GetConstType" and "GetLocAsIntType" infer the type of
-  // integrals based on their bitwidth. This is not a reliable method on
-  // platforms where 
diff erent integrals have the same width.
-  bool skipTest(StringRef TestName) {
-std::string target = GetParam().Target;
-return (target == "powerpc-ibm-aix" || target == "i686-apple-darwin9" ||
-target == "wasm32-unknown-unknown" ||
-target == "wasm64-unknown-unknown") &&
-   (TestName == "GetConstType" || TestName == "GetLocAsIntType");
-  }
-};
+class SValTest : public testing::TestWithParam {};
 
 // SVAL_TEST is a combined way of providing a short code snippet and
 // to test some programmatic predicates on symbolic values produced by the
@@ -152,14 +147,8 @@ class SValTest : public 
testing::TestWithParam {
   }
\

\
   TEST_P(SValTest, NAME) { 
\
-if (skipTest(#NAME)) { 
\
-  std::string target = GetParam().Target;  
\
-  GTEST_SKIP() << "certain integrals have the same bitwidth on "   
\
-   << target;  
\
-  return;  
\
-}  
\
-runCheckerOnCodeWithArgs(
\
-CODE, GetParam().getCommandLineArgs());
\
+EXPECT_TRUE(runCheckerOnCodeWithArgs(
\
+CODE, GetParam().getCommandLineArgs()));   
\
   }
\
   void NAME##SValCollector::test(ExprEngine &Engine,   
\
  const ASTContext &Context) const
@@ -179,27 +168,30 @@ void foo() {
 
   SVal Y = getByName("y");
   ASSERT_FALSE(Y.getType(Context).isNull());
-  EXPECT_EQ(Context.getUInt

[PATCH] D115349: [analyzer][NFC] Re-enable skipped SValTests by relaxing expectations

[Balázs Benics 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 rG881b6a009fb6: [analyzer][NFC] Re-enable skipped SValTests by 
relaxing expectations (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115349

Files:
  clang/unittests/StaticAnalyzer/SValTest.cpp


Index: clang/unittests/StaticAnalyzer/SValTest.cpp
===
--- clang/unittests/StaticAnalyzer/SValTest.cpp
+++ clang/unittests/StaticAnalyzer/SValTest.cpp
@@ -92,20 +92,15 @@
   mutable SVals CollectedSVals;
 };
 
+static void expectSameSignAndBitWidth(QualType ExpectedTy, QualType ActualTy,
+  const ASTContext &Context) {
+  EXPECT_EQ(ExpectedTy->isUnsignedIntegerType(),
+ActualTy->isUnsignedIntegerType());
+  EXPECT_EQ(Context.getTypeSize(ExpectedTy), Context.getTypeSize(ActualTy));
+}
+
 // Fixture class for parameterized SValTest
-class SValTest : public testing::TestWithParam {
-protected:
-  // FIXME: The tests "GetConstType" and "GetLocAsIntType" infer the type of
-  // integrals based on their bitwidth. This is not a reliable method on
-  // platforms where different integrals have the same width.
-  bool skipTest(StringRef TestName) {
-std::string target = GetParam().Target;
-return (target == "powerpc-ibm-aix" || target == "i686-apple-darwin9" ||
-target == "wasm32-unknown-unknown" ||
-target == "wasm64-unknown-unknown") &&
-   (TestName == "GetConstType" || TestName == "GetLocAsIntType");
-  }
-};
+class SValTest : public testing::TestWithParam {};
 
 // SVAL_TEST is a combined way of providing a short code snippet and
 // to test some programmatic predicates on symbolic values produced by the
@@ -152,14 +147,8 @@
   }
\

\
   TEST_P(SValTest, NAME) { 
\
-if (skipTest(#NAME)) { 
\
-  std::string target = GetParam().Target;  
\
-  GTEST_SKIP() << "certain integrals have the same bitwidth on "   
\
-   << target;  
\
-  return;  
\
-}  
\
-runCheckerOnCodeWithArgs(
\
-CODE, GetParam().getCommandLineArgs());
\
+EXPECT_TRUE(runCheckerOnCodeWithArgs(
\
+CODE, GetParam().getCommandLineArgs()));   
\
   }
\
   void NAME##SValCollector::test(ExprEngine &Engine,   
\
  const ASTContext &Context) const
@@ -179,27 +168,30 @@
 
   SVal Y = getByName("y");
   ASSERT_FALSE(Y.getType(Context).isNull());
-  EXPECT_EQ(Context.getUIntPtrType(), Y.getType(Context));
+  expectSameSignAndBitWidth(Context.getUIntPtrType(), Y.getType(Context),
+Context);
 }
 
 SVAL_TEST(GetLocAsIntType, R"(
 void foo(int *x) {
-  long int a = (long int)x;
-  unsigned b = (long unsigned)&a;
-  int c = (long int)nullptr;
+  long int a = (long long int)x;
+  unsigned b = (long long unsigned)&a;
+  int c = (long long int)nullptr;
 })") {
   SVal A = getByName("a");
   ASSERT_FALSE(A.getType(Context).isNull());
+
   // TODO: Turn it into signed long
-  EXPECT_EQ(Context.getUIntPtrType(), A.getType(Context));
+  expectSameSignAndBitWidth(Context.UnsignedLongTy, A.getType(Context),
+Context);
 
   SVal B = getByName("b");
   ASSERT_FALSE(B.getType(Context).isNull());
-  EXPECT_EQ(Context.UnsignedIntTy, B.getType(Context));
+  expectSameSignAndBitWidth(Context.UnsignedIntTy, B.getType(Context), 
Context);
 
   SVal C = getByName("c");
   ASSERT_FALSE(C.getType(Context).isNull());
-  EXPECT_EQ(Context.IntTy, C.getType(Context));
+  expectSameSignAndBitWidth(Context.IntTy, C.getType(Context), Context);
 }
 
 SVAL_TEST(GetSymExprType, R"(


Index: clang/unittests/StaticAnalyzer/SValTest.cpp
===
--- clang/unittests/StaticAnalyzer/SValTest.cpp
+++ clang/unittests/StaticAnalyzer/SValTest.cpp
@@ -92,20 +92,15 @@
   mutable SVals CollectedSVals;
 };
 
+static void expectSameSignAndBitWidth(QualType ExpectedTy, QualType ActualTy,
+  const ASTContext &Context) {
+  EXPECT_EQ(Expected

[PATCH] D117529: [clangd][NFC] Cache ClangTidy check globs to speed up createChecks

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 401209.
njames93 added a comment.

Remove now unnecessary changes in ClangTidyModule.
Fix Cache miss reporting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117529

Files:
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -282,6 +283,41 @@
   }
 }
 
+std::vector>
+buildChecks(tidy::ClangTidyContext *Context) {
+  using FactoryFunc = std::function(
+  llvm::StringRef, tidy::ClangTidyContext *)>;
+  const llvm::Optional &CheckGlob = Context->getOptions().Checks;
+  if (!CheckGlob || CheckGlob->empty())
+return {};
+  thread_local std::pair>>
+  Cached;
+  static std::atomic CacheMiss, CacheAccess;
+  ++CacheAccess;
+  if (*CheckGlob != Cached.first) {
+++CacheMiss;
+vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n"
+ "Rebuilding ClangTidyChecks factory with new glob '{1}'",
+ static_cast(CacheMiss) / CacheAccess, *CheckGlob);
+Cached.first.assign(CheckGlob->data(), CheckGlob->size());
+Cached.second.clear();
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+tidy::GlobList Glob(*CheckGlob);
+for (const auto &Check : CTFactories) {
+  if (Glob.contains(Check.getKey()))
+Cached.second.emplace_back(Check.getKey(), Check.getValue());
+}
+  }
+  std::vector> Result;
+  Result.reserve(Cached.second.size());
+  for (auto &Factory : Cached.second) {
+Result.push_back(Factory.second(Factory.first, Context));
+  }
+  return Result;
+}
 } // namespace
 
 llvm::Optional
@@ -410,15 +446,12 @@
   // diagnostics.
   if (PreserveDiags) {
 trace::Span Tracer("ClangTidyInit");
-tidy::ClangTidyCheckFactories CTFactories;
-for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
-  E.instantiate()->addCheckFactories(CTFactories);
 CTContext.emplace(std::make_unique(
 tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
 CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
 CTContext->setASTContext(&Clang->getASTContext());
 CTContext->setCurrentFile(Filename);
-CTChecks = CTFactories.createChecks(CTContext.getPointer());
+CTChecks = buildChecks(CTContext.getPointer());
 llvm::erase_if(CTChecks, [&](const auto &Check) {
   return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
 });


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -10,6 +10,7 @@
 #include "../clang-tidy/ClangTidyCheck.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "../clang-tidy/GlobList.h"
 #include "AST.h"
 #include "Compiler.h"
 #include "Config.h"
@@ -282,6 +283,41 @@
   }
 }
 
+std::vector>
+buildChecks(tidy::ClangTidyContext *Context) {
+  using FactoryFunc = std::function(
+  llvm::StringRef, tidy::ClangTidyContext *)>;
+  const llvm::Optional &CheckGlob = Context->getOptions().Checks;
+  if (!CheckGlob || CheckGlob->empty())
+return {};
+  thread_local std::pair>>
+  Cached;
+  static std::atomic CacheMiss, CacheAccess;
+  ++CacheAccess;
+  if (*CheckGlob != Cached.first) {
+++CacheMiss;
+vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n"
+ "Rebuilding ClangTidyChecks factory with new glob '{1}'",
+ static_cast(CacheMiss) / CacheAccess, *CheckGlob);
+Cached.first.assign(CheckGlob->data(), CheckGlob->size());
+Cached.second.clear();
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+tidy::GlobList Glob(*CheckGlob);
+for (const auto &Check : CTFactories) {
+  if (Glob.contains(Check.getKey()))
+Cached.second.emplace_back(Check.getKey(), Check.getValue());
+}
+  }
+  std::vector> Result;
+  Result.reserve(Cached.second.size());
+  for (auto &Factory : Cached.second) {
+Result.push_back(Factory.second(Factory.first, Context));
+  }
+  return Result;
+}
 } // namespace
 
 llvm::Optional
@@ -410,15 +446,12 @@
   // diagnostics.
   if (PreserveDiags) {
 trace::Span Tracer("ClangTidyInit");
-tidy::ClangTidyCheckFactories CTFactories;
-for (const auto &E : tidy::ClangTidyModuleRegi

[PATCH] D117475: [clangd] NFC, emit source ranges in selection debug messages.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 401219.
hokein added a comment.

address commment, only printing range on push.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117475

Files:
  clang-tools-extra/clangd/Selection.cpp


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -778,8 +778,8 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
-  dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
+  dlog("{2}skip: {0}{1}", printNodeToString(N, PrintPolicy),
+   S.printToString(SM), indent());
   return true;
 }
 return false;
@@ -798,7 +798,8 @@
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
 SourceRange Early = earlySourceRange(Node);
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{2}push: {0}{1}", printNodeToString(Node, PrintPolicy),
+ Node.getSourceRange().printToString(SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -778,8 +778,8 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
-  dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
+  dlog("{2}skip: {0}{1}", printNodeToString(N, PrintPolicy),
+   S.printToString(SM), indent());
   return true;
 }
 return false;
@@ -798,7 +798,8 @@
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
 SourceRange Early = earlySourceRange(Node);
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{2}push: {0}{1}", printNodeToString(Node, PrintPolicy),
+ Node.getSourceRange().printToString(SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117475: [clangd] NFC, emit source ranges in selection debug messages.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:704
+  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy, SM), indent());
   dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
   return true;

sammccall wrote:
> We're already printing the range here, which only differs rarely. This seems 
> noisy and confusing.
> (S is a better range to print than N.getSourceRange here - no objection if 
> you want to change the log formatting to print it similarly to the others)
I inlined these two into one dlog, which makes the output more dense.



Comment at: clang-tools-extra/clangd/Selection.cpp:733
 Node &N = *Stack.top();
-dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
+dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy, SM),
+ indent(-1));

sammccall wrote:
> are we sure we want to print the range again on pop? Seems like on enter/skip 
> is sufficient. It's useful to be able to visually distinguish push vs pop.
oh, right, we don't need that. I think printing range on push is sufficient. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117475

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

[PATCH] D117475: [clangd] NFC, emit source ranges in selection debug messages.

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/clangd/Selection.cpp:551
   printNodeKind(OS, N);
   OS << " ";
   return std::move(OS.str());

hmm, while here, this extra space seems weird.

Previously it was at EOL and invisible.

After this patch it's used, but I think it'd be clearer to move it to the 
format string.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117475

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

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Digger Lin via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

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

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

If I'm following correctly, this broke the amdgpu buildbot and it has been 
moved into staging as a workaround. I haven't debugged what breaks but 
'DefaultVisibility' is relatively likely to behave differently on cuda vs hsa 
systems so that's my first guess.




Comment at: clang/lib/AST/Decl.cpp:792
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
+  return LinkageInfo::external();
+

Would this change static variables to non-static and thus introduce multiple 
definition errors? Not immediately obvious to me that the variables have to be 
directly visible to other translation units


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Hi, this patch has caused a gcc test suite failure on our SVE bots: 
https://lab.llvm.org/buildbot/#/builders/184/builds/1941
(test source is 
https://github.com/llvm/llvm-test-suite/blob/main/SingleSource/Regression/C/gcc-c-torture/execute/pr79286.c)

I've only just been made aware of the failure so we will spend some time to 
find out the reasoning before asking that anything be changed. Could be 
something specific to SVE in which case we'll handle it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D117475: [clangd] NFC, emit source ranges in selection debug messages.

[Haojian Wu 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 rG55b702c37b7e: [clangd] NFC, emit source ranges in selection 
debug messages. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D117475?vs=401219&id=401226#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117475

Files:
  clang-tools-extra/clangd/Selection.cpp


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -548,7 +548,6 @@
   std::string S;
   llvm::raw_string_ostream OS(S);
   printNodeKind(OS, N);
-  OS << " ";
   return std::move(OS.str());
 }
 #endif
@@ -778,8 +777,8 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
-  dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
+  dlog("{2}skip: {0} {1}", printNodeToString(N, PrintPolicy),
+   S.printToString(SM), indent());
   return true;
 }
 return false;
@@ -798,7 +797,8 @@
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
 SourceRange Early = earlySourceRange(Node);
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{2}push: {0} {1}", printNodeToString(Node, PrintPolicy),
+ Node.getSourceRange().printToString(SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -548,7 +548,6 @@
   std::string S;
   llvm::raw_string_ostream OS(S);
   printNodeKind(OS, N);
-  OS << " ";
   return std::move(OS.str());
 }
 #endif
@@ -778,8 +777,8 @@
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
-  dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
+  dlog("{2}skip: {0} {1}", printNodeToString(N, PrintPolicy),
+   S.printToString(SM), indent());
   return true;
 }
 return false;
@@ -798,7 +797,8 @@
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
 SourceRange Early = earlySourceRange(Node);
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{2}push: {0} {1}", printNodeToString(Node, PrintPolicy),
+ Node.getSourceRange().printToString(SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 55b702c - [clangd] NFC, emit source ranges in selection debug messages.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-01-19T16:06:58+01:00
New Revision: 55b702c37b7e8b5fde04b6a60b5bb12806bdb697

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

LOG: [clangd] NFC, emit source ranges in selection debug messages.

It will make the output more versbose, but I found that these are useful
information when debugging selection tree.

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index cc698631be03..6dfaa8be9591 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -548,7 +548,6 @@ std::string printNodeToString(const DynTypedNode &N, const 
PrintingPolicy &PP) {
   std::string S;
   llvm::raw_string_ostream OS(S);
   printNodeKind(OS, N);
-  OS << " ";
   return std::move(OS.str());
 }
 #endif
@@ -778,8 +777,8 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
  [](const Attr *A) { return !A->isImplicit(); }))
   return false;
 if (!SelChecker.mayHit(S)) {
-  dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
-  dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
+  dlog("{2}skip: {0} {1}", printNodeToString(N, PrintPolicy),
+   S.printToString(SM), indent());
   return true;
 }
 return false;
@@ -798,7 +797,8 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   // Performs early hit detection for some nodes (on the earlySourceRange).
   void push(DynTypedNode Node) {
 SourceRange Early = earlySourceRange(Node);
-dlog("{1}push: {0}", printNodeToString(Node, PrintPolicy), indent());
+dlog("{2}push: {0} {1}", printNodeToString(Node, PrintPolicy),
+ Node.getSourceRange().printToString(SM), indent());
 Nodes.emplace_back();
 Nodes.back().ASTNode = std::move(Node);
 Nodes.back().Parent = Stack.top();



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

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a subscriber: ronlieb.
jhuber6 added a comment.

In D117362#3254681 , @JonChesterfield 
wrote:

> If I'm following correctly, this broke the amdgpu buildbot and it has been 
> moved into staging as a workaround. I haven't debugged what breaks but 
> 'DefaultVisibility' is relatively likely to behave differently on cuda vs hsa 
> systems so that's my first guess.

Yes, this patch is necessary because of the different handling of `hidden` 
visibility by NVIDIA and AMDGPU. Without this patch we cannot run any code that 
uses `#pragma omp declare target` on AMDGPU because they are all hidden. The 
runtime will try to load it and fail so we will crash.




Comment at: clang/lib/AST/Decl.cpp:792
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
+  return LinkageInfo::external();
+

JonChesterfield wrote:
> Would this change static variables to non-static and thus introduce multiple 
> definition errors? Not immediately obvious to me that the variables have to 
> be directly visible to other translation units
That was my first guess, I had @ronlieb run an alternate approach where we run 
everything as normal, but at the end we inherit the linkage but set the 
visibility to default. he said it still caused problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

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

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/AST/Decl.cpp:1080
+  if (const VarDecl *VD = dyn_cast(D))
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))
+  return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);

Possibly address space dependent. I'm not confident shared or thread local 
variables will behave sensibly if annotated with elf visibility flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

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

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 401230.
martong marked 3 inline comments as done.
martong added a comment.

- Describe the different kind of constraints and limitations
- Some rewording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117568

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -552,7 +552,7 @@
"or is EOF.">,
   Dependencies<[StdCLibraryFunctionsChecker]>,
   WeakDependencies<[CallAndMessageChecker, NonNullParamChecker, StreamChecker]>,
-  Documentation;
+  Documentation;
 
 } // end "alpha.unix"
 
Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -2333,6 +2333,88 @@
 alpha.unix
 ^^^
 
+.. _alpha-unix-StdCLibraryFunctionArgs:
+
+alpha.unix.StdCLibraryFunctionArgs (C)
+""
+Check for calls of standard library functions that violate predefined argument
+constraints. For example, it is stated in the C standard that for the ``int
+isalnum(int ch)`` function the behavior is undefined if the value of ``ch`` is
+not representable as unsigned char and is not equal to ``EOF``.
+
+.. code-block:: c
+
+  void test_alnum_concrete(int v) {
+int ret = isalnum(256); // \
+// warning: Function argument constraint is not satisfied
+(void)ret;
+  }
+
+If the argument's value is unknown then the value is assumed to hold the proper value range.
+
+.. code-block:: c
+
+  #define EOF -1
+  int test_alnum_symbolic(int x) {
+int ret = isalnum(x);
+// after the call, ret is assumed to be in the range [-1, 255]
+
+if (ret > 255)  // impossible (infeasible branch)
+  if (x == 0)
+return ret / x; // division by zero is not reported
+return ret;
+  }
+
+If the user disables the checker then the argument violation warning is
+suppressed. However, the assumption about the argument is still modeled. This
+is because exploring an execution path that already contains undefined behavior
+is not valuable.
+
+There are different kind of constraints modeled: range constraint, not null
+constraint, buffer size constraint. A **range constraint** requires the
+argument's value to be in a specific range, see ``isalnum`` as an example above.
+A **not null constraint** requires the pointer argument to be non-null.
+
+A **buffer size** constraint specifies the minimum size of the buffer
+argument. The size might be a known constant. For example, ``asctime_r`` requires
+that the buffer argument's size must be greater than or equal to ``26`` bytes. In
+other cases, the size is denoted by another argument or as a multiplication of
+two arguments.
+For instance, ``size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream)``.
+Here, ``ptr`` is the buffer, and its minimum size is ``size * nmemb``
+
+.. code-block:: c
+
+  void buffer_size_constraint_violation(FILE *file) {
+enum { BUFFER_SIZE = 1024 };
+wchar_t wbuf[BUFFER_SIZE];
+
+const size_t size = sizeof(*wbuf);   // 4
+const size_t nitems = sizeof(wbuf);  // 4096
+
+// Below we receive a warning because the 3rd parameter should be the
+// number of elements to read, not the size in bytes. This case is a known
+// vulnerability described by the the ARR38-C SEI-CERT rule.
+fread(wbuf, size, nitems, file);
+  }
+
+**Limitations**
+
+The checker is in alpha because the reports cannot provide notes about the
+values of the arguments. Without this information it is hard to confirm if the
+constraint is indeed violated. For example, consider the above case for
+``fread``. We display in the warning message that the size of the 1st arg
+should be equal to or less than the value of the 2nd arg times the 3rd arg.
+However, we fail to display the concrete values (``4`` and ``4096``) for those
+arguments.
+
+**Parameters**
+
+The checker models functions (and emits diagnostics) from the C standard by
+default. The ``ModelPOSIX`` option enables the checker to model (and emit
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+
 .. _alpha-unix-BlockInCriticalSection:
 
 alpha.unix.BlockInCriticalSection (C)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:2361
+(void)ret;
+clang_analyzer_eval(EOF <= x && x <= 255); // this reports TRUE
+  }

NoQ wrote:
> I recommend against using `clang_analyzer_eval` in user docs. Users aren't 
> expected to know what it is.
Ok, I've changed to have an infeasible branch condition and below that an 
unreported div zero warning demonstrates the same.



Comment at: clang/docs/analyzer/checkers.rst:2366
+suppressed. However, the assumption about the argument is still modeled 
(otherwise we
+would be further analyzing an illformed program).
+

NoQ wrote:
> Nitpick: the program doesn't become ill-formed just because the user has 
> turned off the checker. Maybe it's better to say that exploring an execution 
> path that already contains undefined behavior is not valuable, or something 
> along those lines(?)
Ok, I've changed the wording.



Comment at: clang/docs/analyzer/checkers.rst:2371
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+

dkrupp wrote:
> I think it would be useful for the user to see one example per constraint 
> type that this checker supports.
> RangeConstraint (was covered), ComparisonConstraint, ValueConstraint, Not 
> null  Constraint, BufferSize constraint etc.
> 
> 
> It would be also nice to add a section "Limitations".
> 
> Describe there well known false positive cases or limitations in the bug 
> diagnostics that limits understandability.
> Essentially the most important well known cases why this checker is alpha.
> 
> This section would be useful for users to understand and help identifying 
> cases that are known false positives and for the developers to know how to 
> improve this checker. I remember many cases when we had to test multiple 
> times "why a checker is in alpha", because we forgot about it. I think it is 
> best to document it.
Ok, I've added a few paragraphs to describe these things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117568

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

[PATCH] D117463: [clangd] Disable expand-auto action on constrained auto.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+  R"cpp(template  concept C = true;
+^C a^uto abc();
+)cpp");

hokein wrote:
> sammccall wrote:
> > This shouldn't be unavailable because it's constrained, it should be 
> > unavailable because it's not deduced.
> > 
> > Does this case pass or fail today?
> > 
> > If the issue with constrained auto is syntactic, can you add a deductible 
> > test case?
> > it should be unavailable because it's not deduced.
> 
> This is ideal, but not the behavior today -- the tweak is available but being 
> failed to apply as we don't check the auto is deduced during prepare stage 
> (it is expensive, requiring an AST traversal). 
> 
> For deducible cases, we replace the `C auto` with the actual type, e.g.
> ```
> template  concept C = true;
> C auto var = 123;  // => int var = 123;
> ```
> I don't think this is an expected behavior. Given the deductible & 
> nondeductible cases, it seems like an improvement to disable the tweak for 
> constrained auto.
> > it should be unavailable because it's not deduced.
> 
> This is ideal, but not the behavior today -- the tweak is available but being 
> failed to apply as we don't check the auto is deduced during prepare stage 
> (it is expensive, requiring an AST traversal). 

Sure, but this doesn't have anything to do with it being constrained.
We offer the tweak on `auto x();` too, and equally shouldn't.

Meanwhile we offer the tweak on `auto x() { return 42; }` and it works 
correctly, and we should also offer it on `Integer auto x() { return 42; }`.


> For deducible cases, we replace the `C auto` with the actual type, e.g.
> ```
> template  concept C = true;
> C auto var = 123;  // => int var = 123;
> ```
> I don't think this is an expected behavior. 

That's exactly what I'd expect this action to do. What would you expect it to 
do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117463

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

[PATCH] D117348: [Preprocessor] Reduce the memory overhead of `#define` directives

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/MacroInfo.cpp:33
+
+// MacroInfo is expected to take 40 bytes on platforms with an 8 byte pointer.
+template  class MacroInfoSizeChecker {

dexonsmith wrote:
> aaron.ballman wrote:
> > Should we do this dance for 32-bit systems as well?
> Do I remember correctly that `SourceLocation`'s size recently became 
> configurable? Or maybe it will be soon? Should that be factored in somehow?
Are you thinking about this review https://reviews.llvm.org/D97204 or something 
else?


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

https://reviews.llvm.org/D117348

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

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

In D105169#3254692 , @DavidSpickett 
wrote:

> Hi, this patch has caused a gcc test suite failure on our SVE bots: 
> https://lab.llvm.org/buildbot/#/builders/184/builds/1941
> (test source is 
> https://github.com/llvm/llvm-test-suite/blob/main/SingleSource/Regression/C/gcc-c-torture/execute/pr79286.c)
>
> I've only just been made aware of the failure so we will spend some time to 
> find out the reason before asking that anything be changed. Could be 
> something specific to SVE in which case we'll handle it.

Thanks!
That test has UB in the printf class as it reads a non-initialized variable. 
Can you check if changing line 6 to 'int e = 0;' solves the problem?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

[Nathan James via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h:25-26
+public:
+  InefficientArrayTraversalCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;

LegalizeAdulthood wrote:
> If you're not doing anything different, I believe you can just do
> `using ClangTidyCheck::ClangTidyCheck;`
> 
> Also, shouldn't you be explicitly overriding the d'tor?
> `~InefficientArrayTraversalCheck() override = default;`
No need to explicitly override the d'tor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116875

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

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 401234.
njames93 marked an inline comment as done.
njames93 added a comment.

Use inheriting constructor for check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116875

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s performance-inefficient-array-traversal %t
+
+constexpr unsigned Rows = 10U;
+constexpr unsigned Cols = 16U;
+
+int Arr[Rows][Cols];
+int *PtrArr[Cols];
+int **Ptr;
+int FlatArray[Rows * Cols];
+
+void foo();
+
+void warned() {
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Ensure it works on array access that use pointers
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  PtrArr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Ptr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Detect += 1 as the loop incriment
+  for (unsigned I = 0; I < Cols; I += 1)
+for (unsigned J = 0; J < Rows; J += 1)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Still warn on this as calls inside the inner loop shouldn't complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  foo();
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  foo();
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  FlatArray[J * Cols + I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+}
+
+void ignored() {
+  // Correct traversal.
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  Arr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  PtrArr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  FlatArray[I * Cols + J]++;
+}
+  }
+  // Don'w warn on these cases as extra code inside the outer loop could complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+foo();
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+foo();
+  }
+
+  // Nested loop increments incorrect variable, don't warn on the traversal.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  Arr[J][I]++;
+
+  // These 2 loops are questionable and definitely a memory safety bug,
+  // but this is not the point of this check.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  FlatArray[I * Cols + J]++;
+  for (unsigned I = 0; I < Rows; ++I)
+for (unsigned J = 0; J < Cols; ++I)
+  FlatArray[J * Rows + I]++;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
@@ 

[PATCH] D117681: [RISCV] Add the policy operand for some masked RVV ternary IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen created this revision.
khchen added reviewers: craig.topper, rogfer01, frasercrmck, kito-cheng, 
arcbbb, monkchiang.
Herald added subscribers: VincentWu, luke957, achieveartificialintelligence, 
vkmr, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, 
jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya.
khchen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, eopXD, MaskRay.
Herald added projects: clang, LLVM.

Masked reduction intrinsics are specical cases which don't need to have policy
operand. The mask only affects which elements are read. It doesn't effect the
destination register.
The reduction intrinsics have a dedicated destination operand. If it
is undef, we use tail agnostic. If it not undef we use tail
undisturbed.

Co-Authored-by: Craig Topper 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117681

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vslidedown.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vslideup.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vslidedown.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vslideup.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vwmacc.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/vfmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfmadd-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfmadd-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsub-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfmsub-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmadd-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmadd-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsub-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfnmsub-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfwmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vfwnmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmacc-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmacc-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmadd-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmadd-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsac-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsac-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsub-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vnmsub-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vslidedown-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vslidedown-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vslideup-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vslideup-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:182
+  MD->getMacroInfo()->isUsedForHeaderGuard() ||
+  MD->getMacroInfo()->isBuiltinMacro() || ConditionScope > 0)
+return;

LegalizeAdulthood wrote:
> LegalizeAdulthood wrote:
> > njames93 wrote:
> > > This `ConditionScope` checks looks like it would prevent warning in 
> > > header files that use a header guard(instead of pragma once) to prevent 
> > > multiple inclusion.
> > Oh, good catch, you're probably right.  I'll test that manually.
> > 
> > (Gee, another case where we need `check_clang_tidy.py` to validate
> > changes to header files!  This keeps coming up!  My implementation
> > of this from several years ago died in review hell and was never born.)
> Any ideas for an algorithm that detects a header guard condition
> from some other condition?  I don't see anything obvious.
So I created a little state machine and that covers my test cases,
but I worry that it might be too fragile, so I'm open to suggestions
for improvement on my state machine algorithm and/or test cases
that could break the algorithm.


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

https://reviews.llvm.org/D117522

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

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 401238.
LegalizeAdulthood added a comment.

- Handle macros inside header files with include guards
- Track state per-file and push/pop that state as files are included


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT:  

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

We should update the description to reflect the cause more accurately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

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

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 401241.
njames93 added a comment.

Uploaded wrong diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116875

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s performance-inefficient-array-traversal %t
+
+constexpr unsigned Rows = 10U;
+constexpr unsigned Cols = 16U;
+
+int Arr[Rows][Cols];
+int *PtrArr[Cols];
+int **Ptr;
+int FlatArray[Rows * Cols];
+
+void foo();
+
+void warned() {
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Ensure it works on array access that use pointers
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  PtrArr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Ptr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Detect += 1 as the loop incriment
+  for (unsigned I = 0; I < Cols; I += 1)
+for (unsigned J = 0; J < Rows; J += 1)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Still warn on this as calls inside the inner loop shouldn't complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  foo();
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  foo();
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  FlatArray[J * Cols + I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+}
+
+void ignored() {
+  // Correct traversal.
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  Arr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  PtrArr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  FlatArray[I * Cols + J]++;
+}
+  }
+  // Don'w warn on these cases as extra code inside the outer loop could complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+foo();
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+foo();
+  }
+
+  // Nested loop increments incorrect variable, don't warn on the traversal.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  Arr[J][I]++;
+
+  // These 2 loops are questionable and definitely a memory safety bug,
+  // but this is not the point of this check.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  FlatArray[I * Cols + J]++;
+  for (unsigned I = 0; I < Rows; ++I)
+for (unsigned J = 0; J < Cols; ++I)
+  FlatArray[J * Rows + I]++;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - performance-inefficie

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

[Richard via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:11
+`ES.31 
`_,
 and
+`ES.32 
`_.
+

carlosgalvezp wrote:
> Is ES.32 really checked by this check? I don't see any example or test that 
> indicates that.
> 
> I'm also unsure if ES.32 should be handled here or via the 
> "readability-identifier-naming" check, which is where you enforce a 
> particular naming convention for different identifiers. Setting ALL_CAPS for 
> macros there would be an effective way of solving ES.32.
It was always handled through an option on this check.
(Look at lines 49-56 of `MacroUsageCheck.cpp`)

It's a little bit odd, because it either checks for the names
or it checks for the constant/function like macros, it never
does both at the same time.

This is the way the check was originally written, I haven't
changed any of that.


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

https://reviews.llvm.org/D116386

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

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/InterfaceStubs/object.c:2
 // RUN: %clang_cc1 -fvisibility default -o - -emit-interface-stubs %s | 
FileCheck -check-prefix=CHECK-TAPI %s
-// RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck 
-check-prefix=CHECK-SYMBOLS %s
+// RUN: %clang -fvisibility=default -c -o %t.o %s | llvm-nm %t.o 2>&1 | 
FileCheck -check-prefix=CHECK-SYMBOLS %s
 

With the temporary file added, the pipe is unnecessary (and possibly unwanted). 
Does the suggestion here work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

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

[PATCH] D116386: [clang-tidy] Narrow cppguidelines-macro-usage to actual constants

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 401242.
LegalizeAdulthood marked an inline comment as done.
LegalizeAdulthood added a comment.

- Update from comments


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

https://reviews.llvm.org/D116386

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t -- -header-filter=.* -system-headers --
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage -std=c++17-or-later %t -- -header-filter=.* -system-headers --
 
 #ifndef INCLUDE_GUARD
 #define INCLUDE_GUARD
@@ -6,6 +6,21 @@
 #define PROBLEMATIC_CONSTANT 0
 // CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT' used to declare a constant; consider using a 'constexpr' constant
 
+#define PROBLEMATIC_CONSTANT_CHAR '0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_WIDE_CHAR L'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_WIDE_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF8_CHAR u8'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF8_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF16_CHAR u'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF16_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_CONSTANT_UTF32_CHAR U'0'
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro 'PROBLEMATIC_CONSTANT_UTF32_CHAR' used to declare a constant; consider using a 'constexpr' constant
+
 #define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
 // CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro 'PROBLEMATIC_FUNCTION' used; consider a 'constexpr' template function
 
@@ -15,4 +30,17 @@
 #define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
 // CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro 'PROBLEMATIC_VARIADIC2' used; consider using a 'constexpr' variadic template function
 
+// These are all examples of common macros that shouldn't have constexpr suggestions.
+#define COMMA ,
+
+#define NORETURN [[noreturn]]
+
+#define DEPRECATED attribute((deprecated))
+
+#if LIB_EXPORTS
+#define DLLEXPORTS __declspec(dllexport)
+#else
+#define DLLEXPORTS __declspec(dllimport)
+#endif
+
 #endif
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
@@ -7,10 +7,40 @@
 constructs exist for the task.
 
 The relevant sections in the C++ Core Guidelines are
-`Enum.1 `_,
-`ES.30 `_,
-`ES.31 `_ and
-`ES.33 `_.
+`ES.31 `_, and
+`ES.32 `_.
+
+Examples:
+
+.. code-block:: c++
+
+  #define C 0
+  #define F1(x, y) ((a) > (b) ? (a) : (b))
+  #define F2(...) (__VA_ARGS__)
+  #define COMMA ,
+  #define NORETURN [[noreturn]]
+  #define DEPRECATED attribute((deprecated))
+  #if LIB_EXPORTS
+  #define DLLEXPORTS __declspec(dllexport)
+  #else
+  #define DLLEXPORTS __declspec(dllimport)
+  #endif
+
+results in the following warnings:
+
+.. code-block:: c++
+
+  4 warnings generated.
+  test.cpp:1:9: warning: macro 'C' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage]
+  #define C 0
+  ^
+  test.cpp:2:9: warning: function-like macro 'F1' used; consider a 'constexpr' template function [cppcoreguidelines-mac

[PATCH] D96286: [clangd][NFC] Change TidyProvider cache to use a linked list approach

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96286

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

[clang] d261660 - Fix the use of -fno-approx-func along with -Ofast or -ffast-math

[Masoud Ataei via cfe-commits]

Author: Masoud Ataei
Date: 2022-01-19T08:05:08-08:00
New Revision: d261660af96d402689d025b4584e925869a9b413

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

LOG: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

Fix how -fapprox-func interact correctly with the other floating point options.
Reported bug Number 52565: https://bugs.llvm.org/show_bug.cgi?id=52565

Differential: https://reviews.llvm.org/D114564
Reviewer: @andrew.w.kaylor

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/fast-math.c
clang/test/Preprocessor/aarch64-target-features.c
clang/test/Preprocessor/arm-target-features.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index fd300fbe40145..253c52cf0ba85 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2902,6 +2902,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   AssociativeMath = true;
   ReciprocalMath = true;
   SignedZeros = false;
+  ApproxFunc = true;
   TrappingMath = false;
   FPExceptionBehavior = "";
   break;
@@ -2909,6 +2910,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   AssociativeMath = false;
   ReciprocalMath = false;
   SignedZeros = true;
+  ApproxFunc = false;
   TrappingMath = true;
   FPExceptionBehavior = "strict";
 
@@ -2928,6 +2930,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   MathErrno = false;
   AssociativeMath = true;
   ReciprocalMath = true;
+  ApproxFunc = true;
   SignedZeros = false;
   TrappingMath = false;
   RoundingFPMath = false;
@@ -2943,6 +2946,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   MathErrno = TC.IsMathErrnoDefault();
   AssociativeMath = false;
   ReciprocalMath = false;
+  ApproxFunc = false;
   SignedZeros = true;
   // -fno_fast_math restores default denormal and fpcontract handling
   DenormalFPMath = DefaultDenormalFPMath;
@@ -2961,7 +2965,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   // If -ffp-model=strict has been specified on command line but
   // subsequent options conflict then emit warning diagnostic.
   if (HonorINFs && HonorNaNs && !AssociativeMath && !ReciprocalMath &&
-  SignedZeros && TrappingMath && RoundingFPMath &&
+  SignedZeros && TrappingMath && RoundingFPMath && !ApproxFunc &&
   DenormalFPMath == llvm::DenormalMode::getIEEE() &&
   DenormalFP32Math == llvm::DenormalMode::getIEEE() &&
   FPContract.equals("off"))
@@ -2994,7 +2998,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
 CmdArgs.push_back("-fmath-errno");
 
   if (!MathErrno && AssociativeMath && ReciprocalMath && !SignedZeros &&
-  !TrappingMath)
+  ApproxFunc && !TrappingMath)
 CmdArgs.push_back("-menable-unsafe-fp-math");
 
   if (!SignedZeros)
@@ -3045,7 +3049,7 @@ static void RenderFloatingPointOptions(const ToolChain 
&TC, const Driver &D,
   // -ffast-math enables the __FAST_MATH__ preprocessor macro, but check for 
the
   // individual features enabled by -ffast-math instead of the option itself as
   // that's consistent with gcc's behaviour.
-  if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath &&
+  if (!HonorINFs && !HonorNaNs && !MathErrno && AssociativeMath && ApproxFunc 
&&
   ReciprocalMath && !SignedZeros && !TrappingMath && !RoundingFPMath) {
 CmdArgs.push_back("-ffast-math");
 if (FPModel.equals("fast")) {

diff  --git a/clang/test/Driver/fast-math.c b/clang/test/Driver/fast-math.c
index 8d293c2ea01ff..a66e9d0eca177 100644
--- a/clang/test/Driver/fast-math.c
+++ b/clang/test/Driver/fast-math.c
@@ -70,6 +70,23 @@
 // CHECK-NO-NANS-NO-FAST-MATH: "-cc1"
 // CHECK-NO-NANS-NO-FAST-MATH-NOT: "-menable-no-nans"
 //
+// RUN: %clang -### -ffast-math -fno-approx-func -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST-MATH-NO-APPROX-FUNC %s
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-cc1"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-menable-no-infs"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-menable-no-nans"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-fno-signed-zeros"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-mreassociate"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-freciprocal-math"
+// CHECK-FAST-MATH-NO-APPROX-FUNC: "-ffp-contract=fast"
+// CHECK-FAST-MATH-NO-APPROX-FUNC-NOT: "-ffast-math"
+// CHECK-FAST-MATH-NO-APPROX-FUNC-NOT: "-fapprox-func"
+//
+// RUN: %clang -### -fno-approx-func -ffast-math -c %s 2>&1 \
+// RUN:   | FileChe

[PATCH] D93164: [AST] Add generator for source location introspection

[Lance Fredrickson via Phabricator via cfe-commits]

lancethepants added a comment.

> @lancethepants @smeenai thanks for the pointers! Unfortunately, it still 
> doesn't work for me after passing -DCMAKE_SYSTEM_NAME="Linux". Passing that 
> option did change the CMake output, so it's definitely recognized at least 
> (the messages about which sanitizer tests will run now call the platform 
> "Linux" as opposed to referring to the platform as default). I am also 
> compiling on Linux for Linux, I'm just trying to build an AArch64 toolchain 
> which I can copy to an SD card for testing on an AArch64 board. 
> @lancethepants could you check your CMake version where you got this to work? 
> I'm using CMake 3.22.1 and trying to build llvmorg-13.0.0
>
> The full invocation I'm using is the following:
>
>   cmake -G Ninja \
> -DLLVM_TABLEGEN=${TOP}/${CLANG_TBLGEN_DIR}/bin/llvm-tblgen \
> -DCLANG_TABLEGEN=${TOP}/${CLANG_TBLGEN_DIR}/bin/clang-tblgen \
> -DCMAKE_SYSTEM_NAME="Linux" \
> -DCMAKE_CROSSCOMPILING=ON \
> -DCLANG_TOOLING_BUILD_AST_INTROSPECTION=OFF \
> -DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \
> -DCMAKE_CXX_COMPILER=aarch64-linux-gnu-g++ \
> -DCMAKE_BUILD_TYPE=MinSizeRel \
> -DCMAKE_INSTALL_PREFIX=${TOP}/${ROOT_DIR} \
> -DLLVM_DEFAULT_TARGET_TRIPLE=aarch64-linux-gnu \
> -DLLVM_NATIVE_ARCH="X86" \
> -DLLVM_TARGET_ARCH="AArch64" \
> -DLLVM_TARGETS_TO_BUILD="AArch64" \
> -DLLVM_ENABLE_PROJECTS="clang;compiler-rt;lld;" \
> -DLLVM_ENABLE_TERMINFO=OFF \
> -DLLVM_ENABLE_BINDINGS=OFF \
> -DLLVM_BUILD_EXAMPLES=OFF \
> -DLLVM_BUILD_TESTS=OFF \
> -DLLVM_BUILD_BENCHMARKS=OFF \
> ${TOP}/${SRC_DIR}/llvm/

Are you stil encountering the same issue?
This is my cmake invocation.
https://github.com/lancethepants/tomatoware/blob/master/scripts/buildroot.sh#L808-L843

I typically update cmake to the most recent whenver I get around to updating my 
project. The latest I've used is 3.21.4. Looks like you're using the very 
newest, so I doubt it's the difference between my version and your version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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

[PATCH] D117431: [IRBuilder] Migrate and-folding to value-based FoldAnd.

[Florian Hahn via Phabricator via cfe-commits]

fhahn updated this revision to Diff 401253.
fhahn added a comment.

rebase to trigger new run of precommit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117431

Files:
  clang/test/CodeGen/catch-nullptr-and-nonzero-offset.c
  clang/test/CodeGen/catch-pointer-overflow.c
  clang/test/CodeGen/cmse-clear-return.c
  llvm/include/llvm/Analysis/InstSimplifyFolder.h
  llvm/include/llvm/Analysis/TargetFolder.h
  llvm/include/llvm/IR/ConstantFolder.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IRBuilderFolder.h
  llvm/include/llvm/IR/NoFolder.h
  llvm/test/Transforms/LoopIdiom/X86/left-shift-until-bittest.ll
  llvm/test/Transforms/LoopVersioning/bound-check-partially-known.ll
  polly/test/CodeGen/invariant_load_base_pointer_conditional.ll
  polly/test/CodeGen/multidim_2d_parametric_array_static_loop_bounds.ll
  polly/test/CodeGen/no-overflow-tracking.ll
  polly/test/CodeGen/run-time-condition-with-scev-parameters.ll
  polly/test/ScopInfo/allow-all-parameters-dereferencable.ll
  polly/test/ScopInfo/invariant_load_zext_parameter.ll

Index: polly/test/ScopInfo/invariant_load_zext_parameter.ll
===
--- polly/test/ScopInfo/invariant_load_zext_parameter.ll
+++ polly/test/ScopInfo/invariant_load_zext_parameter.ll
@@ -25,10 +25,11 @@
 ; CODEGEN-NEXT:   store i32 %polly.access.I0.load, i32* %loadI1a.preload.s2a
 ; CODEGEN-NEXT:   %0 = sext i32 %polly.access.I0.load to i64
 ; CODEGEN-NEXT:   %1 = icmp eq i64 %0, 0
+; CODEGEN-NEXT:   %polly.preload.cond.result = and i1 %1, true
 ; CODEGEN-NEXT:   br label %polly.preload.cond
 ;
 ; CODEGEN:  polly.preload.cond:
-; CODEGEN-NEXT:   br i1 %1, label %polly.preload.exec, label %polly.preload.merge
+; CODEGEN-NEXT:   br i1 %polly.preload.cond.result, label %polly.preload.exec, label %polly.preload.merge
 ;
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
Index: polly/test/ScopInfo/allow-all-parameters-dereferencable.ll
===
--- polly/test/ScopInfo/allow-all-parameters-dereferencable.ll
+++ polly/test/ScopInfo/allow-all-parameters-dereferencable.ll
@@ -25,7 +25,7 @@
 ; Check that without the option `-polly-allow-dereference-of-all-function-parameters`
 ; we do generate the runtime check.
 ; CODE-RTC: polly.preload.cond:   ; preds = %polly.preload.begin
-; CODE-RTC-NEXT: br i1 %{{[a-zA-Z0-9]*}}, label %polly.preload.exec, label %polly.preload.merge
+; CODE-RTC-NEXT: br i1 %{{[a-zA-Z0-9\.]*}}, label %polly.preload.exec, label %polly.preload.merge
 
 ; Check that we don't generate a runtime check because we treat all
 ; parameters as dereferencable.
Index: polly/test/CodeGen/run-time-condition-with-scev-parameters.ll
===
--- polly/test/CodeGen/run-time-condition-with-scev-parameters.ll
+++ polly/test/CodeGen/run-time-condition-with-scev-parameters.ll
@@ -15,7 +15,8 @@
 ; CHECK-NEXT:  %5 = sext i1 %4 to i64
 ; CHECK-NEXT:  %6 = icmp eq i64 0, %5
 ; CHECK-NEXT:  %7 = and i1 %2, %6
-; CHECK-NEXT:  br i1 %7, label %polly.start, label %for.body4
+; CHECK-NEXT:  %polly.rtc.result = and i1 %7, true
+; CHECK-NEXT:  br i1 %polly.rtc.result, label %polly.start, label %for.body4
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
Index: polly/test/CodeGen/no-overflow-tracking.ll
===
--- polly/test/CodeGen/no-overflow-tracking.ll
+++ polly/test/CodeGen/no-overflow-tracking.ll
@@ -30,12 +30,13 @@
 ; IR-NEXT:   %21 = add nsw i64 %20, %19
 ; IR-NEXT:   %22 = icmp sge i64 %21, -2147483648
 ; IR-NEXT:   %23 = and i1 %18, %22
-; IR-NEXT:   br label %polly.preload.cond1
+; IR-NEXT:   %polly.preload.cond.result1 = and i1 %23, true
+; IR-NEXT:   br label %polly.preload.cond2
 ;
-; IR:  polly.preload.cond1:
-; IR-NEXT:   br i1 %23
+; IR:  polly.preload.cond2:
+; IR-NEXT:   br i1 %polly.preload.cond.result1
 ;
-; IR:  polly.preload.exec3:
+; IR:  polly.preload.exec4:
 ; IR-NEXT:   %polly.access.polly.preload.tmp1.merge = getelementptr i32, i32* %polly.preload.tmp1.merge, i64 0
 ; IR-NEXT:   %polly.access.polly.preload.tmp1.merge.load = load i32, i32* %polly.access.polly.preload.tmp1.merge, align 4
 ;
Index: polly/test/CodeGen/multidim_2d_parametric_array_static_loop_bounds.ll
===
--- polly/test/CodeGen/multidim_2d_parametric_array_static_loop_bounds.ll
+++ polly/test/CodeGen/multidim_2d_parametric_array_static_loop_bounds.ll
@@ -12,7 +12,8 @@
 ; CHECK: entry:
 ; CHECK: polly.split_new_and_old:
 ; CHECK: %0 = icmp sge i64 %m, 150
-; CHECK: br i1 %0, label %polly.start, label %for.i
+; CHECK: %polly.rtc.result = and i1 %0, true
+; CHECK: br i1 %polly.rtc.result, label %polly.start, label %for.i
 
 define void @foo(i64 %n

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan updated this revision to Diff 401254.
Jake-Egan marked an inline comment as done.
Jake-Egan added a comment.

Removed the unncessary pipe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

Files:
  clang/test/InterfaceStubs/object.c


Index: clang/test/InterfaceStubs/object.c
===
--- clang/test/InterfaceStubs/object.c
+++ clang/test/InterfaceStubs/object.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fvisibility default -o - -emit-interface-stubs %s | 
FileCheck -check-prefix=CHECK-TAPI %s
-// RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck 
-check-prefix=CHECK-SYMBOLS %s
+// RUN: %clang -fvisibility=default -c -o %t.o %s
+// RUN: llvm-nm %t.o 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
 
 // CHECK-TAPI: data", Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data


Index: clang/test/InterfaceStubs/object.c
===
--- clang/test/InterfaceStubs/object.c
+++ clang/test/InterfaceStubs/object.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fvisibility default -o - -emit-interface-stubs %s | FileCheck -check-prefix=CHECK-TAPI %s
-// RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
+// RUN: %clang -fvisibility=default -c -o %t.o %s
+// RUN: llvm-nm %t.o 2>&1 | FileCheck -check-prefix=CHECK-SYMBOLS %s
 
 // CHECK-TAPI: data", Type: Object, Size: 4 }
 // CHECK-SYMBOLS: data
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added inline comments.



Comment at: clang/test/InterfaceStubs/object.c:2
 // RUN: %clang_cc1 -fvisibility default -o - -emit-interface-stubs %s | 
FileCheck -check-prefix=CHECK-TAPI %s
-// RUN: %clang -fvisibility=default -c -o - %s | llvm-nm - 2>&1 | FileCheck 
-check-prefix=CHECK-SYMBOLS %s
+// RUN: %clang -fvisibility=default -c -o %t.o %s | llvm-nm %t.o 2>&1 | 
FileCheck -check-prefix=CHECK-SYMBOLS %s
 

hubert.reinterpretcast wrote:
> With the temporary file added, the pipe is unnecessary (and possibly 
> unwanted). Does the suggestion here work?
Yes this suggestion works. I updated the patch accordingly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

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

[clang] 8e53ae3 - [clang][dataflow] Add a transfer function for conditional operator

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-19T16:25:05Z
New Revision: 8e53ae3d37190c7442f87915a49f3f292d1d9956

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

LOG: [clang][dataflow] Add a transfer function for conditional operator

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed-by: xazax.hun

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 865191916e502..32a05333923f5 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -404,6 +404,16 @@ class TransferVisitor : public 
ConstStmtVisitor {
 }
   }
 
+  void VisitConditionalOperator(const ConditionalOperator *S) {
+// FIXME: Revisit this once flow conditions are added to the framework. For
+// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
+// condition.
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
   // FIXME: Add support for:
   // - CXXBoolLiteralExpr
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index eef1240f0da2f..5979870f858b1 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1788,4 +1788,44 @@ TEST_F(TransferTest, DerefDependentPtr) {
   });
 }
 
+TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
+  std::string Code = R"(
+struct A {};
+
+void target(A Foo, A Bar, bool Cond) {
+  A Baz = Cond ?  Foo : Bar;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+
+const auto *BazVal =
+dyn_cast(Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_NE(BazVal, FooVal);
+EXPECT_NE(BazVal, BarVal);
+  });
+}
+
 } // namespace



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

[PATCH] D117667: [clang][dataflow] Add a transfer function for conditional operator

[Stanislav Gatev 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 rG8e53ae3d3719: [clang][dataflow] Add a transfer function for 
conditional operator (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117667

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1788,4 +1788,44 @@
   });
 }
 
+TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
+  std::string Code = R"(
+struct A {};
+
+void target(A Foo, A Bar, bool Cond) {
+  A Baz = Cond ?  Foo : Bar;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+
+const auto *BazVal =
+dyn_cast(Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_NE(BazVal, FooVal);
+EXPECT_NE(BazVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -404,6 +404,16 @@
 }
   }
 
+  void VisitConditionalOperator(const ConditionalOperator *S) {
+// FIXME: Revisit this once flow conditions are added to the framework. For
+// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
+// condition.
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
   // FIXME: Add support for:
   // - CXXBoolLiteralExpr
 


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1788,4 +1788,44 @@
   });
 }
 
+TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
+  std::string Code = R"(
+struct A {};
+
+void target(A Foo, A Bar, bool Cond) {
+  A Baz = Cond ?  Foo : Bar;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooVal =
+cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+
+const auto *BazVal =
+dyn_cast(Env.getValue(*BazDecl, SkipPast::None));
+ASSERT_THAT(BazVal, NotNull());
+
+EXPECT_NE(BazVal, FooVal);
+EXPECT_NE(BazVal, BarVal);
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -404,6 +404,16 @@
 }
   }
 
+  void VisitConditionalOperator(const ConditionalOperator *S) {
+// FIXME: Revisit this once flow conditions are added to the framework. For
+// `a = b ? c : d` we can add `b => a == c && !b => a == d` to the flow
+// condition.
+auto &Loc = Env.createStorageLocation(*S);
+Env.setStorageLocation(*S, Loc);
+if (Value *Val = Env.createValue(S->getType()))
+  Env.setValue(Loc, *Val);
+  }
+
   // FIXME: Add support for:
   //

[clang] 9fbd33a - [OMPIRBuilder] Add support for simd (loop) directive.

[Arnamoy Bhattacharyya via cfe-commits]

Author: Arnamoy Bhattacharyya
Date: 2022-01-19T11:32:17-05:00
New Revision: 9fbd33ad623d2b576fc563545bbdf2c257cdf709

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

LOG: [OMPIRBuilder] Add support for simd (loop) directive.

This patch adds OMPIRBuilder support for the simd directive (without any 
clause).  This will be a first step towards lowering simd directive in 
LLVM_Flang.  The patch uses existing CanonicalLoop infrastructure of IRBuilder 
to add the support.  Also adds necessary code to add llvm.access.group and 
llvm.loop metadata wherever needed.

Reviewed By: Meinersbur

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

Added: 
clang/test/OpenMP/irbuilder_simd.cpp

Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 11af2812a4118..0db59dd2624c5 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -2584,7 +2584,67 @@ static void emitOMPSimdRegion(CodeGenFunction &CGF, 
const OMPLoopDirective &S,
   }
 }
 
+static bool isSupportedByOpenMPIRBuilder(const OMPExecutableDirective &S) {
+  // Check for unsupported clauses
+  if (!S.clauses().empty()) {
+// Currently no clause is supported
+return false;
+  }
+
+  // Check if we have a statement with the ordered directive.
+  // Visit the statement hierarchy to find a compound statement
+  // with a ordered directive in it.
+  if (const auto *CanonLoop = dyn_cast(S.getRawStmt())) {
+if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
+  for (const Stmt *SubStmt : SyntacticalLoop->children()) {
+if (!SubStmt)
+  continue;
+if (const CompoundStmt *CS = dyn_cast(SubStmt)) {
+  for (const Stmt *CSSubStmt : CS->children()) {
+if (!CSSubStmt)
+  continue;
+if (isa(CSSubStmt)) {
+  return false;
+}
+  }
+}
+  }
+}
+  }
+  return true;
+}
+
 void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {
+  bool UseOMPIRBuilder =
+  CGM.getLangOpts().OpenMPIRBuilder && isSupportedByOpenMPIRBuilder(S);
+  if (UseOMPIRBuilder) {
+auto &&CodeGenIRBuilder = [this, &S, UseOMPIRBuilder](CodeGenFunction &CGF,
+  PrePostActionTy &) {
+  // Use the OpenMPIRBuilder if enabled.
+  if (UseOMPIRBuilder) {
+// Emit the associated statement and get its loop representation.
+llvm::DebugLoc DL = SourceLocToDebugLoc(S.getBeginLoc());
+const Stmt *Inner = S.getRawStmt();
+llvm::CanonicalLoopInfo *CLI =
+EmitOMPCollapsedCanonicalLoopNest(Inner, 1);
+
+llvm::OpenMPIRBuilder &OMPBuilder =
+CGM.getOpenMPRuntime().getOMPBuilder();
+// Add SIMD specific metadata
+OMPBuilder.applySimd(DL, CLI);
+return;
+  }
+};
+{
+  auto LPCRegion =
+  CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
+  OMPLexicalScope Scope(*this, S, OMPD_unknown);
+  CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_simd,
+  CodeGenIRBuilder);
+}
+return;
+  }
+
   ParentLoopDirectiveForScanRegion ScanRegion(*this, S);
   OMPFirstScanLoop = true;
   auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {

diff  --git a/clang/test/OpenMP/irbuilder_simd.cpp 
b/clang/test/OpenMP/irbuilder_simd.cpp
new file mode 100644
index 0..9f369290f2db5
--- /dev/null
+++ b/clang/test/OpenMP/irbuilder_simd.cpp
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -fopenmp-enable-irbuilder -verify -fopenmp 
-fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s 
+// expected-no-diagnostics
+
+struct S {
+  int a, b;
+};
+
+struct P {
+  int a, b;
+};
+
+void simple(float *a, float *b, int *c) {
+  S s, *p;
+  P pp;
+#pragma omp simd
+  for (int i = 3; i < 32; i += 5) {
+// llvm.access.group test
+// CHECK: %[[A_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[B_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[S:.+]] = alloca %struct.S, align 4
+// CHECK: %[[P:.+]] = alloca %struct.S*, align 8
+// CHECK: %[[I:.+]] = alloca i32, align 4
+// CHECK: %[[TMP3:.+]] = load float*, float** %[[B_ADDR:.+]], align 8, 
!llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[TMP4:.+]] = load i32, i32* %[[I:.+]], align 4, 
!llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[IDXPROM:.+]] = sext i32 %[[TMP4:.+]] to i64
+// CH

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Arnamoy B via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9fbd33ad623d: [OMPIRBuilder] Add support for simd (loop) 
directive. (authored by arnamoy10).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114379

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -1662,6 +1662,37 @@
   EXPECT_FALSE(verifyModule(*M, &errs()));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimd) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(DL, CLI);
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -13,6 +13,7 @@
 //===--===//
 
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/AssumptionCache.h"
@@ -2145,6 +2146,19 @@
   Latch->getTerminator()->setMetadata(LLVMContext::MD_loop, LoopID);
 }
 
+/// Attach llvm.access.group metadata to the memref instructions of \p Block
+static void addSimdMetadata(BasicBlock *Block, MDNode *AccessGroup,
+LoopInfo &LI) {
+  for (Instruction &I : *Block) {
+if (I.mayReadOrWriteMemory()) {
+  // TODO: This instruction may already have access group from
+  // other pragmas e.g. #pragma clang loop vectorize.  Append
+  // so that the existing metadata is not overwritten.
+  I.setMetadata(LLVMContext::MD_access_group, AccessGroup);
+}
+  }
+}
+
 void OpenMPIRBuilder::unrollLoopFull(DebugLoc, CanonicalLoopInfo *Loop) {
   LLVMContext &Ctx = Builder.getContext();
   addLoopMetadata(
@@ -2160,6 +2174,53 @@
 });
 }
 
+void OpenMPIRBuilder::applySimd(DebugLoc, CanonicalLoopInfo *CanonicalLoop) {
+  LLVMContext &Ctx = Builder.getContext();
+
+  Function *F = CanonicalLoop->getFunction();
+
+  FunctionAnalysisManager FAM;
+  FAM.registerPass([]() { return DominatorTreeAnalysis(); });
+  FAM.registerPass([]() { return LoopAnalysis(); });
+  FAM.registerPass([]() { return PassInstrumentationAnalysis(); });
+
+  LoopAnalysis LIA;
+  LoopInfo &&LI = LIA.run(*F, FAM);
+
+  Loop *L = LI.getLoopFor(CanonicalLoop->getHeader());
+
+  SmallSet Reachable;
+
+  // Get the basic blocks from the loop in which memref instructions
+  // can be found.
+  // TODO: Generalize getting all blocks inside a CanonicalizeLoopInfo,
+  // preferably without running any passes.
+  for (BasicBlock *Block : L->getBlocks()) {
+if (Block == CanonicalLoop->getCond() ||
+Block == CanonicalLoop->getHeader())
+  continue;
+Reachable.insert(Block);
+  }
+
+  // Add access group metadata to memory-access instructions.
+  MDNode *AccessGroup = MDNode::getDistinct(Ctx, {});
+  for (BasicBlock *BB : Reachable)
+addSimdMetadata(BB, AccessGroup, LI);
+
+  // Use the above access group metadata to create loop level
+  // metadata, which should be distinct for each loop.
+  ConstantAsMetadata *BoolConst =
+  ConstantAsMetadata::get(ConstantInt::getTrue(Type::getInt1Ty(Ctx)));
+  // TODO:  If the loop has existing parallel access metadata, have
+  // to combine two lists.
+  addLoopMetadata(
+  CanonicalLoop,
+  {MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.parallel_accesses"),
+ AccessGroup}),
+   MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
+ BoolConst})});
+}
+
 /// Create the TargetMachine object to query

[PATCH] D117468: [RISCV] Add intrinsic for Zbt extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll:21
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsr a0, a1, a0, a2
+; RV32ZBT-NEXT:ret

This should have the same register order as the fsl test case. I think the test 
just needs to be re-generated.



Comment at: llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll:27
+
+define i32 @fsri_i32(i32 %a, i32 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i32:

Please add an fsli test case using llvm.riscv.fsl.i32 and a constant shift 
amount



Comment at: llvm/test/CodeGen/RISCV/rv32zbt-intrinsic.ll:30
+; RV32ZBT:   # %bb.0:
+; RV32ZBT-NEXT:fsri a0, a1, a0, 5
+; RV32ZBT-NEXT:ret

This should be fsri a0, a0, a1, 5



Comment at: llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll:27
+
+define i32 @fsri_i32(i32 %a, i32 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i32:

Please add an fsli test case.



Comment at: llvm/test/CodeGen/RISCV/rv64zbt-intrinsic.ll:58
+
+define i64 @fsri_i64(i64 %a, i64 %b) nounwind {
+; RV32ZBT-LABEL: fsri_i64:

Please add a fsri test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117468

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

[PATCH] D117301: [clang][NFC] Wrap TYPE_SWITCH in "do while (0)" in the interpreter

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 401257.
owenpan added a comment.

- Also wrapped `COMPOSITE_TYPE_SWITCH` with do-while.
- Cleaned up the macro definitions by removing the superfluous do-while 
statements.
- Removed the unused `INT_TPYE_SWITCH` macro.


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

https://reviews.llvm.org/D117301

Files:
  clang/lib/AST/Interp/PrimType.h


Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -81,35 +81,27 @@
 /// Helper macro to simplify type switches.
 /// The macro implicitly exposes a type T in the scope of the inner block.
 #define TYPE_SWITCH_CASE(Name, B) \
-  case Name: { using T = PrimConv::T; do {B;} while(0); break; }
+  case Name: { using T = PrimConv::T; B; break; }
 #define TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-TYPE_SWITCH_CASE(PT_Bool, B)   
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-  }
+  do { 
\
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Sint8, B)
\
+  TYPE_SWITCH_CASE(PT_Uint8, B)
\
+  TYPE_SWITCH_CASE(PT_Sint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint16, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint32, B)   
\
+  TYPE_SWITCH_CASE(PT_Sint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Uint64, B)   
\
+  TYPE_SWITCH_CASE(PT_Bool, B) 
\
+  TYPE_SWITCH_CASE(PT_Ptr, B)  
\
+}  
\
+  } while (0)
 #define COMPOSITE_TYPE_SWITCH(Expr, B, D)  
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Ptr, B)
\
-default: do { D; } while(0); break;
\
-  }
-#define INT_TYPE_SWITCH(Expr, B)   
\
-  switch (Expr) {  
\
-TYPE_SWITCH_CASE(PT_Sint8, B)  
\
-TYPE_SWITCH_CASE(PT_Uint8, B)  
\
-TYPE_SWITCH_CASE(PT_Sint16, B) 
\
-TYPE_SWITCH_CASE(PT_Uint16, B) 
\
-TYPE_SWITCH_CASE(PT_Sint32, B) 
\
-TYPE_SWITCH_CASE(PT_Uint32, B) 
\
-TYPE_SWITCH_CASE(PT_Sint64, B) 
\
-TYPE_SWITCH_CASE(PT_Uint64, B) 
\
-default: llvm_unreachable("not an integer");   
\
-  }
+  do { 
\
+switch (Expr) {
\
+  TYPE_SWITCH_CASE(PT_Ptr, B)  
\
+  default: { D; break; }   
\
+}  
\
+  } while (0)
 #endif


Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -81,35 +81,27 @@
 /// Helper macro to simplify type switches.
 /// The macro implicitly exposes a type T in the scope of the inner block.
 #define TYP

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D116203#3220165 , @aaron.ballman 
wrote:

> The summary for the patch explains what's being added, but there's really no 
> information as to why it's being added. Why do we need these builtins?

Btw, I still think the patch summary should be updated with this information.




Comment at: clang/include/clang/AST/Type.h:6488
+  const auto *F = Self.getAs();
+  return F == nullptr ||
+ (F->getMethodQuals().empty() && F->getRefQualifier() == RQ_None);

cjdb wrote:
> aaron.ballman wrote:
> > A function without a prototype is referenceable? (This is more of a "should 
> > this predicate do anything in C?" kind of question, I suppose.)
> Does C++ have a notion of non-prototyped functions? I don't think it does? As 
> such, I'm struggling to model this in a way that makes sense :(
> 
> Maybe that's evidence for NAD, maybe it isn't :shrug:
C++ doesn't have the notion of a function without a prototype (thankfully).

Should this function assert that it's not called in C mode at all? I don't 
think asking the question makes sense in C.



Comment at: clang/include/clang/AST/Type.h:6482
+  //   cv-qualifiers or a ref-qualifier, or a reference type.
+  assert(getTypePtr() && "QualType must be valid in order to be 
referenceable");
+  const Type &Self = **this;

Nope, that will still assert. Here's a better approach.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8595
+def err_make_signed_integral_only : Error<
+  "'%select{make_unsigned|make_signed}0' is only compatible with non-bool 
integers and enum types, but was given %1">;
 def ext_typecheck_cond_incompatible_pointers : ExtWarn<

cjdb wrote:
> aaron.ballman wrote:
> > This can be reformatted, I believe, but did you look around to see if an 
> > existing diagnostic would suffice?
> Do you have any tips on how to narrow my search? I don't really want to 
> //read// 11K lines of diagnostics.
I usually search for "typecheck" for type checking diagnostics. One that looks 
close is:
```
def err_pragma_loop_invalid_argument_type : Error<
  "invalid argument of type %0; expected an integer type">;
```
that could likely be changed to something along the lines of:
```
def err_invalid_argument_type : Error<
  "invalid argument of type %0; expected an integer type %select{|other than 
'bool'}1">;
```
(I think enum types are always integer types and so they may not need to be 
called out in the diagnostic.)



Comment at: clang/lib/AST/ASTContext.cpp:5604
 /// savings are minimal and these are rare.
+// Update 2021-12-16: it might be worth revisiting this
 QualType ASTContext::getUnaryTransformType(QualType BaseType,

cjdb wrote:
> WDYT @aaron.ballman?
Are these expected to be less rare now due to your patch? FWIW, I'm fine 
revisiting if we can measure some value, but I think that's good as a separate 
patch.



Comment at: clang/lib/AST/ItaniumMangle.cpp:3928
 break;
+  case UnaryTransformType::AddConst:
+Out << "2ac";

Are these the suggested manglings from the Itanium mangling document, or 
something you invented yourself?

Should there be corresponding Microsoft manglings or are those already handled 
magically?

Also, test coverage for the manglings?



Comment at: clang/lib/Sema/DeclSpec.cpp:597
   case DeclSpec::TST_decltype_auto: return "decltype(auto)";
+  // clang-format off
   case DeclSpec::TST_underlyingType: return "__underlying_type";

cjdb wrote:
> aaron.ballman wrote:
> > We don't typically add clang-format markings to the source files. I think 
> > this should be removed (it disables the formatting for the remainder of the 
> > file).
> My intention was always to delete these pre-merge. clang-format has some 
> really strong opinions on this that breaks consistency with the rest of the 
> file, and it was disrupting CI.
> 
> > (it disables the formatting for the remainder of the file).
> 
> Line 615 should prevent this :-)
Ah, that makes sense and is fine by me, thanks for letting me know.



Comment at: clang/lib/Sema/SemaType.cpp:9121
+SourceLocation Loc) {
+  if (!BaseType->isPointerType())
+return Context.getUnaryTransformType(BaseType, BaseType, UKind);

cjdb wrote:
> aaron.ballman wrote:
> > Should we care about ObjC pointers (which are a bit special)?
> What's the compat story between ObjC and C++?
Objective-C++ is a thing: 
https://en.wikipedia.org/wiki/Objective-C#Objective-C++

The salient point is that the type system models ObjC pointers as being 
distinct from pointers. e.g., 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/Type.h#L6702


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: rsmith, rjmccall, erichkeane.
aaron.ballman added a comment.

Adding a few more reviewers in case there are more opinions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 401274.
martong marked an inline comment as done.
martong added a comment.

- Move the comment hunk


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
  clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp

Index: clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -verify
+
+template 
+void clang_analyzer_dump(T);
+
+void clang_analyzer_eval(bool);
+
+void test_memory_region_to_integer_cast_and_symbol_simplification(int *p) {
+  long p_as_integer = (long)p;
+  if (42 - p_as_integer < 42)
+return;
+  // 42 - p_as_integer >= 42
+  // p_as_integer <= 0
+
+  if (p)
+return;
+  clang_analyzer_eval(p == 0);// expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{UNKNOWN}}
+
+  if (p_as_integer)
+return;
+  clang_analyzer_eval(p == 0);// expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{TRUE}}
+
+  (void)p_as_integer;
+  (void)p;
+}
Index: clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
===
--- /dev/null
+++ clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -triple x86_64-pc-linux-gnu \
+// RUN:   -verify
+
+template 
+void clang_analyzer_dump(T);
+
+void test_memory_region_to_integer_cast(int *p) {
+  long p_int = (long)p;
+  clang_analyzer_dump(p); // expected-warning{{&SymRegion{reg_$0}}}
+  clang_analyzer_dump(p_int); // expected-warning{{(long) (reg_$0)}}
+}
+
+void test_label_to_integer_cast(bool coin) {
+  int x = 0;
+p: ++x;
+  if (coin)
+goto p;
+
+  // FIXME, below we should produce a SymbolCast instead of the LocAsInteger.
+  // However, for that we'd need to be able to store a loc::GotoLabel SVal
+  // inside the SymbolCast. But currently we can put only another SymExpr
+  // inside a SymbolCast.
+
+  // Use of GNU address-of-label extension.
+  clang_analyzer_dump((long)&&p); // expected-warning{{&&p [as 64 bit integer]}}
+}
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -773,6 +773,16 @@
   //QualType elemTy = cast(originalTy)->getElementType();
   //QualType pointerTy = C.getPointerType(elemTy);
 }
+
+AnalyzerOptions &Opts =
+StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
+if (Opts.ShouldSupportSymbolicIntegerCasts) {
+  const MemRegion *R = V.getRegion();
+  if (R && !OriginalTy.isNull())
+if (const auto *SR = dyn_cast(R))
+  return simplifySymbolCast(SR->getSymbol(), CastTy);
+}
+
 const unsigned BitWidth = Context.getIntWidth(CastTy);
 return makeLocAsInteger(Val.castAs(), BitWidth);
   }
@@ -1009,8 +1019,12 @@
   return V;
 }
 
-SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
+SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
   QualType CastTy) {
+  QualType T = Context.getCanonicalType(SE->getType());
+  if (!isa(SE))
+return makeNonLoc(SE, T, CastTy);
+
   // We use seven conditions to recognize a simplification case.
   // For the clarity let `CastTy` be `C`, SE->getType() - `T`, root type - `R`,
   // prefix `u` for unsigned, `s` for signed, no prefix - any sign:
@@ -1044,15 +1058,6 @@
   //  (uint)(ushort)(ushort x) -> (uint)(ushort x)
   //  (llong)(ulong)(uint x) -> (llong)(uint x) (sizeof(ulong) == sizeof(uint))
 
-  SymbolRef SE = V.getSymbol();
-  QualType T = Context.getCanonicalType(SE->getType());
-
-  if (T == CastTy)
-return V;
-
-  if (!isa(SE))
-return makeNonLoc(SE, T, CastTy);
-
   SymbolRef RootSym = cast(SE)->getOperand();
   QualType RT = RootSym->getType().getCanonicalType();
 
@@ -1080,3 +1085,16 @@
 
   return makeNonLoc(SE, T, CastTy);
 }
+
+SVal clang::ento::SValBuilder::simplifySymbolCast(nonl

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1022
 
+SVal clang::ento::SValBuilder::simplifySymbolCast(SymbolRef SE,
+  QualType CastTy) {

ASDenysPetrov wrote:
> And it'd be great if you bring the original comment here, as the logic is 
> here.
Ok, I've moved it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D117587: [ifs] Use a tmp file instead of "-"

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.

LGTM; thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117587

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

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

No comments other than the ones others have made, though the 
make_signed/make_unsigned needs to work for _BitInt




Comment at: clang/lib/Sema/SemaType.cpp:9113
+  BaseType.isReferenceable() || BaseType->isVoidType()
+  ? BuildPointerType(BaseType.getNonReferenceType(), Loc, EntityName)
+  : BaseType;

Do we at any point want this builtin to be address-space aware?  



Comment at: clang/lib/Sema/SemaType.cpp:9227
 
-DiagnoseUseOfDecl(ED, Loc);
+  QualType Underlying = Context.getIntTypeForBitwidth(
+  Context.getIntWidth(BaseType), IsMakeSigned);

Can you add a couple of tests to make sure this works with _BitInt?  Note that 
this + the libc++ fixes get this done: 
https://github.com/llvm/llvm-project/issues/50427


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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

[PATCH] D117419: [clang][cmake] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added inline comments.



Comment at: clang/CMakeLists.txt:3
 
+include(GNUInstallDirs)
+

beanz wrote:
> If GNUInstallDirs is included before the `project` call enables languages it 
> results in a CMake warning being logged.
See D117639 where we are discussing this problem and what to do about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117419

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

[PATCH] D115521: [Templight] Don't display empty strings for names of unnamed template parameters

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

LGTM


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

https://reviews.llvm.org/D115521

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

[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

[Gabor Marton via Phabricator via cfe-commits]

martong added a subscriber: teemperor.
martong added a comment.

Adding Raphael @teemperor , he might have useful comments about the minimal 
mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155

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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/CodeGen/Address.h:30
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
 

nikic wrote:
> dblaikie wrote:
> > aeubanks wrote:
> > > nikic wrote:
> > > > Are we guaranteed 3 bits even on 32-bit architectures?
> > > Apparently not, a static assert fires for a 32-bit build of clang. I was 
> > > assuming 3 was fine based on the PointerIntPair comments.
> > > ```
> > > /// PointerIntPair - This class implements a pair of a pointer and small
> > > /// integer.  It is designed to represent this in the space required by 
> > > one
> > > /// pointer by bitmangling the integer into the low part of the pointer.  
> > > This
> > > /// can only be done for small integers: typically up to 3 bits, but it 
> > > depends
> > > /// on the number of bits available according to PointerLikeTypeTraits 
> > > for the
> > > /// type.
> > > ```
> > > 
> > > I suppose we could have a separate 32-bit vs 64-bit implementation of 
> > > `Address`, although that's not very nice.
> > You can get more spare bits by overaligning the type that you're pointing 
> > to, if that's suitable/workable. We do that in a few places in LLVM for 
> > this sort of reason.
> As we're storing pointers to core LLVM types here (Value and Type), I don't 
> think it's possible to over-align them.
Ah, right :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D117613: [ASTMatchers] Add `isConsteval` matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: cor3ntin, sammccall.
aaron.ballman added a comment.

Adding a few more reviewers in case there are differing opinions.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5173-5174
 
+/// Matches consteval function declarations,
+///and if consteval.
+///





Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5179
+///   consteval int bar();
+///   void baz() { if consteval {} }
+/// \endcode

It'd be good to show an example of:
```
if ! consteval {}

if ! consteval {} else {}
```
as well so users know what to expect.

Should there be a matcher so users can distinguish between `if consteval` and 
`if ! consteval`?

Test cases for these sort of things would also be appreciated.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:1794-1796
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }





Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:1807-1809
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }

Izaron wrote:
> Actually `if consteval` is a C++23 feature, but there is no 
> `isCXX23orLater()` method. The `is consteval` construction seems to be 
> backported to C++20 in clang, as it compiles and just emits a warning
> ```
> warning: consteval if is a C++2b extension [-Wc++2b-extensions]
> ```
Yes, we support `if consteval` as an extension in older language modes, so I 
think this is reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117613

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

[ksyx via Phabricator via cfe-commits]

ksyx updated this revision to Diff 401293.
ksyx edited the summary of this revision.
ksyx added a comment.

Recognize long (len>=5) uppercased name taking a single line as return type
and fix the problem of adding newline below it:

  void
  afunc(int x) {
return;
  }
  TYPENAME
  func(int x, int y) {
// ...
  }


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

https://reviews.llvm.org/D117520

Files:
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/DefinitionBlockSeparator.h
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp

Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -128,6 +128,60 @@
"\n"
"enum Bar { FOOBAR, BARFOO };\n",
Style);
+
+  FormatStyle BreakAfterReturnTypeStyle = Style;
+  BreakAfterReturnTypeStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+  // Test uppercased long typename
+  verifyFormat(
+  "class Foo {\n"
+  "  void\n"
+  "  Bar(int t, int p) {\n"
+  "int r = t + p;\n"
+  "return r;\n"
+  "  }\n"
+  "\n"
+  "  LONGTYPENAME\n"
+  "  Foobar(int t, int p) {\n"
+  "int r = t * p;\n"
+  "return r;\n"
+  "  }\n"
+  "}\n",
+  BreakAfterReturnTypeStyle);
+}
+
+TEST_F(DefinitionBlockSeparatorTest, CommentBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+  std::string Prefix = "enum Foo { FOO, BAR };\n"
+   "\n"
+   "/*\n"
+   "test1\n"
+   "test2\n"
+   "*/\n"
+   "int foo(int i, int j) {\n"
+   "  int r = i + j;\n"
+   "  return r;\n"
+   "}\n";
+  std::string Suffix = "enum Bar { FOOBAR, BARFOO };\n"
+   "\n"
+   "/* Comment block in one line*/\n"
+   "int bar3(int j, int k) {\n"
+   "  // A comment\n"
+   "  int r = j % k;\n"
+   "  return r;\n"
+   "}\n";
+  std::string CommentedCode = "/*\n"
+  "int bar2(int j, int k) {\n"
+  "  int r = j / k;\n"
+  "  return r;\n"
+  "}\n"
+  "*/\n";
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode + "\n" +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + "\n" + Suffix);
+  verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode +
+   removeEmptyLines(Suffix),
+   Style, Prefix + "\n" + CommentedCode + Suffix);
 }
 
 TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
@@ -172,13 +226,15 @@
   FormatStyle NeverStyle = getLLVMStyle();
   NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
 
-  auto TestKit = MakeUntouchTest("#ifdef FOO\n\n", "\n#elifndef BAR\n\n",
- "\n#endif\n\n", false);
+  auto TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ "#ifdef FOO\n\n",
+ "\n#elifndef BAR\n\n", "\n#endif\n\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
-  TestKit =
-  MakeUntouchTest("#ifdef FOO\n", "#elifndef BAR\n", "#endif\n", false);
+  TestKit = MakeUntouchTest("/* FOOBAR */\n"
+"#ifdef FOO\n",
+"#elifndef BAR\n", "#endif\n", false);
   verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
   verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
 
@@ -210,7 +266,7 @@
   "test1\n"
   "test2\n"
   "*/\n"
-  "int foo(int i, int j) {\n"
+  "/*const*/ int foo(int i, int j) {\n"
   "  int r = i + j;\n"
   "  return r;\n"
   "}\n"
@@ -222,8 +278,10 @@
   "// Comment line 2\n"
   "// Comment line 3\n"
   "int bar(int j, int k) {\n"
-  "  int r = j * k;\n"
-  "  return r;\n"
+  "  {\n"
+  "int r = j * k;\n"
+  "return r;\n"
+  "  }\n"
   "}\n"
   "\n"
   "int bar2(in

[clang] 85c2bd2 - Prevent adding module flag amdgpu_hostcall multiple times

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2022-01-19T12:52:33-05:00
New Revision: 85c2bd2a0e0e2c1706bbf50203d5bbbeedbbd741

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

LOG: Prevent adding module flag amdgpu_hostcall multiple times

HIP program with printf call fails to compile with -fsanitize=address
option, because of appending module flag - amdgpu_hostcall twice, one
for printf and one for sanitize option. This patch fixes that issue.

Patch by: Praveen Velliengiri

Reviewed by: Yaxun Liu, Roman Lebedev

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

Added: 
clang/test/CodeGenCUDA/amdgpu-asan-printf.cu

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index f710b9ad067d4..e91da73d2f03c 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -565,7 +565,9 @@ void CodeGenModule::Release() {
 "__amdgpu_device_library_preserve_asan_functions_ptr", nullptr,
 llvm::GlobalVariable::NotThreadLocal);
 addCompilerUsedGlobal(Var);
-getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+if (!getModule().getModuleFlag("amdgpu_hostcall")) {
+  getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+}
   }
 
   emitLLVMUsed();

diff  --git a/clang/test/CodeGenCUDA/amdgpu-asan-printf.cu 
b/clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
new file mode 100644
index 0..54dbe5bed4475
--- /dev/null
+++ b/clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -O3 -x hip | FileCheck -check-prefixes=MFCHECK %s
+
+// MFCHECK: !llvm.module.flags = !{![[FLAG1:[0-9]+]], ![[FLAG2:[0-9]+]]}
+// MFCHECK: ![[FLAG1]] = !{i32 4, !"amdgpu_hostcall", i32 1}
+
+// Test to check hostcall module flag metadata is generated correctly
+// when a program has printf call and compiled with -fsanitize=address.
+#include "Inputs/cuda.h"
+__device__ void non_kernel() {
+  printf("sanitized device function");
+}
+
+__global__ void kernel() {
+  non_kernel();
+}
+



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

[PATCH] D116216: Prevent adding module flag - amdgpu_hostcall multiple times.

[Yaxun Liu via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85c2bd2a0e0e: Prevent adding module flag amdgpu_hostcall 
multiple times (authored by yaxunl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116216

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/amdgpu-asan-printf.cu


Index: clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -O3 -x hip | FileCheck -check-prefixes=MFCHECK %s
+
+// MFCHECK: !llvm.module.flags = !{![[FLAG1:[0-9]+]], ![[FLAG2:[0-9]+]]}
+// MFCHECK: ![[FLAG1]] = !{i32 4, !"amdgpu_hostcall", i32 1}
+
+// Test to check hostcall module flag metadata is generated correctly
+// when a program has printf call and compiled with -fsanitize=address.
+#include "Inputs/cuda.h"
+__device__ void non_kernel() {
+  printf("sanitized device function");
+}
+
+__global__ void kernel() {
+  non_kernel();
+}
+
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -565,7 +565,9 @@
 "__amdgpu_device_library_preserve_asan_functions_ptr", nullptr,
 llvm::GlobalVariable::NotThreadLocal);
 addCompilerUsedGlobal(Var);
-getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+if (!getModule().getModuleFlag("amdgpu_hostcall")) {
+  getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+}
   }
 
   emitLLVMUsed();


Index: clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-asan-printf.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fsanitize=address \
+// RUN:   -O3 -x hip | FileCheck -check-prefixes=MFCHECK %s
+
+// MFCHECK: !llvm.module.flags = !{![[FLAG1:[0-9]+]], ![[FLAG2:[0-9]+]]}
+// MFCHECK: ![[FLAG1]] = !{i32 4, !"amdgpu_hostcall", i32 1}
+
+// Test to check hostcall module flag metadata is generated correctly
+// when a program has printf call and compiled with -fsanitize=address.
+#include "Inputs/cuda.h"
+__device__ void non_kernel() {
+  printf("sanitized device function");
+}
+
+__global__ void kernel() {
+  non_kernel();
+}
+
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -565,7 +565,9 @@
 "__amdgpu_device_library_preserve_asan_functions_ptr", nullptr,
 llvm::GlobalVariable::NotThreadLocal);
 addCompilerUsedGlobal(Var);
-getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+if (!getModule().getModuleFlag("amdgpu_hostcall")) {
+  getModule().addModuleFlag(llvm::Module::Override, "amdgpu_hostcall", 1);
+}
   }
 
   emitLLVMUsed();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117522: [clang-tidy] Add modernize-macro-to-enum check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 401299.
LegalizeAdulthood added a comment.

- clang-format
- prefer `llvm::any_of` over `std::any_of`


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

https://reviews.llvm.org/D117522

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -0,0 +1,137 @@
+// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum
+
+#if 1
+#include "modernize-macro-to-enum.h"
+
+// These macros are skipped due to being inside a conditional compilation block.
+#define GOO_RED 1
+#define GOO_GREEN 2
+#define GOO_BLUE 3
+
+#endif
+
+#define RED 0xFF
+#define GREEN 0x00FF00
+#define BLUE 0xFF
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: RED = 0xFF,
+// CHECK-FIXES-NEXT: GREEN = 0x00FF00,
+// CHECK-FIXES-NEXT: BLUE = 0xFF
+// CHECK-FIXES-NEXT: };
+
+// Verify that comments are preserved.
+#define CoordModeOrigin 0   /* relative to the origin */
+#define CoordModePrevious   1   /* relative to previous point */
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: CoordModeOrigin = 0,   /* relative to the origin */
+// CHECK-FIXES-NEXT: CoordModePrevious =   1   /* relative to previous point */
+// CHECK-FIXES-NEXT: };
+
+// Verify that multiline comments are preserved.
+#define BadDrawable 9   /* parameter not a Pixmap or Window */
+#define BadAccess   10  /* depending on context:
+- key/button already grabbed
+- attempt to free an illegal 
+  cmap entry 
+- attempt to store into a read-only 
+  color map entry. */
+// - attempt to modify the access control
+//   list from other than the local host.
+//
+#define BadAlloc11  /* insufficient resources */
+// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead
+// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead
+// CHECK-FIXES: enum {
+// CHECK-FIXES-NEXT: BadDrawable = 9,   /* parameter not a Pixmap or Window */
+// CHECK-FIXES-NEXT: BadAccess =   10,  /* depending on context:
+// CHECK-FIXES-NEXT: - key/button already grabbed
+// CHECK-FIXES-NEXT: - attempt to free an illegal 
+// CHECK-FIXES-NEXT:   cmap entry 
+// CHECK-FIXES-NEXT: - attempt to store into a read-only 
+// CHECK-FIXES-NEXT:   color map entry. */
+// CHECK-FIXES-NEXT: // - attempt to modify the access control
+// CHECK-FIXES-NEXT: //   list from other than the local host.
+// CHECK-FIXES-NEXT: //
+// CHECK-FIXES-NEXT: BadAlloc =11  /* insufficie

[clang] baa9b7c - unique_ptrify the ModuleManager's VisitState linked list

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2022-01-19T09:57:46-08:00
New Revision: baa9b7c3c83ab6e4dfb15b8d7815a9958d5b5810

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

LOG: unique_ptrify the ModuleManager's VisitState linked list

Added: 


Modified: 
clang/include/clang/Serialization/ModuleManager.h
clang/lib/Serialization/ModuleManager.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ModuleManager.h 
b/clang/include/clang/Serialization/ModuleManager.h
index 7081eedad4b49..4305bae5ee958 100644
--- a/clang/include/clang/Serialization/ModuleManager.h
+++ b/clang/include/clang/Serialization/ModuleManager.h
@@ -105,10 +105,6 @@ class ModuleManager {
   Stack.reserve(N);
 }
 
-~VisitState() {
-  delete NextState;
-}
-
 /// The stack used when marking the imports of a particular module
 /// as not-to-be-visited.
 SmallVector Stack;
@@ -121,14 +117,14 @@ class ModuleManager {
 unsigned NextVisitNumber = 1;
 
 /// The next visit state.
-VisitState *NextState = nullptr;
+std::unique_ptr NextState;
   };
 
   /// The first visit() state in the chain.
-  VisitState *FirstVisitState = nullptr;
+  std::unique_ptr FirstVisitState;
 
-  VisitState *allocateVisitState();
-  void returnVisitState(VisitState *State);
+  std::unique_ptr allocateVisitState();
+  void returnVisitState(std::unique_ptr State);
 
 public:
   using ModuleIterator = llvm::pointee_iterator<
@@ -142,7 +138,6 @@ class ModuleManager {
   explicit ModuleManager(FileManager &FileMgr, InMemoryModuleCache 
&ModuleCache,
  const PCHContainerReader &PCHContainerRdr,
  const HeaderSearch &HeaderSearchInfo);
-  ~ModuleManager();
 
   /// Forward iterator to traverse all loaded modules.
   ModuleIterator begin() { return Chain.begin(); }

diff  --git a/clang/lib/Serialization/ModuleManager.cpp 
b/clang/lib/Serialization/ModuleManager.cpp
index f4882c7be3f7d..4fd217cf7a6ea 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -304,23 +304,22 @@ ModuleManager::addInMemoryBuffer(StringRef FileName,
   InMemoryBuffers[Entry] = std::move(Buffer);
 }
 
-ModuleManager::VisitState *ModuleManager::allocateVisitState() {
+std::unique_ptr ModuleManager::allocateVisitState() 
{
   // Fast path: if we have a cached state, use it.
   if (FirstVisitState) {
-VisitState *Result = FirstVisitState;
-FirstVisitState = FirstVisitState->NextState;
-Result->NextState = nullptr;
+auto Result = std::move(FirstVisitState);
+FirstVisitState = std::move(Result->NextState);
 return Result;
   }
 
   // Allocate and return a new state.
-  return new VisitState(size());
+  return std::make_unique(size());
 }
 
-void ModuleManager::returnVisitState(VisitState *State) {
+void ModuleManager::returnVisitState(std::unique_ptr State) {
   assert(State->NextState == nullptr && "Visited state is in list?");
-  State->NextState = FirstVisitState;
-  FirstVisitState = State;
+  State->NextState = std::move(FirstVisitState);
+  FirstVisitState = std::move(State);
 }
 
 void ModuleManager::setGlobalIndex(GlobalModuleIndex *Index) {
@@ -351,8 +350,6 @@ ModuleManager::ModuleManager(FileManager &FileMgr,
 : FileMgr(FileMgr), ModuleCache(&ModuleCache),
   PCHContainerRdr(PCHContainerRdr), HeaderSearchInfo(HeaderSearchInfo) {}
 
-ModuleManager::~ModuleManager() { delete FirstVisitState; }
-
 void ModuleManager::visit(llvm::function_ref Visitor,
   llvm::SmallPtrSetImpl *ModuleFilesHit) 
{
   // If the visitation order vector is the wrong size, recompute the order.
@@ -396,11 +393,10 @@ void 
ModuleManager::visit(llvm::function_ref Visitor,
 
 assert(VisitOrder.size() == N && "Visitation order is wrong?");
 
-delete FirstVisitState;
 FirstVisitState = nullptr;
   }
 
-  VisitState *State = allocateVisitState();
+  auto State = allocateVisitState();
   unsigned VisitNumber = State->NextVisitNumber++;
 
   // If the caller has provided us with a hit-set that came from the global
@@ -452,7 +448,7 @@ void 
ModuleManager::visit(llvm::function_ref Visitor,
 } while (true);
   }
 
-  returnVisitState(State);
+  returnVisitState(std::move(State));
 }
 
 bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize,



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

[PATCH] D5980: unique_ptrify ownership of ModuleFiles in the ModuleManager

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D5980#3253378 , @dblaikie wrote:

> Looks like @dexonsmith got this (putting the ownership in `Chain` & using 
> some iterator adapters to narrow the cleanup scope/fallout) in 
> rGa897f7cd40b12f693acbb04b81ef928244637aae 
> . Thanks 
> @dexonsmith!

And just because I was looking at the code anyway, I migrated the 
`FirstVisitState` linked list to `unique_ptr` (thought about `forward_list` but 
given the handling of raw nodes, it seemed simpler not to do that) in 
rGbaa9b7c3c83ab6e4dfb15b8d7815a9958d5b5810 



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

https://reviews.llvm.org/D5980

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

[PATCH] D117419: [clang][cmake] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 401304.
Ericson2314 added a comment.

Avoid warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117419

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake

Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -6,7 +6,8 @@
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  GNUInstallDirs_get_absolute_install_dir(bindir "${outdir}" BINDIR)
+  set(bindir "${DESTDIR}${bindir}")
 
   message(STATUS "Creating ${name}")
 
Index: clang/utils/hmaptool/CMakeLists.txt
===
--- clang/utils/hmaptool/CMakeLists.txt
+++ clang/utils/hmaptool/CMakeLists.txt
@@ -10,7 +10,7 @@
 
 list(APPEND Depends ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
 install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT hmaptool)
 
 add_custom_target(hmaptool ALL DEPENDS ${Depends})
Index: clang/tools/scan-view/CMakeLists.txt
===
--- clang/tools/scan-view/CMakeLists.txt
+++ clang/tools/scan-view/CMakeLists.txt
@@ -20,7 +20,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-view)
   endforeach()
 
@@ -34,7 +34,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-view/${ShareFile})
 install(FILES share/${ShareFile}
-DESTINATION share/scan-view
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-view"
 COMPONENT scan-view)
   endforeach()
 
Index: clang/tools/scan-build/CMakeLists.txt
===
--- clang/tools/scan-build/CMakeLists.txt
+++ clang/tools/scan-build/CMakeLists.txt
@@ -47,7 +47,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -61,7 +61,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libexec/${LibexecFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/libexec/${LibexecFile})
 install(PROGRAMS libexec/${LibexecFile}
-DESTINATION libexec
+DESTINATION "${CMAKE_INSTALL_LIBEXECDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -89,7 +89,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/scan-build/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-build/${ShareFile})
 install(FILES share/scan-build/${ShareFile}
-DESTINATION share/scan-build
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-build"
 COMPONENT scan-build)
   endforeach()
 
Index: clang/tools/scan-build-py/CMakeLists.txt
===
--- clang/tools/scan-build-py/CMakeLists.txt
+++ clang/tools/scan-build-py/CMakeLists.txt
@@ -43,7 +43,7 @@
  ${CMAKE_BINARY_DIR}/bin/scan-build-py
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/scan-build)
 install (PROGRAMS "bin/scan-build"
- DESTINATION bin
+ DESTINATION "${CMAKE_INSTALL_BINDIR}"
  RENAME scan-build-py
  COMPONENT scan-build-py)
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/scan-build-py)
@@ -56,7 +56,7 @@
  ${CMAKE_BINARY_DIR}/bin/
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-build-py)
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
   endif()
@@ -72,7 +72,7 @@
  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libexe

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! The CI failure is finally down to just an unrelated one (yay?). Do you 
need someone to commit on your behalf? If so, what name and email address would 
you like used for patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

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