[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:39
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.

xazax.hun wrote:
> I was wondering if there is a plan to make the framework work for 
> non-functions, like global initializers. 
I believe there may be? I remember talking to someone who mentioned this -- I 
don't know, it might have been you?

This is, really, the only reason I can see for having an overload that takes a 
separate `Stmt`. It doesn't really make sense (I think) to pass a 
`FunctionDecl` to this overload and then pass some `Stmt` that isn't the 
complete function body. (I can't think of any good scenarios where the control 
flow wouldn't escape that `Stmt`, and I don't see any good use cases anyway.) 
So I've been assuming that this overload is there for global initializers. 
Confusingly, the comment says that `D` should be a function, but notably, `D` 
is a `Decl`, not a `FunctionDecl` -- so I think the comment is wrong here.

Anyway, I'll try and get some more insights into this, but until then, I'll 
certainly keep this overload in place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151194: [clang][dataflow] Add support for return values of reference type.

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:196
+  void popCall(const CallExpr *Call, const Environment &CalleeEnv);
+  void popCall(const CXXConstructExpr *Call, const Environment &CalleeEnv);
 

xazax.hun wrote:
> I know that Obj-C is a non-goal, but it might worth a comment to support 
> `ObjCMessageExpr` just in case someone wants to work on this. 
> 
> Btw, this is one of my biggest pet peeves about the Clang AST. We should have 
> a common abstraction for all the callables, instead of having to bifurcate 
> many of the APIs. 
Agreed!

I'm a bit hesitant to add a FIXME here though, as that might make it sound as 
if there's a plan to add Objective-C support or at least imply that that's a 
goal. Also, I think Objective-C support would entail quite a bit more than 
adding the right overloads here -- so I'm not sure how helpful the comment here 
would be. I think once someone goes down the road of seriously working on 
Objective-C support, they'll quickly discover that they need a new overload 
here.

I hope this makes sense? Happy to add something in a followup patch if you feel 
we should add some Objective-C "breadcrumbs" here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151194

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

[PATCH] D151397: [3/N][RISCV] Model vxrm in C intrinsics for RVV fixed-point instruction vaadd

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD updated this revision to Diff 525463.
eopXD marked 3 inline comments as done.
eopXD added a comment.

Address comments from Craig. Now the patch includes changes to 
vaadd/vaaddu/vasub/vasubu.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151397

Files:
  clang/include/clang/Basic/riscv_vector.td
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vasubu.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vaadd.ll

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

[PATCH] D151397: [3/N][RISCV] Model vxrm in C intrinsics for RVV fixed-point instruction vaadd

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1365
   defm vaaddu : RISCVSaturatingBinaryAAX;
-  defm vaadd : RISCVSaturatingBinaryAAX;
-  defm vaadd_rm : RISCVSaturatingBinaryAAXRoundingMode;
+  defm vaadd : RISCVSaturatingBinaryAAXRoundingMode;
   defm vasubu : RISCVSaturatingBinaryAAX;

craig.topper wrote:
> craig.topper wrote:
> > Why did the intrinsic change names?
> Nevermind. I misread this.
Marking this as done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151397

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

[clang] 246626a - [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-25T07:19:12Z
New Revision: 246626a8cfd3d4f910baadeff4d5aa544b9d4550

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

LOG: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a 
`FunctionDecl`.

This is the most common use case, so it makes sense to have a specific overload 
for it.

Reviewed By: xazax.hun

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h 
b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
index b51e2cb23634d..f327011766069 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -31,6 +31,11 @@ namespace dataflow {
 /// analysis.
 class ControlFlowContext {
 public:
+  /// Builds a ControlFlowContext from a `FunctionDecl`.
+  /// `Func.hasBody()` must be true, and `Func.isTemplated()` must be false.
+  static llvm::Expected build(const FunctionDecl &Func,
+  ASTContext &C);
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.
   static llvm::Expected build(const Decl &D, Stmt &S,

diff  --git a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp 
b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
index 4556787d10a8e..c62bff33524cf 100644
--- a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -67,6 +67,16 @@ static llvm::BitVector findReachableBlocks(const CFG &Cfg) {
   return BlockReachable;
 }
 
+llvm::Expected
+ControlFlowContext::build(const FunctionDecl &Func, ASTContext &C) {
+  if (!Func.hasBody())
+return llvm::createStringError(
+std::make_error_code(std::errc::invalid_argument),
+"Cannot analyze function without a body");
+
+  return build(Func, *Func.getBody(), C);
+}
+
 llvm::Expected
 ControlFlowContext::build(const Decl &D, Stmt &S, ASTContext &C) {
   if (D.isTemplated())

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 73428ac250ad3..32612397ec024 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -210,8 +210,8 @@ DataflowAnalysisContext::getControlFlowContext(const 
FunctionDecl *F) {
   if (It != FunctionContexts.end())
 return &It->second;
 
-  if (Stmt *Body = F->getBody()) {
-auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext());
+  if (F->hasBody()) {
+auto CFCtx = ControlFlowContext::build(*F, F->getASTContext());
 // FIXME: Handle errors.
 assert(CFCtx);
 auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index ff7d27d6540cc..d5591bee12dc2 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -241,8 +241,7 @@ checkDataflow(AnalysisInputs AI,
   llvm::errc::invalid_argument, "Could not find the target function.");
 
 // Build the control flow graph for the target function.
-auto MaybeCFCtx =
-ControlFlowContext::build(*Target, *Target->getBody(), Context);
+auto MaybeCFCtx = ControlFlowContext::build(*Target, Context);
 if (!MaybeCFCtx) return MaybeCFCtx.takeError();
 auto &CFCtx = *MaybeCFCtx;
 

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 5bfb9e53778b0..84b10c87f6b19 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -67,8 +67,8 @@ runAnalysis(llvm::StringRef Code, AnalysisT 
(*MakeAnalysis)(ASTContext &)) {
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Anal

[PATCH] D150913: [Clang][BFloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:852
 ``double`` when passed to ``printf``, so the programmer must explicitly cast 
it to
 ``double`` before using it with an ``%f`` or similar specifier.
 

rjmccall wrote:
> pengfei wrote:
> > rjmccall wrote:
> > > Suggested rework:
> > > 
> > > ```
> > > Clang supports three half-precision (16-bit) floating point types: 
> > > ``__fp16``,
> > > ``_Float16`` and ``__bf16``.  These types are supported in all language
> > > modes, but not on all targets:
> > > 
> > > - ``__fp16`` is supported on every target.
> > > 
> > > - ``_Float16`` is currently supported on the following targets:
> > >   * 32-bit ARM (natively on some architecture versions)
> > >   * 64-bit ARM (AArch64) (natively on ARMv8.2a and above)
> > >   * AMDGPU (natively)
> > >   * SPIR (natively)
> > >   * X86 (if SSE2 is available; natively if AVX512-FP16 is also available)
> > > 
> > > - ``__bf16`` is currently supported on the following targets:
> > >   * 32-bit ARM
> > >   * 64-bit ARM (AArch64)
> > >   * X86 (when SSE2 is available)
> > > 
> > > (For X86, SSE2 is available on 64-bit and all recent 32-bit processors.)
> > > 
> > > ``__fp16`` and ``_Float16`` both use the binary16 format from IEEE
> > > 754-2008, which provides a 5-bit exponent and an 11-bit significand
> > > (counting the implicit leading 1).  ``__bf16`` uses the `bfloat16
> > > `_ format,
> > > which provides an 8-bit exponent and an 8-bit significand; this is the 
> > > same
> > > exponent range as `float`, just with greatly reduced precision.
> > > 
> > > ``_Float16`` and ``__bf16`` follow the usual rules for arithmetic
> > > floating-point types.  Most importantly, this means that arithmetic 
> > > operations
> > > on operands of these types are formally performed in the type and produce
> > > values of the type.  ``__fp16`` does not follow those rules: most 
> > > operations
> > > immediately promote operands of type ``__fp16`` to ``float``, and so
> > > arithmetic operations are defined to be performed in ``float`` and so 
> > > result in
> > > a value of type ``float`` (unless further promoted because of other 
> > > operands).
> > > See below for more information on the exact specifications of these types.
> > > 
> > > Only some of the supported processors for ``__fp16`` and ``__bf16`` offer
> > > native hardware support for arithmetic in their corresponding formats.
> > > The exact conditions are described in the lists above.  When compiling 
> > > for a
> > > processor without native support, Clang will perform the arithmetic in
> > > ``float``, inserting extensions and truncations as necessary.  This can be
> > > done in a way that exactly emulates the behavior of hardware support for
> > > arithmetic, but it can require many extra operations.  By default, Clang 
> > > takes
> > > advantage of the C standard's allowances for excess precision in 
> > > intermediate
> > > operands in order to eliminate intermediate truncations within statements.
> > > This is generally much faster but can generate different results from 
> > > strict
> > > operation-by-operation emulation.
> > > 
> > > The use of excess precision can be independently controlled for these two
> > > types with the ``-ffloat16-excess-precision=`` and
> > > ``-fbfloat16-excess-precision=`` options.  Valid values include:
> > > - ``none`` (meaning to perform strict operation-by-operation emulation)
> > > - ``standard`` (meaning that excess precision is permitted under the rules
> > >   described in the standard, i.e. never across explicit casts or 
> > > statements)
> > > - ``fast`` (meaning that excess precision is permitted whenever the
> > >   optimizer sees an opportunity to avoid truncations; currently this has 
> > > no
> > >   effect beyond ``standard``)
> > > 
> > > The ``_Float16`` type is an interchange floating type specified in
> > >  ISO/IEC TS 18661-3:2015 ("Floating-point extensions for C").  It will
> > > be supported on more targets as they define ABIs for it.
> > > 
> > > The ``__bf16`` type is a non-standard extension, but it generally follows
> > > the rules for arithmetic interchange floating types from ISO/IEC TS
> > > 18661-3:2015.  In previous versions of Clang, it was a storage-only type
> > > that forbade arithmetic operations.  It will be supported on more targets
> > > as they define ABIs for it.
> > > 
> > > The ``__fp16`` type was originally an ARM extension and is specified
> > > by the `ARM C Language Extensions 
> > > `_.
> > > Clang uses the ``binary16`` format from IEEE 754-2008 for ``__fp16``,
> > > not the ARM alternative format.  Operators that expect arithmetic operands
> > > immediately promote ``__fp16`` operands to ``float``.
> > > 
> > > It is recommended that portable code use ``_Float16`` instead of 
> > > ``__fp16``,
> > > as it has been d

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG246626a8cfd3: [clang][dataflow] Add a 
`ControlFlowContext::build()` overload taking a… (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -241,8 +241,7 @@
   llvm::errc::invalid_argument, "Could not find the target function.");
 
 // Build the control flow graph for the target function.
-auto MaybeCFCtx =
-ControlFlowContext::build(*Target, *Target->getBody(), Context);
+auto MaybeCFCtx = ControlFlowContext::build(*Target, Context);
 if (!MaybeCFCtx) return MaybeCFCtx.takeError();
 auto &CFCtx = *MaybeCFCtx;
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -210,8 +210,8 @@
   if (It != FunctionContexts.end())
 return &It->second;
 
-  if (Stmt *Body = F->getBody()) {
-auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext());
+  if (F->hasBody()) {
+auto CFCtx = ControlFlowContext::build(*F, F->getASTContext());
 // FIXME: Handle errors.
 assert(CFCtx);
 auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -67,6 +67,16 @@
   return BlockReachable;
 }
 
+llvm::Expected
+ControlFlowContext::build(const FunctionDecl &Func, ASTContext &C) {
+  if (!Func.hasBody())
+return llvm::createStringError(
+std::make_error_code(std::errc::invalid_argument),
+"Cannot analyze function without a body");
+
+  return build(Func, *Func.getBody(), C);
+}
+
 llvm::Expected
 ControlFlowContext::build(const Decl &D, Stmt &S, ASTContext &C) {
   if (D.isTemplated())
Index: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -31,6 +31,11 @@
 /// analysis.
 class ControlFlowContext {
 public:
+  /// Builds a ControlFlowContext from a `FunctionDecl`.
+  /// `Func.hasBody()` must be true, and `Func.isTemplated()` must be false.
+  static llvm::Expected build(const FunctionDecl &Func,
+  ASTContext &C);
+
   /// Builds a ControlFlowContext from an AST node. `D` is the function in 
which
   /// `S` resides. `D.isTemplated()` must be false.
   static llvm::Expected build(const Decl &D, Stmt &S,


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -67,8 +67,8 @@
   Stmt *Body = Func->getBody();
   assert(Body != nullptr);
 
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
+  auto CFCtx =
+  llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
   AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
   DataflowAnalysisContext DACtx(std::make_unique());
Index: clang/unittests/

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:61
+void IncludeCleanerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Suppress", "");
+}

after storing them in the class state on construction, we should provide them 
with current values here:
```
  /// Should store all options supported by this check with their
  /// current values or default values for options that haven't been overridden.
```



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:66
+  llvm::SmallVector SuppressedHeaders;
+  auto SuppressOpt = Options.getLocalOrGlobal("Suppress");
+  if (SuppressOpt)

as mentioned above, rather than doing this on demand, we should run the logic 
on construction and store in the class state



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:66
+  llvm::SmallVector SuppressedHeaders;
+  auto SuppressOpt = Options.getLocalOrGlobal("Suppress");
+  if (SuppressOpt)

kadircet wrote:
> as mentioned above, rather than doing this on demand, we should run the logic 
> on construction and store in the class state
s/Suppress/IgnoreHeader

to be consistent with option in clangd



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:81
+   Result.Nodes.getNodeAs("top")->decls()) {
+if (!SM->isInMainFile(D->getLocation()))
+  continue;

we should actually use FileLoc of the decl location here (i.e. map it back to 
spelling location) as the decl might be introduced by a macro expansion, but if 
the spelling of "primary" location belongs to the main file we should still 
analyze it (e.g. decls introduced via `TEST` macros)

also `SourceManager::isInMainFile` will follow `#line` directives, which can 
create confusion (a declaration from some other file will create a diagnostic 
in the main file), so let's use `isWrittenInMainFile` instead?



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:101
+}
+if (!Satisfied && !Providers.empty() &&
+Ref.RT == include_cleaner::RefType::Explicit)

we should also respect `IgnoreHeaders` here



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:126
+
+if (std::find(IgnoreHeaders.begin(), IgnoreHeaders.end(), I.Spelled) ==
+IgnoreHeaders.end())

sorry i guess i wasn't explicit about this one, but it should actually be a 
suffix match based regex (e.g. `foo/.*` disables analysis for all sources under 
a directory called `foo`) on the resolved path of the include (similar to what 
we do in clangd).

as headers can be spelled differently based on the translation unit and this 
option is mentioned for the whole codebase.



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:131
+
+  llvm::StringRef FileData = SM->getBufferData(SM->getMainFileID());
+  auto FileStyle = format::getStyle(

`FileData` makes it sound like this is some other data about the file rather 
than its contents. maybe just `Code`?



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:144
+HeaderIncludes.remove(Inc->Spelled, Inc->Angled);
+tooling::Replacement R = *Removal.begin();
+auto StartLoc = SM->getComposedLoc(SM->getMainFileID(), R.getOffset());

sorry I know I suggested using HeaderIncludes for removals too, but this seems 
to cause more trouble actually;

we should actually go over rest of the replacements too, HeaderIncludes will 
generate removals for all the includes that match this (spelling, quoting). 
hence we can't just apply the first removal.
but that'll imply we'll remove them multiple times (as analysis above will 
treat each of them as a separate unused include). hence we'll need some 
deduplicaiton.

it might be easier to just use line number we have in `Inc` and use 
`SourceManager:: translateFileLineCol` to drop the whole line (up until start 
of the next line).



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h:43
+  HeaderSearch *HS;
+  llvm::SmallVector getSuppressedHeaders();
+};

the convention is to rather store options in check's state on construction. see 
documentation on ClangTidyCheck::ClangTidyCheck:

```
  /// Initializes the check with \p CheckName and \p Context.
  ///
  /// Derived classes must implement the constructor with this signature or
  /// delegate it. If a check needs to read options, it can do this in the
  /// constructor using the Options.get() methods below.
```



Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst:8
+the main file of a translation unit.
+Findings correspond to https://clangd.llvm.org/design/include-cleaner.

[PATCH] D151397: [3/N][RISCV] Model vxrm in C intrinsics for RVV fixed-point instruction vaadd

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD updated this revision to Diff 525473.
eopXD added a comment.

Bump CI because previous diff failed in patch application.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151397

Files:
  clang/include/clang/Basic/riscv_vector.td
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/overloaded/vasubu.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vaadd.ll

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

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Drive-by nit: want add this to the `disableUnusableChecks()` blocklist in 
`clangd/TidyProvider.cpp`?
Since inside clangd we want to use the direct feature instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148793

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

[clang-tools-extra] a2f7352 - [clangd] Dont run raw-lexer for OOB source locations

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2023-05-25T09:38:04+02:00
New Revision: a2f7352f3e809cb1b267e769d00ea84e4ef46bf0

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

LOG: [clangd] Dont run raw-lexer for OOB source locations

We can get stale source locations from preamble, make sure we don't
access those locations without checking first.

Fixes https://github.com/clangd/clangd/issues/1636.

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index b708f9c3d3b03..4c5def3063f1e 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -96,7 +96,8 @@ bool locationInRange(SourceLocation L, CharSourceRange R,
 
 // Clang diags have a location (shown as ^) and 0 or more ranges ().
 // LSP needs a single range.
-Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
+std::optional diagnosticRange(const clang::Diagnostic &D,
+ const LangOptions &L) {
   auto &M = D.getSourceManager();
   auto PatchedRange = [&M](CharSourceRange &R) {
 R.setBegin(translatePreamblePatchLocation(R.getBegin(), M));
@@ -115,13 +116,18 @@ Range diagnosticRange(const clang::Diagnostic &D, const 
LangOptions &L) {
 if (locationInRange(Loc, R, M))
   return halfOpenToRange(M, PatchedRange(R));
   }
+  // Source locations from stale preambles might become OOB.
+  // FIXME: These diagnostics might point to wrong locations even when they're
+  // not OOB.
+  auto [FID, Offset] = M.getDecomposedLoc(Loc);
+  if (Offset > M.getBufferData(FID).size())
+return std::nullopt;
   // If the token at the location is not a comment, we use the token.
   // If we can't get the token at the location, fall back to using the location
   auto R = CharSourceRange::getCharRange(Loc);
   Token Tok;
-  if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment)) {
+  if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment))
 R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc());
-  }
   return halfOpenToRange(M, PatchedRange(R));
 }
 
@@ -697,7 +703,10 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level 
DiagLevel,
 SourceLocation PatchLoc =
 translatePreamblePatchLocation(Info.getLocation(), SM);
 D.InsideMainFile = isInsideMainFile(PatchLoc, SM);
-D.Range = diagnosticRange(Info, *LangOpts);
+if (auto DRange = diagnosticRange(Info, *LangOpts))
+  D.Range = *DRange;
+else
+  D.Severity = DiagnosticsEngine::Ignored;
 auto FID = SM.getFileID(Info.getLocation());
 if (const auto FE = SM.getFileEntryRefForID(FID)) {
   D.File = FE->getName().str();

diff  --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp 
b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
index 23ba0fc3e52fc..4f2cc3e0abe77 100644
--- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -829,6 +829,37 @@ x>)");
 auto AST = createPatchedAST(Code.code(), NewCode.code());
 EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
   }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble has the conditional but main file doesn't terminate it).
+// We shouldn't emit any diagnotiscs (and shouldn't crash).
+Annotations NewCode("");
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
+  }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble has the conditional but main file doesn't terminate it).
+// We shouldn't emit any diagnotiscs (and shouldn't crash).
+// FIXME: Patch/ignore diagnostics in such cases.
+Annotations NewCode(R"(
+i[[nt]] xyz;
+)");
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+EXPECT_THAT(
+*AST->getDiagnostics(),
+ElementsAre(Diag(NewCode.range(), "pp_unterminated_conditional")));
+  }
 }
 
 MATCHER_P2(Mark, Range, Text, "") {



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

[PATCH] D151321: [clangd] Dont run raw-lexer for OOB source locations

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa2f7352f3e80: [clangd] Dont run raw-lexer for OOB source 
locations (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151321

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -829,6 +829,37 @@
 auto AST = createPatchedAST(Code.code(), NewCode.code());
 EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
   }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble has the conditional but main file doesn't terminate it).
+// We shouldn't emit any diagnotiscs (and shouldn't crash).
+Annotations NewCode("");
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
+  }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble has the conditional but main file doesn't terminate it).
+// We shouldn't emit any diagnotiscs (and shouldn't crash).
+// FIXME: Patch/ignore diagnostics in such cases.
+Annotations NewCode(R"(
+i[[nt]] xyz;
+)");
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+EXPECT_THAT(
+*AST->getDiagnostics(),
+ElementsAre(Diag(NewCode.range(), "pp_unterminated_conditional")));
+  }
 }
 
 MATCHER_P2(Mark, Range, Text, "") {
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -96,7 +96,8 @@
 
 // Clang diags have a location (shown as ^) and 0 or more ranges ().
 // LSP needs a single range.
-Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
+std::optional diagnosticRange(const clang::Diagnostic &D,
+ const LangOptions &L) {
   auto &M = D.getSourceManager();
   auto PatchedRange = [&M](CharSourceRange &R) {
 R.setBegin(translatePreamblePatchLocation(R.getBegin(), M));
@@ -115,13 +116,18 @@
 if (locationInRange(Loc, R, M))
   return halfOpenToRange(M, PatchedRange(R));
   }
+  // Source locations from stale preambles might become OOB.
+  // FIXME: These diagnostics might point to wrong locations even when they're
+  // not OOB.
+  auto [FID, Offset] = M.getDecomposedLoc(Loc);
+  if (Offset > M.getBufferData(FID).size())
+return std::nullopt;
   // If the token at the location is not a comment, we use the token.
   // If we can't get the token at the location, fall back to using the location
   auto R = CharSourceRange::getCharRange(Loc);
   Token Tok;
-  if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment)) {
+  if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment))
 R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc());
-  }
   return halfOpenToRange(M, PatchedRange(R));
 }
 
@@ -697,7 +703,10 @@
 SourceLocation PatchLoc =
 translatePreamblePatchLocation(Info.getLocation(), SM);
 D.InsideMainFile = isInsideMainFile(PatchLoc, SM);
-D.Range = diagnosticRange(Info, *LangOpts);
+if (auto DRange = diagnosticRange(Info, *LangOpts))
+  D.Range = *DRange;
+else
+  D.Severity = DiagnosticsEngine::Ignored;
 auto FID = SM.getFileID(Info.getLocation());
 if (const auto FE = SM.getFileEntryRefForID(FID)) {
   D.File = FE->getName().str();


Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -829,6 +829,37 @@
 auto AST = createPatchedAST(Code.code(), NewCode.code());
 EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
   }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble has the conditional but main file doesn't terminate it).
+// We shouldn't emit any diagnotiscs (and shouldn't crash).
+Annotations NewCode("");
+auto AST = createPatchedAST(Code.code(), NewCode.code());
+EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
+  }
+  {
+Annotations Code(R"(
+#ifndef FOO
+#define FOO
+void foo();
+#endif)");
+// This code will emit a diagnostic for unterminated #ifndef (as stale
+// preamble ha

[PATCH] D151308: -fsanitize=function: fix alignment fault on Arm targets.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

> `-fsanitize=kcfi` only supports aarch64 and x86-64 now. riscv64 is on the 
> plan.
>
>   % fclang -fsanitize=kcfi --traget=armv7-linux-gnueabi -c a.c
>   clang: error: unsupported option '--traget=armv7-linux-gnueabi'

Sorry to contradict, but that error message only indicates that you misspelled 
`--target`! With Peter's test source file, these two commands generate 
different object files, and as Peter says, the `-fsanitize=kcfi` one includes a 
load from offset −4 relative to a potentially odd-valued function pointer:

  clang -O1 --target=armv7-linux-gnueabi -c a.c   # generates a 
bare BX r0
  clang -O1 -fsanitize=kcfi --target=armv7-linux-gnueabi -c a.c   # generates 
the code shown in Peter's example above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151308

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

[clang] 20d6dee - -fsanitize=function: fix alignment fault on Arm targets.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2023-05-25T09:22:45+01:00
New Revision: 20d6dee40d507d467d3312d5e7dfdf088f106d31

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

LOG: -fsanitize=function: fix alignment fault on Arm targets.

Function pointers are checked by loading a prefix structure from just
before the function's entry point. However, on Arm, the function
pointer is not always exactly equal to the address of the entry point,
because Thumb function pointers have the low bit set to tell the BX
instruction to enter them in Thumb state. So the generated code loads
from an odd address and suffers an alignment fault.

Fixed by clearing the low bit of the function pointer before
subtracting 8.

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

Added: 


Modified: 
clang/lib/CodeGen/CGExpr.cpp
clang/test/CodeGen/ubsan-function.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 2c219d6e8411..c074732df2a7 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5364,8 +5364,30 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, 
const CGCallee &OrigCallee
 
   llvm::Value *CalleePtr = Callee.getFunctionPointer();
 
+  // On 32-bit Arm, the low bit of a function pointer indicates whether
+  // it's using the Arm or Thumb instruction set. The actual first
+  // instruction lives at the same address either way, so we must clear
+  // that low bit before using the function address to find the prefix
+  // structure.
+  //
+  // This applies to both Arm and Thumb target triples, because
+  // either one could be used in an interworking context where it
+  // might be passed function pointers of both types.
+  llvm::Value *AlignedCalleePtr;
+  if (CGM.getTriple().isARM() || CGM.getTriple().isThumb()) {
+llvm::Value *CalleeAddress =
+Builder.CreatePtrToInt(CalleePtr, IntPtrTy);
+llvm::Value *Mask = llvm::ConstantInt::get(IntPtrTy, ~1);
+llvm::Value *AlignedCalleeAddress =
+Builder.CreateAnd(CalleeAddress, Mask);
+AlignedCalleePtr =
+Builder.CreateIntToPtr(AlignedCalleeAddress, CalleePtr->getType());
+  } else {
+AlignedCalleePtr = CalleePtr;
+  }
+
   llvm::Value *CalleePrefixStruct = Builder.CreateBitCast(
-  CalleePtr, llvm::PointerType::getUnqual(PrefixStructTy));
+  AlignedCalleePtr, llvm::PointerType::getUnqual(PrefixStructTy));
   llvm::Value *CalleeSigPtr =
   Builder.CreateConstGEP2_32(PrefixStructTy, CalleePrefixStruct, -1, 
0);
   llvm::Value *CalleeSig =

diff  --git a/clang/test/CodeGen/ubsan-function.cpp 
b/clang/test/CodeGen/ubsan-function.cpp
index fc9f60f5b205..ba55ee021cc9 100644
--- a/clang/test/CodeGen/ubsan-function.cpp
+++ b/clang/test/CodeGen/ubsan-function.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,ARM,32
 
 // CHECK: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
 void fun() {}
 
 // CHECK-LABEL: define{{.*}} void @_Z6callerPFvvE(ptr noundef %f)
+// ARM:   ptrtoint ptr {{.*}} to i32, !nosanitize !5
+// ARM:   and i32 {{.*}}, -2, !nosanitize !5
+// ARM:   inttoptr i32 {{.*}} to ptr, !nosanitize !5
 // CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 0, !nosanitize
 // CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
 // CHECK: icmp eq i32 {{.*}}, -1056584962, !nosanitize
@@ -16,7 +20,8 @@ void fun() {}
 // CHECK: icmp eq i32 {{.*}}, -1522505972, !nosanitize
 // CHECK: br i1 {{.*}}, label %[[LABEL3:.*]], label %[[LABEL2:[^,]*]], 
{{.*}}!nosanitize
 // CHECK: [[LABEL2]]:
-// CHECK: call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], 
i64 %[[#]]) #[[#]], !nosanitize
+// 64:call void @__ubsan_handle_function_type_misma

[PATCH] D151308: -fsanitize=function: fix alignment fault on Arm targets.

[Simon Tatham via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20d6dee40d50: -fsanitize=function: fix alignment fault on 
Arm targets. (authored by simon_tatham).

Changed prior to commit:
  https://reviews.llvm.org/D151308?vs=525111&id=525479#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151308

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/ubsan-function.cpp


Index: clang/test/CodeGen/ubsan-function.cpp
===
--- clang/test/CodeGen/ubsan-function.cpp
+++ clang/test/CodeGen/ubsan-function.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,64
+// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s 
-fsanitize=function -fno-sanitize-recover=all | FileCheck %s 
--check-prefixes=CHECK,ARM,32
 
 // CHECK: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
 void fun() {}
 
 // CHECK-LABEL: define{{.*}} void @_Z6callerPFvvE(ptr noundef %f)
+// ARM:   ptrtoint ptr {{.*}} to i32, !nosanitize !5
+// ARM:   and i32 {{.*}}, -2, !nosanitize !5
+// ARM:   inttoptr i32 {{.*}} to ptr, !nosanitize !5
 // CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 0, !nosanitize
 // CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
 // CHECK: icmp eq i32 {{.*}}, -1056584962, !nosanitize
@@ -16,7 +20,8 @@
 // CHECK: icmp eq i32 {{.*}}, -1522505972, !nosanitize
 // CHECK: br i1 {{.*}}, label %[[LABEL3:.*]], label %[[LABEL2:[^,]*]], 
{{.*}}!nosanitize
 // CHECK: [[LABEL2]]:
-// CHECK: call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], 
i64 %[[#]]) #[[#]], !nosanitize
+// 64:call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], 
i64 %[[#]]) #[[#]], !nosanitize
+// 32:call void @__ubsan_handle_function_type_mismatch_abort(ptr @[[#]], 
i32 %[[#]]) #[[#]], !nosanitize
 // CHECK-NEXT: unreachable, !nosanitize
 // CHECK-EMPTY:
 // CHECK-NEXT: [[LABEL3]]:
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5364,8 +5364,30 @@
 
   llvm::Value *CalleePtr = Callee.getFunctionPointer();
 
+  // On 32-bit Arm, the low bit of a function pointer indicates whether
+  // it's using the Arm or Thumb instruction set. The actual first
+  // instruction lives at the same address either way, so we must clear
+  // that low bit before using the function address to find the prefix
+  // structure.
+  //
+  // This applies to both Arm and Thumb target triples, because
+  // either one could be used in an interworking context where it
+  // might be passed function pointers of both types.
+  llvm::Value *AlignedCalleePtr;
+  if (CGM.getTriple().isARM() || CGM.getTriple().isThumb()) {
+llvm::Value *CalleeAddress =
+Builder.CreatePtrToInt(CalleePtr, IntPtrTy);
+llvm::Value *Mask = llvm::ConstantInt::get(IntPtrTy, ~1);
+llvm::Value *AlignedCalleeAddress =
+Builder.CreateAnd(CalleeAddress, Mask);
+AlignedCalleePtr =
+Builder.CreateIntToPtr(AlignedCalleeAddress, CalleePtr->getType());
+  } else {
+AlignedCalleePtr = CalleePtr;
+  }
+
   llvm::Value *CalleePrefixStruct = Builder.CreateBitCast(
-  CalleePtr, llvm::PointerType::getUnqual(PrefixStructTy));
+  AlignedCalleePtr, llvm::PointerType::getUnqual(PrefixStructTy));
   llvm::Value *CalleeSigPtr =
   Builder.CreateConstGEP2_32(PrefixStructTy, CalleePrefixStruct, -1, 
0);
   llvm::Value *CalleeSig =


Index: clang/test/CodeGen/ubsan-function.cpp
===
--- clang/test/CodeGen/ubsan-function.cpp
+++ clang/test/CodeGen/ubsan-function.cpp
@@ -1,11 +1,15 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s
-// RUN:

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

[Sam McCall via Phabricator via cfe-commits]

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

> 150ms is more than noticeable

Fair enough. I also see we *are* avoiding the quadratic algorithm in other 
places in inlay hints, and that we can't easily reuse the getHintRange() 
function in any case without switching to handling tokens instead of strings. 
So overall I think the current version is fine. Thanks for digging into this!




Comment at: clang-tools-extra/clangd/Config.h:150
 bool Designators = true;
+bool BlockEnd = true;
 // Limit the length of type names in inlay hints. (0 means no limit)

this needs to be false initially (a month or so?) so we have a chance to try it 
out in practice, ensure it's not too spammy/has crashes etc, then we can flip 
on by default



Comment at: clang-tools-extra/clangd/InlayHints.cpp:802
+Position HintStart = sourceLocToPosition(SM, RBraceLoc);
+Position HintEnd = {HintStart.line,
+HintStart.character +

I'd prefer `sourceLocToPosition(SM, 
RBraceLoc.getLocationWithOffset(HintRangeText.size()))` which would avoids such 
low-level conversion between coordinate systems, and seems to perform just fine 
(we're going to hit SourceManager's caches).

Will leave this up to you though.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:578
+constexpr unsigned HintMinLineLimit = 2;
+constexpr unsigned HintMaxLengthLimit = 50;
+

daiyousei-qz wrote:
> nridge wrote:
> > daiyousei-qz wrote:
> > > sammccall wrote:
> > > > We actually already have a configurable inlay hint length limit.
> > > > It has the wrong name (`TypeNameLimit`) , but I think we should use it 
> > > > anyway (and maybe try to fix the name later).
> > > > 
> > > > (I'm not sure making this configurable was a great idea, but given that 
> > > > it exists, people will expect it to effectively limit the length of 
> > > > hints)
> > > `TypeNameLimit` was introduced to limit the length of "DeducedTypes" 
> > > only. I don't think using that number for all hints is a good idea 
> > > because different hints appear in different contexts. For deduced type 
> > > hint, it is displayed in the middle of the actual code, so it must be 
> > > concise to not overwhelm the actual code. However, this hint is usually 
> > > displayed at the end of an almost empty line. A much larger number of 
> > > characters should be allowed.
> > > 
> > > (I'm not arguing here, but IMO such options are definitely helpful. Not 
> > > everybody would fork llvm-project and build their own version of clangd 
> > > binary. Without options to configure length of hints, people would 
> > > disable "DeducedTypes" entirely if they cannot tolerate the default 
> > > length limit, while this feature is definitely a cool thing to turn on. 
> > > Personally, I actually think clangd has too little option compared to say 
> > > rust-analyzer. But it's just my understanding)
> > > For deduced type hint, it is displayed in the middle of the actual code, 
> > > so it must be concise to not overwhelm the actual code. However, this 
> > > hint is usually displayed at the end of an almost empty line. A much 
> > > larger number of characters should be allowed.
> > 
> > Another consideration besides the location of the hint that is worth 
> > keeping in mind, is what type of content is being printed.
> > 
> > Type names in C++ are composable in ways that identifiers are not (e.g. 
> > `vector, allocator, 
> > allocator>` etc.), such that there is a need to limit 
> > type hints that doesn't really apply to say, parameter hints.
> > 
> > So, a question I have here is: can template argument lists appear in an 
> > end-definition-comment hint?
> > 
> > For example, for:
> > 
> > ```
> > template 
> > struct S {
> >   void foo();
> > };
> > 
> > template 
> > void S::foo() {
> > }  // <--- HERE
> > ```
> > 
> > is the hint at the indicated location `S::foo()`, or `S::foo()`? In the 
> > latter case, we can imagine the hint getting long due to the type 
> > parameters, and we may want to consider either limiting its length, or 
> > tweaking the code to not print the type parameters.
> Yes, currently this hint is only shown if the total length of hint label is 
> less than 60 characters. I believe what we display should be exactly what is 
> used to define the symbol. In your example,
> 
> ```
> template 
> struct S {
>   void foo();
> };
> 
> template 
> void S::foo() {
> }  // S::foo
> ```
> Basically, "[A]" and "[B]" is the same text (OFC excluding whitespaces).
> ```
> void [A]() {
> } // [B]
> ```
> The hint won't be displayed if "[B]" is more than 60 characters.
OK, I'm sold on wanting a higher limit here, and a hard-coded 60 as a limit on 
the hint seems simple and reasonable for now. We can iterate on this.

(There's another case with arbitrary types being printed: `operator 
vector` or so)

[clang] 6441358 - [clang][dataflow] Add support for return values of reference type.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-25T08:38:33Z
New Revision: 64413584dacba1fccffa345f69043b3509ee1745

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

LOG: [clang][dataflow] Add support for return values of reference type.

This patch changes the way `Environment::ReturnLoc` is set: Whereas previously 
it was set by the caller, it is now set by the callee (obviously, as we 
otherwise would not be able to return references).

The patch also introduces `Environment::ReturnVal`, which is used for 
non-reference-type return values. This allows these to be handled with the 
correct value category semantics; see also https://discourse.llvm.org/t/70086, 
which describes the ongoing migration to strict value category semantics.

Depends On D150776

Reviewed By: ymandel, xazax.hun

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

Added: 


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

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 6f2f7f4004f84..a31f5ec116425 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -192,7 +192,8 @@ class Environment {
 
   /// Moves gathered information back into `this` from a `CalleeEnv` created 
via
   /// `pushCall`.
-  void popCall(const Environment &CalleeEnv);
+  void popCall(const CallExpr *Call, const Environment &CalleeEnv);
+  void popCall(const CXXConstructExpr *Call, const Environment &CalleeEnv);
 
   /// Returns true if and only if the environment is equivalent to `Other`, i.e
   /// the two environments:
@@ -323,8 +324,51 @@ class Environment {
   /// in the environment.
   StorageLocation *getThisPointeeStorageLocation() const;
 
-  /// Returns the storage location of the return value or null, if unset.
-  StorageLocation *getReturnStorageLocation() const;
+  /// Returns the return value of the current function. This can be null if:
+  /// - The function has a void return type
+  /// - No return value could be determined for the function, for example
+  ///   because it calls a function without a body.
+  ///
+  /// Requirements:
+  ///  The current function must have a non-reference return type.
+  Value *getReturnValue() const {
+assert(getCurrentFunc() != nullptr &&
+   !getCurrentFunc()->getReturnType()->isReferenceType());
+return ReturnVal;
+  }
+
+  /// Returns the storage location for the reference returned by the current
+  /// function. This can be null if function doesn't return a single consistent
+  /// reference.
+  ///
+  /// Requirements:
+  ///  The current function must have a reference return type.
+  StorageLocation *getReturnStorageLocation() const {
+assert(getCurrentFunc() != nullptr &&
+   getCurrentFunc()->getReturnType()->isReferenceType());
+return ReturnLoc;
+  }
+
+  /// Sets the return value of the current function.
+  ///
+  /// Requirements:
+  ///  The current function must have a non-reference return type.
+  void setReturnValue(Value *Val) {
+assert(getCurrentFunc() != nullptr &&
+   !getCurrentFunc()->getReturnType()->isReferenceType());
+ReturnVal = Val;
+  }
+
+  /// Sets the storage location for the reference returned by the current
+  /// function.
+  ///
+  /// Requirements:
+  ///  The current function must have a reference return type.
+  void setReturnStorageLocation(StorageLocation *Loc) {
+assert(getCurrentFunc() != nullptr &&
+   getCurrentFunc()->getReturnType()->isReferenceType());
+ReturnLoc = Loc;
+  }
 
   /// Returns a pointer value that represents a null pointer. Calls with
   /// `PointeeType` that are canonically equivalent will return the same 
result.
@@ -472,6 +516,12 @@ class Environment {
   /// returns null.
   const DeclContext *getDeclCtx() const { return CallStack.back(); }
 
+  /// Returns the function currently being analyzed, or null if the code being
+  /// analyzed isn't part of a function.
+  const FunctionDecl *getCurrentFunc() const {
+return dyn_cast(getDeclCtx());
+  }
+
   /// Returns whether this `Environment` can be extended to analyze the given
   /// `Callee` (i.e. if `pushCall` can be used), with recursion disallowed and 
a
   /// given `MaxDepth`.
@@ -515,16 +565,18 @@ class Environment {
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
-
-  // FIXME: move the fields `CallStack`, `ReturnLoc` and `ThisPointeeLoc` into 
a
-  // 

[PATCH] D151194: [clang][dataflow] Add support for return values of reference type.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG64413584dacb: [clang][dataflow] Add support for return 
values of reference type. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151194

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  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
@@ -32,7 +32,9 @@
 using namespace clang;
 using namespace dataflow;
 using namespace test;
+using ::testing::Eq;
 using ::testing::IsNull;
+using ::testing::Ne;
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
@@ -4239,13 +4241,33 @@
   {BuiltinOptions{/*.ContextSensitiveOpts=*/std::nullopt}});
 }
 
+TEST(TransferTest, ContextSensitiveReturnReference) {
+  std::string Code = R"(
+class S {};
+S& target(bool b, S &s) {
+  return s;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *SDecl = findValueDecl(ASTCtx, "s");
+ASSERT_THAT(SDecl, NotNull());
+
+auto *SLoc = Env.getStorageLocation(*SDecl);
+ASSERT_THAT(SLoc, NotNull());
+
+ASSERT_THAT(Env.getReturnStorageLocation(), Eq(SLoc));
+  },
+  {BuiltinOptions{ContextSensitiveOptions{}}});
+}
+
 // This test is a regression test, based on a real crash.
-TEST(TransferTest, ContextSensitiveReturnReferenceFromNonReferenceLvalue) {
-  // This code exercises an unusual code path. If we return an lvalue directly,
-  // the code will catch that it's an l-value based on the `Value`'s kind. If we
-  // pass through a dummy function, the framework won't populate a value at
-  // all. In contrast, this code results in a (fresh) value, but it is not
-  // `ReferenceValue`. This test verifies that we catch this case as well.
+TEST(TransferTest, ContextSensitiveReturnReferenceWithConditionalOperator) {
   std::string Code = R"(
 class S {};
 S& target(bool b, S &s) {
@@ -4260,10 +4282,72 @@
 ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
 const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
+const ValueDecl *SDecl = findValueDecl(ASTCtx, "s");
+ASSERT_THAT(SDecl, NotNull());
+
+auto *SLoc = Env.getStorageLocation(*SDecl);
+ASSERT_THAT(SLoc, NotNull());
+EXPECT_THAT(Env.getValue(*SLoc), NotNull());
+
 auto *Loc = Env.getReturnStorageLocation();
 ASSERT_THAT(Loc, NotNull());
+EXPECT_THAT(Env.getValue(*Loc), NotNull());
+
+// TODO: We would really like to make this stronger assertion, but that
+// doesn't work because we don't propagate values correctly through
+// the conditional operator yet.
+// ASSERT_THAT(Loc, Eq(SLoc));
+  },
+  {BuiltinOptions{ContextSensitiveOptions{}}});
+}
+
+TEST(TransferTest, ContextSensitiveReturnOneOfTwoReferences) {
+  std::string Code = R"(
+class S {};
+S &callee(bool b, S &s1_parm, S &s2_parm) {
+  if (b)
+return s1_parm;
+  else
+return s2_parm;
+}
+void target(bool b) {
+  S s1;
+  S s2;
+  S &return_s1 = s1;
+  S &return_s2 = s2;
+  S &return_dont_know = callee(b, s1, s2);
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
-EXPECT_THAT(Env.getValue(*Loc), IsNull());
+const ValueDecl *S1 = findValueDecl(ASTCtx, "s1");
+ASSERT_THAT(S1, NotNull());
+const ValueDecl *S2 = findValueDecl(ASTCtx, "s2");
+ASSERT_THAT(S2, NotNull());
+const ValueDecl *ReturnS1 = findValueDecl(ASTCtx, "return_s1");
+ASSERT_THAT(ReturnS1, NotNull());
+const ValueDecl *ReturnS2 = findValueDecl(ASTCtx, "return_s2");
+ASSERT_THAT(ReturnS2, NotNull());
+const ValueDecl *ReturnDontKnow =
+findValueDecl(ASTCtx, "return_dont_know");
+ASSERT_THAT(ReturnDontKnow, NotNull());
+
+StorageLocation *S1Loc = Env.getStorageLocation(*S1);
+StorageLocation *S2Loc = Env.getStorageLocation(*S2);
+
+EXPECT_THAT(Env.getStorageLocation(*ReturnS1), Eq(S1Loc));
+EXPECT_THAT(Env.getStorageLocation(*ReturnS2), Eq(S2Loc));
+
+// In the case where we don't have a consistent sto

[clang] 2e6325c - Revert "[-Wunsafe-buffer-usage] Group variables associated by pointer assignments"

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2023-05-25T02:10:32-07:00
New Revision: 2e6325c71feb5ab45ef3dbf4362db38602288a4e

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

LOG: Revert "[-Wunsafe-buffer-usage] Group variables associated by pointer 
assignments"

This reverts commit ee6b08e99375fc48d1e5848704a66c2e8e57eb3b.

One of the added tests warn-unsafe-buffer-usage-multi-decl-warnings.cpp does
not seem to be deterministic, and seems to be especially problematic on Windows.

Failures of this one test on llvm-clang-x86_64-sie-win:
- https://lab.llvm.org/buildbot/#/builders/216/builds/21758
- https://lab.llvm.org/buildbot/#/builders/216/builds/21761
- https://lab.llvm.org/buildbot/#/builders/216/builds/21762
- https://lab.llvm.org/buildbot/#/builders/216/builds/21765
- https://lab.llvm.org/buildbot/#/builders/216/builds/21770
- https://lab.llvm.org/buildbot/#/builders/216/builds/21771
- https://lab.llvm.org/buildbot/#/builders/216/builds/21773
- https://lab.llvm.org/buildbot/#/builders/216/builds/21776
- https://lab.llvm.org/buildbot/#/builders/216/builds/21777
- https://lab.llvm.org/buildbot/#/builders/216/builds/21778
- https://lab.llvm.org/buildbot/#/builders/216/builds/21779

Other random bot failures:
- https://lab.llvm.org/buildbot/#/builders/65/builds/9821
- https://lab.llvm.org/buildbot/#/builders/65/builds/9822
- https://lab.llvm.org/buildbot/#/builders/65/builds/9824
- https://lab.llvm.org/buildbot/#/builders/119/builds/13440
- https://lab.llvm.org/buildbot/#/builders/119/builds/13442
- https://lab.llvm.org/buildbot/#/builders/119/builds/13444
- https://lab.llvm.org/buildbot/#/builders/119/builds/13445
- https://lab.llvm.org/buildbot/#/builders/60/builds/12156
- https://lab.llvm.org/buildbot/#/builders/60/builds/12157
- https://lab.llvm.org/buildbot/#/builders/60/builds/12160

Added: 


Modified: 
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp

Removed: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-fixits-test.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp



diff  --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 617bc7c77c565..10635e8f3a29f 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -19,8 +19,6 @@
 
 namespace clang {
 
-using DefMapTy = llvm::DenseMap>;
-
 /// The interface that lets the caller handle unsafe buffer usage analysis
 /// results by overriding this class's handle... methods.
 class UnsafeBufferUsageHandler {
@@ -36,12 +34,9 @@ class UnsafeBufferUsageHandler {
   virtual void handleUnsafeOperation(const Stmt *Operation,
  bool IsRelatedToDecl) = 0;
 
-  /// Invoked when a fix is suggested against a variable. This function groups
-  /// all variables that must be fixed together (i.e their types must be 
changed to the
-  /// same target type to prevent type mismatches) into a single fixit.
-  virtual void handleUnsafeVariableGroup(const VarDecl *Variable,
- const DefMapTy &VarGrpMap,
- FixItList &&Fixes) = 0;
+  /// Invoked when a fix is suggested against a variable.
+  virtual void handleFixableVariable(const VarDecl *Variable,
+ FixItList &&List) = 0;
 
   /// Returns a reference to the `Preprocessor`:
   virtual bool isSafeBufferOptOut(const SourceLocation &Loc) const = 0;

diff  --git 
a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 57d9dcc5bdcb7..a112b6d105025 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -36,7 +36,6 @@ FIXABLE_GADGET(PointerDereference)
 FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified 
Pointer Context
 FIXABLE_GADGET(UPCStandalonePointer)
 FIXABLE_GADGET(UPCPreIncrement)// '++Ptr' in an Unspecified 
Pointer Context
-FIXABLE_GADGET(PointerAssignment)
 
 #undef FIXABLE_GADGET
 #undef WARNING_GADGET

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a777d43f1468f..f203ac6c2a84e 100644
--- a/clang/include/clang/Bas

[PATCH] D145739: [-Wunsafe-buffer-usage] Group variables associated by pointer assignments

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

@t-rasmud I am sorry, but I had to revert your change, it is randomly failing 
on the Windows bots causing a lot of instability. See the revert commit message 
for examples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145739

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

[PATCH] D150253: [RISCV] Add Zvfhmin extension.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan updated this revision to Diff 525494.
jacquesguan added a comment.

Split into 2 revisions and address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
  clang/test/Sema/riscv-vector-float16-check.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td

Index: llvm/lib/Target/RISCV/RISCVFeatures.td
===
--- llvm/lib/Target/RISCV/RISCVFeatures.td
+++ llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -472,6 +472,11 @@
"'Zvfh' (Vector Half-Precision Floating-Point)",
[FeatureStdExtZve32f, FeatureStdExtZfhmin]>;
 
+def FeatureStdExtZvfhmin
+: SubtargetFeature<"experimental-zvfhmin", "HasStdExtZvfhmin", "true",
+   "'Zvfhmin' (Vector Half-Precision Floating-Point Minimal)",
+   [FeatureStdExtZve32f]>;
+
 def HasVInstructionsF16 : Predicate<"Subtarget->hasVInstructionsF16()">;
 
 def HasStdExtZfhOrZvfh
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -152,6 +152,7 @@
 {"zvfbfmin", RISCVExtensionVersion{0, 6}},
 {"zvfbfwma", RISCVExtensionVersion{0, 6}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
+{"zvfhmin", RISCVExtensionVersion{0, 1}},
 {"ztso", RISCVExtensionVersion{0, 1}},
 
 // vector crypto
@@ -943,6 +944,7 @@
 static const char *ImpliedExtsZvfbfmin[] = {"zve32f"};
 static const char *ImpliedExtsZvfbfwma[] = {"zve32f"};
 static const char *ImpliedExtsZvfh[] = {"zve32f", "zfhmin"};
+static const char *ImpliedExtsZvfhmin[] = {"zve32f"};
 static const char *ImpliedExtsZvkn[] = {"zvbb", "zvbc", "zvkned", "zvknhb",
 "zvkt"};
 static const char *ImpliedExtsZvkng[] = {"zvkg", "zvkn"};
@@ -1004,6 +1006,7 @@
 {{"zvfbfmin"}, {ImpliedExtsZvfbfmin}},
 {{"zvfbfwma"}, {ImpliedExtsZvfbfwma}},
 {{"zvfh"}, {ImpliedExtsZvfh}},
+{{"zvfhmin"}, {ImpliedExtsZvfhmin}},
 {{"zvkn"}, {ImpliedExtsZvkn}},
 {{"zvkng"}, {ImpliedExtsZvkng}},
 {{"zvknhb"}, {ImpliedExtsZvknhb}},
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -645,6 +645,7 @@
   RVVRequire RequireExt = StringSwitch(RequiredFeature)
   .Case("RV64", RVV_REQ_RV64)
   .Case("FullMultiply", RVV_REQ_FullMultiply)
+  .Case("ZvfhminOrZvfh", RVV_REQ_ZvfhminOrZvfh)
   .Case("Xsfvcp", RVV_REQ_Xsfvcp)
   .Default(RVV_REQ_None);
   assert(RequireExt != RVV_REQ_None && "Unrecognized required feature?");
Index: clang/test/Sema/riscv-vector-float16-check.c
===
--- clang/test/Sema/riscv-vector-float16-check.c
+++ clang/test/Sema/riscv-vector-float16-check.c
@@ -4,5 +4,5 @@
 // REQUIRES: riscv-registered-target
 #include 
 
-vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 } /* expected-warning {{non-void function does not return a value}}*/
Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
@@ -0,0 +1,27 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v \
+// RUN:   -target-feature +experimental-zvfhmin -disable-O0-optnone  \
+// RUN:   -emit-llvm %s -o - | opt -S -passes=mem2reg | \
+// RUN:   FileCheck --check-prefix=CHECK-ZVFHMIN %s
+
+#include 
+
+// CHECK-ZVFHMIN-LABEL: @test_vfncvt_f_f_w_f16m1(
+// CHECK-ZVFHMIN-NEXT:  entry:
+// CHECK-ZVFHMIN-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfncvt.f.f.w.nxv4f16.nxv4f32.i64( poison,  [[SRC:%.*]], i64 [[VL:%.*]])
+// CHECK-ZVFHMIN-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vfncvt_f_f_w_f16m1(vfloat32m2_t src, size_t vl) {
+  return __riscv_vfncvt_f(src, vl);
+}
+
+
+// CHECK-ZVFHMIN-LABEL: @test_vfwcvt_f_f_v_f1

[PATCH] D151201: [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 525501.
mboehme added a comment.

Add check that the value of the expression is true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151201

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
@@ -5056,6 +5056,26 @@
   });
 }
 
+// Repro for a crash that used to occur with chained short-circuiting logical
+// operators.
+TEST(TransferTest, ChainedLogicalOps) {
+  std::string Code = R"(
+bool target() {
+  bool b = true || false || false || false;
+  // [[p]]
+  return b;
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+auto &B = getValueForDecl(ASTCtx, Env, "b");
+EXPECT_TRUE(Env.flowConditionImplies(B));
+  });
+}
+
 // Repro for a crash that used to occur when we call a `noreturn` function
 // within one of the operands of a `&&` or `||` operator.
 TEST(TransferTest, NoReturnFunctionInsideShortCircuitedBooleanOp) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -194,24 +194,13 @@
   auto &Loc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, Loc);
 
-  BoolValue *LHSVal = getLogicOperatorSubExprValue(*LHS);
-  // If the LHS was not reachable, this BinaryOperator would also not be
-  // reachable, and we would never get here.
-  assert(LHSVal != nullptr);
-  BoolValue *RHSVal = getLogicOperatorSubExprValue(*RHS);
-  if (RHSVal == nullptr) {
-// If the RHS isn't reachable and we evaluate this BinaryOperator,
-// then the value of the LHS must have triggered the short-circuit
-// logic. This implies that the value of the entire expression must be
-// equal to the value of the LHS.
-Env.setValue(Loc, *LHSVal);
-break;
-  }
+  BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
+  BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);
 
   if (S->getOpcode() == BO_LAnd)
-Env.setValue(Loc, Env.makeAnd(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
   else
-Env.setValue(Loc, Env.makeOr(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
   break;
 }
 case BO_NE:
@@ -806,35 +795,29 @@
   }
 
 private:
-  /// If `SubExpr` is reachable, returns a non-null pointer to the value for
-  /// `SubExpr`. If `SubExpr` is not reachable, returns nullptr.
-  BoolValue *getLogicOperatorSubExprValue(const Expr &SubExpr) {
+  /// Returns the value for the sub-expression `SubExpr` of a logic operator.
+  BoolValue &getLogicOperatorSubExprValue(const Expr &SubExpr) {
 // `SubExpr` and its parent logic operator might be part of different basic
 // blocks. We try to access the value that is assigned to `SubExpr` in the
 // corresponding environment.
-const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr);
-if (!SubExprEnv)
-  return nullptr;
-
-if (auto *Val =
-dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
-  return Val;
-
-if (Env.getValueStrict(SubExpr) == nullptr) {
-  // Sub-expressions that are logic operators are not added in basic blocks
-  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-  // operator, it may not have been evaluated and assigned a value yet. In
-  // that case, we need to first visit `SubExpr` and then try to get the
-  // value that gets assigned to it.
+if (const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr))
+  if (auto *Val =
+  dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
+return *Val;
+
+// The sub-expression may lie within a basic block that isn't reachable,
+// even if we need it to evaluate the current (reachable) expression
+// (see https://discourse.llvm.org/t/70775). In this case, visit `SubExpr`
+// within the current environment and then try to get the value that gets
+// assigned to it.
+if (Env.getValueStrict(SubExpr) == nullptr)
   Visit(&SubExpr);
-}
-
 if (auto *Val = dyn_cast_or_null(Env.getValueStrict(SubExpr)))
-  return Val;
+  return *Val;
 
 // If the value of `SubExpr` is still unknown, we create a fresh symbolic
 // boolean value for it.
-return &Env.makeAtomicBoolValue();
+retu

[PATCH] D151201: [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:5064
+bool target() {
+  return true || false || false || false;
+}

xazax.hun wrote:
> Should we also test that the value of the expression is `true` in the 
> analysis state?
Good point -- done.

I've also verified that this test continues to trigger the assert-fail if the 
fix is not present.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151201

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

[PATCH] D151344: Reland "[CMake] Bumps minimum version to 3.20.0.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

This looks right to me. (I'm out of office at the moment, but this looks like 
what I tested in https://github.com/llvm/llvm-project/issues/62719 so it should 
work.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151344

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

[PATCH] D150902: [ARM][Driver] Warn if -mhard-float is incompatible

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:151
+ const std::vector &Features) {
+  if (llvm::find(Features, "-fpregs") == Features.end())
+return;

This whole patch hinges on the unspoken assumption that this is a correct way 
to test for the absence of FP registers, i.e. that `"-fpregs"` will //always// 
appear in `Features` if there are no FP registers.

Could you add a comment stating that assumption explicitly and explaining why 
it's valid? In particular, why you're confident that the absence of FP 
registers might not sometimes be signalled by the lack of either `"-fpregs"` or 
`"+fpregs"` in that list.

(In a quick look at the code I //think// you're right, but I'm not 100% 
confident, and if you show your working then people will be able to work out 
what went wrong later if things change.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150902

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

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Unfortunately I'm seeing a number of Rust optimization regressions with this 
change. I'll try to reduce those to some PhaseOrdering tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145265

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

[PATCH] D151308: -fsanitize=function: fix alignment fault on Arm targets.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

In D151308#4369828 , @MaskRay wrote:

> In D151308#4367704 , @peter.smith 
> wrote:
>
>> This looks good to me. Will be worth waiting for a day to give the US time 
>> zone time to leave any comments.
>
> Thanks!
>
>> I note that this is also broken in -fsanitize=kcfi [*] 
>> (https://reviews.llvm.org/D135411) although fixing that is a separate patch. 
>> Would you be able to raise a github issue to cover that?
>
> `-fsanitize=kcfi` only supports aarch64 and x86-64 now. riscv64 is on the 
> plan.
>
>   % fclang -fsanitize=kcfi --traget=armv7-linux-gnueabi -c a.c
>   clang: error: unsupported option '--traget=armv7-linux-gnueabi'

IIUC initially kcfi was x86_64 and AArch64 only. In D135411 
 "generic" support was added for all targets, 
quoting from the description.

  The KCFI sanitizer emits "kcfi" operand bundles to indirect
  call instructions, which the LLVM back-end lowers into an
  architecture-specific type check with a known machine instruction
  sequence. Currently, KCFI operand bundle lowering is supported only
  on 64-bit X86 and AArch64 architectures.
  
  As a lightweight forward-edge CFI implementation that doesn't
  require LTO is also useful for non-Linux low-level targets on
  other machine architectures, add a generic KCFI operand bundle
  lowering pass that's only used when back-end lowering support is not
  available and allows -fsanitize=kcfi to be enabled in Clang on all
  architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151308

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

[PATCH] D151081: [Clang][SVE2.1] Add svpext builtins

[Caroline via Phabricator via cfe-commits]

CarolineConcatto updated this revision to Diff 525508.
CarolineConcatto marked 2 inline comments as done.
CarolineConcatto added a comment.

Add doxygen comments to FormSVEBuiltinResult function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151081

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/aarch64-sve2p1-intrinsics/acle_sve2p1_pext.c
  clang/test/Sema/aarch64-sve2p1-intrinsics/acle_sve2p1_imm.cpp
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -73,12 +73,12 @@
 public:
   SVEType() : SVEType(TypeSpec(), 'v') {}
 
-  SVEType(TypeSpec TS, char CharMod)
+  SVEType(TypeSpec TS, char CharMod, unsigned NumVectors = 1)
   : TS(TS), Float(false), Signed(true), Immediate(false), Void(false),
 Constant(false), Pointer(false), BFloat(false), DefaultType(false),
 IsScalable(true), Predicate(false), PredicatePattern(false),
 PrefetchOp(false), Svcount(false), Bitwidth(128), ElementBitwidth(~0U),
-NumVectors(1) {
+NumVectors(NumVectors) {
 if (!TS.empty())
   applyTypespec();
 applyModifier(CharMod);
@@ -194,7 +194,9 @@
   SVEType getReturnType() const { return Types[0]; }
   ArrayRef getTypes() const { return Types; }
   SVEType getParamType(unsigned I) const { return Types[I + 1]; }
-  unsigned getNumParams() const { return Proto.size() - 1; }
+  unsigned getNumParams() const {
+return Proto.size() - (2 * std::count(Proto.begin(), Proto.end(), '.')) - 1;
+  }
 
   uint64_t getFlags() const { return Flags; }
   bool isFlagSet(uint64_t Flag) const { return Flags & Flag;}
@@ -228,11 +230,19 @@
 
   /// Return the parameter index of the splat operand.
   unsigned getSplatIdx() const {
-// These prototype modifiers are described in arm_sve.td.
-auto Idx = Proto.find_first_of("ajfrKLR@");
-assert(Idx != std::string::npos && Idx > 0 &&
-   "Prototype has no splat operand");
-return Idx - 1;
+unsigned I = 1, Param = 0;
+for (; I < Proto.size(); ++I, ++Param) {
+  if (Proto[I] == 'a' || Proto[I] == 'j' || Proto[I] == 'f' ||
+  Proto[I] == 'r' || Proto[I] == 'K' || Proto[I] == 'L' ||
+  Proto[I] == 'R' || Proto[I] == '@')
+break;
+
+  // Multivector modifier can be skipped
+  if (Proto[I] == '.')
+I += 2;
+}
+assert(I != Proto.size() && "Prototype has no splat operand");
+return Param;
   }
 
   /// Emits the intrinsic declaration to the ostream.
@@ -528,15 +538,6 @@
 
 void SVEType::applyModifier(char Mod) {
   switch (Mod) {
-  case '2':
-NumVectors = 2;
-break;
-  case '3':
-NumVectors = 3;
-break;
-  case '4':
-NumVectors = 4;
-break;
   case 'v':
 Void = true;
 break;
@@ -842,11 +843,36 @@
 Float = false;
 BFloat = false;
 break;
+  case '.':
+llvm_unreachable(". should never be a type");
+break;
   default:
 llvm_unreachable("Unhandled character!");
   }
 }
 
+/// Returns the modifier and number of vectors for the given operand \p Op.
+std::pair getProtoModifier(StringRef Proto, unsigned Op) {
+  for (unsigned P = 0; !Proto.empty(); ++P) {
+unsigned NumVectors = 1;
+unsigned CharsToSkip = 1;
+char Mod = Proto[0];
+if (Mod == '2' || Mod == '3' || Mod == '4') {
+  NumVectors = Mod - '0';
+  Mod = 'd';
+  if (Proto.size() > 1 && Proto[1] == '.') {
+Mod = Proto[2];
+CharsToSkip = 3;
+  }
+}
+
+if (P == Op)
+  return {Mod, NumVectors};
+
+Proto = Proto.drop_front(CharsToSkip);
+  }
+  llvm_unreachable("Unexpected Op");
+}
 
 //===--===//
 // Intrinsic implementation
@@ -862,8 +888,11 @@
   MergeSuffix(MergeSuffix.str()), BaseType(BT, 'd'), Flags(Flags),
   ImmChecks(Checks.begin(), Checks.end()) {
   // Types[0] is the return value.
-  for (unsigned I = 0; I < Proto.size(); ++I) {
-SVEType T(BaseTypeSpec, Proto[I]);
+  for (unsigned I = 0; I < (getNumParams() + 1); ++I) {
+char Mod;
+unsigned NumVectors;
+std::tie(Mod, NumVectors) = getProtoModifier(Proto, I);
+SVEType T(BaseTypeSpec, Mod, NumVectors);
 Types.push_back(T);
 
 // Add range checks for immediates
@@ -1092,10 +1121,11 @@
   assert(Arg >= 0 && Kind >= 0 && "Arg and Kind must be nonnegative");
 
   unsigned ElementSizeInBits = 0;
+  char Mod;
+  unsigned NumVectors;
+  std::tie(Mod, NumVectors) = getProtoModifier(Proto, EltSizeArg + 1);
   if (EltSizeArg >= 0)
-ElementSizeInBits =
-SVEType(TS, Proto[EltSizeArg + /* offset by return arg */ 1])
-.getElementSizeInBits();
+El

[PATCH] D145265: [Pipeline] Remove GlobalCleanupPM

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Here are two inputs that currently fold down to constants and no longer do so 
with this patch: https://gist.github.com/nikic/4a714ea550bf2252543570585f642af2 
These need further reduction for PhaseOrdering tests, but should be a good 
starting point for analysis...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145265

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

[PATCH] D151253: [clangd] Move completion signatures and labelDetails

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 525517.
sammccall retitled this revision from "[clangd] Move completion signatures and 
insertion markers into labelDetails" to "[clangd] Move completion signatures 
and labelDetails".
sammccall edited the summary of this revision.
sammccall added a comment.

revert change to insertion bullets: no consensus, and some rendering issues in 
vscode


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151253

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2093,7 +2093,8 @@
   Opts.EnableSnippets = false;
 
   auto R = C.render(Opts);
-  EXPECT_EQ(R.label, "Foo::x(bool) const");
+  EXPECT_EQ(R.label, "Foo::x");
+  EXPECT_EQ(R.labelDetails->detail, "(bool) const");
   EXPECT_EQ(R.insertText, "Foo::x");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
   EXPECT_EQ(R.filterText, "x");
@@ -2121,12 +2122,14 @@
 
   Include.Insertion.emplace();
   R = C.render(Opts);
-  EXPECT_EQ(R.label, "^Foo::x(bool) const");
+  EXPECT_EQ(R.label, "^Foo::x");
+  EXPECT_EQ(R.labelDetails->detail, "(bool) const");
   EXPECT_THAT(R.additionalTextEdits, Not(IsEmpty()));
 
   Opts.ShowOrigins = true;
   R = C.render(Opts);
-  EXPECT_EQ(R.label, "^[AS]Foo::x(bool) const");
+  EXPECT_EQ(R.label, "^[AS]Foo::x");
+  EXPECT_EQ(R.labelDetails->detail, "(bool) const");
 
   C.BundleSize = 2;
   R = C.render(Opts);
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -508,6 +508,10 @@
   /// textDocument.completion.completionItem.documentationFormat
   MarkupKind CompletionDocumentationFormat = MarkupKind::PlainText;
 
+  /// The client has support for completion item label details.
+  /// textDocument.completion.completionItem.labelDetailsSupport.
+  bool CompletionLabelDetail = false;
+
   /// Client supports CodeAction return value for textDocument/codeAction.
   /// textDocument.codeAction.codeActionLiteralSupport.
   bool CodeActionStructure = false;
@@ -1277,11 +1281,28 @@
   Snippet = 2,
 };
 
+/// Additional details for a completion item label.
+struct CompletionItemLabelDetails {
+  /// An optional string which is rendered less prominently directly after label
+	/// without any spacing. Should be used for function signatures or type
+  /// annotations.
+  std::string detail;
+
+  /// An optional string which is rendered less prominently after
+	/// CompletionItemLabelDetails.detail. Should be used for fully qualified
+	/// names or file path.
+  std::string description;
+};
+llvm::json::Value toJSON(const CompletionItemLabelDetails &);
+
 struct CompletionItem {
   /// The label of this completion item. By default also the text that is
   /// inserted when selecting this completion.
   std::string label;
 
+  /// Additional details for the label.
+  std::optional labelDetails;
+
   /// The kind of this completion item. Based of the kind an icon is chosen by
   /// the editor.
   CompletionItemKind kind = CompletionItemKind::Missing;
@@ -1342,6 +1363,10 @@
 llvm::json::Value toJSON(const CompletionItem &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CompletionItem &);
 
+/// Remove the labelDetails field (for clients that don't support it).
+/// Places the information into other fields of the completion item.
+void removeCompletionLabelDetails(CompletionItem &);
+
 bool operator<(const CompletionItem &, const CompletionItem &);
 
 /// Represents a collection of completion items to be presented in the editor.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -393,6 +393,8 @@
   if (auto *Item = Completion->getObject("completionItem")) {
 if (auto SnippetSupport = Item->getBoolean("snippetSupport"))
   R.CompletionSnippets = *SnippetSupport;
+if (auto LabelDetailsSupport = Item->getBoolean("labelDetailsSupport"))
+  R.CompletionLabelDetail = *LabelDetailsSupport;
 if (const auto *DocumentationFormat =
 Item->getArray("documentationFormat")) {
   for (const auto &Format : *DocumentationFormat) {
@@ -1069,6 +1071,25 @@
   return false;
 }
 
+llvm::json::Value toJSON(const CompletionItemLabelDetails &CD) {
+  llvm::json::Object Result;
+  if (!CD.detail.empty()

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

Thanks @aaron.ballman and @erichkeane for your patience in reviewing the patch 
and steering me in the right direction.

What do you think about the other two patches in the series:

- https://reviews.llvm.org/D148699 (comes before this one)
- https://reviews.llvm.org/D148702 (comes after this one)

The first one is pretty mechanical.  The second is going to be a slog to 
review, sorry.  It's one of those things that's much easier to write than to 
check afterwards.  I've tried to capture all the potentially 
non-obvious/non-mechanical parts in the covering note, but a long covering note 
might just make the review even more painful.  I'm happy to try presenting it 
in a different form if you can think of one that would help.

Thanks again for accepting this patch, really appreciate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700

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

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan marked 8 inline comments as done.
jacquesguan added a comment.

https://reviews.llvm.org/D151414 this is the backend part.




Comment at: clang/include/clang/Basic/riscv_vector.td:1856
   def vfwcvt_f_x_v : RVVConvBuiltin<"Fw", "Fwv", "csi", "vfwcvt_f">;
-  def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "xf", "vfwcvt_f">;
+  let RequiredFeatures = ["ZvfhminOrZvfh"] in
+def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "xf", "vfwcvt_f">;

michaelmaitland wrote:
> michaelmaitland wrote:
> > In general, I believe that `vfwcvt_f_f_v` and `vfncvt_f_f_w` do not require 
> > Zvfhmin or Zvfh. The only time that these intrinsics require Zvfhmin or 
> > Zvfh is when the operands to these intrinsics have EEW=16.
> The semantics for `RequiredFeatures` is `Features required to enable for this 
> builtin.` Since not all types in the range require the ZvfhminOrZvfh feature, 
> it may make sense to do some refactoring:
> 
> I think two possible solutions are:
>   1. to split def of `vfwcvt_f_f_v` and `vfncvt_f_f_w ` by type_range and the 
> type range `x` uses the RequiredFeatures
>   2. Use different required features for different type ranges (i.e. 
> RequiredFeatures is a list of lists where the outer list is for each type in 
> the range, and the inner list is the RequiredFeature for that type.)
I split it into 2 definations.



Comment at: 
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c:15
+
+
+

michaelmaitland wrote:
> Do we need a test that checks calls to `__riscv_vfwcvt_f` and 
> `__riscv_vfncvt_f` using type `vfloat16m1_t` without zvfh nor zvfhmin lead to 
> the expected error asking for `zvfh or zvfhmin`?
fp16 vector type check is in `Sema/riscv-vector-float16-check.c`.




Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1018
 
+if (VT.getVectorElementType() == MVT::f16 &&
+!Subtarget.hasVInstructionsF16()) {

craig.topper wrote:
> This needs to be below `By default everything must be expanded.`
Fixed in https://reviews.llvm.org/D151414.



Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2096
   case MVT::f16:
-return Subtarget.hasVInstructionsF16();
+return Subtarget.hasVInstructionsF16Mininal();
   case MVT::f32:

craig.topper wrote:
> Is this correct? This function is called by 
> `RISCVTargetTransformInfo::isLegalToVectorizeReduction`, 
> `isLegalMaskedLoadStore`, `isLegalMaskedGatherScatter`.
Fixed in https://reviews.llvm.org/D151414.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:162
+  bool hasVInstructionsF16Mininal() const {
+return HasStdExtZvfhmin || HasStdExtZvfh;
+  }

craig.topper wrote:
> Doesn't HasStdExtZvfh already imply HasStdExtZvfhmin?
The v spec doesn't metion this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

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

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

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

HazardyKnusperkeks added a comment.

In D151047#4369742 , @Sedeniono wrote:

> To create a new fix, do I understand the guide 
>  correctly that 
> I basically execute `arc diff --verbatim` and mention `Depends on D151047` 
> somewhere in the message? Will it then appear here automatically and reopen 
> the review?

I don't use arc, I just upload the diffs manually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[NAKAMURA Takumi via Phabricator via cfe-commits]

chapuni added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:71
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
 

This was the only user of Body in -Asserts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[clang] 0123deb - [Lex] Warn when defining or undefining any builtin macro

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2023-05-25T11:55:44+01:00
New Revision: 0123deb3a6f0a83095287f51b07c77b7b43ab847

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

LOG: [Lex] Warn when defining or undefining any builtin macro

Currently we warn when MI->isBuiltinMacro, but this is only true for
builtin macros that require processing when expanding. Checking
SourceMgr.isWrittenInBuiltinFile in addition to this will mean that
we catch all builtin macros, though we shouldn't warn on feature test
macros.

As part of doing this I've also moved the handling of undefining from
CheckMacroName to HandleUndefDirective, as it doesn't really make
sense to handle undefining in CheckMacroName but defining in
HandleDefineDirective. It would be nice to instead handle both in
CheckMacroName, but that isn't possible as the handling of defines
requires looking at what the name is being defined to.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Lex/PPDirectives.cpp
clang/test/Lexer/builtin_redef.c
clang/test/Preprocessor/macro-reserved.c
clang/test/Preprocessor/macro-reserved.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0075f94e344ee..6ee73aa3c96fe 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -305,6 +305,8 @@ Improvements to Clang's diagnostics
   (`#58601 `_)
 - Clang's `-Wshadow` warning now warns about shadowings by static local 
variables
   (`#62850: `_).
+- Clang now warns when any predefined macro is undefined or redefined, instead
+  of only some of them.
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 62e51e133b3af..2066c61748efa 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -109,52 +109,52 @@ enum PPElifDiag {
   PED_Elifndef
 };
 
+static bool isFeatureTestMacro(StringRef MacroName) {
+  // list from:
+  // * https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+  // * 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
+  // * man 7 feature_test_macros
+  // The list must be sorted for correct binary search.
+  static constexpr StringRef ReservedMacro[] = {
+  "_ATFILE_SOURCE",
+  "_BSD_SOURCE",
+  "_CRT_NONSTDC_NO_WARNINGS",
+  "_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
+  "_CRT_SECURE_NO_WARNINGS",
+  "_FILE_OFFSET_BITS",
+  "_FORTIFY_SOURCE",
+  "_GLIBCXX_ASSERTIONS",
+  "_GLIBCXX_CONCEPT_CHECKS",
+  "_GLIBCXX_DEBUG",
+  "_GLIBCXX_DEBUG_PEDANTIC",
+  "_GLIBCXX_PARALLEL",
+  "_GLIBCXX_PARALLEL_ASSERTIONS",
+  "_GLIBCXX_SANITIZE_VECTOR",
+  "_GLIBCXX_USE_CXX11_ABI",
+  "_GLIBCXX_USE_DEPRECATED",
+  "_GNU_SOURCE",
+  "_ISOC11_SOURCE",
+  "_ISOC95_SOURCE",
+  "_ISOC99_SOURCE",
+  "_LARGEFILE64_SOURCE",
+  "_POSIX_C_SOURCE",
+  "_REENTRANT",
+  "_SVID_SOURCE",
+  "_THREAD_SAFE",
+  "_XOPEN_SOURCE",
+  "_XOPEN_SOURCE_EXTENDED",
+  "__STDCPP_WANT_MATH_SPEC_FUNCS__",
+  "__STDC_FORMAT_MACROS",
+  };
+  return std::binary_search(std::begin(ReservedMacro), std::end(ReservedMacro),
+MacroName);
+}
+
 static MacroDiag shouldWarnOnMacroDef(Preprocessor &PP, IdentifierInfo *II) {
   const LangOptions &Lang = PP.getLangOpts();
-  if (isReservedInAllContexts(II->isReserved(Lang))) {
-// list from:
-// - https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
-// - 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/security-features-in-the-crt?view=msvc-160
-// - man 7 feature_test_macros
-// The list must be sorted for correct binary search.
-static constexpr StringRef ReservedMacro[] = {
-"_ATFILE_SOURCE",
-"_BSD_SOURCE",
-"_CRT_NONSTDC_NO_WARNINGS",
-"_CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES",
-"_CRT_SECURE_NO_WARNINGS",
-"_FILE_OFFSET_BITS",
-"_FORTIFY_SOURCE",
-"_GLIBCXX_ASSERTIONS",
-"_GLIBCXX_CONCEPT_CHECKS",
-"_GLIBCXX_DEBUG",
-"_GLIBCXX_DEBUG_PEDANTIC",
-"_GLIBCXX_PARALLEL",
-"_GLIBCXX_PARALLEL_ASSERTIONS",
-"_GLIBCXX_SANITIZE_VECTOR",
-"_GLIBCXX_USE_CXX11_ABI",
-"_GLIBCXX_USE_DEPRECATED",
-"_GNU_SOURCE",
-"_ISOC11_SOURCE",
-"_ISOC95_SOURCE",
-"_ISOC99_SOURCE",
-"_LARGEFILE64_SOURCE",
-"_POSIX_C_SOURCE",
-"_REENTRANT",
-"_SVID_SOURCE",
-"_THREAD_SAFE",
-"_XOPEN

[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

[Ritanya via Phabricator via cfe-commits]

RitanyaB updated this revision to Diff 525531.

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

https://reviews.llvm.org/D146418

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/declare_target_variables_ast_print.cpp
  clang/test/OpenMP/nvptx_target_exceptions_messages.cpp

Index: clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
===
--- clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
+++ clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
@@ -95,7 +95,7 @@
 int (*D)() = C; // expected-note {{used here}}
 // host-note@-1 {{used here}}
 #pragma omp end declare target
-int foobar3() { throw 1; }
+int foobar3() { throw 1; } // expected-error {{cannot use 'throw' with exceptions disabled}}
 
 // Check no infinite recursion in deferred diagnostic emitter.
 long E = (long)&E;
Index: clang/test/OpenMP/declare_target_variables_ast_print.cpp
===
--- /dev/null
+++ clang/test/OpenMP/declare_target_variables_ast_print.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 -w -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s --check-prefix=CHECK
+// expected-no-diagnostics
+
+static int variable = 100;
+static float variable1 = 200;
+static float variable2 = variable1;
+
+static int var = 1;
+
+static int var1 = 10;
+static int *var2 = &var1;
+static int **ptr1 = &var2;
+
+int arr[2] = {1,2};
+int (*arrptr)[2] = &arr;
+
+class declare{
+  public: int x;
+  void print();
+};
+declare obj1;
+declare *obj2 = &obj1;
+
+struct target{
+  int x;
+  void print();
+};
+static target S;
+
+#pragma omp declare target
+int target_var = variable;
+float target_var1 = variable2;
+int *ptr = &var;
+int ***ptr2 = &ptr1;
+int (**ptr3)[2] = &arrptr;
+declare **obj3 = &obj2;
+target *S1 = &S;
+#pragma omp end declare target
+// CHECK: #pragma omp declare target
+// CHECK-NEXT: static int variable = 100;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static float variable1 = 200;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static float variable2 = variable1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK: #pragma omp declare target
+// CHECK-NEXT: static int var = 1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int var1 = 10;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int *var2 = &var1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int **ptr1 = &var2;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int arr[2] = {1, 2};
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (*arrptr)[2] = &arr;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: class declare {
+// CHECK-NEXT: public:
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare obj1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare *obj2 = &obj1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: struct target {
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static target S;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int target_var = variable;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: float target_var1 = variable2;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int *ptr = &var;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int ***ptr2 = &ptr1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (**ptr3)[2] = &arrptr;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare **obj3 = &obj2;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: target *S1 = &S;
+// CHECK-NEXT: #pragma omp end declare target
+
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -233,6 +233,42 @@
 #pragma omp declare 

[PATCH] D144654: [Lex] Warn when defining or undefining any builtin macro

[John Brawn via Phabricator via cfe-commits]

john.brawn added a comment.

D150966  should fix the failure that was seen 
in buildbots, so I've now re-committed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144654

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

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D151047#4369742 , @Sedeniono wrote:

> To create a new fix, do I understand the guide 
>  correctly that 
> I basically execute `arc diff --verbatim` and mention `Depends on D151047` 
> somewhere in the message? Will it then appear here automatically and reopen 
> the review?

If you want to create a new review, use `arc diff --create`. It will always 
create a new review and assign it a different number.

If you want to reopen review because the patch was reverted:

- cherry-pick the reverted commit;
- do any necessary adjustments for the tests to pass, amend them;
- If you're using arc, use `arc diff --update D151047` as usual. It will update 
the current review, preserving its number.

(The --update option is not necessary if the commit message contains URL to 
this review, but it prevents you from accidentally creating a new review.)

I don't remember if the above steps will change the review status to Reopened.
If that doesn't happen, you can reopen the review manually from the menu below 
(Add Action... -> Reopen Revision).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added a comment.

> Still looking at issues and not sure whether these are blockers or not.

Friendly reminder:
@benlangmuir, do you need any assistance from my side with it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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

[PATCH] D151383: [clang-tidy] Check for specific return types on all functions

[Piotr Zegar via Phabricator via cfe-commits]

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

- Missing tests
- Missing release notes
- Missing check documentation update




Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:159
 
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(anyOf(

This may cause check method to be executed twice for same AST node.
Instead of adding new matcher you should just extend MatchedCallExpr to include 
all methods that `isInstantiatedFrom(hasAnyName(FunVec))` or returns one of 
those specific types.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:161
+  callExpr(callee(functionDecl(anyOf(
+   hasReturnTypeLoc(loc(asString("std::error_code"))),
+   hasReturnTypeLoc(loc(asString("std::expected"))),

Put those return types into some configuration option instead of hardcoding it.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:162
+   hasReturnTypeLoc(loc(asString("std::error_code"))),
+   hasReturnTypeLoc(loc(asString("std::expected"))),
+   
hasReturnTypeLoc(loc(asString("boost::system::error_code"))),

check if this work with typedefs/aliases...



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:176-177
 void UnusedReturnValueCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *Matched = Result.Nodes.getNodeAs("match")) {
-diag(Matched->getBeginLoc(),
- "the value returned by this function should be used")
-<< Matched->getSourceRange();
-diag(Matched->getBeginLoc(),
- "cast the expression to void to silence this warning",
- DiagnosticIDs::Note);
+  const char *callExprBindingNames[] = {"return-types", "match"};
+  for (const char *callExprBindingName : callExprBindingNames) {
+if (const auto *Matched =

this `for` is not needed, just use same "bind" name, and you wont need to 
change this method at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151383

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

[PATCH] D151426: [Clang][NFC] Prepare C++ DR suite for better test isolation

[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.

The changes LGTM (verified off-list that the regenerated HTML is identical 
before and after this change), but let's not land this until we've gotten 
farther along the review process with the related changes to split-file and the 
diagnostic verifier (just in case those reviews cause us to change direction 
for some reason).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151426

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

[PATCH] D151429: print user provide value in tabstop diagnostic

[csmoe via Phabricator via cfe-commits]

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

github issue: https://github.com/llvm/llvm-project/issues/62912


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151429

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2394,9 +2394,9 @@
 DiagMask = DiagnosticLevelMask::All;
   Opts.setVerifyIgnoreUnexpected(DiagMask);
   if (Opts.TabStop == 0 || Opts.TabStop > DiagnosticOptions::MaxTabStop) {
-Opts.TabStop = DiagnosticOptions::DefaultTabStop;
 Diags->Report(diag::warn_ignoring_ftabstop_value)
 << Opts.TabStop << DiagnosticOptions::DefaultTabStop;
+Opts.TabStop = DiagnosticOptions::DefaultTabStop;
   }
 
   addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings);


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2394,9 +2394,9 @@
 DiagMask = DiagnosticLevelMask::All;
   Opts.setVerifyIgnoreUnexpected(DiagMask);
   if (Opts.TabStop == 0 || Opts.TabStop > DiagnosticOptions::MaxTabStop) {
-Opts.TabStop = DiagnosticOptions::DefaultTabStop;
 Diags->Report(diag::warn_ignoring_ftabstop_value)
 << Opts.TabStop << DiagnosticOptions::DefaultTabStop;
+Opts.TabStop = DiagnosticOptions::DefaultTabStop;
   }
 
   addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148699: [clang] Mark keywords that have their own parsing rules

[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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148699

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

[PATCH] D151308: -fsanitize=function: fix alignment fault on Arm targets.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

In D151308#4367704 , @peter.smith 
wrote:

> I note that this is also broken in -fsanitize=kcfi [*] 
> (https://reviews.llvm.org/D135411) although fixing that is a separate patch. 
> Would you be able to raise a github issue to cover that?

OK, raised https://github.com/llvm/llvm-project/issues/62936 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151308

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

[PATCH] D151426: [Clang][NFC] Prepare C++ DR suite for better test isolation

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

In D151426#4371730 , @aaron.ballman 
wrote:

> The changes LGTM (verified off-list that the regenerated HTML is identical 
> before and after this change),

I've run `make_cxx_dr_status` on top of this patch, and learned that the patch 
didn't change anything in `cxx_dr_status.html`, and that it has been written 
resilient enough to happily digest the new notation of status comments.

> but let's not land this until we've gotten farther along the review process 
> with the related changes to split-file and the diagnostic verifier (just in 
> case those reviews cause us to change direction for some reason).

I don't mind being cautious here. Even though initial feedback on every of them 
doesn't seem to reject the direction we've been pushing towards. Here are the 
patches we're waiting for:

- https://reviews.llvm.org/D150856
- https://reviews.llvm.org/D150990
- https://reviews.llvm.org/D151320


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151426

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

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148700#4371518 , @rsandifo-arm 
wrote:

> Thanks @aaron.ballman and @erichkeane for your patience in reviewing the 
> patch and steering me in the right direction.
>
> What do you think about the other two patches in the series:
>
> - https://reviews.llvm.org/D148699 (comes before this one)
> - https://reviews.llvm.org/D148702 (comes after this one)
>
> The first one is pretty mechanical.  The second is going to be a slog to 
> review, sorry.  It's one of those things that's much easier to write than to 
> check afterwards.  I've tried to capture all the potentially 
> non-obvious/non-mechanical parts in the covering note, but a long covering 
> note might just make the review even more painful.  I'm happy to try 
> presenting it in a different form if you can think of one that would help.
>
> Thanks again for accepting this patch, really appreciate it.

Thank you for the reminder about the other patches in the stack, I had lost 
track of those. I was able to review the first one today, but I don't think 
I'll have the bandwidth to get the second one done today (I'm about to head out 
for a ~week's vacation) so that one may take a bit longer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700

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

[PATCH] D151430: [clang][dataflow][NFC] Remove unused variable.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Thanks to chapuni to pointing this out on
https://reviews.llvm.org/D151183.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151430

Files:
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -64,9 +64,6 @@
 AST->getASTContext()));
   assert(Func != nullptr);
 
-  Stmt *Body = Func->getBody();
-  assert(Body != nullptr);
-
   auto CFCtx =
   llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -64,9 +64,6 @@
 AST->getASTContext()));
   assert(Func != nullptr);
 
-  Stmt *Body = Func->getBody();
-  assert(Body != nullptr);
-
   auto CFCtx =
   llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 49946df - [clang][dataflow][NFC] Remove unused variable.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-25T12:09:52Z
New Revision: 49946df8211e9d36f0b3755e64b55bc28c0a4247

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

LOG: [clang][dataflow][NFC] Remove unused variable.

Thanks to chapuni to pointing this out on
https://reviews.llvm.org/D151183.

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

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git 
a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 84b10c87f6b1..1d94b69cfce8 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -64,9 +64,6 @@ runAnalysis(llvm::StringRef Code, AnalysisT 
(*MakeAnalysis)(ASTContext &)) {
 AST->getASTContext()));
   assert(Func != nullptr);
 
-  Stmt *Body = Func->getBody();
-  assert(Body != nullptr);
-
   auto CFCtx =
   llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 



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

[PATCH] D151430: [clang][dataflow][NFC] Remove unused variable.

[Martin Böhme 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 rG49946df8211e: [clang][dataflow][NFC] Remove unused variable. 
(authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151430

Files:
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -64,9 +64,6 @@
 AST->getASTContext()));
   assert(Func != nullptr);
 
-  Stmt *Body = Func->getBody();
-  assert(Body != nullptr);
-
   auto CFCtx =
   llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 


Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -64,9 +64,6 @@
 AST->getASTContext()));
   assert(Func != nullptr);
 
-  Stmt *Body = Func->getBody();
-  assert(Body != nullptr);
-
   auto CFCtx =
   llvm::cantFail(ControlFlowContext::build(*Func, AST->getASTContext()));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:71
-  auto CFCtx = llvm::cantFail(
-  ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
 

chapuni wrote:
> This was the only user of Body in -Asserts
Thanks for pointing this out. Fixed in https://reviews.llvm.org/D151430.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151183

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

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D151162#4369717 , @SlaterLatiao 
wrote:

> In D151162#4369584 , @aaron.ballman 
> wrote:
>
>> Please be sure to add a release note for the change, and it'd probably be 
>> good to have a test case that shows this triggers under `-Wall` (the 
>> modified test case explicitly names the diagnostic).
>
> Thank you, Aaron. I'll add the test case. Should this change go under 
> `Improvements to Clang’s diagnostics` in the release note?

Yes, that sounds like a good place for the note. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151162

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

[PATCH] D151431: [clang-tidy] Add check bugprone-unique-ptr-array-mismatch.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, steakhal, martong, 
gamesh411, Szelethus, dkrupp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151431

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UniquePtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UniquePtrArrayMismatchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/unique-ptr-array-mismatch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/unique-ptr-array-mismatch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unique-ptr-array-mismatch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unique-ptr-array-mismatch.cpp
@@ -0,0 +1,110 @@
+// RUN: %check_clang_tidy %s bugprone-unique-ptr-array-mismatch %t
+
+namespace std {
+
+template struct default_delete;
+template struct default_delete;
+
+template>
+class unique_ptr {
+public:
+  explicit unique_ptr(T* p) noexcept;
+  unique_ptr(T* p, Deleter d1 ) noexcept;
+};
+
+template 
+class unique_ptr {
+public:
+  template
+  explicit unique_ptr(U p) noexcept;
+  template
+  unique_ptr(U p, Deleter d1) noexcept;
+};
+
+} // namespace std
+
+struct A {};
+
+void f1() {
+  std::unique_ptr P1{new int};
+  std::unique_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr P2{new int[10]};
+  // clang-format off
+  std::unique_ptr<  int  > P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr<  int[]  > P3{new int[10]};
+  // clang-format on
+  std::unique_ptr P4(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr P4(new int[10]);
+  new std::unique_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr P5(new int[10]);
+  A deleter;
+  std::unique_ptr P6(new int[10], deleter);
+  std::unique_ptr P7(new int[10]);
+}
+
+void f2() {
+  std::unique_ptr P1(new A);
+  std::unique_ptr P2(new A[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr P2(new A[10]);
+  std::unique_ptr P3(new A[10]);
+}
+
+void f3() {
+  std::unique_ptr P1{new int}, P2{new int[10]}, P3{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+struct S {
+  std::unique_ptr P1;
+  std::unique_ptr P2{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  std::unique_ptr P3{new int}, P4{new int[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  S() : P1{new int[10]} {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+};
+
+void f_parm(std::unique_ptr);
+
+void f4() {
+  f_parm(std::unique_ptr{new int[10]});
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+std::unique_ptr f_ret() {
+  return std::unique_ptr(new int[10]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+}
+
+template 
+void f_tmpl() {
+  std::unique_ptr P1{new T[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unique pointer to non-array is initialized with array [bugprone-unique-ptr-array-mismatch]
+  // CHECK-FIXES: std::unique_ptr P1{new T[10]};
+}
+
+void f5() {
+  f_tmpl();
+}
+
+#define CHAR_PTR_TYPE std::unique_ptr
+#define CHAR_PTR_VAR(X) \
+  X { new char[10] }
+#define CHAR_PTR_INIT(X, Y) \
+  std::unique_ptr X { Y }
+
+void f6() {
+  CHAR_PTR_TYPE P1{new char[10]};
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning:

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.

This is missing all the usual Sema tests that the attribute only applies to 
functions, takes no arguments, only works in web assembly targets, etc. Also, 
the changes should come with a release note so users know about the new 
attribute.




Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612
+Clang supports the ``__attribute__((wasm_async))``
+attribute for the WebAssembly target. This attribute may be attached to a
+function definition, which indicates the function will be used with JavaScript
+promise integration (JSPI). The attribute will cause the creation of a custom
+section named "async" that contains each wasm_async function's index value.

This could be my ignorance of web assembly showing, but the docs don't really 
help me understand when you'd want to use this attribute. Perhaps a link to 
what JSPI is and a code example would help a little bit? Or is this more of a 
low-level implementation detail kind of attribute where folks already know the 
domain?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:898
+  if (FD->getAttr()) {
+llvm::Function *Fn = cast(GV);
+llvm::AttrBuilder B(GV->getContext());





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7637-7640
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+<< "'wasm_async'" << ExpectedFunction;
+return;

This should be handled automatically for you by the common attribute handling 
code, so I think you can remove this.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7645
+  if (FD->isThisDeclarationADefinition()) {
+S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
+return;

This diagnostic doesn't make sense to me -- how does this attribute relate to 
ifuncs or aliases? Why should users be prohibited from writing the attribute on 
a definition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

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

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko updated this revision to Diff 525557.
ivanmurashko added a comment.

rebase to latest LLVM main branch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/implicit-module-header-maps.cpp


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,54 @@
+// UNSUPPORTED: system-windows
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write a.hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap 
-fmodules-cache-path=%t test.cpp
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" b.hmap.json > hmap.json
+// RUN: %hmaptool write hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap 
-fmodules-cache-path=%t test.cpp
+
+//--- After/Mapping.h
+#ifdef FOO
+#error foo
+#endif
+
+//--- a.hmap.json
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
+
+//--- b.hmap.json
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
+
+//--- module.modulemap
+module a {
+  header "After/Mapping.h"
+}
+
+
+//--- test.cpp
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't used, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -491,7 +491,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto File) -> OptionalFileEntryRef {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -501,6 +502,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&File.getFileEntry(), File.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return std::nullopt;
+}
+return File;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -513,8 +520,7 @@
   }
 
   if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest, OpenFile)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   // Header maps need to be marked as used whenever the filename matches.


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,54 @@
+// UNSUPPORTED: system-windows
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write a.hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap -fmodules-cache-path=%t test.cpp
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" b.hmap.json > hmap.json
+// RUN: %hmaptool write hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap -fmodules-cache-path=%t test.cpp
+
+//--- After/Mapping.h
+#ifdef FOO
+#error foo
+#endif
+
+//--- a.hmap.json
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
+
+//--- b.hmap.json
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
+
+//--- module.modulemap
+module a {
+  header "After/Mapping.h"
+}
+
+
+//--- test.cpp
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't used, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -491,7 +491,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSe

[PATCH] D145302: [clangd] Add library for clangd main function

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko updated this revision to Diff 525558.
ivanmurashko added a comment.

rebased to the latest LLVM main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145302

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/tool/ClangdMain.h
  clang-tools-extra/clangd/tool/ClangdToolMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdToolMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/ClangdToolMain.cpp
@@ -0,0 +1,13 @@
+//===--- ClangdToolMain.cpp - clangd main function ===//
+//
+// 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 "ClangdMain.h"
+
+int main(int argc, char **argv) {
+  return clang::clangd::clangdMain(argc, argv);
+}
Index: clang-tools-extra/clangd/tool/ClangdMain.h
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/ClangdMain.h
@@ -0,0 +1,19 @@
+//===--- ClangdMain.h - clangd main function ===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+
+namespace clang {
+namespace clangd {
+// clangd main function (clangd server loop)
+int clangdMain(int argc, char *argv[]);
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "ClangdMain.h"
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
@@ -710,8 +711,6 @@
   }
 };
 } // namespace
-} // namespace clangd
-} // namespace clang
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
@@ -719,10 +718,7 @@
   CheckFailed = 3
 };
 
-int main(int argc, char *argv[]) {
-  using namespace clang;
-  using namespace clang::clangd;
-
+int clangdMain(int argc, char *argv[]) {
   llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::sys::AddSignalHandler(
@@ -1041,3 +1037,6 @@
 
   return ExitCode;
 }
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -1,6 +1,13 @@
-add_clang_tool(clangd
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangdMain.cpp ClangdToolMain.cpp Check.cpp)
+
+add_clang_library(clangdMain
   ClangdMain.cpp
   Check.cpp
+  )
+
+add_clang_tool(clangd
+  ClangdToolMain.cpp
   $
   )
 
@@ -13,8 +20,32 @@
   list(APPEND CLANGD_XPC_LIBS "clangdXpcJsonConversions" "clangdXpcTransport")
 endif()
 
+clang_target_link_libraries(clangdMain
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
+target_link_libraries(clangdMain
+  PRIVATE
+  clangTidy
+  clangDaemon
+  clangdRemoteIndex
+  clangdSupport
+  ${CLANGD_XPC_LIBS}
+  )
+
 clang_target_link_libraries(clangd
   PRIVATE
+  clangdMain
   clangAST
   clangBasic
   clangFormat
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -219,3 +219,17 @@
 
 add_subdirectory(index/remote)
 add_subdirectory(index/dex/dexp)
+
+if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
+  install(FILES
+${CMAKE_CURRENT_SOURCE_DIR}/tool/ClangdMain.h
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/clangd"
+COMPONENT clangd-headers)
+  add_custom_target(clangd-headers)
+  set_target_properties(clangd-headers PROPERTIES FOLDER "Misc")
+  if(NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-clangd-headers
+ DEPENDS clangd-headers
+ COMPONENT clangd-headers)
+  endif()
+end

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D151087#4363503 , @ebevhan wrote:

> In D151087#4362059 , @aaron.ballman 
> wrote:
>
>> Based on all this, I think we should go with `__addrspace_cast` as a named 
>> cast and not allow the conversion through `reinterpret_cast` unless going 
>> to/from a `[u]intptr_t`.
>
> I think this sounds good. Most of the building blocks for it should already 
> be in place in the form of OpenCL's addrspace_cast. It would remove 
> reinterpret_cast's ability to perform safe conversions, though. Could that 
> affect something else inadvertently? ICS or SCS?
>
> There are other C++ casts that deal with address spaces today. static_cast 
> can also do it when converting from a void pointer, for example. Should it 
> also lose the ability?

I think this is another design approach we could consider -- splitting the 
functionality out between the various C++ named casts.

According to the TR, we should know at compile time which address spaces exist 
and whether space B is a subset of space A or whether they're disjoint. There 
can be implicit conversions from A to B if A is a subset of B, but it might not 
be bit-pattern preserving. That's similar to a derived-to-base conversion. 
`static_cast` can reverse implicit conversions, so it could be used to reverse 
the A->B conversion. It'd be undefined behavior if the pointer isn't 
representable in A's address space, but that's also the same as a 
base-to-derived conversion when the object doesn't have a derived type. For 
disjoint conversions of A to B, static_cast should fail (and perhaps C-style 
casts should as well -- there's no meaningful conversion between disjoint 
address spaces). And then `dynamic_cast` could be used for doing reverse 
implicit conversions that can detect invalid conversion attempts and throw an 
exception/return a nullptr. `reinterpret_cast` could then be used for 
bit-pattern preserving same-sized casts so it remains a bitcast operation.

(This idea came via a different WG21 member I was also discussing this topic 
with.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D151398: [clang] Make `FileEntryRef::getDir()` return the as-requested `DirectoryEntryRef`

[Richard Howell via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/Basic/FileEntry.h:124
+/// Directory the file was found in.
 OptionalDirectoryEntryRef Dir;
 

If this is always set now, should it be a non optional `DirectoryEntryRef`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151398

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

[PATCH] D151060: [Clang][Sema] Generate vector vs scalar builtin overloads

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for the fix! The changes should come with a release note.




Comment at: clang/lib/Sema/SemaOverload.cpp:9041-9044
+  // (allowing splatting the scalar to a vector).
+  for (unsigned Candidate = 0; Candidate < 2; ++Candidate) {
+for (QualType Vec1Ty : CandidateTypes[Candidate].vector_types()) {
+  for (QualType Vec2Ty : CandidateTypes[Candidate].vector_types()) {

I'm a bit confused -- the comment says this is to allow splatting the scalar to 
a vector, but... what is the scalar type in these loops?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151060

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

[PATCH] D151437: [NFC][Driver] Change MultilibBuilder interface

[Michael Platings via Phabricator via cfe-commits]

michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added a subscriber: abrachet.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Decouple the interface of the MultilibBuilder flag method from how flags
are stored internally. Likewise change the addMultilibFlag function.

Currently a multilib flag like "-fexceptions" means a multilib is
*incompatible* with the -fexceptions command line option, which is
counter-intuitive. This change is a step towards changing this scheme.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151437

Files:
  clang/include/clang/Driver/MultilibBuilder.h
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/OHOS.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp

Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -27,20 +27,22 @@
 
   ASSERT_TRUE(MultilibBuilder().isValid()) << "Empty multilib is not valid";
 
-  ASSERT_TRUE(MultilibBuilder().flag("+foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag(true, "-foo").isValid())
   << "Single indicative flag is not valid";
 
-  ASSERT_TRUE(MultilibBuilder().flag("-foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag(false, "-foo").isValid())
   << "Single contraindicative flag is not valid";
 
-  ASSERT_FALSE(MultilibBuilder().flag("+foo").flag("-foo").isValid())
+  ASSERT_FALSE(
+  MultilibBuilder().flag(true, "-foo").flag(false, "-foo").isValid())
   << "Conflicting flags should invalidate the Multilib";
 
-  ASSERT_TRUE(MultilibBuilder().flag("+foo").flag("+foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag(true, "-foo").flag(true, "-foo").isValid())
   << "Multilib should be valid even if it has the same flag "
  "twice";
 
-  ASSERT_TRUE(MultilibBuilder().flag("+foo").flag("-foobar").isValid())
+  ASSERT_TRUE(
+  MultilibBuilder().flag(true, "-foo").flag(false, "-foobar").isValid())
   << "Seemingly conflicting prefixes shouldn't actually conflict";
 }
 
@@ -52,7 +54,8 @@
 }
 
 TEST(MultilibBuilderTest, Construction3) {
-  MultilibBuilder M = MultilibBuilder().flag("+f1").flag("+f2").flag("-f3");
+  MultilibBuilder M =
+  MultilibBuilder().flag(true, "-f1").flag(true, "-f2").flag(false, "-f3");
   for (const std::string &A : M.flags()) {
 ASSERT_TRUE(llvm::StringSwitch(A)
 .Cases("+f1", "+f2", "-f3", true)
@@ -63,7 +66,7 @@
 TEST(MultilibBuilderTest, SetConstruction1) {
   // Single maybe
   MultilibSet MS = MultilibSetBuilder()
-   .Maybe(MultilibBuilder("64").flag("+m64"))
+   .Maybe(MultilibBuilder("64").flag(true, "-m64"))
.makeMultilibSet();
   ASSERT_TRUE(MS.size() == 2);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
@@ -79,8 +82,8 @@
 TEST(MultilibBuilderTest, SetConstruction2) {
   // Double maybe
   MultilibSet MS = MultilibSetBuilder()
-   .Maybe(MultilibBuilder("sof").flag("+sof"))
-   .Maybe(MultilibBuilder("el").flag("+EL"))
+   .Maybe(MultilibBuilder("sof").flag(true, "-sof"))
+   .Maybe(MultilibBuilder("el").flag(true, "-EL"))
.makeMultilibSet();
   ASSERT_TRUE(MS.size() == 4);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
@@ -154,7 +157,7 @@
 
 TEST(MultilibBuilderTest, SetSelection1) {
   MultilibSet MS1 = MultilibSetBuilder()
-.Maybe(MultilibBuilder("64").flag("+m64"))
+.Maybe(MultilibBuilder("64").flag(true, "-m64"))
 .makeMultilibSet();
 
   Multilib::flags_list FlagM64 = {"+m64"};
@@ -174,8 +177,8 @@
 
 TEST(MultilibBuilderTest, SetSelection2) {
   MultilibSet MS2 = MultilibSetBuilder()
-.Maybe(MultilibBuilder("el").flag("+EL"))
-.Maybe(MultilibBuilder("sf").flag("+SF"))
+.Maybe(MultilibBuilder("el").flag(true, "-EL"))
+.Maybe(MultilibBuilder("sf").flag(true, "-SF"))
 .makeMultilibSet();
 
   for (unsigned I = 0; I < 4; ++I) {
Index: clang/lib/Driver/ToolChains/OHOS.cpp
===
--- clang/lib/Driver/ToolChains/OHOS.cpp
+++ clang/lib/Driver/ToolChains/OHOS.cpp
@@ -66,20 +66,20 @@
   bool IsA7 = false;
   if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
 IsA7 = A->getValue() == StringRe

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:2288
+argument is the index to which to store the value into, and the
+third argument is a value of reference type to store in the table.
+It returns nothing.

pmatos wrote:
> aaron.ballman wrote:
> > This sounds like any reference type will work, e.g.,
> > ```
> > static __externref_t table[0];
> > void func(int i) {
> >   int &ref = i;
> >   __builtin_wasm_table_set(table, i, ref);
> > }
> > ```
> > so might want to say "value of ``_externref_t`` type" instead?
> Any reference type will work, i.e. externref or funcref. You can also store 
> funcrefs in a wasm table. But of course, it needs to be well-typed. You 
> cannot store a funcref value in an externref table and vice-versa.
My point is more that "any reference type" suggests you can use an arbitrary 
C++ reference type like `int &` and I think we mean something more specific 
than that, right?



Comment at: clang/docs/LanguageExtensions.rst:2364-2365
+range, the third argument is the value to set in the new entries, and 
+the fourth and the last argument is the size of the range. It returns 
+nothing.
+

pmatos wrote:
> aaron.ballman wrote:
> > What happens if the range is invalid for the table size? e.g., the user 
> > never called `__builtin_wasm_table_grow` before calling 
> > `__builtin_wasm_table_fill`?
> The host executing the WebAssembly instance will trap. This is part of the 
> WebAssembly spec.
Hmmm, do you think we should mention that here? Alternatively, should we have a 
link to other WebAssembly documentation so we can say "see  for more 
details?" to avoid replicating docs too much?



Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:35
+task<__externref_t[]> g() {
+  co_return table;
+}

pmatos wrote:
> aaron.ballman wrote:
> > pmatos wrote:
> > > @aaron.ballman I tried and failed to create a good testcase for 
> > > co_return. However creating coroutines seems to be an stdlib thing which 
> > > I am not sure how to test here. Do you have any suggestions here?
> > ```
> > #include "Inputs/std-coroutine.h"
> > 
> > using std::suspend_always;
> > 
> > struct promise_table {
> >   __externref_t[] get_return_object();
> >   suspend_always initial_suspend();
> >   suspend_always final_suspend() noexcept;
> >   void return_value(__externref_t[]);
> >   void unhandled_exception();
> > };
> > 
> > template 
> > struct std::coroutine_traits<__externref_t[], T...> { using promise_type = 
> > promise_table; };
> > 
> > static __externref_t table[0];
> > __externref_t[] func() {
> >   co_return table; // Cannot return a WebAssembly table?
> > }
> > ```
> > Perhaps something along these lines?
> But you cannot write promise_table because a table cannot be an argument in 
> return_value. Also, you cannot return a table in get_return_object. Doesn't 
> this invalidate straight away the use of a table with co-routines?
Ah! Yeah, I think that does. Okay, let's punt on coroutines, that's turning out 
to be a slog.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

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

[PATCH] D145302: [clangd] Add library for clangd main function

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko updated this revision to Diff 525570.
ivanmurashko added a comment.

removed header install, only lib install has been left after the change

CC: @kadircet


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145302

Files:
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/tool/ClangdMain.h
  clang-tools-extra/clangd/tool/ClangdToolMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdToolMain.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/ClangdToolMain.cpp
@@ -0,0 +1,13 @@
+//===--- ClangdToolMain.cpp - clangd main function ===//
+//
+// 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 "ClangdMain.h"
+
+int main(int argc, char **argv) {
+  return clang::clangd::clangdMain(argc, argv);
+}
Index: clang-tools-extra/clangd/tool/ClangdMain.h
===
--- /dev/null
+++ clang-tools-extra/clangd/tool/ClangdMain.h
@@ -0,0 +1,19 @@
+//===--- ClangdMain.h - clangd main function ===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+
+namespace clang {
+namespace clangd {
+// clangd main function (clangd server loop)
+int clangdMain(int argc, char *argv[]);
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "ClangdMain.h"
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
@@ -710,8 +711,6 @@
   }
 };
 } // namespace
-} // namespace clangd
-} // namespace clang
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
@@ -719,10 +718,7 @@
   CheckFailed = 3
 };
 
-int main(int argc, char *argv[]) {
-  using namespace clang;
-  using namespace clang::clangd;
-
+int clangdMain(int argc, char *argv[]) {
   llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::sys::AddSignalHandler(
@@ -1041,3 +1037,6 @@
 
   return ExitCode;
 }
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -1,6 +1,13 @@
-add_clang_tool(clangd
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangdMain.cpp ClangdToolMain.cpp Check.cpp)
+
+add_clang_library(clangdMain
   ClangdMain.cpp
   Check.cpp
+  )
+
+add_clang_tool(clangd
+  ClangdToolMain.cpp
   $
   )
 
@@ -13,8 +20,32 @@
   list(APPEND CLANGD_XPC_LIBS "clangdXpcJsonConversions" "clangdXpcTransport")
 endif()
 
+clang_target_link_libraries(clangdMain
+  PRIVATE
+  clangAST
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangLex
+  clangSema
+  clangTooling
+  clangToolingCore
+  clangToolingRefactoring
+  clangToolingSyntax
+  )
+
+target_link_libraries(clangdMain
+  PRIVATE
+  clangTidy
+  clangDaemon
+  clangdRemoteIndex
+  clangdSupport
+  ${CLANGD_XPC_LIBS}
+  )
+
 clang_target_link_libraries(clangd
   PRIVATE
+  clangdMain
   clangAST
   clangBasic
   clangFormat
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151438: [NFC][Driver] Change Multilib flag representation

[Michael Platings via Phabricator via cfe-commits]

michaelplatings created this revision.
michaelplatings added a reviewer: phosek.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This new representation means that if a string is a valid command line
option then it can also be used to indicate that the option is required
by a multilib.

To indicate that a flag is required not to be present, its first
character is replaced with '!', which is intended for consistency with
the logical not operator in many programming languages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151438

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/MultilibBuilder.h
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/OHOS.cpp

Index: clang/lib/Driver/ToolChains/OHOS.cpp
===
--- clang/lib/Driver/ToolChains/OHOS.cpp
+++ clang/lib/Driver/ToolChains/OHOS.cpp
@@ -40,15 +40,15 @@
   // -mfloat-abi=soft -mfloat-abi=softfp -mfloat-abi=hard
   // -mfpu=neon-vfpv4
   Multilibs.push_back(
-  Multilib("/a7_soft", {}, {}, {"+mcpu=cortex-a7", "+mfloat-abi=soft"}));
+  Multilib("/a7_soft", {}, {}, {"-mcpu=cortex-a7", "-mfloat-abi=soft"}));
 
   Multilibs.push_back(
   Multilib("/a7_softfp_neon-vfpv4", {}, {},
-   {"+mcpu=cortex-a7", "+mfloat-abi=softfp", "+mfpu=neon-vfpv4"}));
+   {"-mcpu=cortex-a7", "-mfloat-abi=softfp", "-mfpu=neon-vfpv4"}));
 
   Multilibs.push_back(
   Multilib("/a7_hard_neon-vfpv4", {}, {},
-   {"+mcpu=cortex-a7", "+mfloat-abi=hard", "+mfpu=neon-vfpv4"}));
+   {"-mcpu=cortex-a7", "-mfloat-abi=hard", "-mfpu=neon-vfpv4"}));
 
   if (Multilibs.select(Flags, Result.SelectedMultilib)) {
 Result.Multilibs = Multilibs;
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1921,9 +1921,9 @@
 Multilib::flags_list &Flags) {
   assert(Flag.front() == '-');
   if (Enabled) {
-Flags.push_back(("+" + Flag.substr(1)).str());
-  } else {
 Flags.push_back(Flag.str());
+  } else {
+Flags.push_back(("!" + Flag.substr(1)).str());
   }
 }
 
Index: clang/lib/Driver/MultilibBuilder.cpp
===
--- clang/lib/Driver/MultilibBuilder.cpp
+++ clang/lib/Driver/MultilibBuilder.cpp
@@ -76,7 +76,7 @@
 StringRef Flag(Flags[I]);
 llvm::StringMap::iterator SI = FlagSet.find(Flag.substr(1));
 
-assert(StringRef(Flag).front() == '+' || StringRef(Flag).front() == '-');
+assert(StringRef(Flag).front() == '-' || StringRef(Flag).front() == '!');
 
 if (SI == FlagSet.end())
   FlagSet[Flag.substr(1)] = I;
@@ -97,10 +97,10 @@
 
 MultilibSetBuilder &MultilibSetBuilder::Maybe(const MultilibBuilder &M) {
   MultilibBuilder Opposite;
-  // Negate any '+' flags
+  // Negate positive flags
   for (StringRef Flag : M.flags()) {
-if (Flag.front() == '+')
-  Opposite.flags().push_back(("-" + Flag.substr(1)).str());
+if (Flag.front() == '-')
+  Opposite.flags().push_back(("!" + Flag.substr(1)).str());
   }
   return Either(M, Opposite);
 }
Index: clang/lib/Driver/Multilib.cpp
===
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -49,7 +49,7 @@
   }
   OS << ";";
   for (StringRef Flag : Flags) {
-if (Flag.front() == '+')
+if (Flag.front() == '-')
   OS << "@" << Flag.substr(1);
   }
 }
Index: clang/include/clang/Driver/MultilibBuilder.h
===
--- clang/include/clang/Driver/MultilibBuilder.h
+++ clang/include/clang/Driver/MultilibBuilder.h
@@ -70,7 +70,7 @@
   MultilibBuilder &includeSuffix(StringRef S);
 
   /// Get the flags that indicate or contraindicate this multilib's use
-  /// All elements begin with either '+' or '-'
+  /// All elements begin with either '-' or '!'
   const flags_list &flags() const { return Flags; }
   flags_list &flags() { return Flags; }
 
Index: clang/include/clang/Driver/Multilib.h
===
--- clang/include/clang/Driver/Multilib.h
+++ clang/include/clang/Driver/Multilib.h
@@ -58,7 +58,7 @@
   const std::string &includeSuffix() const { return IncludeSuffix; }
 
   /// Get the flags that indicate or contraindicate this multilib's use
-  /// All elements begin with either '+' or '-'
+  /// All elements begin with either '-' or '!'
   const flags_list &flags() const { return Flags; }
 
   LLVM_DUMP_METHOD void dump() const;
___
cfe-commits mailing list
cfe-commits@l

[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: tahonermann, cor3ntin.
aaron.ballman added subscribers: cor3ntin, tahonermann.
aaron.ballman added a comment.

This looks correct to me, but because it involves things like characters, 
lines, and output, I'd appreciate if @tahonermann or @cor3ntin could do the 
final sign-off just in case I missed something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151215

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

[PATCH] D146389: [clang-repl][CUDA] Initial interactive CUDA support for clang-repl

[Anubhab Ghosh via Phabricator via cfe-commits]

argentite updated this revision to Diff 525572.
argentite added a comment.

Workaround for depending on NVPTX symbols: initialize all available targets 
instead. If NVPTX is not available, it will complain when we try to actually 
execute anything in CUDA mode.
Rebased and fixed conflicts on recent value printing related patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146389

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/DeviceOffload.cpp
  clang/lib/Interpreter/DeviceOffload.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/test/Interpreter/CUDA/device-function-template.cu
  clang/test/Interpreter/CUDA/device-function.cu
  clang/test/Interpreter/CUDA/host-and-device.cu
  clang/test/Interpreter/CUDA/lit.local.cfg
  clang/test/Interpreter/CUDA/memory.cu
  clang/test/Interpreter/CUDA/sanity.cu
  clang/test/lit.cfg.py
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -46,7 +46,9 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CB = clang::IncrementalCompilerBuilder();
+  CB.SetCompilerArgs(ClangArgs);
+  auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
   return cantFail(clang::Interpreter::create(std::move(CI)));
Index: clang/unittests/Interpreter/IncrementalProcessingTest.cpp
===
--- clang/unittests/Interpreter/IncrementalProcessingTest.cpp
+++ clang/unittests/Interpreter/IncrementalProcessingTest.cpp
@@ -52,7 +52,9 @@
 
 TEST(IncrementalProcessing, EmitCXXGlobalInitFunc) {
   std::vector ClangArgv = {"-Xclang", "-emit-llvm-only"};
-  auto CI = llvm::cantFail(IncrementalCompilerBuilder::create(ClangArgv));
+  auto CB = clang::IncrementalCompilerBuilder();
+  CB.SetCompilerArgs(ClangArgv);
+  auto CI = cantFail(CB.CreateCpp());
   auto Interp = llvm::cantFail(Interpreter::create(std::move(CI)));
 
   std::array PTUs;
Index: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
===
--- clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
+++ clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
@@ -38,7 +38,9 @@
   DiagnosticConsumer *Client = nullptr) {
   Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
   ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
-  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  auto CB = clang::IncrementalCompilerBuilder();
+  CB.SetCompilerArgs(ClangArgs);
+  auto CI = cantFail(CB.CreateCpp());
   if (Client)
 CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
   return cantFail(clang::Interpreter::create(std::move(CI)));
Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -20,9 +20,13 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h" // llvm_shutdown
 #include "llvm/Support/Signals.h"
-#include "llvm/Support/TargetSelect.h" // llvm::Initialize*
+#include "llvm/Support/TargetSelect.h"
 #include 
 
+static llvm::cl::opt CudaEnabled("cuda", llvm::cl::Hidden);
+static llvm::cl::opt CudaPath("cuda-path", llvm::cl::Hidden);
+static llvm::cl::opt OffloadArch("offload-arch", llvm::cl::Hidden);
+
 static llvm::cl::list
 ClangArgs("Xcc",
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -76,8 +80,11 @@
   std::vector ClangArgv(ClangArgs.size());
   std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
  [](const std::string &s) -> const char * { return s.data(); });
-  llvm::InitializeNativeTarget();
-  llvm::InitializeNativeTargetAsmPrinter();
+  // Initialize all targets (required for device offloading)
+  llvm::InitializeAllTargetInfos();
+  llvm::InitializeAllTargets();
+  llvm::InitializeAllTarg

[PATCH] D146757: [Driver] Enable defining multilib flags verbatim

[Michael Platings via Phabricator via cfe-commits]

michaelplatings abandoned this revision.
michaelplatings added a comment.

Abandoning this in favour of D151437  & 
D151438 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146757

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

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added a comment.

Ping for review :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149872

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

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis marked 2 inline comments as not done.
TIFitis added a comment.

Ping for reviews :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150860

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

[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo updated this revision to Diff 525477.
VitaNuo added a comment.
VitaNuo updated this revision to Diff 525560.
VitaNuo updated this revision to Diff 525566.
VitaNuo updated this revision to Diff 525574.
VitaNuo retitled this revision from "[WIP][include-cleaner] Allow multiple 
strategies for spelling includes." to "[include-cleaner] Allow multiple 
strategies for spelling includes.".
VitaNuo added a reviewer: kadircet.
VitaNuo published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Update


VitaNuo added a comment.

Instantiate the strategies early. Prevent instantiation on each spelling 
request.


VitaNuo added a comment.

Minor update.


VitaNuo added a comment.

Remove extra comment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150185

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -12,6 +12,7 @@
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -24,8 +25,12 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Registry.h"
 #include 
 
+LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy)
+
 namespace clang::include_cleaner {
 
 void walkUsed(llvm::ArrayRef ASTRoots,
@@ -54,20 +59,32 @@
 }
 
 std::string spellHeader(const Header &H, HeaderSearch &HS,
-const FileEntry *Main) {
-  switch (H.kind()) {
-  case Header::Physical: {
-bool IsSystem = false;
-std::string Path = HS.suggestPathToFileForDiagnostics(
-H.physical(), Main->tryGetRealPathName(), &IsSystem);
-return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
-  }
-  case Header::Standard:
+const FileEntry *Main,
+const IncludeSpeller *CustomSpeller) {
+  if (H.kind() == Header::Standard) {
 return H.standard().name().str();
-  case Header::Verbatim:
+  }
+
+  if (H.kind() == Header::Verbatim) {
 return H.verbatim().str();
   }
-  llvm_unreachable("Unknown Header kind");
+
+  if (H.kind() != Header::Physical)
+llvm_unreachable("Unknown Header kind");
+
+  // Spelling physical headers allows for various plug-in strategies.
+  std::string FinalSpelling;
+  if (CustomSpeller)
+FinalSpelling = (*CustomSpeller)(H.physical()->tryGetRealPathName());
+
+  if (!FinalSpelling.empty())
+return FinalSpelling;
+
+  // Fallback to default spelling via header search.
+  bool IsSystem = false;
+  std::string Path = HS.suggestPathToFileForDiagnostics(
+  H.physical(), Main->tryGetRealPathName(), &IsSystem);
+  return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
 }
 
 AnalysisResults analyze(llvm::ArrayRef ASTRoots,
@@ -89,8 +106,11 @@
}
  }
  if (!Satisfied && !Providers.empty() &&
- Ref.RT == RefType::Explicit)
-   Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+ Ref.RT == RefType::Explicit) {
+   ApplyFirstIncludeSpeller Speller;
+   Missing.insert(
+   spellHeader(Providers.front(), HS, MainFile, &Speller));
+ }
});
 
   AnalysisResults Results;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -14,11 +14,14 @@
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/MemoryBufferRef.h"
-#include 
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+#include 
+#include 
 
 namespace clang {
 class SourceLocation;
@@ -75,9 +78,6 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
-std::string spellHeader(const

[PATCH] D147321: [RFC][Flang][OMPIRBuilder] Add nounwind attribute to the LLVM IR

[Dominik Adamski via Phabricator via cfe-commits]

domada added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5063
+  }
+}
+

jdoerfert wrote:
> jdoerfert wrote:
> > Style `F`
> Why does this belong here? We should do it like clang, one function at a 
> time, or in a generic place. This is not OpenMP specific at all.
Hi Johannes,
thank you very much for your feedback.

I would like to explain my idea why I add this function to OMPIRBuilder:

Current situation:
1) MLIR does not support LLVM IR function attributes for production quality 
code. MLIR function passthrough attributes should be used only for PoC work ( 
https://mlir.llvm.org/docs/Dialects/LLVM/#attribute-pass-through )
2) OpenMP MLIR operations can be translated into multiple LLVM IR functions.

My idea was to add functionality of inserting function attributes after the end 
of OpenMP MLIR code translation. OMPIRBuilder has necessary knowledge about 
generated LLVM IR. That's why I decided to do it here and to focus mainly on 
the code generated for OpenMP pragmas.


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

https://reviews.llvm.org/D147321

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

[PATCH] D151439: [Clang][SVE2.1] Add builtins for 2-way svdot (vectors, indexed)

[Caroline via Phabricator via cfe-commits]

CarolineConcatto created this revision.
Herald added subscribers: arphaman, kristof.beyls, tschuett.
Herald added a project: All.
CarolineConcatto requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As described in: https://github.com/ARM-software/acle/pull/257

Patch by: David Sherwood 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151439

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/test/CodeGen/aarch64-sve2p1-intrinsics/acle_sve2p1_dot.c
  clang/test/Sema/aarch64-sve2p1-intrinsics/acle_sve2p1_imm.cpp

Index: clang/test/Sema/aarch64-sve2p1-intrinsics/acle_sve2p1_imm.cpp
===
--- clang/test/Sema/aarch64-sve2p1-intrinsics/acle_sve2p1_imm.cpp
+++ clang/test/Sema/aarch64-sve2p1-intrinsics/acle_sve2p1_imm.cpp
@@ -129,3 +129,12 @@
   svcntp_c14(c, 3); // expected-error {{argument should be a multiple of 2}}
 }
 
+void test_svdot_lane_2way(svint32_t s32, svuint32_t u32, svint16_t s16, svuint16_t u16,
+  svfloat32_t f32, svfloat16_t f16) {
+  svdot_lane_s32_s16_s16(s32, s16, s16, 1); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  svdot_lane_u32_u16_u16(u32, u16, u16, 1); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  svdot_lane_f32_f16_f16(f32, f16, f16, 1); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  svdot_lane_s32_s16_s16(s32, s16, s16, 4); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  svdot_lane_u32_u16_u16(u32, u16, u16, 4); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  svdot_lane_f32_f16_f16(f32, f16, f16, 4); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+}
Index: clang/test/CodeGen/aarch64-sve2p1-intrinsics/acle_sve2p1_dot.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2p1-intrinsics/acle_sve2p1_dot.c
@@ -0,0 +1,107 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve2p1 -S -disable-O0-optnone -Werror -Wall -emit-llvm -o - %s | opt -S -p mem2reg,instcombine,tailcallelim | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve2p1 -S -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ %s | opt -S -p mem2reg,instcombine,tailcallelim | FileCheck %s -check-prefix=CPP-CHECK
+// RUN: %clang_cc1 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2p1 -S -disable-O0-optnone -Werror -Wall -emit-llvm -o - %s | opt -S -p mem2reg,instcombine,tailcallelim | FileCheck %s
+// RUN: %clang_cc1 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2p1 -S -disable-O0-optnone -Werror -Wall -emit-llvm -o - -x c++ %s | opt -S -p mem2reg,instcombine,tailcallelim | FileCheck %s -check-prefix=CPP-CHECK
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve2p1 -S -disable-O0-optnone -Werror -Wall -o /dev/null %s
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3) A1##A2##A3
+#endif
+
+// CHECK-LABEL: @test_svdot_s32_x2(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  @llvm.aarch64.sve.sdot.x2.nxv4i32( [[OP1:%.*]],  [[OP2:%.*]],  [[OP3:%.*]])
+// CHECK-NEXT:ret  [[TMP0]]
+//
+// CPP-CHECK-LABEL: @_Z17test_svdot_s32_x2u11__SVInt32_tu11__SVInt16_tu11__SVInt16_t(
+// CPP-CHECK-NEXT:  entry:
+// CPP-CHECK-NEXT:[[TMP0:%.*]] = tail call  @llvm.aarch64.sve.sdot.x2.nxv4i32( [[OP1:%.*]],  [[OP2:%.*]],  [[OP3:%.*]])
+// CPP-CHECK-NEXT:ret  [[TMP0]]
+//
+svint32_t test_svdot_s32_x2(svint32_t op1, svint16_t op2, svint16_t op3)
+{
+  return SVE_ACLE_FUNC(svdot,_s32_s16_s16,)(op1, op2, op3);
+}
+
+// CHECK-LABEL: @test_svdot_u32_x2(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  @llvm.aarch64.sve.udot.x2.nxv4i32( [[OP1:%.*]],  [[OP2:%.*]],  [[OP3:%.*]])
+// CHECK-NEXT:ret  [[TMP0]]
+//
+// CPP-CHECK-LABEL: @_Z17test_svdot_u32_x2u12__SVUint32_tu12__SVUint16_tu12__SVUint16_t(
+// CPP-CHECK-NEXT:  entry:
+// CPP-CHECK-NEXT:[[TMP0:%.*]] = tail call  @llvm.aarch64.sve.udot.x2.nxv4i32( [[OP1:%.*]],  [[OP2:%.*]],  [[OP3:%.*]])
+// CPP-CHECK-NEXT:ret  [[TMP0]]
+//
+svuint32_t test_svdot_u32_x2(svuint32_t op1, svuint16_t op2, svuint16_t op3)
+{
+  return SVE_ACLE_FUNC(svdot,_u32_u16_u16,)(op1, op2, op3);
+}
+
+// CHECK-LABEL: @test_svdot_f32_x2(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = tail call  @llvm.aarch64.sve.fdot.x2.nxv4f32( [[OP1:%.*]],  [[OP2:%.*]],  [[OP3:%.*]])
+// CHECK-NEXT:ret  [[TMP0]]
+//
+// CPP-CHECK-LABEL: @_Z17test_svdot_f32_x2u13__SVFloat32_tu13__SVFloat16

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Michael Maitland via Phabricator via cfe-commits]

michaelmaitland added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:162
+  bool hasVInstructionsF16Mininal() const {
+return HasStdExtZvfhmin || HasStdExtZvfh;
+  }

jacquesguan wrote:
> craig.topper wrote:
> > Doesn't HasStdExtZvfh already imply HasStdExtZvfhmin?
> The v spec doesn't metion this.
I think the spec conveys this when it says `The Zvfhmin extension depends on 
the Zve32f extension.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

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

[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 525581.
VitaNuo added a comment.

Move the speller out of the loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150185

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp

Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -12,6 +12,7 @@
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -24,8 +25,12 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Registry.h"
 #include 
 
+LLVM_INSTANTIATE_REGISTRY(clang::include_cleaner::IncludeSpellingStrategy)
+
 namespace clang::include_cleaner {
 
 void walkUsed(llvm::ArrayRef ASTRoots,
@@ -54,20 +59,32 @@
 }
 
 std::string spellHeader(const Header &H, HeaderSearch &HS,
-const FileEntry *Main) {
-  switch (H.kind()) {
-  case Header::Physical: {
-bool IsSystem = false;
-std::string Path = HS.suggestPathToFileForDiagnostics(
-H.physical(), Main->tryGetRealPathName(), &IsSystem);
-return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
-  }
-  case Header::Standard:
+const FileEntry *Main,
+const IncludeSpeller *CustomSpeller) {
+  if (H.kind() == Header::Standard) {
 return H.standard().name().str();
-  case Header::Verbatim:
+  }
+
+  if (H.kind() == Header::Verbatim) {
 return H.verbatim().str();
   }
-  llvm_unreachable("Unknown Header kind");
+
+  if (H.kind() != Header::Physical)
+llvm_unreachable("Unknown Header kind");
+
+  // Spelling physical headers allows for various plug-in strategies.
+  std::string FinalSpelling;
+  if (CustomSpeller)
+FinalSpelling = (*CustomSpeller)(H.physical()->tryGetRealPathName());
+
+  if (!FinalSpelling.empty())
+return FinalSpelling;
+
+  // Fallback to default spelling via header search.
+  bool IsSystem = false;
+  std::string Path = HS.suggestPathToFileForDiagnostics(
+  H.physical(), Main->tryGetRealPathName(), &IsSystem);
+  return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
 }
 
 AnalysisResults analyze(llvm::ArrayRef ASTRoots,
@@ -89,8 +106,11 @@
}
  }
  if (!Satisfied && !Providers.empty() &&
- Ref.RT == RefType::Explicit)
-   Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+ Ref.RT == RefType::Explicit) {
+   ApplyFirstIncludeSpeller Speller;
+   Missing.insert(
+   spellHeader(Providers.front(), HS, MainFile, &Speller));
+ }
});
 
   AnalysisResults Results;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -14,11 +14,14 @@
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/Format/Format.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/MemoryBufferRef.h"
-#include 
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+#include 
+#include 
 
 namespace clang {
 class SourceLocation;
@@ -75,9 +78,6 @@
 std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
 const format::FormatStyle &IncludeStyle);
 
-std::string spellHeader(const Header &H, HeaderSearch &HS,
-const FileEntry *Main);
-
 /// Gets all the providers for a symbol by traversing each location.
 /// Returned headers are sorted by relevance, first element is the most
 /// likely provider for the symbol.
@@ -85,6 +85,39 @@
const SourceManager &SM,
const PragmaIncludes *PI);
 
+class IncludeSpeller {
+public:
+  virtual ~IncludeSpeller() = default;
+
+  virtual std::string operator()(llvm::StringRef HeaderPhysicalPath) const = 0;
+};
+
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+const FileEntry *Main,
+

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Yurong via Phabricator via cfe-commits]

yronglin added a comment.

ping～ @erichkeane


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

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

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Michael Maitland via Phabricator via cfe-commits]

michaelmaitland added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:162
+  bool hasVInstructionsF16Mininal() const {
+return HasStdExtZvfhmin || HasStdExtZvfh;
+  }

michaelmaitland wrote:
> jacquesguan wrote:
> > craig.topper wrote:
> > > Doesn't HasStdExtZvfh already imply HasStdExtZvfhmin?
> > The v spec doesn't metion this.
> I think the spec conveys this when it says `The Zvfhmin extension depends on 
> the Zve32f extension.`
My mistake, that says `Zve32f`, not `Zvfh`. However, the spec does say:

`When the Zvfhmin extension is implemented, the vfwcvt.f.f.v and vfncvt.f.f.w 
instructions become defined when SEW=16` and also says `When the Zvfh extension 
is implemented, all instructions in Sections Vector Floating-Point 
Instructions.` Since `vfwcvt.f.f.v and vfncvt.f.f.w` are part of `Vector 
Floating-Point Instructions` section, this is how it is implied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

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

[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, LGTM!




Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:136
+// Removes matching instances of given token preceeding the function defition.
+llvm::Expected
+getDelKeywordReplacements(const FunctionDecl *FD,

bgluzman wrote:
> This was extracted from the `DelKeyword` lambda in `getFunctionSourceCode` 
> since it now is used within `apply` as well.
> 
> Note that since this returns a `tooling::Replacements`, the caller must 
> combine the result with other `Replacement`s via 
> `tooling::Replacements::merge`. This is my first time contributing to this 
> project, so please let me know if using `merge` (instead of `add`) could 
> cause performance issues. I can change this so that we `add` to a 
> `tooling::Replacements` out-parameter, instead.
thanks `merge` is fine, we execute this code path only when the user tries to 
apply the code action. hence it's performance is not as critical as `prepare`.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:137
+llvm::Expected
+getDelKeywordReplacements(const FunctionDecl *FD,
+  const syntax::TokenBuffer &TokBuf,

let's just drop the `FD` from signature, you can get `SourceManager` from 
`syntax::TokenBuffer`.

also can you update the documentation, we should rather say:
```
Returns replacements to delete tokens with kind `Kind` in the range `FromRange`.
```

nit: s/getDelKeywordReplacements/deleteTokensWithKind



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:489
+tooling::Replacements HeaderUpdates;
 const tooling::Replacement DeleteFuncBody(
 Sel.AST->getSourceManager(),

you can directly construct `Replacements` with this particular `Replacement` 
first, that way no need to `add` and check for `Err`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151294

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

[PATCH] D151365: [Sema] cast to CXXRecordDecl correctly when diag a default comparison method

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:969
 << (int)DFK.asComparison()
-<< Context.getTagDeclType(
-   cast(FD->getLexicalDeclContext()));
+<< Context.getRecordType(RecordType->getAsRecordDecl());
   }

What is going on here?  You already have the `RecordType` in the RecordType 
variable.  You don't need to go to RecordDecl and back?  You should just be 
able to put the `RecordType` variable directly.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151365

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

[PATCH] D151194: [clang][dataflow] Add support for return values of reference type.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:196
+  void popCall(const CallExpr *Call, const Environment &CalleeEnv);
+  void popCall(const CXXConstructExpr *Call, const Environment &CalleeEnv);
 

mboehme wrote:
> xazax.hun wrote:
> > I know that Obj-C is a non-goal, but it might worth a comment to support 
> > `ObjCMessageExpr` just in case someone wants to work on this. 
> > 
> > Btw, this is one of my biggest pet peeves about the Clang AST. We should 
> > have a common abstraction for all the callables, instead of having to 
> > bifurcate many of the APIs. 
> Agreed!
> 
> I'm a bit hesitant to add a FIXME here though, as that might make it sound as 
> if there's a plan to add Objective-C support or at least imply that that's a 
> goal. Also, I think Objective-C support would entail quite a bit more than 
> adding the right overloads here -- so I'm not sure how helpful the comment 
> here would be. I think once someone goes down the road of seriously working 
> on Objective-C support, they'll quickly discover that they need a new 
> overload here.
> 
> I hope this makes sense? Happy to add something in a followup patch if you 
> feel we should add some Objective-C "breadcrumbs" here.
Yup, makes sense!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151194

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

[PATCH] D151445: [Flang] Add main-file-name flag to flang -fc1

[Dominik Adamski via Phabricator via cfe-commits]

domada created this revision.
domada added reviewers: awarzynski, kiranchandramohan, jsjodin, kiranktp, 
agozillon, skatrak, TIFitis, RogerV-AMD, NimishMishra, raghavendhra, dpalermo.
domada added a project: Flang.
Herald added subscribers: bviyer, sunshaoce, Moerafaat, zero9178, bzcheeseman, 
sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, 
jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, jdoerfert.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: dcaballe.
Herald added a project: All.
domada requested review of this revision.
Herald added subscribers: cfe-commits, stephenneuendorffer, nicolasvasilache, 
MaskRay.
Herald added a reviewer: nicolasvasilache.
Herald added projects: clang, MLIR.

Flag main-file-name is used to store information about original input file. 
Information about original input file is used for:

1. Providing information about original input Fortran file for *mlir files and 
*.bc files
2. Determining the name of offload function for omp target pragma


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151445

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help.f90
  flang/test/Driver/frontend-forwarding.f90
  flang/test/Driver/mllvm.f90
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -1585,11 +1585,12 @@
 
 static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
  omp::TargetOp targetOp,
- llvm::StringRef parentName = "") {
+ llvm::StringRef parentName = "",
+ llvm::StringRef hostFileName = "") {
   auto fileLoc = targetOp.getLoc()->findInstanceOf();
-
   assert(fileLoc && "No file found from location");
-  StringRef fileName = fileLoc.getFilename().getValue();
+  StringRef fileName =
+  (!hostFileName.empty()) ? hostFileName : fileLoc.getFilename().getValue();
 
   llvm::sys::fs::UniqueID id;
   if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
@@ -1660,9 +1661,12 @@
 
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   StringRef parentName = opInst.getParentOfType().getName();
+  StringRef fileName = "";
   llvm::TargetRegionEntryInfo entryInfo;
+  auto moduleOp = opInst.getParentOfType();
+  fileName = moduleOp.getName().value_or("");
 
-  if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
+  if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName, fileName))
 return failure();
 
   int32_t defaultValTeams = -1;
Index: flang/test/Driver/mllvm.f90
===
--- flang/test/Driver/mllvm.f90
+++ flang/test/Driver/mllvm.f90
@@ -2,14 +2,14 @@
 
 ! 1. Test typical usage.
 ! RUN: %flang -S -mllvm -print-before-all %s -o - 2>&1 | FileCheck %s --check-prefix=OUTPUT
-! RUN: %flang_fc1 -S -mllvm -print-before-all %s -o - 2>&1 | FileCheck %s --check-prefix=OUTPUT
+! RUN: %flang_fc1 -S -mllvm -print-before-all -main-file-name mllvm.f90 %s -o - 2>&1 | FileCheck %s --check-prefix=OUTPUT
 
 ! 2. Test invalid usage (`-print-before` requires an argument)
 ! RUN: not %flang -S -mllvm -print-before %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID_USAGE
 
 ! OUTPUT: *** IR Dump Before Pre-ISel Intrinsic Lowering (pre-isel-intrinsic-lowering) ***
-! OUTPUT-NEXT: ; ModuleID = 'FIRModule'
-! OUTPUT-NEXT: source_filename = "FIRModule"
+! OUTPUT-NEXT: ; ModuleID = 'mllvm.f90'
+! OUTPUT-NEXT: source_filename = "mllvm.f90"
 
 ! INVALID_USAGE: flang (LLVM option parsing): for the --print-before option: requires a value!
 
Index: flang/test/Driver/frontend-forwarding.f90
===
--- flang/test/Driver/frontend-forwarding.f90
+++ flang/test/Driver/frontend-forwarding.f90
@@ -22,6 +22,7 @@
 ! RUN: -P \
 ! RUN:   | FileCheck %s
 
+! CHECK: "-main-file-name" "frontend-forwarding.f90"
 ! CHECK: "-P"
 ! CHECK: "-finput-charset=utf-8"
 ! CHECK: "-fdefault-double-8"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -174,6 +174,7 @@
 ! HELP-FC1-NEXT: -init-only Only execute frontend initialization
 ! HELP-FC1-NEXT: -IAdd directory to the end of the list of 

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Just 1 change I'd like, but otherwise this LGTM.




Comment at: clang/lib/AST/AttrImpl.cpp:244
+  assert(!isAlignmentDependent());
+  auto getAlignmentImpl = [&]() -> unsigned {
+if (isAlignmentExpr()) {

I don't think it makes sense to make this a lambda here rather than just 
implementing it inline if the value isn't cached.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

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

[PATCH] D148702: [clang] Add Parse and Sema support for RegularKeyword attributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So I have 1 issue that is throughout the patch, but I'm ok with the patch 
otherwise.  This'll get a LGTM after that is fixed.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2902
 S.Diag(AL.getRange().getBegin(), diag::err_attribute_wrong_decl_type)
-<< AL << ExpectedTypeOrNamespace;
+<< AL << 0 << ExpectedTypeOrNamespace;
 return;

Every where you are doing just a '0' in a diagnostic here it makes it 
incredibly unreadable.  I'd prefer 1 of 2 solutions:

1- Create an enum somewhere that encodes the meaning here, and use those 
instead.
2- use a `/*Whatever*/` comment every place you're passing a raw literal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148702

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

[PATCH] D151438: [NFC][Driver] Change Multilib flag representation

[Michael Platings via Phabricator via cfe-commits]

michaelplatings added a comment.

@phosek you made some alternative suggestions:

1. Have two separate lists of include and exclude flags.
2. Store flags as a tuple (or a struct?) of string and a tag (that is include 
or exclude).

The reason for not doing that is to keep multilib selection method conceptually 
simple: a multilib is a match if its flags are a subset of flags generated from 
command line arguments. Although historically the Multilib class has had the 
concept of flags being indicated or contraindicated, I've found that's not 
needed for the proposed configurable multilib scheme. Therefore I'd prefer not 
to codify that concept within the Multilib class any more than it already is. 
Over time I expect "contraindicated" flags like `!fexceptions` to be used less 
and less, in favour of real flags like `-fno-exceptions`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151438

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

[clang] 7f6c3a9 - [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-05-25T14:47:03Z
New Revision: 7f6c3a9033b6409ada46609f5f4b650e8556f56d

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

LOG: [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

This patch adds a test that crashes without the fix.

Reviewed By: ymandel

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

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 a67f9ceed82e..8547e5049261 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -194,24 +194,13 @@ class TransferVisitor : public 
ConstStmtVisitor {
   auto &Loc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, Loc);
 
-  BoolValue *LHSVal = getLogicOperatorSubExprValue(*LHS);
-  // If the LHS was not reachable, this BinaryOperator would also not be
-  // reachable, and we would never get here.
-  assert(LHSVal != nullptr);
-  BoolValue *RHSVal = getLogicOperatorSubExprValue(*RHS);
-  if (RHSVal == nullptr) {
-// If the RHS isn't reachable and we evaluate this BinaryOperator,
-// then the value of the LHS must have triggered the short-circuit
-// logic. This implies that the value of the entire expression must be
-// equal to the value of the LHS.
-Env.setValue(Loc, *LHSVal);
-break;
-  }
+  BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
+  BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);
 
   if (S->getOpcode() == BO_LAnd)
-Env.setValue(Loc, Env.makeAnd(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
   else
-Env.setValue(Loc, Env.makeOr(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
   break;
 }
 case BO_NE:
@@ -805,35 +794,29 @@ class TransferVisitor : public 
ConstStmtVisitor {
   }
 
 private:
-  /// If `SubExpr` is reachable, returns a non-null pointer to the value for
-  /// `SubExpr`. If `SubExpr` is not reachable, returns nullptr.
-  BoolValue *getLogicOperatorSubExprValue(const Expr &SubExpr) {
+  /// Returns the value for the sub-expression `SubExpr` of a logic operator.
+  BoolValue &getLogicOperatorSubExprValue(const Expr &SubExpr) {
 // `SubExpr` and its parent logic operator might be part of 
diff erent basic
 // blocks. We try to access the value that is assigned to `SubExpr` in the
 // corresponding environment.
-const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr);
-if (!SubExprEnv)
-  return nullptr;
-
-if (auto *Val =
-dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
-  return Val;
-
-if (Env.getValueStrict(SubExpr) == nullptr) {
-  // Sub-expressions that are logic operators are not added in basic blocks
-  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-  // operator, it may not have been evaluated and assigned a value yet. In
-  // that case, we need to first visit `SubExpr` and then try to get the
-  // value that gets assigned to it.
+if (const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr))
+  if (auto *Val =
+  dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
+return *Val;
+
+// The sub-expression may lie within a basic block that isn't reachable,
+// even if we need it to evaluate the current (reachable) expression
+// (see https://discourse.llvm.org/t/70775). In this case, visit `SubExpr`
+// within the current environment and then try to get the value that gets
+// assigned to it.
+if (Env.getValueStrict(SubExpr) == nullptr)
   Visit(&SubExpr);
-}
-
 if (auto *Val = dyn_cast_or_null(Env.getValueStrict(SubExpr)))
-  return Val;
+  return *Val;
 
 // If the value of `SubExpr` is still unknown, we create a fresh symbolic
 // boolean value for it.
-return &Env.makeAtomicBoolValue();
+return Env.makeAtomicBoolValue();
   }
 
   // If context sensitivity is enabled, try to analyze the body of the callee

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 726f2b7bbdcc..1a2442f0b12d 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5140,6 +5140,26 @@ TEST(TransferTest, UnnamedBitfieldInitializer) {
   });
 }
 
+// Repro for a crash that used to occur with chained short-circuiting logical
+// 

[PATCH] D151201: [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
mboehme marked an inline comment as done.
Closed by commit rG7f6c3a9033b6: [clang][dataflow] Fix a crash in 
`getLogicOperatorSubExprValue()`. (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151201

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
@@ -5140,6 +5140,26 @@
   });
 }
 
+// Repro for a crash that used to occur with chained short-circuiting logical
+// operators.
+TEST(TransferTest, ChainedLogicalOps) {
+  std::string Code = R"(
+bool target() {
+  bool b = true || false || false || false;
+  // [[p]]
+  return b;
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+auto &B = getValueForDecl(ASTCtx, Env, "b");
+EXPECT_TRUE(Env.flowConditionImplies(B));
+  });
+}
+
 // Repro for a crash that used to occur when we call a `noreturn` function
 // within one of the operands of a `&&` or `||` operator.
 TEST(TransferTest, NoReturnFunctionInsideShortCircuitedBooleanOp) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -194,24 +194,13 @@
   auto &Loc = Env.createStorageLocation(*S);
   Env.setStorageLocation(*S, Loc);
 
-  BoolValue *LHSVal = getLogicOperatorSubExprValue(*LHS);
-  // If the LHS was not reachable, this BinaryOperator would also not be
-  // reachable, and we would never get here.
-  assert(LHSVal != nullptr);
-  BoolValue *RHSVal = getLogicOperatorSubExprValue(*RHS);
-  if (RHSVal == nullptr) {
-// If the RHS isn't reachable and we evaluate this BinaryOperator,
-// then the value of the LHS must have triggered the short-circuit
-// logic. This implies that the value of the entire expression must be
-// equal to the value of the LHS.
-Env.setValue(Loc, *LHSVal);
-break;
-  }
+  BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
+  BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);
 
   if (S->getOpcode() == BO_LAnd)
-Env.setValue(Loc, Env.makeAnd(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
   else
-Env.setValue(Loc, Env.makeOr(*LHSVal, *RHSVal));
+Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
   break;
 }
 case BO_NE:
@@ -805,35 +794,29 @@
   }
 
 private:
-  /// If `SubExpr` is reachable, returns a non-null pointer to the value for
-  /// `SubExpr`. If `SubExpr` is not reachable, returns nullptr.
-  BoolValue *getLogicOperatorSubExprValue(const Expr &SubExpr) {
+  /// Returns the value for the sub-expression `SubExpr` of a logic operator.
+  BoolValue &getLogicOperatorSubExprValue(const Expr &SubExpr) {
 // `SubExpr` and its parent logic operator might be part of different basic
 // blocks. We try to access the value that is assigned to `SubExpr` in the
 // corresponding environment.
-const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr);
-if (!SubExprEnv)
-  return nullptr;
-
-if (auto *Val =
-dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
-  return Val;
-
-if (Env.getValueStrict(SubExpr) == nullptr) {
-  // Sub-expressions that are logic operators are not added in basic blocks
-  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-  // operator, it may not have been evaluated and assigned a value yet. In
-  // that case, we need to first visit `SubExpr` and then try to get the
-  // value that gets assigned to it.
+if (const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr))
+  if (auto *Val =
+  dyn_cast_or_null(SubExprEnv->getValueStrict(SubExpr)))
+return *Val;
+
+// The sub-expression may lie within a basic block that isn't reachable,
+// even if we need it to evaluate the current (reachable) expression
+// (see https://discourse.llvm.org/t/70775). In this case, visit `SubExpr`
+// within the current environment and then try to get the value that gets
+// assigned to it.
+if (Env.getValueStrict(SubExpr) == nullptr)
   Visit(&SubExpr);
-}
-
 if (auto *Val = dyn_cast_or_null(Env.getValueStrict(SubExpr)))
-  return Val;
+  return *Val;
 
 // If the value of `SubExpr` is still 

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 525614.
yronglin added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,21 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName()
+ << "Expr)";
+  OS << "  " << getLowerName()
  << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName()
+ << "Type)";
+  OS << "  " << getLowerName()
+ << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-a

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Yurong via Phabricator via cfe-commits]

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

Thanks for your comments, @erichkeane !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

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

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Erich Keane via Phabricator via cfe-commits]

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

1 nit I don't care either way about, 1 suggestion.




Comment at: clang/lib/AST/AttrImpl.cpp:247
+
+  if (isAlignmentExpr()) {
+return alignmentExpr

Typically we don't do curleys on a single-statement... but I am on the fence 
due to how complicated it is below.



Comment at: clang/lib/AST/AttrImpl.cpp:259
+  // alignment of the referenced type.
+  if (const ReferenceType *Ref = T->getAs())
+T = Ref->getPointeeType();

You can replace this 'if' with: `T = T->getNonReferenceType();`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

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

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:162
+  bool hasVInstructionsF16Mininal() const {
+return HasStdExtZvfhmin || HasStdExtZvfh;
+  }

michaelmaitland wrote:
> michaelmaitland wrote:
> > jacquesguan wrote:
> > > craig.topper wrote:
> > > > Doesn't HasStdExtZvfh already imply HasStdExtZvfhmin?
> > > The v spec doesn't metion this.
> > I think the spec conveys this when it says `The Zvfhmin extension depends 
> > on the Zve32f extension.`
> My mistake, that says `Zve32f`, not `Zvfh`. However, the spec does say:
> 
> `When the Zvfhmin extension is implemented, the vfwcvt.f.f.v and vfncvt.f.f.w 
> instructions become defined when SEW=16` and also says `When the Zvfh 
> extension is implemented, all instructions in Sections Vector Floating-Point 
> Instructions.` Since `vfwcvt.f.f.v and vfncvt.f.f.w` are part of `Vector 
> Floating-Point Instructions` section, this is how it is implied.
It's implemented in LLVM by this patch https://reviews.llvm.org/D150016


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

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

[PATCH] D141627: [Clang][OpenMP] Fix the issue that list items in `has_device_addr` are still mapped to the target device

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 525627.
tianshilei1992 added a comment.

add a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141627

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/bug59160.c

Index: clang/test/OpenMP/bug59160.c
===
--- /dev/null
+++ clang/test/OpenMP/bug59160.c
@@ -0,0 +1,175 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --check-globals --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp -fopenmp-version=51 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -DCK1 -verify -fopenmp-simd -fopenmp-version=51 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// expected-no-diagnostics
+
+void zoo(void) {
+  short x[10];
+  short *(xp[10]);
+  xp[1] = &x[0];
+  short **xpp = &xp[0];
+  x[1] = 111;
+#pragma omp target data map(tofrom: xpp[1][1]) use_device_addr(xpp[1][1])
+#pragma omp target has_device_addr(xpp[1][1])
+  {
+xpp[1][1] = 222;
+  }
+}
+//.
+// CHECK: @.offload_sizes = private unnamed_addr constant [2 x i64] [i64 8, i64 2]
+// CHECK: @.offload_maptypes = private unnamed_addr constant [2 x i64] [i64 67, i64 19]
+// CHECK: @0 = private unnamed_addr constant [23 x i8] c"
+// CHECK: @1 = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 22, ptr @0 }, align 8
+// CHECK: @.__omp_offloading_34_735f4a3a_zoo_l13.region_id = weak constant i8 0
+// CHECK: @.offload_sizes.1 = private unnamed_addr constant [1 x i64] [i64 8]
+// CHECK: @.offload_maptypes.2 = private unnamed_addr constant [1 x i64] [i64 288]
+// CHECK: @.omp_offloading.entry_name = internal unnamed_addr constant [37 x i8] c"__omp_offloading_34_735f4a3a_zoo_l13\00"
+// CHECK: @.omp_offloading.entry.__omp_offloading_34_735f4a3a_zoo_l13 = weak constant %struct.__tgt_offload_entry { ptr @.__omp_offloading_34_735f4a3a_zoo_l13.region_id, ptr @.omp_offloading.entry_name, i64 0, i32 0, i32 0 }, section "omp_offloading_entries", align 1
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @.omp_offloading.requires_reg, ptr null }]
+//.
+// CHECK-LABEL: define {{[^@]+}}@zoo
+// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[X:%.*]] = alloca [10 x i16], align 2
+// CHECK-NEXT:[[XP:%.*]] = alloca [10 x ptr], align 8
+// CHECK-NEXT:[[XPP:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS7:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS8:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS9:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[KERNEL_ARGS:%.*]] = alloca [[STRUCT___TGT_KERNEL_ARGUMENTS:%.*]], align 8
+// CHECK-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds [10 x i16], ptr [[X]], i64 0, i64 0
+// CHECK-NEXT:[[ARRAYIDX1:%.*]] = getelementptr inbounds [10 x ptr], ptr [[XP]], i64 0, i64 1
+// CHECK-NEXT:store ptr [[ARRAYIDX]], ptr [[ARRAYIDX1]], align 8
+// CHECK-NEXT:[[ARRAYIDX2:%.*]] = getelementptr inbounds [10 x ptr], ptr [[XP]], i64 0, i64 0
+// CHECK-NEXT:store ptr [[ARRAYIDX2]], ptr [[XPP]], align 8
+// CHECK-NEXT:[[ARRAYIDX3:%.*]] = getelementptr inbounds [10 x i16], ptr [[X]], i64 0, i64 1
+// CHECK-NEXT:store i16 111, ptr [[ARRAYIDX3]], align 2
+// CHECK-NEXT:[[TMP0:%.*]] = load ptr, ptr [[XPP]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load ptr, ptr [[XPP]], align 8
+// CHECK-NEXT:[[ARRAYIDX4:%.*]] = getelementptr inbounds ptr, ptr [[TMP1]], i64 1
+// CHECK-NEXT:[[TMP2:%.*]] = load ptr, ptr [[XPP]], align 8
+// CHECK-NEXT:[[ARRAYIDX5:%.*]] = getelementptr inbounds ptr, ptr [[TMP2]], i64 1
+// CHECK-NEXT:[[TMP3:%.*]] = load ptr, ptr [[ARRAYIDX5]], align 8
+// CHECK-NEXT:[[ARRAYIDX6:%.*]] = getelementptr inbounds i16, ptr [[TMP3]], i64 1
+// CHECK-NEXT:[[TMP4:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 0
+// CHECK-NEXT:store ptr [[TMP0]], ptr [[TMP4]], align 8
+// CHECK-NEXT:[[TMP5:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 0
+// CHECK-NEXT:store ptr [[ARRAYIDX4]], ptr [[TMP5]], align 8
+// CHECK-NEXT:[[TMP6:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 0
+// CHECK-NEXT:store ptr null, ptr [[TMP6]], align 8
+// C

[PATCH] D141627: [Clang][OpenMP] Fix the issue that list items in `has_device_addr` are still mapped to the target device

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 accepted this revision.
jyu2 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/D141627/new/

https://reviews.llvm.org/D141627

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

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

[Ben J via Phabricator via cfe-commits]

Icantjuddle added a comment.

In D151145#4369406 , @MyDeveloperDay 
wrote:

> We don't normally land broken tests, even if they are disabled, its better 
> for us if we get a fix at the same time ;-)



In D151145#4367495 , @krasimir wrote:

> ...
> @Icantjuddle would you like to update this patch to fix the issue and enable 
> your newly added tests?

Sounds like we don't really want this without a real fix; I'll give it a shot, 
hopefully won't be too bad. Thanks for the code pointer.

In D151145#4365580 , 
@HazardyKnusperkeks wrote:

> As stated on Github I don't think we have a bug.

Are you sure, I'm pretty sure this is a bug? Can you elaborate on the 
discussion on github. I wouldn't want to fix something that isn't broken. If 
this is indeed expected maybe I can update the docs.




Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:92-99
 Style.RawStringFormats = {
 {
 /*Language=*/FormatStyle::LK_Cpp,
 /*Delimiters=*/{"cpp"},
 /*EnclosingFunctions=*/{},
 /*CanonicalDelimiter=*/"",
 BasedOnStyle,

HazardyKnusperkeks wrote:
> Couldn't you just set the style like you want, instead of parsing yaml?
Given that the problem seems to be with how the configs are parsed/processed, I 
wanted the repro to be most faithful to the user facing issue.



Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:177
+format(R"test(
+t = R"pb(item:1)pb";)test",
+   getRawStringPbStyleSeparateSection()));

HazardyKnusperkeks wrote:
> EXPECT_EQ
I used the function instead of the macro to conform to the convention of every 
other test in the file. 

The function provides its own justification.
```
  // Gcc 4.8 doesn't support raw string literals in macros, which breaks some
  // build bots. We use this function instead.
  void expect_eq(const std::string Expected, const std::string Actual) {
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151145

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

[PATCH] D151060: [Clang][Sema] Generate vector vs scalar builtin overloads

[Cassie Jones via Phabricator via cfe-commits]

porglezomp added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9041-9044
+  // (allowing splatting the scalar to a vector).
+  for (unsigned Candidate = 0; Candidate < 2; ++Candidate) {
+for (QualType Vec1Ty : CandidateTypes[Candidate].vector_types()) {
+  for (QualType Vec2Ty : CandidateTypes[Candidate].vector_types()) {

aaron.ballman wrote:
> I'm a bit confused -- the comment says this is to allow splatting the scalar 
> to a vector, but... what is the scalar type in these loops?
Right, so my goal here is to generate the two vector-operand overload 
candidates so that overload resolution can pick one of those and then apply the 
implicit scalar->vector conversion with the splat. I //think// this is the 
correct approach (it's based on how the compound assignment operators already 
handle this) but I need to improve my comments here.

Also, how do we like the loop here for trying both sides, as opposed to 
duplicating the code? This is really acting as an `if left operand / else if 
right operand` to avoid duplicating the code but I was worried writing it that 
the loop obscures the intent. Maybe that's also just "make the comment better" 
or maybe that part is clear enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151060

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

[PATCH] D150114: [Headers][doc] Add "add/sub/mul" intrinsic descriptions to avx2intrin.h

[Paul Robinson via Phabricator via cfe-commits]

probinson updated this revision to Diff 525632.
probinson added a comment.

Correct order of horizontal operands


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

https://reviews.llvm.org/D150114

Files:
  clang/lib/Headers/avx2intrin.h

Index: clang/lib/Headers/avx2intrin.h
===
--- clang/lib/Headers/avx2intrin.h
+++ clang/lib/Headers/avx2intrin.h
@@ -65,48 +65,150 @@
   return (__m256i) __builtin_ia32_packusdw256((__v8si)__V1, (__v8si)__V2);
 }
 
+/// Adds 8-bit integers from corresponding bytes of two 256-bit integer
+///vectors and returns the lower 8 bits of each sum in the corresponding
+///byte of the 256-bit integer vector result (overflow is ignored).
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDB instruction.
+///
+/// \param __a
+///A 256-bit integer vector containing one of the source operands.
+/// \param __b
+///A 256-bit integer vector containing one of the source operands.
+/// \returns A 256-bit integer vector containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_add_epi8(__m256i __a, __m256i __b)
 {
   return (__m256i)((__v32qu)__a + (__v32qu)__b);
 }
 
+/// Adds 16-bit integers from corresponding elements of two 256-bit vectors of
+///[16 x i16] and returns the lower 16 bits of each sum in the
+///corresponding element of the [16 x i16] result (overflow is ignored).
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDW instruction.
+///
+/// \param __a
+///A 256-bit vector of [16 x i16] containing one of the source operands.
+/// \param __b
+///A 256-bit vector of [16 x i16] containing one of the source operands.
+/// \returns A 256-bit vector of [16 x i16] containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_add_epi16(__m256i __a, __m256i __b)
 {
   return (__m256i)((__v16hu)__a + (__v16hu)__b);
 }
 
+/// Adds 32-bit integers from corresponding elements of two 256-bit vectors of
+///[8 x i32] and returns the lower 32 bits of each sum in the corresponding
+///element of the [8 x i32] result (overflow is ignored).
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDD instruction.
+///
+/// \param __a
+///A 256-bit vector of [8 x i32] containing one of the source operands.
+/// \param __b
+///A 256-bit vector of [8 x i32] containing one of the source operands.
+/// \returns A 256-bit vector of [8 x i32] containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_add_epi32(__m256i __a, __m256i __b)
 {
   return (__m256i)((__v8su)__a + (__v8su)__b);
 }
 
+/// Adds 64-bit integers from corresponding elements of two 256-bit vectors of
+///[4 x i64] and returns the lower 64 bits of each sum in the corresponding
+///element of the [4 x i64] result (overflow is ignored).
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDQ instruction.
+///
+/// \param __a
+///A 256-bit vector of [4 x i64] containing one of the source operands.
+/// \param __b
+///A 256-bit vector of [4 x i64] containing one of the source operands.
+/// \returns A 256-bit vector of [4 x i64] containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_add_epi64(__m256i __a, __m256i __b)
 {
   return (__m256i)((__v4du)__a + (__v4du)__b);
 }
 
+/// Adds 8-bit integers from corresponding bytes of two 256-bit integer
+///vectors using signed saturation, and returns each sum in the
+///corresponding byte of the 256-bit integer vector result.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDSB instruction.
+///
+/// \param __a
+///A 256-bit integer vector containing one of the source operands.
+/// \param __b
+///A 256-bit integer vector containing one of the source operands.
+/// \returns A 256-bit integer vector containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_adds_epi8(__m256i __a, __m256i __b)
 {
   return (__m256i)__builtin_elementwise_add_sat((__v32qs)__a, (__v32qs)__b);
 }
 
+/// Adds 16-bit integers from corresponding elements of two 256-bit vectors of
+///[16 x i16] using signed saturation, and returns the [16 x i16] result.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPADDSW instruction.
+///
+/// \param __a
+///A 256-bit vector of [16 x i16] containing one of the source operands.
+/// \param __b
+///A 256-bit vector of [16 x i16] containing one of the source operands.
+/// \returns A 256-bit vector of [16 x i16] containing the sums.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_adds_epi16(__m256i __a, __m256i __b)
 {
   return (__m256i)__builtin_elementwise_add_sat((__v16hi)__a, (__v16hi)__b);
 }
 
+/// Adds 8-bit integers from corresponding bytes of two 256-bit integer
+///vectors using unsigned saturation, and returns each sum in the
+///corresponding byte of the 256-bit in

[PATCH] D150114: [Headers][doc] Add "add/sub/mul" intrinsic descriptions to avx2intrin.h

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/Headers/avx2intrin.h:456
+///   j := i*128
+///   result[j+31:j] := __a[j+63:j+32] - __a[j+31:j]
+///   result[j+63:j+32] := __a[j+127:j+96] - __a[j+95:j+64]

craig.topper wrote:
> Intel intrinsics guide says
> 
> ```
> dst[31:0] := a[31:0] - a[63:32]
> dst[63:32] := a[95:64] - a[127:96]
> dst[95:64] := b[31:0] - b[63:32]
> dst[127:96] := b[95:64] - b[127:96]
> dst[159:128] := a[159:128] - a[191:160]
> dst[191:160] := a[223:192] - a[255:224]
> dst[223:192] := b[159:128] - b[191:160]
> dst[255:224] := b[223:192] - b[255:224]
> dst[MAX:256] := 0
> ```
> 
> So I think the operands are in the wrong order here?
Words fail me. Also diagrams. I wanted the add and sub descriptions to look 
similar, and copy-pasted from add to sub without verifying the order.


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

https://reviews.llvm.org/D150114

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Henrik G Olsson via Phabricator via cfe-commits]

hnrklssn added inline comments.



Comment at: 
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:ret i32 [[TMP1]]
+//

nikic wrote:
> hnrklssn wrote:
> > nikic wrote:
> > > nikic wrote:
> > > > hnrklssn wrote:
> > > > > hnrklssn wrote:
> > > > > > nikic wrote:
> > > > > > > hnrklssn wrote:
> > > > > > > > delcypher wrote:
> > > > > > > > > @hnrklssn I just noticed we don't have a `CHECK` for what 
> > > > > > > > > `META2` actually refers to. Should we?
> > > > > > > > > 
> > > > > > > > > Not something that has to be fixed in this patch, more just 
> > > > > > > > > an observation.
> > > > > > > > Indeed this is true for metadata in general, presumably because 
> > > > > > > > the RHS often refer to things like other metadata identifiers. 
> > > > > > > > In the case of annotations they seem to always refer to simple 
> > > > > > > > strings however, so it would be feasible to do a straight match 
> > > > > > > > without having to do recursive matching or complex regexes to 
> > > > > > > > determine which part of the metadata to match against.
> > > > > > > > 
> > > > > > > > In many cases with metadata attached to IR nodes, multiple 
> > > > > > > > nodes refer to the same metadata node, so at least you verify 
> > > > > > > > that they still are consistent. But I agree that verifying the 
> > > > > > > > content would be a great future addition.
> > > > > > > You need to pass `--check-globals` to check the actual metadata.
> > > > > > When I add that to this test case it adds
> > > > > > 
> > > > > > ```
> > > > > > //.
> > > > > > // CHECK: attributes #0 = { noinline nounwind optnone 
> > > > > > "min-legal-vector-width"="0" "no-trapping-math"="true" 
> > > > > > "stack-protector-buffer-size"="8" 
> > > > > > "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
> > > > > > //.
> > > > > > // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
> > > > > > // CHECK: !1 = !{!"clang version 17.0.0 
> > > > > > (g...@github.com:llvm/llvm-project.git 
> > > > > > 684914f47cf59e9ab6d8b0f73c58ca6272ea28d4)"}
> > > > > > // CHECK: !2 = !{!"auto-init"}
> > > > > > //.
> > > > > > ```
> > > > > > 
> > > > > > So it seems to just be doing a simple literal matching on all 
> > > > > > metadata, regardless of whether we captured that metadata in any 
> > > > > > filecheck variable. And it still doesn't use the META2 variable to 
> > > > > > match the definition. Am I missing something? If we use the literal 
> > > > > > metadata names instead of variable matching for the definitions, 
> > > > > > there isn't much point in doing variable matching for the metadata 
> > > > > > uses either, since the test still very much relies on the metadata 
> > > > > > numbering being stable.
> > > > > @nikic Do you have more information to add about how metadata 
> > > > > definition matchers can be generated without hardcoding everything 
> > > > > (which is kind of the opposite of what this patch is trying to do), 
> > > > > or in general if you're happy with the state of the PR?
> > > > This works fine with update_test_checks, so it must be some bug in 
> > > > update_cc_test_checks in particular. From a quick look, I suspect it's 
> > > > because 
> > > > https://github.com/llvm/llvm-project/blob/3d05ab6d3e24e76ff53b8d7d623c436b4be5b809/llvm/utils/update_cc_test_checks.py#L447
> > > >  hardcodes a True value, while update_test_checks makes this dependent 
> > > > on `--preserve-names`, which is disabled by default there.
> > > Or maybe just test this with update_test_checks? This change is valuable 
> > > for that script as well, and it doesn't have this extra issue.
> > I couldn't find any uses of 
> > `--preserve-names` in the entire repo (not even in tests for 
> > update_test_checks), so I went ahead and changed update_cc_test_checks to 
> > pass True instead.
> > 
> > While at it I added 2 new options to match some, but not necessarily all, 
> > globals. I set the least verbose one (other than "none") as the default, 
> > emitting checks for the definitions of all globals that we emit matches for 
> > in the IR. Do you agree that this is reasonable?
> That sounds like a nice approach. My main question would be whether 
> `--check-globals seen` is the right default: My gut feeling here is that this 
> will introduce additional checks for attributes/metadata in tests that don't 
> actually care about it, but I'm not particularly sure about that.
> 
> You might want to do some mass test regeneration and take a look over the 
> diff to see how useful the changes would be.
> 
> If we do change the default, the default change likely need to be part of 
> `--version 3` to avoid substantial test churn.
I've regenerated checks before and after my patch and compared them. Some 
findings:
 - there's a surprising number of custom metadata types that didn't get 
variable checkers but instead checked hardcoded met

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

Sorry I missed the question. What I meant is that a lot of the profile guided 
size optimization (PGSO) done by Hiroshi can be adapted to use the attribute 
based method proposed in the patch as we don't need two different mechanisms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149800

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

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Yurong via Phabricator via cfe-commits]

yronglin updated this revision to Diff 525636.
yronglin added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/AttrImpl.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Sema/aix-attr-aligned-vector-warn.c
  clang/test/Sema/aix-attr-aligned-vector-warn.cpp
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/attr-cxx0x.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp
  clang/test/SemaCXX/sizeless-1.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -508,6 +508,16 @@
   OS << "assert(!is" << getLowerName() << "Expr);\n";
   OS << "return " << getLowerName() << "Type;\n";
   OS << "  }";
+
+  OS << "  std::optional getCached" << getUpperName()
+ << "Value() const {\n";
+  OS << "return " << getLowerName() << "Cache;\n";
+  OS << "  }";
+
+  OS << "  void setCached" << getUpperName()
+ << "Value(unsigned AlignVal) {\n";
+  OS << "" << getLowerName() << "Cache = AlignVal;\n";
+  OS << "  }";
 }
 
 void writeAccessorDefinitions(raw_ostream &OS) const override {
@@ -530,21 +540,6 @@
   OS << "  return " << getLowerName()
  << "Type->getType()->containsErrors();\n";
   OS << "}\n";
-
-  // FIXME: Do not do the calculation here
-  // FIXME: Handle types correctly
-  // A null pointer means maximum alignment
-  OS << "unsigned " << getAttrName() << "Attr::get" << getUpperName()
- << "(ASTContext &Ctx) const {\n";
-  OS << "  assert(!is" << getUpperName() << "Dependent());\n";
-  OS << "  if (is" << getLowerName() << "Expr)\n";
-  OS << "return " << getLowerName() << "Expr ? " << getLowerName()
- << "Expr->EvaluateKnownConstInt(Ctx).getZExtValue()"
- << " * Ctx.getCharWidth() : "
- << "Ctx.getTargetDefaultAlignForAttributeAligned();\n";
-  OS << "  else\n";
-  OS << "return 0; // FIXME\n";
-  OS << "}\n";
 }
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
@@ -601,7 +596,8 @@
   OS << "union {\n";
   OS << "Expr *" << getLowerName() << "Expr;\n";
   OS << "TypeSourceInfo *" << getLowerName() << "Type;\n";
-  OS << "};";
+  OS << "};\n";
+  OS << "std::optional " << getLowerName() << "Cache;\n";
 }
 
 void writePCHReadArgs(raw_ostream &OS) const override {
@@ -628,14 +624,21 @@
 }
 
 std::string getIsOmitted() const override {
-  return "!is" + getLowerName().str() + "Expr || !" + getLowerName().str()
- + "Expr";
+  return "!((is" + getLowerName().str() + "Expr && " +
+ getLowerName().str() + "Expr) || (!is" + getLowerName().str() +
+ "Expr && " + getLowerName().str() + "Type))";
 }
 
 void writeValue(raw_ostream &OS) const override {
   OS << "\";\n";
-  OS << "" << getLowerName()
+  OS << "if (is" << getLowerName() << "Expr && " << getLowerName()
+ << "Expr)";
+  OS << "  " << getLowerName()
  << "Expr->printPretty(OS, nullptr, Policy);\n";
+  OS << "if (!is" << getLowerName() << "Expr && " << getLowerName()
+ << "Type)";
+  OS << "  " << getLowerName()
+ << "Type->getType().print(OS, Policy);\n";
   OS << "OS << \"";
 }
 
Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- clang/test/SemaCXX/sizeless-1.cpp
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -73,7 +73,7 @@
   svint8_t __attribute__((aligned(4))) aligned_int8_2; // expected-error {{'aligned' attribute cannot be applied to sizeless type 'svint8_t'}}
   svint8_t _Alignas(int) aligned_int8_3;   // expected-error {{'_Alignas' attribute cannot be applied to sizeless type 'svint8_t'}}
 
-  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of 'alignof' to sizeless type 'svint8_t'}}
+  int _Alignas(svint8_t) aligned_int; // expected-error {{invalid application of '_Alignas' to sizeless type 'svint8_t'}}
 
   // Using pointers to sizeless data isn't wrong here, but because the
   // type is incomplete, it doesn't provide any alignment guarantees.
Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.c

[PATCH] D151349: [HIP] emit macro `__HIP_NO_IMAGE_SUPPORT`

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 525637.
yaxunl added a comment.
Herald added a subscriber: jdoerfert.
Herald added a reviewer: kiranchandramohan.

update tests


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

https://reviews.llvm.org/D151349

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
  clang/test/CodeGenOpenCL/amdgpu-features.cl
  clang/test/Driver/hip-macros.hip
  clang/test/OpenMP/amdgcn-attributes.cpp
  flang/test/Lower/OpenMP/target_cpu_features.f90
  llvm/lib/TargetParser/TargetParser.cpp

Index: llvm/lib/TargetParser/TargetParser.cpp
===
--- llvm/lib/TargetParser/TargetParser.cpp
+++ llvm/lib/TargetParser/TargetParser.cpp
@@ -280,6 +280,7 @@
   Features["gfx10-3-insts"] = true;
   Features["gfx11-insts"] = true;
   Features["atomic-fadd-rtn-insts"] = true;
+  Features["image-insts"] = true;
   break;
 case GK_GFX1036:
 case GK_GFX1035:
@@ -302,6 +303,7 @@
   Features["gfx9-insts"] = true;
   Features["gfx10-insts"] = true;
   Features["gfx10-3-insts"] = true;
+  Features["image-insts"] = true;
   Features["s-memrealtime"] = true;
   Features["s-memtime-inst"] = true;
   break;
@@ -323,6 +325,7 @@
   Features["gfx8-insts"] = true;
   Features["gfx9-insts"] = true;
   Features["gfx10-insts"] = true;
+  Features["image-insts"] = true;
   Features["s-memrealtime"] = true;
   Features["s-memtime-inst"] = true;
   break;
@@ -334,7 +337,27 @@
   Features["atomic-ds-pk-add-16-insts"] = true;
   Features["atomic-flat-pk-add-16-insts"] = true;
   Features["atomic-global-pk-add-bf16-inst"] = true;
-  [[fallthrough]];
+  Features["gfx90a-insts"] = true;
+  Features["atomic-buffer-global-pk-add-f16-insts"] = true;
+  Features["atomic-fadd-rtn-insts"] = true;
+  Features["dot3-insts"] = true;
+  Features["dot4-insts"] = true;
+  Features["dot5-insts"] = true;
+  Features["dot6-insts"] = true;
+  Features["mai-insts"] = true;
+  Features["dl-insts"] = true;
+  Features["dot1-insts"] = true;
+  Features["dot2-insts"] = true;
+  Features["dot7-insts"] = true;
+  Features["dot10-insts"] = true;
+  Features["gfx9-insts"] = true;
+  Features["gfx8-insts"] = true;
+  Features["16-bit-insts"] = true;
+  Features["dpp"] = true;
+  Features["s-memrealtime"] = true;
+  Features["ci-insts"] = true;
+  Features["s-memtime-inst"] = true;
+  break;
 case GK_GFX90A:
   Features["gfx90a-insts"] = true;
   Features["atomic-buffer-global-pk-add-f16-insts"] = true;
@@ -382,6 +405,7 @@
 case GK_GFX602:
 case GK_GFX601:
 case GK_GFX600:
+  Features["image-insts"] = true;
   Features["s-memtime-inst"] = true;
   break;
 case GK_NONE:
Index: flang/test/Lower/OpenMP/target_cpu_features.f90
===
--- flang/test/Lower/OpenMP/target_cpu_features.f90
+++ flang/test/Lower/OpenMP/target_cpu_features.f90
@@ -7,7 +7,7 @@
 
 !CHECK: omp.target = #omp.target
 !CHECK-LABEL: func.func @_QPomp_target_simple() {
Index: clang/test/OpenMP/amdgcn-attributes.cpp
===
--- clang/test/OpenMP/amdgcn-attributes.cpp
+++ clang/test/OpenMP/amdgcn-attributes.cpp
@@ -33,11 +33,11 @@
 }
 
 // DEFAULT: attributes #0 = { convergent noinline norecurse nounwind optnone "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" }
-// CPU: attributes #0 = { convergent noinline norecurse nounwind optnone "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx900" "target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" "uniform-work-group-size"="true" }
+// CPU: attributes #0 = { convergent noinline norecurse nounwind optnone "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx900" "target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+image-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" "uniform-work-group-size"="true" }
 // NOIEEE: attributes #0 = { convergent noinline norecurse nounwind optnone "amdgpu-ieee"="false" "kernel" "no-nans-fp-math"="true" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" }
 // UNSAFEATOMIC: attributes #0 = { convergent noinline norecurse nounwind optnone "amdgpu-unsafe-fp-atomics"="true" "kernel" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "uniform-work-group-size"="true" }
 
 // DEFAULT: attributes #1 = { convergent mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// CPU: attr

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

[Yurong via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/AttrImpl.cpp:247
+
+  if (isAlignmentExpr()) {
+return alignmentExpr

erichkeane wrote:
> Typically we don't do curleys on a single-statement... but I am on the fence 
> due to how complicated it is below.
I've updated this function, What do you think?



Comment at: clang/lib/AST/AttrImpl.cpp:259
+  // alignment of the referenced type.
+  if (const ReferenceType *Ref = T->getAs())
+T = Ref->getPointeeType();

erichkeane wrote:
> You can replace this 'if' with: `T = T->getNonReferenceType();`
+1, both update `GetAlignOfType` in ExprConstant.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150528

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

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl updated this revision to Diff 525644.
rsundahl added a comment.

Apply proper backticks quotes to options. Remove redundant ellipses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143675

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize-stable-abi.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/docs/ASanABI.rst
  compiler-rt/lib/asan_abi/CMakeLists.txt
  compiler-rt/lib/asan_abi/asan_abi.cpp
  compiler-rt/lib/asan_abi/asan_abi.h
  compiler-rt/lib/asan_abi/asan_abi_shim.cpp
  compiler-rt/lib/asan_abi/asan_abi_tbd.txt
  compiler-rt/test/asan_abi/CMakeLists.txt
  compiler-rt/test/asan_abi/TestCases/Darwin/llvm_interface_symbols.cpp
  compiler-rt/test/asan_abi/TestCases/linkstaticlibrary.cpp
  compiler-rt/test/asan_abi/lit.cfg.py
  compiler-rt/test/asan_abi/lit.site.cfg.py.in

Index: compiler-rt/test/asan_abi/lit.site.cfg.py.in
===
--- /dev/null
+++ compiler-rt/test/asan_abi/lit.site.cfg.py.in
@@ -0,0 +1,18 @@
+@LIT_SITE_CFG_IN_HEADER@
+
+# Tool-specific config options.
+config.name_suffix = "@ASAN_ABI_TEST_CONFIG_SUFFIX@"
+config.target_cflags = "@ASAN_ABI_TEST_TARGET_CFLAGS@"
+config.clang = "@ASAN_ABI_TEST_TARGET_CC@"
+config.bits = "@ASAN_ABI_TEST_BITS@"
+config.arm_thumb = "@COMPILER_RT_ARM_THUMB@"
+config.apple_platform = "@ASAN_ABI_TEST_APPLE_PLATFORM@"
+config.apple_platform_min_deployment_target_flag = "@ASAN_ABI_TEST_MIN_DEPLOYMENT_TARGET_FLAG@"
+config.asan_abi_dynamic = @ASAN_ABI_TEST_DYNAMIC@
+config.target_arch = "@ASAN_ABI_TEST_TARGET_ARCH@"
+
+# Load common config for all compiler-rt lit tests.
+lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
+
+# Load tool-specific config that would do the real work.
+lit_config.load_config(config, "@ASAN_ABI_LIT_SOURCE_DIR@/lit.cfg.py")
Index: compiler-rt/test/asan_abi/lit.cfg.py
===
--- /dev/null
+++ compiler-rt/test/asan_abi/lit.cfg.py
@@ -0,0 +1,74 @@
+# -*- Python -*-
+
+import os
+import platform
+import re
+
+import lit.formats
+
+def get_required_attr(config, attr_name):
+  attr_value = getattr(config, attr_name, None)
+  if attr_value is None:
+lit_config.fatal(
+  'No attribute %r in test configuration! You may need to run '
+  'tests from your build directory or add this attribute '
+  'to lit.site.cfg.py ' % attr_name)
+  return attr_value
+
+# Setup config name.
+config.name = 'AddressSanitizerABI' + config.name_suffix
+
+# Platform-specific default ASAN_ABI_OPTIONS for lit tests.
+default_asan_abi_opts = list(config.default_sanitizer_opts)
+
+default_asan_abi_opts_str = ':'.join(default_asan_abi_opts)
+if default_asan_abi_opts_str:
+  config.environment['ASAN_ABI_OPTIONS'] = default_asan_abi_opts_str
+  default_asan_abi_opts_str += ':'
+config.substitutions.append(('%env_asan_abi_opts=',
+ 'env ASAN_ABI_OPTIONS=' + default_asan_abi_opts_str))
+
+# Setup source root.
+config.test_source_root = os.path.dirname(__file__)
+
+# GCC-ASan doesn't link in all the necessary libraries automatically, so
+# we have to do it ourselves.
+extra_link_flags = []
+
+# Setup default compiler flags used with -fsanitize=address option.
+# FIXME: Review the set of required flags and check if it can be reduced.
+target_cflags = [get_required_attr(config, 'target_cflags')] + extra_link_flags
+target_cxxflags = config.cxx_mode_flags + target_cflags
+clang_asan_abi_static_cflags = (['-fsanitize=address',
+'-fsanitize-stable-abi',
+'-mno-omit-leaf-frame-pointer',
+'-fno-omit-frame-pointer',
+'-fno-optimize-sibling-calls'] +
+config.debug_info_flags + target_cflags)
+clang_asan_abi_static_cxxflags = config.cxx_mode_flags + clang_asan_abi_static_cflags
+
+config.available_features.add('asan_abi-static-runtime')
+clang_asan_abi_cflags = clang_asan_abi_static_cflags
+clang_asan_abi_cxxflags = clang_asan_abi_static_cxxflags
+
+def build_invocation(compile_flags):
+  return ' ' + ' '.join([config.clang] + compile_flags) + ' '
+
+config.substitutions.append( ('%clang ', build_invocation(target_cflags)) )
+config.substitutions.append( ('%clangxx ', build_invocation(target_cxxflags)) )
+config.substitutions.append( ('%clang_asan_abi ', build_invocation(clang_asan_abi_cflags)) )
+config.substitutions.append( ('%clangxx_asan_abi ', build_invocation(clang_asan_abi_cxxflags)) )
+
+libasan_abi_path = os.path.join(config.compiler_rt_libdir, 'libclang_rt.asan_abi_osx.a'.format(config.apple_platform))
+
+if libasan_abi_path is not None:
+  config.substitutions.append( ('%libasan_abi', libasan_abi_path) )
+  config.substitutions.append( ('%

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl marked 2 inline comments as done.
rsundahl added a comment.

Thanks for your time and guidance getting this landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143675

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