[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:10090
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
+  if ((S.getLangOpts().OpenCL || S.getLangOpts().SYCLIsDevice) &&
+  isLHSPointer && isRHSPointer) {

Anastasia wrote:
> bader wrote:
> > Alternative approach would be to remove `S.getLangOpts().OpenCL` to enable 
> > this check for all modes.
> > 
> > @Anastasia, do you know if it's safe to do?
> If I look at embedded C (ISO/IEC TR 18037) s5.3 rules that we are following 
> largely in Clang I believe this is a universal rule!
> 
> Especially looking at the followong statement:
> 
> > Clause 6.5.6 - Additive operators, add another constraint paragraph: 
> > For subtraction, if the two operands are pointers into different address 
> > spaces, the address spaces must overlap. 
> 
> So I think that it should apply to at least OpenCL, C and C++.  I am 
> surprised though that this has not been fixed yet.
> 
> I am CCing @rjmccall and @jeroen.dobbelaere in case they have any more 
> insights on this.
> 
Yes, I agree, this should apply in all modes.

You should able to restructure the `isAddressSpaceOverlapping` function so that 
it can work directly on the address spaces of  `LHSPointeeTy` and 
`RHSPointeeTy`; the code will be both cleaner and faster.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80360: [PCH] Support writing BuiltinBitCastExprs to PCHs

[hyd-dev via Phabricator via cfe-commits]

hyd-dev updated this revision to Diff 265436.
hyd-dev added a comment.

Format the test `builtin-bit-cast.cpp`.


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

https://reviews.llvm.org/D80360

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/PCH/builtin-bit-cast.cpp


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3593,6 +3593,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  S = new (Context) BuiltinBitCastExpr(
+  Empty, /*PathSize*/ Record[ASTStmtReader::NumExprFields]);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1791,6 +1791,9 @@
   /// A CXXFunctionalCastExpr record.
   EXPR_CXX_FUNCTIONAL_CAST,
 
+  /// A BuiltinBitCastExpr record.
+  EXPR_BUILTIN_BIT_CAST,
+
   /// A UserDefinedLiteral record.
   EXPR_USER_DEFINED_LITERAL,
 
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -4785,6 +4785,8 @@
   : ExplicitCastExpr(BuiltinBitCastExprClass, T, VK, CK, SrcExpr, 0,
  DstType),
 KWLoc(KWLoc), RParenLoc(RParenLoc) {}
+  BuiltinBitCastExpr(EmptyShell Empty, unsigned PathSize)
+  : ExplicitCastExpr(BuiltinBitCastExprClass, Empty, PathSize) {}
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return KWLoc; }
   SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }


Index: clang/test/PCH/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/PCH/builtin-bit-cast.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+constexpr T BuiltinBitCastWrapper(const U &Arg) {
+  return __builtin_bit_cast(T, Arg);
+}
+
+#else
+
+int main() {
+  return BuiltinBitCastWrapper(0);
+}
+
+#endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1635,6 +1635,7 @@
   VisitExplicitCastExpr(E);
   Record.AddSourceLocation(E->getBeginLoc());
   Record.AddSourceLocation(E->getEndLoc());
+  Code = serialization::EXPR_BUILTIN_BIT_CAST;
 }
 
 void ASTStmtWriter::VisitUserDefinedLiteral(UserDefinedLiteral *E) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -3593,6 +3593,11 @@
/*PathSize*/ Record[ASTStmtReader::NumExprFields]);
   break;
 
+case EXPR_BUILTIN_BIT_CAST:
+  S = new (Context) BuiltinBitCastExpr(
+  Empty, /*PathSize*/ Record[ASTStmtReader::NumExprFields]);
+  break;
+
 case EXPR_USER_DEFINED_LITERAL:
   S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization

[PATCH] D57890: [analyzer] Fix in self assignment checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.
Herald added subscribers: ASDenysPetrov, martong, steakhal.

I think you can create a unit test for this: create a pre-call checker that 
checks for the assignment operator and asserts that we are not in top level. 
Create a test code with a simple class without user provided copy operator and 
a function that uses the auto-generated copy operator. Run the checker on this 
sample code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57890



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

[clang] c2c36c4 - [clang][index] Fix a crash for accessing a null field decl.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-05-21T09:47:36+02:00
New Revision: c2c36c4f4b69ade6d8610b1dc98ff9f02c94320d

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

LOG: [clang][index] Fix a crash for accessing a null field decl.

getField() may return a nullptr, we already did that in
BodyIndexer::VisitDesignatedInitExpr, but missed one place.

Added: 
clang/test/Index/index-designated-init-recovery.cpp

Modified: 
clang/lib/Index/IndexBody.cpp

Removed: 




diff  --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp
index 07a94f30c883..01cf559d7057 100644
--- a/clang/lib/Index/IndexBody.cpp
+++ b/clang/lib/Index/IndexBody.cpp
@@ -414,7 +414,7 @@ class BodyIndexer : public RecursiveASTVisitor 
{
 
 auto visitSyntacticDesignatedInitExpr = [&](DesignatedInitExpr *E) -> bool 
{
   for (DesignatedInitExpr::Designator &D : 
llvm::reverse(E->designators())) {
-if (D.isFieldDesignator())
+if (D.isFieldDesignator() && D.getField())
   return IndexCtx.handleReference(D.getField(), D.getFieldLoc(),
   Parent, ParentDC, SymbolRoleSet(),
   {}, E);

diff  --git a/clang/test/Index/index-designated-init-recovery.cpp 
b/clang/test/Index/index-designated-init-recovery.cpp
new file mode 100644
index ..182a1aeeb8e6
--- /dev/null
+++ b/clang/test/Index/index-designated-init-recovery.cpp
@@ -0,0 +1,8 @@
+struct Bar {};
+struct Foo {
+  void method(Bar bar) {}
+};
+void NoCrash(Foo t) {
+  t.method({.abc = 50}); // CHECK: field designator 'abc' does not refer to 
any field in type 'Bar'
+}
+// RUN: c-index-test -index-file %s -Xclang -frecovery-ast 2>&1 | FileCheck %s



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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC option -EHa)

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 265442.
tentzen added a comment.

fixed formats


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCatchDotDotDot.cpp
  clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
  clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
  llvm/include/llvm/CodeGen/SelectionDAGISel.h
  llvm/include/llvm/CodeGen/WinEHFuncInfo.h
  llvm/include/llvm/IR/BasicBlock.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Analysis/EHPersonalities.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinException.cpp
  llvm/lib/CodeGen/BranchFolding.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/lib/IR/BasicBlock.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/CodeGen/X86/windows-seh-EHa-CppCatchDotDotDot.ll
  llvm/test/CodeGen/X86/windows-seh-EHa-CppDtors01.ll
  llvm/test/CodeGen/X86/windows-seh-EHa-TryInFinally.ll

Index: llvm/test/CodeGen/X86/windows-seh-EHa-TryInFinally.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/windows-seh-EHa-TryInFinally.ll
@@ -0,0 +1,226 @@
+; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+
+; CHECK-LABEL: "?fin$0@0@main@@"
+; CHECK:  .seh_handlerdata
+; CHECK:  .set ".L?fin$0@0@main@@$parent_frame_offset", 48
+; CHECK-NEXT:.long   (.Llsda_end1-.Llsda_begin1)/16 
+; CHECK-NEXT: .Llsda_begin1:
+; CHECK-NEXT:.long   .Ltmp
+; CHECK-NEXT:.long   .Ltmp
+; CHECK-NEXT:.long   "?dtor$
+; CHECK-NEXT:.long   0
+; CHECK-NEXT: .Llsda_end1:
+
+; ModuleID = 'windows-seh-EHa-TryInFinally.cpp'
+source_filename = "windows-seh-EHa-TryInFinally.cpp"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+$"??_C@_0CI@MDFPIOJJ@?5?9?9?9?5Test?5_Try?5in?5_finally?5?9?9?9?5i@" = comdat any
+
+$"??_C@_0BN@HHKJHLBE@?5?5In?5Inner?5_finally?5i?5?$DN?5?$CFd?5?6?$AA@" = comdat any
+
+$"??_C@_0BN@HAIIIOKI@?5?5In?5outer?5_finally?5i?5?$DN?5?$CFd?5?6?$AA@" = comdat any
+
+$"??_C@_0BJ@OJMMAGCD@?5?5In?5outer?5_try?5i?5?$DN?5?$CFd?5?6?$AA@" = comdat any
+
+$"??_C@_0CG@ENDJHCGA@?5?9?9?9?5In?5outer?5except?5handler?5i?5?$DN@" = comdat any
+
+@"??_C@_0CI@MDFPIOJJ@?5?9?9?9?5Test?5_Try?5in?5_finally?5?9?9?9?5i@" = linkonce_odr dso_local unnamed_addr constant [40 x i8] c" --- Test _Try in _finally --- i = %d \0A\00", comdat, align 1
+@"??_C@_0BN@HHKJHLBE@?5?5In?5Inner?5_finally?5i?5?$DN?5?$CFd?5?6?$AA@" = linkonce_odr dso_local unnamed_addr constant [29 x i8] c"  In Inner _finally i = %d \0A\00", comdat, align 1
+@"??_C@_0BN@HAIIIOKI@?5?5In?5outer?5_finally?5i?5?$DN?5?$CFd?5?6?$AA@" = linkonce_odr dso_local unnamed_addr constant [29 x i8] c"  In outer _finally i = %d \0A\00", comdat, align 1
+@"??_C@_0BJ@OJMMAGCD@?5?5In?5outer?5_try?5i?5?$DN?5?$CFd?5?6?$AA@" = linkonce_odr dso_local unnamed_addr constant [25 x i8] c"  In outer _try i = %d \0A\00", comdat, align 1
+@"??_C@_0CG@ENDJHCGA@?5?9?9?9?5In?5outer?5except?5handler?5i?5?$DN@" = linkonce_odr dso_local unnamed_addr constant [38 x i8] c" --- In outer except handler i = %d \0A\00", comdat, align 1
+
+; Function Attrs: noinline norecurse optnone
+define dso_local i32 @main() #0 personality i8* bitcast (i32 (...)* @__C_specific_handler to i8*) {
+entry:
+  %retval = alloca i32, align 4
+  %i = alloca i32, align 4
+  %__exception_code = alloca i32, align 4
+  call void (...) @llvm.localescape(i32* %i)
+  store i32 0, i32* %retval, align 4
+  store i32 0, i32* %i, align 4
+  br label %for.cond
+
+for.cond: ; preds = %for.inc, %entry
+  %0 = load i32, i32* %i, align 4
+  %cmp = icmp slt i32 %0, 3
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body: ; preds = %for.cond
+  %1 = load i32, i32* %i, align 4
+  call void (...) @"?printf@@YAXZZ"(i8* getelementptr inbounds ([40 x i8], [40 x i8]* @"??_C@_0CI@MDFPIOJJ@?5?9?9?9?5Test?5_Try?5in?5_finally?5?9?9?9?5i@", i64 0, i64 0), i32 %1)
+  invoke void @llvm.seh.try.begin()
+  to label %invoke.cont unwind label %catch.dispatch
+
+invoke.cont:  ; preds = %for.body
+  invoke void @llvm.seh.try.begin()
+  to label %invoke.cont1 unwind label %ehcleanup
+
+invoke.cont1:  

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2283
+  unsigned Src2Reg = Inst.getOperand(1).getReg();
+  if (DestReg == Src2Reg)
+return Error(Loc, "The destination vector register group cannot 
overlap"

HsiangKai wrote:
> fpallares wrote:
> > In this function we are validating the overlap constraints by checking 
> > whether `DestReg` matches a vector operand. However, for widenings and 
> > narrowings those checks should be extended to validate that the operand 
> > doesn't belong to the same register group.
> > 
> > For instance:
> > 
> > `vwadd.vv v4, v4, v7`: The current code would give an error for this 
> > instruction.
> > `vwadd.vv v4, v5, v7`: There would be no error for this, even though `v5` 
> > will always overlap with any widened group of `v4`.
> > `vwadd.vv v4, v6, v7`: This should not emit any diagnostic because we can't 
> > really tell at this point if there will be an overlap.
> > 
> > Perhaps we are already checking this somewhere else and I missed it?
> > 
> > (Note that this will probably change with fractional LMUL in 0.9)
> > 
> Good point!
> 
> Because there is no LMUL information in these instructions, we cannot detect 
> the invalid cases precisely. I only detect destination and source occupy the 
> same register. Maybe I could detect destination and (source + 1). The 
> smallest LMUL is 1 in v0.8.
> 
> After this patch, I am going to prepare another one to upgrade the MC layer 
> to v0.9. I will review this part.
> 
> Do you have any suggestions?
Yes, I was thinking on the (source + 1) case too. I don't think much can be 
done in v0.9 at the MC layer, since this case would be correct for LMUL < 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, martong, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
xazax.hun, whisperity, mgorny.

Checkers should be able to get the return value under construction for a 
`CallEvenet`. This patch adds a function to achieve this which retrieves the 
return value from the construction context of the call.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80366

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

Index: clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
===
--- /dev/null
+++ clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -0,0 +1,55 @@
+//===- unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp --===//
+//
+// 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 "CheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ento {
+namespace {
+
+class TestReturnValueUnderConstructionChecker
+  : public Checker {
+public:
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
+if (!Call.getOriginExpr())
+  return;
+
+Optional RetVal = Call.getReturnValueUnderConstruction(0);
+assert(RetVal);
+assert(RetVal->getAsRegion());
+  }
+};
+
+void addTestReturnValueUnderConstructionChecker(
+AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages =
+{{"test.TestReturnValueUnderConstruction", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+  Registry.addChecker(
+  "test.TestReturnValueUnderConstruction", "", "");
+});
+}
+
+TEST(TestReturnValueUnderConstructionChecker,
+ ReturnValueUnderConstructionChecker) {
+  EXPECT_TRUE(runCheckerOnCode(
+  "class C { public: C(int nn): n(nn) {} virtual ~C() {} "
+  "  private: int n; };"
+  "C returnC(int m) { C c(m); return c; }"
+  "void foo() { C c = returnC(1); }"));
+}
+
+} // namespace
+} // namespace ento
+} // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -8,6 +8,7 @@
   StoreTest.cpp
   RegisterCustomCheckersTest.cpp
   SymbolReaperTest.cpp
+  TestReturnValueUnderConstruction.cpp
   )
 
 clang_target_link_libraries(StaticAnalysisTests
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -109,6 +109,96 @@
   return LValue;
 }
 
+Optional ExprEngine::retrieveFromConstructionContext(
+ProgramStateRef State, const LocationContext *LCtx,
+const ConstructionContext *CC) {
+  if (CC) {
+switch (CC->getKind()) {
+case ConstructionContext::CXX17ElidedCopyVariableKind:
+case ConstructionContext::SimpleVariableKind: {
+  const auto *DSCC = cast(CC);
+  const auto *DS = DSCC->getDeclStmt();
+  return getObjectUnderConstruction(State, DS, LCtx);
+}
+case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
+case ConstructionContext::SimpleConstructorInitializerKind: {
+  const auto *ICC = cast(CC);
+  const auto *Init = ICC->getCXXCtorInitializer();
+  return getObjectUnderConstruction(State, Init, LCtx);
+}
+case ConstructionContext::SimpleReturnedValueKind:
+case ConstructionContext::CXX17ElidedCopyReturnedValueKind: {
+  const StackFrameContext *SFC = LCtx->getStackFrame();
+  if (const LocationContext *CallerLCtx = SFC->getParent()) {
+auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+   .getAs();
+if (!RTC) {
+  // We were unable to find the correct construction context for the
+

[PATCH] D79921: [OPENMP] Fix mixture of omp and clang pragmas

[ISHIGURO, Hiroshi via Phabricator via cfe-commits]

hishiguro updated this revision to Diff 265450.
hishiguro added a comment.

Thank you for your comment. I updated source.


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

https://reviews.llvm.org/D79921

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/omp_with_loop_pragma.c


Index: clang/test/OpenMP/omp_with_loop_pragma.c
===
--- clang/test/OpenMP/omp_with_loop_pragma.c
+++ clang/test/OpenMP/omp_with_loop_pragma.c
@@ -3,12 +3,12 @@
 // expected-no-diagnostics
 
 // CHECK: !{{[0-9]+}} = !{!"llvm.loop.vectorize.width", i32 1}
-void sub(double * restrict a, double * restrict b, int n){ 
+void sub(double *restrict a, double *restrict b, int n) {
   int i;
 
 #pragma omp parallel for
 #pragma clang loop vectorize(disable)
-  for(i=0;i(&S);
-  const CapturedStmt *ICS = OMPED->getInnermostCapturedStmt();
+  const auto &OMPED = cast(S);
+  const CapturedStmt *ICS = OMPED.getInnermostCapturedStmt();
   const Stmt *SS = ICS->getCapturedStmt();
   const AttributedStmt *AS = dyn_cast_or_null(SS);
   if (AS)


Index: clang/test/OpenMP/omp_with_loop_pragma.c
===
--- clang/test/OpenMP/omp_with_loop_pragma.c
+++ clang/test/OpenMP/omp_with_loop_pragma.c
@@ -3,12 +3,12 @@
 // expected-no-diagnostics
 
 // CHECK: !{{[0-9]+}} = !{!"llvm.loop.vectorize.width", i32 1}
-void sub(double * restrict a, double * restrict b, int n){ 
+void sub(double *restrict a, double *restrict b, int n) {
   int i;
 
 #pragma omp parallel for
 #pragma clang loop vectorize(disable)
-  for(i=0;i(&S);
-  const CapturedStmt *ICS = OMPED->getInnermostCapturedStmt();
+  const auto &OMPED = cast(S);
+  const CapturedStmt *ICS = OMPED.getInnermostCapturedStmt();
   const Stmt *SS = ICS->getCapturedStmt();
   const AttributedStmt *AS = dyn_cast_or_null(SS);
   if (AS)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

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

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

Looks good. Not sure if it work in all cases after the `MallocChecker` problems 
but we can fix problems if any later.


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

https://reviews.llvm.org/D77846



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

[PATCH] D80369: [DebugInfo] Remove decl subprograms from 'retainedTypes:'

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro created this revision.
djtodoro added reviewers: dblaikie, aprantl, vsk.
djtodoro added projects: LLVM, debug-info.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

After the D70350 , the `retainedTypes:` isn't 
being used for the purpose of call site debug info for extern calls, so it is 
safe to delete it from IR representation.
We are also adding a test to ensure the subprogram isn't stored within the 
`retainedTypes:` from corresponding `DICompileUnit`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80369

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,6 +1,10 @@
 // When entry values are emitted, expect a subprogram for extern decls so that
 // the dwarf generator can describe call site parameters at extern call sites.
 //
+// Initial implementation relied on the 'retainedTypes:' from the corresponding
+// DICompileUnit, so we also ensure that we do not store the extern declaration
+// subprogram into the 'retainedTypes:'.
+//
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
 
@@ -17,6 +21,8 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: 
![[RETTYPES:[0-9]+]]
+// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3870,8 +3870,6 @@
 
   if (IsDeclForCallSite)
 Fn->setSubprogram(SP);
-
-  DBuilder.retainType(SP);
 }
 
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,6 +1,10 @@
 // When entry values are emitted, expect a subprogram for extern decls so that
 // the dwarf generator can describe call site parameters at extern call sites.
 //
+// Initial implementation relied on the 'retainedTypes:' from the corresponding
+// DICompileUnit, so we also ensure that we do not store the extern declaration
+// subprogram into the 'retainedTypes:'.
+//
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
 
@@ -17,6 +21,8 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
+// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3870,8 +3870,6 @@
 
   if (IsDeclForCallSite)
 Fn->setSubprogram(SP);
-
-  DBuilder.retainType(SP);
 }
 
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a subscriber: NoQ.
Szelethus added a comment.

I have more questions than actual objections, but here we go! The patch looks 
nice overall, we just need to iron a few things out ahead of time.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107
+  /// This value applies to all error states in ErrorState except FEOF.
+  /// An EOF+indeterminate state is the same as EOF state.
+  bool FilePositionIndeterminate = false;

What does this mean? "An EOF+indeterminate state is the same as EOF state." I 
don't understand the message you want to convey here -- is it that we cannot 
have an indeterminate file position indicator if we hit EOF, hence we regard a 
stream that is **known** to be EOF to have its file position indicator 
determinate?

A good followup question for the uninitiated would be that "Well why is it ever 
legal to construct a `StreamState` object that can both have the 
`FilePositionIndeterminate` set to true and the `ErrorState` indicate that the 
steam is **known** to be in EOF?" Well, the answer is that we may only realize 
later that the error state can only be EOF, like in here:
```lang=c++
void f() {
 FILE *F = fopen(...);
 if (fseek(F, ...)) {
// Could be either EOF, ERROR, and ofc indeterminate
if (eof(F)) {
  // This is where we have a seemingly impossible stream state, but its not 
a programming error, its a design decision.
}
}
```
This might warrant a bit on explanation either here, or in 
`ensureNoFilePositionIndeterminate`. Probably the latter.

With that said, can `SteamState`'s constructor ensure that we do not create a 
known to be EOF stream that is indeterminate?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:124
+   const StreamErrorState &ES = ErrorNone,
+   bool FPI = false) {
+return StreamState{L, Opened, ES, FPI};

Let's not cheap out on this parameter name :^)



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:191-193
   mutable std::unique_ptr BT_nullfp, BT_illegalwhence,
-  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof;
+  BT_UseAfterClose, BT_UseAfterOpenFailed, BT_ResourceLeak, BT_StreamEof,
+  BT_IndeterminatePosition;

In time, we could create a new checker for each of these bug types, similar to 
how D77866 does it. But this is clearly a low prio issue, I'm just talking 
aloud.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:306
+  ProgramStateRef
+  ensureNoFilePositionIndeterminate(SVal StreamVal, CheckerContext &C,
+ProgramStateRef State) const;

Or `ensureFilePositionDeterminate`? :D



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:839-840
+
+// If only FEof is possible, report no warning (indeterminate position in
+// FEof is ignored).
+if (SS->ErrorState.isFEof())

I think we shouldn't say its ignored, but rather its impossible. I mean, if we 
have reached the end of the file, how could the file position indicator be 
indeterminate?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:854
+  }
+  return State; // FIXME: nullptr?
+}

Good question. It would make sense... @NoQ, @xazax.hun?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:857-868
+// FEof and other states are possible.
+// The path with FEof is the one that can continue.
+// For this reason a non-fatal error is generated to continue the analysis
+// with only FEof state set.
+ExplodedNode *N = C.generateNonFatalErrorNode(State);
+if (N) {
+  C.emitReport(std::make_unique(

Ugh, tough one. I think this just isn't really *the* error to highlight here, 
but rather that its bad practice to not check on the stream after an operation. 
But I'm willing to accept I might be wrong here.



Comment at: clang/test/Analysis/stream-error.c:182
+} else {
+  fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
+}

Here the user checked whether F is in eof or in ferror, and its in neither. 
Isn't it reasonable to assume then that the stream is fine?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018



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

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107
+  /// This value applies to all error states in ErrorState except FEOF.
+  /// An EOF+indeterminate state is the same as EOF state.
+  bool FilePositionIndeterminate = false;

Szelethus wrote:
> What does this mean? "An EOF+indeterminate state is the same as EOF state." I 
> don't understand the message you want to convey here -- is it that we cannot 
> have an indeterminate file position indicator if we hit EOF, hence we regard 
> a stream that is **known** to be EOF to have its file position indicator 
> determinate?
> 
> A good followup question for the uninitiated would be that "Well why is it 
> ever legal to construct a `StreamState` object that can both have the 
> `FilePositionIndeterminate` set to true and the `ErrorState` indicate that 
> the steam is **known** to be in EOF?" Well, the answer is that we may only 
> realize later that the error state can only be EOF, like in here:
> ```lang=c++
> void f() {
>  FILE *F = fopen(...);
>  if (fseek(F, ...)) {
> // Could be either EOF, ERROR, and ofc indeterminate
> if (eof(F)) {
>   // This is where we have a seemingly impossible stream state, but its 
> not a programming error, its a design decision.
> }
> }
> ```
> This might warrant a bit on explanation either here, or in 
> `ensureNoFilePositionIndeterminate`. Probably the latter.
> 
> With that said, can `SteamState`'s constructor ensure that we do not create a 
> known to be EOF stream that is indeterminate?
Actually, not enforcing this could leave to false positives:

```
void f() {
 FILE *F = fopen(...);
 if (fseek(F, ...)) {
// Could be either EOF, ERROR, and ofc indeterminate
if (eof(F)) {
  clearerr(F);
  fseek(F, ...); // false positive warning
}
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018



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

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Fix a potential assert in use-noexcept check if there is an issue getting the 
`TypeSourceInfo` as well as a small clean up.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80371

Files:
  clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h


Index: clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
@@ -41,7 +41,7 @@
 
 private:
   const std::string NoexceptMacro;
-  bool UseNoexceptFalse;
+  const bool UseNoexceptFalse;
 };
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
@@ -27,22 +27,13 @@
 }
 
 void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  functionDecl(
-  cxxMethodDecl(
-  hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec(,
-  anyOf(hasOverloadedOperatorName("delete[]"),
-hasOverloadedOperatorName("delete"), cxxDestructorDecl()))
-  .bind("del-dtor"))
-  .bind("funcDecl"),
-  this);
-
   Finder->addMatcher(
   functionDecl(
   hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec(,
-  unless(anyOf(hasOverloadedOperatorName("delete[]"),
-   hasOverloadedOperatorName("delete"),
-   cxxDestructorDecl(
+  optionally(cxxMethodDecl(anyOf(hasAnyOverloadedOperatorName(
+ "delete[]", "delete"),
+ cxxDestructorDecl()))
+ .bind("del-dtor")))
   .bind("funcDecl"),
   this);
 
@@ -80,6 +71,10 @@
   .castAs()
   .getExceptionSpecRange();
   }
+
+  if (Range.isInvalid())
+return;
+
   CharSourceRange CRange = Lexer::makeFileCharRange(
   CharSourceRange::getTokenRange(Range), *Result.SourceManager,
   Result.Context->getLangOpts());


Index: clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.h
@@ -41,7 +41,7 @@
 
 private:
   const std::string NoexceptMacro;
-  bool UseNoexceptFalse;
+  const bool UseNoexceptFalse;
 };
 
 } // namespace modernize
Index: clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseNoexceptCheck.cpp
@@ -27,22 +27,13 @@
 }
 
 void UseNoexceptCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  functionDecl(
-  cxxMethodDecl(
-  hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec(,
-  anyOf(hasOverloadedOperatorName("delete[]"),
-hasOverloadedOperatorName("delete"), cxxDestructorDecl()))
-  .bind("del-dtor"))
-  .bind("funcDecl"),
-  this);
-
   Finder->addMatcher(
   functionDecl(
   hasTypeLoc(loc(functionProtoType(hasDynamicExceptionSpec(,
-  unless(anyOf(hasOverloadedOperatorName("delete[]"),
-   hasOverloadedOperatorName("delete"),
-   cxxDestructorDecl(
+  optionally(cxxMethodDecl(anyOf(hasAnyOverloadedOperatorName(
+ "delete[]", "delete"),
+ cxxDestructorDecl()))
+ .bind("del-dtor")))
   .bind("funcDecl"),
   this);
 
@@ -80,6 +71,10 @@
   .castAs()
   .getExceptionSpecRange();
   }
+
+  if (Range.isInvalid())
+return;
+
   CharSourceRange CRange = Lexer::makeFileCharRange(
   CharSourceRange::getTokenRange(Range), *Result.SourceManager,
   Result.Context->getLangOpts());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks fantastic, thanks! I guess let's race with @vsavchenko on whoever commits 
first :p


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

https://reviews.llvm.org/D77802



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added subscribers: tstellar, brad, craig.topper, joerg.
nemanjai added a comment.
This revision is now accepted and ready to land.

Thank you for sorting this out. I think it is quite useful to be able to 
configure the compiler to use complete non-standard toolchains that include a 
dynamic linker.
Could you please ensure that the commit message describes that the 
`--dyld-prefix` option is added to test cases since they check for the dynamic 
linker and would fail on builds that specify a default prefix?
Also, you might want to add someone from some of the other targets/platforms as 
a reviewer to ensure there is no objections (perhaps @craig.topper, @tstellar, 
@brad or @joerg).

LGTM but maybe give others a few days to have a look.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:452
   CmdArgs.push_back("-dynamic-linker");
-  CmdArgs.push_back(Args.MakeArgString(Loader));
+  CmdArgs.push_back(Args.MakeArgString(Twine(D.DyldPrefix) +
+   ToolChain.getDynamicLinker(Args)));

Is this just an orthogonal NFC change? If so, can you please commit it 
separately in an NFC commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/test/Analysis/stream-error.c:182
+} else {
+  fwrite(Buf, 1, 10, F); // expected-warning {{might be 'indeterminate'}}
+}

Szelethus wrote:
> Here the user checked whether F is in eof or in ferror, and its in neither. 
> Isn't it reasonable to assume then that the stream is fine?
Oh wow I meant to delete this inline. Please disregard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018



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

[clang] eeff1a9 - [analyzer][CallAndMessage][NFC] Split up checkPreCall

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-05-21T12:54:56+02:00
New Revision: eeff1a970a6bb09d3c046313e229e2871929cd63

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

LOG: [analyzer][CallAndMessage][NFC] Split up checkPreCall

The patch aims to use CallEvents interface in a more principled manner, and also
to highlight what this checker really does. It in fact checks for 5 different
kinds of errors (from checkPreCall, that is):

 * Invalid function pointer related errors
 * Call of methods from an invalid C++ this object
 * Function calls with incorrect amount of parameters
 * Invalid arguments for operator delete
 * Pass of uninitialized values to pass-by-value parameters

In a previous patch I complained that this checker is responsible for emitting
a lot of different diagnostics all under core.CallAndMessage's name, and this
patch shows where we could start to assign different diagnostics to different
entities.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 00fbe2c8dcb7..58eb329d1bf9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -60,6 +60,25 @@ class CallAndMessageChecker
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
+  ProgramStateRef checkFunctionPointerCall(const CallExpr *CE,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXMethodCall(const CXXInstanceCall *CC,
+ CheckerContext &C,
+ ProgramStateRef State) const;
+
+  ProgramStateRef checkParameterCount(const CallEvent &Call, CheckerContext &C,
+  ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXDeallocation(const CXXDeallocatorCall *DC,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkArgInitializedness(const CallEvent &Call,
+  CheckerContext &C,
+  ProgramStateRef State) const;
+
 private:
   bool PreVisitProcessArg(CheckerContext &C, SVal V, SourceRange ArgRange,
   const Expr *ArgEx, int ArgumentNumber,
@@ -309,123 +328,131 @@ bool 
CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
   return false;
 }
 
-void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
- CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
-  if (const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr())) {
-const Expr *Callee = CE->getCallee()->IgnoreParens();
-const LocationContext *LCtx = C.getLocationContext();
-SVal L = State->getSVal(Callee, LCtx);
-
-if (L.isUndef()) {
-  if (!BT_call_undef)
-BT_call_undef.reset(new BuiltinBug(
-this, "Called function pointer is an uninitialized pointer 
value"));
-  emitBadCall(BT_call_undef.get(), C, Callee);
-  return;
-}
+ProgramStateRef CallAndMessageChecker::checkFunctionPointerCall(
+const CallExpr *CE, CheckerContext &C, ProgramStateRef State) const {
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(L.castAs());
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal L = State->getSVal(Callee, LCtx);
+
+  if (L.isUndef()) {
+if (!BT_call_undef)
+  BT_call_undef.reset(new BuiltinBug(
+  this, "Called function pointer is an uninitialized pointer value"));
+emitBadCall(BT_call_undef.get(), C, Callee);
+return nullptr;
+  }
 
-if (StNull && !StNonNull) {
-  if (!BT_call_null)
-BT_call_null.reset(new BuiltinBug(
-this, "Called function pointer is null (null dereference)"));
-  emitBadCall(BT_call_null.get(), C, Callee);
-  return;
-}
+  ProgramStateRef StNonNull, StNull;
+  std::tie(StNonNull, StNull) = 
State->assume(L.castAs());
 
-State = StNonNull;
+  if (StNull && !StNonNull) {
+if (!BT_call_null)
+  BT_call_null.reset(new BuiltinBug(
+  this, "Called function pointer is null (null dereference)"));
+emitBadCall(BT_call_null.get(), C, Callee);
+return nullptr;
   }
 
-  // If this is a call to a C++ method, check if the callee is null or
-  // undefined.
-  if (const 

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

@NoQ @ASDenysPetrov I will rebase my changes - no worries :)


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

https://reviews.llvm.org/D77802



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

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I think adding a better getter to `StackFrameContext` would make the patch a 
tad nicer, but other than that, I don't have much to add to this patch, 
unfortunately :) The code looks nice and we definitely need something like this.




Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:552
+
+  Index = StackFrame->getIndex();
+

This mustn't be serious. `StackFrameContext::getIndex()` has **no comments** at 
all! So it is an index to the element within a `CFGBlock` to which it also 
stores a field for??? Ugh. Shouldn't we just have a `getCallSiteCFGElement` or 
something? Especially since we have `CFGElementRef`s now.

Would you be so nice to hunt this down please? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80366



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

[PATCH] D46665: [Itanium] Emit type info names with external linkage.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.
Herald added subscribers: mstorsjo, dexonsmith.

Note: this commit was re-reverted in

  commit bbb2655de0049f8e6cf4482702aa616011c851c1
  Author: Richard Smith 
  Date:   Mon May 21 20:10:54 2018 +
  
  Revert r332028; see PR37545 for details.
  
  llvm-svn: 332879


Repository:
  rC Clang

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

https://reviews.llvm.org/D46665



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

[PATCH] D47092: downgrade strong type info names to weak_odr linkage

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I'm revisiting http://llvm.org/PR37398 as it looks like a pretty serious bug to 
me.

I read http://llvm.org/D46665, this patch and http://llvm.org/PR37545, and my 
understanding is that this patch supersedes @EricWF 's original patch 
http://llvm.org/D46665. The only remaining problem would be 
http://llvm.org/PR37545. Is that correct?


Repository:
  rC Clang

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

https://reviews.llvm.org/D47092



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

[PATCH] D80018: [Analyzer][StreamChecker] Added check for "indeterminate file position".

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

balazske marked 4 inline comments as done.
balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:107
+  /// This value applies to all error states in ErrorState except FEOF.
+  /// An EOF+indeterminate state is the same as EOF state.
+  bool FilePositionIndeterminate = false;

Szelethus wrote:
> Szelethus wrote:
> > What does this mean? "An EOF+indeterminate state is the same as EOF state." 
> > I don't understand the message you want to convey here -- is it that we 
> > cannot have an indeterminate file position indicator if we hit EOF, hence 
> > we regard a stream that is **known** to be EOF to have its file position 
> > indicator determinate?
> > 
> > A good followup question for the uninitiated would be that "Well why is it 
> > ever legal to construct a `StreamState` object that can both have the 
> > `FilePositionIndeterminate` set to true and the `ErrorState` indicate that 
> > the steam is **known** to be in EOF?" Well, the answer is that we may only 
> > realize later that the error state can only be EOF, like in here:
> > ```lang=c++
> > void f() {
> >  FILE *F = fopen(...);
> >  if (fseek(F, ...)) {
> > // Could be either EOF, ERROR, and ofc indeterminate
> > if (eof(F)) {
> >   // This is where we have a seemingly impossible stream state, but its 
> > not a programming error, its a design decision.
> > }
> > }
> > ```
> > This might warrant a bit on explanation either here, or in 
> > `ensureNoFilePositionIndeterminate`. Probably the latter.
> > 
> > With that said, can `SteamState`'s constructor ensure that we do not create 
> > a known to be EOF stream that is indeterminate?
> Actually, not enforcing this could leave to false positives:
> 
> ```
> void f() {
>  FILE *F = fopen(...);
>  if (fseek(F, ...)) {
> // Could be either EOF, ERROR, and ofc indeterminate
> if (eof(F)) {
>   clearerr(F);
>   fseek(F, ...); // false positive warning
> }
> }
> ```
The comment wants to say only that if the **ErrorState** contains the 
**ErrorFEof** the value of `filePositionIndeterminate` is to be ignored for the 
EOF case. If the file is in EOF it does not matter what value 
`filePositionIndeterminate` has. The cause for this handling is that ErrorState 
handles multiple possible errors together but the indeterminate position does 
not apply to all. If **ErrorState** contains **ErrorFEof** and **ErrorFError** 
together and the `filePositionIndeterminate` is set, the position is not 
indeterminate in the EOF case. For EOF case we should know that the position is 
at the end of the file, not indeterminate.

Another solution for this problem can be to have a "ErrorFErrorIndeterminate" 
and "ErrorNoneIndeterminate" error type but this makes things more difficult to 
handle.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:306
+  ProgramStateRef
+  ensureNoFilePositionIndeterminate(SVal StreamVal, CheckerContext &C,
+ProgramStateRef State) const;

Szelethus wrote:
> Or `ensureFilePositionDeterminate`? :D
It is better to call "Invalid" or "Unknown" position, "indeterminate" is taken 
from the text of the C standard. I think "indeterminate" is a special name here 
that is better to have in this form always.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:589
+  // other code.
+  StreamState NewState = StreamState::getOpened(Desc, NewES, true);
   StateFailed = StateFailed->set(StreamSym, NewState);

This is a place where `FilePositionIndeterminate` is set in the state. We do 
not know if FEOF or FERROR will happen. If it is FERROR the position is 
indeterminate, if it is FEOF the position is not indeterminate.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:839-840
+
+// If only FEof is possible, report no warning (indeterminate position in
+// FEof is ignored).
+if (SS->ErrorState.isFEof())

Szelethus wrote:
> I think we shouldn't say its ignored, but rather its impossible. I mean, if 
> we have reached the end of the file, how could the file position indicator be 
> indeterminate?
The previous comment tells why it is set. The FEOF case is represented in the 
data structure with indeterminate flag set or not set. Both mean the same state 
of the modeled stream: FEOF and not indeterminate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80018



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

[PATCH] D77846: [analyzer][CallAndMessage][NFC] Split up checkPreCall

[Kristóf Umann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeeff1a970a6b: [analyzer][CallAndMessage][NFC] Split up 
checkPreCall (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77846

Files:
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -60,6 +60,25 @@
 
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
+  ProgramStateRef checkFunctionPointerCall(const CallExpr *CE,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXMethodCall(const CXXInstanceCall *CC,
+ CheckerContext &C,
+ ProgramStateRef State) const;
+
+  ProgramStateRef checkParameterCount(const CallEvent &Call, CheckerContext &C,
+  ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXDeallocation(const CXXDeallocatorCall *DC,
+   CheckerContext &C,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkArgInitializedness(const CallEvent &Call,
+  CheckerContext &C,
+  ProgramStateRef State) const;
+
 private:
   bool PreVisitProcessArg(CheckerContext &C, SVal V, SourceRange ArgRange,
   const Expr *ArgEx, int ArgumentNumber,
@@ -309,123 +328,131 @@
   return false;
 }
 
-void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
- CheckerContext &C) const {
-  ProgramStateRef State = C.getState();
-  if (const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr())) {
-const Expr *Callee = CE->getCallee()->IgnoreParens();
-const LocationContext *LCtx = C.getLocationContext();
-SVal L = State->getSVal(Callee, LCtx);
-
-if (L.isUndef()) {
-  if (!BT_call_undef)
-BT_call_undef.reset(new BuiltinBug(
-this, "Called function pointer is an uninitialized pointer value"));
-  emitBadCall(BT_call_undef.get(), C, Callee);
-  return;
-}
+ProgramStateRef CallAndMessageChecker::checkFunctionPointerCall(
+const CallExpr *CE, CheckerContext &C, ProgramStateRef State) const {
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(L.castAs());
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal L = State->getSVal(Callee, LCtx);
+
+  if (L.isUndef()) {
+if (!BT_call_undef)
+  BT_call_undef.reset(new BuiltinBug(
+  this, "Called function pointer is an uninitialized pointer value"));
+emitBadCall(BT_call_undef.get(), C, Callee);
+return nullptr;
+  }
 
-if (StNull && !StNonNull) {
-  if (!BT_call_null)
-BT_call_null.reset(new BuiltinBug(
-this, "Called function pointer is null (null dereference)"));
-  emitBadCall(BT_call_null.get(), C, Callee);
-  return;
-}
+  ProgramStateRef StNonNull, StNull;
+  std::tie(StNonNull, StNull) = State->assume(L.castAs());
 
-State = StNonNull;
+  if (StNull && !StNonNull) {
+if (!BT_call_null)
+  BT_call_null.reset(new BuiltinBug(
+  this, "Called function pointer is null (null dereference)"));
+emitBadCall(BT_call_null.get(), C, Callee);
+return nullptr;
   }
 
-  // If this is a call to a C++ method, check if the callee is null or
-  // undefined.
-  if (const CXXInstanceCall *CC = dyn_cast(&Call)) {
-SVal V = CC->getCXXThisVal();
-if (V.isUndef()) {
-  if (!BT_cxx_call_undef)
-BT_cxx_call_undef.reset(
-new BuiltinBug(this, "Called C++ object pointer is uninitialized"));
-  emitBadCall(BT_cxx_call_undef.get(), C, CC->getCXXThisExpr());
-  return;
-}
+  return StNonNull;
+}
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(V.castAs());
+ProgramStateRef CallAndMessageChecker::checkParameterCount(
+const CallEvent &Call, CheckerContext &C, ProgramStateRef State) const {
 
-if (StNull && !StNonNull) {
-  if (!BT_cxx_call_null)
-BT_cxx_call_null.reset(
-new BuiltinBug(this, "Called C++ object pointer is null"));
-  emitBadCall(BT_cxx_call_null.get(), C, CC->getCXXThisExpr());
-  return;
-}
+  // If we have a function or block declaration, we can make sure we pass
+  // enough parameters.
+  unsigned Params = Call.parameters().size();
+  if (Call.getNumArgs() >= P

[PATCH] D77474: [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:194
+   const ProgramPointTag *Tag = nullptr) {
+// Say this 3 times fast.
+State = State ? State : getState();

I like the joke, but this comment does not have any value, could you please 
remove?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:195
+// Say this 3 times fast.
+State = State ? State : getState();
+addTransition(State, generateSink(State, getPredecessor()));

balazske wrote:
> ```
> if (!State)
>   State = getState();
> ```
> is better? (I put the same comment into some (other?) patch already but maybe 
> it disappeared?)
+1 for balazske's suggestion.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1744-1745
 
+  // NOTE: There is a philosophical question to be answered when we detect a
+  // bug, but the checker under whose name we would emit the error is disabled.
+  // Generally speaking, the MallocChecker family is an integral part of the

This sentence is too bloated, I'd rather remove it.

AFAIU, the general rule of thumb is that if we have found a bug then we 
terminate further analysis, period. This is independent of whether we emit the 
warning or not, that actually depends on whether the corresponding subchecker 
is enabled or not.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024
+  if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) 
{
+C.addSink();
 return;

This seems to be inverse logic to me.
I'd expect that in a function called `Report...` we do stuff that is related to 
reporting only. That is why I think it would be better to have the condition 
and addSink before calling `Report...`. That way reporting and modeling would 
be even more separated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77474



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

[PATCH] D80374: [Clang] Enable KF and KC mode for [_Complex] __float128

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai created this revision.
nemanjai added reviewers: rjmccall, rsmith, PowerPC, hfinkel.
Herald added subscribers: dexonsmith, kbarton.
Herald added a reviewer: aaron.ballman.
Herald added a project: clang.

The headers provided with recent GNU toolchains for PPC have code that includes 
typedefs such as:
`typedef _Complex float __cfloat128 __attribute__ ((__mode__ (__KC__)))`

Also, when I added `__float128`, I neglected to add support for `_Complex 
__float128` altogether. This patch fixes those oversights and allows clang to 
compile something like:

  #include 
  _Complex __float128 testkf(_Complex __float128 a, _Complex __float128 b) {
return a + b;
  }

with `-mfloat128` which it currently fails to compile due to the two reasons 
listed above.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80374

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/ppc64-complex-parms.c
  clang/test/CodeGen/ppc64-complex-return.c
  clang/test/Sema/attr-mode.c

Index: clang/test/Sema/attr-mode.c
===
--- clang/test/Sema/attr-mode.c
+++ clang/test/Sema/attr-mode.c
@@ -4,6 +4,8 @@
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple powerpc64-pc-linux-gnu -DTEST_64BIT_PPC64 -fsyntax-only \
 // RUN:   -verify %s
+// RUN: %clang_cc1 -triple powerpc64-pc-linux-gnu -DTEST_F128_PPC64 -fsyntax-only \
+// RUN:   -verify -target-feature +float128 %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnux32 -DTEST_64BIT_X86 -fsyntax-only \
 // RUN:   -verify %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -DTEST_MIPS_32 -fsyntax-only \
@@ -90,6 +92,13 @@
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+#elif TEST_F128_PPC64
+typedef _Complex float cf128 __attribute__ ((mode (KC)));
+typedef float f128 __attribute__ ((mode (KF)));
+void f_f128_arg(__float128 *x);
+void f_f128_complex_arg(_Complex __float128 *x);
+void test_KFtype(f128 *a) { f_f128_arg (a); }
+void test_KCtype(cf128 *a) { f_f128_complex_arg (a); }
 #elif TEST_MIPS_32
 typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
 int foo[sizeof(gcc_unwind_word) == 4 ? 1 : -1];
Index: clang/test/CodeGen/ppc64-complex-return.c
===
--- clang/test/CodeGen/ppc64-complex-return.c
+++ clang/test/CodeGen/ppc64-complex-return.c
@@ -1,9 +1,20 @@
 // REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+_Complex __float128 foo_f128(_Complex __float128 x) {
+  return x;
+}
+
+// CHECK-F128: define { fp128, fp128 } @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, fp128 {{[%A-Za-z0-9.]+}}) [[NUW:#[0-9]+]] {
+#endif
 
 _Complex float foo_float(_Complex float x) {
   return x;
@@ -80,6 +91,17 @@
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 0
 // CHECK: extractvalue { ppc_fp128, ppc_fp128 } [[VAR3]], 1
 
+#ifdef TEST_F128
+__float128 bar_f128(void) {
+  return crealf128(foo_f128(2.0Q - 2.5Qi));
+}
+
+// CHECK-F128: define fp128 @bar_f128() [[NUW]] {
+// CHECK-F128: [[VAR3:[%A-Za-z0-9.]+]] = call { fp128, fp128 } @foo_f128
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 0
+// CHECK-F128: extractvalue { fp128, fp128 } [[VAR3]], 1
+#endif
+
 int bar_int(void) {
   return __real__(foo_int(2 - 3i));
 }
Index: clang/test/CodeGen/ppc64-complex-parms.c
===
--- clang/test/CodeGen/ppc64-complex-parms.c
+++ clang/test/CodeGen/ppc64-complex-parms.c
@@ -1,8 +1,20 @@
+// REQUIRES: powerpc-registered-target
 // RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -target-feature +float128 -DTEST_F128 -triple \
+// RUN:   powerpc64le-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s \
+// RUN:   --check-prefix CHECK-F128
 
 float crealf(_Complex float);
 double creal(_Complex double);
 long double creall(_Complex long double);
+#ifdef TEST_F128
+__float128 crealf128(_Complex __float128);
+__float128 foo_f128(_Complex __float128 x) {
+  return crealf128(x);
+}
+// CHECK-F128: define fp128 @foo_f128(fp128 {{[%A-Za-z0-9.]+}}, fp128 {{[%A-Za-z0-9.]+}})
+#endif
+
 
 float foo_float(_Complex float x) {
   return crealf(x);
Index: clang/lib/Sema/SemaDeclAttr.cpp

[PATCH] D78099: [analyzer][RetainCount] Tie diagnostics to osx.cocoa.RetainCount rather then RetainCountBase, for the most part

[Gabor Marton via Phabricator via cfe-commits]

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

Found a small mistake that should be corrected, then will be good to me.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp:15
 #include "RetainCountChecker.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"

Why adding this extra header? Don't blame clangd :D



Comment at: clang/test/Analysis/test-separate-retaincount.cpp:5
 //
-// RUN: %clang_analyze_cc1 -std=c++14 -DNO_OS_OBJECT -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++14 -verify=no-retain-count %s \
 // RUN:   -analyzer-checker=core,osx \

This is mixed up with line 1, this line should be `no-os-object`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78099



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

[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

[Gabor Marton via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2132
+
+  assert(!isDependency(Registry, bt.getCheckerName()) &&
+ "Some checkers depend on this one! We don't allow dependency "

Oh yeah! :)


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

https://reviews.llvm.org/D78126



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

[PATCH] D79710: [clang][BFloat] add create/set/get/dup intrinsics

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

In D79710#2041418 , @LukeGeeson wrote:

> Can you update the commit message in this differential as well please? Same 
> for the other commits :)


done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79710



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 265479.
yaxunl marked an inline comment as done.
yaxunl edited the summary of this revision.
yaxunl added a comment.
Herald added subscribers: kerbowa, nhaehnle, jvesely.

Added TargetInfo::isFPAtomicFetchAddSubSupported to guard fp atomic.


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

https://reviews.llvm.org/D71726

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu
  clang/test/CodeGenOpenCL/atomic-ops.cl
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=spir64
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify=expected,spir \
+// RUN:   -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only \
+// RUN:   -triple=amdgcn-amd-amdhsa
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
@@ -36,7 +38,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *d,
+   atomic_intptr_t *p, atomic_float *d, atomic_double *d2,
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
@@ -70,7 +72,8 @@
 
   __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // spir-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(d2, 1, memory_order_seq_cst, memory_scope_work_group); // spir-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_double *' (aka '__generic _Atomic(double) *') invalid)}}
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
Index: clang/test/CodeGenOpenCL/atomic-ops.cl
===
--- clang/test/CodeGenOpenCL/atomic-ops.cl
+++ clang/test/CodeGenOpenCL/atomic-ops.cl
@@ -1,12 +1,17 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -O0 -o - -triple=amdgcn-amd-amdhsa-amdgizcl | opt -instnamer -S | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -O0 -o - -triple=amdgcn-amd-amdhsa \
+// RUN:   | opt -instnamer -S | FileCheck %s
 
 // Also test serialization of atomic operations here, to avoid duplicating the test.
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-pch -O0 -o %t -triple=amdgcn-amd-amdhsa-amdgizcl
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -include-pch %t -O0 -triple=amdgcn-amd-amdhsa-amdgizcl -emit-llvm -o - | opt -instnamer -S | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-pch -O0 -o %t -triple=amdgcn-amd-amdhsa
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -include-pch %t -O0 -triple=amdgcn-amd-amdhsa \
+// RUN:   -emit-llvm -o - | opt -instnamer -S | FileCheck %s
 
 #ifndef ALREADY_INCLUDED
 #define ALREADY_INCLUDED
 
+#pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+
 typedef __INTPTR_TYPE__ intptr_t;
 typedef int int8 __attribute__((ext_vector_type(8)));
 
@@ -185,6 +190,18 @@
   return __opencl_atomic_exchange(d, 2, memory_order_seq_cst, memory_scope_work_group);
 }
 
+float ff4(global atomic_float *d, float a) {
+  // CHECK-LABEL: @ff4
+  // CHECK: atomicrmw fadd float addrspace(1)* {{.*}} syncscope("workgroup-one-as") monotonic
+  return __opencl_atomic_fetch_add(d, a, memory_order_relaxed, memory_scope_work_group);
+}
+
+float ff5(global atomic_double *d, double a) {
+  // CHECK-LABEL: @ff5
+  // CHECK: atomicrmw fadd double addrspace(1)* {{.*}} syncscope("workgroup-one-as") monotonic
+  return __opencl_atomic_fetch_add(d, a, memory_order_relaxed, memory_scope_w

[PATCH] D80369: WIP: [DebugInfo] Remove decl subprograms from 'retainedTypes:'

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 265481.
djtodoro retitled this revision from "[DebugInfo] Remove decl subprograms from 
'retainedTypes:'" to "WIP: [DebugInfo] Remove decl subprograms from 
'retainedTypes:'".
djtodoro added a comment.

Still have test failing:

  Clang :: Modules/DebugInfoTransitiveImport.m
  Clang :: Modules/ModuleDebugInfo.cpp
  Clang :: Modules/ModuleDebugInfo.m

I haven't looked into the failures, but I guess something Objective-C++ related 
should be handled with some additional work here.


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

https://reviews.llvm.org/D80369

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,6 +1,10 @@
 // When entry values are emitted, expect a subprogram for extern decls so that
 // the dwarf generator can describe call site parameters at extern call sites.
 //
+// Initial implementation relied on the 'retainedTypes:' from the corresponding
+// DICompileUnit, so we also ensure that we do not store the extern declaration
+// subprogram into the 'retainedTypes:'.
+//
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
 
@@ -17,6 +21,8 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: 
![[RETTYPES:[0-9]+]]
+// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3871,7 +3871,7 @@
   if (IsDeclForCallSite)
 Fn->setSubprogram(SP);
 
-  DBuilder.retainType(SP);
+  DBuilder.finalizeSubprogram(SP);
 }
 
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,


Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,6 +1,10 @@
 // When entry values are emitted, expect a subprogram for extern decls so that
 // the dwarf generator can describe call site parameters at extern call sites.
 //
+// Initial implementation relied on the 'retainedTypes:' from the corresponding
+// DICompileUnit, so we also ensure that we do not store the extern declaration
+// subprogram into the 'retainedTypes:'.
+//
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=DECLS-FOR-EXTERN
 
@@ -17,6 +21,8 @@
 // RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=NO-DECLS-FOR-EXTERN
 
+// DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
+// DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
 // DECLS-FOR-EXTERN: !DISubprogram(name: "fn1"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "memcmp"
 // DECLS-FOR-EXTERN-NOT: !DISubprogram(name: "__some_reserved_name"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3871,7 +3871,7 @@
   if (IsDeclForCallSite)
 Fn->setSubprogram(SP);
 
-  DBuilder.retainType(SP);
+  DBuilder.finalizeSubprogram(SP);
 }
 
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79921: [OPENMP] Fix mixture of omp and clang pragmas

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D79921



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

[clang] 361e4f1 - Fix debug info for NoDebug attr

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-05-21T09:02:56-04:00
New Revision: 361e4f14e35981e7498a38ab7c31da7fb7414fb8

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

LOG: Fix debug info for NoDebug attr

NoDebug attr does not totally eliminate debug info about a function when
inlining is enabled. This is inconsistent with when inlining is disabled.

This patch fixes that.

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

Added: 
clang/test/CodeGen/nodebug-attr.c

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCUDA/kernel-dbg-info.cu

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 88ddb44ed762..7ec792ca0e1f 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3885,12 +3885,12 @@ void 
CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   if (Func->getSubprogram())
 return;
 
-  // Do not emit a declaration subprogram for a builtin or if call site info
-  // isn't required. Also, elide declarations for functions with reserved 
names,
-  // as call site-related features aren't interesting in this case (& also, the
-  // compiler may emit calls to these functions without debug locations, which
-  // makes the verifier complain).
-  if (CalleeDecl->getBuiltinID() != 0 ||
+  // Do not emit a declaration subprogram for a builtin, a function with 
nodebug
+  // attribute, or if call site info isn't required. Also, elide declarations
+  // for functions with reserved names, as call site-related features aren't
+  // interesting in this case (& also, the compiler may emit calls to these
+  // functions without debug locations, which makes the verifier complain).
+  if (CalleeDecl->getBuiltinID() != 0 || CalleeDecl->hasAttr() ||
   getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
 return;
   if (const auto *Id = CalleeDecl->getIdentifier())

diff  --git a/clang/test/CodeGen/nodebug-attr.c 
b/clang/test/CodeGen/nodebug-attr.c
new file mode 100644
index ..63a9d30d36fe
--- /dev/null
+++ b/clang/test/CodeGen/nodebug-attr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O3 \
+// RUN:   -debug-info-kind=limited -o - -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   | FileCheck %s
+
+// Makes sure there is no !dbg between function attributes and '{'.
+// CHECK-LABEL: define void @foo{{.*}} #{{[0-9]+}} {
+// CHECK-NOT: ret {{.*}}!dbg
+__attribute__((nodebug)) void foo(int *a) {
+  *a = 1;
+}
+
+// CHECK-LABEL: define {{.*}}@bar{{.*}}!dbg
+void bar(int *a) {
+  foo(a);
+}

diff  --git a/clang/test/CodeGenCUDA/kernel-dbg-info.cu 
b/clang/test/CodeGenCUDA/kernel-dbg-info.cu
index a1a70d2cbaf2..9cfad165e9ce 100644
--- a/clang/test/CodeGenCUDA/kernel-dbg-info.cu
+++ b/clang/test/CodeGenCUDA/kernel-dbg-info.cu
@@ -2,11 +2,28 @@
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O0 \
 // RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
-// RUN:   -o - -x hip | FileCheck %s
+// RUN:   -o - -x hip | FileCheck -check-prefixes=CHECK,O0 %s
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O0 \
 // RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
 // RUN:   -o - -x hip -fcuda-is-device | FileCheck -check-prefix=DEV %s
 
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O0 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   | FileCheck -check-prefixes=CHECK,O0 %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O0 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   -fcuda-is-device | FileCheck -check-prefix=DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O3 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O3 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   -fcuda-is-device | FileCheck -check-prefix=DEV %s
+
 #include "Inputs/cuda.h"
 
 extern "C" __global__ void ckernel(int *a) {
@@ -20,14 +37,14 @@ extern "C" __global__ void ckernel(int *a) {
 // DEV:  store {{.*}}!dbg
 // DEV:  ret {{.*}}!dbg
 
-// CHECK-NOT: define {{.*}}@__device_stub__ckernel{{.*}}!dbg
-// CHECK: define {{.*}}@[[CSTUB:__device_stub__ckernel]]
+// Make sure there is no !dbg between function attributes and '{'
+// CHECK: define void @[[CSTUB:__device_stub__ckernel]]{{.*}} #{{[0-9]+}} {
 // CHECK-NOT: call {{.*}}@hipLaunchByPtr{{.*}}!dbg
 // 

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265487.
bader added a comment.

Enable diagnostics for non-OpenCL modes and applied refactoring proposed by 
John.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address-space-arithmetic.cpp


Index: clang/test/Sema/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/Sema/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type 
 ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,15 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();
+Qualifiers TQ = T.getQualifiers();
+// Address spaces overlap if at least one of them is a superset of another
+return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
+  }
+
   /// Returns gc attribute of this type.
   inline Qualifiers::GC getObjCGCAttr() const;
 


Index: clang/test/Sema/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/Sema/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to }}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,15 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();
+Qualifiers TQ = T.getQualifiers();
+// Address spaces overlap if at least one of them is a superset of another
+return Q.isAddressSpaceSupersetOf(TQ) || TQ.isAddressSpaceSupersetOf(Q);
+  }
+
   /// Returns gc attribute of this type.
   inline Qualifiers::GC getObjCGCAttr() const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79967: Fix debug info for NoDebug attr

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG361e4f14e359: Fix debug info for NoDebug attr (authored by 
yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79967

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/nodebug-attr.c
  clang/test/CodeGenCUDA/kernel-dbg-info.cu


Index: clang/test/CodeGenCUDA/kernel-dbg-info.cu
===
--- clang/test/CodeGenCUDA/kernel-dbg-info.cu
+++ clang/test/CodeGenCUDA/kernel-dbg-info.cu
@@ -2,11 +2,28 @@
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O0 \
 // RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
-// RUN:   -o - -x hip | FileCheck %s
+// RUN:   -o - -x hip | FileCheck -check-prefixes=CHECK,O0 %s
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O0 \
 // RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
 // RUN:   -o - -x hip -fcuda-is-device | FileCheck -check-prefix=DEV %s
 
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O0 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   | FileCheck -check-prefixes=CHECK,O0 %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O0 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   -fcuda-is-device | FileCheck -check-prefix=DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O3 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm %s -O3 \
+// RUN:   -fcuda-include-gpubinary %t -debug-info-kind=limited \
+// RUN:   -o - -x hip -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   -fcuda-is-device | FileCheck -check-prefix=DEV %s
+
 #include "Inputs/cuda.h"
 
 extern "C" __global__ void ckernel(int *a) {
@@ -20,14 +37,14 @@
 // DEV:  store {{.*}}!dbg
 // DEV:  ret {{.*}}!dbg
 
-// CHECK-NOT: define {{.*}}@__device_stub__ckernel{{.*}}!dbg
-// CHECK: define {{.*}}@[[CSTUB:__device_stub__ckernel]]
+// Make sure there is no !dbg between function attributes and '{'
+// CHECK: define void @[[CSTUB:__device_stub__ckernel]]{{.*}} #{{[0-9]+}} {
 // CHECK-NOT: call {{.*}}@hipLaunchByPtr{{.*}}!dbg
 // CHECK: call {{.*}}@hipLaunchByPtr{{.*}}@[[CSTUB]]
 // CHECK-NOT: ret {{.*}}!dbg
 
 // CHECK-LABEL: define {{.*}}@_Z8hostfuncPi{{.*}}!dbg
-// CHECK: call void @[[CSTUB]]{{.*}}!dbg
+// O0: call void @[[CSTUB]]{{.*}}!dbg
 void hostfunc(int *a) {
   ckernel<<<1, 1>>>(a);
 }
Index: clang/test/CodeGen/nodebug-attr.c
===
--- /dev/null
+++ clang/test/CodeGen/nodebug-attr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -O3 \
+// RUN:   -debug-info-kind=limited -o - -debugger-tuning=gdb -dwarf-version=4 \
+// RUN:   | FileCheck %s
+
+// Makes sure there is no !dbg between function attributes and '{'.
+// CHECK-LABEL: define void @foo{{.*}} #{{[0-9]+}} {
+// CHECK-NOT: ret {{.*}}!dbg
+__attribute__((nodebug)) void foo(int *a) {
+  *a = 1;
+}
+
+// CHECK-LABEL: define {{.*}}@bar{{.*}}!dbg
+void bar(int *a) {
+  foo(a);
+}
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3885,12 +3885,12 @@
   if (Func->getSubprogram())
 return;
 
-  // Do not emit a declaration subprogram for a builtin or if call site info
-  // isn't required. Also, elide declarations for functions with reserved 
names,
-  // as call site-related features aren't interesting in this case (& also, the
-  // compiler may emit calls to these functions without debug locations, which
-  // makes the verifier complain).
-  if (CalleeDecl->getBuiltinID() != 0 ||
+  // Do not emit a declaration subprogram for a builtin, a function with 
nodebug
+  // attribute, or if call site info isn't required. Also, elide declarations
+  // for functions with reserved names, as call site-related features aren't
+  // interesting in this case (& also, the compiler may emit calls to these
+  // functions without debug locations, which makes the verifier complain).
+  if (CalleeDecl->getBuiltinID() != 0 || CalleeDecl->hasAttr() ||
   getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
 return;
   if (const auto *Id = CalleeDecl->getIdentifier())


Index: clang/test/CodeGenCUDA/kernel-dbg-info.cu
===
--- clang/test/CodeGenCUDA/kernel-dbg-info.cu
+++ clang/test/CodeGenCUDA/kernel-dbg-info.cu
@@ -2,11 +2,28 @@
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit

[PATCH] D80295: [Sema] Diagnose static data members in classes nested in unnamed classes

[John Brawn via Phabricator via cfe-commits]

john.brawn updated this revision to Diff 265486.
john.brawn marked an inline comment as done.
john.brawn added a comment.

Use the tag of the anonymous struct when emitting a diagnostic.


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

https://reviews.llvm.org/D80295

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/anonymous-struct.cpp


Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
   const Empty E;
 } C;
 } // namespace ImplicitDecls
+
+struct {
+  static int x; // expected-error {{static data member 'x' not allowed in 
anonymous struct}}
+} static_member_1;
+
+class {
+  struct A {
+static int x; // expected-error {{static data member 'x' not allowed in 
anonymous class}}
+  } x;
+} static_member_2;
+
+union {
+  struct A {
+struct B {
+  static int x; // expected-error {{static data member 'x' not allowed in 
anonymous union}}
+} x;
+  } x;
+} static_member_3;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,19 +6885,29 @@
 
 if (SC == SC_Static && CurContext->isRecord()) {
   if (const CXXRecordDecl *RD = dyn_cast(DC)) {
-// C++ [class.static.data]p2:
-//   A static data member shall not be a direct member of an unnamed
-//   or local class
-// FIXME: or of a (possibly indirectly) nested class thereof.
-if (RD->isLocalClass()) {
-  Diag(D.getIdentifierLoc(),
-   diag::err_static_data_member_not_allowed_in_local_class)
-<< Name << RD->getDeclName() << RD->getTagKind();
-} else if (!RD->getDeclName()) {
+const CXXRecordDecl *AnonStruct = nullptr;
+for (DeclContext *Ctxt = DC;
+ const CXXRecordDecl *ParentDecl = dyn_cast(Ctxt);
+ Ctxt = Ctxt->getParent()) {
+  if (!ParentDecl->getDeclName()) {
+AnonStruct = ParentDecl;
+break;
+  }
+}
+if (AnonStruct) {
+  // C++ [class.static.data]p4: Unnamed classes and classes contained
+  // directly or indirectly within unnamed classes shall not contain
+  // static data members.
   Diag(D.getIdentifierLoc(),
diag::err_static_data_member_not_allowed_in_anon_struct)
-<< Name << RD->getTagKind();
+<< Name << AnonStruct->getTagKind();
   Invalid = true;
+} else if (RD->isLocalClass()) {
+  // C++ [class.static.data]p5: A local class shall not have static 
data
+  // members.
+  Diag(D.getIdentifierLoc(),
+   diag::err_static_data_member_not_allowed_in_local_class)
+<< Name << RD->getDeclName() << RD->getTagKind();
 } else if (RD->isUnion()) {
   // C++98 [class.union]p1: If a union contains a static data member,
   // the program is ill-formed. C++11 drops this restriction.


Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -153,3 +153,21 @@
   const Empty E;
 } C;
 } // namespace ImplicitDecls
+
+struct {
+  static int x; // expected-error {{static data member 'x' not allowed in anonymous struct}}
+} static_member_1;
+
+class {
+  struct A {
+static int x; // expected-error {{static data member 'x' not allowed in anonymous class}}
+  } x;
+} static_member_2;
+
+union {
+  struct A {
+struct B {
+  static int x; // expected-error {{static data member 'x' not allowed in anonymous union}}
+} x;
+  } x;
+} static_member_3;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6885,19 +6885,29 @@
 
 if (SC == SC_Static && CurContext->isRecord()) {
   if (const CXXRecordDecl *RD = dyn_cast(DC)) {
-// C++ [class.static.data]p2:
-//   A static data member shall not be a direct member of an unnamed
-//   or local class
-// FIXME: or of a (possibly indirectly) nested class thereof.
-if (RD->isLocalClass()) {
-  Diag(D.getIdentifierLoc(),
-   diag::err_static_data_member_not_allowed_in_local_class)
-<< Name << RD->getDeclName() << RD->getTagKind();
-} else if (!RD->getDeclName()) {
+const CXXRecordDecl *AnonStruct = nullptr;
+for (DeclContext *Ctxt = DC;
+ const CXXRecordDecl *ParentDecl = dyn_cast(Ctxt);
+ Ctxt = Ctxt->getParent()) {
+  if (!ParentDecl->getDeclName()) {
+AnonStruct = ParentDecl;
+break;
+  }
+}
+if (AnonStruct) {
+ 

[clang] 1c8f999 - [analyzer][CallAndMessage] Add checker options for each bug type

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-05-21T15:31:37+02:00
New Revision: 1c8f999e0b59731a4214f76528f83e4196e1fcc3

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

LOG: [analyzer][CallAndMessage] Add checker options for each bug type

iAs listed in the summary D77846, we have 5 different categories of bugs we're
checking for in CallAndMessage. I think the documentation placed in the code
explains my thought process behind my decisions quite well.

A non-obvious change I had here is removing the entry for
CallAndMessageUnInitRefArg. In fact, I removed the CheckerNameRef typed field
back in D77845 (it was dead code), so that checker didn't really exist in any
meaningful way anyways.

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

Added: 
clang/test/Analysis/call-and-message.c
clang/test/Analysis/call-and-message.cpp
clang/test/Analysis/call-and-message.m
clang/test/Analysis/call-and-message.mm

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
clang/test/Analysis/PR40625.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/exercise-ps.c
clang/test/Analysis/uninit-const.c
clang/test/Analysis/uninit-const.cpp

Removed: 
clang/test/Analysis/reference.mm
clang/test/Analysis/uninit-msg-expr.m



diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index d06c64ad118b..93c4d964d772 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -122,14 +122,70 @@ def FuchsiaAlpha : Package<"fuchsia">, 
ParentPackage;
 
 let ParentPackage = Core in {
 
-def DereferenceChecker : Checker<"NullDereference">,
-  HelpText<"Check for dereferences of null pointers">,
+def CallAndMessageModeling : Checker<"CallAndMessageModeling">,
+  HelpText<"Responsible for essential modeling and assumptions after a "
+   "function/method call. For instance, if we can't reason about the "
+   "nullability of the implicit this parameter after a method call, "
+   "this checker conservatively assumes it to be non-null">,
   Documentation;
 
 def CallAndMessageChecker : Checker<"CallAndMessage">,
   HelpText<"Check for logical errors for function calls and Objective-C "
"message expressions (e.g., uninitialized arguments, null function "
"pointers)">,
+  CheckerOptions<[
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+  ]>,
+  Documentation,
+  Dependencies<[CallAndMessageModeling]>;
+
+def DereferenceChecker : Checker<"NullDereference">,
+  HelpText<"Check for dereferences of null pointers">,
   Documentation;
 
 def NonNullParamChecker : Checker<"NonNullParamChecker">,
@@ -211,13 +267,6 @@ def SizeofPointerChecker : Checker<"SizeofPtr">,
   HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
   Documentation;
 
-def CallAndMessageUnInitRefArg : Checker<"CallAndMessageUnInitRefArg">,
-  HelpText<"Check for logical errors for function calls and Objective-C "
-   "message expressions (e.g., uninitialized arguments, null function "
-   "pointers, and pointer to undefined variables)">,
-  Dependencies<[CallAndMessageChecker]>,
-  Documentation;
-
 def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
   HelpText<"Check for division by variable that is later compared against 0. "
"Either the comparison is useless or there is division by zero.">,
@@ -295,7 +344,7 @@ let ParentPackage = APIModeling in {
 
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
-  Dependencies<[NonNullParamChecker, CallAndMessageChecker]>,
+  Dependencies<[NonNullParamChecker, CallAndMessageModeling]>,
   CheckerOptions<[
 CmdLineOption BT_call_few_args;
 
 public:
-  enum CheckKind { CK_CallAndMessageUnInitRefArg, CK_NumCheckKinds };
+  // These correspond with the checker options. Looking at other checkers such
+  // as MallocChecker and CStringChecker, this is similar as to how they pull
+  // off having a modeling class, but emitting diagnostics under a smaller
+  // checker's name that can be safely disabled without disturbing the
+  // underlaying modeling engine.
+  // The reason behind having *checker options* rather then actual *checkers*
+  // here is that CallAndMessage is among the oldest checkers out there, and 
can
+  // be respons

[PATCH] D80294: Add support for vmsumudm

[Ahsan Saghir via Phabricator via cfe-commits]

saghir added a comment.

In D80294#2048049 , @amyk wrote:

> I think this overall looks good, but just curious, why was the builtin 
> support removed?


I was hitting some errors with that, so removed it and would add that later on.


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

https://reviews.llvm.org/D80294



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:452
   CmdArgs.push_back("-dynamic-linker");
-  CmdArgs.push_back(Args.MakeArgString(Loader));
+  CmdArgs.push_back(Args.MakeArgString(Twine(D.DyldPrefix) +
+   ToolChain.getDynamicLinker(Args)));

nemanjai wrote:
> Is this just an orthogonal NFC change? If so, can you please commit it 
> separately in an NFC commit?
Yes; all of the other changes made around here disappeared during the 
development process. Would it be okay to split it out on the commit instead of 
updating the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked 4 inline comments as done.
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:66
+  // convert these to actual checkers.
+  enum CheckKind {
+CK_FunctionPointer,

martong wrote:
> So, we are not going to have subcheckers, rather options, okay. Perhaps the 
> comment would be more appropriate to be in `Checkers.td`? (If we can put 
> comments there.)
I prefer it here, `Checkers.td` is a rarely visited file, and the comments in 
the test cases all point here where the majority of the work has to be done. 
Which still isn't much, but this is the bit we should not screw up.

Btw we can totally put comments there, as seen here: D78120


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77866



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

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I like these changes.
I have mixed feelings about `isCpp()` & co.
As @MyDeveloperDay said, I'd like it mean C++ only. I find it confusing that it 
means C++ or ObjC (even if the latter is a superset of the former). I'd rather 
see it spelt as `isCppOrObjC()` even if it's verbose but at least removes all 
confusion IMO.




Comment at: clang/include/clang/Format/Format.h:1632
+  bool isCppOnly() const { return Language == LK_Cpp; }
+  bool isObjectiveC() const { return Language == LK_ObjC; }
+  bool isCpp() const { return isCppOnly() || isObjectiveC(); }

sammccall wrote:
> Just my 2c - I find the current meaning of isCpp easier to understand, and 
> would prefer isObjectiveC to mean objective-C/C++. h if it exists.
> 
> Reasons:
>  - this is consistent with LangOptions::CPlusPlus and LangOptions::ObjC
>  - when checking for C++, also applying these rules to ObjC++ should be the 
> common/default case, and excluding ObjC++ the special case that justifies 
> more precise syntax (and honestly, I'd find `isCpp && !isObjC` to carry the 
> clearest intent in that case). IOW, this seems like it will attract bugs.
> 
> > perhaps a better name for isCpp() is isCStyleLanguages()
> 
> Clang uses the term "C family languages", and this includes C, C++, ObjC, 
> ObjC++.
> If you really want to avoid the conflict between C++ the boolean language 
> option and C++ the precise language mode, I'd suggest `isPlusPlus()` and 
> `isObjective()`. But I think consistency with LangOptions is worth more.
I'd rather go for coherence with `LanguageKind` options and spell it `isObjC()`.



Comment at: clang/include/clang/Format/Format.h:1635
   bool isCSharp() const { return Language == LK_CSharp; }
+  bool isProtoBuf() const { return Language == LK_Proto; }
+  bool isTableGen() const { return Language == LK_TableGen; }

sammccall wrote:
> These functions that don't *even in principle* do more than compare to an 
> enum seem like extra indirection that hurts understanding of the code (have 
> to look up what isObjectiveC() does, or have subtle bugs).
> 
> I suspect isCSharp() was added due to a misunderstanding of what isCpp() was 
> for.
Ditto, maybe `isProto` and `isTextProto`?


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

https://reviews.llvm.org/D80079



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

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

[Kristóf Umann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Szelethus marked an inline comment as done.
Closed by commit rG1c8f999e0b59: [analyzer][CallAndMessage] Add checker options 
for each bug type (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D77866?vs=256547&id=265498#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77866

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  clang/test/Analysis/PR40625.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/call-and-message.c
  clang/test/Analysis/call-and-message.cpp
  clang/test/Analysis/call-and-message.m
  clang/test/Analysis/call-and-message.mm
  clang/test/Analysis/exercise-ps.c
  clang/test/Analysis/reference.mm
  clang/test/Analysis/uninit-const.c
  clang/test/Analysis/uninit-const.cpp
  clang/test/Analysis/uninit-msg-expr.m

Index: clang/test/Analysis/uninit-msg-expr.m
===
--- clang/test/Analysis/uninit-msg-expr.m
+++ /dev/null
@@ -1,56 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -verify %s
-
-//===--===//
-// The following code is reduced using delta-debugging from
-// Foundation.h (Mac OS X).
-//
-// It includes the basic definitions for the test cases below.
-// Not directly including Foundation.h directly makes this test case 
-// both svelte and portable to non-Mac platforms.
-//===--===//
-
-typedef signed char BOOL;
-typedef unsigned int NSUInteger;
-typedef struct _NSZone NSZone;
-@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
-@protocol NSObject  - (BOOL)isEqual:(id)object; @end
-@protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
-@protocol NSMutableCopying  - (id)mutableCopyWithZone:(NSZone *)zone; @end
-@protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
-@interface NSObject  {} @end
-@class NSString, NSData;
-@class NSString, NSData, NSMutableData, NSMutableDictionary, NSMutableArray;
-typedef struct {} NSFastEnumerationState;
-@protocol NSFastEnumeration
-- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len;
-@end
-@class NSData, NSIndexSet, NSString, NSURL;
-@interface NSArray : NSObject 
-- (NSUInteger)count;
-@end
-@interface NSArray (NSArrayCreation)
-+ (id)array;
-- (NSUInteger)length;
-- (void)addObject:(id)object;
-@end
-extern NSString * const NSUndoManagerCheckpointNotification;
-
-//===--===//
-// Test cases.
-//===--===//
-
-unsigned f1() {
-  NSString *aString;
-  return [aString length]; // expected-warning {{Receiver in message expression is an uninitialized value}}
-}
-
-unsigned f2() {
-  NSString *aString = 0;
-  return [aString length]; // no-warning
-}
-
-void f3() {
-  NSMutableArray *aArray = [NSArray array];
-  NSString *aString;
-  [aArray addObject:aString]; // expected-warning {{1st argument in message expression is an uninitialized value}}
-}
Index: clang/test/Analysis/uninit-const.cpp
===
--- clang/test/Analysis/uninit-const.cpp
+++ clang/test/Analysis/uninit-const.cpp
@@ -1,5 +1,14 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete,core,alpha.core.CallAndMessageUnInitRefArg -analyzer-output=text -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.NewDelete,core,alpha.core.CallAndMessageUnInitRefArg -analyzer-output=text -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-output=text -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDelete \
+// RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
+
+// RUN: %clang_analyze_cc1 -analyzer-output=text -verify %s \
+// RUN:   -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDelete \
+// RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
+
 // Passing uninitialized const data to unknown function
 
 #include "Inputs/system-header-simulator-cxx.h"
Index: clang/test/Analysis/uninit-const.c
===
--- clang/test/Analysis/uninit-const.c
+++ clang/test/Analysis/uninit-const.c
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.Malloc,core,alpha.core.CallAndMessageUnInitRefArg,debug.ExprInspection -analyzer-

[PATCH] D80295: [Sema] Diagnose static data members in classes nested in unnamed classes

[John Brawn via Phabricator via cfe-commits]

john.brawn added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:6904
 << Name << RD->getTagKind();
   Invalid = true;
+} else if (RD->isLocalClass()) {

rjmccall wrote:
> This diagnostic actually ignores the tag kind that passed down to it, which 
> should be fixed.  Also, you should pass in the tag kind for the actual 
> anonymous class you found.
> 
> While I'm looking at this code: `isLocalClass` is mis-designed and doesn't 
> work for any of our non-`FunctionDecl` local contexts.  This check should be 
> `if (RD->getParentFunctionOrMethod())`.
> This diagnostic actually ignores the tag kind that passed down to it, which 
> should be fixed. Also, you should pass in the tag kind for the actual 
> anonymous class you found.

Will do.

> While I'm looking at this code: isLocalClass is mis-designed and doesn't work 
> for any of our non-FunctionDecl local contexts. This check should be if 
> (RD->getParentFunctionOrMethod()).

CXXMethodDecl has FunctionDecl as a parent class, so isLocalClass will find and 
return a CXXMethodDecl. Checking it on

```
class Example {
  void method() {
class X {
  static int x;
};
  }
};
```
I get the error as expected. Do you have an example where it doesn't work?


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

https://reviews.llvm.org/D80295



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

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

[Benson Chu via Phabricator via cfe-commits]

pestctrl added a comment.

Ah, you're right. I don't see the clause in the C11 standard. I'll see what I 
can do. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945



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

[clang] 3ef1134 - Fix DeferredDiagnosticsEmitter for bug#45987

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-05-21T11:01:40-04:00
New Revision: 3ef11346f391e6e3da0cfa25f9f7dac22771438e

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

LOG: Fix DeferredDiagnosticsEmitter for bug#45987

InOMPDeviceContext may be greater than 1. It needs to be clamp to 0 and 1
to be used as index for DoneMap.

Added: 
clang/test/OpenMP/deferred-diags.cpp

Modified: 
clang/lib/Sema/Sema.cpp

Removed: 




diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 0d2877efe3ee..b3aeb1018467 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1539,7 +1539,7 @@ class DeferredDiagnosticsEmitter
   }
 
   void checkFunc(SourceLocation Loc, FunctionDecl *FD) {
-auto &Done = DoneMap[InOMPDeviceContext];
+auto &Done = DoneMap[InOMPDeviceContext > 0 ? 1 : 0];
 FunctionDecl *Caller = UsePath.empty() ? nullptr : UsePath.back();
 if ((!ShouldEmitRootNode && !S.getLangOpts().OpenMP && !Caller) ||
 S.shouldIgnoreInHostDeviceCheck(FD) || InUsePath.count(FD))

diff  --git a/clang/test/OpenMP/deferred-diags.cpp 
b/clang/test/OpenMP/deferred-diags.cpp
new file mode 100644
index ..98c28aff644a
--- /dev/null
+++ b/clang/test/OpenMP/deferred-diags.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64 -verify=expected,dev -std=c++11\
+// RUN:-verify-ignore-unexpected=note \
+// RUN:-fopenmp -fopenmp-version=50 -o - %s
+
+// expected-no-diagnostics
+
+// Test no infinite recursion in DeferredDiagnosticEmitter.
+constexpr int foo(int *x) {
+  return 0;
+}
+
+int a = foo(&a);
+
+// Test no crash when both caller and callee have target directives.
+int foo();
+
+class B {
+public:
+  void barB(int *isHost) {
+  #pragma omp target map(tofrom: isHost)
+ {
+   *isHost = foo();
+ }
+  }
+};
+
+class A : public B {
+public:
+  void barA(int *isHost) {
+  #pragma omp target map(tofrom: isHost)
+ {
+   barB(isHost);
+ }
+  }
+};



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:483
+__OMP_RTL(__kmpc_push_num_teams, false, /* Void? */ Int32, IdentPtr, Int32, 
+  Int32, Int32)
 

For this one there's conflicting information on the return value. Clang expects 
this in its checks
// CK1: {{%.+}} = call i32 @__kmpc_push_num_teams({{.+}}, {{.+}}, i32 
[[TE_VAL]], i32 [[TH_VAL]])

But when I look at the runtime I find this 
void __kmpc_push_num_teams(ident_t *loc, kmp_int32 global_tid,
   kmp_int32 num_teams, kmp_int32 num_threads) {



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

https://reviews.llvm.org/D80222



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

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

Thanks for the review.
I've enabled the diagnostics for all the modes.
I also applied the refactoring suggested by @rjmccall. Hopefully I understand 
it correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I do agree with the feature request, but I'm not sure about the implementation. 
It doesn't seem to work well with the cross-compiling support in the driver as 
clearly shown by the amount of tests that need patching. Is cross-compiling a 
concern for you at all? Otherwise I would suggest going a slightly different 
route and use default values iff no "-target" or "target-command" mechanism is 
active. Might need a way to temporarily disable unused flag warnings, but that 
way would be pretty much toolchain independent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2197
+const Expr *ProbArg = E->getArg(2);
+ProbArg->EvaluateAsFloat(Probability, CGM.getContext());
+llvm::Type *Ty = ConvertType(ProbArg->getType());

You likely need to assert on the return value (as Richard suggested).



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2202
+// won't use it for anything.
+// Note, we still IRGen ExpectedValue because it could have side-effects.
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)

I believe in EvaluateAsFloat you need to pass  'allow side effects', because by 
default it does not.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  return RValue::get(ArgValue);
+

Do you still need to evaluate this for side-effects even when called with O1?  
I think this code only evaluates side-effects in O0 mode at the moment.



Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+const Expr *ProbArg = TheCall->getArg(2);
+if (ProbArg->isValueDependent())
+  return ExprError();

Why is value-dependent an error?  The expression should be able to be a 
template parameter.  For example:


struct S {
static constexpr float value = 1.1;
};

template
void f(bool b) {
__builtin_expect_with_probability(b, 1, T::value);
}

should work.

Additionally, in C++20(though not yet implemented in clang it seems):

template
void f(bool b) {
__builtin_expect_with_probability(b, 1, F);
}

Additionally, how about an integer type?  It would seem that I should be able 
ot do:
template
void f(bool b) {
__builtin_expect_with_probability(b, 1, I); // probability is obviously 0 
or 1
}




Comment at: clang/lib/Sema/SemaChecking.cpp:1812
+  Context)) ||
+!Eval.Val.isFloat()) {
+  Diag(ProbArg->getLocStart(), diag::err_probability_not_constant_float)

BTW, this check should be valid (ensuring its a float), since normal 
conversions should happen as a part of the function call.


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

https://reviews.llvm.org/D79830



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

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

[hyd-dev via Phabricator via cfe-commits]

hyd-dev added a comment.

Fix typos in the summary.


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

https://reviews.llvm.org/D79477



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

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

Great! Can someone take care of merging it? I believe I don't have access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80171



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

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ many thanks. I'll land it.


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

https://reviews.llvm.org/D77802



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

> libomp used to pass until I changed it to us getOrCreateRuntimeFunction I 
> think.

Those tests are flaky, I think. Just run them a few times. I don't expect this 
to influence them at all.




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:203
+  static Function *getOrCreateRuntimeFunction(Module &Md,
+  omp::RuntimeFunction FnID);
 

jhuber6 wrote:
> jdoerfert wrote:
> > Nit: M is the commonly used name for a module ;)
> The OpenMPIRBuilder struct has a Module named M so I wasn't sure if I should 
> be shadowing it.
Go with `M`, it's static :)



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:483
+__OMP_RTL(__kmpc_push_num_teams, false, /* Void? */ Int32, IdentPtr, Int32, 
+  Int32, Int32)
 

jhuber6 wrote:
> For this one there's conflicting information on the return value. Clang 
> expects this in its checks
> // CK1: {{%.+}} = call i32 @__kmpc_push_num_teams({{.+}}, {{.+}}, i32 
> [[TE_VAL]], i32 [[TH_VAL]])
> 
> But when I look at the runtime I find this 
> void __kmpc_push_num_teams(ident_t *loc, kmp_int32 global_tid,
>kmp_int32 num_teams, kmp_int32 num_threads) {
> 
Make it void here please, will need to update the tests:
`sed -i -e 's:i32 @__kmpc_push_num_teams:void @__kmpc_push_num_teams:' 
clang/test/OpenMP/*.*`



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:103
+  }
+} else if (Fn->getName() == "__kmpc_fork_teams") {
+  if (!Fn->hasMetadata(LLVMContext::MD_callback)) {

jhuber6 wrote:
> jdoerfert wrote:
> > Please use `FnID` for the comparison.
> Don't know why I didn't think of doing that, will do.
And if the callback is the same for both, merge the conditionals.


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

https://reviews.llvm.org/D80222



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

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

[Ties Stuij via Phabricator via cfe-commits]

stuij marked 6 inline comments as done.
stuij added inline comments.



Comment at: clang/include/clang/Basic/arm_bf16.td:1
+//===--- arm_fp16.td - ARM FP16 compiler interface 
===//
+//

SjoerdMeijer wrote:
> typo: fp16 - > bf16?
> Here, and a few more places, or is it intentional? If so, I guess that can be 
> a bit confusing?
> 
Thanks for that. I went through the patch and I only found the mistake in this 
file.



Comment at: clang/include/clang/Basic/arm_neon_incl.td:293
+
+  string CartesianProductWith = "";
 }

fpetrogalli wrote:
> What is this for?
Thanks, that is actually part of another patch. I will remove.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2198
 
+static void emitNeonTypeDefs(const std::string& types, raw_ostream &OS) {
+  std::string TypedefTypes(types);

fpetrogalli wrote:
> Is this related to the changes for bfloat? Or is it a just a refactoring that 
> it is nice to have? If the latter, please consider submitting it as a 
> separate patch. If both refactoring and BF16 related, at the moment it is not 
> possible to see clearly which changes are BF16 specific, so please do submit 
> the refactoring first.
Yes, related to bfloat. We're emitting that code twice now.

I can make a new patch, but I'm not sure if the effort justifies the in my mind 
small amount of gain in clarity. It's basically just pasting the removed part 
on the left into this function. If you disagree, tell me, and I will create the 
extra patch.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2411
+/// is comprised of type definitions and function declarations.
+void NeonEmitter::runFP16(raw_ostream &OS) {
+  OS << "/*=== arm_fp16.h - ARM FP16 intrinsics "

SjoerdMeijer wrote:
> I am a bit confused here, we already have a runFP16, I am missing something?
The fairly similar runBF16 fn was added. The way git interprets this is 
probably a bit confusing. There's still just one runFP16 function.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2416
+" *\n"
+" * Permission is hereby granted, free of charge, to any person "
+"obtaining a copy\n"

SjoerdMeijer wrote:
> I can't remember the outcome, but I had a discussion with @sdesmalen about 
> this license, if this should be the new or old copyright notice. I believe, 
> but am not certain, that this should be the new one.
I'll change it, and will ping @sdesmalen to be sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708



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

[PATCH] D80362: [WebAssembly] Warn on exception spec only when Wasm EH is used

[Derek Schuff via Phabricator via cfe-commits]

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

otherwise LGTM




Comment at: clang/docs/DiagnosticsReference.rst:14018
++---+
+|:warning:`warning:` |nbsp| :diagtext:`dynamic exception specifications with 
types are currently ignored in wasm exception handling`|
++---+

I think it's not actually necessary to change the text of the warning. Since 
the warning is about an exception handling language feature, I think adding 
"exception handling" on the end doesn't make it any more clear and sounds a 
little redundant to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80362



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

[PATCH] D79993: Place control block after AST block in PCM files (Patch series 1/3)

[Daniel Grumberg via Phabricator via cfe-commits]

dang abandoned this revision.
dang added a comment.
Herald added a subscriber: sstefan1.

After some further discussion offline with @Bigcheese the cleaner course of 
action is to make the AST block relocatable see revision D80383 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79993



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

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> As @MyDeveloperDay said, I'd like it mean C++ only. I find it confusing that 
> it means C++ or ObjC (even if the latter is a superset of the former). I'd 
> rather see it spelt as `isCppOrObjC()` even if it's verbose but at least 
> removes all confusion IMO.

I'm kind of with you on `isCppOrObjC()`


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

https://reviews.llvm.org/D80079



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

[PATCH] D79998: Add AST_SIGNATURE record to unhashed control block of pcm files (Patch series 2/3)

[Daniel Grumberg via Phabricator via cfe-commits]

dang abandoned this revision.
dang added a comment.

This revision is superseded by D80383 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79998



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added a reviewer: Bigcheese.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

This record is constructed by hashing the bytes of the AST block in a similiar
fashion to the SIGNATURE record. This new signature only means anything if the
AST block is fully relocatable, i.e. it does not embed absolute offsets within
the PCM file. This change ensure this does not happen by repalcing these offsets
with offsets relative to the start of the AST block.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80383

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h

Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t1 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t1.dump
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t2 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t2.dump
+// RUN: cat %t1.dump %t2.dump | FileCheck %s
+
+#include "my_header_2.h"
+
+my_int var = 42;
+
+// CHECK: [[SIGNATURE:]]
+// CHECK: [[SIGNATURE]]
+// The modules built by this test are designed to yield the same AST. If this
+// test fails, it means that the AST block is has become non-relocatable.
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2428,7 +2428,7 @@
   W.Visit(D);
 
   // Emit this declaration to the bitstream.
-  uint64_t Offset = W.Emit(D);
+  uint64_t Offset = W.Emit(D) - ASTBlockRange.first;
 
   // Record the offset for this declaration
   SourceLocation Loc = D->getLocation();
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -65,6 +65,7 @@
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "clang/Serialization/ModuleFile.h"
@@ -960,6 +961,7 @@
 
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
+  RECORD(AST_SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
@@ -1025,22 +1027,38 @@
   return Filename + Pos;
 }
 
-ASTFileSignature ASTWriter::createSignature(StringRef Bytes) {
-  // Calculate the hash till start of UNHASHED_CONTROL_BLOCK.
+std::pair
+ASTWriter::createSignature(StringRef AllBytes, StringRef ASTBlockBytes) {
+  // Calculate the hash of the AST block
   llvm::SHA1 Hasher;
-  Hasher.update(ArrayRef(Bytes.bytes_begin(), Bytes.size()));
+  Hasher.update(ASTBlockBytes);
   auto Hash = Hasher.result();
 
   // Convert to an array [5*i32].
-  ASTFileSignature Signature;
   auto LShift = [&](unsigned char Val, unsigned Shift) {
 return (uint32_t)Val << Shift;
   };
+
+  ASTFileSignature ASTSignature;
+  for (int 

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added inline comments.



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2198
 
+static void emitNeonTypeDefs(const std::string& types, raw_ostream &OS) {
+  std::string TypedefTypes(types);

stuij wrote:
> fpetrogalli wrote:
> > Is this related to the changes for bfloat? Or is it a just a refactoring 
> > that it is nice to have? If the latter, please consider submitting it as a 
> > separate patch. If both refactoring and BF16 related, at the moment it is 
> > not possible to see clearly which changes are BF16 specific, so please do 
> > submit the refactoring first.
> Yes, related to bfloat. We're emitting that code twice now.
> 
> I can make a new patch, but I'm not sure if the effort justifies the in my 
> mind small amount of gain in clarity. It's basically just pasting the removed 
> part on the left into this function. If you disagree, tell me, and I will 
> create the extra patch.
Thank you. I didn't notice you were invoking this twice. Make sense to have it 
in a separate function, in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[Francesco Petrogalli via Phabricator via cfe-commits]

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

LGTM, it would be great if you could address the two comments I added, before 
submitting.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10775
 
-  llvm::VectorType *MaskTy = llvm::VectorType::get(CGF.Builder.getInt1Ty(),
- cast(Mask->getType())->getBitWidth());
+  llvm::VectorType *MaskTy = llvm::FixedVectorType::get(
+  CGF.Builder.getInt1Ty(),

`auto`?



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4670
 if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  auto Vec4Ty = llvm::VectorType::get(
+  auto Vec4Ty = llvm::FixedVectorType::get(
   cast(DstTy)->getElementType(), 4);

`auto *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 265524.
stuij added a comment.

addressed review comments, most of all changed license header on the generated 
bfloat file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708

Files:
  clang/include/clang/Basic/arm_bf16.td
  clang/include/clang/Basic/arm_neon_incl.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/test/Preprocessor/aarch64-target-features.c
  clang/test/Preprocessor/arm-target-features.c
  clang/utils/TableGen/NeonEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -85,6 +85,7 @@
 
 void EmitNeon(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitFP16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitBF16(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeonSema(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeonTest(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 void EmitNeon2(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -63,6 +63,7 @@
   GenClangOpenCLBuiltins,
   GenArmNeon,
   GenArmFP16,
+  GenArmBF16,
   GenArmNeonSema,
   GenArmNeonTest,
   GenArmMveHeader,
@@ -186,6 +187,7 @@
"Generate OpenCL builtin declaration handlers"),
 clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"),
 clEnumValN(GenArmFP16, "gen-arm-fp16", "Generate arm_fp16.h for clang"),
+clEnumValN(GenArmBF16, "gen-arm-bf16", "Generate arm_bf16.h for clang"),
 clEnumValN(GenArmNeonSema, "gen-arm-neon-sema",
"Generate ARM NEON sema support for clang"),
 clEnumValN(GenArmNeonTest, "gen-arm-neon-test",
@@ -360,6 +362,9 @@
   case GenArmFP16:
 EmitFP16(Records, OS);
 break;
+  case GenArmBF16:
+EmitBF16(Records, OS);
+break;
   case GenArmNeonSema:
 EmitNeonSema(Records, OS);
 break;
Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -99,7 +99,8 @@
   Poly128,
   Float16,
   Float32,
-  Float64
+  Float64,
+  BFloat16
 };
 
 } // end namespace NeonTypeFlags
@@ -147,6 +148,7 @@
 SInt,
 UInt,
 Poly,
+BFloat16,
   };
   TypeKind Kind;
   bool Immediate, Constant, Pointer;
@@ -199,6 +201,7 @@
   bool isInt() const { return isInteger() && ElementBitwidth == 32; }
   bool isLong() const { return isInteger() && ElementBitwidth == 64; }
   bool isVoid() const { return Kind == Void; }
+  bool isBFloat16() const { return Kind == BFloat16; }
   unsigned getNumElements() const { return Bitwidth / ElementBitwidth; }
   unsigned getSizeInBits() const { return Bitwidth; }
   unsigned getElementSizeInBits() const { return ElementBitwidth; }
@@ -585,8 +588,11 @@
   // runFP16 - Emit arm_fp16.h.inc
   void runFP16(raw_ostream &o);
 
-  // runHeader - Emit all the __builtin prototypes used in arm_neon.h
-	// and arm_fp16.h
+  // runBF16 - Emit arm_bf16.h.inc
+  void runBF16(raw_ostream &o);
+
+  // runHeader - Emit all the __builtin prototypes used in arm_neon.h,
+  // arm_fp16.h and arm_bf16.h
   void runHeader(raw_ostream &o);
 
   // runTests - Emit tests for all the Neon intrinsics.
@@ -611,6 +617,8 @@
 S += "poly";
   else if (isFloating())
 S += "float";
+  else if (isBFloat16())
+S += "bfloat";
   else
 S += "int";
 
@@ -650,7 +658,10 @@
 case 128: S += "LLLi"; break;
 default: llvm_unreachable("Unhandled case!");
 }
-  else
+  else if (isBFloat16()) {
+assert(ElementBitwidth == 16 && "BFloat16 can only be 16 bits");
+S += "y";
+  } else
 switch (ElementBitwidth) {
 case 16: S += "h"; break;
 case 32: S += "f"; break;
@@ -704,6 +715,11 @@
 Base = (unsigned)NeonTypeFlags::Float16 + (Addend - 1);
   }
 
+  if (isBFloat16()) {
+assert(Addend == 1 && "BFloat16 is only 16 bit");
+Base = (unsigned)NeonTypeFlags::BFloat16;
+  }
+
   if (Bitwidth == 128)
 Base |= (unsigned)NeonTypeFlags::QuadFlag;
   if (isInteger() && !isSigned())
@@ -727,6 +743,9 @@
   } else if (Name.startswith("poly")) {
 T.Kind = Poly;
 Name = Name.drop_front(4);
+  } else if (Name.startswith("bfloat")) {
+T.Kind = BFloat16;
+Name = Name.drop_front(6);
   } else {
 assert(Name.startswith("int"));
 Name = Name.drop_front(3);
@@ -825,6 +844,10 @@
   if (isPoly())
 NumVectors = 0;
   break;
+case 'b':
+

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D80323#2048457 , @rjmccall wrote:

> I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> really invasive change for pretty minimal benefit.  Why not leave 
> `VectorType::get` as meaning a non-scalable vector type and come up with a 
> different method name to get a scalable vector?


Sorry @rjmccall , I saw your comments only after approving this patch. I didn't 
mean to enforce my views over yours.

@ctetreau , please make sure you address @rjmccall comment before submitting.

Kind regards,

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80295: [Sema] Diagnose static data members in classes nested in unnamed classes

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:6904
 << Name << RD->getTagKind();
   Invalid = true;
+} else if (RD->isLocalClass()) {

john.brawn wrote:
> rjmccall wrote:
> > This diagnostic actually ignores the tag kind that passed down to it, which 
> > should be fixed.  Also, you should pass in the tag kind for the actual 
> > anonymous class you found.
> > 
> > While I'm looking at this code: `isLocalClass` is mis-designed and doesn't 
> > work for any of our non-`FunctionDecl` local contexts.  This check should 
> > be `if (RD->getParentFunctionOrMethod())`.
> > This diagnostic actually ignores the tag kind that passed down to it, which 
> > should be fixed. Also, you should pass in the tag kind for the actual 
> > anonymous class you found.
> 
> Will do.
> 
> > While I'm looking at this code: isLocalClass is mis-designed and doesn't 
> > work for any of our non-FunctionDecl local contexts. This check should be 
> > if (RD->getParentFunctionOrMethod()).
> 
> CXXMethodDecl has FunctionDecl as a parent class, so isLocalClass will find 
> and return a CXXMethodDecl. Checking it on
> 
> ```
> class Example {
>   void method() {
> class X {
>   static int x;
> };
>   }
> };
> ```
> I get the error as expected. Do you have an example where it doesn't work?
> Will do.

You need to also update the diagnostic to actually care about this.

>  Do you have an example where it doesn't work?

All of the standard C/C++ local contexts are FunctionDecls, but "blocks", ObjC 
methods, and OpenMP captured statements aren't.  The simplest example would be 
(under `-fblocks`):

```
void test() {
  ^{
class X {
  static int x;
};
  }();
}
```


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

https://reviews.llvm.org/D80295



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

[PATCH] D79708: [clang][BFloat] add NEON emitter for bfloat

[Ties Stuij via Phabricator via cfe-commits]

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



Comment at: clang/utils/TableGen/NeonEmitter.cpp:2416
+" *\n"
+" * Permission is hereby granted, free of charge, to any person "
+"obtaining a copy\n"

stuij wrote:
> SjoerdMeijer wrote:
> > I can't remember the outcome, but I had a discussion with @sdesmalen about 
> > this license, if this should be the new or old copyright notice. I believe, 
> > but am not certain, that this should be the new one.
> I'll change it, and will ping @sdesmalen to be sure.
Just to be clear: I've changed the BF16 text, not the FP16 text, which should 
stay the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79708



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Zhi Zhuang via Phabricator via cfe-commits]

LukeZhuang added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  return RValue::get(ArgValue);
+

erichkeane wrote:
> Do you still need to evaluate this for side-effects even when called with O1? 
>  I think this code only evaluates side-effects in O0 mode at the moment.
Hi, sorry I am not sure about it. Since this part of code is after evaluating 
all three arguments(including evaluate `ProbArg` with allowing side effect), I 
think it will evaluate `ProbArg` with side effect whatever the optimization 
level is. This part of code is just early return the value of first argument 
`ArgValue` and do not generate code to backend. Do I correctly understand this? 
Thank you!



Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+const Expr *ProbArg = TheCall->getArg(2);
+if (ProbArg->isValueDependent())
+  return ExprError();

erichkeane wrote:
> Why is value-dependent an error?  The expression should be able to be a 
> template parameter.  For example:
> 
> 
> struct S {
> static constexpr float value = 1.1;
> };
> 
> template
> void f(bool b) {
> __builtin_expect_with_probability(b, 1, T::value);
> }
> 
> should work.
> 
> Additionally, in C++20(though not yet implemented in clang it seems):
> 
> template
> void f(bool b) {
> __builtin_expect_with_probability(b, 1, F);
> }
> 
> Additionally, how about an integer type?  It would seem that I should be able 
> ot do:
> template
> void f(bool b) {
> __builtin_expect_with_probability(b, 1, I); // probability is obviously 0 
> or 1
> }
> 
Hi, this code is based on Richard's suggestion that checking `ProbArg` is not 
value-dependent before evaluate. I also saw `EvaluateAsFloat` source code used 
in CGBuiltin.cpp that it firstly assert the expression is not value-dependent 
as following:
```
bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
SideEffectsKind AllowSideEffects,
bool InConstantContext) const {
   assert(!isValueDependent() &&
  "Expression evaluator can't be called on a dependent expression.");
```
In fact I am not sure is there anything special should be done when is 
value-dependent. Do you have suggestions about this? Thank you!


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

https://reviews.llvm.org/D79830



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 265530.
dang added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, ormris.

Formatting fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80383

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h

Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t1 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t1.dump
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t2 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t2.dump
+// RUN: cat %t1.dump %t2.dump | FileCheck %s
+
+#include "my_header_2.h"
+
+my_int var = 42;
+
+// CHECK: [[SIGNATURE:]]
+// CHECK: [[SIGNATURE]]
+// The modules built by this test are designed to yield the same AST. If this
+// test fails, it means that the AST block is has become non-relocatable.
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2428,7 +2428,7 @@
   W.Visit(D);
 
   // Emit this declaration to the bitstream.
-  uint64_t Offset = W.Emit(D);
+  uint64_t Offset = W.Emit(D) - ASTBlockRange.first;
 
   // Record the offset for this declaration
   SourceLocation Loc = D->getLocation();
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -10,14 +10,12 @@
 //
 //===--===//
 
-#include "clang/AST/OpenMPClause.h"
-#include "clang/Serialization/ASTRecordWriter.h"
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "MultiOnDiskHashTable.h"
-#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTUnresolvedSet.h"
+#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -31,6 +29,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -65,7 +64,9 @@
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/ModuleFileExtension.h"
@@ -960,6 +961,7 @@
 
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
+  RECORD(AST_SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
@@ -1025,22 +1027,38 @@
   return Filename + 

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau added a comment.

In D80323#2048457 , @rjmccall wrote:

> I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> really invasive change for pretty minimal benefit.  Why not leave 
> `VectorType::get` as meaning a non-scalable vector type and come up with a 
> different method name to get a scalable vector?


As it stands today, if you call VectorType::get(Ty, N, false), you will get an 
instance of FixedVectorType, via a base VectorType pointer. Currently, you can 
call getNumElements() on a base VectorType and it will work, but the ultimate 
endgame is that getNumElements is going to be removed from base VectorType. 
This change makes it so that you don't have to cast your VectorType object to 
FixedVectorType everywhere.

The overall architecture is that there is a derived type for each sort of 
vector. VectorType is the base class, and the two derived vector types are 
FixedVectorType and ScalableVectorType. I suppose I could have named them 
something like BaseVectorType, VectorType(actually fixed width vector type), 
and ScalableVectorType, but I feel that this is a worse solution because it's 
not obvious by the name what a "VectorType" is. Additionally, naming the types 
this would have broken all code that previously worked for scalable vectors. 
It's not obvious to me which naming scheme would have resulted in less changes 
(changes to rename fixed vectors to FixedVectorType vs changes needed to rename 
all generic code to BaseVectorType and to fix code for scalable vectors), but I 
think the consistency in the naming scheme justifies the path I chose.

For your specific proposal, I think it would be very strange if a function with 
the signature `static FixedVectorType *get(Type *, unsigned)` were a member of 
the base VectorType.

If you'd like to know more about the motivation for this work, here is a link 
to the RFC: http://lists.llvm.org/pipermail/llvm-dev/2020-March/139811.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Please add a C test case just using the address_space attribute.




Comment at: clang/include/clang/AST/Type.h:1069
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();

It's idiomatic to take `QualType` by value rather than `const &`.

Can you rewrite the `PointerType` method to delegate to this?  Assuming it 
isn't completely dead, that is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/include/clang/Format/Format.h:1632
+  bool isCppOnly() const { return Language == LK_Cpp; }
+  bool isObjectiveC() const { return Language == LK_ObjC; }
+  bool isCpp() const { return isCppOnly() || isObjectiveC(); }

curdeius wrote:
> sammccall wrote:
> > Just my 2c - I find the current meaning of isCpp easier to understand, and 
> > would prefer isObjectiveC to mean objective-C/C++. h if it exists.
> > 
> > Reasons:
> >  - this is consistent with LangOptions::CPlusPlus and LangOptions::ObjC
> >  - when checking for C++, also applying these rules to ObjC++ should be the 
> > common/default case, and excluding ObjC++ the special case that justifies 
> > more precise syntax (and honestly, I'd find `isCpp && !isObjC` to carry the 
> > clearest intent in that case). IOW, this seems like it will attract bugs.
> > 
> > > perhaps a better name for isCpp() is isCStyleLanguages()
> > 
> > Clang uses the term "C family languages", and this includes C, C++, ObjC, 
> > ObjC++.
> > If you really want to avoid the conflict between C++ the boolean language 
> > option and C++ the precise language mode, I'd suggest `isPlusPlus()` and 
> > `isObjective()`. But I think consistency with LangOptions is worth more.
> I'd rather go for coherence with `LanguageKind` options and spell it 
> `isObjC()`.
Peanut gallery says: Please be consistent! If you're going to do 
`isCppOrObjC()`, then you should also do `isObjC()`, not `isObjectiveC()`. (Or 
vice versa.)

What about Objective-C++? Are people using `isCppOrObjectiveC()` to mean "well 
actually it's Objective-C++ but we have no LanguageKind for that, oops?"

Re the term "C family languages," I've heard "curly-brace languages" — but that 
would include C#, Java, and JavaScript as well.

It seems to be that the members should be
```
bool isC() const { return Language == LK_Cpp; }  // no LK_C yet
bool isCpp() const { return Language == LK_Cpp; }
bool isObjectiveC() const { return Language == LK_ObjectiveC; }
bool isObjectiveCpp() const { return Language == LK_ObjectiveC; }  // no 
LK_ObjectiveCpp yet
bool includesCpp() const { return isCpp() || is ObjectiveCpp(); }
```
and that most of the callers you're talking about should be using 
`includesCpp()` instead of `isCpp()`.



Comment at: clang/include/clang/Format/Format.h:1635
   bool isCSharp() const { return Language == LK_CSharp; }
+  bool isProtoBuf() const { return Language == LK_Proto; }
+  bool isTableGen() const { return Language == LK_TableGen; }

curdeius wrote:
> sammccall wrote:
> > These functions that don't *even in principle* do more than compare to an 
> > enum seem like extra indirection that hurts understanding of the code (have 
> > to look up what isObjectiveC() does, or have subtle bugs).
> > 
> > I suspect isCSharp() was added due to a misunderstanding of what isCpp() 
> > was for.
> Ditto, maybe `isProto` and `isTextProto`?
If the name of the language is "Protobuf", then the name of the function should 
be `isProtobuf()`.


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

https://reviews.llvm.org/D80079



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

[PATCH] D80383: Add AST_SIGNATURE record to unhashed control block of PCM files

[Daniel Grumberg via Phabricator via cfe-commits]

dang updated this revision to Diff 265535.
dang added a comment.

Accidently deleted a line in the previous update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80383

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Modules/ASTSignature.c
  clang/test/Modules/Inputs/ASTHash/module.modulemap
  clang/test/Modules/Inputs/ASTHash/my_header_1.h
  clang/test/Modules/Inputs/ASTHash/my_header_2.h

Index: clang/test/Modules/Inputs/ASTHash/my_header_2.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_2.h
@@ -0,0 +1,3 @@
+#include "my_header_1.h"
+
+extern my_int var;
Index: clang/test/Modules/Inputs/ASTHash/my_header_1.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/my_header_1.h
@@ -0,0 +1 @@
+typedef int my_int;
Index: clang/test/Modules/Inputs/ASTHash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ASTHash/module.modulemap
@@ -0,0 +1,8 @@
+module MyHeader1 {
+  header "my_header_1.h"
+}
+
+module MyHeader2 {
+  header "my_header_2.h"
+  export *
+}
Index: clang/test/Modules/ASTSignature.c
===
--- /dev/null
+++ clang/test/Modules/ASTSignature.c
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -iquote %S/Inputs/ASTHash/ -fsyntax-only -fmodules \
+// RUN:   -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t1
+// RUN: %clang_cc1 -iquote "/dev/null" -iquote %S/Inputs/ASTHash/ -fsyntax-only \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-strict-context-hash \
+// RUN:   -fmodules-cache-path=%t %s -Rmodule-build 2> %t2
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t1 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t1.dump
+// RUN: sed -n "s/.* building module 'MyHeader2' as '\(.*\)' .*/\1/gp" %t2 \
+// RUN:   | xargs llvm-bcanalyzer --dump --disable-histogram | cat > %t2.dump
+// RUN: cat %t1.dump %t2.dump | FileCheck %s
+
+#include "my_header_2.h"
+
+my_int var = 42;
+
+// CHECK: [[SIGNATURE:]]
+// CHECK: [[SIGNATURE]]
+// The modules built by this test are designed to yield the same AST. If this
+// test fails, it means that the AST block is has become non-relocatable.
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2428,7 +2428,7 @@
   W.Visit(D);
 
   // Emit this declaration to the bitstream.
-  uint64_t Offset = W.Emit(D);
+  uint64_t Offset = W.Emit(D) - ASTBlockRange.first;
 
   // Record the offset for this declaration
   SourceLocation Loc = D->getLocation();
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -10,14 +10,12 @@
 //
 //===--===//
 
-#include "clang/AST/OpenMPClause.h"
-#include "clang/Serialization/ASTRecordWriter.h"
 #include "ASTCommon.h"
 #include "ASTReaderInternals.h"
 #include "MultiOnDiskHashTable.h"
-#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTUnresolvedSet.h"
+#include "clang/AST/AbstractTypeWriter.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -31,6 +29,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/OpenMPClause.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -65,7 +64,9 @@
 #include "clang/Sema/ObjCMethodList.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Weak.h"
+#include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
+#include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Serialization/InMemoryModuleCache.h"
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/ModuleFileExtension.h"
@@ -960,6 +961,7 @@
 
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
+  RECORD(AST_SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
@@ -1025,22 +1027,38 @@
   return Filename + Pos;
 }
 
-ASTFileSignature ASTWriter::createSig

[PATCH] D80251: [X86] Update some av512 shift intrinsics to use "unsigned int" parameter instead of int to match Intel documentaiton

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Can we add -Wsign-conversion checks to the tests? That was mentioned on PR45931


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

https://reviews.llvm.org/D80251



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

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

Hi Owen,
Do you plan to land the functionality for emitting documentation URLs in clang 
too?




Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323
+  // A documentation URL has an ID and path size.
+  if (Record.size() != 2)
+return SDError::MalformedDiagnosticRecord;

I am just wondering what happens with the ID. Shouldn't we pass it too?



Comment at: clang/test/Misc/serialized-diags-doumentation.c:4
+// CHECK: hello.swift:1:1: error: non-nominal type '(Int, Int)' cannot be 
extended [] []
+// CHECK: [Documentation URL: 
/Users/owenvoorhees/Documents/Development/swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/share/doc/swift/diagnostics/nominal-types.md]

Tip - you can use regexes in CHECKs.
https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-regex-matching-syntax

I think we should change this to something like:
```
// CHECK: [Documentation URL: {{.*}}/doc/swift/diagnostics/nominal-types.md]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D80300#2049181 , @joerg wrote:

> It doesn't seem to work well with the cross-compiling support in the driver 
> as clearly shown by the amount of tests that need patching.


I don't see the test changes as reflecting that at all. If a user was able to 
use `--dyld-prefix` with the value being set as the default, then it just 
works. That would depend on the user's environment, which is something that the 
packager/builder of Clang might know something about or have control over.

> I would suggest going a slightly different route and use default values iff 
> no "-target" or "target-command" mechanism is active.

I'm not sure I see enough of a difference between the newly added customization 
points and `DEFAULT_SYSROOT`. The latter does not act conditionally in the 
manner proposed above.

> Might need a way to temporarily disable unused flag warnings, but that way 
> would be pretty much toolchain independent?

Can you elaborate on this? I'm not sure I caught the line of reasoning that led 
to this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2202
+// won't use it for anything.
+// Note, we still IRGen ExpectedValue because it could have side-effects.
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)

erichkeane wrote:
> I believe in EvaluateAsFloat you need to pass  'allow side effects', because 
> by default it does not.
Thinking further, Since it is with constant-expressions, we might consider 
disallowing side effects.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2204
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  return RValue::get(ArgValue);
+

LukeZhuang wrote:
> erichkeane wrote:
> > Do you still need to evaluate this for side-effects even when called with 
> > O1?  I think this code only evaluates side-effects in O0 mode at the moment.
> Hi, sorry I am not sure about it. Since this part of code is after evaluating 
> all three arguments(including evaluate `ProbArg` with allowing side effect), 
> I think it will evaluate `ProbArg` with side effect whatever the optimization 
> level is. This part of code is just early return the value of first argument 
> `ArgValue` and do not generate code to backend. Do I correctly understand 
> this? Thank you!
You're right here, I'd read the comment and skipped that ArgValue was evaluated 
above.



Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+const Expr *ProbArg = TheCall->getArg(2);
+if (ProbArg->isValueDependent())
+  return ExprError();

LukeZhuang wrote:
> erichkeane wrote:
> > Why is value-dependent an error?  The expression should be able to be a 
> > template parameter.  For example:
> > 
> > 
> > struct S {
> > static constexpr float value = 1.1;
> > };
> > 
> > template
> > void f(bool b) {
> > __builtin_expect_with_probability(b, 1, T::value);
> > }
> > 
> > should work.
> > 
> > Additionally, in C++20(though not yet implemented in clang it seems):
> > 
> > template
> > void f(bool b) {
> > __builtin_expect_with_probability(b, 1, F);
> > }
> > 
> > Additionally, how about an integer type?  It would seem that I should be 
> > able ot do:
> > template
> > void f(bool b) {
> > __builtin_expect_with_probability(b, 1, I); // probability is obviously 
> > 0 or 1
> > }
> > 
> Hi, this code is based on Richard's suggestion that checking `ProbArg` is not 
> value-dependent before evaluate. I also saw `EvaluateAsFloat` source code 
> used in CGBuiltin.cpp that it firstly assert the expression is not 
> value-dependent as following:
> ```
> bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
> SideEffectsKind AllowSideEffects,
> bool InConstantContext) const {
>assert(!isValueDependent() &&
>   "Expression evaluator can't be called on a dependent expression.");
> ```
> In fact I am not sure is there anything special should be done when is 
> value-dependent. Do you have suggestions about this? Thank you!
Typically in dependent cases (though this file doesn't deal with the much), we 
just presume the arguments are valid. The values then get checked when 
instantiated.  Tests to show this would also be necessary.


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

https://reviews.llvm.org/D79830



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

[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

[MyDeveloperDay via Phabricator via cfe-commits]

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

I feel like there might something of a concencus forming.. If I take the time 
to redo following the suggestions @sammccall do you think you could live with 
it?




Comment at: clang/include/clang/Format/Format.h:1632
+  bool isCppOnly() const { return Language == LK_Cpp; }
+  bool isObjectiveC() const { return Language == LK_ObjC; }
+  bool isCpp() const { return isCppOnly() || isObjectiveC(); }

Quuxplusone wrote:
> curdeius wrote:
> > sammccall wrote:
> > > Just my 2c - I find the current meaning of isCpp easier to understand, 
> > > and would prefer isObjectiveC to mean objective-C/C++. h if it exists.
> > > 
> > > Reasons:
> > >  - this is consistent with LangOptions::CPlusPlus and LangOptions::ObjC
> > >  - when checking for C++, also applying these rules to ObjC++ should be 
> > > the common/default case, and excluding ObjC++ the special case that 
> > > justifies more precise syntax (and honestly, I'd find `isCpp && !isObjC` 
> > > to carry the clearest intent in that case). IOW, this seems like it will 
> > > attract bugs.
> > > 
> > > > perhaps a better name for isCpp() is isCStyleLanguages()
> > > 
> > > Clang uses the term "C family languages", and this includes C, C++, ObjC, 
> > > ObjC++.
> > > If you really want to avoid the conflict between C++ the boolean language 
> > > option and C++ the precise language mode, I'd suggest `isPlusPlus()` and 
> > > `isObjective()`. But I think consistency with LangOptions is worth more.
> > I'd rather go for coherence with `LanguageKind` options and spell it 
> > `isObjC()`.
> Peanut gallery says: Please be consistent! If you're going to do 
> `isCppOrObjC()`, then you should also do `isObjC()`, not `isObjectiveC()`. 
> (Or vice versa.)
> 
> What about Objective-C++? Are people using `isCppOrObjectiveC()` to mean 
> "well actually it's Objective-C++ but we have no LanguageKind for that, oops?"
> 
> Re the term "C family languages," I've heard "curly-brace languages" — but 
> that would include C#, Java, and JavaScript as well.
> 
> It seems to be that the members should be
> ```
> bool isC() const { return Language == LK_Cpp; }  // no LK_C yet
> bool isCpp() const { return Language == LK_Cpp; }
> bool isObjectiveC() const { return Language == LK_ObjectiveC; }
> bool isObjectiveCpp() const { return Language == LK_ObjectiveC; }  // no 
> LK_ObjectiveCpp yet
> bool includesCpp() const { return isCpp() || is ObjectiveCpp(); }
> ```
> and that most of the callers you're talking about should be using 
> `includesCpp()` instead of `isCpp()`.
So I think I'd refrain from adding the `isC()` and `isObjectiveCpp()` unless 
they were added to clang itself. But if they were then I think I'd be 100% with 
you.

I'm not really a fan of the IsCppOnly() this was really to minimize the initial 
changes to use a function, I actually think 'isCppOrObjC()' is a better choice,

but I think I'd do that transistion from `isCpp -> isCppOrObjC()` and 
`isCppOnly() -> isCpp()` in a separate revision.



Comment at: clang/include/clang/Format/Format.h:1635
   bool isCSharp() const { return Language == LK_CSharp; }
+  bool isProtoBuf() const { return Language == LK_Proto; }
+  bool isTableGen() const { return Language == LK_TableGen; }

Quuxplusone wrote:
> curdeius wrote:
> > sammccall wrote:
> > > These functions that don't *even in principle* do more than compare to an 
> > > enum seem like extra indirection that hurts understanding of the code 
> > > (have to look up what isObjectiveC() does, or have subtle bugs).
> > > 
> > > I suspect isCSharp() was added due to a misunderstanding of what isCpp() 
> > > was for.
> > Ditto, maybe `isProto` and `isTextProto`?
> If the name of the language is "Protobuf", then the name of the function 
> should be `isProtobuf()`.
I'm good with changing it to use the correct for obj `isObjC()` and `isProto()` 
annd `isTextProto()`

I don't really mind what we call them I just don't like the extra visual 
complexity that the `Style.Langauge == LK_XXX` and `Style.Langauge != LK_XXX` 
brings


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

https://reviews.llvm.org/D80079



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

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

It's worrying to me that there number of places in LLVM that at the exact 
argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We 
already claim to support arbitrary values and full paths, but if you specify 
"-fuse-ld=/path/to/lld-link" on Windows today, you end up with different 
behavior than "-fuse-ld=lld" (which gets translated to searching for an 
"lld-link" binary, but also triggers other conditions).

That's not a reason to not make this particular change, but if conditionals on 
the flavor of linker are going to be used, that seems like perhaps a reason why 
we should not accept arbitrary values at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80225



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:1418
+  /// Whether floating point atomic fetch add/sub is supported.
+  virtual bool isFPAtomicFetchAddSubSupported() const { return false; }
+

I think it should be predicated on specific type.
E.g. NVPTX supports atomic ops on fp32 ~everywhere, but fp64 atomic add/sub is 
only supported on newer GPUs.
And then there's fp16...


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

https://reviews.llvm.org/D71726



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau updated this revision to Diff 265542.
ctetreau added a comment.

address code review issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/utils/TableGen/MveEmitter.cpp

Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -300,7 +300,7 @@
 return Element->cNameBase() + "x" + utostr(Lanes);
   }
   std::string llvmName() const override {
-return "llvm::VectorType::get(" + Element->llvmName() + ", " +
+return "llvm::FixedVectorType::get(" + Element->llvmName() + ", " +
utostr(Lanes) + ")";
   }
 
@@ -354,7 +354,7 @@
 // explanation.
 unsigned ModifiedLanes = (Lanes == 2 ? 4 : Lanes);
 
-return "llvm::VectorType::get(Builder.getInt1Ty(), " +
+return "llvm::FixedVectorType::get(Builder.getInt1Ty(), " +
utostr(ModifiedLanes) + ")";
   }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1478,8 +1478,8 @@
   // registers and we need to make sure to pick a type the LLVM
   // backend will like.
   if (Size == 128)
-return ABIArgInfo::getDirect(llvm::VectorType::get(
-  llvm::Type::getInt64Ty(getVMContext()), 2));
+return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+llvm::Type::getInt64Ty(getVMContext()), 2));
 
   // Always return in register if it fits in a general purpose
   // register, or if it is 64 bits and has a single element.
@@ -3122,8 +3122,8 @@
 cast(IRType)->getElementType()->isIntegerTy(128)) {
   // Use a vXi64 vector.
   uint64_t Size = getContext().getTypeSize(Ty);
-  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+Size / 64);
 }
 
 return IRType;
@@ -3138,8 +3138,8 @@
 
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
-  return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
-   Size / 64);
+  return llvm::FixedVectorType::get(llvm::Type::getDoubleTy(getVMContext()),
+Size / 64);
 }
 
 /// BitsContainNoUserData - Return true if the specified [start,end) bit range
@@ -3273,7 +3273,8 @@
   // case.
   if (ContainsFloatAtOffset(IRType, IROffset, getDataLayout()) &&
   ContainsFloatAtOffset(IRType, IROffset+4, getDataLayout()))
-return llvm::VectorType::get(llvm::Type::getFloatTy(getVMContext()), 2);
+return llvm::FixedVectorType::get(llvm::Type::getFloatTy(getVMContext()),
+  2);
 
   return llvm::Type::getDoubleTy(getVMContext());
 }
@@ -4140,8 +4141,8 @@
 
   // Mingw64 GCC returns i128 in XMM0. Coerce to v2i64 to handle that.
   // Clang matches them for compatibility.
-  return ABIArgInfo::getDirect(
-  llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()), 2));
+  return ABIArgInfo::getDirect(llvm::FixedVectorType::get(
+  llvm::Type::getInt64Ty(getVMContext()), 2));
 
 default:
   break;
@@ -5478,13 +5479,13 @@
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 64) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 2);
   return ABIArgInfo::getDirect(ResType);
 }
 if (Size == 128) {
-  llvm::Type *ResType =
-  llvm::VectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
+  auto *ResType =
+  llvm::FixedVectorType::get(llvm::Type::getInt32Ty(getVMContext()), 4);
   return ABIArgInfo::getDirect(ResType);
 }
 return getNaturalAlignIndirect(Ty, /*ByVal=*/false);
@@ -6209,7 +6210,7 @@
 return ABIArgInfo::getDirect(ResType);
   }
   if (Size == 64 || Size == 128) {
-llvm::Type *ResType = llvm::VectorType::get(
+auto *ResType = llvm::FixedVectorType::get(
 llvm::Type::getInt32Ty(getVMContext()), Size / 32);
 return ABIArgInfo::getDirect(ResType);
   }
@@ -6225,7 +6226,7 @@
 // FP16 vectors should be converted to integer vectors
 if (!getTarget().hasLegalHalfType() && containsAnyFP16Vectors(Ty)) {
   uint64_t Size = getContext().getTypeSize(VT);
-  llvm::Type *NewVecTy = llvm::VectorType::get(
+  auto *NewVecTy =

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D80323#2049374 , @ctetreau wrote:

> In D80323#2048457 , @rjmccall wrote:
>
> > I'm sympathetic to wanting to get rid of the boolean flag, but this is a 
> > really invasive change for pretty minimal benefit.  Why not leave 
> > `VectorType::get` as meaning a non-scalable vector type and come up with a 
> > different method name to get a scalable vector?
>
>
> As it stands today, if you call VectorType::get(Ty, N, false), you will get 
> an instance of FixedVectorType, via a base VectorType pointer. Currently, you 
> can call getNumElements() on a base VectorType and it will work, but the 
> ultimate endgame is that getNumElements is going to be removed from base 
> VectorType. This change makes it so that you don't have to cast your 
> VectorType object to FixedVectorType everywhere.
>
> The overall architecture is that there is a derived type for each sort of 
> vector. VectorType is the base class, and the two derived vector types are 
> FixedVectorType and ScalableVectorType. I suppose I could have named them 
> something like BaseVectorType, VectorType(actually fixed width vector type), 
> and ScalableVectorType, but I feel that this is a worse solution because it's 
> not obvious by the name what a "VectorType" is. Additionally, naming the 
> types this would have broken all code that previously worked for scalable 
> vectors. It's not obvious to me which naming scheme would have resulted in 
> less changes (changes to rename fixed vectors to FixedVectorType vs changes 
> needed to rename all generic code to BaseVectorType and to fix code for 
> scalable vectors), but I think the consistency in the naming scheme justifies 
> the path I chose.


Yeah, I understand what you're trying to do.  If I had to guess, I'd say the 
vast majority of existing frontend, optimizer, and backend code does *not* work 
with scalable vectors as a free generalization.  The code that *does* work with 
them needs to at least be audited, and the clearest way of marking that you've 
done that audit would be to change the types.  So this is breaking a ton of 
code, including a lot that's not in-tree and which you are therefore not on the 
hook to update (and it's particularly likely that there's a lot of vector code 
out-of-tree), as well as setting up the "audit polarity" exactly backwards from 
what it should be — all just so that you can use the name `VectorType` for the 
common base type, which is really not much of a win vs. `VectorBaseType` or 
`AnyVectorType` or `AbstractVectorType` or some similar.

The fact is that people come up with ways to generalize the representation all 
the time, and it's great for LLVM to take those.  Sometimes generalizations are 
necessarily disruptive because they're really pointing out a common conflation 
that's dangerous in some way, but in this case it's purely "additive" and 
there's really no good reason that any of the existing code that's happily 
assuming fixed-width vectors should have to change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265547.
bader added a comment.

- Added C test case with address_space attribute.
- Move isAddressSpaceOverlapping from PointerType to QualType.
- Move C++ test case to clang/test/SemaCXX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_spaces.c
  clang/test/SemaCXX/address-space-arithmetic.cpp

Index: clang/test/SemaCXX/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -74,6 +74,10 @@
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
+char* sub(_AS1 char *x,  _AS2 char *y) {
+  return x - y; // expected-error {{arithmetic operation with operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
 struct SomeStruct {
   int a;
   long b;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -11444,8 +11442,7 @@
 if (LCanPointeeTy != RCanPointeeTy) {
   // Treat NULL constant as a special case in OpenCL.
   if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
-const PointerType *LHSPtr = LHSType->castAs();
-if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->castAs())) {
+if (!LCanPointeeTy.isAddressSpaceOverlapping(RCanPointeeTy)) {
   Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSType << RHSType << 0 /* comparison */
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2391,7 +2391,7 @@
 return TC_NotApplicable;
   auto SrcPointeeType = SrcPtrType->getPointeeType();
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
 msg = diag::err_bad_cxx_cast_addr_space_mismatch;
 return TC_Failed;
   }
@@ -2434,9 +2434,9 @@
   const PointerType *SrcPPtr = cast(SrcPtr);
   QualType DestPPointee = DestPPtr->getPointeeType();
   QualType SrcPPointee = SrcPPtr->getPointeeType();
-  if (Nested ? DestPPointee.getAddressSpace() !=
-   SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+  if (Nested
+  ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace()
+  : !DestPPointee.isAddressSpaceOverlapping(SrcPPointee)) {
 Self.Diag(OpRange.getBegin(), DiagID)
 << SrcType << DestType << Sema::AA_Casting
 << SrcExpr.get()->getSourceRange();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,21 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  /// OpenCL C defines conversion rules for pointers to different address spaces
+  /// and notion of overlapping address spaces.
+  /// CL1.1 or CL1.2:
+  ///   address spaces overlap iff they are they same.
+  /// Op

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/Type.h:1069
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();

rjmccall wrote:
> It's idiomatic to take `QualType` by value rather than `const &`.
> 
> Can you rewrite the `PointerType` method to delegate to this?  Assuming it 
> isn't completely dead, that is.
It isn't completely dead, but there were just a few uses of the `PointerType` 
method, so I've updated all of them to avoid code duplication in two classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aprantl, dblaikie, echristo, JDevlieghere, jhenderson, 
probinson, thakis.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

RFC: http://lists.llvm.org/pipermail/cfe-dev/2020-May/065430.html
Agreement from GCC: 
https://sourceware.org/pipermail/gcc-patches/2020-May/545688.html

g_flags_Group options generally don't affect the amount of debugging
information. -gsplit-dwarf is an exception. Its order dependency with
other gN_Group options make it inconvenient in a build system:

- -g0 -gsplit-dwarf -> level 2 -gsplit-dwarf "upgrades" the amount of debugging 
information despite the previous intention (-g0) to drop debugging information
- -g1 -gsplit-dwarf -> level 2 -gsplit-dwarf "upgrades" the amount of debugging 
information.
- If we have a higher-level -gN, -gN -gsplit-dwarf will supposedly decrease the 
amount of debugging information. This happens with GCC -g3.

The non-orthogonality has confused many users. This patch fixes the
problem by dropping the -g2 implication of -gsplit-dwarf. New semantics:

- If there is a g_Group, allow split DWARF. (There are still conditions split 
DWARF is disabled if it is not beneficial: -g0, -gline-directives-only, -g1 
-fno-split-dwarf-inlining)
- Otherwise, no-op.

To restore the original behavior, replace -gsplit-dwarf with -gsplit-dwarf -g.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80391

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c
  clang/test/Driver/fuchsia.c
  clang/test/Driver/split-debug.c

Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -1,61 +1,66 @@
 // Check that we split debug output properly
 //
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -g -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 //
 // CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -c -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=split -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=single -c -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=single -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-ACTIONS-SINGLE-SPLIT < %t %s
 //
 // CHECK-ACTIONS-SINGLE-SPLIT: "-split-dwarf-file" "split-debug.o"
 // CHECK-ACTIONS-SINGLE-SPLIT-NOT: "-split-dwarf-output"
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=single -c -### -o %tfoo.o %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf=single -g -c -### -o %tfoo.o %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SINGLE-SPLIT-FILENAME < %t %s
 //
 // CHECK-SINGLE-SPLIT-FILENAME: "-split-dwarf-file" "{{.*}}foo.o"
 // CHECK-SINGLE-SPLIT-FILENAME-NOT: "-split-dwarf-output"
 
-// RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
+/// -gsplit-dwarf requires -g. It is also a no-op on a non-ELF platform.
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
+// RUN: %clang -target x86_64-macosx -gsplit-dwarf -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
 //
-// CHECK-NO-ACTIONS-NOT: -split-dwarf
+// CHECK-NO-ACTIONS-NOT: "-split-dwarf
 
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -o Bad.x -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g -o Bad.x -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-BAD < %t %s
 //
 // CHECK-BAD-NOT: "Bad.dwo"
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -c -### %s 2> %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
 //
-// RUN: %clang -target x86_64-pc-freebsd12 -gsplit-dwarf -c -### %s 2> %t
+// RUN: %clang -target x86_64-pc-freebsd12 -gsplit-dwarf -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
 //
-// RUN: %clang -target amdgcn-amd-amdhsa -gsplit-dwarf -c -### %s 2> %t
+// RUN: %clang -target amdgcn-amd-amdhsa -gsplit-dwarf -g -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-OPTION < %t %s
 //
 // CHECK-OPTION: "-split-dwarf-file" "split-debug.dwo" "-split-dwarf-output" "split-debug.dwo"
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -S -### %s 2

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Zhi Zhuang via Phabricator via cfe-commits]

LukeZhuang marked 6 inline comments as done.
LukeZhuang added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+const Expr *ProbArg = TheCall->getArg(2);
+if (ProbArg->isValueDependent())
+  return ExprError();

erichkeane wrote:
> LukeZhuang wrote:
> > erichkeane wrote:
> > > Why is value-dependent an error?  The expression should be able to be a 
> > > template parameter.  For example:
> > > 
> > > 
> > > struct S {
> > > static constexpr float value = 1.1;
> > > };
> > > 
> > > template
> > > void f(bool b) {
> > > __builtin_expect_with_probability(b, 1, T::value);
> > > }
> > > 
> > > should work.
> > > 
> > > Additionally, in C++20(though not yet implemented in clang it seems):
> > > 
> > > template
> > > void f(bool b) {
> > > __builtin_expect_with_probability(b, 1, F);
> > > }
> > > 
> > > Additionally, how about an integer type?  It would seem that I should be 
> > > able ot do:
> > > template
> > > void f(bool b) {
> > > __builtin_expect_with_probability(b, 1, I); // probability is 
> > > obviously 0 or 1
> > > }
> > > 
> > Hi, this code is based on Richard's suggestion that checking `ProbArg` is 
> > not value-dependent before evaluate. I also saw `EvaluateAsFloat` source 
> > code used in CGBuiltin.cpp that it firstly assert the expression is not 
> > value-dependent as following:
> > ```
> > bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
> > SideEffectsKind AllowSideEffects,
> > bool InConstantContext) const {
> >assert(!isValueDependent() &&
> >   "Expression evaluator can't be called on a dependent 
> > expression.");
> > ```
> > In fact I am not sure is there anything special should be done when is 
> > value-dependent. Do you have suggestions about this? Thank you!
> Typically in dependent cases (though this file doesn't deal with the much), 
> we just presume the arguments are valid. The values then get checked when 
> instantiated.  Tests to show this would also be necessary.
Hi, is that mean I just not not need this check, and then add test case as you 
suggest to check it does not yield error, is that correct? Thank you!



Comment at: clang/lib/Sema/SemaChecking.cpp:1812
+  Context)) ||
+!Eval.Val.isFloat()) {
+  Diag(ProbArg->getLocStart(), diag::err_probability_not_constant_float)

erichkeane wrote:
> BTW, this check should be valid (ensuring its a float), since normal 
> conversions should happen as a part of the function call.
Do you mean that when customer pass an integer here and make sure this can 
handle? I just test this and find `isFloat()` returns true when this argument 
is `constexpr int`. Seems it is automatically converted here.


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

https://reviews.llvm.org/D79830



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

[PATCH] D80323: [SVE] Eliminate calls to default-false VectorType::get() from Clang

[John McCall via Phabricator via cfe-commits]

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

I've responded to the llvmdev thread, which I missed before.  I would like us 
to hold off on this line or work until we come to a resolution there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80323



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

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I think it's probably best to keep this subject on the cfe-dev thread until 
there's agreement there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80391



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

[PATCH] D79830: Add support of __builtin_expect_with_probability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

FYI: I'm more of a clang contributor, so I'm unable to review the LLVM code, 
hopefully someone will come along who can check on that.




Comment at: clang/lib/Sema/SemaChecking.cpp:1805
+const Expr *ProbArg = TheCall->getArg(2);
+if (ProbArg->isValueDependent())
+  return ExprError();

LukeZhuang wrote:
> erichkeane wrote:
> > LukeZhuang wrote:
> > > erichkeane wrote:
> > > > Why is value-dependent an error?  The expression should be able to be a 
> > > > template parameter.  For example:
> > > > 
> > > > 
> > > > struct S {
> > > > static constexpr float value = 1.1;
> > > > };
> > > > 
> > > > template
> > > > void f(bool b) {
> > > > __builtin_expect_with_probability(b, 1, T::value);
> > > > }
> > > > 
> > > > should work.
> > > > 
> > > > Additionally, in C++20(though not yet implemented in clang it seems):
> > > > 
> > > > template
> > > > void f(bool b) {
> > > > __builtin_expect_with_probability(b, 1, F);
> > > > }
> > > > 
> > > > Additionally, how about an integer type?  It would seem that I should 
> > > > be able ot do:
> > > > template
> > > > void f(bool b) {
> > > > __builtin_expect_with_probability(b, 1, I); // probability is 
> > > > obviously 0 or 1
> > > > }
> > > > 
> > > Hi, this code is based on Richard's suggestion that checking `ProbArg` is 
> > > not value-dependent before evaluate. I also saw `EvaluateAsFloat` source 
> > > code used in CGBuiltin.cpp that it firstly assert the expression is not 
> > > value-dependent as following:
> > > ```
> > > bool Expr::EvaluateAsFloat(APFloat &Result, const ASTContext &Ctx,
> > > SideEffectsKind AllowSideEffects,
> > > bool InConstantContext) const {
> > >assert(!isValueDependent() &&
> > >   "Expression evaluator can't be called on a dependent 
> > > expression.");
> > > ```
> > > In fact I am not sure is there anything special should be done when is 
> > > value-dependent. Do you have suggestions about this? Thank you!
> > Typically in dependent cases (though this file doesn't deal with the much), 
> > we just presume the arguments are valid. The values then get checked when 
> > instantiated.  Tests to show this would also be necessary.
> Hi, is that mean I just not not need this check, and then add test case as 
> you suggest to check it does not yield error, is that correct? Thank you!
Right, exactly.  This code path should end up being evaluated 2x in the case of 
an instantiation, the first time it is with the dependent placeholder types.  
The second time it will be instantiated.  SO if you cannot validate the rules 
against dependent values, you just assume it would work and let the 2nd pass 
catch it.

Include some tests to make sure you catch this right and that this is usable in 
templates.



Comment at: clang/lib/Sema/SemaChecking.cpp:1812
+  Context)) ||
+!Eval.Val.isFloat()) {
+  Diag(ProbArg->getLocStart(), diag::err_probability_not_constant_float)

LukeZhuang wrote:
> erichkeane wrote:
> > BTW, this check should be valid (ensuring its a float), since normal 
> > conversions should happen as a part of the function call.
> Do you mean that when customer pass an integer here and make sure this can 
> handle? I just test this and find `isFloat()` returns true when this argument 
> is `constexpr int`. Seems it is automatically converted here.
Ok, thanks for checking. I was unsure, but I presumed that the implicit 
type-conversion based on the builtin definition would have caused that to 
happen.  


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

https://reviews.llvm.org/D79830



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

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 265562.
cchen added a comment.
Herald added a subscriber: sstefan1.

Remove redundant code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,142 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-64
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+// RUN: %clang_cc1 -DCK19 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s  --check-prefix CK19 --check-prefix CK19-32
+
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// RUN: %clang_cc1 -DCK19 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck --check-prefix SIMD-ONLY19 %s
+// SIMD-ONLY19-NOT: {{__kmpc|__tgt}}
+#ifdef CK19
+
+// CK19: [[STRUCT_DESCRIPTOR:%.+]]  = type { i64, i64, i64 }
+
+// CK19: [[MSIZE:@.+]] = {{.+}}constant [1 x i64] [i64 3]
+// CK19: [[MTYPE:@.+]] = {{.+}}constant [1 x i64] [i64 2081]
+
+// CK19-LABEL: _Z3foo
+void foo(int arg) {
+  int arr[3][4][5];
+
+  // CK19: [[DIMS:%.+]] = alloca [3 x [[STRUCT_DESCRIPTOR]]],
+  // CK19: [[ARRAY_IDX:%.+]] = getelementptr inbounds [3 x [4 x [5 x i32]]], [3 x [4 x [5 x i32]]]* [[ARR:%.+]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_DECAY:%.+]] = getelementptr inbounds [4 x [5 x i32]], [4 x [5 x i32]]* [[ARRAY_IDX]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_1:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_DECAY]], {{.+}}
+  // CK19: [[ARRAY_DECAY_2:%.+]] = getelementptr inbounds [5 x i32], [5 x i32]* [[ARRAY_IDX_1]], {{.+}} 0, {{.+}} 0
+  // CK19: [[ARRAY_IDX_3:%.+]] = getelementptr inbounds {{.+}}, {{.+}}* [[ARRAY_DECAY_2]], {{.+}} 1
+  // CK19: [[LEN:%.+]] = sub nuw i64 4, [[ARG_ADDR:%.+]]
+  // CK19: [[BP0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[BP:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[P0:%.+]] = getelementptr inbounds [1 x i8*], [1 x i8*]* [[P:%.+]], i{{.+}} 0, i{{.+}} 0
+  // CK19: [[DIM_1:%.+]] = getelementptr inbounds [3 x [[STRUCT_DESCRIPTOR]]], [3 x [[STRUCT_DESCRIPTOR]]]* [[DIMS]], {{.+}} 0, {{.+}} 0
+  // CK19: [[OFFSET:%.+]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]], [[STRUCT_DESCRIPTOR]]* [[DIM_1]], {{.+}} 0, {{.+}} 0
+  // CK19: store i64 0, i64* [[OFFSET]],
+  // CK19: [[COUNT:%.+]] = geteleme

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: clang/test/CodeGen/atomic-ops.c:296
+  // CHECK: fsub
+  return __atomic_sub_fetch(p, 1.0, memory_order_relaxed);
+}

yaxunl wrote:
> ldionne wrote:
> > Sorry if that's a dumb question, but I'm a bit confused: `p` is  a 
> > `float*`, but then we add a double `1.0` to it. Is that intended, or should 
> > that be `double *p` instead (or `1.0f`)?
> In this case, the value type is converted to the pointee type of the pointer 
> operand.
Ok, thanks for the clarification. Yeah, it was a dumb question after all. I 
still think it should be made clearer by using `1.0f`.


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

https://reviews.llvm.org/D71726



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

[PATCH] D80222: Replace Clang's createRuntimeFunction with the definitions in OMPKinds.def

[Joseph Huber via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:203
+  static Function *getOrCreateRuntimeFunction(Module &Md,
+  omp::RuntimeFunction FnID);
 

jdoerfert wrote:
> jhuber6 wrote:
> > jdoerfert wrote:
> > > Nit: M is the commonly used name for a module ;)
> > The OpenMPIRBuilder struct has a Module named M so I wasn't sure if I 
> > should be shadowing it.
> Go with `M`, it's static :)
Forgot to write the file when I changed it here, not work updating the diff for 
a single character. I'll put it in the next one.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:244
 
-// TODO: Replace this with the real size_t type
-#define __OMP_SIZE_TYPE(NAME) OMP_TYPE(NAME, Type::getInt64Ty(Ctx))
+#define __OMP_SIZE_TYPE(NAME) OMP_TYPE(NAME, 
M.getDataLayout().getIntPtrType(Ctx))
 __OMP_SIZE_TYPE(SizeTy)

I'm just directly getting the SizeTy from the Module, I'm not sure if this is a 
permanent solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80222



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[John McCall via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/include/clang/AST/Type.h:1069
+  /// qualifiers.
+  bool isAddressSpaceOverlapping(const QualType &T) const {
+Qualifiers Q = getQualifiers();

bader wrote:
> rjmccall wrote:
> > It's idiomatic to take `QualType` by value rather than `const &`.
> > 
> > Can you rewrite the `PointerType` method to delegate to this?  Assuming it 
> > isn't completely dead, that is.
> It isn't completely dead, but there were just a few uses of the `PointerType` 
> method, so I've updated all of them to avoid code duplication in two classes.
Even better, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317



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

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7c7e8a523f5: [Analyzer][WebKit] 
RefCntblBaseVirtualDtorChecker (authored by jkorous).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D77177?vs=263915&id=265565#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77177

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
  clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
  clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
  clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
  clang/test/Analysis/Checkers/WebKit/mock-types.h
  clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
  clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp

Index: clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.WebKitRefCntblBaseVirtualDtor -verify %s
+
+struct RefCntblBase {
+  void ref() {}
+  void deref() {}
+};
+
+struct Derived : RefCntblBase { };
+// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'Derived' but doesn't have virtual destructor}}
+
+struct DerivedWithVirtualDtor : RefCntblBase {
+// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedWithVirtualDtor' but doesn't have virtual destructor}}
+  virtual ~DerivedWithVirtualDtor() {}
+};
+
+
+
+template
+struct DerivedClassTmpl : T { };
+typedef DerivedClassTmpl Foo;
+
+
+
+struct RandomBase {};
+struct RandomDerivedClass : RandomBase { };
+
+
+
+struct FakeRefCntblBase1 {
+  private:
+  void ref() {}
+  void deref() {}
+};
+struct Quiet1 : FakeRefCntblBase1 {};
+
+struct FakeRefCntblBase2 {
+  protected:
+  void ref() {}
+  void deref() {}
+};
+struct Quiet2 : FakeRefCntblBase2 {};
+
+class FakeRefCntblBase3 {
+  void ref() {}
+  void deref() {}
+};
+struct Quiet3 : FakeRefCntblBase3 {};
+struct Quiet4 : private RefCntblBase {};
+class Quiet5 : RefCntblBase {};
+
+void foo () {
+  Derived d;
+}
Index: clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.WebKitRefCntblBaseVirtualDtor -verify %s
+
+struct RefCntblBase {
+  void ref() {}
+  void deref() {}
+};
+
+template
+struct DerivedClassTmpl1 : T { };
+// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl1' but doesn't have virtual destructor}}
+
+DerivedClassTmpl1 a;
+
+
+
+template
+struct DerivedClassTmpl2 : T { };
+// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl2' but doesn't have virtual destructor}}
+
+template int foo(T) { DerivedClassTmpl2 f; return 42; }
+int b = foo(RefCntblBase{});
+
+
+
+template
+struct DerivedClassTmpl3 : T { };
+// expected-warning@-1{{Struct 'RefCntblBase' is used as a base of struct 'DerivedClassTmpl3' but doesn't have virtual destructor}}
+
+typedef DerivedClassTmpl3 Foo;
+Foo c;
Index: clang/test/Analysis/Checkers/WebKit/mock-types.h
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/mock-types.h
@@ -0,0 +1,48 @@
+#ifndef mock_types_1103988513531
+#define mock_types_1103988513531
+
+template  struct Ref {
+  T t;
+
+  Ref() : t{} {};
+  Ref(T *) {}
+  T *get() { return nullptr; }
+  operator const T &() const { return t; }
+  operator T &() { return t; }
+};
+
+template  struct RefPtr {
+  T *t;
+
+  RefPtr() : t(new T) {}
+  RefPtr(T *t) : t(t) {}
+  T *get() { return t; }
+  T *operator->() { return t; }
+  T &operator*() { return *t; }
+  RefPtr &operator=(T *) { return *this; }
+};
+
+template  bool operator==(const RefPtr &, const RefPtr &) {
+  return false;
+}
+
+template  bool operator==(const RefPtr &, T *) { return false; }
+
+template  bool operator==(const RefPtr &, T &) { return false; }
+
+template  bool operator!=(const RefPtr &, const RefPtr &) {
+  return false;
+}
+
+template  bool operator!=(const RefPtr &, T *) { return false; }
+
+template  bool operator!=(const RefPtr &, T &) { return false; }
+
+struct RefCountable {
+  void ref() {}
+  void deref() {}
+};
+
+template  T *downcast(T *t) { return t; }
+
+#endif
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntbl

[clang] f7c7e8a - [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker

[Jan Korous via cfe-commits]

Author: Jan Korous
Date: 2020-05-21T11:54:49-07:00
New Revision: f7c7e8a523f56b0ed1b14c0756ba4e5d1ccb48d2

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

LOG: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker

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

Added: 
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
clang/test/Analysis/Checkers/WebKit/mock-types.h

clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp

Modified: 
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 0bfb6456dc82..79ba8fb18ba8 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1374,6 +1374,33 @@ double freed, or use after freed. This check attempts to 
find such problems.
zx_handle_close(sb);
  }
 
+WebKit
+^^
+
+WebKit is an open-source web browser engine available for macOS, iOS and Linux.
+This section describes checkers that can find issues in WebKit codebase.
+
+Most of the checkers focus on memory management for which WebKit uses custom 
implementation of reference counted smartpointers.
+Checker are formulated in terms related to ref-counting:
+* *Ref-counted type* is either ``Ref`` or ``RefPtr``.
+* *Ref-countable type* is any type that implements ``ref()`` and ``deref()`` 
methods as ``RefPtr<>`` is a template (i. e. relies on duck typing).
+* *Uncounted type* is ref-countable but not ref-counted type.
+
+.. _webkit-WebKitRefCntblBaseVirtualDtor:
+
+webkit.WebKitRefCntblBaseVirtualDtor
+
+All uncounted types used as base classes must have a virtual destructor.
+
+Ref-counted types hold their ref-countable data by a raw pointer and allow 
implicit upcasting from ref-counted pointer to derived type to ref-counted 
pointer to base type. This might lead to an object of (dynamic) derived type 
being deleted via pointer to the base class type which C++ standard defines as 
UB in case the base class doesn't have virtual destructor ``[expr.delete]``.
+
+.. code-block:: cpp
+ struct RefCntblBase {
+   void ref() {}
+   void deref() {}
+ };
+
+ struct Derived : RefCntblBase { }; // warn
 
 .. _alpha-checkers:
 

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 93c4d964d772..ec65afb30dd0 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -116,6 +116,9 @@ def NonDeterminismAlpha : Package<"nondeterminism">, 
ParentPackage;
 def Fuchsia : Package<"fuchsia">;
 def FuchsiaAlpha : Package<"fuchsia">, ParentPackage;
 
+def WebKit : Package<"webkit">;
+def WebKitAlpha : Package<"webkit">, ParentPackage;
+
 
//===--===//
 // Core Checkers.
 
//===--===//
@@ -1620,3 +1623,13 @@ def FuchsiaLockChecker : Checker<"Lock">,
 
 } // end fuchsia
 
+//===--===//
+// WebKit checkers.
+//===--===//
+
+let ParentPackage = WebKit in {
+
+def WebKitRefCntblBaseVirtualDtorChecker : 
Checker<"WebKitRefCntblBaseVirtualDtor">,
+  HelpText<"Check for any ref-countable base class having virtual 
destructor.">,
+  Documentation;
+} // end webkit

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index bcf2dfdb8326..4f885fadf415 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -121,6 +121,8 @@ add_clang_library(clangStaticAnalyzerCheckers
   VLASizeChecker.cpp
   ValistChecker.cpp
   VirtualCallChecker.cpp
+  WebKit/PtrTypesSemantics.cpp
+  WebKit/RefCntblBaseVirtualDtorChecker.cpp
 
   LINK_LIBS
   clangAST

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
new file mode 100644
index ..26d79cfcd9b5
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -0,0 +1,70 @@
+//===- ASTUtis.h -

[PATCH] D66564: [clang-tidy] new altera struct pack align check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies added a comment.

@Eugene.Zelenko Just checking in, is there anything I missed regarding what we 
need to do for these checks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66564



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

[PATCH] D76077: [ARM] Add __bf16 as new Bfloat16 C Type

[Luke Geeson via Phabricator via cfe-commits]

LukeGeeson added inline comments.



Comment at: clang/test/CodeGen/arm-mangle-16bit-float.cpp:4
+
+// CHECK64: define {{.*}}void @_Z3foou6__bf16(half %b)
+// CHECK32: define {{.*}}void @_Z3foou6__bf16(i32 %b.coerce)

SjoerdMeijer wrote:
> LukeGeeson wrote:
> > craig.topper wrote:
> > > How can bfloat16 be passed as half? Don't they have a different format?
> > see the above comment about soft-abis
> Probably I am bit confused too now... are the three possible types that we 
> are expecing not bfloat, i32, or i16? 
Hi Sjoerd, Good spot here I forgot to add a default case somewhere, which means 
AArch32 didn't observe `-mfloat-abi softfp` by default - and hence used bfloat 
where i32 was expected. 

I have a local patch for this and will send to Ties to merge into this one :) 


We're not sure what you mean by i16 however?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76077



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

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

I see that more as a short-coming in the existing DEFAULT_SYSROOT behavior and 
less an argument for making more cases like it. So the general idea is that for 
turnkey toolchains,
we want to allow customizing the "default" target of the toolchain to hard-code 
options like --sysroot, --target, -Wl,-rpath etc. Those are all related, so 
when using a different target, they no longer make sense. One way to deal with 
all those options in a consistent manner is hook into the logic for determining 
the current target, if none is explicitly specified on the command line or 
implicit from the executable name, then prepend some additional flags on the 
command line based on some cmake variable or so. This flags shouldn't trigger 
the unused argument warnings, so you can always pass -Wl,-rpath, --sysroot etc, 
independent of whether the compiler is in preprocessing / compile / assemble / 
link mode. That seems to be a more general and systematic approach than adding 
many additional build-time options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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

[PATCH] D56456: [Driver] Default to -fno-addrsig on Android.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Do we still need this? I was surprised by clang not using faddrsing on Andriod 
by default.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56456



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

[clang] 1108f5c - Revert "[Analyzer][WebKit] RefCntblBaseVirtualDtorChecker"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-05-21T15:49:46-04:00
New Revision: 1108f5c737dbdab0277874a7e5b237491839c43a

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

LOG: Revert "[Analyzer][WebKit] RefCntblBaseVirtualDtorChecker"

This reverts commit f7c7e8a523f56b0ed1b14c0756ba4e5d1ccb48d2.
Breaks build everywhere.

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

Removed: 
clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.cpp
clang/lib/StaticAnalyzer/Checkers/WebKit/PtrTypesSemantics.h
clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
clang/test/Analysis/Checkers/WebKit/mock-types.h

clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor-templates.cpp
clang/test/Analysis/Checkers/WebKit/ref-cntbl-base-virtual-dtor.cpp



diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 79ba8fb18ba8..0bfb6456dc82 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1374,33 +1374,6 @@ double freed, or use after freed. This check attempts to 
find such problems.
zx_handle_close(sb);
  }
 
-WebKit
-^^
-
-WebKit is an open-source web browser engine available for macOS, iOS and Linux.
-This section describes checkers that can find issues in WebKit codebase.
-
-Most of the checkers focus on memory management for which WebKit uses custom 
implementation of reference counted smartpointers.
-Checker are formulated in terms related to ref-counting:
-* *Ref-counted type* is either ``Ref`` or ``RefPtr``.
-* *Ref-countable type* is any type that implements ``ref()`` and ``deref()`` 
methods as ``RefPtr<>`` is a template (i. e. relies on duck typing).
-* *Uncounted type* is ref-countable but not ref-counted type.
-
-.. _webkit-WebKitRefCntblBaseVirtualDtor:
-
-webkit.WebKitRefCntblBaseVirtualDtor
-
-All uncounted types used as base classes must have a virtual destructor.
-
-Ref-counted types hold their ref-countable data by a raw pointer and allow 
implicit upcasting from ref-counted pointer to derived type to ref-counted 
pointer to base type. This might lead to an object of (dynamic) derived type 
being deleted via pointer to the base class type which C++ standard defines as 
UB in case the base class doesn't have virtual destructor ``[expr.delete]``.
-
-.. code-block:: cpp
- struct RefCntblBase {
-   void ref() {}
-   void deref() {}
- };
-
- struct Derived : RefCntblBase { }; // warn
 
 .. _alpha-checkers:
 

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index ec65afb30dd0..93c4d964d772 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -116,9 +116,6 @@ def NonDeterminismAlpha : Package<"nondeterminism">, 
ParentPackage;
 def Fuchsia : Package<"fuchsia">;
 def FuchsiaAlpha : Package<"fuchsia">, ParentPackage;
 
-def WebKit : Package<"webkit">;
-def WebKitAlpha : Package<"webkit">, ParentPackage;
-
 
//===--===//
 // Core Checkers.
 
//===--===//
@@ -1623,13 +1620,3 @@ def FuchsiaLockChecker : Checker<"Lock">,
 
 } // end fuchsia
 
-//===--===//
-// WebKit checkers.
-//===--===//
-
-let ParentPackage = WebKit in {
-
-def WebKitRefCntblBaseVirtualDtorChecker : 
Checker<"WebKitRefCntblBaseVirtualDtor">,
-  HelpText<"Check for any ref-countable base class having virtual 
destructor.">,
-  Documentation;
-} // end webkit

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4f885fadf415..bcf2dfdb8326 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -121,8 +121,6 @@ add_clang_library(clangStaticAnalyzerCheckers
   VLASizeChecker.cpp
   ValistChecker.cpp
   VirtualCallChecker.cpp
-  WebKit/PtrTypesSemantics.cpp
-  WebKit/RefCntblBaseVirtualDtorChecker.cpp
 
   LINK_LIBS
   clangAST

diff  --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
deleted file mode 100644
index 26d79cfcd9b5..
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ /dev/null
@@ -1,70 +0

[PATCH] D56456: [Driver] Default to -fno-addrsig on Android.

[Dan Albert via Phabricator via cfe-commits]

danalbert added a comment.

The NDK still supports linkers other than LLD, but we are changing the default 
to LLD in the next release. I'd prefer to keep this for the time being, but 
don't feel strongly about it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56456



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

[PATCH] D80317: [SYCL] Prohibit arithmetic operations for incompatible pointers

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 265579.
bader marked an inline comment as done.
bader added a comment.

Fix formatting in clang/test/Sema/address_spaces.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80317

Files:
  clang/include/clang/AST/Type.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/address_spaces.c
  clang/test/SemaCXX/address-space-arithmetic.cpp

Index: clang/test/SemaCXX/address-space-arithmetic.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-space-arithmetic.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int *foo(__attribute__((opencl_private)) int *p,
+ __attribute__((opencl_local)) int *l) {
+  return p - l; // expected-error {{arithmetic operation with operands of type  ('__private int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -74,6 +74,10 @@
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
+char *sub(_AS1 char *x, _AS2 char *y) {
+  return x - y; // expected-error {{arithmetic operation with operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
+}
+
 struct SomeStruct {
   int a;
   long b;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10087,10 +10087,8 @@
   if (isRHSPointer) RHSPointeeTy = RHSExpr->getType()->getPointeeType();
 
   // if both are pointers check if operation is valid wrt address spaces
-  if (S.getLangOpts().OpenCL && isLHSPointer && isRHSPointer) {
-const PointerType *lhsPtr = LHSExpr->getType()->castAs();
-const PointerType *rhsPtr = RHSExpr->getType()->castAs();
-if (!lhsPtr->isAddressSpaceOverlapping(*rhsPtr)) {
+  if (isLHSPointer && isRHSPointer) {
+if (!LHSPointeeTy.isAddressSpaceOverlapping(RHSPointeeTy)) {
   S.Diag(Loc,
  diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSExpr->getType() << RHSExpr->getType() << 1 /*arithmetic op*/
@@ -11444,8 +11442,7 @@
 if (LCanPointeeTy != RCanPointeeTy) {
   // Treat NULL constant as a special case in OpenCL.
   if (getLangOpts().OpenCL && !LHSIsNull && !RHSIsNull) {
-const PointerType *LHSPtr = LHSType->castAs();
-if (!LHSPtr->isAddressSpaceOverlapping(*RHSType->castAs())) {
+if (!LCanPointeeTy.isAddressSpaceOverlapping(RCanPointeeTy)) {
   Diag(Loc,
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
   << LHSType << RHSType << 0 /* comparison */
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2391,7 +2391,7 @@
 return TC_NotApplicable;
   auto SrcPointeeType = SrcPtrType->getPointeeType();
   auto DestPointeeType = DestPtrType->getPointeeType();
-  if (!DestPtrType->isAddressSpaceOverlapping(*SrcPtrType)) {
+  if (!DestPointeeType.isAddressSpaceOverlapping(SrcPointeeType)) {
 msg = diag::err_bad_cxx_cast_addr_space_mismatch;
 return TC_Failed;
   }
@@ -2434,9 +2434,9 @@
   const PointerType *SrcPPtr = cast(SrcPtr);
   QualType DestPPointee = DestPPtr->getPointeeType();
   QualType SrcPPointee = SrcPPtr->getPointeeType();
-  if (Nested ? DestPPointee.getAddressSpace() !=
-   SrcPPointee.getAddressSpace()
- : !DestPPtr->isAddressSpaceOverlapping(*SrcPPtr)) {
+  if (Nested
+  ? DestPPointee.getAddressSpace() != SrcPPointee.getAddressSpace()
+  : !DestPPointee.isAddressSpaceOverlapping(SrcPPointee)) {
 Self.Diag(OpRange.getBegin(), DiagID)
 << SrcType << DestType << Sema::AA_Casting
 << SrcExpr.get()->getSourceRange();
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1064,6 +1064,21 @@
   /// Return the address space of this type.
   inline LangAS getAddressSpace() const;
 
+  /// Returns true if address space qualifiers overlap with T address space
+  /// qualifiers.
+  /// OpenCL C defines conversion rules for pointers to different address spaces
+  /// and notion of overlapping address spaces.
+  /// CL1.1 or CL1.2:
+  ///   address spaces overlap iff they are they same.
+  /// OpenCL C v2.0 s6.5.5 adds:
+  ///   __generic overlaps with any addr